forked from rpms/cloud-init
		
	import cloud-init-21.1-19.el9_0.3
This commit is contained in:
		
							parent
							
								
									1698d19cee
								
							
						
					
					
						commit
						9a4c65c0f8
					
				| @ -0,0 +1,481 @@ | ||||
| From 8e539e045371769a63f3b33fc53eed568d644695 Mon Sep 17 00:00:00 2001 | ||||
| From: Emanuele Giuseppe Esposito <eesposit@redhat.com> | ||||
| Date: Tue, 31 May 2022 09:33:08 +0200 | ||||
| Subject: [PATCH] Leave the details of service management to the distro (#1074) | ||||
| 
 | ||||
| RH-Author: Emanuele Giuseppe Esposito <eesposit@redhat.com> | ||||
| RH-MergeRequest: 73: Leave the details of service management to the distro (#1074) | ||||
| RH-Commit: [1/1] d33e0976d8811a6cb2755eb08fd3a4c811be53f5 (eesposit/cloud-init) | ||||
| RH-Bugzilla: 2091935 | ||||
| RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com> | ||||
| RH-Acked-by: Mohamed Gamal Morsy <mmorsy@redhat.com> | ||||
| 
 | ||||
| commit 8c89009e75c7cf6c2f87635b82656f07f58095e1 | ||||
| Author: Andy Fiddaman <omnios@citrus-it.co.uk> | ||||
| Date:   Wed Oct 20 20:58:27 2021 +0000 | ||||
| 
 | ||||
|     Leave the details of service management to the distro (#1074) | ||||
| 
 | ||||
|     Various modules restart services and they all have logic to try and | ||||
|     detect if they are running on a system that needs 'systemctl' or | ||||
|     'service', and then have code to decide which order the arguments | ||||
|     need to be etc. On top of that, not all modules do this in the same way. | ||||
| 
 | ||||
|     The duplication and different approaches are not ideal but this also | ||||
|     makes it hard to add support for a new distribution that does not use | ||||
|     either 'systemctl' or 'service'. | ||||
| 
 | ||||
|     This change adds a new manage_service() method to the distro class | ||||
|     and updates several modules to use it. | ||||
| 
 | ||||
| Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> | ||||
| ---
 | ||||
|  cloudinit/config/cc_fan.py                    | 42 ++++++++----------- | ||||
|  cloudinit/config/cc_ntp.py                    | 20 ++------- | ||||
|  cloudinit/config/cc_rsyslog.py                | 17 +++----- | ||||
|  cloudinit/config/cc_set_passwords.py          | 17 ++------ | ||||
|  cloudinit/config/tests/test_set_passwords.py  | 41 ++++++++---------- | ||||
|  cloudinit/distros/__init__.py                 | 28 +++++++++++++ | ||||
|  .../test_distros/test_manage_service.py       | 38 +++++++++++++++++ | ||||
|  .../test_handler/test_handler_ntp.py          | 29 +++---------- | ||||
|  8 files changed, 119 insertions(+), 113 deletions(-) | ||||
|  create mode 100644 tests/unittests/test_distros/test_manage_service.py | ||||
| 
 | ||||
| diff --git a/cloudinit/config/cc_fan.py b/cloudinit/config/cc_fan.py
 | ||||
| index 77984bca..91f50e22 100644
 | ||||
| --- a/cloudinit/config/cc_fan.py
 | ||||
| +++ b/cloudinit/config/cc_fan.py
 | ||||
| @@ -52,35 +52,26 @@ BUILTIN_CFG = {
 | ||||
|  } | ||||
|   | ||||
|   | ||||
| -def stop_update_start(service, config_file, content, systemd=False):
 | ||||
| -    if systemd:
 | ||||
| -        cmds = {'stop': ['systemctl', 'stop', service],
 | ||||
| -                'start': ['systemctl', 'start', service],
 | ||||
| -                'enable': ['systemctl', 'enable', service]}
 | ||||
| -    else:
 | ||||
| -        cmds = {'stop': ['service', 'stop'],
 | ||||
| -                'start': ['service', 'start']}
 | ||||
| -
 | ||||
| -    def run(cmd, msg):
 | ||||
| -        try:
 | ||||
| -            return subp.subp(cmd, capture=True)
 | ||||
| -        except subp.ProcessExecutionError as e:
 | ||||
| -            LOG.warning("failed: %s (%s): %s", service, cmd, e)
 | ||||
| -            return False
 | ||||
| -
 | ||||
| -    stop_failed = not run(cmds['stop'], msg='stop %s' % service)
 | ||||
| +def stop_update_start(distro, service, config_file, content):
 | ||||
| +    try:
 | ||||
| +        distro.manage_service('stop', service)
 | ||||
| +        stop_failed = False
 | ||||
| +    except subp.ProcessExecutionError as e:
 | ||||
| +        stop_failed = True
 | ||||
| +        LOG.warning("failed to stop %s: %s", service, e)
 | ||||
| +
 | ||||
|      if not content.endswith('\n'): | ||||
|          content += '\n' | ||||
|      util.write_file(config_file, content, omode="w") | ||||
|   | ||||
| -    ret = run(cmds['start'], msg='start %s' % service)
 | ||||
| -    if ret and stop_failed:
 | ||||
| -        LOG.warning("success: %s started", service)
 | ||||
| -
 | ||||
| -    if 'enable' in cmds:
 | ||||
| -        ret = run(cmds['enable'], msg='enable %s' % service)
 | ||||
| +    try:
 | ||||
| +        distro.manage_service('start', service)
 | ||||
| +        if stop_failed:
 | ||||
| +            LOG.warning("success: %s started", service)
 | ||||
| +    except subp.ProcessExecutionError as e:
 | ||||
| +        LOG.warning("failed to start %s: %s", service, e)
 | ||||
|   | ||||
| -    return ret
 | ||||
| +    distro.manage_service('enable', service)
 | ||||
|   | ||||
|   | ||||
|  def handle(name, cfg, cloud, log, args): | ||||
| @@ -99,7 +90,8 @@ def handle(name, cfg, cloud, log, args):
 | ||||
|          distro.install_packages(['ubuntu-fan']) | ||||
|   | ||||
|      stop_update_start( | ||||
| +        distro,
 | ||||
|          service='ubuntu-fan', config_file=mycfg.get('config_path'), | ||||
| -        content=mycfg.get('config'), systemd=distro.uses_systemd())
 | ||||
| +        content=mycfg.get('config'))
 | ||||
|   | ||||
|  # vi: ts=4 expandtab | ||||
| diff --git a/cloudinit/config/cc_ntp.py b/cloudinit/config/cc_ntp.py
 | ||||
| index e183993f..4f358c8a 100644
 | ||||
| --- a/cloudinit/config/cc_ntp.py
 | ||||
| +++ b/cloudinit/config/cc_ntp.py
 | ||||
| @@ -459,21 +459,6 @@ def write_ntp_config_template(distro_name, service_name=None, servers=None,
 | ||||
|          util.del_file(template_fn) | ||||
|   | ||||
|   | ||||
| -def reload_ntp(service, systemd=False):
 | ||||
| -    """Restart or reload an ntp system service.
 | ||||
| -
 | ||||
| -    @param service: A string specifying the name of the service to be affected.
 | ||||
| -    @param systemd: A boolean indicating if the distro uses systemd, defaults
 | ||||
| -    to False.
 | ||||
| -    @returns: A tuple of stdout, stderr results from executing the action.
 | ||||
| -    """
 | ||||
| -    if systemd:
 | ||||
| -        cmd = ['systemctl', 'reload-or-restart', service]
 | ||||
| -    else:
 | ||||
| -        cmd = ['service', service, 'restart']
 | ||||
| -    subp.subp(cmd, capture=True)
 | ||||
| -
 | ||||
| -
 | ||||
|  def supplemental_schema_validation(ntp_config): | ||||
|      """Validate user-provided ntp:config option values. | ||||
|   | ||||
| @@ -583,10 +568,11 @@ def handle(name, cfg, cloud, log, _args):
 | ||||
|                         packages=ntp_client_config['packages'], | ||||
|                         check_exe=ntp_client_config['check_exe']) | ||||
|      try: | ||||
| -        reload_ntp(ntp_client_config['service_name'],
 | ||||
| -                   systemd=cloud.distro.uses_systemd())
 | ||||
| +        cloud.distro.manage_service('reload',
 | ||||
| +                                    ntp_client_config.get('service_name'))
 | ||||
|      except subp.ProcessExecutionError as e: | ||||
|          LOG.exception("Failed to reload/start ntp service: %s", e) | ||||
|          raise | ||||
|   | ||||
| +
 | ||||
|  # vi: ts=4 expandtab | ||||
| diff --git a/cloudinit/config/cc_rsyslog.py b/cloudinit/config/cc_rsyslog.py
 | ||||
| index 2a2bc931..dd2bbd00 100644
 | ||||
| --- a/cloudinit/config/cc_rsyslog.py
 | ||||
| +++ b/cloudinit/config/cc_rsyslog.py
 | ||||
| @@ -207,16 +207,11 @@ HOST_PORT_RE = re.compile(
 | ||||
|      r'([:](?P<port>[0-9]+))?$') | ||||
|   | ||||
|   | ||||
| -def reload_syslog(command=DEF_RELOAD, systemd=False):
 | ||||
| -    service = 'rsyslog'
 | ||||
| +def reload_syslog(distro, command=DEF_RELOAD):
 | ||||
|      if command == DEF_RELOAD: | ||||
| -        if systemd:
 | ||||
| -            cmd = ['systemctl', 'reload-or-try-restart', service]
 | ||||
| -        else:
 | ||||
| -            cmd = ['service', service, 'restart']
 | ||||
| -    else:
 | ||||
| -        cmd = command
 | ||||
| -    subp.subp(cmd, capture=True)
 | ||||
| +        service = distro.get_option('rsyslog_svcname', 'rsyslog')
 | ||||
| +        return distro.manage_service('try-reload', service)
 | ||||
| +    return subp.subp(command, capture=True)
 | ||||
|   | ||||
|   | ||||
|  def load_config(cfg): | ||||
| @@ -429,9 +424,7 @@ def handle(name, cfg, cloud, log, _args):
 | ||||
|          return | ||||
|   | ||||
|      try: | ||||
| -        restarted = reload_syslog(
 | ||||
| -            command=mycfg[KEYNAME_RELOAD],
 | ||||
| -            systemd=cloud.distro.uses_systemd()),
 | ||||
| +        restarted = reload_syslog(cloud.distro, command=mycfg[KEYNAME_RELOAD])
 | ||||
|      except subp.ProcessExecutionError as e: | ||||
|          restarted = False | ||||
|          log.warning("Failed to reload syslog", e) | ||||
| diff --git a/cloudinit/config/cc_set_passwords.py b/cloudinit/config/cc_set_passwords.py
 | ||||
| index 433de751..9efbf61f 100755
 | ||||
| --- a/cloudinit/config/cc_set_passwords.py
 | ||||
| +++ b/cloudinit/config/cc_set_passwords.py
 | ||||
| @@ -94,18 +94,15 @@ PW_SET = (''.join([x for x in ascii_letters + digits
 | ||||
|                     if x not in 'loLOI01'])) | ||||
|   | ||||
|   | ||||
| -def handle_ssh_pwauth(pw_auth, service_cmd=None, service_name="ssh"):
 | ||||
| +def handle_ssh_pwauth(pw_auth, distro):
 | ||||
|      """Apply sshd PasswordAuthentication changes. | ||||
|   | ||||
|      @param pw_auth: config setting from 'pw_auth'. | ||||
|                      Best given as True, False, or "unchanged". | ||||
| -    @param service_cmd: The service command list (['service'])
 | ||||
| -    @param service_name: The name of the sshd service for the system.
 | ||||
| +    @param distro: an instance of the distro class for the target distribution
 | ||||
|   | ||||
|      @return: None""" | ||||
|      cfg_name = "PasswordAuthentication" | ||||
| -    if service_cmd is None:
 | ||||
| -        service_cmd = ["service"]
 | ||||
|   | ||||
|      if util.is_true(pw_auth): | ||||
|          cfg_val = 'yes' | ||||
| @@ -124,11 +121,7 @@ def handle_ssh_pwauth(pw_auth, service_cmd=None, service_name="ssh"):
 | ||||
|          LOG.debug("No need to restart SSH service, %s not updated.", cfg_name) | ||||
|          return | ||||
|   | ||||
| -    if 'systemctl' in service_cmd:
 | ||||
| -        cmd = list(service_cmd) + ["restart", service_name]
 | ||||
| -    else:
 | ||||
| -        cmd = list(service_cmd) + [service_name, "restart"]
 | ||||
| -    subp.subp(cmd)
 | ||||
| +    distro.manage_service('restart', distro.get_option('ssh_svcname', 'ssh'))
 | ||||
|      LOG.debug("Restarted the SSH daemon.") | ||||
|   | ||||
|   | ||||
| @@ -229,9 +222,7 @@ def handle(_name, cfg, cloud, log, args):
 | ||||
|              if expired_users: | ||||
|                  log.debug("Expired passwords for: %s users", expired_users) | ||||
|   | ||||
| -    handle_ssh_pwauth(
 | ||||
| -        cfg.get('ssh_pwauth'), service_cmd=cloud.distro.init_cmd,
 | ||||
| -        service_name=cloud.distro.get_option('ssh_svcname', 'ssh'))
 | ||||
| +    handle_ssh_pwauth(cfg.get('ssh_pwauth'), cloud.distro)
 | ||||
|   | ||||
|      if len(errors): | ||||
|          log.debug("%s errors occured, re-raising the last one", len(errors)) | ||||
| diff --git a/cloudinit/config/tests/test_set_passwords.py b/cloudinit/config/tests/test_set_passwords.py
 | ||||
| index bbe2ee8f..79118a12 100644
 | ||||
| --- a/cloudinit/config/tests/test_set_passwords.py
 | ||||
| +++ b/cloudinit/config/tests/test_set_passwords.py
 | ||||
| @@ -14,57 +14,52 @@ class TestHandleSshPwauth(CiTestCase):
 | ||||
|   | ||||
|      with_logs = True | ||||
|   | ||||
| -    @mock.patch(MODPATH + "subp.subp")
 | ||||
| +    @mock.patch("cloudinit.distros.subp.subp")
 | ||||
|      def test_unknown_value_logs_warning(self, m_subp): | ||||
| -        setpass.handle_ssh_pwauth("floo")
 | ||||
| +        cloud = self.tmp_cloud(distro='ubuntu')
 | ||||
| +        setpass.handle_ssh_pwauth("floo", cloud.distro)
 | ||||
|          self.assertIn("Unrecognized value: ssh_pwauth=floo", | ||||
|                        self.logs.getvalue()) | ||||
|          m_subp.assert_not_called() | ||||
|   | ||||
|      @mock.patch(MODPATH + "update_ssh_config", return_value=True) | ||||
| -    @mock.patch(MODPATH + "subp.subp")
 | ||||
| +    @mock.patch("cloudinit.distros.subp.subp")
 | ||||
|      def test_systemctl_as_service_cmd(self, m_subp, m_update_ssh_config): | ||||
|          """If systemctl in service cmd: systemctl restart name.""" | ||||
| -        setpass.handle_ssh_pwauth(
 | ||||
| -            True, service_cmd=["systemctl"], service_name="myssh")
 | ||||
| -        self.assertEqual(mock.call(["systemctl", "restart", "myssh"]),
 | ||||
| -                         m_subp.call_args)
 | ||||
| -
 | ||||
| -    @mock.patch(MODPATH + "update_ssh_config", return_value=True)
 | ||||
| -    @mock.patch(MODPATH + "subp.subp")
 | ||||
| -    def test_service_as_service_cmd(self, m_subp, m_update_ssh_config):
 | ||||
| -        """If systemctl in service cmd: systemctl restart name."""
 | ||||
| -        setpass.handle_ssh_pwauth(
 | ||||
| -            True, service_cmd=["service"], service_name="myssh")
 | ||||
| -        self.assertEqual(mock.call(["service", "myssh", "restart"]),
 | ||||
| -                         m_subp.call_args)
 | ||||
| +        cloud = self.tmp_cloud(distro='ubuntu')
 | ||||
| +        cloud.distro.init_cmd = ['systemctl']
 | ||||
| +        setpass.handle_ssh_pwauth(True, cloud.distro)
 | ||||
| +        m_subp.assert_called_with(
 | ||||
| +            ["systemctl", "restart", "ssh"], capture=True)
 | ||||
|   | ||||
|      @mock.patch(MODPATH + "update_ssh_config", return_value=False) | ||||
| -    @mock.patch(MODPATH + "subp.subp")
 | ||||
| +    @mock.patch("cloudinit.distros.subp.subp")
 | ||||
|      def test_not_restarted_if_not_updated(self, m_subp, m_update_ssh_config): | ||||
|          """If config is not updated, then no system restart should be done.""" | ||||
| -        setpass.handle_ssh_pwauth(True)
 | ||||
| +        cloud = self.tmp_cloud(distro='ubuntu')
 | ||||
| +        setpass.handle_ssh_pwauth(True, cloud.distro)
 | ||||
|          m_subp.assert_not_called() | ||||
|          self.assertIn("No need to restart SSH", self.logs.getvalue()) | ||||
|   | ||||
|      @mock.patch(MODPATH + "update_ssh_config", return_value=True) | ||||
| -    @mock.patch(MODPATH + "subp.subp")
 | ||||
| +    @mock.patch("cloudinit.distros.subp.subp")
 | ||||
|      def test_unchanged_does_nothing(self, m_subp, m_update_ssh_config): | ||||
|          """If 'unchanged', then no updates to config and no restart.""" | ||||
| -        setpass.handle_ssh_pwauth(
 | ||||
| -            "unchanged", service_cmd=["systemctl"], service_name="myssh")
 | ||||
| +        cloud = self.tmp_cloud(distro='ubuntu')
 | ||||
| +        setpass.handle_ssh_pwauth("unchanged", cloud.distro)
 | ||||
|          m_update_ssh_config.assert_not_called() | ||||
|          m_subp.assert_not_called() | ||||
|   | ||||
| -    @mock.patch(MODPATH + "subp.subp")
 | ||||
| +    @mock.patch("cloudinit.distros.subp.subp")
 | ||||
|      def test_valid_change_values(self, m_subp): | ||||
|          """If value is a valid changen value, then update should be called.""" | ||||
| +        cloud = self.tmp_cloud(distro='ubuntu')
 | ||||
|          upname = MODPATH + "update_ssh_config" | ||||
|          optname = "PasswordAuthentication" | ||||
|          for value in util.FALSE_STRINGS + util.TRUE_STRINGS: | ||||
|              optval = "yes" if value in util.TRUE_STRINGS else "no" | ||||
|              with mock.patch(upname, return_value=False) as m_update: | ||||
| -                setpass.handle_ssh_pwauth(value)
 | ||||
| +                setpass.handle_ssh_pwauth(value, cloud.distro)
 | ||||
|                  m_update.assert_called_with({optname: optval}) | ||||
|          m_subp.assert_not_called() | ||||
|   | ||||
| diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py
 | ||||
| index 220bd11f..a8c57cb8 100755
 | ||||
| --- a/cloudinit/distros/__init__.py
 | ||||
| +++ b/cloudinit/distros/__init__.py
 | ||||
| @@ -784,6 +784,34 @@ class Distro(persistence.CloudInitPickleMixin, metaclass=abc.ABCMeta):
 | ||||
|              args.append(message) | ||||
|          return args | ||||
|   | ||||
| +    def manage_service(self, action, service):
 | ||||
| +        """
 | ||||
| +        Perform the requested action on a service. This handles the common
 | ||||
| +        'systemctl' and 'service' cases and may be overridden in subclasses
 | ||||
| +        as necessary.
 | ||||
| +        May raise ProcessExecutionError
 | ||||
| +        """
 | ||||
| +        init_cmd = self.init_cmd
 | ||||
| +        if self.uses_systemd() or 'systemctl' in init_cmd:
 | ||||
| +            init_cmd = ['systemctl']
 | ||||
| +            cmds = {'stop': ['stop', service],
 | ||||
| +                    'start': ['start', service],
 | ||||
| +                    'enable': ['enable', service],
 | ||||
| +                    'restart': ['restart', service],
 | ||||
| +                    'reload': ['reload-or-restart', service],
 | ||||
| +                    'try-reload': ['reload-or-try-restart', service],
 | ||||
| +                    }
 | ||||
| +        else:
 | ||||
| +            cmds = {'stop': [service, 'stop'],
 | ||||
| +                    'start': [service, 'start'],
 | ||||
| +                    'enable': [service, 'start'],
 | ||||
| +                    'restart': [service, 'restart'],
 | ||||
| +                    'reload': [service, 'restart'],
 | ||||
| +                    'try-reload': [service, 'restart'],
 | ||||
| +                    }
 | ||||
| +        cmd = list(init_cmd) + list(cmds[action])
 | ||||
| +        return subp.subp(cmd, capture=True)
 | ||||
| +
 | ||||
|   | ||||
|  def _apply_hostname_transformations_to_url(url: str, transformations: list): | ||||
|      """ | ||||
| diff --git a/tests/unittests/test_distros/test_manage_service.py b/tests/unittests/test_distros/test_manage_service.py
 | ||||
| new file mode 100644 | ||||
| index 00000000..47e7cfb0
 | ||||
| --- /dev/null
 | ||||
| +++ b/tests/unittests/test_distros/test_manage_service.py
 | ||||
| @@ -0,0 +1,38 @@
 | ||||
| +# This file is part of cloud-init. See LICENSE file for license information.
 | ||||
| +
 | ||||
| +from cloudinit.tests.helpers import (CiTestCase, mock)
 | ||||
| +from tests.unittests.util import TestingDistro
 | ||||
| +
 | ||||
| +
 | ||||
| +class TestManageService(CiTestCase):
 | ||||
| +
 | ||||
| +    with_logs = True
 | ||||
| +
 | ||||
| +    def setUp(self):
 | ||||
| +        super(TestManageService, self).setUp()
 | ||||
| +        self.dist = TestingDistro()
 | ||||
| +
 | ||||
| +    @mock.patch.object(TestingDistro, 'uses_systemd', return_value=False)
 | ||||
| +    @mock.patch("cloudinit.distros.subp.subp")
 | ||||
| +    def test_manage_service_systemctl_initcmd(self, m_subp, m_sysd):
 | ||||
| +        self.dist.init_cmd = ['systemctl']
 | ||||
| +        self.dist.manage_service('start', 'myssh')
 | ||||
| +        m_subp.assert_called_with(['systemctl', 'start', 'myssh'],
 | ||||
| +                                  capture=True)
 | ||||
| +
 | ||||
| +    @mock.patch.object(TestingDistro, 'uses_systemd', return_value=False)
 | ||||
| +    @mock.patch("cloudinit.distros.subp.subp")
 | ||||
| +    def test_manage_service_service_initcmd(self, m_subp, m_sysd):
 | ||||
| +        self.dist.init_cmd = ['service']
 | ||||
| +        self.dist.manage_service('start', 'myssh')
 | ||||
| +        m_subp.assert_called_with(['service', 'myssh', 'start'], capture=True)
 | ||||
| +
 | ||||
| +    @mock.patch.object(TestingDistro, 'uses_systemd', return_value=True)
 | ||||
| +    @mock.patch("cloudinit.distros.subp.subp")
 | ||||
| +    def test_manage_service_systemctl(self, m_subp, m_sysd):
 | ||||
| +        self.dist.init_cmd = ['ignore']
 | ||||
| +        self.dist.manage_service('start', 'myssh')
 | ||||
| +        m_subp.assert_called_with(['systemctl', 'start', 'myssh'],
 | ||||
| +                                  capture=True)
 | ||||
| +
 | ||||
| +# vi: ts=4 sw=4 expandtab
 | ||||
| diff --git a/tests/unittests/test_handler/test_handler_ntp.py b/tests/unittests/test_handler/test_handler_ntp.py
 | ||||
| index 6b9c8377..c059e2e6 100644
 | ||||
| --- a/tests/unittests/test_handler/test_handler_ntp.py
 | ||||
| +++ b/tests/unittests/test_handler/test_handler_ntp.py
 | ||||
| @@ -112,22 +112,6 @@ class TestNtp(FilesystemMockingTestCase):
 | ||||
|                                    check_exe='timesyncd') | ||||
|          install_func.assert_called_once_with([]) | ||||
|   | ||||
| -    @mock.patch("cloudinit.config.cc_ntp.subp")
 | ||||
| -    def test_reload_ntp_defaults(self, mock_subp):
 | ||||
| -        """Test service is restarted/reloaded (defaults)"""
 | ||||
| -        service = 'ntp_service_name'
 | ||||
| -        cmd = ['service', service, 'restart']
 | ||||
| -        cc_ntp.reload_ntp(service)
 | ||||
| -        mock_subp.subp.assert_called_with(cmd, capture=True)
 | ||||
| -
 | ||||
| -    @mock.patch("cloudinit.config.cc_ntp.subp")
 | ||||
| -    def test_reload_ntp_systemd(self, mock_subp):
 | ||||
| -        """Test service is restarted/reloaded (systemd)"""
 | ||||
| -        service = 'ntp_service_name'
 | ||||
| -        cc_ntp.reload_ntp(service, systemd=True)
 | ||||
| -        cmd = ['systemctl', 'reload-or-restart', service]
 | ||||
| -        mock_subp.subp.assert_called_with(cmd, capture=True)
 | ||||
| -
 | ||||
|      def test_ntp_rename_ntp_conf(self): | ||||
|          """When NTP_CONF exists, rename_ntp moves it.""" | ||||
|          ntpconf = self.tmp_path("ntp.conf", self.new_root) | ||||
| @@ -488,10 +472,11 @@ class TestNtp(FilesystemMockingTestCase):
 | ||||
|              cc_ntp.handle('notimportant', cfg, mycloud, None, None) | ||||
|              self.assertEqual(0, m_select.call_count) | ||||
|   | ||||
| +    @mock.patch("cloudinit.distros.subp")
 | ||||
|      @mock.patch("cloudinit.config.cc_ntp.subp") | ||||
|      @mock.patch('cloudinit.config.cc_ntp.select_ntp_client') | ||||
|      @mock.patch("cloudinit.distros.Distro.uses_systemd") | ||||
| -    def test_ntp_the_whole_package(self, m_sysd, m_select, m_subp):
 | ||||
| +    def test_ntp_the_whole_package(self, m_sysd, m_select, m_subp, m_dsubp):
 | ||||
|          """Test enabled config renders template, and restarts service """ | ||||
|          cfg = {'ntp': {'enabled': True}} | ||||
|          for distro in cc_ntp.distros: | ||||
| @@ -509,7 +494,7 @@ class TestNtp(FilesystemMockingTestCase):
 | ||||
|   | ||||
|              if distro == 'alpine': | ||||
|                  uses_systemd = False | ||||
| -                expected_service_call = ['service', service_name, 'restart']
 | ||||
| +                expected_service_call = ['rc-service', service_name, 'restart']
 | ||||
|                  # _mock_ntp_client_config call above did not specify a client | ||||
|                  # value and so it defaults to "ntp" which on Alpine Linux only | ||||
|                  # supports servers and not pools. | ||||
| @@ -525,7 +510,7 @@ class TestNtp(FilesystemMockingTestCase):
 | ||||
|                  m_util.is_false.return_value = util.is_false( | ||||
|                      cfg['ntp']['enabled']) | ||||
|                  cc_ntp.handle('notimportant', cfg, mycloud, None, None) | ||||
| -                m_subp.subp.assert_called_with(
 | ||||
| +                m_dsubp.subp.assert_called_with(
 | ||||
|                      expected_service_call, capture=True) | ||||
|   | ||||
|              self.assertEqual(expected_content, util.load_file(confpath)) | ||||
| @@ -673,9 +658,8 @@ class TestNtp(FilesystemMockingTestCase):
 | ||||
|              self.assertEqual(sorted(expected_cfg), sorted(result)) | ||||
|              m_which.assert_has_calls([]) | ||||
|   | ||||
| -    @mock.patch('cloudinit.config.cc_ntp.reload_ntp')
 | ||||
|      @mock.patch('cloudinit.config.cc_ntp.install_ntp_client') | ||||
| -    def test_ntp_user_provided_config_with_template(self, m_install, m_reload):
 | ||||
| +    def test_ntp_user_provided_config_with_template(self, m_install):
 | ||||
|          custom = r'\n#MyCustomTemplate' | ||||
|          user_template = NTP_TEMPLATE + custom | ||||
|          confpath = os.path.join(self.new_root, 'etc/myntp/myntp.conf') | ||||
| @@ -702,11 +686,10 @@ class TestNtp(FilesystemMockingTestCase):
 | ||||
|                  util.load_file(confpath)) | ||||
|   | ||||
|      @mock.patch('cloudinit.config.cc_ntp.supplemental_schema_validation') | ||||
| -    @mock.patch('cloudinit.config.cc_ntp.reload_ntp')
 | ||||
|      @mock.patch('cloudinit.config.cc_ntp.install_ntp_client') | ||||
|      @mock.patch('cloudinit.config.cc_ntp.select_ntp_client') | ||||
|      def test_ntp_user_provided_config_template_only(self, m_select, m_install, | ||||
| -                                                    m_reload, m_schema):
 | ||||
| +                                                    m_schema):
 | ||||
|          """Test custom template for default client""" | ||||
|          custom = r'\n#MyCustomTemplate' | ||||
|          user_template = NTP_TEMPLATE + custom | ||||
| -- 
 | ||||
| 2.31.1 | ||||
| 
 | ||||
| @ -0,0 +1,547 @@ | ||||
| From 304be0559f6e6e471b648e559175408b59d15909 Mon Sep 17 00:00:00 2001 | ||||
| From: Emanuele Giuseppe Esposito <eesposit@redhat.com> | ||||
| Date: Thu, 2 Jun 2022 15:56:02 +0200 | ||||
| Subject: [PATCH] cc_set_hostname: do not write "localhost" when no hostname is | ||||
|  given (#1453) | ||||
| 
 | ||||
| RH-Author: Emanuele Giuseppe Esposito <eesposit@redhat.com> | ||||
| RH-MergeRequest: 77: cc_set_hostname: do not write "localhost" when no hostname is given (#1453) | ||||
| RH-Commit: [1/1] 9b54be1a52eff87bf2b9e396edbf998bb9d68d49 (eesposit/cloud-init) | ||||
| RH-Bugzilla: 2092909 | ||||
| RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com> | ||||
| RH-Acked-by: Mohamed Gamal Morsy <mmorsy@redhat.com> | ||||
| 
 | ||||
| commit 74e43496f353db52e15d96abeb54ad63baac5be9 | ||||
| Author: Emanuele Giuseppe Esposito <eesposit@redhat.com> | ||||
| Date:   Tue May 31 16:03:44 2022 +0200 | ||||
| 
 | ||||
|     cc_set_hostname: do not write "localhost" when no hostname is given (#1453) | ||||
| 
 | ||||
|     Systemd used to sometimes ignore localhost in /etc/hostnames, and many programs | ||||
|     like cloud-init used this as a workaround to set a default hostname. | ||||
| 
 | ||||
|     From https://github.com/systemd/systemd/commit/d39079fcaa05e23540d2b1f0270fa31c22a7e9f1: | ||||
| 
 | ||||
|             We would sometimes ignore localhost-style names in /etc/hostname. That is | ||||
|             brittle. If the user configured some hostname, it's most likely because they | ||||
|             want to use that as the hostname. If they don't want to use such a hostname, | ||||
|             they should just not create the config. Everything becomes simples if we just | ||||
|             use the configured hostname as-is. | ||||
| 
 | ||||
|             This behaviour seems to have been a workaround for Anaconda installer and other | ||||
|             tools writing out /etc/hostname with the default of "localhost.localdomain". | ||||
|             Anaconda PR to stop doing that: rhinstaller/anaconda#3040. | ||||
|             That might have been useful as a work-around for other programs misbehaving if | ||||
|             /etc/hostname was not present, but nowadays it's not useful because systemd | ||||
|             mostly controls the hostname and it is perfectly happy without that file. | ||||
| 
 | ||||
|             Apart from making things simpler, this allows users to set a hostname like | ||||
|             "localhost" and have it honoured, if such a whim strikes them. | ||||
| 
 | ||||
|     As also suggested by the Anaconda PR, we need to stop writing default "localhost" | ||||
|     in /etc/hostnames, and let the right service (networking, user) do that if they | ||||
|     need to. Otherwise, "localhost" will permanently stay as hostname and will | ||||
|     prevent other tools like NetworkManager from setting the right one. | ||||
| 
 | ||||
|     Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> | ||||
| 
 | ||||
|     RHBZ: 1980403 | ||||
| 
 | ||||
| Conflicts: | ||||
| 	almost all test files had conflicts because they use '' instead of "" | ||||
| 
 | ||||
| Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> | ||||
| ---
 | ||||
|  cloudinit/cmd/main.py                         |  5 ++-- | ||||
|  cloudinit/config/cc_apt_configure.py          |  2 +- | ||||
|  cloudinit/config/cc_debug.py                  |  2 +- | ||||
|  cloudinit/config/cc_phone_home.py             |  6 ++-- | ||||
|  cloudinit/config/cc_set_hostname.py           | 14 ++++++++- | ||||
|  cloudinit/config/cc_spacewalk.py              | 11 ++++--- | ||||
|  cloudinit/config/cc_update_etc_hosts.py       |  6 ++-- | ||||
|  cloudinit/config/cc_update_hostname.py        | 14 ++++++++- | ||||
|  cloudinit/sources/DataSourceAliYun.py         |  8 ++++- | ||||
|  cloudinit/sources/DataSourceCloudSigma.py     |  9 ++++-- | ||||
|  cloudinit/sources/DataSourceGCE.py            |  6 +++- | ||||
|  cloudinit/sources/DataSourceScaleway.py       |  3 +- | ||||
|  cloudinit/sources/__init__.py                 | 28 ++++++++++++++---- | ||||
|  cloudinit/util.py                             | 29 +++++++++++++++---- | ||||
|  .../unittests/test_datasource/test_aliyun.py  |  5 ++-- | ||||
|  .../test_datasource/test_cloudsigma.py        | 14 +++++---- | ||||
|  .../unittests/test_datasource/test_hetzner.py |  2 +- | ||||
|  .../unittests/test_datasource/test_vmware.py  |  4 ++- | ||||
|  18 files changed, 125 insertions(+), 43 deletions(-) | ||||
| 
 | ||||
| diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py
 | ||||
| index baf1381f..ebd3719d 100644
 | ||||
| --- a/cloudinit/cmd/main.py
 | ||||
| +++ b/cloudinit/cmd/main.py
 | ||||
| @@ -701,8 +701,9 @@ def _maybe_set_hostname(init, stage, retry_stage):
 | ||||
|      @param retry_stage: String represented logs upon error setting hostname. | ||||
|      """ | ||||
|      cloud = init.cloudify() | ||||
| -    (hostname, _fqdn) = util.get_hostname_fqdn(
 | ||||
| -        init.cfg, cloud, metadata_only=True)
 | ||||
| +    (hostname, _fqdn, _) = util.get_hostname_fqdn(
 | ||||
| +        init.cfg, cloud, metadata_only=True
 | ||||
| +    )
 | ||||
|      if hostname:  # meta-data or user-data hostname content | ||||
|          try: | ||||
|              cc_set_hostname.handle('set-hostname', init.cfg, cloud, LOG, None) | ||||
| diff --git a/cloudinit/config/cc_apt_configure.py b/cloudinit/config/cc_apt_configure.py
 | ||||
| index bb8a1278..8a2b57b5 100644
 | ||||
| --- a/cloudinit/config/cc_apt_configure.py
 | ||||
| +++ b/cloudinit/config/cc_apt_configure.py
 | ||||
| @@ -926,7 +926,7 @@ def search_for_mirror_dns(configured, mirrortype, cfg, cloud):
 | ||||
|              raise ValueError("unknown mirror type") | ||||
|   | ||||
|          # if we have a fqdn, then search its domain portion first | ||||
| -        (_, fqdn) = util.get_hostname_fqdn(cfg, cloud)
 | ||||
| +        fqdn = util.get_hostname_fqdn(cfg, cloud).fqdn
 | ||||
|          mydom = ".".join(fqdn.split(".")[1:]) | ||||
|          if mydom: | ||||
|              doms.append(".%s" % mydom) | ||||
| diff --git a/cloudinit/config/cc_debug.py b/cloudinit/config/cc_debug.py
 | ||||
| index 4d5a6aa2..7ca4ea0f 100644
 | ||||
| --- a/cloudinit/config/cc_debug.py
 | ||||
| +++ b/cloudinit/config/cc_debug.py
 | ||||
| @@ -88,7 +88,7 @@ def handle(name, cfg, cloud, log, args):
 | ||||
|      to_print.write("Datasource: %s\n" % | ||||
|                     (type_utils.obj_name(cloud.datasource))) | ||||
|      to_print.write("Distro: %s\n" % (type_utils.obj_name(cloud.distro))) | ||||
| -    to_print.write("Hostname: %s\n" % (cloud.get_hostname(True)))
 | ||||
| +    to_print.write("Hostname: %s\n" % (cloud.get_hostname(True).hostname))
 | ||||
|      to_print.write("Instance ID: %s\n" % (cloud.get_instance_id())) | ||||
|      to_print.write("Locale: %s\n" % (cloud.get_locale())) | ||||
|      to_print.write("Launch IDX: %s\n" % (cloud.launch_index)) | ||||
| diff --git a/cloudinit/config/cc_phone_home.py b/cloudinit/config/cc_phone_home.py
 | ||||
| index 733c3910..a71237dc 100644
 | ||||
| --- a/cloudinit/config/cc_phone_home.py
 | ||||
| +++ b/cloudinit/config/cc_phone_home.py
 | ||||
| @@ -99,9 +99,9 @@ def handle(name, cfg, cloud, log, args):
 | ||||
|          post_list = POST_LIST_ALL | ||||
|   | ||||
|      all_keys = {} | ||||
| -    all_keys['instance_id'] = cloud.get_instance_id()
 | ||||
| -    all_keys['hostname'] = cloud.get_hostname()
 | ||||
| -    all_keys['fqdn'] = cloud.get_hostname(fqdn=True)
 | ||||
| +    all_keys["instance_id"] = cloud.get_instance_id()
 | ||||
| +    all_keys["hostname"] = cloud.get_hostname().hostname
 | ||||
| +    all_keys["fqdn"] = cloud.get_hostname(fqdn=True).hostname
 | ||||
|   | ||||
|      pubkeys = { | ||||
|          'pub_key_dsa': '/etc/ssh/ssh_host_dsa_key.pub', | ||||
| diff --git a/cloudinit/config/cc_set_hostname.py b/cloudinit/config/cc_set_hostname.py
 | ||||
| index 1d23d80d..37e5ac1d 100644
 | ||||
| --- a/cloudinit/config/cc_set_hostname.py
 | ||||
| +++ b/cloudinit/config/cc_set_hostname.py
 | ||||
| @@ -62,7 +62,15 @@ def handle(name, cfg, cloud, log, _args):
 | ||||
|          log.debug(("Configuration option 'preserve_hostname' is set," | ||||
|                     " not setting the hostname in module %s"), name) | ||||
|          return | ||||
| -    (hostname, fqdn) = util.get_hostname_fqdn(cfg, cloud)
 | ||||
| +
 | ||||
| +    # Set prefer_fqdn_over_hostname value in distro
 | ||||
| +    hostname_fqdn = util.get_cfg_option_bool(
 | ||||
| +        cfg, "prefer_fqdn_over_hostname", None
 | ||||
| +    )
 | ||||
| +    if hostname_fqdn is not None:
 | ||||
| +        cloud.distro.set_option("prefer_fqdn_over_hostname", hostname_fqdn)
 | ||||
| +
 | ||||
| +    (hostname, fqdn, is_default) = util.get_hostname_fqdn(cfg, cloud)
 | ||||
|      # Check for previous successful invocation of set-hostname | ||||
|   | ||||
|      # set-hostname artifact file accounts for both hostname and fqdn | ||||
| @@ -79,6 +87,10 @@ def handle(name, cfg, cloud, log, _args):
 | ||||
|      if not hostname_changed: | ||||
|          log.debug('No hostname changes. Skipping set-hostname') | ||||
|          return | ||||
| +    if is_default and hostname == "localhost":
 | ||||
| +        # https://github.com/systemd/systemd/commit/d39079fcaa05e23540d2b1f0270fa31c22a7e9f1
 | ||||
| +        log.debug("Hostname is localhost. Let other services handle this.")
 | ||||
| +        return
 | ||||
|      log.debug("Setting the hostname to %s (%s)", fqdn, hostname) | ||||
|      try: | ||||
|          cloud.distro.set_hostname(hostname, fqdn) | ||||
| diff --git a/cloudinit/config/cc_spacewalk.py b/cloudinit/config/cc_spacewalk.py
 | ||||
| index 95083607..29bf8415 100644
 | ||||
| --- a/cloudinit/config/cc_spacewalk.py
 | ||||
| +++ b/cloudinit/config/cc_spacewalk.py
 | ||||
| @@ -79,10 +79,13 @@ def handle(name, cfg, cloud, log, _args):
 | ||||
|          # Need to have this installed before further things will work. | ||||
|          cloud.distro.install_packages(required_packages) | ||||
|          if not is_registered(): | ||||
| -            do_register(spacewalk_server,
 | ||||
| -                        cloud.datasource.get_hostname(fqdn=True),
 | ||||
| -                        proxy=cfg.get("proxy"), log=log,
 | ||||
| -                        activation_key=cfg.get('activation_key'))
 | ||||
| +            do_register(
 | ||||
| +                spacewalk_server,
 | ||||
| +                cloud.datasource.get_hostname(fqdn=True).hostname,
 | ||||
| +                proxy=cfg.get("proxy"),
 | ||||
| +                log=log,
 | ||||
| +                activation_key=cfg.get("activation_key"),
 | ||||
| +            )
 | ||||
|      else: | ||||
|          log.debug("Skipping module named %s, 'spacewalk/server' key" | ||||
|                    " was not found in configuration", name) | ||||
| diff --git a/cloudinit/config/cc_update_etc_hosts.py b/cloudinit/config/cc_update_etc_hosts.py
 | ||||
| index 03fffb96..69c5a2f6 100644
 | ||||
| --- a/cloudinit/config/cc_update_etc_hosts.py
 | ||||
| +++ b/cloudinit/config/cc_update_etc_hosts.py
 | ||||
| @@ -59,8 +59,8 @@ frequency = PER_ALWAYS
 | ||||
|   | ||||
|  def handle(name, cfg, cloud, log, _args): | ||||
|      manage_hosts = util.get_cfg_option_str(cfg, "manage_etc_hosts", False) | ||||
| -    if util.translate_bool(manage_hosts, addons=['template']):
 | ||||
| -        (hostname, fqdn) = util.get_hostname_fqdn(cfg, cloud)
 | ||||
| +    if util.translate_bool(manage_hosts, addons=["template"]):
 | ||||
| +        (hostname, fqdn, _) = util.get_hostname_fqdn(cfg, cloud)
 | ||||
|          if not hostname: | ||||
|              log.warning(("Option 'manage_etc_hosts' was set," | ||||
|                           " but no hostname was found")) | ||||
| @@ -78,7 +78,7 @@ def handle(name, cfg, cloud, log, _args):
 | ||||
|                                   {'hostname': hostname, 'fqdn': fqdn}) | ||||
|   | ||||
|      elif manage_hosts == "localhost": | ||||
| -        (hostname, fqdn) = util.get_hostname_fqdn(cfg, cloud)
 | ||||
| +        (hostname, fqdn, _) = util.get_hostname_fqdn(cfg, cloud)
 | ||||
|          if not hostname: | ||||
|              log.warning(("Option 'manage_etc_hosts' was set," | ||||
|                           " but no hostname was found")) | ||||
| diff --git a/cloudinit/config/cc_update_hostname.py b/cloudinit/config/cc_update_hostname.py
 | ||||
| index d5f4eb5a..61496525 100644
 | ||||
| --- a/cloudinit/config/cc_update_hostname.py
 | ||||
| +++ b/cloudinit/config/cc_update_hostname.py
 | ||||
| @@ -45,7 +45,19 @@ def handle(name, cfg, cloud, log, _args):
 | ||||
|                     " not updating the hostname in module %s"), name) | ||||
|          return | ||||
|   | ||||
| -    (hostname, fqdn) = util.get_hostname_fqdn(cfg, cloud)
 | ||||
| +    # Set prefer_fqdn_over_hostname value in distro
 | ||||
| +    hostname_fqdn = util.get_cfg_option_bool(
 | ||||
| +        cfg, "prefer_fqdn_over_hostname", None
 | ||||
| +    )
 | ||||
| +    if hostname_fqdn is not None:
 | ||||
| +        cloud.distro.set_option("prefer_fqdn_over_hostname", hostname_fqdn)
 | ||||
| +
 | ||||
| +    (hostname, fqdn, is_default) = util.get_hostname_fqdn(cfg, cloud)
 | ||||
| +    if is_default and hostname == "localhost":
 | ||||
| +        # https://github.com/systemd/systemd/commit/d39079fcaa05e23540d2b1f0270fa31c22a7e9f1
 | ||||
| +        log.debug("Hostname is localhost. Let other services handle this.")
 | ||||
| +        return
 | ||||
| +
 | ||||
|      try: | ||||
|          prev_fn = os.path.join(cloud.get_cpath('data'), "previous-hostname") | ||||
|          log.debug("Updating hostname to %s (%s)", fqdn, hostname) | ||||
| diff --git a/cloudinit/sources/DataSourceAliYun.py b/cloudinit/sources/DataSourceAliYun.py
 | ||||
| index 09052873..365ce176 100644
 | ||||
| --- a/cloudinit/sources/DataSourceAliYun.py
 | ||||
| +++ b/cloudinit/sources/DataSourceAliYun.py
 | ||||
| @@ -3,6 +3,7 @@
 | ||||
|  from cloudinit import dmi | ||||
|  from cloudinit import sources | ||||
|  from cloudinit.sources import DataSourceEc2 as EC2 | ||||
| +from cloudinit.sources import DataSourceHostname
 | ||||
|   | ||||
|  ALIYUN_PRODUCT = "Alibaba Cloud ECS" | ||||
|   | ||||
| @@ -17,7 +18,12 @@ class DataSourceAliYun(EC2.DataSourceEc2):
 | ||||
|      extended_metadata_versions = [] | ||||
|   | ||||
|      def get_hostname(self, fqdn=False, resolve_ip=False, metadata_only=False): | ||||
| -        return self.metadata.get('hostname', 'localhost.localdomain')
 | ||||
| +        hostname = self.metadata.get("hostname")
 | ||||
| +        is_default = False
 | ||||
| +        if hostname is None:
 | ||||
| +            hostname = "localhost.localdomain"
 | ||||
| +            is_default = True
 | ||||
| +        return DataSourceHostname(hostname, is_default)
 | ||||
|   | ||||
|      def get_public_ssh_keys(self): | ||||
|          return parse_public_keys(self.metadata.get('public-keys', {})) | ||||
| diff --git a/cloudinit/sources/DataSourceCloudSigma.py b/cloudinit/sources/DataSourceCloudSigma.py
 | ||||
| index f63baf74..9f6f1d42 100644
 | ||||
| --- a/cloudinit/sources/DataSourceCloudSigma.py
 | ||||
| +++ b/cloudinit/sources/DataSourceCloudSigma.py
 | ||||
| @@ -12,6 +12,8 @@ from cloudinit.cs_utils import Cepko, SERIAL_PORT
 | ||||
|  from cloudinit import dmi | ||||
|  from cloudinit import log as logging | ||||
|  from cloudinit import sources | ||||
| +from cloudinit.sources import DataSourceHostname
 | ||||
| +from cloudinit.sources.helpers.cloudsigma import SERIAL_PORT, Cepko
 | ||||
|   | ||||
|  LOG = logging.getLogger(__name__) | ||||
|   | ||||
| @@ -89,10 +91,11 @@ class DataSourceCloudSigma(sources.DataSource):
 | ||||
|          Cleans up and uses the server's name if the latter is set. Otherwise | ||||
|          the first part from uuid is being used. | ||||
|          """ | ||||
| -        if re.match(r'^[A-Za-z0-9 -_\.]+$', self.metadata['name']):
 | ||||
| -            return self.metadata['name'][:61]
 | ||||
| +        if re.match(r"^[A-Za-z0-9 -_\.]+$", self.metadata["name"]):
 | ||||
| +            ret = self.metadata["name"][:61]
 | ||||
|          else: | ||||
| -            return self.metadata['uuid'].split('-')[0]
 | ||||
| +            ret = self.metadata["uuid"].split("-")[0]
 | ||||
| +        return DataSourceHostname(ret, False)
 | ||||
|   | ||||
|      def get_public_ssh_keys(self): | ||||
|          return [self.ssh_public_key] | ||||
| diff --git a/cloudinit/sources/DataSourceGCE.py b/cloudinit/sources/DataSourceGCE.py
 | ||||
| index 746caddb..ac5d3e5b 100644
 | ||||
| --- a/cloudinit/sources/DataSourceGCE.py
 | ||||
| +++ b/cloudinit/sources/DataSourceGCE.py
 | ||||
| @@ -13,6 +13,8 @@ from cloudinit import log as logging
 | ||||
|  from cloudinit import sources | ||||
|  from cloudinit import url_helper | ||||
|  from cloudinit import util | ||||
| +from cloudinit.net.dhcp import EphemeralDHCPv4
 | ||||
| +from cloudinit.sources import DataSourceHostname
 | ||||
|   | ||||
|  LOG = logging.getLogger(__name__) | ||||
|   | ||||
| @@ -100,7 +102,9 @@ class DataSourceGCE(sources.DataSource):
 | ||||
|   | ||||
|      def get_hostname(self, fqdn=False, resolve_ip=False, metadata_only=False): | ||||
|          # GCE has long FDQN's and has asked for short hostnames. | ||||
| -        return self.metadata['local-hostname'].split('.')[0]
 | ||||
| +        return DataSourceHostname(
 | ||||
| +            self.metadata["local-hostname"].split(".")[0], False
 | ||||
| +        )
 | ||||
|   | ||||
|      @property | ||||
|      def availability_zone(self): | ||||
| diff --git a/cloudinit/sources/DataSourceScaleway.py b/cloudinit/sources/DataSourceScaleway.py
 | ||||
| index 41be7665..875b5bb3 100644
 | ||||
| --- a/cloudinit/sources/DataSourceScaleway.py
 | ||||
| +++ b/cloudinit/sources/DataSourceScaleway.py
 | ||||
| @@ -33,6 +33,7 @@ from cloudinit import util
 | ||||
|  from cloudinit import net | ||||
|  from cloudinit.net.dhcp import EphemeralDHCPv4, NoDHCPLeaseError | ||||
|  from cloudinit.event import EventType | ||||
| +from cloudinit.sources import DataSourceHostname
 | ||||
|   | ||||
|  LOG = logging.getLogger(__name__) | ||||
|   | ||||
| @@ -271,7 +272,7 @@ class DataSourceScaleway(sources.DataSource):
 | ||||
|          return ssh_keys | ||||
|   | ||||
|      def get_hostname(self, fqdn=False, resolve_ip=False, metadata_only=False): | ||||
| -        return self.metadata['hostname']
 | ||||
| +        return DataSourceHostname(self.metadata["hostname"], False)
 | ||||
|   | ||||
|      @property | ||||
|      def availability_zone(self): | ||||
| diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py
 | ||||
| index 7d74f8d9..6f5d7ea6 100644
 | ||||
| --- a/cloudinit/sources/__init__.py
 | ||||
| +++ b/cloudinit/sources/__init__.py
 | ||||
| @@ -134,6 +134,11 @@ def redact_sensitive_keys(metadata, redact_value=REDACT_SENSITIVE_VALUE):
 | ||||
|  URLParams = namedtuple( | ||||
|      'URLParms', ['max_wait_seconds', 'timeout_seconds', 'num_retries']) | ||||
|   | ||||
| +DataSourceHostname = namedtuple(
 | ||||
| +    "DataSourceHostname",
 | ||||
| +    ["hostname", "is_default"],
 | ||||
| +)
 | ||||
| +
 | ||||
|   | ||||
|  class DataSource(CloudInitPickleMixin, metaclass=abc.ABCMeta): | ||||
|   | ||||
| @@ -233,7 +238,7 @@ class DataSource(CloudInitPickleMixin, metaclass=abc.ABCMeta):
 | ||||
|   | ||||
|      def _get_standardized_metadata(self, instance_data): | ||||
|          """Return a dictionary of standardized metadata keys.""" | ||||
| -        local_hostname = self.get_hostname()
 | ||||
| +        local_hostname = self.get_hostname().hostname
 | ||||
|          instance_id = self.get_instance_id() | ||||
|          availability_zone = self.availability_zone | ||||
|          # In the event of upgrade from existing cloudinit, pickled datasource | ||||
| @@ -593,22 +598,33 @@ class DataSource(CloudInitPickleMixin, metaclass=abc.ABCMeta):
 | ||||
|          @param metadata_only: Boolean, set True to avoid looking up hostname | ||||
|              if meta-data doesn't have local-hostname present. | ||||
|   | ||||
| -        @return: hostname or qualified hostname. Optionally return None when
 | ||||
| +        @return: a DataSourceHostname namedtuple
 | ||||
| +            <hostname or qualified hostname>, <is_default> (str, bool).
 | ||||
| +            is_default is a bool and
 | ||||
| +            it's true only if hostname is localhost and was
 | ||||
| +            returned by util.get_hostname() as a default.
 | ||||
| +            This is used to differentiate with a user-defined
 | ||||
| +            localhost hostname.
 | ||||
| +            Optionally return (None, False) when
 | ||||
|              metadata_only is True and local-hostname data is not available. | ||||
|          """ | ||||
|          defdomain = "localdomain" | ||||
|          defhost = "localhost" | ||||
|          domain = defdomain | ||||
| +        is_default = False
 | ||||
|   | ||||
|          if not self.metadata or not self.metadata.get('local-hostname'): | ||||
|              if metadata_only: | ||||
| -                return None
 | ||||
| +                return DataSourceHostname(None, is_default)
 | ||||
|              # this is somewhat questionable really. | ||||
|              # the cloud datasource was asked for a hostname | ||||
|              # and didn't have one. raising error might be more appropriate | ||||
|              # but instead, basically look up the existing hostname | ||||
|              toks = [] | ||||
|              hostname = util.get_hostname() | ||||
| +            if hostname == "localhost":
 | ||||
| +                # default hostname provided by socket.gethostname()
 | ||||
| +                is_default = True
 | ||||
|              hosts_fqdn = util.get_fqdn_from_hosts(hostname) | ||||
|              if hosts_fqdn and hosts_fqdn.find(".") > 0: | ||||
|                  toks = str(hosts_fqdn).split(".") | ||||
| @@ -641,9 +657,9 @@ class DataSource(CloudInitPickleMixin, metaclass=abc.ABCMeta):
 | ||||
|              hostname = toks[0] | ||||
|   | ||||
|          if fqdn and domain != defdomain: | ||||
| -            return "%s.%s" % (hostname, domain)
 | ||||
| -        else:
 | ||||
| -            return hostname
 | ||||
| +            hostname = "%s.%s" % (hostname, domain)
 | ||||
| +
 | ||||
| +        return DataSourceHostname(hostname, is_default)
 | ||||
|   | ||||
|      def get_package_mirror_info(self): | ||||
|          return self.distro.get_package_mirror_info(data_source=self) | ||||
| diff --git a/cloudinit/util.py b/cloudinit/util.py
 | ||||
| index fe37ae89..8a2b13ba 100644
 | ||||
| --- a/cloudinit/util.py
 | ||||
| +++ b/cloudinit/util.py
 | ||||
| @@ -32,7 +32,8 @@ import subprocess
 | ||||
|  import sys | ||||
|  import time | ||||
|  from base64 import b64decode, b64encode | ||||
| -from errno import ENOENT
 | ||||
| +from collections import deque, namedtuple
 | ||||
| +from errno import EACCES, ENOENT
 | ||||
|  from functools import lru_cache | ||||
|  from urllib import parse | ||||
|  from typing import List | ||||
| @@ -967,6 +968,12 @@ def dos2unix(contents):
 | ||||
|      return contents.replace('\r\n', '\n') | ||||
|   | ||||
|   | ||||
| +HostnameFqdnInfo = namedtuple(
 | ||||
| +    "HostnameFqdnInfo",
 | ||||
| +    ["hostname", "fqdn", "is_default"],
 | ||||
| +)
 | ||||
| +
 | ||||
| +
 | ||||
|  def get_hostname_fqdn(cfg, cloud, metadata_only=False): | ||||
|      """Get hostname and fqdn from config if present and fallback to cloud. | ||||
|   | ||||
| @@ -974,9 +981,17 @@ def get_hostname_fqdn(cfg, cloud, metadata_only=False):
 | ||||
|      @param cloud: Cloud instance from init.cloudify(). | ||||
|      @param metadata_only: Boolean, set True to only query cloud meta-data, | ||||
|          returning None if not present in meta-data. | ||||
| -    @return: a Tuple of strings <hostname>, <fqdn>. Values can be none when
 | ||||
| +    @return: a namedtuple of
 | ||||
| +        <hostname>, <fqdn>, <is_default> (str, str, bool).
 | ||||
| +        Values can be none when
 | ||||
|          metadata_only is True and no cfg or metadata provides hostname info. | ||||
| +        is_default is a bool and
 | ||||
| +        it's true only if hostname is localhost and was
 | ||||
| +        returned by util.get_hostname() as a default.
 | ||||
| +        This is used to differentiate with a user-defined
 | ||||
| +        localhost hostname.
 | ||||
|      """ | ||||
| +    is_default = False
 | ||||
|      if "fqdn" in cfg: | ||||
|          # user specified a fqdn.  Default hostname then is based off that | ||||
|          fqdn = cfg['fqdn'] | ||||
| @@ -990,12 +1005,16 @@ def get_hostname_fqdn(cfg, cloud, metadata_only=False):
 | ||||
|          else: | ||||
|              # no fqdn set, get fqdn from cloud. | ||||
|              # get hostname from cfg if available otherwise cloud | ||||
| -            fqdn = cloud.get_hostname(fqdn=True, metadata_only=metadata_only)
 | ||||
| +            fqdn = cloud.get_hostname(
 | ||||
| +                fqdn=True, metadata_only=metadata_only
 | ||||
| +            ).hostname
 | ||||
|              if "hostname" in cfg: | ||||
|                  hostname = cfg['hostname'] | ||||
|              else: | ||||
| -                hostname = cloud.get_hostname(metadata_only=metadata_only)
 | ||||
| -    return (hostname, fqdn)
 | ||||
| +                hostname, is_default = cloud.get_hostname(
 | ||||
| +                    metadata_only=metadata_only
 | ||||
| +                )
 | ||||
| +    return HostnameFqdnInfo(hostname, fqdn, is_default)
 | ||||
|   | ||||
|   | ||||
|  def get_fqdn_from_hosts(hostname, filename="/etc/hosts"): | ||||
| diff --git a/tests/unittests/test_datasource/test_aliyun.py b/tests/unittests/test_datasource/test_aliyun.py
 | ||||
| index cab1ac2b..acdee0c9 100644
 | ||||
| --- a/tests/unittests/test_datasource/test_aliyun.py
 | ||||
| +++ b/tests/unittests/test_datasource/test_aliyun.py
 | ||||
| @@ -127,8 +127,9 @@ class TestAliYunDatasource(test_helpers.HttprettyTestCase):
 | ||||
|                           self.ds.get_instance_id()) | ||||
|   | ||||
|      def _test_host_name(self): | ||||
| -        self.assertEqual(self.default_metadata['hostname'],
 | ||||
| -                         self.ds.get_hostname())
 | ||||
| +        self.assertEqual(
 | ||||
| +            self.default_metadata["hostname"], self.ds.get_hostname().hostname
 | ||||
| +        )
 | ||||
|   | ||||
|      @mock.patch("cloudinit.sources.DataSourceAliYun._is_aliyun") | ||||
|      def test_with_mock_server(self, m_is_aliyun): | ||||
| diff --git a/tests/unittests/test_datasource/test_cloudsigma.py b/tests/unittests/test_datasource/test_cloudsigma.py
 | ||||
| index 7aa3b1d1..79d52725 100644
 | ||||
| --- a/tests/unittests/test_datasource/test_cloudsigma.py
 | ||||
| +++ b/tests/unittests/test_datasource/test_cloudsigma.py
 | ||||
| @@ -57,12 +57,14 @@ class DataSourceCloudSigmaTest(test_helpers.CiTestCase):
 | ||||
|   | ||||
|      def test_get_hostname(self): | ||||
|          self.datasource.get_data() | ||||
| -        self.assertEqual("test_server", self.datasource.get_hostname())
 | ||||
| -        self.datasource.metadata['name'] = ''
 | ||||
| -        self.assertEqual("65b2fb23", self.datasource.get_hostname())
 | ||||
| -        utf8_hostname = b'\xd1\x82\xd0\xb5\xd1\x81\xd1\x82'.decode('utf-8')
 | ||||
| -        self.datasource.metadata['name'] = utf8_hostname
 | ||||
| -        self.assertEqual("65b2fb23", self.datasource.get_hostname())
 | ||||
| +        self.assertEqual(
 | ||||
| +            "test_server", self.datasource.get_hostname().hostname
 | ||||
| +        )
 | ||||
| +        self.datasource.metadata["name"] = ""
 | ||||
| +        self.assertEqual("65b2fb23", self.datasource.get_hostname().hostname)
 | ||||
| +        utf8_hostname = b"\xd1\x82\xd0\xb5\xd1\x81\xd1\x82".decode("utf-8")
 | ||||
| +        self.datasource.metadata["name"] = utf8_hostname
 | ||||
| +        self.assertEqual("65b2fb23", self.datasource.get_hostname().hostname)
 | ||||
|   | ||||
|      def test_get_public_ssh_keys(self): | ||||
|          self.datasource.get_data() | ||||
| diff --git a/tests/unittests/test_datasource/test_hetzner.py b/tests/unittests/test_datasource/test_hetzner.py
 | ||||
| index eadb92f1..0ccb07c9 100644
 | ||||
| --- a/tests/unittests/test_datasource/test_hetzner.py
 | ||||
| +++ b/tests/unittests/test_datasource/test_hetzner.py
 | ||||
| @@ -97,7 +97,7 @@ class TestDataSourceHetzner(CiTestCase):
 | ||||
|   | ||||
|          self.assertTrue(m_readmd.called) | ||||
|   | ||||
| -        self.assertEqual(METADATA.get('hostname'), ds.get_hostname())
 | ||||
| +        self.assertEqual(METADATA.get("hostname"), ds.get_hostname().hostname)
 | ||||
|   | ||||
|          self.assertEqual(METADATA.get('public-keys'), | ||||
|                           ds.get_public_ssh_keys()) | ||||
| diff --git a/tests/unittests/test_datasource/test_vmware.py b/tests/unittests/test_datasource/test_vmware.py
 | ||||
| index 597db7c8..3bffd136 100644
 | ||||
| --- a/tests/unittests/test_datasource/test_vmware.py
 | ||||
| +++ b/tests/unittests/test_datasource/test_vmware.py
 | ||||
| @@ -356,7 +356,9 @@ class TestDataSourceVMwareGuestInfo_InvalidPlatform(FilesystemMockingTestCase):
 | ||||
|   | ||||
|  def assert_metadata(test_obj, ds, metadata): | ||||
|      test_obj.assertEqual(metadata.get("instance-id"), ds.get_instance_id()) | ||||
| -    test_obj.assertEqual(metadata.get("local-hostname"), ds.get_hostname())
 | ||||
| +    test_obj.assertEqual(
 | ||||
| +        metadata.get("local-hostname"), ds.get_hostname().hostname
 | ||||
| +    )
 | ||||
|   | ||||
|      expected_public_keys = metadata.get("public_keys") | ||||
|      if not isinstance(expected_public_keys, list): | ||||
| -- 
 | ||||
| 2.31.1 | ||||
| 
 | ||||
| @ -1,6 +1,6 @@ | ||||
| Name:           cloud-init | ||||
| Version:        21.1 | ||||
| Release:        19%{?dist}.1 | ||||
| Release:        19%{?dist}.3 | ||||
| Summary:        Cloud instance init scripts | ||||
| License:        ASL 2.0 or GPLv3 | ||||
| URL:            http://launchpad.net/cloud-init | ||||
| @ -66,6 +66,10 @@ Patch29: ci-Adding-_netdev-to-the-default-mount-configuration.patch | ||||
| Patch30: ci-Setting-highest-autoconnect-priority-for-network-scr.patch | ||||
| # For bz#2088027 - [RHEL-9.1] SSH keys with \r\n line breaks are not properly handled on Azure [rhel-9.0.0.z] | ||||
| Patch31: ci-Add-r-n-check-for-SSH-keys-in-Azure-889.patch | ||||
| # For bz#2091935 - cloud-init has an undeclared dependency on the initscripts rpm [rhel-9.0.0.z] | ||||
| Patch32: ci-Leave-the-details-of-service-management-to-the-distr.patch | ||||
| # For bz#2092909 - [RHV] RHEL 9 VM with cloud-init without hostname set doesn't result in the FQDN as hostname [rhel-9.0.0.z] | ||||
| Patch33: ci-cc_set_hostname-do-not-write-localhost-when-no-hostn.patch | ||||
| 
 | ||||
| # Source-git patches | ||||
| 
 | ||||
| @ -267,6 +271,16 @@ fi | ||||
| %config(noreplace) %{_sysconfdir}/rsyslog.d/21-cloudinit.conf | ||||
| 
 | ||||
| %changelog | ||||
| * Wed Jun 08 2022 Miroslav Rezanina <mrezanin@redhat.com> - 21.1-19.el9_0.3 | ||||
| - ci-cc_set_hostname-do-not-write-localhost-when-no-hostn.patch [bz#2092909] | ||||
| - Resolves: bz#2092909 | ||||
|   ([RHV] RHEL 9 VM with cloud-init without hostname set doesn't result in the FQDN as hostname [rhel-9.0.0.z]) | ||||
| 
 | ||||
| * Fri Jun 03 2022 Miroslav Rezanina <mrezanin@redhat.com> - 21.1-19.el9_0.2 | ||||
| - ci-Leave-the-details-of-service-management-to-the-distr.patch [bz#2091935] | ||||
| - Resolves: bz#2091935 | ||||
|   (cloud-init has an undeclared dependency on the initscripts rpm [rhel-9.0.0.z]) | ||||
| 
 | ||||
| * Wed May 18 2022 Miroslav Rezanina <mrezanin@redhat.com> - 21.1-19.el9_0.1 | ||||
| - ci-Add-r-n-check-for-SSH-keys-in-Azure-889.patch [bz#2088027] | ||||
| - Resolves: bz#2088027 | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user