feat(cc_install_hotplug): trigger hook on known ec2 drivers (canonica…

…l#4799)

Add extra logic to only trigger hook-hotplug on NICs with known drivers
on EC2. This aviods the hook being triggered on any add/remove event on
net devices, causing uneeded CPU usage, as on instance that start and
stop a lot of docker containers, see [1].

Rename 10-cloud-init-hook-hotplug.rules to
99-cloud-init-hook-hotplug.rules as ID_NET_DRIVER was not yet defined
at matching-time.

References:

[1] https://bugs.launchpad.net/cloud-init/+bug/1946003

cloudinit/config/cc_install_hotplug.py

@@ -21,7 +21,7 @@
         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``.
+        ``/etc/udev/rules.d/99-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,
@@ -59,10 +59,10 @@
 LOG = logging.getLogger(__name__)
 
 
-HOTPLUG_UDEV_PATH = "/etc/udev/rules.d/10-cloud-init-hook-hotplug.rules"
+HOTPLUG_UDEV_PATH = "/etc/udev/rules.d/99-cloud-init-hook-hotplug.rules"
 HOTPLUG_UDEV_RULES_TEMPLATE = """\
 # Installed by cloud-init due to network hotplug userdata
-ACTION!="add|remove", GOTO="cloudinit_end"
+ACTION!="add|remove", GOTO="cloudinit_end"{extra_rules}
 LABEL="cloudinit_hook"
 SUBSYSTEM=="net", RUN+="{libexecdir}/hook-hotplug"
 LABEL="cloudinit_end"
@@ -104,12 +104,24 @@ def handle(name: str, cfg: Config, cloud: Cloud, args: list) -> None:
         LOG.debug("Skipping hotplug install, udevadm not found")
         return
 
+    extra_rules = ""
+    if cloud.datasource.dsname == "Ec2":
+        # Only trigger hook-hotplug on NICs with Ec2 drivers. Avoid triggering
+        # it on docker virtual NICs and the like. LP: #1946003
+        extra_rules = """
+ENV{ID_NET_DRIVER}=="vif|ena|ixgbevf", GOTO="cloudinit_hook"
+GOTO="cloudinit_end"
+"""
+    if extra_rules:
+        extra_rules = "\n" + extra_rules
     # This may need to turn into a distro property at some point
     libexecdir = "/usr/libexec/cloud-init"
     if not os.path.exists(libexecdir):
         libexecdir = "/usr/lib/cloud-init"
     util.write_file(
         filename=HOTPLUG_UDEV_PATH,
-        content=HOTPLUG_UDEV_RULES_TEMPLATE.format(libexecdir=libexecdir),
+        content=HOTPLUG_UDEV_RULES_TEMPLATE.format(
+            extra_rules=extra_rules, libexecdir=libexecdir
+        ),
     )
     subp.subp(["udevadm", "control", "--reload-rules"])

systemd/cloud-init-hotplugd.service

@@ -1,6 +1,6 @@
 # Paired with cloud-init-hotplugd.socket to read from the FIFO
 # /run/cloud-init/hook-hotplug-cmd which is created during a udev network
-# add or remove event as processed by 10-cloud-init-hook-hotplug.rules.
+# add or remove event as processed by 99-cloud-init-hook-hotplug.rules.
 
 # On start, read args from the FIFO, process and provide structured arguments
 # to `cloud-init devel hotplug-hook` which will setup or teardown network

systemd/cloud-init-hotplugd.socket

@@ -1,6 +1,6 @@
 # cloud-init-hotplugd.socket listens on the FIFO file
 # /run/cloud-init/hook-hotplug-cmd which is created during a udev network
-# add or remove event as processed by 10-cloud-init-hook-hotplug.rules.
+# add or remove event as processed by 99-cloud-init-hook-hotplug.rules.
 
 # Known bug with an enforcing SELinux policy: LP: #1936229
 [Unit]

tests/integration_tests/modules/test_hotplug.py

@@ -63,7 +63,7 @@ def test_hotplug_add_remove(client: IntegrationInstance):
     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"
+        "test -f /etc/udev/rules.d/99-cloud-init-hook-hotplug.rules"
     ).ok
 
     # Add new NIC
@@ -109,7 +109,7 @@ def test_no_hotplug_in_userdata(client: IntegrationInstance):
     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"
+        "test -f /etc/udev/rules.d/99-cloud-init-hook-hotplug.rules"
     ).failed
 
     # Add new NIC
@@ -213,3 +213,27 @@ def test_multi_nic_hotplug(setup_image, session_cloud: IntegrationCloud):
                 )
             with contextlib.suppress(Exception):
                 ec2.release_address(AllocationId=allocation["AllocationId"])
+
+
+@pytest.mark.skipif(PLATFORM != "ec2", reason="test is ec2 specific")
+@pytest.mark.user_data(USER_DATA)
+def test_no_hotplug_triggered_by_docker(client: IntegrationInstance):
+    # Install docker
+    r = client.execute("curl -fsSL https://get.docker.com | sh")
+    assert r.ok, r.stderr
+
+    # Start and stop a container
+    r = client.execute("docker run -dit --name ff ubuntu:focal")
+    assert r.ok, r.stderr
+    r = client.execute("docker stop ff")
+    assert r.ok, r.stderr
+
+    # Verify hotplug-hook was not called
+    log = client.read_from_file("/var/log/cloud-init.log")
+    assert "Exiting hotplug handler" not in log
+    assert "hotplug-hook" not in log
+
+    # Verify hotplug was enabled
+    assert "enabled" == client.execute(
+        "cloud-init devel hotplug-hook -s net query"
+    )

tests/unittests/config/test_cc_install_hotplug.py

@@ -10,6 +10,7 @@
     handle,
 )
 from cloudinit.event import EventScope, EventType
+from cloudinit.sources.DataSourceEc2 import DataSourceEc2
 
 
 @pytest.fixture()
@@ -61,7 +62,7 @@ def test_rules_installed_when_supported_and_enabled(
             mocks.m_write.assert_called_once_with(
                 filename=HOTPLUG_UDEV_PATH,
                 content=HOTPLUG_UDEV_RULES_TEMPLATE.format(
-                    libexecdir=libexecdir
+                    extra_rules="", libexecdir=libexecdir
                 ),
             )
         assert mocks.m_subp.call_args_list == [
@@ -127,3 +128,41 @@ def test_rules_not_installed_when_no_udevadm(self, mocks):
         assert mocks.m_del.call_args_list == []
         assert mocks.m_write.call_args_list == []
         assert mocks.m_subp.call_args_list == []
+
+    def test_rules_installed_on_ec2(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}
+        }
+        m_cloud.datasource.dsname = DataSourceEc2.dsname
+
+        with mock.patch("os.path.exists", return_value=True):
+            handle(None, {}, m_cloud, None)
+
+        udev_rules = """\
+# Installed by cloud-init due to network hotplug userdata
+ACTION!="add|remove", GOTO="cloudinit_end"
+
+ENV{ID_NET_DRIVER}=="vif|ena|ixgbevf", GOTO="cloudinit_hook"
+GOTO="cloudinit_end"
+
+LABEL="cloudinit_hook"
+SUBSYSTEM=="net", RUN+="/usr/libexec/cloud-init/hook-hotplug"
+LABEL="cloudinit_end"
+"""
+        mocks.m_write.assert_called_once_with(
+            filename=HOTPLUG_UDEV_PATH,
+            content=udev_rules,
+        )
+        assert mocks.m_subp.call_args_list == [
+            mock.call(
+                [
+                    "udevadm",
+                    "control",
+                    "--reload-rules",
+                ]
+            )
+        ]
+        assert mocks.m_del.call_args_list == []