Differential D858

createhdds: use a temp file for virt-builder image creation
ClosedPublic
Actions

Authored by adamwill on May 20 2016, 4:06 PM.

Details

Reviewers
garretraziel
jskladan
Commits
rOPENQAde4261023d4d: createhdds: use a temp file for virt-builder image creation
Summary

Creating the virt-builder images directly with their final file
name means that if we're rebuilding them for age and the build
fails, we'll lose the existing image: it seems better to keep
it than have no image at all, when this happens. It also means
that while the rebuild is in process, the file might exist but
be useless and cause any tests that happen to be running at the
time to fail. So just like the guestfs images, create the file
with a .tmp extension initially and rename it after a successful
build.

Test Plan

Do some image builds and check they work, check that
temp file is cleaned up on failure and ctrl-c if you can...

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.
adamwill retitled this revision from to createhdds: use a temp file for virt-builder image creation.May 20 2016, 4:06 PM
adamwill updated this object.
adamwill edited the test plan for this revision. (Show Details)
adamwill added reviewers: jskladan, garretraziel.
adamwill updated this revision to Diff 2210.May 20 2016, 10:17 PM

add some timeouts for virt-builder too

garretraziel accepted this revision.May 23 2016, 11:13 AM

Looks OK.

This revision is now accepted and ready to land.May 23 2016, 11:13 AM
Closed by commit rOPENQAde4261023d4d: createhdds: use a temp file for virt-builder image creation (authored by adamwill). · Explain WhyMay 23 2016, 1:55 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathPackages
Mtools/createhdds.py (90 lines)

Diff 2211

View Options

tools/createhdds.py

Show First 20 Lines • Show All 200 Lines • ▼ Show 20 Line(s)198 def __init__(self, name, release, arch, size=0, imgver='', maxage=14):
201 self.filename = "disk_f{0}_{1}".format(str(release), name) 201​ self.filename = "disk_f{0}_{1}".format(str(release), name)
202 if imgver: 202​ if imgver:
203 self.filename = "{0}_{1}".format(self.filename, imgver) 203​ self.filename = "{0}_{1}".format(self.filename, imgver)
204 self.filename = "{0}_{1}.img".format(self.filename, arch) 204​ self.filename = "{0}_{1}.img".format(self.filename, arch)
205 self.release = release 205​ self.release = release
206 self.arch = arch 206​ self.arch = arch
207 self.maxage = maxage 207​ self.maxage = maxage
208 208​
209 def create(self): 209​ def create(self, retries=3):
210 """Create the image.""" 210​ """Create the image."""
211 # Basic creation command with standard params 211​ tmpfile = "{0}.tmp".format(self.filename)
212 args = ["virt-builder", "fedora-{0}".format(str(self.release)), "-o", self.filename, 212​ try:
213 "--arch", self.arch] 213​ # Basic creation command with standard params
214 if self.size: 214​ args = ["virt-builder", "fedora-{0}".format(str(self.release)), "-o", tmpfile,
215 args.extend(["--size", "{0}b".format(str(int(self.size)))]) 215​ "--arch", self.arch]
216 # We use guestfs's ability to read customization commands from 216​ if self.size:
217 # a file. The convention is to have a file 'name.commands' in 217​ args.extend(["--size", "{0}b".format(str(int(self.size)))])
218 # SCRIPTDIR; if this file exists, we pass it to virt-builder. 218​ # We use guestfs's ability to read customization commands from
219 if os.path.isfile("{0}/{1}.commands".format(SCRIPTDIR, self.name)): 219​ # a file. The convention is to have a file 'name.commands' in
220 args.extend(["--commands-from-file", "{0}/{1}.commands".format(SCRIPTDIR, self.name)]) 220​ # SCRIPTDIR; if this file exists, we pass it to virt-builder.
221 ret = subprocess.call(args) 221​ if os.path.isfile("{0}/{1}.commands".format(SCRIPTDIR, self.name)):
222 if ret > 0: 222​ args.extend(["--commands-from-file", "{0}/{1}.commands".format(SCRIPTDIR, self.name)])
223 sys.exit("virt-builder command {0} failed!".format(' '.join(args))) 223​ # run the command, timing out after 1 hour; sometimes creation
224 # We have to boot the disk to make SELinux relabelling happen; 224​ # seems to just get mysteriously stuck, we need to bail and
225 # virt-builder can't do it unless the policy version on the host 225​ # retry in this case
226 # is the same as the guest(?) There are lots of bad ways to do 226​ try:
227 # this, expect is the one we're using. 227​ ret = subprocess.call(args, timeout=3600)
228 child = pexpect.spawnu("qemu-kvm -m 2G -nographic {0}".format(self.filename), timeout=None) 228​ except subprocess.TimeoutExpired:
229 child.expect(u"localhost login:") 229​ logger.warning("Image creation timed out!")
230 child.sendline(u"root") 230​ if os.path.isfile(tmpfile):
231 child.expect(u"Password:") 231​ os.remove(tmpfile)
232 child.sendline(u"weakpassword") 232​ if retries:
233 child.expect(u"~]#") 233​ logger.info("Retrying: %s retries remain after this", str(retries))
234 child.sendline(u"poweroff") 234​ return self.create(retries=retries-1)
235 child.expect(u"reboot: Power down") 235​ else:
236 child.close() 236​ sys.exit("Image creation timed out too many times!")
237​ if ret > 0:
238​ # wipe the temp file
239​ if os.path.isfile(tmpfile):
240​ os.remove(tmpfile)
241​ sys.exit("virt-builder command {0} failed!".format(' '.join(args)))
242​ # We have to boot the disk to make SELinux relabelling happen;
243​ # virt-builder can't do it unless the policy version on the host
244​ # is the same as the guest(?) There are lots of bad ways to do
245​ # this, expect is the one we're using. This can also get stuck
246​ # apparently, so let's set a 300 sec timeout for each expect
247​ # and bail if it's hit.
248​ try:
249​ logger.info("Booting image to trigger SELinux relabel...")
250​ child = pexpect.spawnu("qemu-kvm -m 2G -nographic {0}".format(tmpfile), timeout=300)
251​ child.expect(u"localhost login:")
252​ child.sendline(u"root")
253​ child.expect(u"Password:")
254​ child.sendline(u"weakpassword")
255​ child.expect(u"~]#")
256​ child.sendline(u"poweroff")
257​ child.expect(u"reboot: Power down")
258​ child.close()
259​ except pexpect.TIMEOUT:
260​ if os.path.isfile(tmpfile):
261​ os.remove(tmpfile)
262​ sys.exit("Timed out booting image!")
263​ # we're all done! rename to the correct name
264​ os.rename(tmpfile, self.filename)
265​ except:
266​ # if anything went wrong, we want to wipe the temp file
267​ # then raise
268​ if os.path.isfile(tmpfile):
269​ os.remove(tmpfile)
270​ raise
237 271​
238 @property 272​ @property
239 def outdated(self): 273​ def outdated(self):
240 """Whether the image is outdated - if self.maxage is set and 274​ """Whether the image is outdated - if self.maxage is set and
241 it's older than that. 275​ it's older than that.
242 """ 276​ """
243 if not os.path.isfile(self.filename): 277​ if not os.path.isfile(self.filename):
244 return False 278​ return False
▲ Show 20 Lines • Show All 403 Lines • ▼ Show 20 Line(s)681 with open('{0}/hdds.json'.format(SCRIPTDIR), 'r') as fout:
648 hdds = json.load(fout) 682​ hdds = json.load(fout)
649 args = parse_args(hdds) 683​ args = parse_args(hdds)
650 loglevel = getattr( 684​ loglevel = getattr(
651 logging, args.loglevel.upper(), logging.INFO) 685​ logging, args.loglevel.upper(), logging.INFO)
652 logging.basicConfig(level=loglevel) 686​ logging.basicConfig(level=loglevel)
653 args.func(args, hdds) 687​ args.func(args, hdds)
654 except KeyboardInterrupt: 688​ except KeyboardInterrupt:
655 sys.stderr.write("Interrupted, exiting...\n") 689​ sys.stderr.write("Interrupted, exiting...\n")
656 # there may be a guestfs temp image file lying around... 690​ # there may be temp image files lying around...
657 tmps = [fl for fl in os.listdir('.') if fl.startswith('disk') and fl.endswith('.tmp')] 691​ tmps = [fl for fl in os.listdir('.') if fl.startswith('disk') and fl.endswith('.tmp')]
658 for tmp in tmps: 692​ for tmp in tmps:
659 os.remove(tmp) 693​ os.remove(tmp)
660 sys.exit(1) 694​ sys.exit(1)
661 695​
662if __name__ == '__main__': 696​if __name__ == '__main__':
663 main() 697​ main()