Clarify the ks repo only error message
This also moves the run_creator kickstart checks into check_kickstart so that tests may be added. Related: rhbz#1673744
This commit is contained in:
Brian C. Lane
parent
3d72aea6b2
commit
1f716641cf
@ -578,6 +578,47 @@ def make_live_images(opts, work_dir, disk_img):
|
||||
|
||||
return work_dir
|
||||
|
||||
def check_kickstart(ks, opts):
|
||||
"""Check the parsed kickstart object for errors
|
||||
|
||||
:param ks: Parsed Kickstart object
|
||||
:type ks: pykickstart.parser.KickstartParser
|
||||
:param opts: Commandline options to control the process
|
||||
:type opts: Either a DataHolder or ArgumentParser
|
||||
:returns: List of error strings or empty list
|
||||
:rtype: list
|
||||
"""
|
||||
errors = []
|
||||
if opts.no_virt and ks.handler.method.method not in ("url", "nfs") \
|
||||
and not ks.handler.ostreesetup.seen:
|
||||
errors.append("Only url, nfs and ostreesetup install methods are currently supported."
|
||||
"Please fix your kickstart file." )
|
||||
|
||||
if ks.handler.repo.seen and ks.handler.method.method != "url":
|
||||
errors.append("repo can only be used with the url install method. Add url to your "
|
||||
"kickstart file.")
|
||||
|
||||
if ks.handler.method.method in ("url", "nfs") and not ks.handler.network.seen:
|
||||
errors.append("The kickstart must activate networking if "
|
||||
"the url or nfs install method is used.")
|
||||
|
||||
if ks.handler.displaymode.displayMode is not None:
|
||||
errors.append("The kickstart must not set a display mode (text, cmdline, "
|
||||
"graphical), this will interfere with livemedia-creator.")
|
||||
|
||||
if opts.make_fsimage or (opts.make_pxe_live and opts.no_virt):
|
||||
# Make sure the kickstart isn't using autopart and only has a / mountpoint
|
||||
part_ok = not any(p for p in ks.handler.partition.partitions
|
||||
if p.mountpoint not in ["/", "swap"])
|
||||
if not part_ok or ks.handler.autopart.seen:
|
||||
errors.append("Filesystem images must use a single / part, not autopart or "
|
||||
"multiple partitions. swap is allowed but not used.")
|
||||
|
||||
if not opts.no_virt and ks.handler.reboot.action != KS_SHUTDOWN:
|
||||
errors.append("The kickstart must include shutdown when using virt installation.")
|
||||
|
||||
return errors
|
||||
|
||||
def run_creator(opts, cancel_func=None):
|
||||
"""Run the image creator process
|
||||
|
||||
@ -612,31 +653,8 @@ def run_creator(opts, cancel_func=None):
|
||||
if not opts.ks:
|
||||
raise RuntimeError("Image creation requires a kickstart file")
|
||||
|
||||
errors = []
|
||||
if opts.no_virt and ks.handler.method.method not in ("url", "nfs") \
|
||||
and not ks.handler.ostreesetup.seen:
|
||||
errors.append("Only url, nfs and ostreesetup install methods are currently supported."
|
||||
"Please fix your kickstart file." )
|
||||
|
||||
if ks.handler.method.method in ("url", "nfs") and not ks.handler.network.seen:
|
||||
errors.append("The kickstart must activate networking if "
|
||||
"the url or nfs install method is used.")
|
||||
|
||||
if ks.handler.displaymode.displayMode is not None:
|
||||
errors.append("The kickstart must not set a display mode (text, cmdline, "
|
||||
"graphical), this will interfere with livemedia-creator.")
|
||||
|
||||
if opts.make_fsimage or (opts.make_pxe_live and opts.no_virt):
|
||||
# Make sure the kickstart isn't using autopart and only has a / mountpoint
|
||||
part_ok = not any(p for p in ks.handler.partition.partitions
|
||||
if p.mountpoint not in ["/", "swap"])
|
||||
if not part_ok or ks.handler.autopart.seen:
|
||||
errors.append("Filesystem images must use a single / part, not autopart or "
|
||||
"multiple partitions. swap is allowed but not used.")
|
||||
|
||||
if not opts.no_virt and ks.handler.reboot.action != KS_SHUTDOWN:
|
||||
errors.append("The kickstart must include shutdown when using virt installation.")
|
||||
|
||||
# Check the kickstart for problems
|
||||
errors = check_kickstart(ks, opts)
|
||||
if errors:
|
||||
list(log.error(e) for e in errors)
|
||||
raise RuntimeError("\n".join(errors))
|
||||
|
||||
@ -19,11 +19,15 @@ import tempfile
|
||||
import unittest
|
||||
import xml.etree.ElementTree as ET
|
||||
|
||||
# For kickstart check tests
|
||||
from pykickstart.parser import KickstartParser
|
||||
from pykickstart.version import makeVersion
|
||||
|
||||
from ..lib import get_file_magic
|
||||
from pylorax import find_templates
|
||||
from pylorax.base import DataHolder
|
||||
from pylorax.creator import FakeDNF, create_pxe_config, make_appliance, make_squashfs, squashfs_args
|
||||
from pylorax.creator import get_arch, find_ostree_root
|
||||
from pylorax.creator import get_arch, find_ostree_root, check_kickstart
|
||||
from pylorax.executils import runcmd
|
||||
from pylorax.imgutils import mksparse
|
||||
from pylorax.sysutils import joinpaths
|
||||
@ -138,6 +142,108 @@ class CreatorTest(unittest.TestCase):
|
||||
os.makedirs(joinpaths(work_dir, ostree_path))
|
||||
self.assertEqual(find_ostree_root(work_dir), ostree_path)
|
||||
|
||||
def good_ks_novirt_test(self):
|
||||
"""Test a good kickstart with novirt"""
|
||||
opts = DataHolder(no_virt=True, make_fsimage=False, make_pxe_live=False)
|
||||
ks_version = makeVersion()
|
||||
ks = KickstartParser(ks_version, errorsAreFatal=False, missingIncludeIsFatal=False)
|
||||
ks.readKickstartFromString("url --url=http://dl.fedoraproject.com\n"
|
||||
"network --bootproto=dhcp --activate\n"
|
||||
"repo --name=other --baseurl=http://dl.fedoraproject.com\n"
|
||||
"part / --size=4096\n"
|
||||
"shutdown\n")
|
||||
self.assertEqual(check_kickstart(ks, opts), [])
|
||||
|
||||
def good_ks_virt_test(self):
|
||||
"""Test a good kickstart with virt"""
|
||||
opts = DataHolder(no_virt=False, make_fsimage=False, make_pxe_live=False)
|
||||
ks_version = makeVersion()
|
||||
ks = KickstartParser(ks_version, errorsAreFatal=False, missingIncludeIsFatal=False)
|
||||
ks.readKickstartFromString("url --url=http://dl.fedoraproject.com\n"
|
||||
"network --bootproto=dhcp --activate\n"
|
||||
"repo --name=other --baseurl=http://dl.fedoraproject.com\n"
|
||||
"part / --size=4096\n"
|
||||
"shutdown\n")
|
||||
self.assertEqual(check_kickstart(ks, opts), [])
|
||||
|
||||
def nomethod_novirt_test(self):
|
||||
"""Test a kickstart with repo and no url"""
|
||||
opts = DataHolder(no_virt=True, make_fsimage=False, make_pxe_live=False)
|
||||
ks_version = makeVersion()
|
||||
ks = KickstartParser(ks_version, errorsAreFatal=False, missingIncludeIsFatal=False)
|
||||
ks.readKickstartFromString("network --bootproto=dhcp --activate\n"
|
||||
"repo --name=other --baseurl=http://dl.fedoraproject.com\n"
|
||||
"part / --size=4096\n"
|
||||
"shutdown\n")
|
||||
errors = check_kickstart(ks, opts)
|
||||
self.assertTrue("Only url, nfs and ostreesetup" in errors[0])
|
||||
self.assertTrue("repo can only be used with the url" in errors[1])
|
||||
|
||||
def no_network_test(self):
|
||||
"""Test a kickstart with missing network command"""
|
||||
opts = DataHolder(no_virt=True, make_fsimage=False, make_pxe_live=False)
|
||||
ks_version = makeVersion()
|
||||
ks = KickstartParser(ks_version, errorsAreFatal=False, missingIncludeIsFatal=False)
|
||||
ks.readKickstartFromString("url --url=http://dl.fedoraproject.com\n"
|
||||
"part / --size=4096\n"
|
||||
"shutdown\n")
|
||||
errors = check_kickstart(ks, opts)
|
||||
self.assertTrue("The kickstart must activate networking" in errors[0])
|
||||
|
||||
def displaymode_test(self):
|
||||
"""Test a kickstart with displaymode set"""
|
||||
opts = DataHolder(no_virt=True, make_fsimage=False, make_pxe_live=False)
|
||||
ks_version = makeVersion()
|
||||
ks = KickstartParser(ks_version, errorsAreFatal=False, missingIncludeIsFatal=False)
|
||||
ks.readKickstartFromString("url --url=http://dl.fedoraproject.com\n"
|
||||
"network --bootproto=dhcp --activate\n"
|
||||
"repo --name=other --baseurl=http://dl.fedoraproject.com\n"
|
||||
"part / --size=4096\n"
|
||||
"shutdown\n"
|
||||
"graphical\n")
|
||||
errors = check_kickstart(ks, opts)
|
||||
self.assertTrue("must not set a display mode" in errors[0])
|
||||
|
||||
def autopart_test(self):
|
||||
"""Test a kickstart with autopart"""
|
||||
opts = DataHolder(no_virt=True, make_fsimage=True, make_pxe_live=False)
|
||||
ks_version = makeVersion()
|
||||
ks = KickstartParser(ks_version, errorsAreFatal=False, missingIncludeIsFatal=False)
|
||||
ks.readKickstartFromString("url --url=http://dl.fedoraproject.com\n"
|
||||
"network --bootproto=dhcp --activate\n"
|
||||
"repo --name=other --baseurl=http://dl.fedoraproject.com\n"
|
||||
"autopart\n"
|
||||
"shutdown\n")
|
||||
errors = check_kickstart(ks, opts)
|
||||
self.assertTrue("Filesystem images must use a single" in errors[0])
|
||||
|
||||
def boot_part_test(self):
|
||||
"""Test a kickstart with a boot partition"""
|
||||
opts = DataHolder(no_virt=True, make_fsimage=True, make_pxe_live=False)
|
||||
ks_version = makeVersion()
|
||||
ks = KickstartParser(ks_version, errorsAreFatal=False, missingIncludeIsFatal=False)
|
||||
ks.readKickstartFromString("url --url=http://dl.fedoraproject.com\n"
|
||||
"network --bootproto=dhcp --activate\n"
|
||||
"repo --name=other --baseurl=http://dl.fedoraproject.com\n"
|
||||
"part / --size=4096\n"
|
||||
"part /boot --size=1024\n"
|
||||
"shutdown\n")
|
||||
errors = check_kickstart(ks, opts)
|
||||
self.assertTrue("Filesystem images must use a single" in errors[0])
|
||||
|
||||
def shutdown_virt_test(self):
|
||||
"""Test a kickstart with reboot instead of shutdown"""
|
||||
opts = DataHolder(no_virt=False, make_fsimage=True, make_pxe_live=False)
|
||||
ks_version = makeVersion()
|
||||
ks = KickstartParser(ks_version, errorsAreFatal=False, missingIncludeIsFatal=False)
|
||||
ks.readKickstartFromString("url --url=http://dl.fedoraproject.com\n"
|
||||
"network --bootproto=dhcp --activate\n"
|
||||
"repo --name=other --baseurl=http://dl.fedoraproject.com\n"
|
||||
"part / --size=4096\n"
|
||||
"reboot\n")
|
||||
errors = check_kickstart(ks, opts)
|
||||
self.assertTrue("must include shutdown when using virt" in errors[0])
|
||||
|
||||
@unittest.skipUnless(os.geteuid() == 0 and not os.path.exists("/.in-container"), "requires root privileges, and no containers")
|
||||
def boot_over_root_test(self):
|
||||
"""Test the mount_boot_part_over_root ostree function"""
|
||||
|
||||
Loading…
Reference in New Issue
Block a user