Merge ~mthaddon/charm-k8s-ingress/+git/charm-k8s-ingress:ingress-relation into charm-k8s-ingress:master

[Tom Haddon]

Implement basic ingress relation, which takes precedence over juju config

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Unable to determine commit message from repository - please click "Set commit message" and enter the commit message manually.

[🤖 prod-jenkaas-is (prod-jenkaas-is)]

A CI job is currently in progress. A follow up comment will be added when it completes.

[🤖 prod-jenkaas-is (prod-jenkaas-is)]

FAILED: Continuous integration, rev:089ed6f6ce7245d4db33b11895638bfb62f7f336

No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want jenkins to rebuild you need to trigger it yourself):
https://code.launchpad.net/~mthaddon/charm-k8s-ingress/+git/charm-k8s-ingress/+merge/400278/+edit-commit-message

https://jenkins.canonical.com/is/job/lp-charm-k8s-ingress-ci/10/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/is/job/lp-charm-test/40/
    None: https://jenkins.canonical.com/is/job/lp-update-mp/51957/

Click here to trigger a rebuild:
https://jenkins.canonical.com/is/job/lp-charm-k8s-ingress-ci/10//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-is (prod-jenkaas-is)]

A CI job is currently in progress. A follow up comment will be added when it completes.

[🤖 prod-jenkaas-is (prod-jenkaas-is)]

PASSED: Continuous integration, rev:f7061af49cb8f81b5495564b1935db6a7c74fa77
https://jenkins.canonical.com/is/job/lp-charm-k8s-ingress-ci/11/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/is/job/lp-charm-test/41/
    None: https://jenkins.canonical.com/is/job/lp-update-mp/51962/

Click here to trigger a rebuild:
https://jenkins.canonical.com/is/job/lp-charm-k8s-ingress-ci/11//rebuild

review: Approve (continuous-integration)

[Stuart Bishop (stub)]

This looks fine.

I think the relation should be using application relation data rather than per-unit relation data, per inline comments. The code should be about the, essentially changing the event.relation.data[event.unit] invocations to event.relation.data[event.application] (you already have the is_leader() guard in place). This can be done on a future branch, if you think it is warranted for what should be a 'temporary' charm.

review: Approve

[🤖 prod-jenkaas-is (prod-jenkaas-is)]

A CI job is currently in progress. A follow up comment will be added when it completes.

[🤖 prod-jenkaas-is (prod-jenkaas-is)]

PASSED: Continuous integration, rev:be8f9b5b6e3a828ae0c4fe706bf85568f6be6938
https://jenkins.canonical.com/is/job/lp-charm-k8s-ingress-ci/12/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/is/job/lp-charm-test/42/
    None: https://jenkins.canonical.com/is/job/lp-update-mp/57052/

Click here to trigger a rebuild:
https://jenkins.canonical.com/is/job/lp-charm-k8s-ingress-ci/12//rebuild

review: Approve (continuous-integration)

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision cce119b69540fb49ecd7f6d3e73ba6fa1dca2dbe

diff --git a/README.md b/README.md
index 4dcbc5c..fe5769b 100644
--- a/README.md
+++ b/README.md
@@ -10,7 +10,10 @@ size, etc.).
 
 ## Usage
 
-TODO: Provide high-level usage, such as required config or relations
+This charm needs to be built locally as charmhub.io can't yet host charms with
+"bases" in `metadata.yaml`. Once that's been added, deployment instructions
+will be updated, but for now, see the "Running the charm locally" section
+below.
 
 ## Running the charm locally
 
@@ -19,7 +22,7 @@ To build the charm, run `charmcraft build`.
 You'll need version 1.14 or later of Go (`go version` will confirm your current version), and a custom version of the Juju 2.9 branch, as below:
 
 ```
-git clone -b demo-pebble https://github.com/benhoyt/juju
+git clone -b 2.9 https://github.com/juju/juju
 cd juju
 make install
 make microk8s-operator-update  # to make the microk8s image and push to Docker
@@ -27,109 +30,43 @@ export PATH="/home/${USER}/go/bin:$PATH"
 juju bootstrap microk8s --no-gui
 juju add-model ingress-test
 ```
-Now from the directory where you've run `charmcraft build` as above:
-```
-juju deploy ./ingress.charm --resource placeholder-image='google/pause'
-```
-This is currently failing with the following error in `juju debug-log`.
-```
-unit-ingress-0: 05:12:02 INFO juju.worker.uniter awaiting error resolution for "config-changed" hook
-unit-ingress-0: 05:12:03 ERROR unit.ingress/0.juju-log Uncaught exception while in charm code:
-Traceback (most recent call last):
-  File "./src/charm.py", line 80, in <module>
-    main(CharmK8SIngressCharm)
-  File "/var/lib/juju/agents/unit-ingress-0/charm/venv/ops/main.py", line 402, in main
-    _emit_charm_event(charm, dispatcher.event_name)
-  File "/var/lib/juju/agents/unit-ingress-0/charm/venv/ops/main.py", line 140, in _emit_charm_event
-    event_to_emit.emit(*args, **kwargs)
-  File "/var/lib/juju/agents/unit-ingress-0/charm/venv/ops/framework.py", line 278, in emit
-    framework._emit(event)
-  File "/var/lib/juju/agents/unit-ingress-0/charm/venv/ops/framework.py", line 722, in _emit
-    self._reemit(event_path)
-  File "/var/lib/juju/agents/unit-ingress-0/charm/venv/ops/framework.py", line 767, in _reemit
-    custom_handler(event)
-  File "./src/charm.py", line 75, in _on_config_changed
-    self._get_pods()
-  File "./src/charm.py", line 63, in _get_pods
-    self.k8s_auth()
-  File "./src/charm.py", line 59, in k8s_auth
-    kubernetes.config.load_incluster_config()
-  File "/var/lib/juju/agents/unit-ingress-0/charm/venv/kubernetes/config/incluster_config.py", line 118, in load_incluster_config
-    InClusterConfigLoader(
-  File "/var/lib/juju/agents/unit-ingress-0/charm/venv/kubernetes/config/incluster_config.py", line 54, in load_and_set
-    self._load_config()
-  File "/var/lib/juju/agents/unit-ingress-0/charm/venv/kubernetes/config/incluster_config.py", line 73, in _load_config
-    raise ConfigException("Service token file does not exists.")
-kubernetes.config.config_exception.ConfigException: Service token file does not exists.
-unit-ingress-0: 05:12:03 ERROR juju.worker.uniter.operation hook "config-changed" (via hook dispatching script: dispatch) failed: exit status 1
-```
-Per https://github.com/kubernetes-client/python/issues/1331 the file it's looking for is
-/var/run/secrets/kubernetes.io/serviceaccount/token, but this file isn't present in the
-'charm' container for the deployed instance. The file does exist in the model operator, however.
-
-This has been filed as https://bugs.launchpad.net/juju/+bug/1920102.
-
-To work around this, we've added a `kube-config` config option. This should be the contents of your
-kubernetes client configuration. If you're using microk8s you can get this via `microk8s config`,
-and you'd then deploy the charm as follows:
+Once https://bugs.launchpad.net/juju/+bug/1920102 has been addressed, this
+charm will be able to use the credentials provided in cluster. However, for
+now, you will need to provide this charm with credentials to be able to talk
+to the K8s API directly. This is done via the `kube-config` config option,
+which should be set to the contents of your kubernetes client configuration.
+If you're using microk8s you can get this via `microk8s config`. As an example
+you could deploy this charm as follows:
 ```
 juju deploy ./ingress.charm --resource placeholder-image='google/pause' --config kube-config="$(microk8s config)"
 ```
-We've also added some initial config options to configure the ingress. As an
-example:
+To create an ingress for your service, you'd then add a relation to a charm
+that supports the `ingress` relation. As an example:
 ```
-juju config ingress service-namespace=ingress-test service-name=gunicorn service-port=80 service-hostname=foo.internal
+juju deploy ./gunicorn.charm --resource gunicorn-image='gunicorncharmers/gunicorn-app:edge'
+juju relate ingress gunicorn
 ```
-This will give you something like this (deployed alongside the non-sidecar
-version of the charm for comparison):
+This will create an K8s ingress called `gunicorn-ingress` and a K8s service
+called `gunicorn-service`. The gunicorn charm in question, which can be found
+https://code.launchpad.net/~mthaddon/charm-k8s-gunicorn/+git/charm-k8s-gunicorn/+ref/pebble
+implements the relation as follows, as a trivial example:
 ```
-mthaddon@tenaya:~/repos/charm-k8s-ingress/charm-k8s-ingress$ microk8s.kubectl get ingress --all-namespaces
-NAMESPACE      NAME                   CLASS    HOSTS          ADDRESS   PORTS   AGE
-ingress-test   gunicorn-old-ingress   <none>   foo.internal             80      27m
-ingress-test   gunicorn-ingress       <none>   foo.internal             80      91s
-mthaddon@tenaya:~/repos/charm-k8s-ingress/charm-k8s-ingress$ juju status
-Model         Controller          Cloud/Region        Version  SLA          Timestamp
-ingress-test  microk8s-localhost  microk8s/localhost  2.9-rc7  unsupported  18:12:17+01:00
-
-App           Version            Status  Scale  Charm        Store  Channel  Rev  OS          Address        Message
-gunicorn                         active      1  gunicorn     local             3  ubuntu
-gunicorn-old  gunicorn-app:edge  active      1  gunicorn     local             1  kubernetes  10.152.183.69
-ingress                          active      1  ingress  local             3  ubuntu
+# In __init__:
+self.framework.observe(self.on['ingress'].relation_changed, self._on_ingress_changed)
 
-Unit             Workload  Agent  Address      Ports   Message
-gunicorn-old/1*  active    idle   10.1.234.25  80/TCP
-gunicorn/0*      active    idle   10.1.234.21
-ingress/0*       active    idle   10.1.234.28
+def _on_ingress_changed(self, event: ops.framework.EventBase) -> None:
+    """Handle the ingress relation changed event."""
+    if self.unit.is_leader():
+        event.relation.data[self.app]["service-hostname"] = self.config["external_hostname"]
+        event.relation.data[self.app]["service-name"] = self.model.name
+        event.relation.data[self.app]["service-port"] = "80"
+```
 
-mthaddon@tenaya:~/repos/charm-k8s-ingress/charm-k8s-ingress$ microk8s.kubectl describe ingress -n ingress-test gunicorn-ingress
-Name:             gunicorn-ingress
-Namespace:        ingress-test
-Address:
-Default backend:  default-http-backend:80 (<error: endpoints "default-http-backend" not found>)
-Rules:
-  Host          Path  Backends
-  ----          ----  --------
-  foo.internal  
-                /   gunicorn:80 ()
-Annotations:    nginx.ingress.kubernetes.io/rewrite-target: /
-Events:         <none>
-mthaddon@tenaya:~/repos/charm-k8s-ingress/charm-k8s-ingress$ microk8s.kubectl describe ingress -n ingress-test gunicorn-old-ingress
-Name:             gunicorn-old-ingress
-Namespace:        ingress-test
-Address:
-Default backend:  default-http-backend:80 (<error: endpoints "default-http-backend" not found>)
-Rules:
-  Host          Path  Backends
-  ----          ----  --------
-  foo.internal  
-                /   gunicorn-old:80 (10.1.234.25:80)
-Annotations:    controller.juju.is/id: ac6cacf4-3ed5-4313-88d2-89eef6ae5822
-                model.juju.is/id: 5cbdb63d-0847-4b92-8c4b-f4af185b60b0
-                nginx.ingress.kubernetes.io/ssl-redirect: false
-Events:         <none>
-mthaddon@tenaya:~/repos/charm-k8s-ingress/charm-k8s-ingress$ 
+Alternatively, you can configure the same ingress via Juju config options, to
+avoid needing to modify the charm you're deploying it alongside. As an example:
+```
+juju config ingress service-name=gunicorn service-port=80 service-hostname=foo.internal
 ```
-This needs further work and testing, but produces an initial ingress.
 
 ## Testing
 
diff --git a/metadata.yaml b/metadata.yaml
index 100436a..a6ee850 100644
--- a/metadata.yaml
+++ b/metadata.yaml
@@ -17,3 +17,8 @@ resources:
   placeholder-image:
     type: oci-image
     description: A placeholder image, operator code won't do anything with it.
+
+requires:
+  ingress:
+    interface: ingress
+    limit: 1
diff --git a/src/charm.py b/src/charm.py
index d9639d1..a206ba7 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -19,11 +19,29 @@ import kubernetes
 # from kubernetes.client.rest import ApiException as K8sApiException
 
 from ops.charm import CharmBase
+from ops.framework import StoredState
 from ops.main import main
-from ops.model import ActiveStatus
+from ops.model import (
+    ActiveStatus,
+    BlockedStatus,
+)
+
 
 logger = logging.getLogger(__name__)
 
+REQUIRED_INGRESS_RELATION_FIELDS = {
+    "service-hostname",
+    "service-name",
+    "service-port",
+}
+
+OPTIONAL_INGRESS_RELATION_FIELDS = {
+    "max-body-size",
+    "service-namespace",
+    "session-cookie-max-age",
+    "tls-secret-name",
+}
+
 
 def _core_v1_api():
     """Use the v1 k8s API."""
@@ -48,27 +66,78 @@ class CharmK8SIngressCharm(CharmBase):
     """Charm the service."""
 
     _authed = False
+    _stored = StoredState()
 
     def __init__(self, *args):
         super().__init__(*args)
         self.framework.observe(self.on.config_changed, self._on_config_changed)
 
+        # ingress relation handling.
+        self.framework.observe(self.on["ingress"].relation_changed, self._on_ingress_relation_changed)
+
+        self._stored.set_default(ingress_relation_data=dict())
+
+    def _on_ingress_relation_changed(self, event):
+        """Handle a change to the ingress relation."""
+        if not self.unit.is_leader():
+            return
+
+        ingress_fields = {
+            field: event.relation.data[event.app].get(field)
+            for field in REQUIRED_INGRESS_RELATION_FIELDS | OPTIONAL_INGRESS_RELATION_FIELDS
+        }
+
+        missing_fields = sorted(
+            [field for field in REQUIRED_INGRESS_RELATION_FIELDS if ingress_fields.get(field) is None]
+        )
+
+        if missing_fields:
+            logger.error("Missing required data fields for ingress relation: {}".format(", ".join(missing_fields)))
+            self.unit.status = BlockedStatus("Missing fields for ingress: {}".format(", ".join(missing_fields)))
+            return
+
+        # Set our relation data to stored_state.
+        self._stored.ingress_relation_data = ingress_fields
+
+        # Now trigger our config_changed handler.
+        self._on_config_changed(event)
+
     @property
     def _ingress_name(self):
         """Return an ingress name for use creating a k8s ingress."""
         # Follow the same naming convention as Juju.
-        return "{}-ingress".format(self.config["service-name"])
+        return "{}-ingress".format(
+            self._stored.ingress_relation_data.get("service-name") or self.config["service-name"]
+        )
 
     @property
     def _namespace(self):
         """Return the namespace to operate on."""
-        return self.config["service-namespace"] or self.model.name
+        return (
+            self._stored.ingress_relation_data.get("service-namespace")
+            or self.config["service-namespace"]
+            or self.model.name
+        )
 
     @property
-    def _service_name(self):
+    def _k8s_service_name(self):
         """Return a service name for the use creating a k8s service."""
-        # Avoid collision with service name created by Juju.
-        return "{}-service".format(self.config["service-name"])
+        # Avoid collision with service name created by Juju. Currently
+        # Juju creates a K8s service listening on port 65535/TCP so we
+        # need to create a separate one.
+        return "{}-service".format(self._service_name)
+
+    @property
+    def _service_name(self):
+        """Return the name of the service we're connecting to."""
+        return self._stored.ingress_relation_data.get("service-name") or self.config["service-name"]
+
+    @property
+    def _service_port(self):
+        """Return the port for the service we're connecting to."""
+        if self._stored.ingress_relation_data.get("service-port"):
+            return int(self._stored.ingress_relation_data.get("service-port"))
+        return self.config["service-port"]
 
     def k8s_auth(self):
         """Authenticate to kubernetes."""
@@ -92,14 +161,14 @@ class CharmK8SIngressCharm(CharmBase):
         return kubernetes.client.V1Service(
             api_version="v1",
             kind="Service",
-            metadata=kubernetes.client.V1ObjectMeta(name=self._service_name),
+            metadata=kubernetes.client.V1ObjectMeta(name=self._k8s_service_name),
             spec=kubernetes.client.V1ServiceSpec(
-                selector={"app.kubernetes.io/name": self.config["service-name"]},
+                selector={"app.kubernetes.io/name": self._service_name},
                 ports=[
                     kubernetes.client.V1ServicePort(
-                        name="tcp-{}".format(self.config["service-port"]),
-                        port=self.config["service-port"],
-                        target_port=self.config["service-port"],
+                        name="tcp-{}".format(self._service_port),
+                        port=self._service_port,
+                        target_port=self._service_port,
                     )
                 ],
             ),
@@ -116,8 +185,8 @@ class CharmK8SIngressCharm(CharmBase):
                             kubernetes.client.NetworkingV1beta1HTTPIngressPath(
                                 path="/",
                                 backend=kubernetes.client.NetworkingV1beta1IngressBackend(
-                                    service_port=self.config["service-port"],
-                                    service_name=self._service_name,
+                                    service_port=self._service_port,
+                                    service_name=self._k8s_service_name,
                                 ),
                             )
                         ]
@@ -137,7 +206,7 @@ class CharmK8SIngressCharm(CharmBase):
                 self.config["session-cookie-max-age"]
             )
             annotations["nginx.ingress.kubernetes.io/session-cookie-name"] = "{}_AFFINITY".format(
-                self.config["service-name"].upper()
+                self._service_name.upper()
             )
             annotations["nginx.ingress.kubernetes.io/session-cookie-samesite"] = "Lax"
         tls_secret_name = self.config["tls-secret-name"]
@@ -164,7 +233,7 @@ class CharmK8SIngressCharm(CharmBase):
         self.k8s_auth()
         api = _core_v1_api()
         services = api.list_namespaced_service(namespace=self._namespace)
-        return [x.spec.cluster_ip for x in services.items if x.metadata.name == self._service_name]
+        return [x.spec.cluster_ip for x in services.items if x.metadata.name == self._k8s_service_name]
 
     def _define_service(self):
         """Create or update a service in kubernetes."""
@@ -172,7 +241,7 @@ class CharmK8SIngressCharm(CharmBase):
         api = _core_v1_api()
         body = self._get_k8s_service()
         services = api.list_namespaced_service(namespace=self._namespace)
-        if self._service_name in [x.metadata.name for x in services.items]:
+        if self._k8s_service_name in [x.metadata.name for x in services.items]:
             # Currently failing with port[1].name required but we're only
             # defining one port above...
             # api.patch_namespaced_service(
@@ -181,7 +250,7 @@ class CharmK8SIngressCharm(CharmBase):
             #    body=body,
             # )
             api.delete_namespaced_service(
-                name=self._service_name,
+                name=self._k8s_service_name,
                 namespace=self._namespace,
             )
             api.create_namespaced_service(
@@ -191,7 +260,7 @@ class CharmK8SIngressCharm(CharmBase):
             logger.info(
                 "Service updated in namespace %s with name %s",
                 self._namespace,
-                self.config["service-name"],
+                self._service_name,
             )
         else:
             api.create_namespaced_service(
@@ -201,7 +270,7 @@ class CharmK8SIngressCharm(CharmBase):
             logger.info(
                 "Service created in namespace %s with name %s",
                 self._namespace,
-                self.config["service-name"],
+                self._service_name,
             )
 
     def _define_ingress(self):
@@ -219,7 +288,7 @@ class CharmK8SIngressCharm(CharmBase):
             logger.info(
                 "Ingress updated in namespace %s with name %s",
                 self._namespace,
-                self.config["service-name"],
+                self._service_name,
             )
         else:
             api.create_namespaced_ingress(
@@ -229,7 +298,7 @@ class CharmK8SIngressCharm(CharmBase):
             logger.info(
                 "Ingress created in namespace %s with name %s",
                 self._namespace,
-                self.config["service-name"],
+                self._service_name,
             )
 
     def _on_config_changed(self, _):
@@ -237,7 +306,7 @@ class CharmK8SIngressCharm(CharmBase):
         msg = ""
         # We only want to do anything here if we're the leader to avoid
         # collision if we've scaled out this application.
-        if self.unit.is_leader() and self.config["service-name"]:
+        if self.unit.is_leader() and self._service_name:
             self._define_service()
             self._define_ingress()
             # It's not recommended to do this via ActiveStatus, but we don't
diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
index 3eaebc6..3efb784 100644
--- a/tests/unit/test_charm.py
+++ b/tests/unit/test_charm.py
@@ -1,12 +1,16 @@
 # Copyright 2021 Tom Haddon
 # See LICENSE file for licensing details.
 
-import mock
 import unittest
 
+from unittest.mock import MagicMock, patch
+
 import kubernetes
 
-from ops.model import ActiveStatus
+from ops.model import (
+    ActiveStatus,
+    BlockedStatus,
+)
 from ops.testing import Harness
 from charm import CharmK8SIngressCharm
 
@@ -18,9 +22,9 @@ class TestCharm(unittest.TestCase):
         self.addCleanup(self.harness.cleanup)
         self.harness.begin()
 
-    @mock.patch('charm.CharmK8SIngressCharm._report_service_ips')
-    @mock.patch('charm.CharmK8SIngressCharm._define_ingress')
-    @mock.patch('charm.CharmK8SIngressCharm._define_service')
+    @patch('charm.CharmK8SIngressCharm._report_service_ips')
+    @patch('charm.CharmK8SIngressCharm._define_ingress')
+    @patch('charm.CharmK8SIngressCharm._define_service')
     def test_config_changed(self, _define_service, _define_ingress, _report_service_ips):
         """Test our config changed handler."""
         # First of all test, with leader set to True.
@@ -56,10 +60,81 @@ class TestCharm(unittest.TestCase):
 
     def test_namespace(self):
         """Test for the namespace property."""
+        # If charm config and _stored is empty, use model name.
+        self.assertEqual(self.harness.charm._stored.ingress_relation_data.get("service-namespace"), None)
         self.assertEqual(self.harness.charm.config["service-namespace"], "")
         self.assertEqual(self.harness.charm._namespace, self.harness.charm.model.name)
+        # If we set config, that takes precedence.
         self.harness.update_config({"service-namespace": "mymodelname"})
         self.assertEqual(self.harness.charm._namespace, "mymodelname")
+        # And if we set _stored, that takes precedence.
+        self.harness.charm._stored.ingress_relation_data["service-namespace"] = "relationnamespace"
+        self.assertEqual(self.harness.charm._namespace, "relationnamespace")
+
+    def test_service_port(self):
+        """Test the service-port property."""
+        # First set via config.
+        self.harness.update_config({"service-port": 80})
+        self.assertEqual(self.harness.charm._service_port, 80)
+        # Now set via the StoredState. This will be set to a string, as all
+        # relation data must be a string.
+        self.harness.charm._stored.ingress_relation_data["service-port"] = "88"
+        self.assertEqual(self.harness.charm._service_port, 88)
+
+    @patch('charm.CharmK8SIngressCharm._on_config_changed')
+    def test_on_ingress_relation_changed(self, _on_config_changed):
+        """Test ingress relation changed handler."""
+        # Confirm we do nothing if we're not the leader.
+        self.assertFalse(self.harness.charm.unit.is_leader())
+        mock_event = MagicMock()
+        self.assertEqual(self.harness.charm._stored.ingress_relation_data, {})
+        self.harness.charm._on_ingress_relation_changed(mock_event)
+        # Confirm no relation data has been set.
+        self.assertEqual(self.harness.charm._stored.ingress_relation_data, {})
+        # Confirm config_changed hasn't been called.
+        _on_config_changed.assert_not_called()
+
+        # Now test on the leader, but with missing fields in the relation data.
+        # We don't want leader-set to fire.
+        self.harness.disable_hooks()
+        self.harness.set_leader(True)
+        mock_event.app = "gunicorn"
+        mock_event.relation.data = {"gunicorn": {"service-name": "gunicorn"}}
+        with self.assertLogs(level="ERROR") as logger:
+            self.harness.charm._on_ingress_relation_changed(mock_event)
+            msg = "ERROR:charm:Missing required data fields for ingress relation: service-hostname, service-port"
+            self.assertEqual(sorted(logger.output), [msg])
+            # Confirm no relation data has been set.
+            self.assertEqual(self.harness.charm._stored.ingress_relation_data, {})
+            # Confirm config_changed hasn't been called.
+            _on_config_changed.assert_not_called()
+            # Confirm blocked status.
+            self.assertEqual(
+                self.harness.charm.unit.status,
+                BlockedStatus("Missing fields for ingress: service-hostname, service-port"),
+            )
+
+        # Now test with complete relation data.
+        mock_event.relation.data = {
+            "gunicorn": {
+                "service-hostname": "foo.internal",
+                "service-name": "gunicorn",
+                "service-port": "80",
+            }
+        }
+        self.harness.charm._on_ingress_relation_changed(mock_event)
+        expected = {
+            "max-body-size": None,
+            "service-hostname": "foo.internal",
+            "service-name": "gunicorn",
+            "service-namespace": None,
+            "service-port": "80",
+            "session-cookie-max-age": None,
+            "tls-secret-name": None,
+        }
+        self.assertEqual(self.harness.charm._stored.ingress_relation_data, expected)
+        # Confirm config_changed has been called.
+        _on_config_changed.assert_called_once()
 
     def test_get_k8s_ingress(self):
         """Test getting our definition of a k8s ingress."""
diff --git a/tox.ini b/tox.ini
index 066c5a8..b4c85dc 100644
--- a/tox.ini
+++ b/tox.ini
@@ -43,7 +43,7 @@ exclude =
     .git,
     __pycache__,
     .tox,
-# Ignore E231 because using black creates errors with this
-ignore = E231
+# Ignore E231, W503 because using black creates errors with this
+ignore = E231, W503
 max-line-length = 120
 max-complexity = 10