Fix for set-name bug in networkd renderer

[akutz]

Proposed Commit Message

Fix for set-name bug in networkd renderer

This patch address an issue where the use of the "set-name"
directive caused the networkd renderer to fail.

LP: #1949407

Additional Context

Please note this was root caused by @pradipd at pradipd@756742a, and this PR is merely an optimization on their existing fix as well as the addition of a unit test to validate the fix.

Fixes Kubernetes image builder bug described in this comment - kubernetes-sigs/image-builder#712 (comment)

CC @MaxRink @zawachte-msft @kkeshavamurthy @randomvariable @codenrhoden

Test Steps

The following command and output demonstrates the patch works as intended:

$ make clean_pyc && PYTHONPATH="$(pwd)" python3 -m pytest -v --log-level=DEBUG cloudinit/net/tests/test_networkd.py
====================================================================== test session starts ======================================================================
platform darwin -- Python 3.9.7, pytest-6.2.4, py-1.10.0, pluggy-0.13.1 -- /usr/local/opt/python@3.9/bin/python3.9
cachedir: .pytest_cache
rootdir: /Users/akutz/Projects/cloud-init, configfile: tox.ini
collected 1 item                                                                                                                                                

cloudinit/net/tests/test_networkd.py::TestNetworkdRenderState::test_networkd_render_with_set_name PASSED                                                  [100%]

======================================================================= warnings summary ========================================================================
../../../../usr/local/lib/python3.9/site-packages/_pytest/config/__init__.py:1183
  /usr/local/lib/python3.9/site-packages/_pytest/config/__init__.py:1183: PytestDeprecationWarning: The --strict option is deprecated, use --strict-markers instead.
    self.issue_config_time_warning(

conftest.py:68
  /Users/akutz/Projects/cloud-init/conftest.py:68: PytestDeprecationWarning: @pytest.yield_fixture is deprecated.
  Use @pytest.fixture instead; they are the same.
    @pytest.yield_fixture(autouse=True)

conftest.py:169
  /Users/akutz/Projects/cloud-init/conftest.py:169: PytestDeprecationWarning: @pytest.yield_fixture is deprecated.
  Use @pytest.fixture instead; they are the same.
    def httpretty():

-- Docs: https://docs.pytest.org/en/stable/warnings.html
================================================================= 1 passed, 3 warnings in 0.05s =================================================================

However, if the patch to the file is removed and the test is executed again, the original error occurs:

diff --git a/cloudinit/net/networkd.py b/cloudinit/net/networkd.py
index 62ee3bcc..c97aaa68 100644
--- a/cloudinit/net/networkd.py
+++ b/cloudinit/net/networkd.py
@@ -243,19 +243,6 @@ class Renderer(renderer.Renderer):
                 # network state doesn't give dhcp domain info
                 # using ns.config as a workaround here
 
-                # Check to see if this interface matches against an interface
-                # from the network state that specified a set-name directive.
-                # If there is a device with a set-name directive and it has
-                # set-name value that matches the current name, then update the
-                # current name to the device's name. That will be the valeu in
-                # the ns.config['ethernets'] dict below.
-                for dev_name, dev_cfg in ns.config['ethernets'].items():
-                    if 'set-name' in dev_cfg:
-                        set_name_iface = dev_cfg.get('set-name')
-                        if set_name_iface:
-                            if set_name_iface == name:
-                                name = dev_name
-                                break
                 self.dhcp_domain(ns.config['ethernets'][name], cfg)
 
             ret_dict.update({link: cfg.get_final_conf()})

$ make clean_pyc && PYTHONPATH="$(pwd)" python3 -m pytest -v --log-level=DEBUG cloudinit/net/tests/test_networkd.py
====================================================================== test session starts ======================================================================
platform darwin -- Python 3.9.7, pytest-6.2.4, py-1.10.0, pluggy-0.13.1 -- /usr/local/opt/python@3.9/bin/python3.9
cachedir: .pytest_cache
rootdir: /Users/akutz/Projects/cloud-init, configfile: tox.ini
collected 1 item                                                                                                                                                

cloudinit/net/tests/test_networkd.py::TestNetworkdRenderState::test_networkd_render_with_set_name FAILED                                                  [100%]

=========================================================================== FAILURES ============================================================================
__________________________________________________ TestNetworkdRenderState.test_networkd_render_with_set_name ___________________________________________________

self = <cloudinit.net.tests.test_networkd.TestNetworkdRenderState object at 0x1023966d0>

    def test_networkd_render_with_set_name(self):
        ns = self._parse_network_state_from_config(V2_CONFIG_NAMESERVERS)
        renderer = networkd.Renderer()
>       rendered_content = renderer._render_content(ns)

cloudinit/net/tests/test_networkd.py:39: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <cloudinit.net.networkd.Renderer object at 0x10230dd30>, ns = <cloudinit.net.network_state.NetworkState object at 0x102396b50>

    def _render_content(self, ns):
        ret_dict = {}
        for iface in ns.iter_interfaces():
            cfg = CfgParser()
    
            link = self.generate_match_section(iface, cfg)
            self.generate_link_section(iface, cfg)
            self.parse_subnets(iface, cfg)
            self.parse_dns(iface, cfg, ns)
    
            for route in ns.iter_routes():
                self.parse_routes(route, cfg)
    
            if ns.version == 2:
                name = iface['name']
                # network state doesn't give dhcp domain info
                # using ns.config as a workaround here
    
>               self.dhcp_domain(ns.config['ethernets'][name], cfg)
E               KeyError: 'ens92'

cloudinit/net/networkd.py:246: KeyError
----------------------------------------------------------------------- Captured log call -----------------------------------------------------------------------
2021-11-04 18:23:55 DEBUG     cloudinit.net.network_state:network_state.py:670 v2(ethernets) -> v1(physical):
{'type': 'physical', 'name': 'eth0', 'mac_address': '00:11:22:33:44:55', 'match': {'macaddress': '00:11:22:33:44:55'}}
2021-11-04 18:23:55 DEBUG     cloudinit.net.network_state:network_state.py:670 v2(ethernets) -> v1(physical):
{'type': 'physical', 'name': 'ens92', 'mac_address': '66:77:88:99:00:11', 'match': {'macaddress': '66:77:88:99:00:11'}}
2021-11-04 18:23:55 DEBUG     cloudinit.net.network_state:network_state.py:711 v2_common: handling config:
{'eth0': {'match': {'macaddress': '00:11:22:33:44:55'}, 'nameservers': {'search': ['spam.local', 'eggs.local'], 'addresses': ['8.8.8.8']}}, 'eth1': {'match': {'macaddress': '66:77:88:99:00:11'}, 'set-name': 'ens92', 'nameservers': {'search': ['foo.local', 'bar.local'], 'addresses': ['4.4.4.4']}}}
======================================================================= warnings summary ========================================================================
../../../../usr/local/lib/python3.9/site-packages/_pytest/config/__init__.py:1183
  /usr/local/lib/python3.9/site-packages/_pytest/config/__init__.py:1183: PytestDeprecationWarning: The --strict option is deprecated, use --strict-markers instead.
    self.issue_config_time_warning(

conftest.py:68
  /Users/akutz/Projects/cloud-init/conftest.py:68: PytestDeprecationWarning: @pytest.yield_fixture is deprecated.
  Use @pytest.fixture instead; they are the same.
    @pytest.yield_fixture(autouse=True)

conftest.py:169
  /Users/akutz/Projects/cloud-init/conftest.py:169: PytestDeprecationWarning: @pytest.yield_fixture is deprecated.
  Use @pytest.fixture instead; they are the same.
    def httpretty():

-- Docs: https://docs.pytest.org/en/stable/warnings.html
==================================================================== short test summary info ====================================================================
FAILED cloudinit/net/tests/test_networkd.py::TestNetworkdRenderState::test_networkd_render_with_set_name - KeyError: 'ens92'
================================================================= 1 failed, 3 warnings in 0.18s =================================================================

Checklist:

• My code follows the process laid out in the documentation
• I have updated or added any unit tests accordingly
• I have updated or added any documentation accordingly

[pradipd]

Thank you!

[akutz]

Thank you!

Hi @pradipd,

I hope you do not think me rude to steal your patch and turn it into a PR. I credited you up above, and hopefully they can just set you as the author on the commit. I also knew we needed some unit testing around this. Since I'd fixed the previous set-name issue, I figured I could whip up a test quickly. Again, thank you for RCAing this!

[pradipd]

Not at all. I definitely appreciate you cleaning it up, adding unit tests, and pushing it through.

[holmanb]

Thanks for the PR. I left a couple of nitpick style comments. Generally this looks good.

[akutz]

Hi @holmanb,

Thanks for the review. I've addressed your feedback and pushed the updates. Thanks!

[randomvariable]

Thanks as ever for the speedy turnaround.

[blackboxsw]

I think minimally the unit test needs to be a bit tighter, but I think we can "fix" the rendering in networkd, but not sure I have all the context here.

[akutz]

I think minimally the unit test needs to be a bit tighter, but I think we can "fix" the rendering in networkd, but not sure I have all the context here.

Thanks @blackboxsw, I will see if I can fix it.

[blackboxsw]

LGTM! thanks for the work on this @akutz