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

[Tom Haddon]

Configure an ingress defined by juju config options, switch to tox following existing charm patterns.

[🤖 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.

[Stuart Bishop (stub)]

Yup. This is of course still in a state of flux (per the documented failure), but it is good to land this as we go.

review: Approve

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

Change successfully merged at revision 20535875cd9ac90ff7a7fa6ef41367e2272b597d

diff --git a/Makefile b/Makefile
new file mode 100644
index 0000000..a3166a1
--- /dev/null
+++ b/Makefile
@@ -0,0 +1,23 @@
+blacken:
+	@echo "Normalising python layout with black."
+	@tox -e black
+
+lint: blacken
+	@echo "Running flake8"
+	@tox -e lint
+
+# We actually use the build directory created by charmcraft,
+# but the .charm file makes a much more convenient sentinel.
+unittest: k8s-ingress.charm
+	@tox -e unit
+
+test: lint unittest
+
+clean:
+	@echo "Cleaning files"
+	@git clean -fXd
+
+mattermost.charm: src/*.py requirements.txt
+	charmcraft build
+
+.PHONY: lint test unittest clean
diff --git a/README.md b/README.md
index 9d215b0..7458fdc 100644
--- a/README.md
+++ b/README.md
@@ -1,25 +1,136 @@
-# charm-k8s-ingress
+# Ingress Operator
 
 ## Description
 
-TODO: Describe your charm in a few paragraphs of Markdown
+This charm is intended to provide an ingress for sidecar charms using the
+Operator Framework until such time as Juju can expose the relevant primitives
+to enable charms to configure an ingress natively via Juju (e.g. with TLS as
+required, with session affinity as required, allowing for upload of a given
+size, etc.).
 
 ## Usage
 
 TODO: Provide high-level usage, such as required config or relations
 
+## Running the charm locally
 
-## Developing
+To build the charm, run `charmcraft build`.
 
-Create and activate a virtualenv with the development requirements:
+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:
 
-    virtualenv -p python3 venv
-    source venv/bin/activate
-    pip install -r requirements-dev.txt
+```
+git clone -b demo-pebble https://github.com/benhoyt/juju
+cd juju
+make install
+make microk8s-operator-update  # to make the microk8s image and push to Docker
+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 ./k8s-ingress.charm --resource placeholder-image='google/pause'
+```
+This is currently failing with the following error in `juju debug-log`.
+```
+unit-k8s-ingress-0: 05:12:02 INFO juju.worker.uniter awaiting error resolution for "config-changed" hook
+unit-k8s-ingress-0: 05:12:03 ERROR unit.k8s-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-k8s-ingress-0/charm/venv/ops/main.py", line 402, in main
+    _emit_charm_event(charm, dispatcher.event_name)
+  File "/var/lib/juju/agents/unit-k8s-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-k8s-ingress-0/charm/venv/ops/framework.py", line 278, in emit
+    framework._emit(event)
+  File "/var/lib/juju/agents/unit-k8s-ingress-0/charm/venv/ops/framework.py", line 722, in _emit
+    self._reemit(event_path)
+  File "/var/lib/juju/agents/unit-k8s-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-k8s-ingress-0/charm/venv/kubernetes/config/incluster_config.py", line 118, in load_incluster_config
+    InClusterConfigLoader(
+  File "/var/lib/juju/agents/unit-k8s-ingress-0/charm/venv/kubernetes/config/incluster_config.py", line 54, in load_and_set
+    self._load_config()
+  File "/var/lib/juju/agents/unit-k8s-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-k8s-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.
 
-## Testing
+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:
+```
+juju deploy ./k8s-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:
+```
+juju config k8s-ingress service-namespace=ingress-test service-name=gunicorn service-port=80 service-hostname=foo.internal
+```
+This will give you something like this (deployed alongside the non-sidecar
+version of the charm for comparison):
+```
+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
+k8s-ingress                      active      1  k8s-ingress  local             3  ubuntu
 
-The Python operator framework includes a very nice harness for testing
-operator behaviour without full deployment. Just `run_tests`:
+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
+k8s-ingress/0*   active    idle   10.1.234.28
+
+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$ 
+```
+This needs further work and testing, but produces an initial ingress.
+
+## Testing
 
-    ./run_tests
+Simply run `make test`.
diff --git a/config.yaml b/config.yaml
index 5323e7e..b2bd3c8 100644
--- a/config.yaml
+++ b/config.yaml
@@ -1,6 +1,26 @@
 # Copyright 2021 Tom Haddon
 # See LICENSE file for licensing details.
 options:
+  kube-config:
+    default: ""
+    description: k8s config. If you're using microk8s, this should be the output of `microk8s config`.
+    type: string
+  service-hostname:
+    default: ""
+    description: The hostname of the service to create an ingress for.
+    type: string
+  service-name:
+    default: ""
+    description: The name of the service to create an ingress for.
+    type: string
+  service-namespace:
+    default: ""
+    description: The namespace of the service to create an ingress for.
+    type: string
+  service-port:
+    default: 0
+    description: The port of the service to create an ingress for.
+    type: int
   thing:
     default: 🎁
     description: A thing used by the charm.
diff --git a/requirements.txt b/requirements.txt
index 2d81d3b..cd914f9 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1 +1,2 @@
+kubernetes
 ops
diff --git a/run_tests b/run_tests
deleted file mode 100755
index 697472e..0000000
--- a/run_tests
+++ /dev/null
@@ -1,17 +0,0 @@
-#!/bin/sh -e
-# Copyright 2021 Tom Haddon
-# See LICENSE file for licensing details.
-
-if [ -z "$VIRTUAL_ENV" -a -d venv/ ]; then
-    . venv/bin/activate
-fi
-
-if [ -z "$PYTHONPATH" ]; then
-    export PYTHONPATH=src
-else
-    export PYTHONPATH="src:$PYTHONPATH"
-fi
-
-flake8
-coverage run --source=src -m unittest -v "$@"
-coverage report -m
diff --git a/src/charm.py b/src/charm.py
index ced295f..4b85b26 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -11,6 +11,12 @@ develop a new k8s charm using the Operator Framework:
 """
 
 import logging
+import os
+from pathlib import Path
+
+import kubernetes
+
+# from kubernetes.client.rest import ApiException as K8sApiException
 
 from ops.charm import CharmBase
 from ops.main import main
@@ -20,9 +26,29 @@ from ops.framework import StoredState
 logger = logging.getLogger(__name__)
 
 
+def _core_v1_api():
+    """Use the v1 k8s API."""
+    cl = kubernetes.client.ApiClient()
+    return kubernetes.client.CoreV1Api(cl)
+
+
+def _networking_v1_beta1_api():
+    """Use the v1 beta1 networking API."""
+    return kubernetes.client.NetworkingV1beta1Api()
+
+
+def _fix_lp_1892255():
+    """Workaround for lp:1892255."""
+    # Remove os.environ.update when lp:1892255 is FIX_RELEASED.
+    os.environ.update(
+        dict(e.split("=") for e in Path("/proc/1/environ").read_text().split("\x00") if "KUBERNETES_SERVICE" in e)
+    )
+
+
 class CharmK8SIngressCharm(CharmBase):
     """Charm the service."""
 
+    _authed = False
     _stored = StoredState()
 
     def __init__(self, *args):
@@ -30,13 +56,149 @@ class CharmK8SIngressCharm(CharmBase):
         self.framework.observe(self.on.config_changed, self._on_config_changed)
         self._stored.set_default(things=[])
 
+    def k8s_auth(self):
+        """Authenticate to kubernetes."""
+        if self._authed:
+            return
+
+        _fix_lp_1892255()
+
+        # Work around for lp#1920102 - allow the user to pass in k8s config manually.
+        if self.config["kube-config"]:
+            with open('/kube-config', 'w') as kube_config:
+                kube_config.write(self.config["kube-config"])
+            kubernetes.config.load_kube_config(config_file='/kube-config')
+        else:
+            kubernetes.config.load_incluster_config()
+
+        self._authed = True
+
+    def _define_service(self):
+        """Create or update a service in kubernetes."""
+        self.k8s_auth()
+        api = _core_v1_api()
+
+        # Juju will create a service named the same as the application. We
+        # need to use a different name to avoid collision.
+        service_name = "{}-service".format(self.config["service-name"])
+
+        body = kubernetes.client.V1Service(
+            api_version="v1",
+            kind="Service",
+            metadata=kubernetes.client.V1ObjectMeta(name=service_name),
+            spec=kubernetes.client.V1ServiceSpec(
+                selector={"app.kubernetes.io/name": self.config["service-name"]},
+                ports=[
+                    kubernetes.client.V1ServicePort(
+                        name="tcp-{}".format(self.config["service-port"]),
+                        port=self.config["service-port"],
+                        target_port=self.config["service-port"],
+                    )
+                ],
+            ),
+        )
+        services = api.list_namespaced_service(namespace=self.config["service-namespace"])
+        if 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(
+            #    name=service_name,
+            #    namespace=self.config["service-namespace"],
+            #    body=body,
+            # )
+            api.delete_namespaced_service(
+                name=service_name,
+                namespace=self.config["service-namespace"],
+            )
+            api.create_namespaced_service(
+                namespace=self.config["service-namespace"],
+                body=body,
+            )
+            logger.info(
+                "Service updated in namespace %s with name %s",
+                self.config["service-namespace"],
+                self.config["service-name"],
+            )
+        else:
+            api.create_namespaced_service(
+                namespace=self.config["service-namespace"],
+                body=body,
+            )
+            logger.info(
+                "Service created in namespace %s with name %s",
+                self.config["service-namespace"],
+                self.config["service-name"],
+            )
+
+    def _define_ingress(self):
+        """Create or update an ingress in kubernetes."""
+        self.k8s_auth()
+        api = _networking_v1_beta1_api()
+
+        # Follow the same naming convention as Juju.
+        ingress_name = "{}-ingress".format(self.config["service-name"])
+
+        body = kubernetes.client.NetworkingV1beta1Ingress(
+            api_version="networking.k8s.io/v1beta1",
+            kind="Ingress",
+            metadata=kubernetes.client.V1ObjectMeta(
+                name=ingress_name,
+                annotations={
+                    "nginx.ingress.kubernetes.io/rewrite-target": "/",
+                    "nginx.ingress.kubernetes.io/ssl-redirect": "false",
+                },
+            ),
+            spec=kubernetes.client.NetworkingV1beta1IngressSpec(
+                rules=[
+                    kubernetes.client.NetworkingV1beta1IngressRule(
+                        host=self.config["service-hostname"],
+                        http=kubernetes.client.NetworkingV1beta1HTTPIngressRuleValue(
+                            paths=[
+                                kubernetes.client.NetworkingV1beta1HTTPIngressPath(
+                                    path="/",
+                                    backend=kubernetes.client.NetworkingV1beta1IngressBackend(
+                                        service_port=self.config["service-port"],
+                                        service_name="{}-service".format(self.config["service-name"]),
+                                    ),
+                                )
+                            ]
+                        ),
+                    )
+                ]
+            ),
+        )
+        ingresses = api.list_namespaced_ingress(namespace=self.config["service-namespace"])
+        if ingress_name in [x.metadata.name for x in ingresses.items]:
+            api.patch_namespaced_ingress(
+                name=ingress_name,
+                namespace=self.config["service-namespace"],
+                body=body,
+            )
+            logger.info(
+                "Ingress updated in namespace %s with name %s",
+                self.config["service-namespace"],
+                self.config["service-name"],
+            )
+        else:
+            api.create_namespaced_ingress(
+                namespace=self.config["service-namespace"],
+                body=body,
+            )
+            logger.info(
+                "Ingress created in namespace %s with name %s",
+                self.config["service-namespace"],
+                self.config["service-name"],
+            )
+
     def _on_config_changed(self, _):
-        # Note: you need to uncomment the example in the config.yaml file for this to work (ensure
-        # to not just leave the example, but adapt to your configuration needs)
+        """Handle the config changed event."""
         current = self.config["thing"]
         if current not in self._stored.things:
             logger.debug("found a new thing: %r", current)
             self._stored.things.append(current)
+        if self.config["service-name"]:
+            self._define_service()
+            self._define_ingress()
         self.unit.status = ActiveStatus()
 
 
diff --git a/tests/unit/requirements.txt b/tests/unit/requirements.txt
new file mode 100644
index 0000000..65431fc
--- /dev/null
+++ b/tests/unit/requirements.txt
@@ -0,0 +1,4 @@
+mock
+pytest
+pytest-cov
+pyyaml
diff --git a/tests/test_charm.py b/tests/unit/test_charm.py
similarity index 55%
rename from tests/test_charm.py
rename to tests/unit/test_charm.py
index eb459e0..861051d 100644
--- a/tests/test_charm.py
+++ b/tests/unit/test_charm.py
@@ -1,35 +1,20 @@
 # Copyright 2021 Tom Haddon
 # See LICENSE file for licensing details.
 
+import mock
 import unittest
-from unittest.mock import Mock
 
 from ops.testing import Harness
 from charm import CharmK8SIngressCharm
 
 
 class TestCharm(unittest.TestCase):
-    def test_config_changed(self):
+    @mock.patch('charm.CharmK8SIngressCharm._define_ingress')
+    @mock.patch('charm.CharmK8SIngressCharm._define_service')
+    def test_config_changed(self, _define_service, _define_ingress):
         harness = Harness(CharmK8SIngressCharm)
         self.addCleanup(harness.cleanup)
         harness.begin()
         self.assertEqual(list(harness.charm._stored.things), [])
         harness.update_config({"thing": "foo"})
         self.assertEqual(list(harness.charm._stored.things), ["foo"])
-
-    def test_action(self):
-        harness = Harness(CharmK8SIngressCharm)
-        harness.begin()
-        # the harness doesn't (yet!) help much with actions themselves
-        action_event = Mock(params={"fail": ""})
-        harness.charm._on_fortune_action(action_event)
-
-        self.assertTrue(action_event.set_results.called)
-
-    def test_action_fail(self):
-        harness = Harness(CharmK8SIngressCharm)
-        harness.begin()
-        action_event = Mock(params={"fail": "fail this"})
-        harness.charm._on_fortune_action(action_event)
-
-        self.assertEqual(action_event.fail.call_args, [("fail this",)])
diff --git a/tox.ini b/tox.ini
new file mode 100644
index 0000000..066c5a8
--- /dev/null
+++ b/tox.ini
@@ -0,0 +1,49 @@
+[tox]
+skipsdist=True
+envlist = unit, functional
+skip_missing_interpreters = True
+
+[testenv]
+basepython = python3
+setenv =
+  PYTHONPATH = {toxinidir}/build/lib:{toxinidir}/build/venv
+
+[testenv:unit]
+commands =
+    pytest --ignore mod --ignore {toxinidir}/tests/functional \
+      {posargs:-v  --cov=src --cov-report=term-missing --cov-branch}
+deps = -r{toxinidir}/tests/unit/requirements.txt
+       -r{toxinidir}/requirements.txt
+setenv =
+  PYTHONPATH={toxinidir}/src:{toxinidir}/build/lib:{toxinidir}/build/venv
+  TZ=UTC
+
+[testenv:functional]
+passenv =
+  HOME
+  JUJU_REPOSITORY
+  PATH
+commands =
+	pytest -v --ignore {toxinidir}/tests/unit {posargs}
+deps = -r{toxinidir}/tests/functional/requirements.txt
+       -r{toxinidir}/requirements.txt
+
+[testenv:black]
+commands = black --skip-string-normalization --line-length=120 src/ tests/
+deps = black
+
+[testenv:lint]
+commands = flake8 src/ tests/
+# Pin flake8 to 3.7.9 to match focal
+deps =
+    flake8==3.7.9
+
+[flake8]
+exclude =
+    .git,
+    __pycache__,
+    .tox,
+# Ignore E231 because using black creates errors with this
+ignore = E231
+max-line-length = 120
+max-complexity = 10