Repository: canonical/operator
Pull request #418 (2020-09-30 20:52:50)

Allow for missing fields in network-get response
There are some cases where network-get may return less information than is currently expected. This mainly seems to happen in K8s charms, since not all of the info may be available yet. This makes the model more tolerant to missing information.

chipaca: Thank you for this! I think we need to split your libyaml extension test fix to a separate PR. Could you do that easily? Otherwise I will.
johnsca: Yep, will do.  I included it because it was blocking my testing but it definitely should be a separate PR.
johnsca: TBH, I didn't notice that, but it seems like it would be better for the tests to handle it gracefully.  Split that change off to #420

Patch:
diff --git a/ops/model.py b/ops/model.py
index 55addf5..3502d2b 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -535,15 +535,17 @@ class Network:
         self.interfaces = []
         # Treat multiple addresses on an interface as multiple logical
         # interfaces with the same name.
-        for interface_info in network_info['bind-addresses']:
-            interface_name = interface_info['interface-name']
-            for address_info in interface_info['addresses']:
+        for interface_info in network_info.get('bind-addresses', []):
+            interface_name = interface_info.get('interface-name')
+            if not interface_name:
+                continue
+            for address_info in interface_info.get('addresses', []):
                 self.interfaces.append(NetworkInterface(interface_name, address_info))
         self.ingress_addresses = []
-        for address in network_info['ingress-addresses']:
+        for address in network_info.get('ingress-addresses', []):
             self.ingress_addresses.append(ipaddress.ip_address(address))
         self.egress_subnets = []
-        for subnet in network_info['egress-subnets']:
+        for subnet in network_info.get('egress-subnets', []):
             self.egress_subnets.append(ipaddress.ip_network(subnet))
 
     @property
@@ -554,7 +556,10 @@ class Network:
         address from :attr:`.interfaces` that can be used to configure where your
         application should bind() and listen().
         """
-        return self.interfaces[0].address
+        if self.interfaces:
+            return self.interfaces[0].address
+        else:
+            return None
 
     @property
     def ingress_address(self):
@@ -564,7 +569,10 @@ class Network:
         to is not always the address other people can use to connect() to you.
         This is just the first address from :attr:`.ingress_addresses`.
         """
-        return self.ingress_addresses[0]
+        if self.ingress_addresses:
+            return self.ingress_addresses[0]
+        else:
+            return None
 
 
 class NetworkInterface:
@@ -582,14 +590,21 @@ class NetworkInterface:
     def __init__(self, name: str, address_info: dict):
         self.name = name
         # TODO: expose a hardware address here, see LP: #1864070.
-        self.address = ipaddress.ip_address(address_info['value'])
-        cidr = address_info['cidr']
-        if not cidr:
-            # The cidr field may be empty, see LP: #1864102.
-            # In this case, make it a /32 or /128 IP network.
-            self.subnet = ipaddress.ip_network(address_info['value'])
+        address = address_info.get('value')
+        # The value field may be empty.
+        if address:
+            self.address = ipaddress.ip_address(address)
         else:
+            self.address = None
+        cidr = address_info.get('cidr')
+        # The cidr field may be empty, see LP: #1864102.
+        if cidr:
             self.subnet = ipaddress.ip_network(cidr)
+        elif address:
+            # If we have an address, convert it to a /32 or /128 IP network.
+            self.subnet = ipaddress.ip_network(address)
+        else:
+            self.subnet = None
         # TODO: expose a hostname/canonical name for the address here, see LP: #1864086.
 
 


Test patch:
diff --git a/test/test_model.py b/test/test_model.py
index 35c3010..6f8f7fe 100755
--- a/test/test_model.py
+++ b/test/test_model.py
@@ -874,6 +874,60 @@ class TestModelBindings(unittest.TestCase):
         self._check_binding_data(binding_name, binding)
         self.assertEqual(fake_script_calls(self, clear=True), expected_calls)
 
+    def test_missing_bind_addresses(self):
+        network_data = json.dumps({})
+        fake_script(self, 'network-get',
+                    '''[ "$1" = db0 ] && echo '{}' || exit 1'''.format(network_data))
+        binding_name = 'db0'
+        binding = self.model.get_binding(self.model.get_relation(binding_name))
+        self.assertEqual(binding.network.interfaces, [])
+
+    def test_empty_bind_addresses(self):
+        network_data = json.dumps({'bind-addresses': [{}]})
+        fake_script(self, 'network-get',
+                    '''[ "$1" = db0 ] && echo '{}' || exit 1'''.format(network_data))
+        binding_name = 'db0'
+        binding = self.model.get_binding(self.model.get_relation(binding_name))
+        self.assertEqual(binding.network.interfaces, [])
+
+    def test_empty_interface_info(self):
+        network_data = json.dumps({
+            'bind-addresses': [{
+                'interface-name': 'eth0',
+                'addresses': [{}],
+            }],
+        })
+        fake_script(self, 'network-get',
+                    '''[ "$1" = db0 ] && echo '{}' || exit 1'''.format(network_data))
+        binding_name = 'db0'
+        binding = self.model.get_binding(self.model.get_relation(binding_name))
+        self.assertEqual(len(binding.network.interfaces), 1)
+        interface = binding.network.interfaces[0]
+        self.assertIsNone(interface.address)
+        self.assertIsNone(interface.subnet)
+
+    def test_missing_ingress_addresses(self):
+        network_data = json.dumps({
+            'bind-addresses': [],
+        })
+        fake_script(self, 'network-get',
+                    '''[ "$1" = db0 ] && echo '{}' || exit 1'''.format(network_data))
+        binding_name = 'db0'
+        binding = self.model.get_binding(self.model.get_relation(binding_name))
+        self.assertEqual(binding.network.ingress_addresses, [])
+        self.assertEqual(binding.network.ingress_address, None)
+
+    def test_missing_egress_subnets(self):
+        network_data = json.dumps({
+            'bind-addresses': [],
+            'ingress-addresses': [],
+        })
+        fake_script(self, 'network-get',
+                    '''[ "$1" = db0 ] && echo '{}' || exit 1'''.format(network_data))
+        binding_name = 'db0'
+        binding = self.model.get_binding(self.model.get_relation(binding_name))
+        self.assertEqual(binding.network.egress_subnets, [])
+
 
 class TestModelBackend(unittest.TestCase):