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

[Tom Haddon]

Add the retry-errors option

Add the retry-errors option.

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

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

[🤖 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:52fe1d914f6c52023e8f2d5e501e919a5428ea9c
https://jenkins.canonical.com/is/job/lp-charm-k8s-ingress-ci/33/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/is/job/lp-charm-test/70/
    None: https://jenkins.canonical.com/is/job/lp-update-mp/81314/

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

review: Approve (continuous-integration)

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

Change successfully merged at revision 0f23f2610e89e9fab3786e832919bc5433410b50

diff --git a/config.yaml b/config.yaml
index c7b2377..9a9b7cf 100644
--- a/config.yaml
+++ b/config.yaml
@@ -9,6 +9,15 @@ options:
     default: 0
     description: Max allowed body-size (for file uploads) in megabytes, set to 0 to disable limits.
     type: int
+  retry-errors:
+    default: ""
+    description: >
+      Specifies in which cases a request should be retried against the next server.
+      Comma-separated list, e.g. "error,timeout,http_502,http_503,http_504".
+      See http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_next_upstream
+      for more details. Unrecognised values will be ignored. The nginx default
+      will be used if this config option is set to an empty value.
+    type: string
   service-hostname:
     default: ""
     description: The hostname of the service to create an ingress for.
diff --git a/lib/charms/ingress/v0/ingress.py b/lib/charms/ingress/v0/ingress.py
index 2412e06..acef2c0 100644
--- a/lib/charms/ingress/v0/ingress.py
+++ b/lib/charms/ingress/v0/ingress.py
@@ -12,10 +12,13 @@ Import `IngressRequires` in your charm, with two required options:
     - service-name (required)
     - service-port (required)
     - max_body-size
+    - retry-errors
     - service-namespace
     - session-cookie-max-age
     - tls-secret-name
 
+See `config.yaml` for descriptions of each, along with the required type.
+
 As an example:
 ```
 from charms.ingress.v0.ingress import IngressRequires
@@ -44,7 +47,7 @@ LIBAPI = 0
 
 # Increment this PATCH version before using `charmcraft push-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 7
+LIBPATCH = 8
 
 logger = logging.getLogger(__name__)
 
@@ -56,6 +59,7 @@ REQUIRED_INGRESS_RELATION_FIELDS = {
 
 OPTIONAL_INGRESS_RELATION_FIELDS = {
     "max-body-size",
+    "retry-errors",
     "service-namespace",
     "session-cookie-max-age",
     "tls-secret-name",
diff --git a/src/charm.py b/src/charm.py
index 17ee96d..e14f76a 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -97,6 +97,29 @@ class IngressCharm(CharmBase):
         return self._get_config_or_relation_data("service-namespace", self.model.name)
 
     @property
+    def _retry_errors(self):
+        """Return the retry-errors setting from config or relation."""
+        retry = self._get_config_or_relation_data("retry-errors", "")
+        if not retry:
+            return ""
+        # See http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_next_upstream.
+        accepted_values = [
+            "error",
+            "timeout",
+            "invalid_header",
+            "http_500",
+            "http_502",
+            "http_503",
+            "http_504",
+            "http_403",
+            "http_404",
+            "http_429",
+            "non_idempotent",
+            "off",
+        ]
+        return " ".join([x.strip() for x in retry.split(",") if x.strip() in accepted_values])
+
+    @property
     def _service_hostname(self):
         """Return the hostname for the service we're connecting to."""
         return self._get_config_or_relation_data("service-hostname", "")
@@ -185,6 +208,8 @@ class IngressCharm(CharmBase):
         }
         if self._max_body_size:
             annotations["nginx.ingress.kubernetes.io/proxy-body-size"] = self._max_body_size
+        if self._retry_errors:
+            annotations["nginx.ingress.kubernetes.io/proxy-next-upstream"] = self._retry_errors
         if self._session_cookie_max_age:
             annotations["nginx.ingress.kubernetes.io/affinity"] = "cookie"
             annotations["nginx.ingress.kubernetes.io/affinity-mode"] = "balanced"
@@ -262,7 +287,7 @@ class IngressCharm(CharmBase):
             logger.info(
                 "Ingress updated in namespace %s with name %s",
                 self._namespace,
-                self._service_name,
+                self._ingress_name,
             )
         else:
             api.create_namespaced_ingress(
@@ -272,7 +297,7 @@ class IngressCharm(CharmBase):
             logger.info(
                 "Ingress created in namespace %s with name %s",
                 self._namespace,
-                self._service_name,
+                self._ingress_name,
             )
 
     def _on_config_changed(self, _):
diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
index 4075546..ac2f0c5 100644
--- a/tests/unit/test_charm.py
+++ b/tests/unit/test_charm.py
@@ -132,6 +132,19 @@ class TestCharm(unittest.TestCase):
         self.harness.update_relation_data(relation_id, 'gunicorn', relations_data)
         self.assertEqual(self.harness.charm._namespace, "relationnamespace")
 
+    def test_retry_errors(self):
+        """Test the retry-errors property."""
+        # Test empty value.
+        self.assertEqual(self.harness.charm._retry_errors, "")
+        # Test we deal with spaces or not spaces properly.
+        self.harness.update_config({"retry-errors": "error, timeout, http_502, http_503"})
+        self.assertEqual(self.harness.charm._retry_errors, "error timeout http_502 http_503")
+        self.harness.update_config({"retry-errors": "error,timeout,http_502,http_503"})
+        self.assertEqual(self.harness.charm._retry_errors, "error timeout http_502 http_503")
+        # Test unknown value.
+        self.harness.update_config({"retry-errors": "error,timeout,http_502,http_418"})
+        self.assertEqual(self.harness.charm._retry_errors, "error timeout http_502")
+
     def test_service_port(self):
         """Test the service-port property."""
         # First set via config.
@@ -323,8 +336,15 @@ class TestCharm(unittest.TestCase):
         )
         self.harness.update_config({"tls-secret-name": "gunicorn_tls"})
         self.assertEqual(self.harness.charm._get_k8s_ingress(), expected)
-        # Test max_body_size and session-cookie-max-age config options.
-        self.harness.update_config({"tls-secret-name": "", "max-body-size": 20, "session-cookie-max-age": 3600})
+        # Test max_body_size, retry_http_errors and session-cookie-max-age config options.
+        self.harness.update_config(
+            {
+                "max-body-size": 20,
+                "retry-errors": "error,timeout,http_502,http_503",
+                "session-cookie-max-age": 3600,
+                "tls-secret-name": "",
+            }
+        )
         expected = kubernetes.client.NetworkingV1beta1Ingress(
             api_version="networking.k8s.io/v1beta1",
             kind="Ingress",
@@ -334,6 +354,7 @@ class TestCharm(unittest.TestCase):
                     "nginx.ingress.kubernetes.io/affinity": "cookie",
                     "nginx.ingress.kubernetes.io/affinity-mode": "balanced",
                     "nginx.ingress.kubernetes.io/proxy-body-size": "20m",
+                    "nginx.ingress.kubernetes.io/proxy-next-upstream": "error timeout http_502 http_503",
                     "nginx.ingress.kubernetes.io/rewrite-target": "/",
                     "nginx.ingress.kubernetes.io/session-cookie-change-on-failure": "true",
                     "nginx.ingress.kubernetes.io/session-cookie-max-age": "3600",