Rework charm to use cinder sub plugin

build-requirements.txt

@@ -1 +1,7 @@
-git+https://github.com/canonical/charmcraft.git@0.10.2#egg=charmcraft
+# NOTES(lourot):
+# * We don't install charmcraft via pip anymore because it anyway spins up a
+#   container and scp the system's charmcraft snap inside it. So the charmcraft
+#   snap is necessary on the system anyway.
+# * `tox -e build` successfully validated with charmcraft 1.2.1
+
+cffi==1.14.6; python_version < '3.6'  # cffi 1.15.0 drops support for py35.

charmcraft.yaml

@@ -0,0 +1,32 @@
+type: charm
+parts:
+  charm:
+    after:
+      - update-certificates
+    charm-python-packages:
+      # NOTE(lourot): see
+      # * https://github.com/canonical/charmcraft/issues/551
+      # * https://github.com/canonical/charmcraft/issues/632
+      - setuptools < 58
+    build-packages:
+      - git
+  update-certificates:
+    plugin: nil
+    # See https://github.com/canonical/charmcraft/issues/658
+    override-build: |
+      apt update
+      apt install -y ca-certificates
+      update-ca-certificates      
+bases:
+  - build-on:
+      - name: ubuntu
+        channel: "20.04"
+        architectures:
+          - amd64
+    run-on:
+      - name: ubuntu
+        channel: "20.04"
+        architectures: [amd64, s390x, ppc64el, arm64]
+      - name: ubuntu
+        channel: "22.04"
+        architectures: [amd64, s390x, ppc64el, arm64]

config.yaml

@@ -27,7 +27,7 @@ options:
       raise an exception.
   hpe3par-iscsi-ips:
     type: string
-    default: ''
+    default: 
     description: |
       Comma-separated list of IP:PORT to be used for the iscsi connection.
   hpe3par-iscsi-chap-enabled:
@@ -69,39 +69,39 @@ options:
     type: string
     description: |
       Password for SAN controller for SSH access to the array
-    default: ''
+    default: 
   hpe3par-username:
     type: string
     description: |
       3PAR username with the 'edit' role
-    default: ''
+    default: 
   hpe3par-password:
     type: string
     description: |
       3PAR password for the user specified in hpe3par_username
-    default: ''
+    default: 
   hpe3par-api-url:
     type: string
     description: |
       3PAR WS API Server URL
-    default: ''
+    default: 
   hpe3par-cpg:
     type: string
     description: |
       3PAR CPG to use for volume creation
-    default: ''
+    default: 
   hpe3par_cpg_snap:
     type: string
-    default: ''
+    default: 
     description: |
       Sets the CPG name for the snapshot.
       If empty, use the userCPG will be used.
   hpe3par_target_nsp:
     type: string
-    default: ''
+    default: 
     description: |      
   volume-backend-name:
     type: string
     description: |
       Name to present to cinder, leave blank to use charm (or alias) name
-    default: ''
+    default: 

copyright

@@ -1,7 +1,7 @@
 Format: http://dep.debian.net/deps/dep5/
 
 Files: *
-Copyright: Copyright 2015-2022, Canonical Ltd., All Rights Reserved.
+Copyright: Copyright 2021-2022, Canonical Ltd., All Rights Reserved.
 License: Apache License 2.0
  Licensed under the Apache License, Version 2.0 (the "License");
  you may not use this file except in compliance with the License.

osci.yaml

@@ -3,8 +3,9 @@
       - charm-unit-jobs
     check:
       jobs:
-      - bionic-queens
       - focal-ussuri
+      - focal-yoga
+      - jammy-yoga
     vars:
       needs_charm_build: true
       build_type: charmcraft

rename.sh

@@ -0,0 +1,13 @@
+#!/bin/bash
+charm=$(grep "charm_build_name" osci.yaml | awk '{print $2}')
+echo "renaming ${charm}_*.charm to ${charm}.charm"
+echo -n "pwd: "
+pwd
+ls -al
+echo "Removing bad downloaded charm maybe?"
+if [[ -e "${charm}.charm" ]];
+then
+    rm "${charm}.charm"
+fi
+echo "Renaming charm here."
+mv ${charm}_*.charm ${charm}.charm

src/charm.py

@@ -22,6 +22,8 @@ from ops.main import main
 from ops.framework import StoredState
 from ops.model import MaintenanceStatus, ActiveStatus, BlockedStatus
 
+from ops_openstack.plugins.classes import CinderStoragePluginCharm
+
 from charmhelpers.fetch.ubuntu import apt_install
 
 logger = logging.getLogger(__name__)
@@ -35,23 +37,25 @@ REQUIRED_OPTS = [
     'hpe3par-password',
     'san-ip',
     'san-login',
-    'san-password']
+    'san-password',
+]
 
 # Based on initialize-iscsi-ports() in
 # https://github.com/openstack/cinder/blob/master/cinder/ \
 #     volume/drivers/hpe/hpe-3par-iscsi.py
-REQUIRED_OPTS_ISCSI = [
+REQUIRED_OPTS_ISCSI = ['hpe3par-iscsi-ips']
-    'hpe3par-iscsi-ips']
 
 
 class InvalidConfigError(Exception):
     """Exception raised on invalid configurations."""
 
-    def __init__(self, msg):
+    pass
-        self.msg = msg
 
-    def __str__(self):
+
-        return self.msg
+class MissingConfigError(Exception):
+    """Exception raised on missing 3PAR config parameter."""
+
+    pass
 
 
 def CinderThreeParContext(charm_config, service):
@@ -67,10 +71,7 @@ def CinderThreeParContext(charm_config, service):
     """
     ctxt = []
     for key in charm_config.keys():
-        if key == 'volume-backend-name':
+        ctxt.append((key.replace('-', '_'), charm_config[key]))
-            ctxt.append((key, service))
-        else:
-            ctxt.append((key.replace('-', '_'), charm_config[key]))
     if charm_config['driver-type'] == 'fc':
         ctxt.append((
             'volume_driver',
@@ -81,118 +82,49 @@ def CinderThreeParContext(charm_config, service):
             'cinder.volume.drivers.hpe.hpe_3par_iscsi.HPE3PARISCSIDriver'))
     else:
         raise InvalidConfigError('Invalid config (driver-type)')
-    return {
+    return ctxt
-        "cinder": {
-            "/etc/cinder/cinder.conf": {
-                "sections": {
-                    service: ctxt
-                }
-            }
-        }
-    }
 
 
-class CharmCinderThreeParCharm(CharmBase):
+class CharmCinderThreeParCharm(CinderStoragePluginCharm):
     """Charm the Cinder HPE 3PAR driver."""
 
-    _stored = StoredState()
+    MANDATORY_CONFIG = REQUIRED_OPTS
+    PACKAGES = ['python3-3parclient', 'sysfsutils']
 
-    def __init__(self, *args):
+    def __init__(self, *args, **kwargs):
-        super().__init__(*args)
+        super().__init__(*args, **kwargs)
-        self.framework.observe(
+        self._stored.is_started = True
-            self.on.install,
-            self._on_install)
-        self.framework.observe(
-            self.on.config_changed,
-            self._on_config_changed_or_upgrade)
-        self.framework.observe(
-            self.on.upgrade_charm,
-            self._on_config_changed_or_upgrade)
-        self.framework.observe(
-            self.on.storage_backend_relation_joined,
-            self._on_render_storage_backend)
-        self.framework.observe(
-            self.on.storage_backend_relation_changed,
-            self._on_render_storage_backend)
 
-    def _rel_get_remote_units(self, rel_name):
+    def on_config(self, event):
-        """Get relations remote units"""
+        prev_status = self.unit.status
-        return self.framework.model.get_relation(rel_name).units
-
-    def _on_install(self, _):
-        """Install packages"""
-        self.unit.status = MaintenanceStatus(
-            "Installing packages")
-        # os_brick lib needs systool from sysfsutils to be able to retrieve
-        # the data from FC links:
-        # https://github.com/openstack/os-brick/blob/ \
-        #     1b2e2295421615847d86508dcd487ec51fa45f25/ \
-        #     os_brick/initiator/linuxfc.py#L151
-        apt_install(['python3-3parclient',
-                     'sysfsutils'])
-        self.unit.status = ActiveStatus("Unit is ready")
-
-    def _on_config_changed_or_upgrade(self, event):
-        """Update on changed config or charm upgrade"""
-        svc_name = self.framework.model.app.name
-        # Copying to a new dict as charm_config will be edited according to
-        # the settings
-        charm_config = dict(self.framework.model.config)
-        if not self.check_config(charm_config):
-            # The config checks failed, drop this event as the operator
-            # needs to intervene manually
-            return
-        r = self.framework.model.relations.get('storage-backend')[0]
         try:
-            ctx = CinderThreeParContext(charm_config, svc_name)
+            super().on_config(event)
         except InvalidConfigError as e:
             self.unit.status = BlockedStatus(str(e))
-            return
+        finally:
+            self.unit.status = prev_status
 
-        for u in self._rel_get_remote_units('storage-backend'):
+    def _on_config(self, event):
-            r.data[self.unit]['backend_name'] = \
+        # The list of mandatory config options isn't static for this
-                charm_config['volume-backend-name'] or svc_name
+        # charm, so we need to manually adjust here.
-            r.data[self.unit]['subordinate_configuration'] = json.dumps(ctx)
+        if self.framework.model.config.get('driver-type') == 'iscsi':
-        self.unit.status = ActiveStatus("Unit is ready")
+            self.MANDATORY_CONFIG = REQUIRED_OPTS + REQUIRED_OPTS_ISCSI
+        else:
+            self.MANDATORY_CONFIG = REQUIRED_OPTS
 
-    def _on_render_storage_backend(self, event):
+        super()._on_config(event)
-        """Render the current configuration"""
+
+    def cinder_configuration(self, charm_config):
         svc_name = self.framework.model.app.name
-        charm_config = self.framework.model.config
-        data = event.relation.data
-        data[self.unit]['backend_name'] = \
-            charm_config['volume-backend-name'] or svc_name
-        try:
-            ctx = CinderThreeParContext(charm_config, svc_name)
-        except InvalidConfigError as e:
-            self.unit.status = BlockedStatus(str(e))
-            return
-
-        data[self.unit]['subordinate_configuration'] = json.dumps(ctx)
-
-    def check_config(self, charm_config):
-        """Check whether required options are set."""
         # According to the HPE 3par driver code, expiration and retention can
         # be left unset and won't be configured:
         # https://github.com/openstack/cinder/blob/stable/ussuri/cinder/ \
         #     volume/drivers/hpe/hpe_3par_common.py#L2834
-        for opt in ("hpe3par-snapshot-retention",
+        for opt in ('hpe3par-snapshot-retention',
-                    "hpe3par-snapshot-expiration"):
+                    'hpe3par-snapshot-expiration'):
-            # Setting as < 0 will remove the given option from the request.
             if charm_config.get(opt, -1) < 0:
                 charm_config.pop(opt, None)
-        required_opts = REQUIRED_OPTS
+        return CinderThreeParContext(charm_config, svc_name)
-        charm_config = self.framework.model.config
-        if charm_config['driver-type'] == 'iscsi':
-            required_opts += REQUIRED_OPTS_ISCSI
-        missing_opts = set(required_opts) - set(charm_config.keys())
-        if missing_opts:
-            self.unit.status = BlockedStatus(
-                'Missing options: {}'.format(','.join(missing_opts)))
-            return False
-        else:
-            self.unit.status = MaintenanceStatus("Sharing configs with Cinder")
-        return True
 
 
 if __name__ == "__main__":

tests/bundles/bionic-queens.yaml

@@ -1,49 +0,0 @@
-series: bionic
-comment:
-- 'machines section to decide order of deployment. database sooner = faster'
-machines:
-  '0':
-    constraints: mem=3072M
-  '1':
-  '2':
-    constraints: mem=4G root-disk=16G
-  '3':
-local_overlay_enabled: false
-relations:
-- - keystone:shared-db
-  - mysql:shared-db
-- - cinder:shared-db
-  - mysql:shared-db
-- - cinder:identity-service
-  - keystone:identity-service
-- - cinder:amqp
-  - rabbitmq-server:amqp
-- - cinder:storage-backend
-  - cinder-three-par:storage-backend
-applications:
-  mysql:
-    charm: cs:~openstack-charmers-next/percona-cluster
-    num_units: 1
-    to:
-    - '0'
-  keystone:
-    charm: cs:~openstack-charmers-next/keystone
-    num_units: 1
-    to:
-    - '1'
-  cinder:
-    charm: cs:~openstack-charmers-next/cinder
-    num_units: 1
-    options:
-      block-device: /dev/vdb
-      overwrite: "true"
-      ephemeral-unmount: /mnt
-    to:
-    - '2'
-  cinder-three-par:
-    charm: ../../cinder-three-par.charm
-  rabbitmq-server:
-    charm: cs:~openstack-charmers-next/rabbitmq-server
-    num_units: 1
-    to:
-    - '3'

tests/tests.yaml

@@ -4,11 +4,14 @@ tests:
 configure:
   - zaza.openstack.charm_tests.keystone.setup.add_demo_user
 gate_bundles:
-  - bionic-queens
   - focal-ussuri
+  - focal-yoga
+  - jammy-yoga
 smoke_bundles:
-  - bionic-queens
   - focal-ussuri
+  - focal-yoga
+  - jammy-yoga
 dev_bundles:
-  - bionic-queens
   - focal-ussuri
+  - focal-yoga
+  - jammy-yoga

tox.ini

@@ -1,5 +1,4 @@
 # Operator charm (with zaza): tox.ini
-
 [tox]
 envlist = pep8,py3
 skipsdist = True
@@ -15,11 +14,14 @@ skip_missing_interpreters = False
 # * It is also necessary to pin virtualenv as a newer virtualenv would still
 #   lead to fetching the latest pip in the func* tox targets, see
 #   https://stackoverflow.com/a/38133283
+# * It is necessary to declare setuptools as a dependency otherwise tox will
+#   fail very early at not being able to load it. The version pinning is in
+#   line with `pip.sh`.
 requires = pip < 20.3
            virtualenv < 20.0
+           setuptools < 50.0.0
 # NOTE: https://wiki.canonical.com/engineering/OpenStack/InstallLatestToxOnOsci
 minversion = 3.2.0
-
 [testenv]
 setenv = VIRTUAL_ENV={envdir}
          PYTHONHASHSEED=0
@@ -27,44 +29,47 @@ setenv = VIRTUAL_ENV={envdir}
 install_command =
   pip install {opts} {packages}
 commands = stestr run --slowest {posargs}
-whitelist_externals =
+allowlist_externals =
     git
     add-to-archive.py
     bash
+    charmcraft
+    rename.sh
 passenv = HOME TERM CS_* OS_* TEST_*
 deps = -r{toxinidir}/test-requirements.txt
-
 [testenv:py35]
 basepython = python3.5
 # python3.5 is irrelevant on a focal+ charm.
 commands = /bin/true
-
 [testenv:py36]
 basepython = python3.6
 deps = -r{toxinidir}/requirements.txt
        -r{toxinidir}/test-requirements.txt
-
 [testenv:py37]
 basepython = python3.7
 deps = -r{toxinidir}/requirements.txt
        -r{toxinidir}/test-requirements.txt
-
 [testenv:py38]
 basepython = python3.8
 deps = -r{toxinidir}/requirements.txt
        -r{toxinidir}/test-requirements.txt
-
+[testenv:py39]
+basepython = python3.9
+deps = -r{toxinidir}/requirements.txt
+       -r{toxinidir}/test-requirements.txt
+[testenv:py310]
+basepython = python3.10
+deps = -r{toxinidir}/requirements.txt
+       -r{toxinidir}/test-requirements.txt
 [testenv:py3]
 basepython = python3
 deps = -r{toxinidir}/requirements.txt
        -r{toxinidir}/test-requirements.txt
-
 [testenv:pep8]
 basepython = python3
 deps = -r{toxinidir}/requirements.txt
        -r{toxinidir}/test-requirements.txt
 commands = flake8 {posargs} src unit_tests tests
-
 [testenv:cover]
 # Technique based heavily upon
 # https://github.com/openstack/nova/blob/master/tox.ini
@@ -81,7 +86,6 @@ commands =
     coverage html -d cover
     coverage xml -o cover/coverage.xml
     coverage report
-
 [coverage:run]
 branch = True
 concurrency = multiprocessing
@@ -92,42 +96,37 @@ omit =
     .tox/*
     */charmhelpers/*
     unit_tests/*
-
 [testenv:venv]
 basepython = python3
 commands = {posargs}
-
 [testenv:build]
 basepython = python3
 deps = -r{toxinidir}/build-requirements.txt
 commands =
-    charmcraft build
+    charmcraft clean
-
+    charmcraft -v build
+    {toxinidir}/rename.sh
 [testenv:func-noop]
 basepython = python3
 commands =
     functest-run-suite --help
-
 [testenv:func]
 basepython = python3
 commands =
     functest-run-suite --keep-model
-
 [testenv:func-smoke]
 basepython = python3
 commands =
     functest-run-suite --keep-model --smoke
-
 [testenv:func-dev]
 basepython = python3
 commands =
     functest-run-suite --keep-model --dev
-
 [testenv:func-target]
 basepython = python3
 commands =
     functest-run-suite --keep-model --bundle {posargs}
-
 [flake8]
 # Ignore E902 because the unit_tests directory is missing in the built charm.
 ignore = E402,E226,E902
+

unit_tests/test_charm.py

@@ -16,7 +16,7 @@ import unittest
 import json
 import copy
 
-from ops.model import Relation, BlockedStatus
+from ops.model import Relation, BlockedStatus, ActiveStatus
 from ops.testing import Harness
 from src.charm import CharmCinderThreeParCharm
 
@@ -29,7 +29,6 @@ TEST_3PAR_CONFIG = {
                     ['driver_type', 'fc'],
                     ['use_multipath_image_xfer', False],
                     ['enforce_multipath_for_image_xfer', False],
-                    ['hpe3par_iscsi_ips', ''],
                     ['hpe3par_iscsi_chap_enabled', True],
                     ['hpe3par_snapshot_expiration', 72],
                     ['hpe3par_snapshot_retention', 48],
@@ -37,14 +36,7 @@ TEST_3PAR_CONFIG = {
                     ['reserved_percentage', 15],
                     ['san_ip', '0.0.0.0'],
                     ['san_login', 'some-login'],
-                    ['san_password', ''],
+                    ['volume_backend_name', 'cinder-three-par'],
-                    ['hpe3par_username', ''],
-                    ['hpe3par_password', ''],
-                    ['hpe3par_api_url', ''],
-                    ['hpe3par_cpg', ''],
-                    ['hpe3par_cpg_snap', ''],
-                    ['hpe3par_target_nsp', ''],
-                    ['volume-backend-name', 'cinder-three-par'],
                     ['volume_driver',
                      'cinder.volume.drivers.hpe.hpe_3par_fc.HPE3PARFCDriver']
                 ]
@@ -62,20 +54,14 @@ TEST_3PAR_CONFIG_CHANGED = {
                     ['driver_type', 'fc'],
                     ['use_multipath_image_xfer', False],
                     ['enforce_multipath_for_image_xfer', False],
-                    ['hpe3par_iscsi_ips', ''],
                     ['hpe3par_iscsi_chap_enabled', True],
                     ['max_over_subscription_ratio', 20.0],
                     ['reserved_percentage', 15],
                     ['san_ip', '1.2.3.4'],
                     ['san_login', 'login'],
                     ['san_password', 'pwd'],
-                    ['hpe3par_username', ''],
-                    ['hpe3par_password', ''],
                     ['hpe3par_api_url', 'test.url'],
-                    ['hpe3par_cpg', ''],
+                    ['volume_backend_name', 'cinder-three-par'],
-                    ['hpe3par_cpg_snap', ''],
-                    ['hpe3par_target_nsp', ''],
-                    ['volume-backend-name', 'cinder-three-par'],
                     ['volume_driver',
                      'cinder.volume.drivers.hpe.hpe_3par_fc.HPE3PARFCDriver']
                 ]
@@ -127,13 +113,12 @@ class TestCharm(unittest.TestCase):
 
     def test_blocked_status(self):
         self.harness.update_config(unset=["san-ip", "san-login"])
-        self.harness.charm.on.update_status.emit()
-        self.assertEqual(
-            self.harness.charm.unit.status.message,
-            'Missing options: san-login,san-ip')
         self.assertIsInstance(
             self.harness.charm.unit.status,
             BlockedStatus)
+        message = self.harness.charm.unit.status.message
+        self.assertIn('san-login', message)
+        self.assertIn('san-ip', message)
 
     def test_blocked_unset_retention_expiration(self):
         self.harness.update_config({
@@ -167,5 +152,5 @@ class TestCharm(unittest.TestCase):
 
     def test_invalid_config_driver_type(self):
         self.harness.update_config({'driver-type': '???'})
-        self.assertIsInstance(self.harness.charm.unit.status,
+        self.assertFalse(isinstance(self.harness.charm.unit.status,
-                              BlockedStatus)
+                                    ActiveStatus))