Add "install hotplug" module (SC-476) (#1069)

This commit removes automatically installing udev rules for hotplug
and adds a module to install them instead.

Automatically including the udev rules and checking if hotplug was
enabled consumed too many resources in certain circumstances. Moving the
rules to a module ensures we don't spend extra extra cycles on hotplug
if hotplug functionality isn't desired.

LP: #1946003

11 files changed, 328 insertions, 66 deletions

diff --git a/cloudinit/cmd/devel/hotplug_hook.py b/cloudinit/cmd/devel/hotplug_hook.py
index d4f0547e..f6f36a00 100644
--- a/cloudinit/cmd/devel/hotplug_hook.py
+++ b/cloudinit/cmd/devel/hotplug_hook.py
@@ -8,6 +8,7 @@ import time
 
 from cloudinit import log
 from cloudinit import reporting
+from cloudinit import stages
 from cloudinit.event import EventScope, EventType
 from cloudinit.net import activators, read_sys_net_safe
 from cloudinit.net.network_state import parse_net_config_data
@@ -164,7 +165,9 @@ def is_enabled(hotplug_init, subsystem):
                 subsystem)
         ) from e
 
-    return hotplug_init.update_event_enabled(
+    return stages.update_event_enabled(
+        datasource=hotplug_init.datasource,
+        cfg=hotplug_init.cfg,
         event_source_type=EventType.HOTPLUG,
         scope=scope
     )
diff --git a/cloudinit/config/cc_install_hotplug.py b/cloudinit/config/cc_install_hotplug.py
new file mode 100644
index 00000000..d6b2a2df
--- /dev/null
+++ b/cloudinit/config/cc_install_hotplug.py
@@ -0,0 +1,136 @@
+# This file is part of cloud-init. See LICENSE file for license information.
+"""Install hotplug udev rules if supported and enabled"""
+import os
+from textwrap import dedent
+
+from cloudinit import util
+from cloudinit import subp
+from cloudinit import stages
+from cloudinit.config.schema import get_schema_doc, validate_cloudconfig_schema
+from cloudinit.distros import ALL_DISTROS
+from cloudinit.event import EventType, EventScope
+from cloudinit.settings import PER_INSTANCE
+
+
+frequency = PER_INSTANCE
+distros = [ALL_DISTROS]
+
+schema = {
+    "id": "cc_install_hotplug",
+    "name": "Install Hotplug",
+    "title": "Install hotplug if supported and enabled",
+    "description": dedent("""\
+        This module will install the udev rules to enable hotplug if
+        supported by the datasource and enabled in the userdata. The udev
+        rules will be installed as
+        ``/etc/udev/rules.d/10-cloud-init-hook-hotplug.rules``.
+
+        When hotplug is enabled, newly added network devices will be added
+        to the system by cloud-init. After udev detects the event,
+        cloud-init will referesh the instance metadata from the datasource,
+        detect the device in the updated metadata, then apply the updated
+        network configuration.
+
+        Currently supported datasources: Openstack, EC2
+    """),
+    "distros": distros,
+    "examples": [
+        dedent("""\
+            # Enable hotplug of network devices
+            updates:
+              network:
+                when: ["hotplug"]
+        """),
+        dedent("""\
+            # Enable network hotplug alongside boot event
+            updates:
+              network:
+                when: ["boot", "hotplug"]
+        """),
+    ],
+    "frequency": frequency,
+    "type": "object",
+    "properties": {
+        "updates": {
+            "type": "object",
+            "additionalProperties": False,
+            "properties": {
+                "network": {
+                    "type": "object",
+                    "required": ["when"],
+                    "additionalProperties": False,
+                    "properties": {
+                        "when": {
+                            "type": "array",
+                            "additionalProperties": False,
+                            "items": {
+                                "type": "string",
+                                "additionalProperties": False,
+                                "enum": [
+                                    "boot-new-instance",
+                                    "boot-legacy",
+                                    "boot",
+                                    "hotplug",
+                                ]
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    }
+}
+
+__doc__ = get_schema_doc(schema)
+
+
+HOTPLUG_UDEV_PATH = "/etc/udev/rules.d/10-cloud-init-hook-hotplug.rules"
+HOTPLUG_UDEV_RULES = """\
+# Installed by cloud-init due to network hotplug userdata
+ACTION!="add|remove", GOTO="cloudinit_end"
+LABEL="cloudinit_hook"
+SUBSYSTEM=="net", RUN+="/usr/lib/cloud-init/hook-hotplug"
+LABEL="cloudinit_end"
+"""
+
+
+def handle(_name, cfg, cloud, log, _args):
+    validate_cloudconfig_schema(cfg, schema)
+    network_hotplug_enabled = (
+        'updates' in cfg and
+        'network' in cfg['updates'] and
+        'when' in cfg['updates']['network'] and
+        'hotplug' in cfg['updates']['network']['when']
+    )
+    hotplug_supported = EventType.HOTPLUG in (
+        cloud.datasource.get_supported_events(
+            [EventType.HOTPLUG]).get(EventScope.NETWORK, set())
+    )
+    hotplug_enabled = stages.update_event_enabled(
+        datasource=cloud.datasource,
+        cfg=cfg,
+        event_source_type=EventType.HOTPLUG,
+        scope=EventScope.NETWORK,
+    )
+    if not (hotplug_supported and hotplug_enabled):
+        if os.path.exists(HOTPLUG_UDEV_PATH):
+            log.debug("Uninstalling hotplug, not enabled")
+            util.del_file(HOTPLUG_UDEV_PATH)
+            subp.subp(["udevadm", "control", "--reload-rules"])
+        elif network_hotplug_enabled:
+            log.warning(
+                "Hotplug is unsupported by current datasource. "
+                "Udev rules will NOT be installed."
+            )
+        else:
+            log.debug("Skipping hotplug install, not enabled")
+        return
+    if not subp.which("udevadm"):
+        log.debug("Skipping hotplug install, udevadm not found")
+        return
+
+    util.write_file(
+        filename=HOTPLUG_UDEV_PATH,
+        content=HOTPLUG_UDEV_RULES,
+    )
+    subp.subp(["udevadm", "control", "--reload-rules"])
diff --git a/cloudinit/stages.py b/cloudinit/stages.py
index 80aa9f5e..731b2982 100644
--- a/cloudinit/stages.py
+++ b/cloudinit/stages.py
@@ -49,6 +49,54 @@ NULL_DATA_SOURCE = None
 NO_PREVIOUS_INSTANCE_ID = "NO_PREVIOUS_INSTANCE_ID"
 
 
+def update_event_enabled(
+    datasource: sources.DataSource,
+    cfg: dict,
+    event_source_type: EventType,
+    scope: EventScope = None
+) -> bool:
+    """Determine if a particular EventType is enabled.
+
+    For the `event_source_type` passed in, check whether this EventType
+    is enabled in the `updates` section of the userdata. If `updates`
+    is not enabled in userdata, check if defined as one of the
+    `default_events` on the datasource. `scope` may be used to
+    narrow the check to a particular `EventScope`.
+
+    Note that on first boot, userdata may NOT be available yet. In this
+    case, we only have the data source's `default_update_events`,
+    so an event that should be enabled in userdata may be denied.
+    """
+    default_events = datasource.default_update_events  # type: Dict[EventScope, Set[EventType]]    # noqa: E501
+    user_events = userdata_to_events(cfg.get('updates', {}))  # type: Dict[EventScope, Set[EventType]]  # noqa: E501
+    # A value in the first will override a value in the second
+    allowed = util.mergemanydict([
+        copy.deepcopy(user_events),
+        copy.deepcopy(default_events),
+    ])
+    LOG.debug('Allowed events: %s', allowed)
+
+    if not scope:
+        scopes = allowed.keys()
+    else:
+        scopes = [scope]
+    scope_values = [s.value for s in scopes]
+
+    for evt_scope in scopes:
+        if event_source_type in allowed.get(evt_scope, []):
+            LOG.debug(
+                'Event Allowed: scope=%s EventType=%s',
+                evt_scope.value, event_source_type
+            )
+            return True
+
+    LOG.debug(
+        'Event Denied: scopes=%s EventType=%s',
+        scope_values, event_source_type
+    )
+    return False
+
+
 class Init(object):
     def __init__(self, ds_deps=None, reporter=None):
         if ds_deps is not None:
@@ -715,46 +763,6 @@ class Init(object):
         return (self.distro.generate_fallback_config(),
                 NetworkConfigSource.fallback)
 
-    def update_event_enabled(
-        self, event_source_type: EventType, scope: EventScope = None
-    ) -> bool:
-        """Determine if a particular EventType is enabled.
-
-        For the `event_source_type` passed in, check whether this EventType
-        is enabled in the `updates` section of the userdata. If `updates`
-        is not enabled in userdata, check if defined as one of the
-        `default_events` on the datasource. `scope` may be used to
-        narrow the check to a particular `EventScope`.
-
-        Note that on first boot, userdata may NOT be available yet. In this
-        case, we only have the data source's `default_update_events`,
-        so an event that should be enabled in userdata may be denied.
-        """
-        default_events = self.datasource.default_update_events  # type: Dict[EventScope, Set[EventType]]    # noqa: E501
-        user_events = userdata_to_events(self.cfg.get('updates', {}))  # type: Dict[EventScope, Set[EventType]]  # noqa: E501
-        # A value in the first will override a value in the second
-        allowed = util.mergemanydict([
-            copy.deepcopy(user_events),
-            copy.deepcopy(default_events),
-        ])
-        LOG.debug('Allowed events: %s', allowed)
-
-        if not scope:
-            scopes = allowed.keys()
-        else:
-            scopes = [scope]
-        scope_values = [s.value for s in scopes]
-
-        for evt_scope in scopes:
-            if event_source_type in allowed.get(evt_scope, []):
-                LOG.debug('Event Allowed: scope=%s EventType=%s',
-                          evt_scope.value, event_source_type)
-                return True
-
-        LOG.debug('Event Denied: scopes=%s EventType=%s',
-                  scope_values, event_source_type)
-        return False
-
     def _apply_netcfg_names(self, netcfg):
         try:
             LOG.debug("applying net config names for %s", netcfg)
@@ -784,8 +792,11 @@ class Init(object):
             return
 
         def event_enabled_and_metadata_updated(event_type):
-            return self.update_event_enabled(
-                event_type, scope=EventScope.NETWORK
+            return update_event_enabled(
+                datasource=self.datasource,
+                cfg=self.cfg,
+                event_source_type=event_type,
+                scope=EventScope.NETWORK
             ) and self.datasource.update_metadata_if_supported([event_type])
 
         def should_run_on_boot_event():
diff --git a/config/cloud.cfg.tmpl b/config/cloud.cfg.tmpl
index 66c48fd5..b66bbe60 100644
--- a/config/cloud.cfg.tmpl
+++ b/config/cloud.cfg.tmpl
@@ -166,6 +166,7 @@ cloud_final_modules:
  - scripts-user
  - ssh-authkey-fingerprints
  - keys-to-console
+ - install-hotplug
  - phone-home
  - final-message
  - power-state-change
diff --git a/doc/rtd/topics/modules.rst b/doc/rtd/topics/modules.rst
index e30fe0fe..3ca6b9e3 100644
--- a/doc/rtd/topics/modules.rst
+++ b/doc/rtd/topics/modules.rst
@@ -22,6 +22,7 @@ Modules
 .. automodule:: cloudinit.config.cc_foo
 .. automodule:: cloudinit.config.cc_growpart
 .. automodule:: cloudinit.config.cc_grub_dpkg
+.. automodule:: cloudinit.config.cc_install_hotplug
 .. automodule:: cloudinit.config.cc_keys_to_console
 .. automodule:: cloudinit.config.cc_landscape
 .. automodule:: cloudinit.config.cc_locale
diff --git a/tests/integration_tests/modules/test_hotplug.py b/tests/integration_tests/modules/test_hotplug.py
index 88cd8c16..f5abc86f 100644
--- a/tests/integration_tests/modules/test_hotplug.py
+++ b/tests/integration_tests/modules/test_hotplug.py
@@ -49,10 +49,13 @@ def test_hotplug_add_remove(client: IntegrationInstance):
     ips_before = _get_ip_addr(client)
     log = client.read_from_file('/var/log/cloud-init.log')
     assert 'Exiting hotplug handler' not in log
+    assert client.execute(
+        'test -f /etc/udev/rules.d/10-cloud-init-hook-hotplug.rules'
+    ).ok
 
     # Add new NIC
     added_ip = client.instance.add_network_interface()
-    _wait_till_hotplug_complete(client, expected_runs=2)
+    _wait_till_hotplug_complete(client, expected_runs=1)
     ips_after_add = _get_ip_addr(client)
     new_addition = [ip for ip in ips_after_add if ip.ip4 == added_ip][0]
 
@@ -67,7 +70,7 @@ def test_hotplug_add_remove(client: IntegrationInstance):
 
     # Remove new NIC
     client.instance.remove_network_interface(added_ip)
-    _wait_till_hotplug_complete(client, expected_runs=4)
+    _wait_till_hotplug_complete(client, expected_runs=2)
     ips_after_remove = _get_ip_addr(client)
     assert len(ips_after_remove) == len(ips_before)
     assert added_ip not in [ip.ip4 for ip in ips_after_remove]
@@ -86,12 +89,14 @@ def test_no_hotplug_in_userdata(client: IntegrationInstance):
     ips_before = _get_ip_addr(client)
     log = client.read_from_file('/var/log/cloud-init.log')
     assert 'Exiting hotplug handler' not in log
+    assert client.execute(
+        'test -f /etc/udev/rules.d/10-cloud-init-hook-hotplug.rules'
+    ).failed
 
     # Add new NIC
     client.instance.add_network_interface()
-    _wait_till_hotplug_complete(client)
     log = client.read_from_file('/var/log/cloud-init.log')
-    assert "Event Denied: scopes=['network'] EventType=hotplug" in log
+    assert 'hotplug-hook' not in log
 
     ips_after_add = _get_ip_addr(client)
     if len(ips_after_add) == len(ips_before) + 1:
diff --git a/tests/unittests/cmd/devel/test_hotplug_hook.py b/tests/unittests/cmd/devel/test_hotplug_hook.py
index 63d2490e..e1c64e2f 100644
--- a/tests/unittests/cmd/devel/test_hotplug_hook.py
+++ b/tests/unittests/cmd/devel/test_hotplug_hook.py
@@ -30,6 +30,11 @@ def mocks():
         return_value=FAKE_MAC
     )
 
+    update_event_enabled = mock.patch(
+        'cloudinit.stages.update_event_enabled',
+        return_value=True,
+    )
+
     m_network_state = mock.MagicMock(spec=NetworkState)
     parse_net = mock.patch(
         'cloudinit.cmd.devel.hotplug_hook.parse_net_config_data',
@@ -45,6 +50,7 @@ def mocks():
     sleep = mock.patch('time.sleep')
 
     read_sys_net.start()
+    update_event_enabled.start()
     parse_net.start()
     select_activator.start()
     m_sleep = sleep.start()
@@ -57,6 +63,7 @@ def mocks():
     )
 
     read_sys_net.stop()
+    update_event_enabled.stop()
     parse_net.stop()
     select_activator.stop()
     sleep.stop()
@@ -122,13 +129,16 @@ class TestHotplug:
 
     def test_update_event_disabled(self, mocks, caplog):
         init = mocks.m_init
-        init.update_event_enabled.return_value = False
-        handle_hotplug(
-            hotplug_init=init,
-            devpath='/dev/fake',
-            udevaction='remove',
-            subsystem='net'
-        )
+        with mock.patch(
+            'cloudinit.stages.update_event_enabled',
+            return_value=False
+        ):
+            handle_hotplug(
+                hotplug_init=init,
+                devpath='/dev/fake',
+                udevaction='remove',
+                subsystem='net'
+            )
         assert 'hotplug not enabled for event of type' in caplog.text
         init.datasource.update_metadata_if_supported.assert_not_called()
         mocks.m_activator.bring_up_interface.assert_not_called()
diff --git a/tests/unittests/test_handler/test_handler_install_hotplug.py b/tests/unittests/test_handler/test_handler_install_hotplug.py
new file mode 100644
index 00000000..19b0cc41
--- /dev/null
+++ b/tests/unittests/test_handler/test_handler_install_hotplug.py
@@ -0,0 +1,104 @@
+# This file is part of cloud-init. See LICENSE file for license information.
+from collections import namedtuple
+from unittest import mock
+
+import pytest
+
+from cloudinit.config.cc_install_hotplug import (
+    handle,
+    HOTPLUG_UDEV_PATH,
+    HOTPLUG_UDEV_RULES,
+)
+from cloudinit.event import EventScope, EventType
+
+
+@pytest.yield_fixture()
+def mocks():
+    m_update_enabled = mock.patch('cloudinit.stages.update_event_enabled')
+    m_write = mock.patch('cloudinit.util.write_file', autospec=True)
+    m_del = mock.patch('cloudinit.util.del_file', autospec=True)
+    m_subp = mock.patch('cloudinit.subp.subp')
+    m_which = mock.patch('cloudinit.subp.which', return_value=None)
+    m_path_exists = mock.patch('os.path.exists', return_value=False)
+
+    yield namedtuple(
+        'Mocks',
+        'm_update_enabled m_write m_del m_subp m_which m_path_exists'
+    )(
+        m_update_enabled.start(), m_write.start(), m_del.start(),
+        m_subp.start(), m_which.start(), m_path_exists.start()
+    )
+
+    m_update_enabled.stop()
+    m_write.stop()
+    m_del.stop()
+    m_subp.stop()
+    m_which.stop()
+    m_path_exists.stop()
+
+
+class TestInstallHotplug:
+    def test_rules_installed_when_supported_and_enabled(self, mocks):
+        mocks.m_which.return_value = 'udevadm'
+        mocks.m_update_enabled.return_value = True
+        m_cloud = mock.MagicMock()
+        m_cloud.datasource.get_supported_events.return_value = {
+            EventScope.NETWORK: {EventType.HOTPLUG}
+        }
+
+        handle(None, {}, m_cloud, mock.Mock(), None)
+        mocks.m_write.assert_called_once_with(
+            filename=HOTPLUG_UDEV_PATH,
+            content=HOTPLUG_UDEV_RULES,
+        )
+        assert mocks.m_subp.call_args_list == [mock.call([
+            'udevadm', 'control', '--reload-rules',
+        ])]
+        assert mocks.m_del.call_args_list == []
+
+    def test_rules_not_installed_when_unsupported(self, mocks):
+        mocks.m_update_enabled.return_value = True
+        m_cloud = mock.MagicMock()
+        m_cloud.datasource.get_supported_events.return_value = {}
+
+        handle(None, {}, m_cloud, mock.Mock(), None)
+        assert mocks.m_write.call_args_list == []
+        assert mocks.m_del.call_args_list == []
+        assert mocks.m_subp.call_args_list == []
+
+    def test_rules_not_installed_when_disabled(self, mocks):
+        mocks.m_update_enabled.return_value = False
+        m_cloud = mock.MagicMock()
+        m_cloud.datasource.get_supported_events.return_value = {
+            EventScope.NETWORK: {EventType.HOTPLUG}
+        }
+
+        handle(None, {}, m_cloud, mock.Mock(), None)
+        assert mocks.m_write.call_args_list == []
+        assert mocks.m_del.call_args_list == []
+        assert mocks.m_subp.call_args_list == []
+
+    def test_rules_uninstalled_when_disabled(self, mocks):
+        mocks.m_path_exists.return_value = True
+        mocks.m_update_enabled.return_value = False
+        m_cloud = mock.MagicMock()
+        m_cloud.datasource.get_supported_events.return_value = {}
+
+        handle(None, {}, m_cloud, mock.Mock(), None)
+        mocks.m_del.assert_called_with(HOTPLUG_UDEV_PATH)
+        assert mocks.m_subp.call_args_list == [mock.call([
+            'udevadm', 'control', '--reload-rules',
+        ])]
+        assert mocks.m_write.call_args_list == []
+
+    def test_rules_not_installed_when_no_udevadm(self, mocks):
+        mocks.m_update_enabled.return_value = True
+        m_cloud = mock.MagicMock()
+        m_cloud.datasource.get_supported_events.return_value = {
+            EventScope.NETWORK: {EventType.HOTPLUG}
+        }
+
+        handle(None, {}, m_cloud, mock.Mock(), None)
+        assert mocks.m_del.call_args_list == []
+        assert mocks.m_write.call_args_list == []
+        assert mocks.m_subp.call_args_list == []
diff --git a/tests/unittests/test_handler/test_schema.py b/tests/unittests/test_handler/test_schema.py
index 59f58f7c..1dae223d 100644
--- a/tests/unittests/test_handler/test_schema.py
+++ b/tests/unittests/test_handler/test_schema.py
@@ -36,7 +36,8 @@ class GetSchemaTest(CiTestCase):
                 'cc_write_files',
                 'cc_write_files_deferred',
                 'cc_zypper_add_repo',
-                'cc_chef'
+                'cc_chef',
+                'cc_install_hotplug',
             ],
             [subschema['id'] for subschema in schema['allOf']])
         self.assertEqual('cloud-config-schema', schema['id'])
diff --git a/tools/hook-hotplug b/tools/hook-hotplug
index ced268b3..35bd3da2 100755
--- a/tools/hook-hotplug
+++ b/tools/hook-hotplug
@@ -8,11 +8,7 @@ is_finished() {
     [ -e /run/cloud-init/result.json ]
 }
 
-hotplug_enabled() {
-    [ "$(cloud-init devel hotplug-hook -s "${SUBSYSTEM}" query)" == "enabled" ]
-}
-
-if is_finished && hotplug_enabled; then
+if is_finished; then
     # open cloud-init's hotplug-hook fifo rw
     exec 3<>/run/cloud-init/hook-hotplug-cmd
     env_params=(
diff --git a/udev/10-cloud-init-hook-hotplug.rules b/udev/10-cloud-init-hook-hotplug.rules
deleted file mode 100644
index 2e382679..00000000
--- a/udev/10-cloud-init-hook-hotplug.rules
+++ /dev/null
@@ -1,6 +0,0 @@
-# This file is part of cloud-init. See LICENSE file for license information.
-# Handle device adds only
-ACTION!="add|remove", GOTO="cloudinit_end"
-LABEL="cloudinit_hook"
-SUBSYSTEM=="net|block", RUN+="/usr/lib/cloud-init/hook-hotplug"
-LABEL="cloudinit_end"