Merge ~hloeung/charm-k8s-content-cache:master into charm-k8s-content-cache:master

[Haw Loeung]

Implement charm functionality and add unit tests

Implement charm functionality and add unit tests. Example is proxying / caching of archive.ubuntu.com.

| ubuntu@juju-87625f-hloeung-65:~/haw/docker$ jsft
| Model Controller Cloud/Region Version SLA Timestamp
| charm-k8s micro microk8s/localhost 2.8.1 unsupported 03:20:19Z
|
| App Version Status Scale Charm Store Rev OS Address Notes
| content-cache active 1 charm-k8s-content-cache local 22 kubernetes 10.152.183.163
|
| Unit Workload Agent Address Ports Message
| content-cache/42* active idle 10.1.75.118 80/TCP

Generated Nginx config - https://paste.ubuntu.com/p/yCg6W6pMw6/

Test against the unit itself (10.1.75.118) - https://paste.ubuntu.com/p/Wq3tp5JnTQ/

With the access logs:

| ubuntu@juju-87625f-hloeung-65:~/haw/docker$ kubectl -n charm-k8s logs content-cache-774767fd45-njkzs | grep -v 'kube-probe'
| - - - [08/Sep/2020:03:02:11 +0000] "GET / HTTP/1.0" 200 965 "-" "-" 0.281 MISS 0.128
| - - - [08/Sep/2020:03:02:21 +0000] "GET / HTTP/1.0" 200 964 "-" "-" 0.580 HIT -
| - - - [08/Sep/2020:03:04:36 +0000] "GET / HTTP/1.0" 200 964 "-" "-" 0.203 HIT -

kubectl describe ingress - https://paste.ubuntu.com/p/qJ3ZkqDWp5/

Test against the ingress address (10.152.183.163) - https://paste.ubuntu.com/p/qGT2cngrwJ/

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

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

[Barry Price (barryprice)]

Looks good, left a few inline comments - you could also consider adding a lint target for the shell script(s) included under the docker/ dir - charm-k8s-bind uses shellcheck for this.

Also seems to be missing the standard COPYRIGHT/LICENSE files, plus the boilerplate copyright headers in indidivual files, but code itself looks good.

[Tom Haddon (mthaddon)]

As discussed we'll want to break this up into two MPs, one with non-operator framework changes that we can review/approve/merge internally and then another with operator framework changes (src/charm.py + unit tests) that we can review internally and then put up for review with the charmcrafters LP team.

One comment inline about the name of the charm to deploy, which as discussed means we want to update metadata.yaml so the name of the charm is "content-cache".

Also, just a note that we'll want docstrings on all unit tests for when it comes to that MP.

Thanks!

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

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

[Haw Loeung (hloeung)]

Split up.

* charm and associated unit tests:

| https://code.launchpad.net/~hloeung/charm-k8s-content-cache/+git/charm-k8s-content-cache/+merge/390393

* The rest of the stuff (non-charm):

| https://code.launchpad.net/~hloeung/charm-k8s-content-cache/+git/charm-k8s-content-cache/+merge/390392

[Tom Haddon (mthaddon)]

Two very minor comments inline, and then I think we're ready to put this up for review with charmcrafters. Thanks!

[Tom Haddon (mthaddon)]

Actually, I think I've misread the ops code comments on this:

use_juju_for_storage: whether to use controller-side storage. If not specified
  then kubernetes charms that haven't previously used local storage and that
  are running on a new enough Juju default to controller-side storage,
  otherwise local storage is used.

So I think this means it's not needed at this point (there won't be instances deployed that have used local storage already, so it'll default to using controller-side storage. So we don't need the use_juju_for_storage option set after all.

[Tom Haddon (mthaddon)]

Lgtm, will add charmcrafters to reviewers

[Facundo Batista (facundo)]

Added some minor inline comments, thanks!

review: Needs Fixing

[Haw Loeung (hloeung)]

review: Needs Resubmitting

[Facundo Batista (facundo)]

Great, thanks!

review: Approve

[Tom Haddon (mthaddon)]

Adding an approving comment so this can be merged.

review: Approve

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

Change successfully merged at revision 49946ba05969ee713fb15c8fb8aa297ab2800d9e

diff --git a/config.yaml b/config.yaml
index 1cf64d6..141b70d 100644
--- a/config.yaml
+++ b/config.yaml
@@ -1,6 +1,61 @@
 options:
-  sites:
+  image_path:
     type: string
-    description: >
-      YAML-formatted virtual hosts/sites. See the README.md for more details
-      and examples.
+    description: >-
+      The location of the image to use, e.g. "localhost:32000/myimage:latest"
+
+      This setting is required.
+  image_username:
+    type: string
+    description: >-
+      The username for accessing the registry specified in image_path.
+    default: ""
+  image_password:
+    type: string
+    description: >-
+      The password associated with image_username for accessing the registry
+      specified in image_path.
+    default: ""
+  site:
+    type: string
+    description: >-
+      The site name, e.g. "mysite.local"
+
+      This setting is required.
+  backend:
+    type: string
+    description: >-
+      The backend to use for site, e.g. "http://mybackend.local:80"
+
+      This setting is required.
+  cache_inactive_time:
+    type: string
+    description: >-
+      The maximum age/time inactive objects are stored in cache.
+    default: "10m"
+  cache_max_size:
+    type: string
+    description: >-
+      The size of the Nginx storage cache.
+    default: "10G"
+  cache_use_stale:
+    type: string
+    description: >-
+      Determines in which cases a stale cached response can be used
+      during communication with the proxied server.
+    default: "error timeout updating http_500 http_502 http_503 http_504"
+  cache_valid:
+    type: string
+    decription: >-
+      Sets caching time for different response codes
+    default: "200 1h"
+  client_max_body_size:
+    type: string
+    description: >-
+      Override max. request body size (default 1m).
+    default: ""
+  tls_secret_name:
+    type: string
+    description: >-
+      The name of the K8s secret to be associated with the ingress resource.
+    default: ""
diff --git a/src/charm.py b/src/charm.py
index c5ab80b..171e521 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -1,38 +1,226 @@
 #!/usr/bin/env python3
-# Copyright 2020 hloeung
+
+# Copyright (C) 2020 Canonical Ltd.
 # See LICENSE file for licensing details.
 
+import hashlib
 import logging
 
 from ops.charm import CharmBase
 from ops.main import main
-from ops.framework import StoredState
+from ops.model import (
+    ActiveStatus,
+    BlockedStatus,
+    MaintenanceStatus,
+)
 
 logger = logging.getLogger(__name__)
 
+CACHE_PATH = '/var/lib/nginx/proxy/cache'
+CONTAINER_PORT = 80
+REQUIRED_JUJU_CONFIGS = ['image_path', 'site', 'backend']
 
-class CharmK8SContentCacheCharm(CharmBase):
-    _stored = StoredState()
 
+class ContentCacheCharm(CharmBase):
     def __init__(self, *args):
         super().__init__(*args)
+
+        self.framework.observe(self.on.start, self._on_start)
         self.framework.observe(self.on.config_changed, self._on_config_changed)
-        self.framework.observe(self.on.fortune_action, self._on_fortune_action)
-        self._stored.set_default(things=[])
-
-    def _on_config_changed(self, _):
-        current = self.model.config["thing"]
-        if current not in self._stored.things:
-            logger.debug("found a new thing: %r", current)
-            self._stored.things.append(current)
-
-    def _on_fortune_action(self, event):
-        fail = event.params["fail"]
-        if fail:
-            event.fail(fail)
+        self.framework.observe(self.on.leader_elected, self._on_leader_elected)
+        self.framework.observe(self.on.upgrade_charm, self._on_upgrade_charm)
+
+    def _on_start(self, event) -> None:
+        """Handle pod started."""
+        self.model.unit.status = ActiveStatus('Started')
+
+    def _on_config_changed(self, event) -> None:
+        """Check that we're the leader, and if so, configure/set up pod."""
+        if not self.unit.is_leader():
+            logger.info('Spec changes ignored by non-leader')
+            self.unit.status = ActiveStatus('Ready')
+            return
+        msg = 'Configuring pod (config-changed)'
+        logger.info(msg)
+        self.model.unit.status = MaintenanceStatus(msg)
+
+        self.configure_pod(event)
+
+    def _on_leader_elected(self, event) -> None:
+        """Check that we're the leader, and if so, configure/set up pod."""
+        msg = 'Configuring pod (leader-elected)'
+        logger.info(msg)
+        self.model.unit.status = MaintenanceStatus(msg)
+        self.configure_pod(event)
+
+    def _on_upgrade_charm(self, event) -> None:
+        """Check that we're the leader, and if so, configure/set up pod."""
+        if not self.unit.is_leader():
+            logger.info('Spec changes ignored by non-leader')
+            self.unit.status = ActiveStatus('Ready')
+            return
+        msg = 'Configuring pod (upgrade-charm)'
+        logger.info(msg)
+        self.model.unit.status = MaintenanceStatus(msg)
+        self.configure_pod(event)
+
+    def configure_pod(self, event) -> None:
+        """Assemble both K8s ingress and pod spec and apply."""
+        missing = self._missing_charm_configs()
+        if missing:
+            msg = 'Required config(s) empty: {}'.format(', '.join(sorted(missing)))
+            logger.warning(msg)
+            self.unit.status = BlockedStatus(msg)
+            return
+
+        msg = 'Assembling K8s ingress spec'
+        logger.info(msg)
+        self.unit.status = MaintenanceStatus(msg)
+        ingress_spec = self._make_k8s_ingress_spec()
+        k8s_resources = {'kubernetesResources': {'ingressResources': ingress_spec}}
+
+        msg = 'Assembling pod spec'
+        logger.info(msg)
+        self.unit.status = MaintenanceStatus(msg)
+        pod_spec = self._make_pod_spec()
+
+        msg = 'Setting pod spec'
+        logger.info(msg)
+        self.unit.status = MaintenanceStatus(msg)
+        self.model.pod.set_spec(pod_spec, k8s_resources=k8s_resources)
+
+        msg = 'Done applying updated pod spec'
+        logger.info(msg)
+        self.unit.status = ActiveStatus('Ready')
+
+    def _generate_keys_zone(self, name):
+        """Generate hashed name to be used by Nginx's key zone."""
+        return '{}-cache'.format(hashlib.md5(name.encode('UTF-8')).hexdigest()[0:12])
+
+    def _make_k8s_ingress_spec(self) -> list:
+        """Return an assembled K8s ingress spec to be used by pod.set_spec()'s k8s_resources."""
+        config = self.model.config
+
+        annotations = {}
+        ingress = {
+            'name': '{}-ingress'.format(self.app.name),
+            'spec': {
+                'rules': [
+                    {
+                        'host': config['site'],
+                        'http': {
+                            'paths': [
+                                {'path': '/', 'backend': {'serviceName': self.app.name, 'servicePort': CONTAINER_PORT}}
+                            ],
+                        },
+                    }
+                ],
+            },
+        }
+
+        client_max_body_size = config.get('client_max_body_size')
+        if client_max_body_size:
+            annotations['nginx.ingress.kubernetes.io/proxy-body-size'] = client_max_body_size
+
+        tls_secret_name = config.get('tls_secret_name')
+        if tls_secret_name:
+            ingress['spec']['tls'] = [{'hosts': config['site'], 'secretName': tls_secret_name}]
         else:
-            event.set_results({"fortune": "A bug in the code is worth two in the documentation."})
+            annotations['nginx.ingress.kubernetes.io/ssl-redirect'] = 'false'
+
+        if annotations:
+            ingress['annotations'] = annotations
+
+        return [ingress]
+
+    def _make_pod_spec(self) -> dict:
+        """Return an assembled K8s pod spec with appropriate configs set."""
+        config = self.model.config
+
+        image_details = {
+            'imagePath': config['image_path'],
+        }
+        if config.get('image_username', None):
+            image_details.update({'username': config['image_username'], 'password': config['image_password']})
+
+        pod_config = self._make_pod_config()
+
+        pod_spec = {
+            'version': 3,  # otherwise resources are ignored
+            'containers': [
+                {
+                    'name': self.app.name,
+                    'envConfig': pod_config,
+                    'imageDetails': image_details,
+                    'imagePullPolicy': 'Always',
+                    'kubernetes': {
+                        'livenessProbe': {
+                            'httpGet': {'path': '/', 'port': CONTAINER_PORT},
+                            'initialDelaySeconds': 3,
+                            'periodSeconds': 3,
+                        },
+                        'readinessProbe': {
+                            'httpGet': {'path': '/', 'port': CONTAINER_PORT},
+                            'initialDelaySeconds': 3,
+                            'periodSeconds': 3,
+                        },
+                    },
+                    'ports': [{'containerPort': CONTAINER_PORT, 'protocol': 'TCP'}],
+                    'volumeConfig': [
+                        {
+                            'name': 'cache-volume',
+                            'mountPath': CACHE_PATH,
+                            'emptyDir': {'sizeLimit': config['cache_max_size']},
+                        }
+                    ],
+                }
+            ],
+        }
+
+        return pod_spec
+
+    def _make_pod_config(self) -> dict:
+        """Return dict to be used as pod spec's envConfig."""
+        config = self.model.config
+
+        client_max_body_size = '1m'
+        if config.get('client_max_body_size'):
+            client_max_body_size = config.get('client_max_body_size')
+
+        pod_config = {
+            'NGINX_BACKEND': config['backend'],
+            'NGINX_CACHE_INACTIVE_TIME': config.get('cache_inactive_time', '10m'),
+            'NGINX_CACHE_MAX_SIZE': config.get('cache_max_size', '10G'),
+            'NGINX_CACHE_PATH': CACHE_PATH,
+            'NGINX_CACHE_USE_STALE': config['cache_use_stale'],
+            'NGINX_CACHE_VALID': config['cache_valid'],
+            'NGINX_CLIENT_MAX_BODY_SIZE': client_max_body_size,
+            'NGINX_KEYS_ZONE': self._generate_keys_zone(config['site']),
+            'NGINX_SITE_NAME': config['site'],
+        }
+
+        # https://bugs.launchpad.net/juju/+bug/1894782
+        config_fields = {
+            "JUJU_NODE_NAME": "spec.nodeName",
+            "JUJU_POD_NAME": "metadata.name",
+            "JUJU_POD_NAMESPACE": "metadata.namespace",
+            "JUJU_POD_IP": "status.podIP",
+            "JUJU_POD_SERVICE_ACCOUNT": "spec.serviceAccountName",
+        }
+        juju_env_config = {k: {"field": {"path": p, "api-version": "v1"}} for k, p in config_fields.items()}
+        pod_config.update(juju_env_config)
+
+        return pod_config
+
+    def _missing_charm_configs(self) -> list:
+        """Check and return list of required but missing configs."""
+        config = self.model.config
+        missing = [setting for setting in REQUIRED_JUJU_CONFIGS if not config[setting]]
+
+        if config.get('image_username') and not config.get('image_password'):
+            missing.append('image_password')
+        return sorted(missing)
 
 
-if __name__ == "__main__":
-    main(CharmK8SContentCacheCharm)
+if __name__ == '__main__':  # pragma: no cover
+    main(ContentCacheCharm)
diff --git a/tests/test_charm.py b/tests/test_charm.py
new file mode 100644
index 0000000..02466ed
--- /dev/null
+++ b/tests/test_charm.py
@@ -0,0 +1,422 @@
+# Copyright (C) 2020 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+import copy
+import unittest
+from unittest import mock
+
+from ops.model import (
+    ActiveStatus,
+    BlockedStatus,
+    MaintenanceStatus,
+)
+from ops.testing import Harness
+from charm import ContentCacheCharm
+
+BASE_CONFIG = {
+    'image_path': 'localhost:32000/myimage:latest',
+    'site': 'mysite.local',
+    'backend': 'http://mybackend.local:80',
+    'cache_max_size': '10G',
+    'cache_use_stale': 'error timeout updating http_500 http_502 http_503 http_504',
+    'cache_valid': '200 1h',
+}
+CACHE_PATH = '/var/lib/nginx/proxy/cache'
+CONTAINER_PORT = 80
+JUJU_ENV_CONFIG = {
+    'JUJU_NODE_NAME': {'field': {'api-version': 'v1', 'path': 'spec.nodeName'}},
+    'JUJU_POD_NAME': {'field': {'api-version': 'v1', 'path': 'metadata.name'}},
+    'JUJU_POD_NAMESPACE': {'field': {'api-version': 'v1', 'path': 'metadata.namespace'}},
+    'JUJU_POD_IP': {'field': {'api-version': 'v1', 'path': 'status.podIP'}},
+    'JUJU_POD_SERVICE_ACCOUNT': {'field': {'api-version': 'v1', 'path': 'spec.serviceAccountName'}},
+}
+K8S_RESOURCES_INGRESS_RULES = {
+    'host': 'mysite.local',
+    'http': {
+        'paths': [
+            {
+                'backend': {'serviceName': 'content-cache', 'servicePort': 80},
+                'path': '/',
+            }
+        ]
+    },
+}
+K8S_RESOURCES_TMPL = {
+    'kubernetesResources': {
+        'ingressResources': [
+            {
+                'annotations': {'nginx.ingress.kubernetes.io/ssl-redirect': 'false'},
+                'name': 'content-cache-ingress',
+                'spec': {
+                    'rules': [K8S_RESOURCES_INGRESS_RULES],
+                },
+            }
+        ]
+    }
+}
+POD_SPEC_TMPL = {
+    'version': 3,
+    'containers': [
+        {
+            'name': 'content-cache',
+            'envConfig': None,
+            'imageDetails': None,
+            'imagePullPolicy': 'Always',
+            'kubernetes': {
+                'livenessProbe': {
+                    'httpGet': {'path': '/', 'port': CONTAINER_PORT},
+                    'initialDelaySeconds': 3,
+                    'periodSeconds': 3,
+                },
+                'readinessProbe': {
+                    'httpGet': {'path': '/', 'port': CONTAINER_PORT},
+                    'initialDelaySeconds': 3,
+                    'periodSeconds': 3,
+                },
+            },
+            'ports': [{'containerPort': CONTAINER_PORT, 'protocol': 'TCP'}],
+            'volumeConfig': None,
+        }
+    ],
+}
+
+
+class TestCharm(unittest.TestCase):
+    def setUp(self):
+        self.maxDiff = None
+        self.harness = Harness(ContentCacheCharm)
+
+    def tearDown(self):
+        # starting from ops 0.8, we also need to do:
+        self.addCleanup(self.harness.cleanup)
+
+    def test_on_start(self):
+        """Test on_start, nothing but setting ActiveStatus."""
+        harness = self.harness
+        action_event = mock.Mock()
+
+        harness.begin()
+        harness.charm._on_start(action_event)
+        self.assertEqual(harness.charm.unit.status, ActiveStatus('Started'))
+
+    @mock.patch('charm.ContentCacheCharm.configure_pod')
+    def test_on_config_changed(self, configure_pod):
+        """Test on_config_changed, ensure configure_pod is called just once."""
+        harness = self.harness
+
+        # Intentionally before harness.begin() to avoid firing leadership events.
+        harness.set_leader(True)
+        harness.begin()
+
+        config = copy.deepcopy(BASE_CONFIG)
+        harness.update_config(config)
+        self.assertEqual(harness.charm.unit.status, MaintenanceStatus('Configuring pod (config-changed)'))
+        configure_pod.assert_called_once()
+
+    @mock.patch('charm.ContentCacheCharm.configure_pod')
+    def test_on_config_changed_not_leader(self, configure_pod):
+        """Test on_config_chaned, not the leader so configure_pod not called."""
+        harness = self.harness
+
+        # Intentionally before harness.begin() to avoid firing leadership events.
+        harness.set_leader(False)
+        harness.begin()
+
+        config = copy.deepcopy(BASE_CONFIG)
+        harness.update_config(config)
+        self.assertNotEqual(harness.charm.unit.status, MaintenanceStatus('Configuring pod (config-changed)'))
+        configure_pod.assert_not_called()
+
+    @mock.patch('charm.ContentCacheCharm.configure_pod')
+    def test_on_leader_elected(self, configure_pod):
+        """Test on_leader_elected, ensure configure_pod is called just once."""
+        harness = self.harness
+
+        harness.begin()
+        # Intentionally after harness.begin() to trigger leadership events.
+        harness.set_leader(True)
+        self.assertEqual(harness.charm.unit.status, MaintenanceStatus('Configuring pod (leader-elected)'))
+        configure_pod.assert_called_once()
+
+    @mock.patch('charm.ContentCacheCharm.configure_pod')
+    def test_on_leader_elected_not_leader(self, configure_pod):
+        """Test on_leader_elected, not the leader so configure_pod is not called."""
+        harness = self.harness
+
+        harness.begin()
+        # Intentionally after harness.begin() to trigger leadership events.
+        harness.set_leader(False)
+        self.assertNotEqual(harness.charm.unit.status, MaintenanceStatus('Configuring pod (leader-elected)'))
+        configure_pod.assert_not_called()
+
+    @mock.patch('charm.ContentCacheCharm.configure_pod')
+    def test_on_upgrade_charm(self, configure_pod):
+        """Test on_upgrade_charm, ensure configure_pod is called just once."""
+        harness = self.harness
+
+        harness.set_leader(True)
+        harness.begin()
+        harness.charm.on.upgrade_charm.emit()
+        self.assertEqual(harness.charm.unit.status, MaintenanceStatus('Configuring pod (upgrade-charm)'))
+        configure_pod.assert_called_once()
+
+    @mock.patch('charm.ContentCacheCharm.configure_pod')
+    def test_on_upgrade_charm_not_leader(self, configure_pod):
+        """Test on_upgrade_charm, not the leader so configure_pod is not called."""
+        harness = self.harness
+
+        harness.set_leader(False)
+        harness.begin()
+        harness.charm.on.upgrade_charm.emit()
+        self.assertNotEqual(harness.charm.unit.status, MaintenanceStatus('Configuring pod (upgrade-charm)'))
+        configure_pod.assert_not_called()
+
+    @mock.patch('charm.ContentCacheCharm._make_pod_spec')
+    def test_configure_pod(self, make_pod_spec):
+        """Test configure_pod, ensure make_pod_spec is called and the generated pod spec is correct."""
+        harness = self.harness
+
+        harness.set_leader(True)
+        harness.begin()
+
+        config = copy.deepcopy(BASE_CONFIG)
+        harness.update_config(config)
+        make_pod_spec.assert_called_once()
+        self.assertEqual(harness.charm.unit.status, ActiveStatus('Ready'))
+        pod_spec = harness.charm._make_pod_spec()
+        k8s_resources = copy.deepcopy(K8S_RESOURCES_TMPL)
+        self.assertEqual(harness.get_pod_spec(), (pod_spec, k8s_resources))
+
+    @mock.patch('charm.ContentCacheCharm._make_pod_spec')
+    def test_configure_pod_missing_configs(self, make_pod_spec):
+        """Test configure_pod, missing configs so ensure make_pod_spec is not called and no pod spec set."""
+        harness = self.harness
+
+        harness.set_leader(True)
+        harness.begin()
+
+        config = copy.deepcopy(BASE_CONFIG)
+        config['site'] = None
+        harness.update_config(config)
+        make_pod_spec.assert_not_called()
+        self.assertEqual(harness.charm.unit.status, BlockedStatus('Required config(s) empty: site'))
+        self.assertEqual(harness.get_pod_spec(), None)
+
+    def test_generate_keys_zone(self):
+        """Test generating hashed name for Nginx's cache key zone."""
+        harness = self.harness
+
+        harness.disable_hooks()
+        harness.begin()
+
+        expected = '39c631ffb52d-cache'
+        self.assertEqual(harness.charm._generate_keys_zone('mysite.local'), expected)
+        expected = '8b79f9e4b3e8-cache'
+        self.assertEqual(harness.charm._generate_keys_zone('my-really-really-really-long-site-name.local'), expected)
+        expected = 'd41d8cd98f00-cache'
+        self.assertEqual(harness.charm._generate_keys_zone(''), expected)
+
+    def test_make_k8s_ingress_spec(self):
+        """Test generation of K8s ingress spec and ensure it is correct."""
+        harness = self.harness
+
+        harness.disable_hooks()
+        harness.begin()
+
+        config = copy.deepcopy(BASE_CONFIG)
+        harness.update_config(config)
+        k8s_resources = copy.deepcopy(K8S_RESOURCES_TMPL)
+        expected = k8s_resources['kubernetesResources']['ingressResources']
+        self.assertEqual(harness.charm._make_k8s_ingress_spec(), expected)
+
+    def test_make_k8s_ingress_spec_client_max_body_size(self):
+        """Test charm config's client_max_body_size with correct annotation in generated K8s ingress spec."""
+        harness = self.harness
+
+        harness.disable_hooks()
+        harness.begin()
+
+        config = copy.deepcopy(BASE_CONFIG)
+        config['client_max_body_size'] = '32m'
+        harness.update_config(config)
+        k8s_resources = copy.deepcopy(K8S_RESOURCES_TMPL)
+        t = k8s_resources['kubernetesResources']['ingressResources'][0]['annotations']
+        t['nginx.ingress.kubernetes.io/proxy-body-size'] = '32m'
+        expected = k8s_resources['kubernetesResources']['ingressResources']
+        self.assertEqual(harness.charm._make_k8s_ingress_spec(), expected)
+
+    def test_make_k8s_ingress_spec_tls_secrets(self):
+        """Test charm config's tls_secret_name with TLS/SSL enabled in generated K8s ingress spec."""
+        harness = self.harness
+
+        harness.disable_hooks()
+        harness.begin()
+
+        config = copy.deepcopy(BASE_CONFIG)
+        config['tls_secret_name'] = '{}-tls'.format(config['site'])
+        harness.update_config(config)
+        k8s_resources = copy.deepcopy(K8S_RESOURCES_TMPL)
+        t = k8s_resources['kubernetesResources']['ingressResources'][0]
+        t.pop('annotations')
+        t['spec']['tls'] = [{'hosts': 'mysite.local', 'secretName': 'mysite.local-tls'}]
+        expected = k8s_resources['kubernetesResources']['ingressResources']
+        self.assertEqual(harness.charm._make_k8s_ingress_spec(), expected)
+
+    def test_make_pod_spec(self):
+        """Test make_pod_spec, ensure correct spec and is applied and returned by operator's get_pod_spec."""
+        harness = self.harness
+
+        harness.set_leader(True)
+        harness.begin()
+
+        config = copy.deepcopy(BASE_CONFIG)
+        harness.update_config(config)
+        spec = copy.deepcopy(POD_SPEC_TMPL)
+        t = spec['containers'][0]
+        t['envConfig'] = harness.charm._make_pod_config()
+        t['imageDetails'] = {'imagePath': 'localhost:32000/myimage:latest'}
+        t['volumeConfig'] = [
+            {'name': 'cache-volume', 'mountPath': '/var/lib/nginx/proxy/cache', 'emptyDir': {'sizeLimit': '10G'}}
+        ]
+        k8s_resources = copy.deepcopy(K8S_RESOURCES_TMPL)
+        expected = (spec, k8s_resources)
+        self.assertEqual(harness.get_pod_spec(), expected)
+
+    def test_make_pod_spec_image_username(self):
+        """Test charm config image_username/image_password, ensure correct pod spec with details to pod image."""
+        harness = self.harness
+
+        harness.set_leader(True)
+        harness.begin()
+
+        config = copy.deepcopy(BASE_CONFIG)
+        config['image_username'] = 'myuser'
+        config['image_password'] = 'mypassword'
+        harness.update_config(config)
+        spec = copy.deepcopy(POD_SPEC_TMPL)
+        t = spec['containers'][0]
+        t['envConfig'] = harness.charm._make_pod_config()
+        t['imageDetails'] = {
+            'imagePath': 'localhost:32000/myimage:latest',
+            'username': 'myuser',
+            'password': 'mypassword',
+        }
+        t['volumeConfig'] = [
+            {'name': 'cache-volume', 'mountPath': '/var/lib/nginx/proxy/cache', 'emptyDir': {'sizeLimit': '10G'}}
+        ]
+        k8s_resources = copy.deepcopy(K8S_RESOURCES_TMPL)
+        expected = (spec, k8s_resources)
+        self.assertEqual(harness.get_pod_spec(), expected)
+
+    def test_make_pod_spec_cache_max_size(self):
+        """Test charm config's cache_max_size, ensure correct pod spec utilising volumeConfig with size limit."""
+        harness = self.harness
+
+        harness.set_leader(True)
+        harness.begin()
+
+        config = copy.deepcopy(BASE_CONFIG)
+        config['cache_max_size'] = '201G'
+        harness.update_config(config)
+        spec = copy.deepcopy(POD_SPEC_TMPL)
+        t = spec['containers'][0]
+        t['envConfig'] = harness.charm._make_pod_config()
+        t['imageDetails'] = {'imagePath': 'localhost:32000/myimage:latest'}
+        t['volumeConfig'] = [
+            {'name': 'cache-volume', 'mountPath': '/var/lib/nginx/proxy/cache', 'emptyDir': {'sizeLimit': '201G'}}
+        ]
+        k8s_resources = copy.deepcopy(K8S_RESOURCES_TMPL)
+        expected = (spec, k8s_resources)
+        self.assertEqual(harness.get_pod_spec(), expected)
+
+    def test_make_pod_config(self):
+        """Test make_pod_config, ensure envConfig returned is correct."""
+        harness = self.harness
+
+        harness.disable_hooks()
+        harness.begin()
+
+        config = copy.deepcopy(BASE_CONFIG)
+        harness.update_config(config)
+        expected = {
+            'NGINX_BACKEND': 'http://mybackend.local:80',
+            'NGINX_CACHE_INACTIVE_TIME': '10m',
+            'NGINX_CACHE_MAX_SIZE': '10G',
+            'NGINX_CACHE_PATH': CACHE_PATH,
+            'NGINX_CACHE_USE_STALE': 'error timeout updating http_500 http_502 http_503 http_504',
+            'NGINX_CACHE_VALID': '200 1h',
+            'NGINX_CLIENT_MAX_BODY_SIZE': '1m',
+            'NGINX_KEYS_ZONE': '39c631ffb52d-cache',
+            'NGINX_SITE_NAME': 'mysite.local',
+        }
+        expected.update(JUJU_ENV_CONFIG)
+        self.assertEqual(harness.charm._make_pod_config(), expected)
+
+    def test_make_pod_config_client_max_body_size(self):
+        """Test make_pod_config with charm config client_max_body_size, ensure envConfig returned is correct."""
+        harness = self.harness
+
+        harness.disable_hooks()
+        harness.begin()
+
+        config = copy.deepcopy(BASE_CONFIG)
+        config['client_max_body_size'] = '50m'
+        harness.update_config(config)
+        expected = {
+            'NGINX_BACKEND': 'http://mybackend.local:80',
+            'NGINX_CACHE_INACTIVE_TIME': '10m',
+            'NGINX_CACHE_MAX_SIZE': '10G',
+            'NGINX_CACHE_PATH': CACHE_PATH,
+            'NGINX_CACHE_USE_STALE': 'error timeout updating http_500 http_502 http_503 http_504',
+            'NGINX_CACHE_VALID': '200 1h',
+            'NGINX_CLIENT_MAX_BODY_SIZE': '50m',
+            'NGINX_KEYS_ZONE': '39c631ffb52d-cache',
+            'NGINX_SITE_NAME': 'mysite.local',
+        }
+        expected.update(JUJU_ENV_CONFIG)
+        self.assertEqual(harness.charm._make_pod_config(), expected)
+
+    def test_missing_charm_configs(self):
+        """Test missing_charm_config, ensure required configs present and return those missing."""
+        harness = self.harness
+
+        harness.disable_hooks()
+        harness.begin()
+
+        # None missing.
+        config = copy.deepcopy(BASE_CONFIG)
+        harness.update_config(config)
+        expected = []
+        self.assertEqual(harness.charm._missing_charm_configs(), expected)
+
+        # One missing.
+        config = copy.deepcopy(BASE_CONFIG)
+        config['site'] = None
+        harness.update_config(config)
+        expected = ['site']
+        self.assertEqual(harness.charm._missing_charm_configs(), expected)
+
+        # More than one missing.
+        config = copy.deepcopy(BASE_CONFIG)
+        config['image_path'] = None
+        config['site'] = None
+        harness.update_config(config)
+        expected = ['image_path', 'site']
+        self.assertEqual(harness.charm._missing_charm_configs(), expected)
+
+        # All missing, should be sorted.
+        config = copy.deepcopy(BASE_CONFIG)
+        config['image_path'] = None
+        config['image_username'] = 'myuser'
+        config['backend'] = None
+        config['site'] = None
+        harness.update_config(config)
+        expected = ['backend', 'image_password', 'image_path', 'site']
+        self.assertEqual(harness.charm._missing_charm_configs(), expected)
+
+        # image_password missing when image_username present.
+        config = copy.deepcopy(BASE_CONFIG)
+        config['image_username'] = 'myuser'
+        harness.update_config(config)
+        expected = ['image_password']
+        self.assertEqual(harness.charm._missing_charm_configs(), expected)
diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
deleted file mode 100644
index 2f3db20..0000000
--- a/tests/unit/test_charm.py
+++ /dev/null
@@ -1,36 +0,0 @@
-# Copyright 2020 hloeung
-# See LICENSE file for licensing details.
-
-import unittest
-from unittest.mock import Mock
-
-from ops.testing import Harness
-from charm import CharmK8SContentCacheCharm
-
-
-class TestCharm(unittest.TestCase):
-    def test_config_changed(self):
-        harness = Harness(CharmK8SContentCacheCharm)
-        # from 0.8 you should also do:
-        # 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(CharmK8SContentCacheCharm)
-        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(CharmK8SContentCacheCharm)
-        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",)])