[1/2] DHCP: Refactor dhcp client code (canonical#2122)

Move isc-dhclient code to dhcp.py

In support of the upcoming deprecation of
isc-dhcp-client, this code refactors current
dhcp code into classes in dhcp.py. The
primary user-visible change should be the
addition of the following log:

dhcp.py[DEBUG]: DHCP client selected: dhclient

This code lays groundwork to enable
alternate implementations to live side by
side in the codebase to be selected with
distro-defined priority fallback. Note that
maybe_perform_dhcp_discovery() now selects
which dhcp client to call, and then runs the
corresponding client's dhcp_discovery()
method. Currently only class IscDhclient is
implemented, however a yet-to-be-implemented
class Dhcpcd exists to test fallback behavior
and this will be implemented in part two of
this series.

Part of this refactor includes shifting
dhclient service management from hardcoded
calls to the distro-defined manage_service()
method in the *BSDs. Future work is required
in this area to support multiple clients via
select_dhcp_client().

cloudinit/distros/__init__.py

@@ -32,7 +32,7 @@
 from cloudinit.distros.networking import LinuxNetworking, Networking
 from cloudinit.distros.parsers import hosts
 from cloudinit.features import ALLOW_EC2_MIRRORS_ON_NON_AWS_INSTANCE_TYPES
-from cloudinit.net import activators, eni, network_state, renderers
+from cloudinit.net import activators, dhcp, eni, network_state, renderers
 from cloudinit.net.network_state import parse_net_config_data
 from cloudinit.net.renderer import Renderer
 
@@ -110,12 +110,14 @@ class Distro(persistence.CloudInitPickleMixin, metaclass=abc.ABCMeta):
     resolve_conf_fn = "/etc/resolv.conf"
 
     osfamily: str
+    dhcp_client_priority = [dhcp.IscDhclient, dhcp.Dhcpcd]
 
     def __init__(self, name, cfg, paths):
         self._paths = paths
         self._cfg = cfg
         self.name = name
         self.networking: Networking = self.networking_cls()
+        self.dhcp_client_priority = [dhcp.IscDhclient, dhcp.Dhcpcd]
 
     def _unpickle(self, ci_pkl_version: int) -> None:
         """Perform deserialization fixes for Distro."""
@@ -185,7 +187,8 @@ def set_hostname(self, hostname, fqdn=None):
         self._write_hostname(writeable_hostname, self.hostname_conf_fn)
         self._apply_hostname(writeable_hostname)
 
-    def uses_systemd(self):
+    @staticmethod
+    def uses_systemd():
         """Wrapper to report whether this distro uses systemd or sysvinit."""
         return uses_systemd()
 
@@ -916,15 +919,18 @@ def shutdown_command(self, *, mode, delay, message):
             args.append(message)
         return args
 
-    def manage_service(self, action: str, service: str):
+    @classmethod
+    def manage_service(
+        cls, action: str, service: str, *extra_args: str, rcs=None
+    ):
         """
         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 = cls.init_cmd
+        if cls.uses_systemd() or "systemctl" in init_cmd:
             init_cmd = ["systemctl"]
             cmds = {
                 "stop": ["stop", service],
@@ -948,7 +954,7 @@ def manage_service(self, action: str, service: str):
                 "status": [service, "status"],
             }
         cmd = list(init_cmd) + list(cmds[action])
-        return subp.subp(cmd, capture=True)
+        return subp.subp(cmd, capture=True, rcs=rcs)
 
     def set_keymap(self, layout, model, variant, options):
         if self.uses_systemd():
@@ -1193,6 +1199,3 @@ def uses_systemd():
         return stat.S_ISDIR(res.st_mode)
     except Exception:
         return False
-
-
-# vi: ts=4 expandtab

cloudinit/distros/alpine.py

@@ -173,13 +173,17 @@ def shutdown_command(self, mode="poweroff", delay="now", message=None):
 
         return command
 
-    def uses_systemd(self):
+    @staticmethod
+    def uses_systemd():
         """
         Alpine uses OpenRC, not systemd
         """
         return False
 
-    def manage_service(self, action: str, service: str):
+    @classmethod
+    def manage_service(
+        self, action: str, service: str, *extra_args: str, rcs=None
+    ):
         """
         Perform the requested action on a service. This handles OpenRC
         specific implementation details.
@@ -202,4 +206,4 @@ def manage_service(self, action: str, service: str):
             "status": list(init_cmd) + [service, "status"],
         }
         cmd = list(cmds[action])
-        return subp.subp(cmd, capture=True)
+        return subp.subp(cmd, capture=True, rcs=rcs)

cloudinit/distros/freebsd.py

@@ -37,14 +37,17 @@ class Distro(cloudinit.distros.bsd.BSD):
     prefer_fqdn = True  # See rc.conf(5) in FreeBSD
     home_dir = "/usr/home"
 
-    def manage_service(self, action: str, service: str):
+    @classmethod
+    def manage_service(
+        cls, action: str, service: str, *extra_args: str, rcs=None
+    ):
         """
         Perform the requested action on a service. This handles FreeBSD's
         'service' case. The FreeBSD 'service' is closer in features to
         'systemctl' than SysV init's 'service', so we override it.
         May raise ProcessExecutionError
         """
-        init_cmd = self.init_cmd
+        init_cmd = cls.init_cmd
         cmds = {
             "stop": [service, "stop"],
             "start": [service, "start"],
@@ -55,8 +58,8 @@ def manage_service(self, action: str, service: str):
             "try-reload": [service, "restart"],
             "status": [service, "status"],
         }
-        cmd = list(init_cmd) + list(cmds[action])
-        return subp.subp(cmd, capture=True)
+        cmd = init_cmd + cmds[action] + list(extra_args)
+        return subp.subp(cmd, capture=True, rcs=rcs)
 
     def _get_add_member_to_group_cmd(self, member_name, group_name):
         return ["pw", "usermod", "-n", member_name, "-G", group_name]
@@ -191,6 +194,3 @@ def update_package_sources(self):
             ["update"],
             freq=PER_INSTANCE,
         )
-
-
-# vi: ts=4 expandtab

cloudinit/distros/openbsd.py

@@ -25,13 +25,14 @@ def _write_hostname(self, hostname, filename):
     def _get_add_member_to_group_cmd(self, member_name, group_name):
         return ["usermod", "-G", group_name, member_name]
 
-    def manage_service(self, action: str, service: str):
+    @classmethod
+    def manage_service(cls, action: str, service: str, *extra_args, rcs=None):
         """
         Perform the requested action on a service. This handles OpenBSD's
         'rcctl'.
         May raise ProcessExecutionError
         """
-        init_cmd = self.init_cmd
+        init_cmd = cls.init_cmd
         cmds = {
             "stop": ["stop", service],
             "start": ["start", service],
@@ -43,7 +44,7 @@ def manage_service(self, action: str, service: str):
             "status": ["check", service],
         }
         cmd = list(init_cmd) + list(cmds[action])
-        return subp.subp(cmd, capture=True)
+        return subp.subp(cmd, capture=True, rcs=rcs)
 
     def lock_passwd(self, name):
         try:
@@ -59,6 +60,3 @@ def _get_pkg_cmd_environ(self):
         """Return env vars used in OpenBSD package_command operations"""
         e = os.environ.copy()
         return e
-
-
-# vi: ts=4 expandtab