From 9bdbb29662eb4e1c5faf4dfa38c3cecaab0df721 Mon Sep 17 00:00:00 2001 From: "Brian C. Lane" Date: Tue, 9 Apr 2019 16:37:24 -0700 Subject: [PATCH] lorax-composer: Add timezone support to blueprint For example: [customizations.timezone] timezone = "US/Samoa" ntpservers = ["0.pool.ntp.org"] Also includes tests. This removes the timezone kickstart command from all of the templates except for google.ks which needs to set it's own ntp servers and timezone. If timezone isn't included in the blueprint, and it is not already in a template, it will be set to 'timezone UTC' by default. If timezone is set in a template it is left as-is, under the assumption that the image type requires it to boot correctly. --- share/composer/alibaba.ks | 2 - share/composer/ami.ks | 2 - share/composer/ext4-filesystem.ks | 2 - share/composer/hyper-v.ks | 2 - share/composer/live-iso.ks | 2 - share/composer/openstack.ks | 2 - share/composer/partitioned-disk.ks | 2 - share/composer/qcow2.ks | 2 - share/composer/tar.ks | 2 - share/composer/vhd.ks | 2 - share/composer/vmdk.ks | 2 - src/pylorax/api/compose.py | 100 ++++++++++++++--- tests/pylorax/test_compose.py | 166 +++++++++++++++++++++++++++-- tests/pylorax/test_recipes.py | 34 +++++- 14 files changed, 279 insertions(+), 43 deletions(-) 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 9cbb5472..43770a9b 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 1f3166cb..30af6fc2 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/hyper-v.ks b/share/composer/hyper-v.ks index e6e27eef..bd3498b7 100644 --- a/share/composer/hyper-v.ks +++ b/share/composer/hyper-v.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/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 d953b23f..236cbd0d 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 ecfba176..b8b42630 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 410374d7..b7b0003f 100644 --- a/src/pylorax/api/compose.py +++ b/src/pylorax/api/compose.py @@ -80,7 +80,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))) @@ -164,6 +164,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 @@ -172,26 +210,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 1d9de95a..3c4ba99b 100644 --- a/tests/pylorax/test_compose.py +++ b/tests/pylorax/test_compose.py @@ -22,7 +22,8 @@ import unittest from pylorax import get_buildarch from pylorax.api.compose import add_customizations, get_extra_pkgs, compose_types -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 @@ -38,6 +39,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" @@ -232,6 +237,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""" @@ -247,28 +268,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/" @@ -279,7 +420,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 a33b1ea7..16e9ee80 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 @@ -393,6 +393,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) @@ -444,6 +445,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" @@ -554,6 +579,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) @@ -593,3 +622,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"])