Differential D583

set what payload, arch and image type to download in config file
ClosedPublic
Actions

Authored by garretraziel on Sep 15 2015, 1:29 PM.

Details

Reviewers
adamwill
jskladan
Maniphest Tasks
T616: Make payload downloading configurable
Commits
rOPENQA927c970266c8: set what payload, arch and image type to download in config file
Summary

Let user decide what architectures, image types and
payloads to download in config file.

Test Plan

Try to set different value in config file, observer
that correct images get downloaded.

Diff Detail

Repository
rOPENQA fedora_openqa
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 set what payload, arch and image type to download in config file.Sep 15 2015, 1:29 PM
garretraziel updated this object.
garretraziel edited the test plan for this revision. (Show Details)
garretraziel added reviewers: adamwill, jskladan.
adamwill requested changes to this revision.Sep 15 2015, 3:58 PM

the PERSISTENT change should be a different commit, I think, and I'd prefer it be in section schedule (it is tied to the scheduling code, nothing else uses it).

One very nitpicky thing: in check-compose I also have a list-y config value (email addresses to send the report to), and I made it space-separated, not comma-separated. As these things are at least kinda related, I guess maybe we should make them consistent. Perhaps we should simply tolerate both comma and space separation? I'm OK with a rule that fedfind attributes never include , or , and that's alreadt the case for email addresses...

fedora_openqa_schedule/schedule.py
291

I don't actually like this, the original intent wasn't a catch-all +1 (which is actually meaningless - if we +1 everything we may as well just not do anything), it was specifically to +1 Workstation. It was meant to weakly prefer Workstation netinst/DVD to Cloud if we couldn't find Server or 'generic', IIRC. So it should go back to being 'workstation', I think.

This revision now requires changes to proceed.Sep 15 2015, 3:58 PM
adamwill added inline comments.Sep 15 2015, 4:04 PM
fedora_openqa_schedule/schedule.py
43

I am not a huge fan of adding *more* module-level variables, I was rather hoping to get rid of them. Could we rather follow the way other config-set defaults are done? That is, add imagetypes and payloads arguments to jobs_from_fedfind() with default values of None, and have them read in the values from config if the arguments are set to None? (and make arches work the same)

garretraziel updated this revision to Diff 1528.Sep 16 2015, 12:02 PM
  • correct things with schedule configuration loading
garretraziel updated this revision to Diff 1529.Sep 16 2015, 12:04 PM
  • fix typos in docstrings
adamwill accepted this revision.Sep 17 2015, 7:22 AM

Seems fine. There are ways to handle comma and space separation that don't require regexes, but I don't really mind this way. It might be worth a comment explaining why we're doing it that way, though.

We should probably add a moderate preference for netinst/boot (lower than dvd but higher than anything else) to the 'universal image' stuff, but it's OK without for now.

This revision is now accepted and ready to land.Sep 17 2015, 7:22 AM
Closed by commit rOPENQA927c970266c8: set what payload, arch and image type to download in config file (authored by garretraziel). · Explain WhySep 17 2015, 7:58 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

Diff 1545

View Options

fedora_openqa_schedule/config.py

Show All 32 Lines
33CONFIG.add_section('report') 33​CONFIG.add_section('report')
34CONFIG.add_section('schedule') 34​CONFIG.add_section('schedule')
35CONFIG.set('cli', 'log-file', '') 35​CONFIG.set('cli', 'log-file', '')
36CONFIG.set('cli', 'log-level', 'info') 36​CONFIG.set('cli', 'log-level', 'info')
37CONFIG.set('report', 'submit', 'true') 37​CONFIG.set('report', 'submit', 'true')
38CONFIG.set('report', 'jobs-wait', '360') 38​CONFIG.set('report', 'jobs-wait', '360')
39CONFIG.set('schedule', 'iso-path', '/var/lib/openqa/factory/iso/') 39​CONFIG.set('schedule', 'iso-path', '/var/lib/openqa/factory/iso/')
40CONFIG.set('schedule', 'compose-wait', '480') 40​CONFIG.set('schedule', 'compose-wait', '480')
41​CONFIG.set('schedule', 'imagetype', 'boot live dvd')
42​CONFIG.set('schedule', 'arches', 'x86_64 i386')
43​CONFIG.set('schedule', 'payload', 'server generic workstation kde')
44​CONFIG.set('schedule', 'persistent', '/var/tmp/openqa_watcher.json')
41CONFIG.read('/etc/fedora-openqa/schedule.conf') 45​CONFIG.read('/etc/fedora-openqa/schedule.conf')
42CONFIG.read('{0}/.config/fedora-openqa/schedule.conf'.format(os.path.expanduser('~'))) 46​CONFIG.read('{0}/.config/fedora-openqa/schedule.conf'.format(os.path.expanduser('~')))
View Options

fedora_openqa_schedule/schedule.py

Show All 21 Lines
22"""Scheduler module for fedora-openqa-schedule. Functions related to 22​"""Scheduler module for fedora-openqa-schedule. Functions related to
23job scheduling go here. 23​job scheduling go here.
24""" 24​"""
25 25​
26# Standard libraries 26​# Standard libraries
27import json 27​import json
28import logging 28​import logging
29import os.path 29​import os.path
30​import re
30 31​
31# External dependencies 32​# External dependencies
32import fedfind.release 33​import fedfind.release
33from openqa_client.client import OpenQA_Client 34​from openqa_client.client import OpenQA_Client
34from six.moves import configparser 35​from six.moves import configparser
35import urlgrabber 36​import urlgrabber
36 37​
37# Internal dependencies 38​# Internal dependencies
38from fedora_openqa_schedule.config import CONFIG 39​from fedora_openqa_schedule.config import CONFIG
39 40​
40logger = logging.getLogger(__name__) 41​logger = logging.getLogger(__name__)
41 42​
42PERSISTENT = "/var/tmp/openqa_watcher.json" ​
43ARCHES = ('x86_64', 'i386') ​
44# Default value is '/var/lib/openqa/factory/iso/' ​
45ISO_PATH = CONFIG.get('schedule', 'iso-path') ​
46# List of all desktops we want to test ​
47DESKTOPS = ('workstation', 'kde') ​
48 ​
49 43​
adamwillUnsubmitted
Not Done

I am not a huge fan of adding *more* module-level variables, I was rather hoping to get rid of them. Could we rather follow the way other config-set defaults are done? That is, add imagetypes and payloads arguments to jobs_from_fedfind() with default values of None, and have them read in the values from config if the arguments are set to None? (and make arches work the same)

50class TriggerException(Exception): 44​class TriggerException(Exception):
51 pass 45​ pass
52 46​
53 47​
54class WaitError(Exception): 48​class WaitError(Exception):
55 """Raised when we waited for something too long.""" 49​ """Raised when we waited for something too long."""
56 pass 50​ pass
57 51​
58 52​
59def read_last(): 53​def read_last(arches=None):
60 """Read last tested version from file.""" 54​ """Read last tested version from file.
61 logger.debug("reading latest checked version from %s", PERSISTENT) 55​ If no architectures were provided, load them from config file.
56​ """
57​ persistent = CONFIG.get('schedule', 'persistent')
58​ if not arches:
59​ # split arches by words (works for both space- and comma-separated values)
60​ arches = re.split(r'\W+', CONFIG.get('schedule', 'arches').strip())
61​ logger.debug("reading latest checked version from %s", persistent)
62 result = {} 62​ result = {}
63 try: 63​ try:
64 f = open(PERSISTENT, "r") 64​ f = open(persistent, "r")
65 json_raw = f.read() 65​ json_raw = f.read()
66 f.close() 66​ f.close()
67 json_parsed = json.loads(json_raw) 67​ json_parsed = json.loads(json_raw)
68 except IOError: 68​ except IOError:
69 logger.warning("cannot read file %s", PERSISTENT) 69​ logger.warning("cannot read file %s", persistent)
70 return result, {} 70​ return result, {}
71 71​
72 for arch in ARCHES: 72​ for arch in arches:
73 result[arch] = json_parsed.get(arch, None) 73​ result[arch] = json_parsed.get(arch, None)
74 logger.info("latest version for %s: %s", arch, result[arch]) 74​ logger.info("latest version for %s: %s", arch, result[arch])
75 return result, json_parsed 75​ return result, json_parsed
76 76​
77 77​
78def download_image(image): 78​def download_image(image, iso_path=None):
79 """Download a given image with a name that should be unique. 79​ """Download a given image with a name that should be unique,
80​ optionally specifying destination directory.
80 Returns the filename of the image (not the path). 81​ Returns the filename of the image (not the path).
81 """ 82​ """
83​ # if no iso_path is specified, parse it from config file
84​ if not iso_path:
85​ iso_path = CONFIG.get('schedule', 'iso-path')
82 ver = image.version.replace(' ', '_') 86​ ver = image.version.replace(' ', '_')
83 if image.imagetype == 'boot': 87​ if image.imagetype == 'boot':
84 isoname = "{0}_{1}_{2}_boot.iso".format(ver, image.payload, image.arch) 88​ isoname = "{0}_{1}_{2}_boot.iso".format(ver, image.payload, image.arch)
85 else: 89​ else:
86 isoname = "{0}_{1}".format(ver, image.filename) 90​ isoname = "{0}_{1}".format(ver, image.filename)
87 filename = os.path.join(ISO_PATH, isoname) 91​ filename = os.path.join(iso_path, isoname)
88 if not os.path.isfile(filename): 92​ if not os.path.isfile(filename):
89 logger.info("downloading %s (%s) to %s", image.url, image.desc, filename) 93​ logger.info("downloading %s (%s) to %s", image.url, image.desc, filename)
90 # Icky hack around a urlgrabber bug: 94​ # Icky hack around a urlgrabber bug:
91 # https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=715416 95​ # https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=715416
92 urlgrabber.urlgrab(image.url.replace('https', 'http'), filename) 96​ urlgrabber.urlgrab(image.url.replace('https', 'http'), filename)
93 else: 97​ else:
94 logger.info("%s already exists", filename) 98​ logger.info("%s already exists", filename)
95 return isoname 99​ return isoname
Show All 38 Lines102def run_openqa_jobs(isoname, flavor, arch, build, force=False):
134 138​
135 output = client.openqa_request('POST', 'isos', params) 139​ output = client.openqa_request('POST', 'isos', params)
136 logger.debug("run_openqa_jobs: executed") 140​ logger.debug("run_openqa_jobs: executed")
137 logger.debug("run_openqa_jobs: planned jobs: %s", output["ids"]) 141​ logger.debug("run_openqa_jobs: planned jobs: %s", output["ids"])
138 142​
139 return output["ids"] 143​ return output["ids"]
140 144​
141 145​
142def jobs_from_current(wiki, force=False): 146​def jobs_from_current(wiki, force=False, arches=None):
143 """Schedule jobs against the 'current' release validation event 147​ """Schedule jobs against the 'current' release validation event
144 (according to wikitcms) if we have not already. Returns the job 148​ (according to wikitcms) if we have not already. Returns the job
145 list. If force is False, for each ISO, jobs will only be scheduled 149​ list. If force is False, for each ISO, jobs will only be scheduled
146 if there are no existing, non-cancelled jobs for the same ISO and 150​ if there are no existing, non-cancelled jobs for the same ISO and
147 flavor. 151​ flavor. If not set, load list of arches from config file.
148 """ 152​ """
149 if not wiki: 153​ if not wiki:
150 logger.warning("python-wikitcms is required for current validation event discovery.") 154​ logger.warning("python-wikitcms is required for current validation event discovery.")
151 return ([], None) 155​ return ([], None)
156​ # if arches weren't specified as argument, parse them from config file
157​ if not arches:
158​ arches = re.split(r'\W+', CONFIG.get('schedule', 'arches').strip())
152 last_versions, json_parsed = read_last() 159​ last_versions, json_parsed = read_last()
153 currev = wiki.current_event 160​ currev = wiki.current_event
154 logger.info("current event: %s", currev.version) 161​ logger.info("current event: %s", currev.version)
155 runarches = [] 162​ runarches = []
156 for arch in ARCHES: 163​ for arch in arches:
157 last_version = last_versions.get(arch, None) 164​ last_version = last_versions.get(arch, None)
158 if last_version and last_version >= currev.sortname: 165​ if last_version and last_version >= currev.sortname:
159 logger.info("skipped: %s: %s is newer or equal to %s", arch, last_version, currev.sortname) 166​ logger.info("skipped: %s: %s is newer or equal to %s", arch, last_version, currev.sortname)
160 else: 167​ else:
161 runarches.append(arch) 168​ runarches.append(arch)
162 logger.debug("%s will be tested in version %s", arch, currev.sortname) 169​ logger.debug("%s will be tested in version %s", arch, currev.sortname)
163 json_parsed[arch] = currev.sortname 170​ json_parsed[arch] = currev.sortname
164 171​
165 jobs = [] 172​ jobs = []
166 173​
167 if not runarches: 174​ if not runarches:
168 raise TriggerException("Skipped all arches, nothing to do.") 175​ raise TriggerException("Skipped all arches, nothing to do.")
169 176​
170 jobs = jobs_from_fedfind(currev.ff_release, arches=runarches, force=force) 177​ jobs = jobs_from_fedfind(currev.ff_release, arches=runarches, force=force)
171 logger.info("jobs_from_current: planned jobs: %s", ' '.join(str(j) for j in jobs)) 178​ logger.info("jobs_from_current: planned jobs: %s", ' '.join(str(j) for j in jobs))
172 179​
173 # write info about latest versions 180​ # write info about latest versions
174 f = open(PERSISTENT, "w") 181​ f = open(CONFIG.get('schedule', 'persistent'), "w")
175 f.write(json.dumps(json_parsed)) 182​ f.write(json.dumps(json_parsed))
176 f.close() 183​ f.close()
177 logger.debug("written info about newest version") 184​ logger.debug("written info about newest version")
178 185​
179 return jobs 186​ return jobs
180 187​
181 188​
182def jobs_from_compose(release='', milestone='', compose='', arches=None, force=False, wait=None): 189​def jobs_from_compose(release='', milestone='', compose='', arches=None, force=False, wait=None):
183 """Schedule jobs against a specific release/compose. Returns the 190​ """Schedule jobs against a specific release/compose. Returns the
184 list of job IDs. 191​ list of job IDs.
185 192​
186 'release', 'milestone' and 'compose' identify a release according 193​ 'release', 'milestone' and 'compose' identify a release according
187 to fedfind conventions (e.g. '23', 'Beta', 'TC2' or '24', 194​ to fedfind conventions (e.g. '23', 'Beta', 'TC2' or '24',
188 'Branched', '20160301'). A ValueError may be raised (by fedfind) 195​ 'Branched', '20160301'). A ValueError may be raised (by fedfind)
189 if there is something wrong with these values. 196​ if there is something wrong with these values.
190 197​
191 arches may be an iterable of arches to run on. If not specified, 198​ arches may be an iterable of arches to run on. If not specified,
192 the default ARCHES will be used. 199​ the default from config file will be used.
193 200​
194 If force is False, for each ISO, jobs will only be scheduled if 201​ If force is False, for each ISO, jobs will only be scheduled if
195 there are no existing, non-cancelled jobs for the same ISO and 202​ there are no existing, non-cancelled jobs for the same ISO and
196 flavor. 203​ flavor.
197 204​
198 'wait' is a number of minutes to wait for the compose to appear 205​ 'wait' is a number of minutes to wait for the compose to appear
199 before scheduling the jobs; if None, the value will be read from 206​ before scheduling the jobs; if None, the value will be read from
200 configuration (see config.py for default). Will raise WaitError if 207​ configuration (see config.py for default). Will raise WaitError if
Show All 17 Lines221 if wait:
218 except fedfind.exceptions.WaitError as err: 225​ except fedfind.exceptions.WaitError as err:
219 raise WaitError(err) 226​ raise WaitError(err)
220 227​
221 jobs = jobs_from_fedfind(ff_release, arches=arches, force=force) 228​ jobs = jobs_from_fedfind(ff_release, arches=arches, force=force)
222 logger.info("jobs_from_compose: planned jobs: %s", ' '.join(str(j) for j in jobs)) 229​ logger.info("jobs_from_compose: planned jobs: %s", ' '.join(str(j) for j in jobs))
223 return jobs 230​ return jobs
224 231​
225 232​
226def jobs_from_fedfind(ff_release, arches=None, force=False): 233​def jobs_from_fedfind(ff_release, force=False, arches=None, imagetypes=None, payloads=None):
227 """Given a fedfind.Release object, find the ISOs we want and run 234​ """Given a fedfind.Release object, find the ISOs we want and run
228 jobs on them. arches is an iterable of arches to run on, if not 235​ jobs on them. arches is an iterable of arches to run on, if not
229 specified, we'll use ARCHES (defined above). If force is False, 236​ specified, we'll use value from config file. If force is False,
230 for each ISO, jobs will only be scheduled if there are no 237​ for each ISO, jobs will only be scheduled if there are no
231 existing, non-cancelled jobs for the same ISO and flavor. 238​ existing, non-cancelled jobs for the same ISO and flavor.
232 """ 239​ """
240​ # if arches, imagetypes or payload weren't specified as argument, parse them from config file
233 if not arches: 241​ if not arches:
234 arches = ARCHES 242​ # split arches by words (works for both space- and comma-separated values)
243​ arches = re.split(r'\W+', CONFIG.get('schedule', 'arches').strip())
244​ if not imagetypes:
245​ # split imagetypes by words (works for both space- and comma-separated values)
246​ imagetypes = re.split(r'\W+', CONFIG.get('schedule', 'imagetype').strip())
247​ if not payloads:
248​ # split payloads by words (works for both space- and comma-separated values)
249​ payloads = re.split(r'\W+', CONFIG.get('schedule', 'payload').strip())
235 # Find currently-testable images for our arches. 250​ # Find currently-testable images for our arches.
236 jobs = [] 251​ jobs = []
237 queries = ( 252​ queries = (
238 fedfind.release.Query('imagetype', ('boot', 'live')), 253​ fedfind.release.Query('imagetype', imagetypes),
239 fedfind.release.Query('arch', arches), 254​ fedfind.release.Query('arch', arches),
240 fedfind.release.Query('payload', ('server', 'generic') + DESKTOPS)) 255​ fedfind.release.Query('payload', payloads))
241 logger.debug("querying fedfind for images") 256​ logger.debug("querying fedfind for images")
242 images = ff_release.find_images(queries) 257​ images = ff_release.find_images(queries)
243 258​
244 if len(images) == 0: 259​ if len(images) == 0:
245 raise TriggerException("no available images") 260​ raise TriggerException("no available images")
246 261​
247 # Now schedule jobs. First, let's get the BUILD value for openQA. 262​ # Now schedule jobs. First, let's get the BUILD value for openQA.
248 build = '_'.join((ff_release.release, ff_release.milestone, ff_release.compose)) 263​ build = '_'.join((ff_release.release, ff_release.milestone, ff_release.compose))
Show All 19 Lines282 for img in okimgs:
268 if img.imagetype == 'dvd': 283​ if img.imagetype == 'dvd':
269 score = 10 284​ score = 10
270 else: 285​ else:
271 score = 1 286​ score = 1
272 if img.payload == 'generic': 287​ if img.payload == 'generic':
273 score += 5 288​ score += 5
274 elif img.payload == 'server': 289​ elif img.payload == 'server':
275 score += 3 290​ score += 3
276 elif img.payload in DESKTOPS: 291​ elif img.payload == 'workstation':
adamwillUnsubmitted
Not Done

I don't actually like this, the original intent wasn't a catch-all +1 (which is actually meaningless - if we +1 everything we may as well just not do anything), it was specifically to +1 Workstation. It was meant to weakly prefer Workstation netinst/DVD to Cloud if we couldn't find Server or 'generic', IIRC. So it should go back to being 'workstation', I think.

277 score += 1 292​ score += 1
278 if score > bestscore: 293​ if score > bestscore:
279 bestimg = img 294​ bestimg = img
280 bestscore = score 295​ bestscore = score
281 if not bestimg: 296​ if not bestimg:
282 logger.warn("no universal tests image found for %s", arch) 297​ logger.warn("no universal tests image found for %s", arch)
283 continue 298​ continue
284 logger.info("running universal tests for %s with %s", arch, bestimg.desc) 299​ logger.info("running universal tests for %s with %s", arch, bestimg.desc)
Show All 11 Lines
View Options

schedule.conf.sample

Show All 19 Lines
20 20​
21[schedule] 21​[schedule]
22# Where to download ISOs (should be the directory openQA will look in) 22​# Where to download ISOs (should be the directory openQA will look in)
23iso-path: /var/lib/openqa/factory/iso/ 23​iso-path: /var/lib/openqa/factory/iso/
24 24​
25# Minutes to wait for the compose to be complete, when scheduling jobs for a 25​# Minutes to wait for the compose to be complete, when scheduling jobs for a
26# specific compose. 'current' never waits. 26​# specific compose. 'current' never waits.
27compose-wait: 480 27​compose-wait: 480
28​
29​# Image types to download, space-separated list - boot, live, dvd
30​imagetype: boot live dvd
31​
32​# Architectures to download, space-separated list - x86_64, i386
33​arches: x86_64 i386
34​
35​# Payloads to download, space-separated list - server, generic or one of desktops
36​payload: server generic workstation kde
37​
38​# Where file with info about latest checked version is located
39​persistent: /var/tmp/openqa_watcher.json