fix docker autotest suite task for libtaskotron
ClosedPublic
Actions

Authored by garretraziel on Mar 7 2016, 10:41 AM.

Details

Reviewers

kparal
tflink

Maniphest Tasks

T723: Run the Docker Autotest Suite

Commits

rDOCKER7743fa6cc16c: fix docker autotest suite task for libtaskotron

Summary

This builds on code from task-dockerautotest. It corrects some things (it
correctly starts docker service) and it also reports differently (one summary result
for whole check instead of check for every subtest). Some simple fixes in docker autotest
repo were needed (https://github.com/autotest/autotest-docker/commit/c75622 and
https://github.com/autotest/autotest-docker/issues/572).

It also installs docker from Koji (I assume that this is how it should work).

Test Plan

Run whole testsuite with
runtask --libvirt -i docker-1.9.1-6.git6ec29ef.fc23 -t koji_build -a x86_64 dockerautotest.yml.
59 subtests pass, 39 fail and 2 error out (see T735), but failing tests are mainly garbage checks
(one of the the tests lefts some image behind and all consequent garbage checks fail because of that).
It's the same result as running on the local machine.

Diff Detail

Repository

rDOCKER task-dockerautotest

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

garretraziel retitled this revision from to fix docker autotest suite task for libtaskotron.Mar 7 2016, 10:41 AM

garretraziel updated this object.

garretraziel edited the test plan for this revision. (Show Details)

garretraziel added reviewers: tflink, kparal.

garretraziel added a task: T723: Run the Docker Autotest Suite.Mar 7 2016, 10:41 AM

I have executed the task and it seems to work OK and the generated artifacts look nice. Good job. Some nitpicks and questions below.

dockerautotest.yml
27	Remark for taskotron developers: We will need to look at `arch` handling, because currently our disposable minion support does not take it into account at all.
run_dockerautotest.py
8–9	Do we need to use autotest from git, or would it be enough to use the `autotest` package in Fedora (added to task formula)?
10–11	You're not checking the exitcode of any executed command. Is that intentional? The recommended usage of `subprocess` is: output = subprocess.check_output(['some', 'command], stderr=subprocess.STDOUT) which raises `subprocess.CalledProcessError` for non-zero exit code. Also, if you want to print command output automatically, you can use `stdout=subprocess.PIPE`.
22–23	What's the purpose of this? I think it will fail with `KeyError` a few lines below.
83	In this case I'd actually argue that `NEEDS_INSPECTION` makes sense for an overall result, because `CRASHED` seems to imply the whole task crashed, and not that just 2 out of 80 subtests crashed. OTOH for those individual results, `CRASHED` is clearer than `NEEDS_INSPECTION`, or you could could use their own terminology when creating the summary line. It depends whether you want to play with it or not.

kparal mentioned this in D769: remote_exec: speed up minion->host copy operations a bit.Mar 8 2016, 3:10 PM

use autotest from repos, rework running commands a little bit

My only concern about using the packaged version of autotest is if that package doesn't get updated and something changes upstream. I wonder if we should be asking the docker-autotest folks about that.

overall, it looks good to me outside of the inline comments and question about why we're only running a subset of the tests in docker-autotest

run_dockerautotest.py
60	why are we limiting the tests run to just these?
83	yeah, that makes sense to me. if we set one result to `CRASHED`, the whole result becomes `CRASHED`, right? In that case, it may make more sense to use `NEEDS_INSPECTION` as the status for a single result but keep `ERROR` in the summary to use upstream terminology instead of stuffing our own classification into text where it's not needed

Seems OK overall, modulo Tim's questions.

run_dockerautotest.py
36–37	You don't want interactive copying here, I believe.
83	Agreed that I would use their own terms in the summary, if it is not too complicated.

use autotest from repos, rework running commands a little bit
create custom summary (drop AutotestJsonParser), use correct subprocess function

kparal accepted this revision.Mar 11 2016, 1:16 PM

This revision is now accepted and ready to land.Mar 11 2016, 1:16 PM

tflink accepted this revision.Mar 11 2016, 1:29 PM

Closed by commit rDOCKER7743fa6cc16c: fix docker autotest suite task for libtaskotron (authored by garretraziel). · Explain WhyMar 11 2016, 2:29 PM

This revision was automatically updated to reflect the committed changes.

kparal mentioned this in T723: Run the Docker Autotest Suite.Mar 11 2016, 3:51 PM

Revision Contents

		Path
M		dockerautotest.yml (19 lines)
M		run_dockerautotest.py (135 lines)
D	M	tests/conftest.py (28 lines)
D	M	tests/test_parse_autotest_json.py (63 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	1964	02531f6		Mar 7 2016, 10:41 AM	★	★
Diff 2	1975	02531f6	- use autotest from repos, rework running commands a little bit	Mar 9 2016, 2:31 PM	★	★
Diff 3	1981	02531f6	- use autotest from repos, rework running commands a little bit	Mar 11 2016, 10:01 AM	★	★
Diff 4	1982	02531f6	rDOCKER7743fa6cc16c2f1cc6961598bca8e415ce12a6da	Mar 11 2016, 1:51 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
86575a1f2dea	536493ec4f13	ac98b5bdc9cc	Jan Sedlák	create custom summary (drop AutotestJsonParser), use correct subprocess function	Mar 11 2016, 10:00 AM
ac98b5bdc9cc	814bdc0ca368	d604c94e6b32	Jan Sedlák	use autotest from repos, rework running commands a little bit	Mar 9 2016, 2:30 PM
d604c94e6b32	be80c57243ef	7817c7c35508	Jan Sedlák	fix docker autotest suite task for libtaskotron (Show More…)	Mar 7 2016, 10:17 AM
7817c7c35508	4e9d7fcb491d	28bac4acad4f	Jan Sedlák	PEP8 reformatting	Mar 1 2016, 2:10 PM
28bac4acad4f	8284f370faff	02531f68d0a2	Jan Sedlák	start docker service before test, remove unnecessary reporting after prep	Mar 1 2016, 2:07 PM

Diff 1982

View Options

dockerautotest.yml

1	---	1		---
2	name: dockerautotest	2		name: dockerautotest
3	desc: "run docker autotest"	3		desc: "run docker autotest"
4	maintainer: tflink	4		maintainer: tflink
5		5
6	# I got all these from the default vars in adept for rhel	6		# I got all these from the default vars in adept for rhel
7	# no idea if they're all needed for fedora	7		# no idea if they're all needed for fedora
8	environment:	8		environment:
9	rpm:	9		rpm:
10	- docker	10		- autotest-framework
11	- procps	11		- procps
12	- tar	12		- tar
13	- findutils	13		- findutils
14	- bzip2	14		- bzip2
15	- gdb	15		- gdb
16	- bridge-utils	16		- bridge-utils
17	- nfs-utils	17		- nfs-utils
18	- git	18		- git
19	- glibc-devel	19		- glibc-devel
20	- python-bugzilla	20		- python-bugzilla
21	- which	21		- which
22	- cloud-utils-growpart	22		- cloud-utils-growpart
23		23
24	input:	24		input:
25	args:	25		args:
26	- koji_build	26		- koji_build
		27		- arch
			kparalUnsubmitted Not Done Remark for taskotron developers: We will need to look at `arch` handling, because currently our disposable minion support does not take it into account at all.
27		28
28	actions:	29		actions:
		30		- name: download docker from koji
		31		koji:
		32		action: download
		33		koji_build: "${koji_build}"
		34		arch: "${arch}"
		35		export: downloaded_rpms
		36
29	- name: prepare for docker autotest run	37		- name: prepare for docker autotest run
30	python:	38		python:
31	file: run_dockerautotest.py	39		file: run_dockerautotest.py
32	callable: prepare	40		callable: prepare
33	workdir: "${workdir}"	41		workdir: "${workdir}"
34	export: prep_output	42		downloaded_rpms: "${downloaded_rpms}"
35
36	- name: report prep results to resultsdb
37	resultsdb:
38	results: ${prep_output}
39		43
40	- name: run docker autotest	44		- name: run docker autotest
41	python:	45		python:
42	file: run_dockerautotest.py	46		file: run_dockerautotest.py
43	callable: runtest	47		callable: runtest
44	item: "${item}"	48		item: "${item}"
45	workdir: "${workdir}"	49		workdir: "${workdir}"
46	artifactsdir: "${artifactsdir}"	50		artifactsdir: "${artifactsdir}"
47	export: run_output	51		export: run_output
48		52
49		53
50	- name: report test results to resultsdb	54		- name: report test results to resultsdb
51	resultsdb:	55		resultsdb:
52	results: ${run_output}	56		results: "${run_output}"
53

View Options

run_dockerautotest.py

		1		from __future__ import print_function
		2		from collections import OrderedDict
1	import subprocess	3		import subprocess
2	from pprint import pprint
3	import json	4		import json
4	import os
5	import shutil	5		import shutil
6		6
7	from libtaskotron import check	7		from libtaskotron import check
8		8
9
10	AUTOTEST_GIT = 'https://github.com/autotest/autotest.git'
11	DOCKERAUTOTEST_GIT = 'https://github.com/autotest/autotest-docker.git'	9		DOCKERAUTOTEST_GIT = 'https://github.com/autotest/autotest-docker.git'
			kparalUnsubmitted Not Done Do we need to use autotest from git, or would it be enough to use the `autotest` package in Fedora (added to task formula)?
12		10
13	def run_command(command, cwd=None):
14	p = subprocess.Popen(command, cwd=cwd)
15	return p.communicate()
16
17		11
			kparalUnsubmitted Not Done You're not checking the exitcode of any executed command. Is that intentional? The recommended usage of `subprocess` is: output = subprocess.check_output(['some', 'command], stderr=subprocess.STDOUT) which raises `subprocess.CalledProcessError` for non-zero exit code. Also, if you want to print command output automatically, you can use `stdout=subprocess.PIPE`.
		12		def prepare(workdir, downloaded_rpms):
		13		"""Prepare for a run of docker-autotest"""
18		14
19	def prepare(workdir='.'):	15		# install docker
20	detail = check.CheckDetail(item='docker',	16		dockerinstall = ['dnf',
21	report_type=check.ReportType.KOJI_BUILD)	17		'install',
		18		'--assumeyes',
		19		] + downloaded_rpms['downloaded_rpms']
		20		subprocess.check_call(dockerinstall, stderr=subprocess.STDOUT)
22		21
23	"""Prepare for a run of docker-autotest"""	22		mkdircmd = ['mkdir',
24	# clone autotest in to ./autotest	23		'-p',
			kparalUnsubmitted Not Done What's the purpose of this? I think it will fail with `KeyError` a few lines below.
25	autotestclone = ['git', 'clone', AUTOTEST_GIT, '{}/autotest'.format(workdir)]	24		'{}/tests'.format(workdir)]
26	cldata = run_command(autotestclone)	25		subprocess.check_call(mkdircmd, stderr=subprocess.STDOUT)
27	pprint(cldata)
28		26
29	# clone docker-autotest in to ./autotest/client/tests/docker	27		# clone docker-autotest in to ./autotest/client/tests/docker
30	dautotestclone = ['git',	28		dautotestclone = ['git',
31	'clone',	29		'clone',
32	DOCKERAUTOTEST_GIT,	30		'--depth', '1',
33	'{}/autotest/client/tests/docker'.format(workdir)]	31		DOCKERAUTOTEST_GIT,
34		32		'{}/tests/docker'.format(workdir)]
35	dcldata= run_command(dautotestclone)	33		subprocess.check_call(dautotestclone, stderr=subprocess.STDOUT)
36	pprint(dcldata)
37		34
38	# copy default configuration	35		# copy default configuration
39	cpconfig = ['cp',	36		cpconfig = ['cp',
40	'-abi',	37		'-ab',
			kparalUnsubmitted Not Done You don't want interactive copying here, I believe.
41	'config_defaults/defaults.ini',	38		'{}/tests/docker/config_defaults/defaults.ini'.format(workdir),
42	'config_custom'	39		'{}/tests/docker/config_custom'.format(workdir)
43	]	40		]
44	cfgdata = run_command(cpconfig,	41		subprocess.check_call(cpconfig, stderr=subprocess.STDOUT)
45	cwd='{}/autotest/client/tests/docker'.format(workdir))
46	pprint(cfgdata)
47
48	detail.note = "this doesn't have much detail yet"
49	detail.store("something")
50		42
51	return check.export_YAML([detail])	43		# service run
		44		service = ['systemctl',
		45		'start',
		46		'docker.service'
		47		]
		48		subprocess.check_call(service, stderr=subprocess.STDOUT)
52		49
53		50
54	def runtest(item, workdir='.', artifactsdir='.'):	51		def runtest(item, workdir='.', artifactsdir='.'):
55	root_dir = '{}/autotest/client'.format(workdir)	52		# run autotest suite
56		53		runcmd = ['autotest-local',
57	runcmd = ['./autotest-local',	54		'-d',
		55		'{}/tests'.format(workdir),
		56		'--output_dir',
		57		workdir,
58	'run',	58		'run',
59	'docker',	59		'{}/tests/docker'.format(workdir)]
60	]	60		subprocess.check_call(runcmd, stderr=subprocess.STDOUT)
			tflinkUnsubmitted Not Done why are we limiting the tests run to just these?
61	rundata = run_command(runcmd,
62	cwd=root_dir)
63	pprint(rundata)
64
65	parser = AutotestJsonParser(item)
66		61
67	json_filename = '{}/autotest/client/results/default/status.json'.format(workdir)	62		# parse autotest result JSON
		63		json_filename = '{}/results/default/status.json'.format(workdir)
68		64
69	json_rawdata = None	65		json_rawdata = None
70	with open(json_filename, 'r') as json_file:	66		with open(json_filename, 'r') as json_file:
71	json_rawdata = json_file.read()	67		json_rawdata = json_file.read()
72		68
73	json_data = json.loads(json_rawdata)	69		json_data = json.loads(json_rawdata)
74		70
75	details = parser.to_check_detail(json_data)
76
77	# copy output to artifacts	71		# copy output to artifacts
78	artifacts_out = '{}/results'.format(artifactsdir)	72		artifacts_out = '{}/results'.format(artifactsdir)
79		73
80	print("copying {} to {} for results".format(json_data['results_dir'], artifacts_out))	74		print("copying {} to {} for results".format(json_data['results_dir'], artifacts_out))
81	#os.mkdir(artifacts_dir)
82	shutil.copytree(json_data['results_dir'], artifacts_out)	75		shutil.copytree(json_data['results_dir'], artifacts_out)
83		76
84	return check.export_YAML(details)	77		# create overall CheckDetail
85		78		overall = check.CheckDetail(item=item, report_type=check.ReportType.KOJI_BUILD)
86	def parse_autotest_json():
87	pass
88
89	class AutotestJsonParser(object):
90		79
91	# mapping status from autotest json to our statuses in CheckDetail	80		overall.note, overall.outcome = create_autotest_summary(json_data)
92	status_map = {"GOOD" : "PASSED",	81		overall.artifact = '{}/job_report.html'.format(artifacts_out)
93	"FAIL" : "FAILED",
94	"ERROR" : "CRASHED"}
95		82
96	def __init__(self, item_version):	83		return check.export_YAML(overall)
			kparalUnsubmitted Not Done In this case I'd actually argue that `NEEDS_INSPECTION` makes sense for an overall result, because `CRASHED` seems to imply the whole task crashed, and not that just 2 out of 80 subtests crashed. OTOH for those individual results, `CRASHED` is clearer than `NEEDS_INSPECTION`, or you could could use their own terminology when creating the summary line. It depends whether you want to play with it or not.
			tflinkUnsubmitted Not Done yeah, that makes sense to me. if we set one result to `CRASHED`, the whole result becomes `CRASHED`, right? In that case, it may make more sense to use `NEEDS_INSPECTION` as the status for a single result but keep `ERROR` in the summary to use upstream terminology instead of stuffing our own classification into text where it's not needed
			kparalUnsubmitted Not Done Agreed that I would use their own terms in the summary, if it is not too complicated.
97	self.item_version = item_version
98		84
99	def parse_single_result(self, result):
100	"""return CheckDetail from single result json"""
101		85
102	detail = check.CheckDetail(item=self.item_version, report_type=check.ReportType.KOJI_BUILD)	86		def create_autotest_summary(json_data):
		87		"""Parses autotests' json result and creates summary and outcome for CheckDetail"""
103		88
104	try:	89		outcomes = OrderedDict((("GOOD", 0), ("FAIL", 0), ("ERROR", 0), ("NEEDS_INSPECTION", 0)))
105	detail.outcome = self.status_map[result['status_code']]	90		outcome_map = {"GOOD": "PASSED", "FAIL": "FAILED"}
106	except KeyError:
107	detail.outcome = 'NEEDS_INSPECTION'
108		91
109	detail.artifact = '{}/debug'.format(result['subdir'])	92		cd = check.CheckDetail(item=None) # create dummy checkdetail to use update_outcome
110	detail.output = result['message']
111		93
112	detail.note = result['subdir']	94		for operation in json_data['operations']:
		95		status = operation['status_code']
		96		if status in outcomes:
		97		outcomes[status] += 1
		98		else:
		99		outcomes["NEEDS_INSPECTION"] += 1
113		100
114	return detail	101		cd.update_outcome(outcome_map.get(status, "NEEDS_INSPECTION"))
115		102
116	def to_check_detail(self, data):	103		summary = []
117	details = []	104		for outcome, cnt in outcomes.iteritems():
118	for operation in data['operations']:	105		if cnt:
119	details.append(self.parse_single_result(operation))	106		summary.append("{} {}".format(cnt, outcome))
120		107
121	return details	108		return ", ".join(summary), cd.outcome

View Options

tests/conftest.py

This file was deleted.

This file was completely deleted. Show File Contents

View Options

tests/test_parse_autotest_json.py

This file was deleted.

This file was completely deleted. Show File Contents

fix docker autotest suite task for libtaskotronClosedPublicActions

Details

Diff Detail

Revision Contents

Diff 1982

dockerautotest.yml

run_dockerautotest.py

tests/conftest.py

tests/test_parse_autotest_json.py

fix docker autotest suite task for libtaskotron
ClosedPublic
Actions