Skip to content

Fix set-name/interface DNS bug#1058

Merged
TheRealFalcon merged 2 commits into
canonical:mainfrom
akutz:bugfix/set-name-dns
Oct 8, 2021
Merged

Fix set-name/interface DNS bug#1058
TheRealFalcon merged 2 commits into
canonical:mainfrom
akutz:bugfix/set-name-dns

Conversation

@akutz
Copy link
Copy Markdown
Contributor

@akutz akutz commented Oct 8, 2021

Proposed Commit Message

Fix set-name/interface DNS bug

This patch addresses an issue caused when the v2 network config
directive "set-name" was used in conjunction with interface-
specific DNS settings. The patch adds a test to validate the fix.

LP: #1946493

Additional Context

Please see kubernetes-sigs/image-builder#712 for more information.

Test Steps

Here is a patch that shows the update to a Cloud-Init test to validate the issue:

diff --git a/cloudinit/net/tests/test_network_state.py b/cloudinit/net/tests/test_network_state.py
index 84e8308a..c0aa78a0 100644
--- a/cloudinit/net/tests/test_network_state.py
+++ b/cloudinit/net/tests/test_network_state.py
@@ -52,6 +52,7 @@ network:
     eth1:
       match:
         macaddress: '66:77:88:99:00:11'
+      set-name: "eth2"
       nameservers:
         search: [foo.local, bar.local]
         addresses: [4.4.4.4]

And here is the test:

$ make clean_pyc && \
  PYTHONPATH="$(pwd)" \
  python3 -m pytest -v cloudinit/net/tests/test_network_state.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 10 items                                                                                                                                              

cloudinit/net/tests/test_network_state.py::TestNetworkStateParseConfig::test_empty_v1_config_gets_network_state PASSED                                    [ 10%]
cloudinit/net/tests/test_network_state.py::TestNetworkStateParseConfig::test_empty_v2_config_gets_network_state PASSED                                    [ 20%]
cloudinit/net/tests/test_network_state.py::TestNetworkStateParseConfig::test_missing_version_returns_none PASSED                                          [ 30%]
cloudinit/net/tests/test_network_state.py::TestNetworkStateParseConfig::test_unknown_versions_returns_none PASSED                                         [ 40%]
cloudinit/net/tests/test_network_state.py::TestNetworkStateParseConfig::test_valid_config_gets_network_state PASSED                                       [ 50%]
cloudinit/net/tests/test_network_state.py::TestNetworkStateParseConfig::test_version_2_passes_self_as_config PASSED                                       [ 60%]
cloudinit/net/tests/test_network_state.py::TestNetworkStateParseConfigV2::test_version_2_ignores_renderer_key PASSED                                      [ 70%]
cloudinit/net/tests/test_network_state.py::TestNetworkStateParseNameservers::test_v1_nameservers_valid PASSED                                             [ 80%]
cloudinit/net/tests/test_network_state.py::TestNetworkStateParseNameservers::test_v1_nameservers_invalid PASSED                                           [ 90%]
cloudinit/net/tests/test_network_state.py::TestNetworkStateParseNameservers::test_v2_nameservers FAILED                                                   [100%]

=========================================================================== FAILURES ============================================================================
_____________________________________________________ TestNetworkStateParseNameservers.test_v2_nameservers ______________________________________________________

self = <cloudinit.net.tests.test_network_state.TestNetworkStateParseNameservers object at 0x10a7db9d0>

    def test_v2_nameservers(self):
>       config = self._parse_network_state_from_config(V2_CONFIG_NAMESERVERS)

cloudinit/net/tests/test_network_state.py:139: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
cloudinit/net/tests/test_network_state.py:114: in _parse_network_state_from_config
    return network_state.parse_net_config_data(yaml['network'])
cloudinit/net/network_state.py:1074: in parse_net_config_data
    nsi.parse_config(skip_broken=skip_broken)
cloudinit/net/network_state.py:261: in parse_config
    self.parse_config_v2(skip_broken=skip_broken)
cloudinit/net/network_state.py:310: in parse_config_v2
    self._v2_common(command)
cloudinit/net/network_state.py:722: in _v2_common
    self._handle_individual_nameserver(name_cmd, iface)
cloudinit/net/network_state.py:91: in decorator
    return func(self, command, *args, **kwargs)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <cloudinit.net.network_state.NetworkStateInterpreter object at 0x10a7e4f10>
command = {'address': ['4.4.4.4'], 'search': ['foo.local', 'bar.local'], 'type': 'nameserver'}, iface = 'eth1'

    @ensure_command_keys(['address'])
    def _handle_individual_nameserver(self, command, iface):
        _iface = self._network_state.get('interfaces')
        nameservers, search = self._parse_dns(command)
>       _iface[iface]['dns'] = {'nameservers': nameservers, 'search': search}
E       KeyError: 'eth1'

cloudinit/net/network_state.py:546: KeyError
----------------------------------------------------------------------- Captured log call -----------------------------------------------------------------------
2021-10-08 00:57:41 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-10-08 00:57:41 DEBUG     cloudinit.net.network_state:network_state.py:670 v2(ethernets) -> v1(physical):
{'type': 'physical', 'name': 'eth2', 'mac_address': '66:77:88:99:00:11', 'match': {'macaddress': '66:77:88:99:00:11'}}
2021-10-08 00:57:41 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': 'eth2', '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_network_state.py::TestNetworkStateParseNameservers::test_v2_nameservers - KeyError: 'eth1'
============================================================ 1 failed, 9 passed, 3 warnings in 0.60s ============================================================

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

This patch addresses an issue caused when the v2 network config
directive "set-name" was used in conjunction with interface-
specific DNS settings. The patch adds a test to validate the fix.

For more information please see bug 1946493 as well as the issue
kubernetes-sigs/image-builder#712.
Copy link
Copy Markdown
Contributor

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @akutz for the investigation and the fix!

@TheRealFalcon TheRealFalcon merged commit ca0da04 into canonical:main Oct 8, 2021
@akutz akutz deleted the bugfix/set-name-dns branch October 8, 2021 18:40
@zawachte
Copy link
Copy Markdown

I think there might be a bug in a similar vein at ...
https://github.com/canonical/cloud-init/blob/main/cloudinit/net/networkd.py#L245

I applied this patch to a local setup and I got past this error but now I am hitting a KeyError here. Seems related to set-name as we are checking for a key name with what has been inputted into the "set-name" and that doesn't exist.

@TheRealFalcon
Copy link
Copy Markdown
Contributor

@zawachte-msft , Can you create a bug at https://bugs.launchpad.net/cloud-init ? Including the full traceback and the network config used to trigger the error will allow us to reproduce and fix the issue.

@zawachte
Copy link
Copy Markdown

zawachte commented Nov 1, 2021

@pradipd just posted the bug: https://bugs.launchpad.net/cloud-init/+bug/1949407.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants