Differential D1210

resultsdb_directive: change default log link to artifactsdir
ClosedPublic
Actions

Authored by kparal on Jun 15 2017, 4:06 PM.

Details

Reviewers
mkrizek
jskladan
Group Reviewers
libtaskotron
Maniphest Tasks
T954: allow resultsdb directive to use the root 'task_output' dir as a log link
Commits
rLTRN52ca1b425735: resultsdb_directive: change default log link to artifactsdir
Summary

This changes the default log link visible in resultsdb from task stdio
output to artifactsdir directory (which contains stdio, debug log and
others). This only applies if the task hasn't set artifact in
resultyaml (that overrides the default).

The reasoning is that taskotron stdio has grown larger and larger and
nowadays it's very hard to find relevant task output inside it anyways.
Tasks really need to create specialized logs at least in case of failure
(and all existing ones do). The new default is much better for
a) people debugging a problem - because they have clear access to all
available files
b) tasks with multiple log files which don't want to set any of them as
default (because it would make accessing the other ones much harder) -
with the new solution they see all artifacts in a directory view

Test Plan

I tested this with local resultsdb instance and the generated URLs seem correct. However, further verification from reviewers is very welcome.

Diff Detail

Repository
rLTRN libtaskotron
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kparal created this revision.Jun 15 2017, 4:06 PM
kparal added subscribers: mkrizek, jskladan.Jun 19 2017, 8:26 AM

As an example, say rpmdeplint doesn't set custom artifact in resultyaml (which it does). The default link would go from:
https://taskotron.fedoraproject.org/artifacts/all/43567c16-54c3-11e7-b089-5254008e42f6/task_output/taskotron.log.gz
to:
https://taskotron.fedoraproject.org/artifacts/all/43567c16-54c3-11e7-b089-5254008e42f6/
(Please note that taskotron logs should be in that root dir, instead of being in task_output, it's a bug).

Ping @tflink @mkrizek @jskladan.

mkrizek accepted this revision.Jun 19 2017, 12:04 PM
This revision is now accepted and ready to land.Jun 19 2017, 12:04 PM
jskladan accepted this revision.Jun 19 2017, 12:23 PM
Closed by commit rLTRN52ca1b425735: resultsdb_directive: change default log link to artifactsdir (authored by Kamil Páral <kparal@redhat.com>). · Explain WhyJun 19 2017, 12:46 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

Diff 3071

View Options

libtaskotron/directives/resultsdb_directive.py

Show First 20 Lines • Show All 97 Lines • ▼ Show 20 Line(s)
98 98​
99import os 99​import os
100import configparser 100​import configparser
101 101​
102from libtaskotron.directives import BaseDirective 102​from libtaskotron.directives import BaseDirective
103 103​
104from libtaskotron import check 104​from libtaskotron import check
105from libtaskotron import config 105​from libtaskotron import config
106from libtaskotron import buildbot_utils ​
107from libtaskotron.exceptions import TaskotronDirectiveError, TaskotronValueError 106​from libtaskotron.exceptions import TaskotronDirectiveError, TaskotronValueError
108from libtaskotron.logger import log 107​from libtaskotron.logger import log
109from libtaskotron.ext.fedora import rpm_utils 108​from libtaskotron.ext.fedora import rpm_utils
110 109​
111import resultsdb_api 110​import resultsdb_api
112 111​
113directive_class = 'ResultsdbDirective' 112​directive_class = 'ResultsdbDirective'
114 113​
▲ Show 20 Lines • Show All 159 Lines • ▼ Show 20 Line(s)251 def process(self, params, arg_data):
274 273​
275 conf = config.get_config() 274​ conf = config.get_config()
276 if not conf.report_to_resultsdb: 275​ if not conf.report_to_resultsdb:
277 log.info("Reporting to ResultsDB is disabled. Once enabled, the " 276​ log.info("Reporting to ResultsDB is disabled. Once enabled, the "
278 "following would get reported:\n%s" % params['results']) 277​ "following would get reported:\n%s" % params['results'])
279 return check.export_YAML(check_details) 278​ return check.export_YAML(check_details)
280 279​
281 checkname = '%s.%s' % (arg_data['namespace'], arg_data['checkname']) 280​ checkname = '%s.%s' % (arg_data['namespace'], arg_data['checkname'])
281​ artifactsdir_url = '%s/all/%s' % (self.artifacts_baseurl, arg_data['uuid'])
282 282​
283 # find out if the task is allowed to post results into the namespace 283​ # find out if the task is allowed to post results into the namespace
284 if config.get_config().profile == config.ProfileName.PRODUCTION: 284​ if config.get_config().profile == config.ProfileName.PRODUCTION:
285 self.check_namespace(checkname, arg_data) 285​ self.check_namespace(checkname, arg_data)
286 286​
287 # for now, we're creating the resultsdb group at reporting time 287​ # for now, we're creating the resultsdb group at reporting time
288 _, log_url = buildbot_utils.get_urls(arg_data['jobid'], ​
289 self.masterurl, ​
290 self.task_stepname) ​
291 artifactsdir_url = '%s/all/%s' % (self.artifacts_baseurl, arg_data['uuid']) ​
292 ​
293 group_data = self.create_resultsdb_group(uuid=arg_data['uuid'], name=checkname) 288​ group_data = self.create_resultsdb_group(uuid=arg_data['uuid'], name=checkname)
294 289​
295 log.info('Posting %s results to ResultsDB...' % len(check_details)) 290​ log.info('Posting %s results to ResultsDB...' % len(check_details))
296 for detail in check_details: 291​ for detail in check_details:
297 checkname = '%s.%s' % (arg_data['namespace'], 292​ checkname = '%s.%s' % (arg_data['namespace'],
298 detail.checkname or arg_data['checkname']) 293​ detail.checkname or arg_data['checkname'])
299 294​
300 self.ensure_testcase_exists(checkname) 295​ self.ensure_testcase_exists(checkname)
301 result_log_url = log_url 296​ result_log_url = artifactsdir_url
302 if detail.artifact: 297​ if detail.artifact:
303 artifact_path = self.get_artifact_path( 298​ artifact_path = self.get_artifact_path(
304 arg_data['artifactsdir'], 299​ arg_data['artifactsdir'],
305 detail.artifact 300​ detail.artifact
306 ) 301​ )
307 if artifact_path: 302​ if artifact_path:
308 result_log_url = "%s/task_output/%s" % (artifactsdir_url, artifact_path) 303​ result_log_url = "%s/task_output/%s" % (artifactsdir_url, artifact_path)
309 try: 304​ try:
Show All 19 Lines
View Options

testing/functest_resultsdb_directive.py

Show First 20 Lines • Show All 99 Lines • ▼ Show 20 Line(s)81 def test_param_file(self, tmpdir):
100 # This could be written as self.stub_rdb.calls()[0] at the moment, but 100​ # This could be written as self.stub_rdb.calls()[0] at the moment, but
101 # this is more future-proof, and accidental addition of resultsdb 101​ # this is more future-proof, and accidental addition of resultsdb
102 # calls is handled by the previous assert. 102​ # calls is handled by the previous assert.
103 call = [call for call in self.stub_rdb.calls() if call[0] == 'create_result'][0] 103​ call = [call for call in self.stub_rdb.calls() if call[0] == 'create_result'][0]
104 # Select the keyword arguments of that call 104​ # Select the keyword arguments of that call
105 call_data = call[2] 105​ call_data = call[2]
106 106​
107 # the log url depends on the arg_data, so construct it here 107​ # the log url depends on the arg_data, so construct it here
108 ref_builder, ref_jobid = self.ref_arg_data['jobid'].split('/') 108​ ref_log_url = '%s/all/%s' % (self.conf.artifacts_baseurl, self.ref_arg_data['uuid'])
109 ref_log_url = '%s/builders/%s/builds/%s/steps/%s/logs/stdio' %\ ​
110 (self.conf.taskotron_master, ref_builder, ref_jobid, ​
111 self.conf.buildbot_task_step) ​
112 ref_testcase_name = '%s.%s' % (self.ref_arg_data['namespace'], 109​ ref_testcase_name = '%s.%s' % (self.ref_arg_data['namespace'],
113 self.ref_arg_data['checkname']) 110​ self.ref_arg_data['checkname'])
114 111​
115 assert call_data['testcase'] == ref_testcase_name 112​ assert call_data['testcase'] == ref_testcase_name
116 assert call_data['outcome'] == self.cd.outcome 113​ assert call_data['outcome'] == self.cd.outcome
117 assert call_data['note'] == self.cd.note 114​ assert call_data['note'] == self.cd.note
118 assert call_data['ref_url'] == ref_log_url 115​ assert call_data['ref_url'] == ref_log_url
119 assert call_data['item'] == self.cd.item 116​ assert call_data['item'] == self.cd.item
Show All 10 Lines
View Options

testing/test_resultsdb_directive.py

Show First 20 Lines • Show All 147 Lines • ▼ Show 20 Line(s)131 def test_report(self):
148 # This could be written as self.stub_rdb.calls()[0] at the moment, but 148​ # This could be written as self.stub_rdb.calls()[0] at the moment, but
149 # this is more future-proof, and accidental addition of resultsdb 149​ # this is more future-proof, and accidental addition of resultsdb
150 # calls is handled by the previous assert. 150​ # calls is handled by the previous assert.
151 call = [call for call in self.stub_rdb.calls() if call[0] == 'create_result'][0] 151​ call = [call for call in self.stub_rdb.calls() if call[0] == 'create_result'][0]
152 # Select the keyword arguments of that call 152​ # Select the keyword arguments of that call
153 call_data = call[2] 153​ call_data = call[2]
154 154​
155 # the log url depends on the arg_data, so construct it here 155​ # the log url depends on the arg_data, so construct it here
156 ref_builder, ref_jobid = self.ref_arg_data['jobid'].split('/') 156​ ref_log_url = '%s/all/%s' % (self.conf.artifacts_baseurl, self.ref_arg_data['uuid'])
157 ref_log_url = '%s/builders/%s/builds/%s/steps/%s/logs/stdio' %\ ​
158 (self.conf.taskotron_master, ref_builder, ref_jobid, ​
159 self.conf.buildbot_task_step) ​
160 ref_testcase_name = '%s.%s' % (self.ref_arg_data['namespace'], 157​ ref_testcase_name = '%s.%s' % (self.ref_arg_data['namespace'],
161 self.ref_arg_data['checkname']) 158​ self.ref_arg_data['checkname'])
162 159​
163 assert call_data['testcase'] == ref_testcase_name 160​ assert call_data['testcase'] == ref_testcase_name
164 assert call_data['outcome'] == self.cd.outcome 161​ assert call_data['outcome'] == self.cd.outcome
165 assert call_data['note'] == self.cd.note 162​ assert call_data['note'] == self.cd.note
166 assert call_data['ref_url'] == ref_log_url 163​ assert call_data['ref_url'] == ref_log_url
167 assert call_data['item'] == self.cd.item 164​ assert call_data['item'] == self.cd.item
▲ Show 20 Lines • Show All 205 Lines • Show Last 20 Lines