cloud-init-dev team mailing list archive
-
cloud-init-dev team
-
Mailing list archive
-
Message #05232
Re: [Merge] ~sw37th/cloud-init:opennebula_fix_null_gateway6 into cloud-init:master
Akihiko,
This looks great, thank you for the extended test coverage.
I have a few little things inline.
https://jenkins.ubuntu.com/server/job/cloud-init-ci/179/
is running against your build here and will post its results.
Diff comments:
> diff --git a/tests/unittests/test_datasource/test_opennebula.py b/tests/unittests/test_datasource/test_opennebula.py
> index ab42f34..fc0065c 100644
> --- a/tests/unittests/test_datasource/test_opennebula.py
> +++ b/tests/unittests/test_datasource/test_opennebula.py
> @@ -354,6 +354,332 @@ class TestOpenNebulaNetwork(unittest.TestCase):
>
> system_nics = ('eth0', 'ens3')
>
> + def test_context_devname(self):
for all the tests added, can you please add a docstring?
def test_context_devname(self):
""""Verify context_devname correctly returns mac and name."""
The docstrings aren't terribly important to get perfect, but generally we're working on adding them when we add new tests.
> + context = {
> + 'ETH0_MAC': '02:00:0a:12:01:01',
> + 'ETH1_MAC': '02:00:0a:12:0f:0f', }
> + expected = {
> + '02:00:0a:12:01:01': 'ETH0',
> + '02:00:0a:12:0f:0f': 'ETH1', }
> + net = ds.OpenNebulaNetwork(context)
> + self.assertEqual(net.context_devname, expected)
> +
> + def test_get_nameservers(self):
> + context = {
> + 'DNS': '1.2.3.8',
> + 'ETH0_DNS': '1.2.3.6 1.2.3.7',
> + 'ETH0_SEARCH_DOMAIN': 'example.com example.org', }
> + expected = {
> + 'addresses': ['1.2.3.6', '1.2.3.7', '1.2.3.8'],
> + 'search': ['example.com', 'example.org']}
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_nameservers('eth0')
> + self.assertEqual(val, expected)
self.assertEqual(expected, val)
https://stackoverflow.com/questions/2404978/why-are-assertequals-parameters-in-the-order-expected-actual
As the first response there suggests, the order is somewhat arbitrary.
It doesn't surprise me if cloud-init isn't perfectly consistent, but lets at least add new tests with the "right" order.
> +
> + def test_get_mtu(self):
> + context = {'ETH0_MTU': '1280'}
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_mtu('eth0')
> + self.assertEqual(val, '1280')
> +
> + def test_get_ip(self):
> + context = {'ETH0_IP': PUBLIC_IP}
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_ip('eth0', MACADDR)
> + self.assertEqual(val, PUBLIC_IP)
> +
> + # empty ETH0_IP
seems like you might as well just split this into a separate test.
> + # expect to return IP address by MAC address
> + context = {'ETH0_IP': ''}
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_ip('eth0', MACADDR)
> + self.assertEqual(val, IP_BY_MACADDR)
> +
> + def test_get_ip6(self):
> + context = {
> + 'ETH0_IP6': IP6_GLOBAL,
> + 'ETH0_IP6_ULA': IP6_ULA, }
> + expected = [IP6_GLOBAL, IP6_ULA]
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_ip6('eth0')
> + self.assertEqual(val, expected)
> +
> + context = {'ETH0_IP6': IP6_GLOBAL}
> + expected = [IP6_GLOBAL]
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_ip6('eth0')
> + self.assertEqual(val, expected)
> +
> + context = {'ETH0_IP6': IP6_ULA}
> + expected = [IP6_ULA]
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_ip6('eth0')
> + self.assertEqual(val, expected)
> +
> + def test_get_ip6_prefix(self):
> + context = {'ETH0_IP6_PREFIX_LENGTH': IP6_PREFIX}
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_ip6_prefix('eth0')
> + self.assertEqual(val, IP6_PREFIX)
> +
> + # empty ETH0_IP6_PREFIX_LENGTH
> + # expect to return default value '64'
> + context = {'ETH0_IP6_PREFIX_LENGTH': ''}
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_ip6_prefix('eth0')
> + self.assertEqual(val, '64')
> +
> + def test_get_gateway(self):
> + context = {'ETH0_GATEWAY': '1.2.3.5'}
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_gateway('eth0')
> + self.assertEqual(val, '1.2.3.5')
> +
> + def test_get_gateway6(self):
> + context = {'ETH0_GATEWAY6': IP6_GW}
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_gateway6('eth0')
> + self.assertEqual(val, IP6_GW)
> +
> + def test_get_mask(self):
> + context = {'ETH0_MASK': '255.255.0.0'}
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_mask('eth0')
> + self.assertEqual(val, '255.255.0.0')
> +
> + # empty ETH0_MASK
> + # expect to return default value '255.255.255.0'
> + context = {'ETH0_MASK': ''}
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_mask('eth0')
> + self.assertEqual(val, '255.255.255.0')
> +
> + def test_get_network(self):
> + context = {'ETH0_NETWORK': '1.2.3.0'}
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_network('eth0', MACADDR)
> + self.assertEqual(val, '1.2.3.0')
> +
> + # empty ETH0_NETWORK
> + # expect to return default value by MAC address
> + context = {'ETH0_NETWORK': ''}
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_network('eth0', MACADDR)
> + self.assertEqual(val, '10.18.1.0')
> +
> + def test_get_field(self):
> + context = {'ETH9_DUMMY': 'DUMMY_VALUE'}
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_field('eth9', 'dummy')
> + self.assertEqual(val, 'DUMMY_VALUE')
> +
> + context = {'ETH9_DUMMY': ''}
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_field('eth9', 'dummy', 'DEFAULT_VALUE')
> + self.assertEqual(val, 'DEFAULT_VALUE')
> +
> + context = {'ETH9_DUMMY': 'DUMMY_VALUE'}
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_field('eth9', 'dummy', 'DEFAULT_VALUE')
> + self.assertEqual(val, 'DUMMY_VALUE')
> +
> + context = {'ETH9_DUMMY': None}
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_field('eth9', 'dummy')
> + self.assertIsNone(val)
> +
> + context = {'ETH9_DUMMY': ''}
> + net = ds.OpenNebulaNetwork(context)
> + val = net.get_field('eth9', 'dummy')
> + self.assertIsNone(val)
> +
> + @mock.patch(DS_PATH + ".get_physical_nics_by_mac")
> + def test_gen_conf_gateway(self, m_get_phys_by_mac):
> + """Test rendering with/without IPv4 gateway"""
> + self.maxDiff = None
> + # empty ETH0_GATEWAY
> + context = {
> + 'ETH0_MAC': '02:00:0a:12:01:01',
> + 'ETH0_GATEWAY': '', }
> + for nic in self.system_nics:
> + expected = {
> + 'version': 2,
> + 'ethernets': {
> + nic: {
> + 'match': {'macaddress': MACADDR},
> + 'addresses': [IP_BY_MACADDR + '/' + IP4_PREFIX]}}}
> + m_get_phys_by_mac.return_value = {MACADDR: nic}
> + net = ds.OpenNebulaNetwork(context)
> + self.assertEqual(net.gen_conf(), expected)
> +
> + # set ETH0_GATEWAY
> + context = {
> + 'ETH0_MAC': '02:00:0a:12:01:01',
> + 'ETH0_GATEWAY': '1.2.3.5', }
> + for nic in self.system_nics:
> + expected = {
> + 'version': 2,
> + 'ethernets': {
> + nic: {
> + 'gateway4': '1.2.3.5',
> + 'match': {'macaddress': MACADDR},
> + 'addresses': [IP_BY_MACADDR + '/' + IP4_PREFIX]}}}
> + m_get_phys_by_mac.return_value = {MACADDR: nic}
> + net = ds.OpenNebulaNetwork(context)
> + self.assertEqual(net.gen_conf(), expected)
> +
> + @mock.patch(DS_PATH + ".get_physical_nics_by_mac")
> + def test_gen_conf_gateway6(self, m_get_phys_by_mac):
> + """Test rendering with/without IPv6 gateway"""
> + self.maxDiff = None
> + # empty ETH0_GATEWAY6
> + context = {
> + 'ETH0_MAC': '02:00:0a:12:01:01',
> + 'ETH0_GATEWAY6': '', }
> + for nic in self.system_nics:
> + expected = {
> + 'version': 2,
> + 'ethernets': {
> + nic: {
> + 'match': {'macaddress': MACADDR},
> + 'addresses': [IP_BY_MACADDR + '/' + IP4_PREFIX]}}}
> + m_get_phys_by_mac.return_value = {MACADDR: nic}
> + net = ds.OpenNebulaNetwork(context)
> + self.assertEqual(net.gen_conf(), expected)
> +
> + # set ETH0_GATEWAY6
> + context = {
> + 'ETH0_MAC': '02:00:0a:12:01:01',
> + 'ETH0_GATEWAY6': IP6_GW, }
> + for nic in self.system_nics:
> + expected = {
> + 'version': 2,
> + 'ethernets': {
> + nic: {
> + 'gateway6': IP6_GW,
> + 'match': {'macaddress': MACADDR},
> + 'addresses': [IP_BY_MACADDR + '/' + IP4_PREFIX]}}}
> + m_get_phys_by_mac.return_value = {MACADDR: nic}
> + net = ds.OpenNebulaNetwork(context)
> + self.assertEqual(net.gen_conf(), expected)
> +
> + @mock.patch(DS_PATH + ".get_physical_nics_by_mac")
> + def test_gen_conf_ipv6address(self, m_get_phys_by_mac):
> + """Test rendering with/without IPv6 address"""
> + self.maxDiff = None
> + # empty ETH0_IP6, ETH0_IP6_ULA, ETH0_IP6_PREFIX_LENGTH
> + context = {
> + 'ETH0_MAC': '02:00:0a:12:01:01',
> + 'ETH0_IP6': '',
> + 'ETH0_IP6_ULA': '',
> + 'ETH0_IP6_PREFIX_LENGTH': '', }
> + for nic in self.system_nics:
> + expected = {
> + 'version': 2,
> + 'ethernets': {
> + nic: {
> + 'match': {'macaddress': MACADDR},
> + 'addresses': [IP_BY_MACADDR + '/' + IP4_PREFIX]}}}
> + m_get_phys_by_mac.return_value = {MACADDR: nic}
> + net = ds.OpenNebulaNetwork(context)
> + self.assertEqual(net.gen_conf(), expected)
> +
> + # set ETH0_IP6, ETH0_IP6_ULA, ETH0_IP6_PREFIX_LENGTH
> + context = {
> + 'ETH0_MAC': '02:00:0a:12:01:01',
> + 'ETH0_IP6': IP6_GLOBAL,
> + 'ETH0_IP6_PREFIX_LENGTH': IP6_PREFIX,
> + 'ETH0_IP6_ULA': IP6_ULA, }
> + for nic in self.system_nics:
> + expected = {
> + 'version': 2,
> + 'ethernets': {
> + nic: {
> + 'match': {'macaddress': MACADDR},
> + 'addresses': [
> + IP_BY_MACADDR + '/' + IP4_PREFIX,
> + IP6_GLOBAL + '/' + IP6_PREFIX,
> + IP6_ULA + '/' + IP6_PREFIX]}}}
> + m_get_phys_by_mac.return_value = {MACADDR: nic}
> + net = ds.OpenNebulaNetwork(context)
> + self.assertEqual(net.gen_conf(), expected)
> +
> + @mock.patch(DS_PATH + ".get_physical_nics_by_mac")
> + def test_gen_conf_dns(self, m_get_phys_by_mac):
> + """Test rendering with/without DNS server, search domain"""
> + self.maxDiff = None
> + # empty DNS, ETH0_DNS, ETH0_SEARCH_DOMAIN
> + context = {
> + 'ETH0_MAC': '02:00:0a:12:01:01',
> + 'DNS': '',
> + 'ETH0_DNS': '',
> + 'ETH0_SEARCH_DOMAIN': '', }
> + for nic in self.system_nics:
> + expected = {
> + 'version': 2,
> + 'ethernets': {
> + nic: {
> + 'match': {'macaddress': MACADDR},
> + 'addresses': [IP_BY_MACADDR + '/' + IP4_PREFIX]}}}
> + m_get_phys_by_mac.return_value = {MACADDR: nic}
> + net = ds.OpenNebulaNetwork(context)
> + self.assertEqual(net.gen_conf(), expected)
> +
> + # set DNS, ETH0_DNS, ETH0_SEARCH_DOMAIN
> + context = {
> + 'ETH0_MAC': '02:00:0a:12:01:01',
> + 'DNS': '1.2.3.8',
> + 'ETH0_DNS': '1.2.3.6 1.2.3.7',
> + 'ETH0_SEARCH_DOMAIN': 'example.com example.org', }
> + for nic in self.system_nics:
> + expected = {
> + 'version': 2,
> + 'ethernets': {
> + nic: {
> + 'nameservers': {
> + 'addresses': ['1.2.3.6', '1.2.3.7', '1.2.3.8'],
> + 'search': ['example.com', 'example.org']},
> + 'match': {'macaddress': MACADDR},
> + 'addresses': [IP_BY_MACADDR + '/' + IP4_PREFIX]}}}
> + m_get_phys_by_mac.return_value = {MACADDR: nic}
> + net = ds.OpenNebulaNetwork(context)
> + self.assertEqual(net.gen_conf(), expected)
> +
> + @mock.patch(DS_PATH + ".get_physical_nics_by_mac")
> + def test_gen_conf_mtu(self, m_get_phys_by_mac):
> + """Test rendering with/without MTU"""
> + self.maxDiff = None
> + # empty ETH0_MTU
> + context = {
> + 'ETH0_MAC': '02:00:0a:12:01:01',
> + 'ETH0_MTU': '', }
> + for nic in self.system_nics:
> + expected = {
> + 'version': 2,
> + 'ethernets': {
> + nic: {
> + 'match': {'macaddress': MACADDR},
> + 'addresses': [IP_BY_MACADDR + '/' + IP4_PREFIX]}}}
> + m_get_phys_by_mac.return_value = {MACADDR: nic}
> + net = ds.OpenNebulaNetwork(context)
> + self.assertEqual(net.gen_conf(), expected)
> +
> + # set ETH0_MTU
> + context = {
> + 'ETH0_MAC': '02:00:0a:12:01:01',
> + 'ETH0_MTU': '1280', }
> + for nic in self.system_nics:
> + expected = {
> + 'version': 2,
> + 'ethernets': {
> + nic: {
> + 'mtu': '1280',
> + 'match': {'macaddress': MACADDR},
> + 'addresses': [IP_BY_MACADDR + '/' + IP4_PREFIX]}}}
> + m_get_phys_by_mac.return_value = {MACADDR: nic}
> + net = ds.OpenNebulaNetwork(context)
> + self.assertEqual(net.gen_conf(), expected)
> +
> @mock.patch(DS_PATH + ".get_physical_nics_by_mac")
> def test_eth0(self, m_get_phys_by_mac):
> for nic in self.system_nics:
--
https://code.launchpad.net/~sw37th/cloud-init/+git/cloud-init/+merge/350428
Your team cloud-init commiters is requested to review the proposed merge of ~sw37th/cloud-init:opennebula_fix_null_gateway6 into cloud-init:master.
References
