From 97d48ada162bb01792d94ed68a541aa44db8e726 Mon Sep 17 00:00:00 2001 From: Adrian Vladu Date: Fri, 21 Jun 2024 14:30:14 +0300 Subject: [PATCH] nocloud: fix network config regression With commit https://github.com/cloudbase/cloudbase-init/commit/f0fe66b30bf95d05ebecb74d8059ea2cdda37d0f, the network config in case of NoCloudConfigDrive was supported only if it was in the format: ```yaml network: config: ... version: 1|2 ``` Before the commit, the following format was also supported: ```yaml config: ... version: 1 ``` Added support for both formats and made sure that the `network` key is not required. Change-Id: I7478a8654443db83f0f3995839811910c468a06c Signed-off-by: Adrian Vladu --- .../metadata/services/nocloudservice.py | 29 ++- .../metadata/services/test_nocloudservice.py | 231 ++++++++++-------- 2 files changed, 143 insertions(+), 117 deletions(-) diff --git a/cloudbaseinit/metadata/services/nocloudservice.py b/cloudbaseinit/metadata/services/nocloudservice.py index f3b6b921..987a6161 100644 --- a/cloudbaseinit/metadata/services/nocloudservice.py +++ b/cloudbaseinit/metadata/services/nocloudservice.py @@ -237,10 +237,11 @@ def parse(self, network_config): networks = [] services = [] - network_config = network_config.get('network') \ - if network_config else {} - network_config = network_config.get('config') \ - if network_config else None + if network_config and network_config.get('network'): + network_config = network_config.get('network') + if network_config: + network_config = network_config.get('config') + if not network_config: LOG.warning("Network configuration is empty") return @@ -479,8 +480,9 @@ def parse(self, network_config): networks = [] services = [] - network_config = network_config.get('network') \ - if network_config else {} + if network_config and network_config.get('network'): + network_config = network_config.get('network') + if not network_config: LOG.warning("Network configuration is empty") return @@ -529,12 +531,21 @@ class NoCloudNetworkConfigParser(object): @staticmethod def parse(network_data): - network_data_version = network_data.get("network", {}).get("version") + # we can have a network key in some cases + if network_data.get("network"): + network_data = network_data.get("network") + network_data_version = network_data.get("version") + if network_data_version == 1: network_config_parser = NoCloudNetworkConfigV1Parser() - return network_config_parser.parse(network_data) + elif network_data_version == 2: + network_config_parser = NoCloudNetworkConfigV2Parser() + else: + raise exception.CloudbaseInitException( + "Unsupported network_data_version: '%s'" + % network_data_version) - return NoCloudNetworkConfigV2Parser().parse(network_data) + return network_config_parser.parse(network_data) class NoCloudConfigDriveService(baseconfigdrive.BaseConfigDriveService): diff --git a/cloudbaseinit/tests/metadata/services/test_nocloudservice.py b/cloudbaseinit/tests/metadata/services/test_nocloudservice.py index c07713f0..fab3fe2d 100644 --- a/cloudbaseinit/tests/metadata/services/test_nocloudservice.py +++ b/cloudbaseinit/tests/metadata/services/test_nocloudservice.py @@ -15,6 +15,7 @@ import ddt import importlib import os +import textwrap import unittest import unittest.mock as mock @@ -48,55 +49,57 @@ config: - type: router """ -NOCLOUD_NETWORK_CONFIG_TEST_DATA_V1 = """ -network: - version: 1 - config: - - type: physical - name: interface0 - mac_address: "52:54:00:12:34:00" - mtu: 1450 - subnets: - - type: static - address: 192.168.1.10 - netmask: 255.255.255.0 - gateway: 192.168.1.1 - dns_nameservers: - - 192.168.1.11 - - type: bond - name: bond0 - bond_interfaces: - - gbe0 - - gbe1 - mac_address: "52:54:00:12:34:00" - params: - bond-mode: active-backup - bond-lacp-rate: false - mtu: 1450 - subnets: - - type: static - address: 192.168.1.10 - netmask: 255.255.255.0 - dns_nameservers: - - 192.168.1.11 - - type: vlan - name: vlan0 - vlan_link: eth1 - vlan_id: 150 - mac_address: "52:54:00:12:34:00" - mtu: 1450 - subnets: - - type: static - address: 192.168.1.10 - netmask: 255.255.255.0 - dns_nameservers: - - 192.168.1.11 - - type: nameserver - address: - - 192.168.23.2 - - 8.8.8.8 - search: acme.local +NOCLOUD_NETWORK_CONFIG_TEST_DATA_V1_LEGACY = """ +version: 1 +config: +- type: physical + name: interface0 + mac_address: "52:54:00:12:34:00" + mtu: 1450 + subnets: + - type: static + address: 192.168.1.10 + netmask: 255.255.255.0 + gateway: 192.168.1.1 + dns_nameservers: + - 192.168.1.11 +- type: bond + name: bond0 + bond_interfaces: + - gbe0 + - gbe1 + mac_address: "52:54:00:12:34:00" + params: + bond-mode: active-backup + bond-lacp-rate: false + mtu: 1450 + subnets: + - type: static + address: 192.168.1.10 + netmask: 255.255.255.0 + dns_nameservers: + - 192.168.1.11 +- type: vlan + name: vlan0 + vlan_link: eth1 + vlan_id: 150 + mac_address: "52:54:00:12:34:00" + mtu: 1450 + subnets: + - type: static + address: 192.168.1.10 + netmask: 255.255.255.0 + dns_nameservers: + - 192.168.1.11 +- type: nameserver + address: + - 192.168.23.2 + - 8.8.8.8 + search: acme.local """ +NOCLOUD_NETWORK_CONFIG_TEST_DATA_V1 = """ +network:%s +""" % (textwrap.indent(NOCLOUD_NETWORK_CONFIG_TEST_DATA_V1_LEGACY, " ")) NOCLOUD_NETWORK_CONFIG_TEST_DATA_V2_EMPTY_CONFIG = """ """ NOCLOUD_NETWORK_CONFIG_TEST_DATA_V2_CONFIG_IS_NOT_DICT = """ @@ -116,67 +119,69 @@ eth0: - test """ -NOCLOUD_NETWORK_CONFIG_TEST_DATA_V2 = """ -network: - version: 2 - ethernets: - interface0: - match: - macaddress: "52:54:00:12:34:00" - set-name: "eth0" +NOCLOUD_NETWORK_CONFIG_TEST_DATA_V2_LEGACY = """ +version: 2 +ethernets: + interface0: + match: + macaddress: "52:54:00:12:34:00" + set-name: "eth0" + addresses: + - 192.168.1.10/24 + gateway4: 192.168.1.1 + nameservers: addresses: - - 192.168.1.10/24 - gateway4: 192.168.1.1 - nameservers: - addresses: - - 192.168.1.11 - - 192.168.1.12 - search: - - acme.local - mtu: 1450 - interface1: - set-name: "interface1" + - 192.168.1.11 + - 192.168.1.12 + search: + - acme.local + mtu: 1450 + interface1: + set-name: "interface1" + addresses: + - 192.168.1.100/24 + gateway4: 192.168.1.1 + nameservers: addresses: - - 192.168.1.100/24 - gateway4: 192.168.1.1 - nameservers: - addresses: - - 192.168.1.11 - - 192.168.1.12 - search: - - acme.local - bonds: - bond0: - interfaces: ["gbe0", "gbe1"] - match: - macaddress: "52:54:00:12:34:00" - parameters: - mode: active-backup - lacp-rate: false + - 192.168.1.11 + - 192.168.1.12 + search: + - acme.local +bonds: + bond0: + interfaces: ["gbe0", "gbe1"] + match: + macaddress: "52:54:00:12:34:00" + parameters: + mode: active-backup + lacp-rate: false + addresses: + - 192.168.1.10/24 + nameservers: addresses: - - 192.168.1.10/24 - nameservers: - addresses: - - 192.168.1.11 - mtu: 1450 - vlans: - vlan0: - id: 150 - link: eth1 - dhcp4: yes - match: - macaddress: "52:54:00:12:34:00" + - 192.168.1.11 + mtu: 1450 +vlans: + vlan0: + id: 150 + link: eth1 + dhcp4: yes + match: + macaddress: "52:54:00:12:34:00" + addresses: + - 192.168.1.10/24 + nameservers: addresses: - - 192.168.1.10/24 - nameservers: - addresses: - - 192.168.1.11 - mtu: 1450 - bridges: - br0: - interfaces: ['eth0'] - dhcp4: true + - 192.168.1.11 + mtu: 1450 +bridges: + br0: + interfaces: ['eth0'] + dhcp4: true """ +NOCLOUD_NETWORK_CONFIG_TEST_DATA_V2 = """ +network:%s +""" % (textwrap.indent(NOCLOUD_NETWORK_CONFIG_TEST_DATA_V2_LEGACY, " ")) @ddt.ddt @@ -207,7 +212,12 @@ def test_parse_empty_result(self, input, expected_result): self.assertEqual(True, expected_result[0] in self.snatcher.output[0]) self.assertEqual(result, expected_result[1]) - def test_network_details_v2(self): + @ddt.data( + (NOCLOUD_NETWORK_CONFIG_TEST_DATA_V1, True), + (NOCLOUD_NETWORK_CONFIG_TEST_DATA_V1_LEGACY, True) + ) + @ddt.unpack + def test_network_details_v2(self, test_data, expected_result): expected_bond = nm.Bond( members=["gbe0", "gbe1"], type=nm.BOND_TYPE_ACTIVE_BACKUP, @@ -275,7 +285,7 @@ def test_network_details_v2(self): search='acme.local') result = self._parser.parse( - serialization.parse_json_yaml(NOCLOUD_NETWORK_CONFIG_TEST_DATA_V1)) + serialization.parse_json_yaml(test_data)) self.assertEqual(result.links[0], expected_link) self.assertEqual(result.networks[0], expected_network) @@ -318,7 +328,12 @@ def test_parse_empty_result(self, input, expected_result): self.assertEqual(True, expected_result[0] in self.snatcher.output[0]) self.assertEqual(result, expected_result[1]) - def test_network_details_v2(self): + @ddt.data( + (NOCLOUD_NETWORK_CONFIG_TEST_DATA_V2, True), + (NOCLOUD_NETWORK_CONFIG_TEST_DATA_V2_LEGACY, True) + ) + @ddt.unpack + def test_network_details_v2(self, test_data, expected_result): expected_bond = nm.Bond( members=["gbe0", "gbe1"], type=nm.BOND_TYPE_ACTIVE_BACKUP, @@ -406,7 +421,7 @@ def test_network_details_v2(self): search='acme.local') result = self._parser.parse( - serialization.parse_json_yaml(NOCLOUD_NETWORK_CONFIG_TEST_DATA_V2)) + serialization.parse_json_yaml(test_data)) self.assertEqual(result.links[0], expected_link) self.assertEqual(result.links[1], expected_link_if1)