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. _______________________________________________ Mailing list: https://launchpad.net/~cloud-init-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
