gce: activate network discovery on every boot (#2128)

Google wants to allow users to make changes on nics while the instance
is stopped. Activate network discovery on every boot.

Additionally, skip the call to `netplan generate` if the rendered
config is the same on subsequent boots.

9 files changed, 203 insertions, 12 deletions

diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py
index 1c28e16e..8b52a641 100644
--- a/cloudinit/net/netplan.py
+++ b/cloudinit/net/netplan.py
@@ -1,6 +1,7 @@
 # This file is part of cloud-init.  See LICENSE file ...
 
 import copy
+import io
 import ipaddress
 import os
 import textwrap
@@ -277,31 +278,48 @@ class Renderer(renderer.Renderer):
         fpnplan = os.path.join(subp.target_path(target), self.netplan_path)
 
         util.ensure_dir(os.path.dirname(fpnplan))
-        header = self.netplan_header if self.netplan_header else ""
 
         # render from state
         content = self._render_content(network_state)
 
+        # normalize header
+        header = self.netplan_header if self.netplan_header else ""
         if not header.endswith("\n"):
             header += "\n"
+        content = header + content
 
-        mode = 0o600 if features.NETPLAN_CONFIG_ROOT_READ_ONLY else 0o644
+        # determine if existing config files have the same content
+        same_content = False
         if os.path.exists(fpnplan):
+            hashed_content = util.hash_buffer(io.BytesIO(content.encode()))
+            with open(fpnplan, "rb") as f:
+                hashed_original_content = util.hash_buffer(f)
+            if hashed_content == hashed_original_content:
+                same_content = True
+
+        mode = 0o600 if features.NETPLAN_CONFIG_ROOT_READ_ONLY else 0o644
+        if not same_content and os.path.exists(fpnplan):
             current_mode = util.get_permissions(fpnplan)
             if current_mode & mode == current_mode:
                 # preserve mode if existing perms are more strict than default
                 mode = current_mode
-        util.write_file(fpnplan, header + content, mode=mode)
+        util.write_file(fpnplan, content, mode=mode)
 
         if self.clean_default:
             _clean_default(target=target)
-        self._netplan_generate(run=self._postcmds)
+        self._netplan_generate(run=self._postcmds, same_content=same_content)
         self._net_setup_link(run=self._postcmds)
 
-    def _netplan_generate(self, run=False):
+    def _netplan_generate(self, run: bool = False, same_content: bool = False):
         if not run:
             LOG.debug("netplan generate postcmd disabled")
             return
+        if same_content:
+            LOG.debug(
+                "skipping call to `netplan generate`."
+                " reason: identical netplan config"
+            )
+            return
         subp.subp(self.NETPLAN_GENERATE, capture=True)
 
     def _net_setup_link(self, run=False):
diff --git a/cloudinit/net/renderer.py b/cloudinit/net/renderer.py
index 72813e32..c429d068 100644
--- a/cloudinit/net/renderer.py
+++ b/cloudinit/net/renderer.py
@@ -24,7 +24,7 @@ def filter_by_attr(match_name):
 filter_by_physical = filter_by_type("physical")
 
 
-class Renderer:
+class Renderer(abc.ABC):
     def __init__(self, config=None):
         pass
 
diff --git a/cloudinit/sources/DataSourceGCE.py b/cloudinit/sources/DataSourceGCE.py
index 041c8914..27d6089a 100644
--- a/cloudinit/sources/DataSourceGCE.py
+++ b/cloudinit/sources/DataSourceGCE.py
@@ -11,6 +11,7 @@ from cloudinit import dmi
 from cloudinit import log as logging
 from cloudinit import sources, url_helper, util
 from cloudinit.distros import ug_util
+from cloudinit.event import EventScope, EventType
 from cloudinit.net.ephemeral import EphemeralDHCPv4
 from cloudinit.sources import DataSourceHostname
 
@@ -63,6 +64,12 @@ class DataSourceGCE(sources.DataSource):
 
     dsname = "GCE"
     perform_dhcp_setup = False
+    default_update_events = {
+        EventScope.NETWORK: {
+            EventType.BOOT_NEW_INSTANCE,
+            EventType.BOOT,
+        }
+    }
 
     def __init__(self, sys_cfg, distro, paths):
         sources.DataSource.__init__(self, sys_cfg, distro, paths)
diff --git a/cloudinit/util.py b/cloudinit/util.py
index fc777b82..2eb79d33 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -1725,7 +1725,7 @@ def logexc(log, msg, *args):
     log.debug(msg, exc_info=exc_info, *args)
 
 
-def hash_blob(blob, routine, mlen=None):
+def hash_blob(blob, routine: str, mlen=None) -> str:
     hasher = hashlib.new(routine)
     hasher.update(encode_text(blob))
     digest = hasher.hexdigest()
@@ -1736,6 +1736,18 @@ def hash_blob(blob, routine, mlen=None):
         return digest
 
 
+def hash_buffer(f: io.BufferedIOBase) -> bytes:
+    """Hash the content of a binary buffer using SHA1.
+
+    @param f: buffered binary stream to hash.
+    @return: digested data as bytes.
+    """
+    hasher = hashlib.sha1()
+    for chunk in iter(lambda: f.read(io.DEFAULT_BUFFER_SIZE), b""):
+        hasher.update(chunk)
+    return hasher.digest()
+
+
 def is_user(name):
     try:
         if pwd.getpwnam(name):
diff --git a/tests/integration_tests/modules/test_user_events.py b/tests/integration_tests/modules/test_user_events.py
index 79d88022..810f8727 100644
--- a/tests/integration_tests/modules/test_user_events.py
+++ b/tests/integration_tests/modules/test_user_events.py
@@ -28,8 +28,7 @@ def _add_dummy_bridge_to_netplan(client: IntegrationInstance):
 
 
 @pytest.mark.skipif(
-    PLATFORM
-    not in ["lxd_container", "lxd_vm", "ec2", "gce", "oci", "openstack"],
+    PLATFORM not in ["lxd_container", "lxd_vm", "ec2", "oci", "openstack"],
     reason="Default boot events testing is datasource specific",
 )
 def test_boot_event_disabled_by_default(client: IntegrationInstance):
@@ -93,7 +92,7 @@ def _test_network_config_applied_on_reboot(client: IntegrationInstance):
 
 
 @pytest.mark.skipif(
-    PLATFORM != "azure",
+    PLATFORM not in ("azure", "gce"),
     reason=(
         f"{PLATFORM} doesn't support updates every boot event by default "
         "(or hasn't been testing for it)."
diff --git a/tests/integration_tests/test_networking.py b/tests/integration_tests/test_networking.py
new file mode 100644
index 00000000..cde69afc
--- /dev/null
+++ b/tests/integration_tests/test_networking.py
@@ -0,0 +1,93 @@
+"""Networking-related tests."""
+import pytest
+import yaml
+
+from tests.integration_tests.instances import IntegrationInstance
+from tests.integration_tests.integration_settings import PLATFORM
+
+
+def _add_dummy_bridge_to_netplan(client: IntegrationInstance):
+    # Update netplan configuration to ensure it doesn't change on reboot
+    netplan = yaml.safe_load(
+        client.execute("cat /etc/netplan/50-cloud-init.yaml")
+    )
+    # Just a dummy bridge to do nothing
+    try:
+        netplan["network"]["bridges"]["dummy0"] = {"dhcp4": False}
+    except KeyError:
+        netplan["network"]["bridges"] = {"dummy0": {"dhcp4": False}}
+
+    dumped_netplan = yaml.dump(netplan)
+    client.write_to_file("/etc/netplan/50-cloud-init.yaml", dumped_netplan)
+
+
+USER_DATA = """\
+#cloud-config
+updates:
+  network:
+    when: [boot]
+"""
+
+
+@pytest.mark.skipif(
+    PLATFORM not in ("lxd_container", "lxd_vm"),
+    reason=(
+        f"{PLATFORM} could make nic changes in a reboot event invalidating"
+        f" these tests."
+    ),
+)
+@pytest.mark.user_data(USER_DATA)
+class TestNetplanGenerateBehaviorOnReboot:
+    def test_skip(self, client: IntegrationInstance):
+        log = client.read_from_file("/var/log/cloud-init.log")
+        assert "Applying network configuration" in log
+        assert "Selected renderer 'netplan'" in log
+        client.execute(
+            "mv /var/log/cloud-init.log /var/log/cloud-init.log.bak"
+        )
+        netplan = yaml.safe_load(
+            client.execute("cat /etc/netplan/50-cloud-init.yaml")
+        )
+
+        client.restart()
+
+        log = client.read_from_file("/var/log/cloud-init.log")
+        assert "Event Allowed: scope=network EventType=boot" in log
+        assert "Applying network configuration" in log
+        assert "Running command ['netplan', 'generate']" not in log
+        assert (
+            "skipping call to `netplan generate`."
+            " reason: identical netplan config"
+        ) in log
+        netplan_new = yaml.safe_load(
+            client.execute("cat /etc/netplan/50-cloud-init.yaml")
+        )
+        assert netplan == netplan_new, "no changes expected in netplan config"
+
+    def test_applied(self, client: IntegrationInstance):
+        log = client.read_from_file("/var/log/cloud-init.log")
+        assert "Applying network configuration" in log
+        assert "Selected renderer 'netplan'" in log
+        client.execute(
+            "mv /var/log/cloud-init.log /var/log/cloud-init.log.bak"
+        )
+        # fake a change in the rendered network config file
+        _add_dummy_bridge_to_netplan(client)
+        netplan = yaml.safe_load(
+            client.execute("cat /etc/netplan/50-cloud-init.yaml")
+        )
+
+        client.restart()
+
+        log = client.read_from_file("/var/log/cloud-init.log")
+        assert "Event Allowed: scope=network EventType=boot" in log
+        assert "Applying network configuration" in log
+        assert (
+            "skipping call to `netplan generate`."
+            " reason: identical netplan config"
+        ) not in log
+        assert "Running command ['netplan', 'generate']" in log
+        netplan_new = yaml.safe_load(
+            client.execute("cat /etc/netplan/50-cloud-init.yaml")
+        )
+        assert netplan != netplan_new, "changes expected in netplan config"
diff --git a/tests/unittests/net/test_netplan.py b/tests/unittests/net/test_netplan.py
new file mode 100644
index 00000000..28b0891d
--- /dev/null
+++ b/tests/unittests/net/test_netplan.py
@@ -0,0 +1,41 @@
+import os
+from unittest import mock
+
+import pytest
+
+from cloudinit import util
+from cloudinit.net import netplan
+
+
+@pytest.fixture
+def renderer(tmp_path):
+    config = {
+        "netplan_path": str(tmp_path / "netplan/50-cloud-init.yaml"),
+        "postcmds": True,
+    }
+    yield netplan.Renderer(config)
+
+
+class TestNetplanRenderer:
+    @pytest.mark.parametrize("write_config", [True, False])
+    def test_skip_netplan_generate(self, renderer, write_config, mocker):
+        """Check `netplan generate` is called if netplan config has changed."""
+        header = "\n"
+        content = "foo"
+        renderer_mocks = mocker.patch.multiple(
+            renderer,
+            _render_content=mocker.Mock(return_value=content),
+            _netplan_generate=mocker.DEFAULT,
+            _net_setup_link=mocker.DEFAULT,
+        )
+        if write_config:
+            util.ensure_dir(os.path.dirname(renderer.netplan_path))
+            with open(renderer.netplan_path, "w") as f:
+                f.write(header)
+                f.write(content)
+
+        renderer.render_network_state(mocker.Mock())
+
+        assert renderer_mocks["_netplan_generate"].call_args_list == [
+            mock.call(run=True, same_content=write_config)
+        ]
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index d7640d70..a8b18ee2 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -6470,7 +6470,7 @@ class TestNetplanPostcommands(CiTestCase):
     @mock.patch.object(netplan.Renderer, "_net_setup_link")
     @mock.patch("cloudinit.subp.subp")
     def test_netplan_render_calls_postcmds(
-        self, mock_subp, mock_netplan_generate, mock_net_setup_link
+        self, mock_subp, mock_net_setup_link, mock_netplan_generate
     ):
         tmp_dir = self.tmp_dir()
         ns = network_state.parse_net_config_data(self.mycfg, skip_broken=False)
@@ -6485,7 +6485,7 @@ class TestNetplanPostcommands(CiTestCase):
         mock_subp.side_effect = iter([subp.ProcessExecutionError])
         renderer.render_network_state(ns, target=render_dir)
 
-        mock_netplan_generate.assert_called_with(run=True)
+        mock_netplan_generate.assert_called_with(run=True, same_content=False)
         mock_net_setup_link.assert_called_with(run=True)
 
     @mock.patch("cloudinit.util.SeLinuxGuard")
diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py
index e5243ef3..c23f6399 100644
--- a/tests/unittests/test_util.py
+++ b/tests/unittests/test_util.py
@@ -3043,3 +3043,24 @@ class TestResolvable:
         assert util.is_resolvable("http://169.254.169.254/") is True
         assert util.is_resolvable("http://[fd00:ec2::254]/") is True
         assert not m_getaddr.called
+
+
+class TestHashBuffer:
+    def test_in_memory(self):
+        buf = io.BytesIO(b"hola")
+        assert (
+            util.hash_buffer(buf)
+            == b"\x99\x80\x0b\x85\xd38>:/\xb4^\xb7\xd0\x06jHy\xa9\xda\xd0"
+        )
+
+    def test_file(self, tmp_path):
+        content = b"hola"
+        file = tmp_path / "file.txt"
+        with file.open("wb") as f:
+            f.write(content)
+
+        with file.open("rb") as f:
+            assert (
+                util.hash_buffer(f)
+                == b"\x99\x80\x0b\x85\xd38>:/\xb4^\xb7\xd0\x06jHy\xa9\xda\xd0"
+            )