diff --git a/share/composer/alibaba.ks b/share/composer/alibaba.ks index 614fc88a..e68d0d09 100644 --- a/share/composer/alibaba.ks +++ b/share/composer/alibaba.ks @@ -16,8 +16,6 @@ selinux --enforcing logging --level=info # Shutdown after installation shutdown -# System timezone -timezone US/Eastern # System bootloader configuration bootloader --location=mbr diff --git a/share/composer/ami.ks b/share/composer/ami.ks index 0b416451..f2decf7b 100644 --- a/share/composer/ami.ks +++ b/share/composer/ami.ks @@ -16,8 +16,6 @@ selinux --enforcing logging --level=info # Shutdown after installation shutdown -# System timezone -timezone US/Eastern # System bootloader configuration bootloader --location=mbr --append="no_timer_check console=ttyS0,115200n8 console=tty1 net.ifnames=0" # Add platform specific partitions diff --git a/share/composer/ext4-filesystem.ks b/share/composer/ext4-filesystem.ks index 90e2a364..a60a6cb4 100644 --- a/share/composer/ext4-filesystem.ks +++ b/share/composer/ext4-filesystem.ks @@ -16,8 +16,6 @@ selinux --enforcing logging --level=info # Shutdown after installation shutdown -# System timezone -timezone US/Eastern # System bootloader configuration (unpartitioned fs image doesn't use a bootloader) bootloader --location=none diff --git a/share/composer/live-iso.ks b/share/composer/live-iso.ks index a88e8c43..970eb22a 100644 --- a/share/composer/live-iso.ks +++ b/share/composer/live-iso.ks @@ -21,8 +21,6 @@ logging --level=info shutdown # System services services --disabled="network,sshd" --enabled="NetworkManager" -# System timezone -timezone US/Eastern # System bootloader configuration bootloader --location=none diff --git a/share/composer/openstack.ks b/share/composer/openstack.ks index 11226b71..b5849b04 100644 --- a/share/composer/openstack.ks +++ b/share/composer/openstack.ks @@ -16,8 +16,6 @@ selinux --enforcing logging --level=info # Shutdown after installation shutdown -# System timezone -timezone US/Eastern # System bootloader configuration bootloader --location=mbr --append="no_timer_check console=ttyS0,115200n8 console=tty1 net.ifnames=0" # Add platform specific partitions diff --git a/share/composer/partitioned-disk.ks b/share/composer/partitioned-disk.ks index 33b54dba..449fe65f 100644 --- a/share/composer/partitioned-disk.ks +++ b/share/composer/partitioned-disk.ks @@ -16,8 +16,6 @@ selinux --enforcing logging --level=info # Shutdown after installation shutdown -# System timezone -timezone US/Eastern # System bootloader configuration bootloader --location=mbr # Add platform specific partitions diff --git a/share/composer/qcow2.ks b/share/composer/qcow2.ks index 404b8582..e653512d 100644 --- a/share/composer/qcow2.ks +++ b/share/composer/qcow2.ks @@ -16,8 +16,6 @@ selinux --enforcing logging --level=info # Shutdown after installation shutdown -# System timezone -timezone US/Eastern # System bootloader configuration bootloader --location=mbr # Add platform specific partitions diff --git a/share/composer/tar.ks b/share/composer/tar.ks index f6af8032..ee15b673 100644 --- a/share/composer/tar.ks +++ b/share/composer/tar.ks @@ -16,8 +16,6 @@ selinux --enforcing logging --level=info # Shutdown after installation shutdown -# System timezone -timezone US/Eastern # System bootloader configuration (tar doesn't need a bootloader) bootloader --location=none diff --git a/share/composer/vhd.ks b/share/composer/vhd.ks index 84bd457f..eefbc027 100644 --- a/share/composer/vhd.ks +++ b/share/composer/vhd.ks @@ -16,8 +16,6 @@ selinux --enforcing logging --level=info # Shutdown after installation shutdown -# System timezone -timezone US/Eastern # System bootloader configuration bootloader --location=mbr --append="no_timer_check console=ttyS0,115200n8 earlyprintk=ttyS0,115200 rootdelay=300 net.ifnames=0" # Add platform specific partitions diff --git a/share/composer/vmdk.ks b/share/composer/vmdk.ks index 2da78346..a1b7b443 100644 --- a/share/composer/vmdk.ks +++ b/share/composer/vmdk.ks @@ -16,8 +16,6 @@ selinux --enforcing logging --level=info # Shutdown after installation shutdown -# System timezone -timezone US/Eastern # System bootloader configuration bootloader --location=mbr # Add platform specific partitions diff --git a/src/pylorax/api/compose.py b/src/pylorax/api/compose.py index e1270513..c5e0636b 100644 --- a/src/pylorax/api/compose.py +++ b/src/pylorax/api/compose.py @@ -79,7 +79,7 @@ def test_templates(dbo, share_dir): pkgs = [(name, "*") for name in ks.handler.packages.packageList] grps = [grp.name for grp in ks.handler.packages.groupList] try: - _ = projects_depsolve(dbo, pkgs, grps) + projects_depsolve(dbo, pkgs, grps) except ProjectsError as e: template_errors.append("Error depsolving %s: %s" % (compose_type, str(e))) @@ -163,6 +163,44 @@ def get_kernel_append(recipe): return recipe["customizations"]["kernel"]["append"] +def timezone_cmd(line, settings): + """ Update the timezone line with the settings + + :param line: The timezone ... line + :type line: str + :param settings: A dict with timezone and/or ntpservers list + :type settings: dict + + Using pykickstart to process the line is the best way to make sure it + is parsed correctly, and re-assembled for inclusion into the final kickstart + """ + ks_version = makeVersion() + ks = KickstartParser(ks_version, errorsAreFatal=False, missingIncludeIsFatal=False) + ks.readKickstartFromString(line) + + if "timezone" in settings: + ks.handler.timezone.timezone = settings["timezone"] + if "ntpservers" in settings: + ks.handler.timezone.ntpservers = settings["ntpservers"] + + # Converting back to a string includes a comment, return just the timezone line + return str(ks.handler.timezone).splitlines()[-1] + + +def get_timezone_settings(recipe): + """Return the customizations.timezone dict + + :param recipe: + :type recipe: Recipe object + :returns: append value or empty string + :rtype: dict + """ + if "customizations" not in recipe or \ + "timezone" not in recipe["customizations"]: + return {} + return recipe["customizations"]["timezone"] + + def customize_ks_template(ks_template, recipe): """ Customize the kickstart template and return it @@ -171,26 +209,60 @@ def customize_ks_template(ks_template, recipe): :param recipe: :type recipe: Recipe object + Apply customizations to existing template commands, or add defaults for ones that are + missing and required. + Apply customizations.kernel.append to the bootloader argument in the template. Add bootloader line if it is missing. + + Add default timezone if needed. It does NOT replace an existing timezone entry """ - kernel_append = get_kernel_append(recipe) - if not kernel_append: - return ks_template - found_bootloader = False + # Commands to be modified [NEW-COMMAND-FUNC, NEW-VALUE, DEFAULT, REPLACE] + # The function is called with a kickstart command string and the value to replace + # The value is specific to the command, and is understood by the function + # The default is a complete kickstart command string, suitable for writing to the template + # If REPLACE is False it will not change an existing entry only add a missing one + commands = {"bootloader": [bootloader_append, + get_kernel_append(recipe), + 'bootloader --location=none', True], + "timezone": [timezone_cmd, + get_timezone_settings(recipe), + 'timezone UTC', False], + } + found = {} + output = StringIO() for line in ks_template.splitlines(): - if not line.startswith("bootloader"): + for cmd in commands: + (new_command, value, default, replace) = commands[cmd] + if line.startswith(cmd): + found[cmd] = True + if value and replace: + log.debug("Replacing %s with %s", cmd, value) + print(new_command(line, value), file=output) + else: + log.debug("Skipping %s", cmd) + print(line, file=output) + break + else: + # No matches, write the line as-is print(line, file=output) - continue - found_bootloader = True - log.debug("Found bootloader line: %s", line) - print(bootloader_append(line, kernel_append), file=output) - if found_bootloader: - return output.getvalue() - else: - return 'bootloader --append="%s" --location=none' % kernel_append + output.getvalue() + # Write out defaults for the ones not found + # These must go FIRST because the template still needs to have the packages added + defaults = StringIO() + for cmd in commands: + if cmd in found: + continue + (new_command, value, default, _) = commands[cmd] + if value and default: + log.debug("Setting %s to use %s", cmd, value) + print(new_command(default, value), file=defaults) + elif default: + log.debug("Setting %s to %s", cmd, default) + print(default, file=defaults) + + return defaults.getvalue() + output.getvalue() def write_ks_root(f, user): diff --git a/tests/pylorax/test_compose.py b/tests/pylorax/test_compose.py index ce4be741..7e9e052f 100644 --- a/tests/pylorax/test_compose.py +++ b/tests/pylorax/test_compose.py @@ -21,7 +21,8 @@ import unittest from pylorax import get_buildarch from pylorax.api.compose import add_customizations, compose_types, get_extra_pkgs -from pylorax.api.compose import bootloader_append, customize_ks_template +from pylorax.api.compose import timezone_cmd, get_timezone_settings +from pylorax.api.compose import get_kernel_append, bootloader_append, customize_ks_template from pylorax.api.config import configure, make_dnf_dirs from pylorax.api.dnfbase import get_base_object from pylorax.api.recipes import recipe_from_toml @@ -37,6 +38,10 @@ HOSTNAME = BASE_RECIPE + """[customizations] hostname = "testhostname" """ +TIMEZONE = BASE_RECIPE + """[customizations] +timezone = "US/Samoa" +""" + SSHKEY = BASE_RECIPE + """[[customizations.sshkey]] user = "root" key = "ROOT SSH KEY" @@ -231,6 +236,22 @@ class CustomizationsTestCase(unittest.TestCase): self.assertCustomization(ROOT_PLAIN_KEY, 'sshkey --user root "A SSH KEY FOR THE USER"') self.assertNotCustomization(ROOT_PLAIN_KEY, "rootpw --lock") + def test_get_kernel_append(self): + """Test get_kernel_append function""" + blueprint_data = """name = "test-kernel" +description = "test recipe" +version = "0.0.1" +""" + blueprint2_data = blueprint_data + """ +[customizations.kernel] +append="nosmt=force" +""" + recipe = recipe_from_toml(blueprint_data) + self.assertEqual(get_kernel_append(recipe), "") + + recipe = recipe_from_toml(blueprint2_data) + self.assertEqual(get_kernel_append(recipe), "nosmt=force") + def test_bootloader_append(self): """Test bootloader_append function""" @@ -246,28 +267,148 @@ class CustomizationsTestCase(unittest.TestCase): self.assertEqual(bootloader_append('bootloader --append="console=tty1" --location=mbr --password="BADPASSWORD"', "nosmt=force"), 'bootloader --append="console=tty1 nosmt=force" --location=mbr --password="BADPASSWORD"') - def _checkBootloader(self, result, append_str): - """Find the bootloader line and make sure append_str is in it""" + def test_get_timezone_settings(self): + """Test get_timezone_settings function""" + blueprint_data = """name = "test-kernel" +description = "test recipe" +version = "0.0.1" +""" + blueprint2_data = blueprint_data + """ +[customizations.timezone] +timezone = "US/Samoa" +""" + blueprint3_data = blueprint_data + """ +[customizations.timezone] +ntpservers = ["0.north-america.pool.ntp.org", "1.north-america.pool.ntp.org"] +""" + blueprint4_data = blueprint_data + """ +[customizations.timezone] +timezone = "US/Samoa" +ntpservers = ["0.north-america.pool.ntp.org", "1.north-america.pool.ntp.org"] +""" + recipe = recipe_from_toml(blueprint_data) + self.assertEqual(get_timezone_settings(recipe), {}) + recipe = recipe_from_toml(blueprint2_data) + self.assertEqual(get_timezone_settings(recipe), {"timezone": "US/Samoa"}) + + recipe = recipe_from_toml(blueprint3_data) + self.assertEqual(get_timezone_settings(recipe), + {"ntpservers": ["0.north-america.pool.ntp.org", "1.north-america.pool.ntp.org"]}) + + recipe = recipe_from_toml(blueprint4_data) + self.assertEqual(get_timezone_settings(recipe), + {"timezone": "US/Samoa", + "ntpservers": ["0.north-america.pool.ntp.org", "1.north-america.pool.ntp.org"]}) + + def test_timezone_cmd(self): + """Test timezone_cmd function""" + + self.assertEqual(timezone_cmd("timezone UTC", {}), 'timezone UTC') + self.assertEqual(timezone_cmd("timezone FOO", {"timezone": "US/Samoa"}), + 'timezone US/Samoa') + self.assertEqual(timezone_cmd("timezone FOO", + {"timezone": "US/Samoa", "ntpservers": ["0.ntp.org", "1.ntp.org"]}), + 'timezone US/Samoa --ntpservers=0.ntp.org,1.ntp.org') + + self.assertEqual(timezone_cmd("timezone --ntpservers=a,b,c FOO", + {"timezone": "US/Samoa", "ntpservers": ["0.pool.ntp.org", "1.pool.ntp.org"]}), + 'timezone US/Samoa --ntpservers=0.pool.ntp.org,1.pool.ntp.org') + + def _checkBootloader(self, result, append_str, line_limit=0): + """Find the bootloader line and make sure append_str is in it""" + # Optionally check to make sure the change is at the top of the template + line_num = 0 for line in result.splitlines(): if line.startswith("bootloader") and append_str in line: - return True + if line_limit == 0 or line_num < line_limit: + return True + else: + print("FAILED: bootloader not in the first %d lines of the output" % line_limit) + return False + line_num += 1 return False + def _checkTimezone(self, result, settings, line_limit=0): + """Find the timezone line and make sure it is as expected""" + # Optionally check to make sure the change is at the top of the template + line_num = 0 + for line in result.splitlines(): + if line.startswith("timezone"): + if settings["timezone"] in line and all([True for n in settings["ntpservers"] if n in line]): + if line_limit == 0 or line_num < line_limit: + return True + else: + print("FAILED: timezone not in the first %d lines of the output" % line_limit) + return False + else: + print("FAILED: %s not matching %s" % (settings, line)) + line_num += 1 + return False + + def test_template_defaults(self): + """Test that customize_ks_template includes defaults correctly""" + blueprint_data = """name = "test-kernel" +description = "test recipe" +version = "0.0.1" + +[[packages]] +name = "lorax" +version = "*" +""" + recipe = recipe_from_toml(blueprint_data) + + # Make sure that a kickstart with no bootloader and no timezone has them added + result = customize_ks_template("firewall --enabled\n", recipe) + print(result) + self.assertEqual(sum([1 for l in result.splitlines() if l.startswith("bootloader")]), 1) + self.assertTrue(self._checkBootloader(result, "none", line_limit=2)) + self.assertEqual(sum([1 for l in result.splitlines() if l.startswith("timezone")]), 1) + self.assertTrue(self._checkTimezone(result, {"timezone": "UTC", "ntpservers": []}, line_limit=2)) + + # Make sure that a kickstart with a bootloader, and no timezone has timezone added to the top + result = customize_ks_template("firewall --enabled\nbootloader --location=mbr\n", recipe) + print(result) + self.assertEqual(sum([1 for l in result.splitlines() if l.startswith("bootloader")]), 1) + self.assertTrue(self._checkBootloader(result, "mbr")) + self.assertEqual(sum([1 for l in result.splitlines() if l.startswith("timezone")]), 1) + self.assertTrue(self._checkTimezone(result, {"timezone": "UTC", "ntpservers": []}, line_limit=1)) + + # Make sure that a kickstart with a bootloader and timezone has neither added + result = customize_ks_template("firewall --enabled\nbootloader --location=mbr\ntimezone US/Samoa\n", recipe) + print(result) + self.assertEqual(sum([1 for l in result.splitlines() if l.startswith("bootloader")]), 1) + self.assertTrue(self._checkBootloader(result, "mbr")) + self.assertEqual(sum([1 for l in result.splitlines() if l.startswith("timezone")]), 1) + self.assertTrue(self._checkTimezone(result, {"timezone": "US/Samoa", "ntpservers": []})) + def test_customize_ks_template(self): - """Test that [customizations.kernel] works correctly""" + """Test that customize_ks_template works correctly""" blueprint_data = """name = "test-kernel" description = "test recipe" version = "0.0.1" [customizations.kernel] append="nosmt=force" + +[customizations.timezone] +timezone = "US/Samoa" +ntpservers = ["0.north-america.pool.ntp.org", "1.north-america.pool.ntp.org"] """ + tz_dict = {"timezone": "US/Samoa", "ntpservers": ["0.north-america.pool.ntp.org", "1.north-america.pool.ntp.org"]} recipe = recipe_from_toml(blueprint_data) # Test against a kickstart without bootloader result = customize_ks_template("firewall --enabled\n", recipe) + self.assertTrue(self._checkBootloader(result, "nosmt=force", line_limit=2)) + self.assertEqual(sum([1 for l in result.splitlines() if l.startswith("bootloader")]), 1) + self.assertTrue(self._checkTimezone(result, tz_dict, line_limit=2)) + self.assertEqual(sum([1 for l in result.splitlines() if l.startswith("timezone")]), 1) + + # Test against a kickstart with a bootloader line + result = customize_ks_template("firewall --enabled\nbootloader --location=mbr\n", recipe) self.assertTrue(self._checkBootloader(result, "nosmt=force")) + self.assertTrue(self._checkTimezone(result, tz_dict, line_limit=2)) # Test against all of the available templates share_dir = "./share/" @@ -278,7 +419,20 @@ append="nosmt=force" ks_template = open(ks_template_path, "r").read() result = customize_ks_template(ks_template, recipe) if not self._checkBootloader(result, "nosmt=force"): - errors.append(("compose_type %s failed" % compose_type, result)) + errors.append(("bootloader for compose_type %s failed" % compose_type, result)) + if sum([1 for l in result.splitlines() if l.startswith("bootloader")]) != 1: + errors.append(("bootloader for compose_type %s failed: More than 1 entry" % compose_type, result)) + + + # google images should retain their timezone settings + if compose_type == "google": + if self._checkTimezone(result, tz_dict): + errors.append(("timezone for compose_type %s failed" % compose_type, result)) + elif not self._checkTimezone(result, tz_dict, line_limit=2): + # None of the templates have a timezone to modify, it should be placed at the top + errors.append(("timezone for compose_type %s failed" % compose_type, result)) + if sum([1 for l in result.splitlines() if l.startswith("timezone")]) != 1: + errors.append(("timezone for compose_type %s failed: More than 1 entry" % compose_type, result)) # Print the bad results for e, r in errors: diff --git a/tests/pylorax/test_recipes.py b/tests/pylorax/test_recipes.py index 9156934e..9718accc 100644 --- a/tests/pylorax/test_recipes.py +++ b/tests/pylorax/test_recipes.py @@ -22,7 +22,7 @@ import tempfile import unittest import pylorax.api.recipes as recipes -from pylorax.api.compose import add_customizations +from pylorax.api.compose import add_customizations, customize_ks_template from pylorax.sysutils import joinpaths from pykickstart.parser import KickstartParser @@ -364,6 +364,7 @@ class CustomizationsTests(unittest.TestCase): # write out the customization data, and parse the resulting kickstart with tempfile.NamedTemporaryFile(prefix="lorax.test.customizations", mode="w") as f: + f.write(customize_ks_template("", recipe_obj)) add_customizations(f, recipe_obj) f.flush() ks.readKickstart(f.name) @@ -415,6 +416,30 @@ hostname = "testy.example.com" ks = self._blueprint_to_ks(blueprint_data) self.assertEqual(ks.handler.network.hostname, "testy.example.com") + def test_timezone(self): + blueprint_data = """name = "test-timezone" +description = "test recipe" +version = "0.0.1" + +[customizations.timezone] +timezone = "US/Samoa" +""" + ks = self._blueprint_to_ks(blueprint_data) + self.assertEqual(ks.handler.timezone.timezone, "US/Samoa") + + def test_timezone_ntpservers(self): + blueprint_data = """name = "test-ntpservers" +description = "test recipe" +version = "0.0.1" + +[customizations.timezone] +timezone = "US/Samoa" +ntpservers = ["1.north-america.pool.ntp.org"] +""" + ks = self._blueprint_to_ks(blueprint_data) + self.assertEqual(ks.handler.timezone.timezone, "US/Samoa") + self.assertEqual(ks.handler.timezone.ntpservers, ["1.north-america.pool.ntp.org"]) + def test_user(self): blueprint_data = """name = "test-user" description = "test recipe" @@ -525,6 +550,10 @@ name = "widget" [[customizations.group]] name = "students" + +[customizations.timezone] +timezone = "US/Samoa" +ntpservers = ["0.north-america.pool.ntp.org", "1.north-america.pool.ntp.org"] """ ks = self._blueprint_to_ks(blueprint_data) @@ -564,3 +593,6 @@ name = "students" studentsGroup = self._find_group(ks, "students") self.assertIsNotNone(studentsGroup) self.assertEqual(studentsGroup.name, "students") + + self.assertEqual(ks.handler.timezone.timezone, "US/Samoa") + self.assertEqual(ks.handler.timezone.ntpservers, ["0.north-america.pool.ntp.org", "1.north-america.pool.ntp.org"])