Differential D290

ansible - buildmaster steps to retrieve artifacts from buildslaves
ClosedPublic
Actions

Authored by mkrizek on Feb 23 2015, 12:23 PM.

Details

Reviewers
kparal
tflink
Maniphest Tasks
T401: Add buildmaster steps to retrieve artifacts from buildslaves
Summary

See T401. Created for @mkrizek.

Test Plan

unknown

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
kparal retitled this revision from to ansible - buildmaster steps to retrieve artifacts from buildslaves.Feb 23 2015, 12:23 PM
kparal updated this object.
kparal edited the test plan for this revision. (Show Details)
kparal added a reviewer: tflink.
kparal added a project: infrastructure.
kparal added a subscriber: mkrizek.
kparal added inline comments.Feb 23 2015, 12:36 PM
roles/taskotron/buildmaster-configure/templates/taskotron.master.cfg.j2
176–177

This will effectively overwrite any artifact called taskotron.log that the task could have created with our own log file.

I'm not saying this approach is unreasonable, I just want to make sure it's expected. An alternative approach would be to create a subdirectory for task-generated artifacts, so that we don't need to deal with file name conflicts for taskotrong.log, stdout/stderr files, etc possibly some future files. That has one additional benefit - if everything is in the same directory, it can be a bit of a mess if the task generates tens or hundreds of artifacts. Separating "our" files from task-generated files would look cleaner.

Of course, we would need to come up with a good name for that subdirectory, which is always difficult :-)

kparal updated this object.Feb 23 2015, 12:37 PM
tflink requested changes to this revision.Feb 23 2015, 1:34 PM

Looks pretty good to start with - some small change requests, though

roles/taskotron/buildmaster-configure/templates/taskotron.master.cfg.j2
165

this requires an execdb-style uuid now, doesn't it? Or are there more changes to trigger that aren't submitted for review yet?

173

I'd prefer to see the output dir rendered from a template instead of hard coded - especially if/when we start using it in more places

176–177

output or debug might work. it'd be a good place to put the stdout/stderr output, too if we get that working. Either way, we should document what we're using and state clearly that anything output into a directory of identical name will either be renamed or overwritten.

This revision now requires changes to proceed.Feb 23 2015, 1:34 PM
tflink added inline comments.Feb 23 2015, 1:46 PM
roles/taskotron/buildmaster-configure/templates/taskotron.master.cfg.j2
176–177

It occurs to me that another solution would be to put all of our output into the root /srv/taskotron/artifacts/<uuid> dir and create something like /srv/taskotron/artifacts/<uuid>/task_output` for the artifacts generated by the task. That way, we don't have to worry about any name collisions.

mkrizek commandeered this revision.Feb 23 2015, 1:54 PM
mkrizek added a reviewer: kparal.
mkrizek added inline comments.Feb 23 2015, 2:03 PM
roles/taskotron/buildmaster-configure/templates/taskotron.master.cfg.j2
165

Yes, changes to trigger are needed.

173

Good point, will do, thanks.

176–177

Let's create task_output directory, acks?

kparal added inline comments.Feb 23 2015, 2:04 PM
roles/taskotron/buildmaster-configure/templates/taskotron.master.cfg.j2
176–177

Yes, that's what I had on mind, just not explained it clearly :-) Like this:

/srv/taskotron/artifacts/<uuid>
/srv/taskotron/artifacts/<uuid>/taskotron.log
/srv/taskotron/artifacts/<uuid>/task.log
/srv/taskotron/artifacts/<uuid>/output/
/srv/taskotron/artifacts/<uuid>/output/NVR1.txt
/srv/taskotron/artifacts/<uuid>/output/report.html

The <uuid>/ root dir would be ours, and everything under <uuid>/output/ would be maintained by task authors. Replace output with your preferred subdir name.

mkrizek updated this revision to Diff 789.Feb 23 2015, 4:28 PM
mkrizek updated this revision to Diff 790.Feb 23 2015, 4:32 PM
tflink requested changes to this revision.Feb 23 2015, 4:56 PM

should have noticed this the first time around, but the playbooks would effectively enable artifacts on dev/stg/prod at the same time (since they're run on a regular basis) and I think we should avoid putting new stuff in prod without testing it first

roles/taskotron/buildmaster-configure/templates/taskotron.master.cfg.j2
188

should have noticed this earlier, but this will effectively enable artifacts storage on dev, stg and prod all at the same time. can you do an if/else block to make sure that the new stuff only shows up on dev?

roles/taskotron/buildmaster/tasks/main.yml
26

same as above, can you add a 'when: deployment_type == "dev"' to these to make sure that the new config doesn't make it to production until we're ready?

This revision now requires changes to proceed.Feb 23 2015, 4:56 PM
mkrizek updated this revision to Diff 795.Feb 24 2015, 7:42 AM
tflink accepted this revision.Feb 25 2015, 6:21 AM

Looks good to me

This revision is now accepted and ready to land.Feb 25 2015, 6:21 AM
mkrizek closed this revision.Feb 27 2015, 8:44 AM

Revision Contents

Diff 795

View Options

inventory/group_vars/taskotron-dev

Context not available.
35fakefedorainfra_url: https://taskotron-dev.fedoraproject.org/fakefedorainfra 35​fakefedorainfra_url: https://taskotron-dev.fedoraproject.org/fakefedorainfra
36taskotron_docs_url: https://docs.qadevel.cloud.fedoraproject.org/libtaskotron/latest/ 36​taskotron_docs_url: https://docs.qadevel.cloud.fedoraproject.org/libtaskotron/latest/
37freezes: false 37​freezes: false
38​public_artifacts_dir: /srv/taskotron/artifacts
Context not available.
View Options

inventory/group_vars/taskotron-prod

Context not available.
26deployment_type: prod 26​deployment_type: prod
27tcp_ports: [ 80, 443, "{{ buildslave_port }}" ] 27​tcp_ports: [ 80, 443, "{{ buildslave_port }}" ]
28taskotron_docs_url: https://docs.qadevel.cloud.fedoraproject.org/libtaskotron/latest/ 28​taskotron_docs_url: https://docs.qadevel.cloud.fedoraproject.org/libtaskotron/latest/
29​public_artifacts_dir: /srv/taskotron/artifacts
Context not available.
View Options

inventory/group_vars/taskotron-stg

Context not available.
31fakefedorainfra_url: https://taskotron.stg.fedoraproject.org/fakefedorainfra 31​fakefedorainfra_url: https://taskotron.stg.fedoraproject.org/fakefedorainfra
32taskotron_docs_url: https://docs.qadevel.cloud.fedoraproject.org/libtaskotron/latest/ 32​taskotron_docs_url: https://docs.qadevel.cloud.fedoraproject.org/libtaskotron/latest/
33freezes: false 33​freezes: false
34​public_artifacts_dir: /srv/taskotron/artifacts
Context not available.
View Options

roles/taskotron/buildmaster-configure/templates/taskotron.master.cfg.j2

Context not available.
142from buildbot.steps.shell import ShellCommand 142​from buildbot.steps.shell import ShellCommand
143from buildbot.process.properties import Property, Interpolate 143​from buildbot.process.properties import Property, Interpolate
144from buildbot.steps.slave import RemoveDirectory 144​from buildbot.steps.slave import RemoveDirectory
145​from buildbot.steps.transfer import DirectoryUpload, FileUpload
145 146​
146factory = BuildFactory() 147​factory = BuildFactory()
147 148​
Context not available.
161 Interpolate('%(prop:item_type)s'), 162​ Interpolate('%(prop:item_type)s'),
162 '-a', Interpolate('%(prop:arch)s'), 163​ '-a', Interpolate('%(prop:arch)s'),
163 '-j', Interpolate('%(prop:buildername)s/%(prop:buildnumber)s'), 164​ '-j', Interpolate('%(prop:buildername)s/%(prop:buildnumber)s'),
165​ '--uuid', Interpolate('%(prop:uuid)s'),
tflinkUnsubmitted
Not Done

this requires an execdb-style uuid now, doesn't it? Or are there more changes to trigger that aren't submitted for review yet?

mkrizekAuthorUnsubmitted
Not Done

Yes, changes to trigger are needed.

164 Interpolate('%(prop:taskname)s.yml')], 166​ Interpolate('%(prop:taskname)s.yml')],
165 descriptionDone=[Interpolate('%(prop:taskname)s on %(prop:item)s')], 167​ descriptionDone=[Interpolate('%(prop:taskname)s on %(prop:item)s')],
166 name='runtask', 168​ name='runtask',
167 logfiles={'taskotron.log': {'filename': '/var/log/taskotron/taskotron.log', }})) 169​ logfiles={'taskotron.log': {'filename': '/var/log/taskotron/taskotron.log', }}))
168 170​
171​
172​{% if deployment_type == 'dev' %}
173​# create artifacts task_output directory
tflinkUnsubmitted
Not Done

I'd prefer to see the output dir rendered from a template instead of hard coded - especially if/when we start using it in more places

mkrizekAuthorUnsubmitted
Not Done

Good point, will do, thanks.

174​factory.addStep(ShellCommand(command=['mkdir', '-p', '{{ public_artifacts_dir }}/%(prop:uuid)s/task_output']))
175​
176​# copy artifacts to master
177​factory.addStep(DirectoryUpload(slavesrc=Interpolate('/var/lib/taskotron/artifacts/%(prop:uuid)s/'),
kparalUnsubmitted
Not Done

This will effectively overwrite any artifact called taskotron.log that the task could have created with our own log file.

I'm not saying this approach is unreasonable, I just want to make sure it's expected. An alternative approach would be to create a subdirectory for task-generated artifacts, so that we don't need to deal with file name conflicts for taskotrong.log, stdout/stderr files, etc possibly some future files. That has one additional benefit - if everything is in the same directory, it can be a bit of a mess if the task generates tens or hundreds of artifacts. Separating "our" files from task-generated files would look cleaner.

Of course, we would need to come up with a good name for that subdirectory, which is always difficult :-)

tflinkUnsubmitted
Not Done

output or debug might work. it'd be a good place to put the stdout/stderr output, too if we get that working. Either way, we should document what we're using and state clearly that anything output into a directory of identical name will either be renamed or overwritten.

tflinkUnsubmitted
Not Done

It occurs to me that another solution would be to put all of our output into the root /srv/taskotron/artifacts/<uuid> dir and create something like /srv/taskotron/artifacts/<uuid>/task_output` for the artifacts generated by the task. That way, we don't have to worry about any name collisions.

mkrizekAuthorUnsubmitted
Not Done

Let's create task_output directory, acks?

kparalUnsubmitted
Not Done

Yes, that's what I had on mind, just not explained it clearly :-) Like this:

/srv/taskotron/artifacts/<uuid>
/srv/taskotron/artifacts/<uuid>/taskotron.log
/srv/taskotron/artifacts/<uuid>/task.log
/srv/taskotron/artifacts/<uuid>/output/
/srv/taskotron/artifacts/<uuid>/output/NVR1.txt
/srv/taskotron/artifacts/<uuid>/output/report.html

The <uuid>/ root dir would be ours, and everything under <uuid>/output/ would be maintained by task authors. Replace output with your preferred subdir name.

178​ masterdest=Interpolate('{{ public_artifacts_dir }}/%(prop:uuid)s/task_output')))
179​
180​# copy taskotron log to master
181​factory.addStep(FileUpload(slavesrc="/var/log/taskotron/taskotron.log",
182​ masterdest=Interpolate('{{ public_artifacts_dir }}/%(prop:uuid)s/taskotron.log')))
183​{% else %}
169# capture the taskotron log 184​# capture the taskotron log
170factory.addStep(ShellCommand(command=["cat", "/var/log/taskotron/taskotron.log"], name="cat_log", 185​factory.addStep(ShellCommand(command=["cat", "/var/log/taskotron/taskotron.log"], name="cat_log",
171 descriptionDone=['Store log'])) 186​ descriptionDone=['Store log']))
172 187​{% endif %}
173 188​
tflinkUnsubmitted
Not Done

should have noticed this earlier, but this will effectively enable artifacts storage on dev, stg and prod all at the same time. can you do an if/else block to make sure that the new stuff only shows up on dev?

174####### RESOURCE LOCKS 189​####### RESOURCE LOCKS
175# 190​#
Context not available.
View Options

roles/taskotron/buildmaster/tasks/main.yml

Context not available.
23 notify: 23​ notify:
24 - restart httpd 24​ - restart httpd
25 25​
26​- name: copy artifacts httpd config
tflinkUnsubmitted
Not Done

same as above, can you add a 'when: deployment_type == "dev"' to these to make sure that the new config doesn't make it to production until we're ready?

27​ when: deployment_type == 'dev'
28​ template: src=artifacts.conf.j2 dest=/etc/httpd/conf.d/artifacts.conf owner=root group=root
29​ notify:
30​ - restart httpd
31​
32​- name: create artifacts directory
33​ when: deployment_type == 'dev'
34​ file: path={{ item }} state=directory owner=buildmaster group=buildmaster mode=0775 setype=httpd_sys_content_t
35​ with_items:
36​ - /srv/taskotron
37​ - /srv/taskotron/artifacts
38​
26- name: ensure buildmaster database is created 39​- name: ensure buildmaster database is created
27 when: buildmaster_db_host != 'localhost' 40​ when: buildmaster_db_host != 'localhost'
28 delegate_to: "{{ buildmaster_db_host }}" 41​ delegate_to: "{{ buildmaster_db_host }}"
Context not available.
View Options

roles/taskotron/buildmaster/templates/artifacts.conf.j2

  • This file was added.
1​Alias /artifacts /srv/taskotron/artifacts
2​<Directory "/srv/taskotron/artifacts">
3​ Options +Indexes
4​ AllowOverride None
5​ Require all granted
6​</Directory>