Merge ~mthaddon/charm-k8s-openldap/+git/charm-k8s-openldap:oci-resource into charm-k8s-openldap:master

[Tom Haddon]

Switch to using the oci-image resource over static config for docker image

Switch to using the oci-image resource over static config for docker image

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

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

[David Lawson (deej)]

LGTM

review: Approve

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

Change successfully merged at revision 50ad4ca45e1f299015a380c83c33ad24b9921556

diff --git a/.gitignore b/.gitignore
index 490cc43..aa250e5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,5 +1,6 @@
 *~
 *.charm
+*.swp
 .tox
 .coverage
 __pycache__
diff --git a/config.yaml b/config.yaml
index c442062..899ebfd 100644
--- a/config.yaml
+++ b/config.yaml
@@ -1,20 +1,10 @@
 # Copyright 2020 Canonical Ltd.
 # See LICENSE file for licensing details.
 options:
-    image_path:
-        type: string
+    container_port:
+        type: int
         description: |
-          The location of the image to use.
+          The port to use for the container, which translates to k8s 'containerPort' and 'ReadinessProbe'.
 
           This setting is required.
-        default: 'openldapcharmers/openldap:edge'
-    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: ''
+        default: 389
diff --git a/metadata.yaml b/metadata.yaml
index 39ef633..0cf8dc9 100644
--- a/metadata.yaml
+++ b/metadata.yaml
@@ -13,3 +13,9 @@ requires:
   db:
     interface: pgsql
     limit: 1
+resources:
+  openldap-image:
+    type: oci-image
+    description: docker image for OpenLDAP
+    auto-fetch: true
+    upstream-source: 'openldapcharmers/openldap:2.4.50'
diff --git a/requirements.txt b/requirements.txt
index 12aa474..1326f30 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,3 +1,4 @@
 charmhelpers
 ops
 ops-lib-pgsql
+https://github.com/juju-solutions/resource-oci-image/archive/master.zip
diff --git a/src/charm.py b/src/charm.py
index f8d5489..cab7f6c 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -5,6 +5,7 @@
 import logging
 
 from charmhelpers.core import host
+from oci_image import OCIImageResource, OCIImageResourceError
 import ops.lib
 from ops.charm import (
     CharmBase,
@@ -18,6 +19,7 @@ from ops.framework import (
 )
 from ops.model import (
     ActiveStatus,
+    BlockedStatus,
     MaintenanceStatus,
     WaitingStatus,
 )
@@ -50,6 +52,8 @@ class OpenLDAPK8sCharm(CharmBase):
     def __init__(self, *args):
         super().__init__(*args)
 
+        self.image = OCIImageResource(self, 'openldap-image')
+
         self.leader_data = LeadershipSettings()
 
         self.framework.observe(self.on.start, self._configure_pod)
@@ -98,12 +102,16 @@ class OpenLDAPK8sCharm(CharmBase):
 
     def _make_pod_spec(self):
         """Return a pod spec with some core configuration."""
+        # get image details using OCI image helper library
+        try:
+            image_details = self.image.fetch()
+            logging.info("using imageDetails: {}")
+        except OCIImageResourceError:
+            logging.exception('An error occurred while fetching the image info')
+            self.unit.status = BlockedStatus('Error fetching image information')
+            return {}
+
         config = self.model.config
-        image_details = {
-            'imagePath': config['image_path'],
-        }
-        if config['image_username']:
-            image_details.update({'username': config['image_username'], 'password': config['image_password']})
         pod_config = self._make_pod_config()
 
         return {
@@ -112,10 +120,10 @@ class OpenLDAPK8sCharm(CharmBase):
                 {
                     'name': self.app.name,
                     'imageDetails': image_details,
-                    'ports': [{'containerPort': 389, 'protocol': 'TCP'}],
+                    'ports': [{'containerPort': config['container_port'], 'protocol': 'TCP'}],
                     'envConfig': pod_config,
                     'kubernetes': {
-                        'readinessProbe': {'tcpSocket': {'port': 389}},
+                        'readinessProbe': {'tcpSocket': {'port': config['container_port']}},
                     },
                 }
             ],
diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
index 00aba40..5d8df25 100644
--- a/tests/unit/test_charm.py
+++ b/tests/unit/test_charm.py
@@ -14,30 +14,6 @@ from ops.model import (
 
 from unittest.mock import MagicMock
 
-CONFIG_ALL = {
-    'image_path': 'example.com/openldap:latest',
-    'image_username': 'image_user',
-    'image_password': 'image_pass',
-}
-
-CONFIG_IMAGE_NO_CREDS = {
-    'image_path': 'example.com/openldap:latest',
-    'image_username': '',
-    'image_password': '',
-}
-
-CONFIG_IMAGE_NO_IMAGE = {
-    'image_path': '',
-    'image_username': '',
-    'image_password': '',
-}
-
-CONFIG_IMAGE_NO_PASSWORD = {
-    'image_path': 'example.com/openldap:latest',
-    'image_username': 'production',
-    'image_password': '',
-}
-
 DB_URI = {
     'dbname': 'openldap',
     'user': 'ldap_user',
@@ -52,10 +28,10 @@ class TestOpenLDAPK8sCharmHooksDisabled(unittest.TestCase):
         self.harness = testing.Harness(OpenLDAPK8sCharm)
         self.harness.begin()
         self.harness.disable_hooks()
+        self.harness.add_oci_resource('openldap-image')
 
     def test_make_pod_config(self):
         """Make basic, correct pod config."""
-        self.harness.update_config(CONFIG_IMAGE_NO_CREDS)
         self.harness.charm._state.postgres = DB_URI
         expected = {
             'POSTGRES_NAME': 'openldap',
@@ -71,32 +47,6 @@ class TestOpenLDAPK8sCharmHooksDisabled(unittest.TestCase):
 
     def test_make_pod_spec(self):
         """Basic, correct pod spec."""
-        self.harness.update_config(CONFIG_ALL)
-        self.harness.charm._state.postgres = DB_URI
-        with patch.object(self.harness.charm, "get_admin_password") as get_admin_password:
-            get_admin_password.return_value = 'badmin_password'
-            expected = {
-                'version': 3,
-                'containers': [
-                    {
-                        'name': 'openldap',
-                        'imageDetails': {
-                            'imagePath': 'example.com/openldap:latest',
-                            'username': 'image_user',
-                            'password': 'image_pass',
-                        },
-                        'ports': [{'containerPort': 389, 'protocol': 'TCP'}],
-                        'envConfig': self.harness.charm._make_pod_config(),
-                        'kubernetes': {
-                            'readinessProbe': {'tcpSocket': {'port': 389}},
-                        },
-                    }
-                ],
-            }
-            self.assertEqual(self.harness.charm._make_pod_spec(), expected)
-
-    def test_make_pod_spec_no_image_creds(self):
-        self.harness.update_config(CONFIG_IMAGE_NO_CREDS)
         self.harness.charm._state.postgres = DB_URI
         with patch.object(self.harness.charm, "get_admin_password") as get_admin_password:
             get_admin_password.return_value = 'badmin_password'
@@ -106,7 +56,9 @@ class TestOpenLDAPK8sCharmHooksDisabled(unittest.TestCase):
                     {
                         'name': 'openldap',
                         'imageDetails': {
-                            'imagePath': 'example.com/openldap:latest',
+                            'imagePath': 'registrypath',
+                            'username': 'username',
+                            'password': 'password',
                         },
                         'ports': [{'containerPort': 389, 'protocol': 'TCP'}],
                         'envConfig': self.harness.charm._make_pod_config(),
@@ -122,7 +74,6 @@ class TestOpenLDAPK8sCharmHooksDisabled(unittest.TestCase):
         """Check that we block correctly without a Postgres relation."""
         mock_event = MagicMock()
 
-        self.harness.update_config(CONFIG_ALL)
         expected = WaitingStatus('Waiting for database relation')
         self.harness.charm._configure_pod(mock_event)
         self.assertEqual(self.harness.charm.unit.status, expected)
@@ -131,7 +82,6 @@ class TestOpenLDAPK8sCharmHooksDisabled(unittest.TestCase):
         """Test pod config as a non-leader."""
         mock_event = MagicMock()
 
-        self.harness.update_config(CONFIG_ALL)
         self.harness.charm._state.postgres = DB_URI
         expected = ActiveStatus()
         self.harness.charm._configure_pod(mock_event)
@@ -141,7 +91,6 @@ class TestOpenLDAPK8sCharmHooksDisabled(unittest.TestCase):
         """Test pod configuration with everything working appropriately."""
         mock_event = MagicMock()
 
-        self.harness.update_config(CONFIG_ALL)
         self.harness.charm._state.postgres = DB_URI
         self.harness.set_leader(True)
         expected = ActiveStatus()
@@ -153,7 +102,6 @@ class TestOpenLDAPK8sCharmHooksDisabled(unittest.TestCase):
     def test_on_database_relation_joined(self):
         mock_event = MagicMock()
 
-        self.harness.update_config(CONFIG_ALL)
         self.harness.set_leader(True)
         expected = "openldap"
         self.harness.charm._on_database_relation_joined(mock_event)
@@ -162,7 +110,6 @@ class TestOpenLDAPK8sCharmHooksDisabled(unittest.TestCase):
     def test_on_master_changed(self):
         mock_event = MagicMock()
 
-        self.harness.update_config(CONFIG_ALL)
         self.harness.set_leader(True)
         master = namedtuple('master', ['dbname', 'user', 'password', 'host', 'port'])
         master.dbname = "openldap"
@@ -181,7 +128,6 @@ class TestOpenLDAPK8sCharmHooksDisabled(unittest.TestCase):
     def test_on_get_admin_password_action(self):
         mock_event = MagicMock()
 
-        self.harness.update_config(CONFIG_ALL)
         with patch.object(self.harness.charm, "get_admin_password") as get_admin_password:
             get_admin_password.return_value = 'badmin_password'
             self.harness.charm._on_get_admin_password_action(mock_event)