← Back to team overview

cloud-init-dev team mailing list archive

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