Only invoke hotplug socket when functionality is enabled (#952)

Alters hotplug hook to have a query mechanism checking if the
functionality is enabled. This allows us to avoid using the hotplug
socket and service when hotplug is disabled.

4 files changed, 112 insertions, 52 deletions

diff --git a/cloudinit/cmd/devel/hotplug_hook.py b/cloudinit/cmd/devel/hotplug_hook.py
index 0282f24a..a0058f03 100644
--- a/cloudinit/cmd/devel/hotplug_hook.py
+++ b/cloudinit/cmd/devel/hotplug_hook.py
@@ -3,6 +3,7 @@
 import abc
 import argparse
 import os
+import sys
 import time
 
 from cloudinit import log
@@ -12,7 +13,7 @@ from cloudinit.net import activators, read_sys_net_safe
 from cloudinit.net.network_state import parse_net_config_data
 from cloudinit.reporting import events
 from cloudinit.stages import Init
-from cloudinit.sources import DataSource
+from cloudinit.sources import DataSource, DataSourceNotFoundException
 
 
 LOG = log.getLogger(__name__)
@@ -31,15 +32,35 @@ def get_parser(parser=None):
         parser = argparse.ArgumentParser(prog=NAME, description=__doc__)
 
     parser.description = __doc__
-    parser.add_argument("-d", "--devpath", required=True,
-                        metavar="PATH",
-                        help="sysfs path to hotplugged device")
-    parser.add_argument("-s", "--subsystem", required=True,
-                        help="subsystem to act on",
-                        choices=['net'])
-    parser.add_argument("-u", "--udevaction", required=True,
-                        help="action to take",
-                        choices=['add', 'remove'])
+    parser.add_argument(
+        "-s", "--subsystem", required=True,
+        help="subsystem to act on",
+        choices=['net']
+    )
+
+    subparsers = parser.add_subparsers(
+        title='Hotplug Action',
+        dest='hotplug_action'
+    )
+    subparsers.required = True
+
+    subparsers.add_parser(
+        'query',
+        help='query if hotplug is enabled for given subsystem'
+    )
+
+    parser_handle = subparsers.add_parser(
+        'handle', help='handle the hotplug event')
+    parser_handle.add_argument(
+        "-d", "--devpath", required=True,
+        metavar="PATH",
+        help="sysfs path to hotplugged device"
+    )
+    parser_handle.add_argument(
+        "-u", "--udevaction", required=True,
+        help="action to take",
+        choices=['add', 'remove']
+    )
 
     return parser
 
@@ -133,27 +154,42 @@ SUBSYSTEM_PROPERTES_MAP = {
 }
 
 
-def handle_hotplug(
-    hotplug_init: Init, devpath, subsystem, udevaction
-):
-    handler_cls, event_scope = SUBSYSTEM_PROPERTES_MAP.get(
-        subsystem, (None, None)
-    )
-    if handler_cls is None:
+def is_enabled(hotplug_init, subsystem):
+    try:
+        scope = SUBSYSTEM_PROPERTES_MAP[subsystem][1]
+    except KeyError as e:
         raise Exception(
             'hotplug-hook: cannot handle events for subsystem: {}'.format(
-                subsystem))
+                subsystem)
+        ) from e
+
+    return hotplug_init.update_event_enabled(
+        event_source_type=EventType.HOTPLUG,
+        scope=scope
+    )
+
 
+def initialize_datasource(hotplug_init, subsystem):
     LOG.debug('Fetching datasource')
     datasource = hotplug_init.fetch(existing="trust")
 
-    if not hotplug_init.update_event_enabled(
-        event_source_type=EventType.HOTPLUG,
-        scope=EventScope.NETWORK
-    ):
-        LOG.debug('hotplug not enabled for event of type %s', event_scope)
+    if not datasource.get_supported_events([EventType.HOTPLUG]):
+        LOG.debug('hotplug not supported for event of type %s', subsystem)
         return
 
+    if not is_enabled(hotplug_init, subsystem):
+        LOG.debug('hotplug not enabled for event of type %s', subsystem)
+        return
+    return datasource
+
+
+def handle_hotplug(
+    hotplug_init: Init, devpath, subsystem, udevaction
+):
+    datasource = initialize_datasource(hotplug_init, subsystem)
+    if not datasource:
+        return
+    handler_cls = SUBSYSTEM_PROPERTES_MAP[subsystem][0]
     LOG.debug('Creating %s event handler', subsystem)
     event_handler = handler_cls(
         datasource=datasource,
@@ -200,29 +236,36 @@ def handle_args(name, args):
     log.setupLogging(hotplug_init.cfg)
     if 'reporting' in hotplug_init.cfg:
         reporting.update_configuration(hotplug_init.cfg.get('reporting'))
-
     # Logging isn't going to be setup until now
     LOG.debug(
-        '%s called with the following arguments: {udevaction: %s, '
-        'subsystem: %s, devpath: %s}',
-        name, args.udevaction, args.subsystem, args.devpath
-    )
-    LOG.debug(
-        '%s called with the following arguments:\n'
-        'udevaction: %s\n'
-        'subsystem: %s\n'
-        'devpath: %s',
-        name, args.udevaction, args.subsystem, args.devpath
+        '%s called with the following arguments: {'
+        'hotplug_action: %s, subsystem: %s, udevaction: %s, devpath: %s}',
+        name,
+        args.hotplug_action,
+        args.subsystem,
+        args.udevaction if 'udevaction' in args else None,
+        args.devpath if 'devpath' in args else None,
     )
 
     with hotplug_reporter:
         try:
-            handle_hotplug(
-                hotplug_init=hotplug_init,
-                devpath=args.devpath,
-                subsystem=args.subsystem,
-                udevaction=args.udevaction,
-            )
+            if args.hotplug_action == 'query':
+                try:
+                    datasource = initialize_datasource(
+                        hotplug_init, args.subsystem)
+                except DataSourceNotFoundException:
+                    print(
+                        "Unable to determine hotplug state. No datasource "
+                        "detected")
+                    sys.exit(1)
+                print('enabled' if datasource else 'disabled')
+            else:
+                handle_hotplug(
+                    hotplug_init=hotplug_init,
+                    devpath=args.devpath,
+                    subsystem=args.subsystem,
+                    udevaction=args.udevaction,
+                )
         except Exception:
             LOG.exception('Received fatal exception handling hotplug!')
             raise
diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py
index bf6bf139..cc7e1c3c 100644
--- a/cloudinit/sources/__init__.py
+++ b/cloudinit/sources/__init__.py
@@ -679,6 +679,16 @@ class DataSource(CloudInitPickleMixin, metaclass=abc.ABCMeta):
     def get_package_mirror_info(self):
         return self.distro.get_package_mirror_info(data_source=self)
 
+    def get_supported_events(self, source_event_types: List[EventType]):
+        supported_events = {}  # type: Dict[EventScope, set]
+        for event in source_event_types:
+            for update_scope, update_events in self.supported_update_events.items():  # noqa: E501
+                if event in update_events:
+                    if not supported_events.get(update_scope):
+                        supported_events[update_scope] = set()
+                    supported_events[update_scope].add(event)
+        return supported_events
+
     def update_metadata_if_supported(
         self, source_event_types: List[EventType]
     ) -> bool:
@@ -694,13 +704,7 @@ class DataSource(CloudInitPickleMixin, metaclass=abc.ABCMeta):
         @return True if the datasource did successfully update cached metadata
             due to source_event_type.
         """
-        supported_events = {}  # type: Dict[EventScope, set]
-        for event in source_event_types:
-            for update_scope, update_events in self.supported_update_events.items():  # noqa: E501
-                if event in update_events:
-                    if not supported_events.get(update_scope):
-                        supported_events[update_scope] = set()
-                    supported_events[update_scope].add(event)
+        supported_events = self.get_supported_events(source_event_types)
         for scope, matched_events in supported_events.items():
             LOG.debug(
                 "Update datasource metadata and %s config due to events: %s",
diff --git a/tests/integration_tests/modules/test_hotplug.py b/tests/integration_tests/modules/test_hotplug.py
index b683566f..a42d1c8c 100644
--- a/tests/integration_tests/modules/test_hotplug.py
+++ b/tests/integration_tests/modules/test_hotplug.py
@@ -48,7 +48,7 @@ def test_hotplug_add_remove(client: IntegrationInstance):
 
     # Add new NIC
     added_ip = client.instance.add_network_interface()
-    _wait_till_hotplug_complete(client)
+    _wait_till_hotplug_complete(client, expected_runs=2)
     ips_after_add = _get_ip_addr(client)
     new_addition = [ip for ip in ips_after_add if ip.ip4 == added_ip][0]
 
@@ -63,7 +63,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=2)
+    _wait_till_hotplug_complete(client, expected_runs=4)
     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]
@@ -72,6 +72,10 @@ def test_hotplug_add_remove(client: IntegrationInstance):
     config = yaml.safe_load(netplan_cfg)
     assert new_addition.interface not in config['network']['ethernets']
 
+    assert 'enabled' == client.execute(
+        'cloud-init devel hotplug-hook -s net query'
+    )
+
 
 @pytest.mark.openstack
 def test_no_hotplug_in_userdata(client: IntegrationInstance):
@@ -83,7 +87,7 @@ def test_no_hotplug_in_userdata(client: IntegrationInstance):
     client.instance.add_network_interface()
     _wait_till_hotplug_complete(client)
     log = client.read_from_file('/var/log/cloud-init.log')
-    assert 'hotplug not enabled for event of type network' in log
+    assert "Event Denied: scopes=['network'] EventType=hotplug" in log
 
     ips_after_add = _get_ip_addr(client)
     if len(ips_after_add) == len(ips_before) + 1:
@@ -92,3 +96,7 @@ def test_no_hotplug_in_userdata(client: IntegrationInstance):
         assert new_ip.state == 'DOWN'
     else:
         assert len(ips_after_add) == len(ips_before)
+
+    assert 'disabled' == client.execute(
+        'cloud-init devel hotplug-hook -s net query'
+    )
diff --git a/tools/hook-hotplug b/tools/hook-hotplug
index 34e95929..ced268b3 100755
--- a/tools/hook-hotplug
+++ b/tools/hook-hotplug
@@ -8,12 +8,17 @@ is_finished() {
     [ -e /run/cloud-init/result.json ]
 }
 
-if is_finished; then
+hotplug_enabled() {
+    [ "$(cloud-init devel hotplug-hook -s "${SUBSYSTEM}" query)" == "enabled" ]
+}
+
+if is_finished && hotplug_enabled; then
     # open cloud-init's hotplug-hook fifo rw
     exec 3<>/run/cloud-init/hook-hotplug-cmd
     env_params=(
-        --devpath="${DEVPATH}"
         --subsystem="${SUBSYSTEM}"
+        handle
+        --devpath="${DEVPATH}"
         --udevaction="${ACTION}"
     )
     # write params to cloud-init's hotplug-hook fifo