Repository: canonical/operator

Issue:
traefik errors on cos layer with error: hook failed: "metrics-endpoint-relation-joined"
### Bug Description

Solutions QA team has two runs in which traefik errors on cos layer with error: hook failed: "metrics-endpoint-relation-joined"

The cos layer is built on top of kubernetes-aws. 

From the logs: 

2022-08-21 02:20:42 ERROR juju-log metrics-endpoint:10: Uncaught exception while in charm code:
Traceback (most recent call last):
  File "./src/charm.py", line 678, in <module>
    main(TraefikIngressCharm)
  File "/var/lib/juju/agents/unit-traefik-0/charm/venv/ops/main.py", line 431, in main
    _emit_charm_event(charm, dispatcher.event_name)
  File "/var/lib/juju/agents/unit-traefik-0/charm/venv/ops/main.py", line 142, in _emit_charm_event
    event_to_emit.emit(*args, **kwargs)
  File "/var/lib/juju/agents/unit-traefik-0/charm/venv/ops/framework.py", line 316, in emit
    framework._emit(event)
  File "/var/lib/juju/agents/unit-traefik-0/charm/venv/ops/framework.py", line 784, in _emit
    self._reemit(event_path)
  File "/var/lib/juju/agents/unit-traefik-0/charm/venv/ops/framework.py", line 857, in _reemit
    custom_handler(event)
  File "/var/lib/juju/agents/unit-traefik-0/charm/lib/charms/prometheus_k8s/v0/prometheus_scrape.py", line 1545, in _set_scrape_job_spec
    self._set_unit_ip(event)
  File "/var/lib/juju/agents/unit-traefik-0/charm/lib/charms/prometheus_k8s/v0/prometheus_scrape.py", line 1576, in _set_unit_ip
    unit_ip = str(self._charm.model.get_binding(relation).network.bind_address)
  File "/var/lib/juju/agents/unit-traefik-0/charm/venv/ops/model.py", line 679, in network
    self._network = self._network_get(self.name, self._relation_id)
  File "/var/lib/juju/agents/unit-traefik-0/charm/venv/ops/model.py", line 672, in _network_get
    return Network(self._backend.network_get(name, relation_id))
  File "/var/lib/juju/agents/unit-traefik-0/charm/venv/ops/model.py", line 724, in __init__
    self.ingress_addresses.append(ipaddress.ip_address(address))
  File "/usr/lib/python3.8/ipaddress.py", line 53, in ip_address
    raise ValueError('%r does not appear to be an IPv4 or IPv6 address' %
ValueError: 'acb396cf5563d429f9ebc8aa23ae47ed-1516332866.us-east-1.elb.amazonaws.com' does not appear to be an IPv4 or IPv6 address

Failed runs: 

https://solutions.qa.canonical.com/testruns/testRun/0b35ee4c-efcf-4f50-a393-696803b31ac9
https://solutions.qa.canonical.com/testruns/testRun/ef230601-4af3-4d6f-89af-4e578985e666

Logs are found on the bottom on the page, on the artifacts repository. 



### To Reproduce

These errors were found on the  fkb-master-kubernetes-focal-aws SKU of our automated test suite.

### Environment

charmed-kubernetes  1.24
stable channel for cos charms. 



### Relevant log output

```shell
Traceback (most recent call last):
  File "./src/charm.py", line 678, in <module>
    main(TraefikIngressCharm)
  File "/var/lib/juju/agents/unit-traefik-0/charm/venv/ops/main.py", line 431, in main
    _emit_charm_event(charm, dispatcher.event_name)
  File "/var/lib/juju/agents/unit-traefik-0/charm/venv/ops/main.py", line 142, in _emit_charm_event
    event_to_emit.emit(*args, **kwargs)
  File "/var/lib/juju/agents/unit-traefik-0/charm/venv/ops/framework.py", line 316, in emit
    framework._emit(event)
  File "/var/lib/juju/agents/unit-traefik-0/charm/venv/ops/framework.py", line 784, in _emit
    self._reemit(event_path)
  File "/var/lib/juju/agents/unit-traefik-0/charm/venv/ops/framework.py", line 857, in _reemit
    custom_handler(event)
  File "/var/lib/juju/agents/unit-traefik-0/charm/lib/charms/prometheus_k8s/v0/prometheus_scrape.py", line 1545, in _set_scrape_job_spec
    self._set_unit_ip(event)
  File "/var/lib/juju/agents/unit-traefik-0/charm/lib/charms/prometheus_k8s/v0/prometheus_scrape.py", line 1576, in _set_unit_ip
    unit_ip = str(self._charm.model.get_binding(relation).network.bind_address)
  File "/var/lib/juju/agents/unit-traefik-0/charm/venv/ops/model.py", line 679, in network
    self._network = self._network_get(self.name, self._relation_id)
  File "/var/lib/juju/agents/unit-traefik-0/charm/venv/ops/model.py", line 672, in _network_get
    return Network(self._backend.network_get(name, relation_id))
  File "/var/lib/juju/agents/unit-traefik-0/charm/venv/ops/model.py", line 724, in __init__
    self.ingress_addresses.append(ipaddress.ip_address(address))
  File "/usr/lib/python3.8/ipaddress.py", line 53, in ip_address
    raise ValueError('%r does not appear to be an IPv4 or IPv6 address' %
ValueError: 'acb396cf5563d429f9ebc8aa23ae47ed-1516332866.us-east-1.elb.amazonaws.com' does not appear to be an IPv4 or IPv6 address
```


### Additional context

_No response_

Discussion:
rwcarlsen: Are we sure we want the union return type?  Or would it possibly make sense to just validate the ip (and log/note if it's not an ip) and then just always return str?  This seems like it would make it easier for user code to handle - only one return type to deal with rather than isinstance branching (or sloppily ignoring the case where it isn't an ip).
PietroPasotti: > Are we sure we want the union return type? Or would it possibly make sense to just validate the ip (and log/note if it's not an ip) and then just always return str? This seems like it would make it easier for user code to handle - only one return type to deal with rather than isinstance branching (or sloppily ignoring the case where it isn't an ip).

I think I'd prefer that approach as well, but I'd be reluctant to just break any existing code that's instance-checking the address or using some IPaddress attribute without checking the type first.

that looks like a separate issue/PR. 
Will work out some tests for this
PietroPasotti: Added a test for non-ip-like ingress-address return values. Also verified that the test fails with the expected error message in main.

Patch:
diff --git a/ops/model.py b/ops/model.py
index 6c51a81..5532a70 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -93,7 +93,7 @@ if typing.TYPE_CHECKING:
     _RelationsMeta_Raw = Dict[str, ops.charm.RelationMeta]
     # mapping from container name to container metadata
     _ContainerMeta_Raw = Dict[str, ops.charm.ContainerMeta]
-    _IPAddress = Union[ipaddress.IPv4Address, ipaddress.IPv6Address]
+    _NetworkAddress = Union[ipaddress.IPv4Address, ipaddress.IPv6Address, str]
     _Network = Union[ipaddress.IPv4Network, ipaddress.IPv6Network]
 
     _ServiceInfoMapping = Mapping[str, pebble.ServiceInfo]
@@ -692,6 +692,17 @@ class Binding:
         return self._network
 
 
+def _cast_network_address(raw: str) -> '_NetworkAddress':
+    # fields marked as network addresses need not be IPs; they could be
+    # hostnames that juju failed to resolve. In that case, we'll log a
+    # debug message and leave it as-is.
+    try:
+        return ipaddress.ip_address(raw)
+    except ValueError:
+        logger.debug("could not cast {} to IPv4/v6 address".format(raw))
+        return raw
+
+
 class Network:
     """Network space details.
 
@@ -725,15 +736,15 @@ class Network:
             if addrs is not None:
                 for address_info in addrs:
                     self.interfaces.append(NetworkInterface(interface_name, address_info))
-        self.ingress_addresses = []  # type: List[_IPAddress]
+        self.ingress_addresses = []  # type: List[_NetworkAddress]
         for address in network_info.get('ingress-addresses', []):
-            self.ingress_addresses.append(ipaddress.ip_address(address))
+            self.ingress_addresses.append(_cast_network_address(address))
         self.egress_subnets = []  # type: List[_Network]
         for subnet in network_info.get('egress-subnets', []):
             self.egress_subnets.append(ipaddress.ip_network(subnet))
 
     @property
-    def bind_address(self) -> Optional['_IPAddress']:
+    def bind_address(self) -> Optional['_NetworkAddress']:
         """A single address that your application should bind() to.
 
         For the common case where there is a single answer. This represents a single
@@ -746,7 +757,7 @@ class Network:
             return None
 
     @property
-    def ingress_address(self):
+    def ingress_address(self) -> Optional['_NetworkAddress']:
         """The address other applications should use to connect to your unit.
 
         Due to things like public/private addresses, NAT and tunneling, the address you bind()
@@ -782,8 +793,8 @@ class NetworkInterface:
             address = address_info.get('address')
 
         # The value field may be empty.
-        address_ = ipaddress.ip_address(address) if address else None
-        self.address = address_  # type: Optional[_IPAddress]
+        address_ = _cast_network_address(address) if address else None
+        self.address = address_  # type: Optional[_NetworkAddress]
         cidr = address_info.get('cidr')  # type: str
         # The cidr field may be empty, see LP: #1864102.
         if cidr:


Test patch:
diff --git a/test/test_model.py b/test/test_model.py
index 495859f..173168f 100755
--- a/test/test_model.py
+++ b/test/test_model.py
@@ -1912,6 +1912,16 @@ class TestModelBindings(unittest.TestCase):
             self.assertEqual(binding.network.interfaces[i].address, ipaddress.ip_address(address))
             self.assertEqual(binding.network.interfaces[i].subnet, ipaddress.ip_network(subnet))
 
+        for (i, (name, address, subnet)) in enumerate([
+                ('lo', '192.0.2.2', '192.0.2.0/24'),
+                ('lo', 'dead:beef::1', 'dead:beef::/64'),
+                ('tun', '192.0.3.3', '192.0.3.3/32'),
+                ('tun', '2001:db8::3', '2001:db8::3/128'),
+                ('tun', 'fe80::1:1', 'fe80::/64')]):
+            self.assertEqual(binding.network.interfaces[i].name, name)
+            self.assertEqual(binding.network.interfaces[i].address, ipaddress.ip_address(address))
+            self.assertEqual(binding.network.interfaces[i].subnet, ipaddress.ip_network(subnet))
+
     def test_invalid_keys(self):
         # Basic validation for passing invalid keys.
         for name in (object, 0):
@@ -2058,6 +2068,20 @@ class TestModelBindings(unittest.TestCase):
         binding = self.model.get_binding(self.model.get_relation(binding_name))
         self.assertEqual(binding.network.egress_subnets, [])
 
+    def test_unresolved_ingress_addresses(self):
+        # sometimes juju fails to resolve an url to an IP, in which case
+        # ingress-addresses will be the 'raw' url instead of an IP.
+        network_data = json.dumps({
+            'ingress-addresses': [
+                'foo.bar.baz.com'
+            ],
+        })
+        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, ['foo.bar.baz.com'])
+
 
 class TestModelBackend(unittest.TestCase):