Differential D1094

createhdds: allow text installs, branched and rawhide installs
ClosedPublic
Actions

Authored by adamwill on Jan 25 2017, 11:20 AM.

Details

Reviewers
garretraziel_but_actually_jsedlak_who_uses_stupid_nicknames
tflink
jskladan
Summary

This is an attempt to add features desirable for creating
Taskotron base images. It extends the 'release' handling for
virt-install images in several ways to allow requesting of
'branched' and 'rawhide' image creation. It also adds an arg
to request virt-install image creation run in text mode, not
graphical mode. Graphical mode is what we always want for
openQA (so the installed OS doesn't have kernel params intended
for serial console interaction), but for Taskotron purposes,
we want the install run in text mode.

Test Plan

Try creating Rawhide and Branched images through
CLI (Branched for now should just tell you there is no branched,
we cannot test that creation works right now). Try creating
stable release images through CLI (make sure it still works
properly). Try creating Rawhide and Branched images from an
hdds.json file (will have to modify the existing hdds.json a
bit or create a simple test one). Check that creating stable
release images from the current hdds.json still works. Check
that the check feature works properly for both cases (stable
and Branched/Rawhide), including not expecting a Branched image
to exist when there is no Branched. Test using the -t arg to
run the install in text mode, and see if the text output is as
desired.

Diff Detail

Repository
rOPENQA fedora_openqa
Branch
textinst (branched from develop)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 980
Build 980: arc lint + arc unit
adamwill retitled this revision from to createhdds: allow text installs, branched and rawhide installs.Jan 25 2017, 11:20 AM
adamwill updated this object.
adamwill edited the test plan for this revision. (Show Details)
adamwill added reviewers: jskladan, tflink, garretraziel_but_actually_jsedlak_who_uses_stupid_nicknames.
garretraziel_but_actually_jsedlak_who_uses_stupid_nicknames added a comment.Jan 25 2017, 1:18 PM

Code works OK. Personally, I don't have a problem doing this in createhdds.py, but you should ask Josef whether it suits his usecase.

adamwill updated this revision to Diff 2787.Jan 25 2017, 1:24 PM

Simplify Branched handling

adamwill added a comment.EditedJan 25 2017, 1:28 PM

Just a quick note on the fedfind usage - current git master fedfind (and the 3.3.0 release, but I'm not shipping packages for that yet as I might want to take some of the features back out again now) caches the collections JSON that's used for get_current_release, so it's essentially free to call it multiple times, which is why I don't worry about doing that here. If you're using a slightly older fedfind it'll do multiple trips.

edit: and on the change to the libosinfo stuff - while thinking about how to handle that for Rawhide I realized the 'fedora-unknown' variant is treated by virt-install as meaning "just use the highest numbered Fedora variant that exists", so we don't need the old code that essentially does the same thing, if we can't find a variant for the actual release number. All we have to do is try to use the 'correct' variant, and if that's not available, use 'fedora-unknown', as that's always going to be the best choice we can make, it does the 'right thing' for Branched, Rawhide, and brand new releases that haven't made it into libvirt / libosinfo yet.

garretraziel_but_actually_jsedlak_who_uses_stupid_nicknames added a comment.Jan 25 2017, 3:00 PM

Only strange thing is that when you build image for Rawhide, it's called disk_frawhide_..., but it doesn't matter that much.

This revision is now accepted and ready to land.Jan 25 2017, 3:10 PM
adamwill added a comment.Jan 25 2017, 4:08 PM

Eh, I dunno if I'd call it 'strange'. It's just...correct. The "release" for Rawhide is 'rawhide', it's not '26' or anything like that. We're pretty consistent about that. I guess 'frawhide' looks slightly odder than 'f25', if that's all you meant :)

adamwill closed this revision.Feb 15 2017, 1:44 AM

Transferred to Pagure for the new createhdds project: https://pagure.io/fedora-qa/createhdds/pull-request/1

Revision Contents

PathPackages
Mtools/createhdds.py (94 lines)

Diff 2787

View Options

tools/createhdds.py

Show First 20 Lines • Show All 93 Lines • ▼ Show 20 Line(s)76 def __init__(self, name, size, imgver='', filesystem='ext4', label='mbr', parts=None,
94 self.filename = "disk_{0}".format(name) 94​ self.filename = "disk_{0}".format(name)
95 if imgver: 95​ if imgver:
96 self.filename = "{0}_{1}".format(self.filename, imgver) 96​ self.filename = "{0}_{1}".format(self.filename, imgver)
97 if name_extras: 97​ if name_extras:
98 for item in name_extras: 98​ for item in name_extras:
99 self.filename = "{0}_{1}".format(self.filename, item) 99​ self.filename = "{0}_{1}".format(self.filename, item)
100 self.filename = "{0}.img".format(self.filename) 100​ self.filename = "{0}.img".format(self.filename)
101 101​
102 def create(self): 102​ def create(self, _):
103 """Create the image.""" 103​ """Create the image. The unused arg is the 'textinst' arg that
104​ only VirtInstallImages care about (but which has to be passed
105​ here too).
106​ """
104 gfs = guestfs.GuestFS(python_return_dict=True) 107​ gfs = guestfs.GuestFS(python_return_dict=True)
105 try: 108​ try:
106 # Create the disk image with a temporary name 109​ # Create the disk image with a temporary name
107 tmpfile = "{0}.tmp".format(self.filename) 110​ tmpfile = "{0}.tmp".format(self.filename)
108 gfs.disk_create(tmpfile, "raw", int(self.size)) 111​ gfs.disk_create(tmpfile, "raw", int(self.size))
109 # 'launch' guestfs with the disk attached 112​ # 'launch' guestfs with the disk attached
110 gfs.add_drive_opts(tmpfile, format="raw", readonly=0) 113​ gfs.add_drive_opts(tmpfile, format="raw", readonly=0)
111 gfs.launch() 114​ gfs.launch()
▲ Show 20 Lines • Show All 73 Lines • ▼ Show 20 Line(s)182 def __init__(self, name, release, arch, size, variant=None, imgver='', maxage=14):
185 self.filename = "{0}_{1}.img".format(self.filename, arch) 188​ self.filename = "{0}_{1}.img".format(self.filename, arch)
186 self.release = release 189​ self.release = release
187 self.variant = variant 190​ self.variant = variant
188 self.arch = arch 191​ self.arch = arch
189 self.maxage = maxage 192​ self.maxage = maxage
190 if variant: 193​ if variant:
191 self.variant = variant 194​ self.variant = variant
192 else: 195​ else:
193 if release < 24: 196​ if str(release).isdigit() and int(release) < 24:
194 self.variant = "Server" 197​ self.variant = "Server"
195 else: 198​ else:
196 self.variant = "Everything" 199​ self.variant = "Everything"
197 200​
198 def create(self, retries=3): 201​ def create(self, textinst, retries=3):
199 """Create the image.""" 202​ """Create the image."""
200 # figure out the best os-variant. this is harder than it ought 203​ # figure out the best os-variant. NOTE: libosinfo >= 0.3.1
201 # to be thanks to RHBZ #1351718... 204​ # properly returns 1 on failure, but using workaround for old
202 found = False 205​ # bug where it didn't in case EPEL doesn't have 0.3.1
203 rel = self.release + 1 206​ shortid = "fedora{0}".format(self.release)
204 shortid = "" 207​ args = ["osinfo-query", "os", "short-id={0}".format(shortid)]
205 while not found: 208​ process = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL)
206 rel -= 1 209​ out = process.communicate()[0].decode()
207 shortid = "fedora{0}".format(str(rel)) 210​ if shortid not in out:
208 args = ["osinfo-query", "os", "short-id={0}".format(shortid)] 211​ # this will just use the most recent fedora release number
209 process = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL) 212​ # virt-install / osinfo knows
210 out = process.communicate()[0].decode() 213​ shortid = 'fedora-unknown'
211 if shortid in out: ​
212 found = True ​
213 214​
214 # destroy and delete the domain we use for all virt-installs 215​ # destroy and delete the domain we use for all virt-installs
215 conn = libvirt.open() 216​ conn = libvirt.open()
216 try: 217​ try:
217 dom = conn.lookupByName('createhdds') 218​ dom = conn.lookupByName('createhdds')
218 try: 219​ try:
219 dom.destroy() 220​ dom.destroy()
220 except libvirt.libvirtError: 221​ except libvirt.libvirtError:
221 # domain may not be running, so this is fine 222​ # domain may not be running, so this is fine
222 pass 223​ pass
223 dom.undefine() 224​ dom.undefine()
224 except libvirt.libvirtError: 225​ except libvirt.libvirtError:
225 # domain may not exist, so this is fine 226​ # domain may not exist, so this is fine
226 pass 227​ pass
227 conn.close() 228​ conn.close()
228 229​
229 tmpfile = "{0}.tmp".format(self.filename) 230​ tmpfile = "{0}.tmp".format(self.filename)
230 try: 231​ try:
231 loctmp = "https://download.fedoraproject.org/pub/fedora/linux/releases/{0}/{1}/{2}/os" 232​ # this is almost complex enough to need fedfind but not
233​ # quite, I think. also fedfind can't find the 'transient'
234​ # rawhide and branched locations at present
235​ if self.release == 'rawhide':
236​ loctmp = "https://dl.fedoraproject.org/pub/fedora/linux/development/rawhide/{1}/{2}/os"
237​ elif int(self.release) > fedfind.helpers.get_current_release(branched=False):
238​ # branched
239​ loctmp = "https://dl.fedoraproject.org/pub/fedora/linux/development/"
240​ loctmp += self.release[1:]
241​ loctmp += "/{1}/{2}/os"
242​ else:
243​ loctmp = "https://download.fedoraproject.org/pub/fedora/linux/releases/{0}/{1}/{2}/os"
232 arch = self.arch 244​ arch = self.arch
233 if arch == 'i686': 245​ if arch == 'i686':
234 arch = 'i386' 246​ arch = 'i386'
235 args = ["virt-install", "--disk", "size={0},path={1}".format(self.size, tmpfile), 247​ args = ["virt-install", "--disk", "size={0},path={1}".format(self.size, tmpfile),
236 "--os-variant", shortid, "-x", 248​ "--os-variant", shortid, "-x",
237 "inst.ks=file:/{0}.ks".format(self.name), "--initrd-inject", 249​ "inst.ks=file:/{0}.ks".format(self.name), "--initrd-inject",
238 "{0}/{1}.ks".format(SCRIPTDIR, self.name), "--location", 250​ "{0}/{1}.ks".format(SCRIPTDIR, self.name), "--location",
239 loctmp.format(str(self.release), self.variant, arch), "--graphics", "vnc", 251​ loctmp.format(str(self.release), self.variant, arch), "--name", "createhdds",
240 "--name", "createhdds", "--memory", "2048", "--noreboot", "--noautoconsole", 252​ "--memory", "2048", "--noreboot", "--wait", "-1"]
241 "--wait", "-1"] 253​ if textinst:
254​ args.extend(("--graphics", "none", "--extra-args", "console=ttyS0"))
255​ else:
256​ args.extend(("--graphics", "vnc", "--noautoconsole"))
242 # this is a hacky workaround for a weird bug on Fedora's prod 257​ # this is a hacky workaround for a weird bug on Fedora's prod
243 # openQA server: 258​ # openQA server:
244 # https://bugzilla.redhat.com/show_bug.cgi?id=1387798 259​ # https://bugzilla.redhat.com/show_bug.cgi?id=1387798
245 args.extend(("--network", "user")) 260​ args.extend(("--network", "user"))
246 # run the command, timing out after 1 hour; sometimes creation 261​ # run the command, timing out after 1 hour; sometimes creation
247 # seems to just get mysteriously stuck, we need to bail and 262​ # seems to just get mysteriously stuck, we need to bail and
248 # retry in this case 263​ # retry in this case
249 try: 264​ try:
250 logger.info("Install running, connect via VNC to monitor") 265​ logger.info("Install starting...")
266​ logger.debug("Command: %s", ' '.join(args))
267​ if not textinst:
268​ logger.info("Connect via VNC to monitor")
251 ret = subprocess.call(args, timeout=3600) 269​ ret = subprocess.call(args, timeout=3600)
252 except subprocess.TimeoutExpired: 270​ except subprocess.TimeoutExpired:
253 logger.warning("Image creation timed out!") 271​ logger.warning("Image creation timed out!")
254 if os.path.isfile(tmpfile): 272​ if os.path.isfile(tmpfile):
255 os.remove(tmpfile) 273​ os.remove(tmpfile)
256 if retries: 274​ if retries:
257 logger.info("Retrying: %s retries remain after this", str(retries)) 275​ logger.info("Retrying: %s retries remain after this", str(retries))
258 return self.create(retries=retries-1) 276​ return self.create(retries=retries-1)
▲ Show 20 Lines • Show All 121 Lines • ▼ Show 20 Line(s)374def get_virtinstall_images(imggrp, nextrel=None, releases=None):
380 maxage = int(imggrp.get('maxage', 14)) 398​ maxage = int(imggrp.get('maxage', 14))
381 # ditto variant 399​ # ditto variant
382 variant = imggrp.get('variant') 400​ variant = imggrp.get('variant')
383 if not releases: 401​ if not releases:
384 releases = imggrp['releases'] 402​ releases = imggrp['releases']
385 size = imggrp.get('size', 0) 403​ size = imggrp.get('size', 0)
386 imgver = imggrp.get('imgver') 404​ imgver = imggrp.get('imgver')
387 # add an image for each release/arch combination 405​ # add an image for each release/arch combination
388 for (relnum, arches) in releases.items(): 406​ for (release, arches) in releases.items():
389 if int(relnum) < 0: 407​ if release.lower() == 'branched':
390 # negative relnum indicates 'relative to next release' 408​ # find Branched, if it exists
409​ curr = fedfind.helpers.get_current_release(branched=False)
410​ branch = fedfind.helpers.get_current_release(branched=True)
411​ if branch > curr:
412​ release = branch
413​ else:
414​ logger.info("Branched image requested, but Branched does not currently exist")
415​ continue
416​ elif release != 'rawhide' and int(release) < 0:
417​ # negative release indicates 'relative to next release'
391 if not nextrel: 418​ if not nextrel:
392 nextrel = fedfind.helpers.get_current_release() + 1 419​ nextrel = fedfind.helpers.get_current_release() + 1
393 relnum = int(nextrel) + int(relnum) 420​ release = int(nextrel) + int(release)
394 for arch in arches: 421​ for arch in arches:
395 imgs.append( 422​ imgs.append(
396 VirtInstallImage(name, relnum, arch, variant=variant, size=size, imgver=imgver, 423​ VirtInstallImage(name, release, arch, variant=variant, size=size, imgver=imgver,
397 maxage=maxage)) 424​ maxage=maxage))
398 return imgs 425​ return imgs
399 426​
400def get_all_images(hdds, nextrel=None): 427​def get_all_images(hdds, nextrel=None):
401 """Simply iterates over the 'image group' dicts in hdds.json and 428​ """Simply iterates over the 'image group' dicts in hdds.json and
402 calls the appropriate get_foo_images() function for each, then 429​ calls the appropriate get_foo_images() function for each, then
403 returns the list of all images. No overrides for labels, 430​ returns the list of all images. No overrides for labels,
404 filesystems, releases or arches are provided here, this function 431​ filesystems, releases or arches are provided here, this function
▲ Show 20 Lines • Show All 112 Lines • ▼ Show 20 Line(s)527def cli_all(args, hdds):
517 if args.clean: 544​ if args.clean:
518 clean(unknown) 545​ clean(unknown)
519 546​
520 # 'missing' plus 'outdated' is all the images we need to build; if 547​ # 'missing' plus 'outdated' is all the images we need to build; if
521 # args.delete was set, all images will be in this list 548​ # args.delete was set, all images will be in this list
522 missing.extend(outdated) 549​ missing.extend(outdated)
523 for (num, img) in enumerate(missing, 1): 550​ for (num, img) in enumerate(missing, 1):
524 logger.info("Creating image %s...[%s/%s]", img.filename, str(num), str(len(missing))) 551​ logger.info("Creating image %s...[%s/%s]", img.filename, str(num), str(len(missing)))
525 img.create() 552​ img.create(args.textinst)
526 553​
527def cli_check(args, hdds): 554​def cli_check(args, hdds):
528 """Function for the CLI 'check' subcommand. Basically just calls 555​ """Function for the CLI 'check' subcommand. Basically just calls
529 check() and prints the results. Does renames before checking if 556​ check() and prints the results. Does renames before checking if
530 args.rename was set, and wipes 'unknown' images after checking if 557​ args.rename was set, and wipes 'unknown' images after checking if
531 args.clean was set. Exits with status 2 if any images are missing, 558​ args.clean was set. Exits with status 2 if any images are missing,
532 1 if all images are present but one or more is outdated. 559​ 1 if all images are present but one or more is outdated.
533 """ 560​ """
▲ Show 20 Lines • Show All 61 Lines • ▼ Show 20 Line(s)621 if args.arch:
595 arches = [args.arch] 622​ arches = [args.arch]
596 else: 623​ else:
597 arches = ['x86_64'] 624​ arches = ['x86_64']
598 releases = {args.release: arches} 625​ releases = {args.release: arches}
599 imgs = get_virtinstall_images(imggrp, releases=releases) 626​ imgs = get_virtinstall_images(imggrp, releases=releases)
600 627​
601 for (num, img) in enumerate(imgs, 1): 628​ for (num, img) in enumerate(imgs, 1):
602 logger.info("Creating image %s...[%s/%s]", img.filename, str(num), str(len(imgs))) 629​ logger.info("Creating image %s...[%s/%s]", img.filename, str(num), str(len(imgs)))
603 img.create() 630​ img.create(args.textinst)
604 631​
605def parse_args(hdds): 632​def parse_args(hdds):
606 """Parse arguments with argparse.""" 633​ """Parse arguments with argparse."""
607 parser = argparse.ArgumentParser(description=( 634​ parser = argparse.ArgumentParser(description=(
608 "Tool for creating hard disk images for Fedora openQA.")) 635​ "Tool for creating hard disk images for Fedora openQA."))
609 parser.add_argument( 636​ parser.add_argument(
610 '-l', '--loglevel', help="The level of log messages to show", 637​ '-l', '--loglevel', help="The level of log messages to show",
611 choices=('debug', 'info', 'warning', 'error', 'critical'), 638​ choices=('debug', 'info', 'warning', 'error', 'critical'),
612 default='info') 639​ default='info')
640​ parser.add_argument(
641​ '-t', '--textinst', help="For any virt-install images, run the install in text mode "
642​ "and show details on stdout", action='store_true')
613 643​
614 # This is a workaround for a somewhat infamous argparse bug 644​ # This is a workaround for a somewhat infamous argparse bug
615 # in Python 3. See: 645​ # in Python 3. See:
616 # https://stackoverflow.com/questions/23349349/argparse-with-required-subparser 646​ # https://stackoverflow.com/questions/23349349/argparse-with-required-subparser
617 # http://bugs.python.org/issue16308 647​ # http://bugs.python.org/issue16308
618 subparsers = parser.add_subparsers(dest='subcommand') 648​ subparsers = parser.add_subparsers(dest='subcommand')
619 subparsers.required = True 649​ subparsers.required = True
620 650​
▲ Show 20 Lines • Show All 55 Lines • ▼ Show 20 Line(s)
676 # arch combination; using args.release will always result in just a 706​ # arch combination; using args.release will always result in just a
677 # single image being built, for x86_64 unless args.arch is set to 707​ # single image being built, for x86_64 unless args.arch is set to
678 # i686. 708​ # i686.
679 for imggrp in hdds['virtinstall']: 709​ for imggrp in hdds['virtinstall']:
680 imgparser = subparsers.add_parser( 710​ imgparser = subparsers.add_parser(
681 imggrp['name'], description="Create {0} image(s)".format(imggrp['name'])) 711​ imggrp['name'], description="Create {0} image(s)".format(imggrp['name']))
682 imgparser.add_argument( 712​ imgparser.add_argument(
683 '-r', '--release', help="The release to build the image(s) for. If not " 713​ '-r', '--release', help="The release to build the image(s) for. If not "
684 "set or set to 0, createhdds will attempt to determine the current " 714​ "set, createhdds will attempt to determine the current release and build "
685 "release and build for appropriate releases relative to that", 715​ "for appropriate releases relative to that",
686 type=int, default=0) 716​ default='')
687 imgparser.add_argument( 717​ imgparser.add_argument(
688 '-a', '--arch', help="The arch to build the image(s) for. If neither " 718​ '-a', '--arch', help="The arch to build the image(s) for. If neither "
689 "this nor --release is set, createhdds will decide the appropriate " 719​ "this nor --release is set, createhdds will decide the appropriate "
690 "arch(es) to build for each release. If this is not set but --release " 720​ "arch(es) to build for each release. If this is not set but --release "
691 "is set, only x86_64 image(s) will be built.", 721​ "is set, only x86_64 image(s) will be built.",
692 choices=('x86_64', 'i686')) 722​ choices=('x86_64', 'i686'))
693 imgparser.set_defaults(func=cli_image) 723​ imgparser.set_defaults(func=cli_image)
694 # Here we're stuffing the type of the image and the dict from 724​ # Here we're stuffing the type of the image and the dict from
Show All 26 Lines