dhclient_hook: remove vestigal dhclient_hook command (#2015)

At inception[1], dhclient hooks were used to filter environment variables into /run/cloud-init/dhclient.hooks/<interface>.json which was consumed by WALinuxAgentShim. The fallback method was to parse the dhcp client lease file.

Today the Azure datasource directly uses the parsed lease file[2], and loading /run/cloud-init/dhclient.hook/<interface>.json file was removed in 22.2[3]. With no other consumers, we can remove this.

[1] https://github.com/canonical/cloud-init/commit/648dbbf6b090c81e989f1ab70bf99f4de16a6a70
[2] https://github.com/canonical/cloud-init/blob/main/cloudinit/sources/DataSourceAzure.py#L447
[3] https://github.com/canonical/cloud-init/commit/5ad0768a796bc07232476d0d29b5225f1e6e131c

13 files changed, 3 insertions, 325 deletions

diff --git a/bash_completion/cloud-init b/bash_completion/cloud-init
index 579005d2..be783977 100644
--- a/bash_completion/cloud-init
+++ b/bash_completion/cloud-init
@@ -10,7 +10,7 @@ _cloudinit_complete()
     cur_word="${COMP_WORDS[COMP_CWORD]}"
     prev_word="${COMP_WORDS[COMP_CWORD-1]}"
 
-    subcmds="analyze clean collect-logs devel dhclient-hook features init modules query schema single status"
+    subcmds="analyze clean collect-logs devel features init modules query schema single status"
     base_params="--help --file --version --debug --force"
     case ${COMP_CWORD} in
         1)
@@ -30,9 +30,6 @@ _cloudinit_complete()
                 devel)
                     COMPREPLY=($(compgen -W "--help hotplug-hook net-convert" -- $cur_word))
                     ;;
-                dhclient-hook)
-                    COMPREPLY=($(compgen -W "--help up down" -- $cur_word))
-                    ;;
                 features)
                     COMPREPLY=($(compgen -W "--help" -- $cur_word))
                     ;;
diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py
index f16c59a7..9c2307ac 100755
--- a/cloudinit/cmd/main.py
+++ b/cloudinit/cmd/main.py
@@ -47,7 +47,6 @@ from cloudinit.settings import PER_INSTANCE, PER_ALWAYS, PER_ONCE, CLOUD_CONFIG
 from cloudinit import atomic_helper
 
 from cloudinit.config import cc_set_hostname
-from cloudinit import dhclient_hook
 from cloudinit.cmd.devel import read_cfg_paths
 
 
@@ -946,11 +945,6 @@ def main(sysv_args=None):
         help="Query standardized instance metadata from the command line.",
     )
 
-    parser_dhclient = subparsers.add_parser(
-        dhclient_hook.NAME, help=dhclient_hook.__doc__
-    )
-    dhclient_hook.get_parser(parser_dhclient)
-
     parser_features = subparsers.add_parser(
         "features", help="List defined features."
     )
diff --git a/cloudinit/dhclient_hook.py b/cloudinit/dhclient_hook.py
deleted file mode 100644
index 46b2e8d9..00000000
--- a/cloudinit/dhclient_hook.py
+++ /dev/null
@@ -1,90 +0,0 @@
-# This file is part of cloud-init. See LICENSE file for license information.
-
-"""Run the dhclient hook to record network info."""
-
-import argparse
-import os
-
-from cloudinit import atomic_helper
-from cloudinit import log as logging
-from cloudinit import stages
-
-LOG = logging.getLogger(__name__)
-
-NAME = "dhclient-hook"
-UP = "up"
-DOWN = "down"
-EVENTS = (UP, DOWN)
-
-
-def _get_hooks_dir():
-    i = stages.Init()
-    return os.path.join(i.paths.get_runpath(), "dhclient.hooks")
-
-
-def _filter_env_vals(info):
-    """Given info (os.environ), return a dictionary with
-    lower case keys for each entry starting with DHCP4_ or new_."""
-    new_info = {}
-    for k, v in info.items():
-        if k.startswith("DHCP4_") or k.startswith("new_"):
-            key = (k.replace("DHCP4_", "").replace("new_", "")).lower()
-            new_info[key] = v
-    return new_info
-
-
-def run_hook(interface, event, data_d=None, env=None):
-    if event not in EVENTS:
-        raise ValueError(
-            "Unexpected event '%s'. Expected one of: %s" % (event, EVENTS)
-        )
-    if data_d is None:
-        data_d = _get_hooks_dir()
-    if env is None:
-        env = os.environ
-    hook_file = os.path.join(data_d, interface + ".json")
-
-    if event == UP:
-        if not os.path.exists(data_d):
-            os.makedirs(data_d)
-        atomic_helper.write_json(hook_file, _filter_env_vals(env))
-        LOG.debug("Wrote dhclient options in %s", hook_file)
-    elif event == DOWN:
-        if os.path.exists(hook_file):
-            os.remove(hook_file)
-            LOG.debug("Removed dhclient options file %s", hook_file)
-
-
-def get_parser(parser=None):
-    if parser is None:
-        parser = argparse.ArgumentParser(prog=NAME, description=__doc__)
-    parser.add_argument(
-        "event", help="event taken on the interface", choices=EVENTS
-    )
-    parser.add_argument(
-        "interface", help="the network interface being acted upon"
-    )
-    # cloud-init main uses 'action'
-    parser.set_defaults(action=(NAME, handle_args))
-    return parser
-
-
-def handle_args(name, args, data_d=None):
-    """Handle the Namespace args.
-    Takes 'name' as passed by cloud-init main. not used here."""
-    return run_hook(interface=args.interface, event=args.event, data_d=data_d)
-
-
-if __name__ == "__main__":
-    import sys
-
-    parser = get_parser()
-    args = parser.parse_args(args=sys.argv[1:])
-    return_value = handle_args(
-        NAME, args, data_d=os.environ.get("_CI_DHCP_HOOK_DATA_D")
-    )
-    if return_value:
-        sys.exit(return_value)
-
-
-# vi: ts=4 expandtab
diff --git a/doc/man/cloud-init.1 b/doc/man/cloud-init.1
index 388617de..beca40c6 100644
--- a/doc/man/cloud-init.1
+++ b/doc/man/cloud-init.1
@@ -58,10 +58,6 @@ Remove logs and artifacts so cloud-init can re-run.
 Run development tools. See help output for subcommand details.
 
 .TP
-.B "dhclient-hook"
-Run the dhclient hook to record network info.
-
-.TP
 .B "features"
 List defined features.
 
diff --git a/doc/rtd/reference/cli.rst b/doc/rtd/reference/cli.rst
index 246b9721..75beb8ec 100644
--- a/doc/rtd/reference/cli.rst
+++ b/doc/rtd/reference/cli.rst
@@ -16,7 +16,7 @@ Example output:
 .. code-block::
 
    usage: cloud-init [-h] [--version] [--file FILES] [--debug] [--force]
-                                                               {init,modules,single,query,dhclient-hook,features,analyze,devel,collect-logs,clean,status,schema} ...
+                                                               {init,modules,single,query,features,analyze,devel,collect-logs,clean,status,schema} ...
 
     options:
       -h, --help            show this help message and exit
@@ -27,12 +27,11 @@ Example output:
       --force               Force running even if no datasource is found (use at your own risk).
 
     Subcommands:
-      {init,modules,single,query,dhclient-hook,features,analyze,devel,collect-logs,clean,status,schema}
+      {init,modules,single,query,features,analyze,devel,collect-logs,clean,status,schema}
         init                Initialize cloud-init and perform initial modules.
         modules             Activate modules using a given configuration key.
         single              Run a single module.
         query               Query standardized instance metadata from the command line.
-        dhclient-hook       Run the dhclient hook to record network info.
         features            List defined features.
         analyze             Devel tool: Analyze cloud-init logs and data.
         devel               Run development tools.
diff --git a/packages/redhat/cloud-init.spec.in b/packages/redhat/cloud-init.spec.in
index 5cbf828a..97e95096 100644
--- a/packages/redhat/cloud-init.spec.in
+++ b/packages/redhat/cloud-init.spec.in
@@ -179,9 +179,6 @@ fi
 %attr(0755, root, root) %{_initddir}/cloud-init
 %endif
 
-%{_sysconfdir}/NetworkManager/dispatcher.d/hook-network-manager
-%{_sysconfdir}/dhcp/dhclient-exit-hooks.d/hook-dhclient
-
 # Program binaries
 %{_bindir}/cloud-init*
 %{_bindir}/cloud-id*
diff --git a/packages/suse/cloud-init.spec.in b/packages/suse/cloud-init.spec.in
index 2586f248..62a9129b 100644
--- a/packages/suse/cloud-init.spec.in
+++ b/packages/suse/cloud-init.spec.in
@@ -126,8 +126,6 @@ version_pys=$(cd "%{buildroot}" && find . -name version.py -type f)
 # Bash completion script
 %{_datadir}/bash-completion/completions/cloud-init
 
-%{_sysconfdir}/dhcp/dhclient-exit-hooks.d/hook-dhclient
-%{_sysconfdir}/NetworkManager/dispatcher.d/hook-network-manager
 %{_sysconfdir}/systemd/system/sshd-keygen@.service.d/disable-sshd-keygen-if-cloud-init-active.conf
 
 # Python code is here...
diff --git a/setup.py b/setup.py
index 04aae5b2..a6dbc5c2 100644
--- a/setup.py
+++ b/setup.py
@@ -316,11 +316,6 @@ if not platform.system().endswith("BSD"):
 
     data_files.extend(
         [
-            (
-                ETC + "/NetworkManager/dispatcher.d/",
-                ["tools/hook-network-manager"],
-            ),
-            (ETC + "/dhcp/dhclient-exit-hooks.d/", ["tools/hook-dhclient"]),
             (RULES_PATH + "/udev/rules.d", [f for f in glob("udev/*.rules")]),
             (
                 ETC + "/systemd/system/sshd-keygen@.service.d/",
diff --git a/tests/unittests/test_cli.py b/tests/unittests/test_cli.py
index 2d57ba04..07294214 100644
--- a/tests/unittests/test_cli.py
+++ b/tests/unittests/test_cli.py
@@ -149,7 +149,6 @@ class TestCLI:
             "analyze",
             "clean",
             "devel",
-            "dhclient-hook",
             "features",
             "init",
             "modules",
@@ -319,19 +318,6 @@ class TestCLI:
         assert "cc_ntp" == parseargs.name
         assert False is parseargs.report
 
-    @mock.patch("cloudinit.cmd.main.dhclient_hook.handle_args")
-    def test_dhclient_hook_subcommand(self, m_handle_args):
-        """The subcommand 'dhclient-hook' calls dhclient_hook with args."""
-        self._call_main(["cloud-init", "dhclient-hook", "up", "eth0"])
-        (name, parseargs) = m_handle_args.call_args_list[0][0]
-        assert "dhclient-hook" == name
-        assert "dhclient-hook" == parseargs.subcommand
-        assert "dhclient-hook" == parseargs.action[0]
-        assert False is parseargs.debug
-        assert False is parseargs.force
-        assert "up" == parseargs.event
-        assert "eth0" == parseargs.interface
-
     @mock.patch("cloudinit.cmd.main.main_features")
     def test_features_hook_subcommand(self, m_features):
         """The subcommand 'features' calls main_features with args."""
diff --git a/tests/unittests/test_dhclient_hook.py b/tests/unittests/test_dhclient_hook.py
deleted file mode 100644
index 7e5b54c0..00000000
--- a/tests/unittests/test_dhclient_hook.py
+++ /dev/null
@@ -1,112 +0,0 @@
-# This file is part of cloud-init. See LICENSE file for license information.
-
-"""Tests for cloudinit.dhclient_hook."""
-
-import argparse
-import json
-import os
-from unittest import mock
-
-from cloudinit import dhclient_hook as dhc
-from tests.unittests.helpers import CiTestCase, dir2dict, populate_dir
-
-
-class TestDhclientHook(CiTestCase):
-
-    ex_env = {
-        "interface": "eth0",
-        "new_dhcp_lease_time": "3600",
-        "new_host_name": "x1",
-        "new_ip_address": "10.145.210.163",
-        "new_subnet_mask": "255.255.255.0",
-        "old_host_name": "x1",
-        "PATH": "/usr/sbin:/usr/bin:/sbin:/bin",
-        "pid": "614",
-        "reason": "BOUND",
-    }
-
-    # some older versions of dhclient put the same content,
-    # but in upper case with DHCP4_ instead of new_
-    ex_env_dhcp4 = {
-        "REASON": "BOUND",
-        "DHCP4_dhcp_lease_time": "3600",
-        "DHCP4_host_name": "x1",
-        "DHCP4_ip_address": "10.145.210.163",
-        "DHCP4_subnet_mask": "255.255.255.0",
-        "INTERFACE": "eth0",
-        "PATH": "/usr/sbin:/usr/bin:/sbin:/bin",
-        "pid": "614",
-    }
-
-    expected = {
-        "dhcp_lease_time": "3600",
-        "host_name": "x1",
-        "ip_address": "10.145.210.163",
-        "subnet_mask": "255.255.255.0",
-    }
-
-    def setUp(self):
-        super(TestDhclientHook, self).setUp()
-        self.tmp = self.tmp_dir()
-
-    def test_handle_args(self):
-        """quick test of call to handle_args."""
-        nic = "eth0"
-        args = argparse.Namespace(event=dhc.UP, interface=nic)
-        with mock.patch.dict("os.environ", clear=True, values=self.ex_env):
-            dhc.handle_args(dhc.NAME, args, data_d=self.tmp)
-        found = dir2dict(self.tmp + os.path.sep)
-        self.assertEqual([nic + ".json"], list(found.keys()))
-        self.assertEqual(self.expected, json.loads(found[nic + ".json"]))
-
-    def test_run_hook_up_creates_dir(self):
-        """If dir does not exist, run_hook should create it."""
-        subd = self.tmp_path("subdir", self.tmp)
-        nic = "eth1"
-        dhc.run_hook(nic, "up", data_d=subd, env=self.ex_env)
-        self.assertEqual(
-            set([nic + ".json"]), set(dir2dict(subd + os.path.sep))
-        )
-
-    def test_run_hook_up(self):
-        """Test expected use of run_hook_up."""
-        nic = "eth0"
-        dhc.run_hook(nic, "up", data_d=self.tmp, env=self.ex_env)
-        found = dir2dict(self.tmp + os.path.sep)
-        self.assertEqual([nic + ".json"], list(found.keys()))
-        self.assertEqual(self.expected, json.loads(found[nic + ".json"]))
-
-    def test_run_hook_up_dhcp4_prefix(self):
-        """Test run_hook filters correctly with older DHCP4_ data."""
-        nic = "eth0"
-        dhc.run_hook(nic, "up", data_d=self.tmp, env=self.ex_env_dhcp4)
-        found = dir2dict(self.tmp + os.path.sep)
-        self.assertEqual([nic + ".json"], list(found.keys()))
-        self.assertEqual(self.expected, json.loads(found[nic + ".json"]))
-
-    def test_run_hook_down_deletes(self):
-        """down should delete the created json file."""
-        nic = "eth1"
-        populate_dir(
-            self.tmp, {nic + ".json": "{'abcd'}", "myfile.txt": "text"}
-        )
-        dhc.run_hook(nic, "down", data_d=self.tmp, env={"old_host_name": "x1"})
-        self.assertEqual(
-            set(["myfile.txt"]), set(dir2dict(self.tmp + os.path.sep))
-        )
-
-    def test_get_parser(self):
-        """Smoke test creation of get_parser."""
-        # cloud-init main uses 'action'.
-        event, interface = (dhc.UP, "mynic0")
-        self.assertEqual(
-            argparse.Namespace(
-                event=event,
-                interface=interface,
-                action=(dhc.NAME, dhc.handle_args),
-            ),
-            dhc.get_parser().parse_args([event, interface]),
-        )
-
-
-# vi: ts=4 expandtab
diff --git a/tools/hook-dhclient b/tools/hook-dhclient
deleted file mode 100755
index 02122f37..00000000
--- a/tools/hook-dhclient
+++ /dev/null
@@ -1,27 +0,0 @@
-#!/bin/sh
-# This file is part of cloud-init. See LICENSE file for license information.
-
-# This script writes DHCP lease information into the cloud-init run directory
-# It is sourced, not executed.  For more information see dhclient-script(8).
-
-is_azure() {
-    local dmi_path="/sys/class/dmi/id/board_vendor" vendor=""
-    if [ -e "$dmi_path" ] && read vendor < "$dmi_path"; then
-        [ "$vendor" = "Microsoft Corporation" ] && return 0
-    fi
-    return 1
-}
-
-is_enabled() {
-    # only execute hooks if cloud-init is enabled and on azure
-    [ -e /run/cloud-init/enabled ] || return 1
-    is_azure
-}
-
-if is_enabled; then
-   case "$reason" in
-      BOUND) cloud-init dhclient-hook up "$interface";;
-      DOWN|RELEASE|REBOOT|STOP|EXPIRE)
-         cloud-init dhclient-hook down "$interface";;
-   esac
-fi
diff --git a/tools/hook-network-manager b/tools/hook-network-manager
deleted file mode 100755
index 67d9044a..00000000
--- a/tools/hook-network-manager
+++ /dev/null
@@ -1,26 +0,0 @@
-#!/bin/sh
-# This file is part of cloud-init. See LICENSE file for license information.
-
-# This script hooks into NetworkManager(8) via its scripts
-# arguments are 'interface-name' and 'action'
-#
-is_azure() {
-    local dmi_path="/sys/class/dmi/id/board_vendor" vendor=""
-    if [ -e "$dmi_path" ] && read vendor < "$dmi_path"; then
-        [ "$vendor" = "Microsoft Corporation" ] && return 0
-    fi
-    return 1
-}
-
-is_enabled() {
-    # only execute hooks if cloud-init is enabled and on azure
-    [ -e /run/cloud-init/enabled ] || return 1
-    is_azure
-}
-
-if is_enabled; then
-    case "$1:$2" in
-        *:up) exec cloud-init dhclient-hook up "$1";;
-        *:down) exec cloud-init dhclient-hook down "$1";;
-    esac
-fi
diff --git a/tools/hook-rhel.sh b/tools/hook-rhel.sh
deleted file mode 100755
index 513a5515..00000000
--- a/tools/hook-rhel.sh
+++ /dev/null
@@ -1,29 +0,0 @@
-#!/bin/sh
-# This file is part of cloud-init. See LICENSE file for license information.
-
-# Current versions of RHEL and CentOS do not honor the directory
-# /etc/dhcp/dhclient-exit-hooks.d so this file can be placed in
-# /etc/dhcp/dhclient.d instead
-is_azure() {
-    local dmi_path="/sys/class/dmi/id/board_vendor" vendor=""
-    if [ -e "$dmi_path" ] && read vendor < "$dmi_path"; then
-        [ "$vendor" = "Microsoft Corporation" ] && return 0
-    fi
-    return 1
-}
-
-is_enabled() {
-    # only execute hooks if cloud-init is enabled and on azure
-    [ -e /run/cloud-init/enabled ] || return 1
-    is_azure
-}
-
-hook-rhel_config(){
-    is_enabled || return 0
-    cloud-init dhclient-hook up "$interface"
-}
-
-hook-rhel_restore(){
-    is_enabled || return 0
-    cloud-init dhclient-hook down "$interface"
-}