diff --git a/.vscode/tasks.json b/.vscode/tasks.json new file mode 100644 index 0000000..f0b6d52 --- /dev/null +++ b/.vscode/tasks.json @@ -0,0 +1,12 @@ +{ + // See https://go.microsoft.com/fwlink/?LinkId=733558 + // for the documentation about the tasks.json format + "version": "2.0.0", + "tasks": [ + { + "label": "Charm build", + "type": "shell", + "command": "charmcraft pack" + } + ] +} \ No newline at end of file diff --git a/config.yaml b/config.yaml index 70ebe53..9ee4775 100644 --- a/config.yaml +++ b/config.yaml @@ -1,4 +1,23 @@ options: + install_sources: + type: string + default: "deb https://repo.infinidat.com/packages/main-stable/apt/linux-ubuntu {distrib_codename} main" + description: | + Optional configuration to support use of additional sources such as: + - ppa:myteam/ppa + - cloud:trusty-proposed/kilo + - http://my.archive.com/ubuntu main + The last option should be used in conjunction with the key configuration + option. See https://repo.infinidat.com/home/main-stable for details. + The charm also supports templating of the distribution codename via + automatic expansion of {distrib_codename} depending on the host system + version. + install_keys: + type: string + default: + description: | + Key ID to import to the apt keyring to support use with arbitary source + configuration from outside of Launchpad archives or PPA's. use-multipath: type: boolean default: True @@ -8,9 +27,9 @@ options: "use-multipath" option in the nova-compute charm. protocol: type: string - default: + default: iscsi description: | - SAN protocol to use. Choose between iscsi or fc. + SAN protocol to use. Choose between "iscsi" or "fc". volume-backend-name: type: string description: | @@ -20,3 +39,64 @@ options: A common backend name can be set to multiple backends with the same characters so that those can be treated as a single virtual backend associated with a single volume type. + infinibox-ip: + type: string + description: | + Management VIP address of the Infinibox. + default: !!null + infinibox-login: + type: string + description: | + The username for management api on the Infinibox. + default: !!null + infinibox-password: + type: string + description: | + The password for management api on the Infinibox. + default: !!null + iscsi-netspaces: + type: string + description: Comma seperated list of iSCSI netspaces to use. + default: !!null "" + use-chap: + type: boolean + description: | + Choose whether to use CHAP authentication for iSCSI. + default: !!bool "false" + chap-username: + type: string + description: | + Username for CHAP authentication. + default: !!null "" + chap-password: + type: string + description: | + Password for CHAP authentication. + default: !!null "" + pool-name: + type: string + description: | + Storage pool on the Infinibox from which Cinder allocates volumes. + The pool must exist, otherwise Cinder would fail. + default: !!null "" + use-compression: + type: boolean + description: | + Choose whether to use compression on volumes. + Volume compression is available on InfiniBox 3.0 onward. + default: !!bool "true" + thin-provision: + type: boolean + description: | + Choose whether to allocate thin provisioned volumes. + default: !!bool "true" + infinibox-use-ssl: + type: boolean + description: | + Specifies whether to use SSL for Cinder to Infinibox network communication. + default: !!bool "true" + infinibox-ssl-ca: + type: string + description: | + Optional CA certificate used to verify Infinibox API certificate. + default: !!null diff --git a/osci.yaml b/osci.yaml index 450e4ec..0cb98f8 100644 --- a/osci.yaml +++ b/osci.yaml @@ -4,5 +4,5 @@ - charm-unit-jobs-py39 vars: needs_charm_build: true - charm_build_name: infinidat + charm_build_name: cinder-infinidat build_type: charmcraft diff --git a/src/charm.py b/src/charm.py index 7645a1b..61e0f5e 100755 --- a/src/charm.py +++ b/src/charm.py @@ -14,16 +14,50 @@ # See the License for the specific language governing permissions and # limitations under the License. - from ops_openstack.plugins.classes import CinderStoragePluginCharm -from ops_openstack.core import charm_class, get_charm_class_for_release from ops.main import main +from charmhelpers.core.host import ( + service_start, + service_running, + service_resume, + install_ca_cert, + lsb_release, +) -class CinderCharmBase(CinderStoragePluginCharm): +from charmhelpers.fetch import ( + apt_install, + apt_update, + add_source, +) + +from ops.model import ( + ActiveStatus, + BlockedStatus, +) + +import logging + + +class CinderInfinidatCharm(CinderStoragePluginCharm): + + PACKAGES = ['python3-infinisdk', 'infinishell'] + + MANDATORY_CONFIG = ['infinibox-ip', 'infinibox-login', + 'infinibox-password', 'pool-name', 'protocol'] + + REQUIRED_RELATIONS = ['storage-backend'] + + RESTART_MAP = {'/etc/iscsi/iscsid.conf': ['iscsid']} + + PROTOCOL_VALID_VALUES = ['fc', 'iscsi'] + VOLUME_DRIVER = 'cinder.volume.drivers.infinidat.InfiniboxVolumeDriver' + + DISTRIB_CODENAME_PATTERN = '$codename' + + DEFAULT_REPO_BASEURL = \ + 'https://repo.infinidat.com/packages/main-stable/apt/linux-ubuntu' - PACKAGES = ['cinder-common'] - MANDATORY_CONFIG = ['protocol'] # Overriden from the parent. May be set depending on the charm's properties stateless = True active_active = True @@ -31,29 +65,154 @@ class CinderCharmBase(CinderStoragePluginCharm): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) + self.register_status_check(self.check_mandatory_params) + self.register_status_check(self.check_protocol_valid) + self.register_status_check(self.check_iscsi_netspaces) + + def check_protocol_valid(self): + if self.config.get('protocol').lower() not in \ + self.PROTOCOL_VALID_VALUES: + return BlockedStatus( + "valid values for 'protocol' are " + + ','.join(self.PROTOCOL_VALID_VALUES) + ) + return ActiveStatus() + + def check_iscsi_netspaces(self): + if self.config.get('protocol').lower() == 'iscsi' \ + and not self.config.get('iscsi-netspaces'): + + return BlockedStatus("'iscsi-netspaces' must be set " + "when using 'iscsi' protocol") + return ActiveStatus() + + def _validate_config(self): + for func in self.custom_status_checks: + _result = func() + + if not isinstance(_result, ActiveStatus): + self.unit.status = _result + logging.error(_result.message) + return False + return True + + def check_mandatory_params(self): + """ + Implements more detailed validation of the config params. + Also prevents cinder config from updating if some of the crucial charm + options are not specified. Add more checks that should prevent + cinder.conf from updating. + """ + + missing = [] + for param in self.MANDATORY_CONFIG: + if param not in self.config: + missing.append(param) + if missing: + return BlockedStatus( + 'missing option(s): ' + ','.join(missing) + ) + return ActiveStatus() + + def _install_ca_cert(self): + config = dict(self.framework.model.config) + install_ca_cert(config.get('infinibox-ssl-ca')) + + def _on_config(self, event): + + self._install_ca_cert() + + if not self._validate_config(): + return + + # This is called to trigger set_data() + # for the cinder relation on a config change. + self.on_config(event) + try: + self.install_pkgs() + except Exception as e: + self.unit.status = BlockedStatus(str(e)) + + # See self._stored.is_started in OSBaseCharm + # Without this line here the charm will be stuck in 'waiting' until + # all the config parameters are provided correctly by the user + # completely bypassing 'blocked' status, and we need 'blocked' status + # because human intervention is needed. + + self._stored.is_started = True + self.update_status() + + def on_storage_backend(self, event): + # Prevent broken config from being propagated to Cinder + if self._validate_config(): + super().on_storage_backend(event) + else: + event.defer() + + def install_pkgs(self): + logging.info("Installing packages") + + # we implement $codename expansion here + # see the default value for 'source' in config.yaml + if self.model.config.get('install_sources'): + distrib_codename = lsb_release()['DISTRIB_CODENAME'].lower() + add_source( + self.model.config['install_sources'] + .format(distrib_codename=distrib_codename), + self.model.config.get('install_keys')) + apt_update(fatal=True) + apt_install(self.PACKAGES, fatal=True) + self.update_status() + + def on_install(self, event): + self.install_pkgs() + + # start iscsid if it is not running + # neded for cinder's boot-from-volume + if not service_running('iscsid'): + logging.info('Starting iscsid service') + service_resume('iscsid') + service_start('iscsid') + + self.update_status() + def cinder_configuration(self, config): # Return the configuration to be set by the principal. backend_name = config.get('volume-backend-name', self.framework.model.app.name) - volume_driver = '' + # As per https://docs.openstack.org/cinder/latest/configuration/block-storage/drivers/infinidat-volume-driver.html # noqa: E501 + options = [ - ('volume_driver', volume_driver), + ('volume_driver', self.VOLUME_DRIVER), + ('use_multipath_for_image_xfer', config.get('use-multipath')), + ('infinidat_storage_protocol', config.get('protocol')), ('volume_backend_name', backend_name), + ('san_ip', config.get('infinibox-ip')), + ('san_login', config.get('infinibox-login')), + ('san_password', config.get('infinibox-password')), + ('infinidat_iscsi_netspaces', config.get('iscsi-netspaces')), ] - if config.get('use-multipath'): + use_chap_auth = config.get('use-chap', False) + options.extend([ + ('use_chap_auth', use_chap_auth), + ]) + + if use_chap_auth: options.extend([ - ('use_multipath_for_image_xfer', True), - ('enforce_multipath_for_image_xfer', True) + ('chap_username', config.get('chap-username')), + ('chap_password', config.get('chap-password')), ]) + options.extend([ + ('infinidat_pool_name', config.get('pool-name')), + ('infinidat_use_compression', config.get('use-compression')), + ('san_thin_provision', config.get('thin-provision')), + ('infinidat_use_ssl', config.get('infinibox-use-ssl')), + ]) + return options -@charm_class -class CinderInfinidatCharm(CinderCharmBase): - release = 'ussuri' - - if __name__ == '__main__': - main(get_charm_class_for_release()) + main(CinderInfinidatCharm) diff --git a/test-requirements.txt b/test-requirements.txt index e6f71d3..1af9893 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,7 +1,6 @@ # This file is managed centrally. If you find the need to modify this as a # one-off, please don't. Intead, consult #openstack-charms and ask about # requirements management in charms via bot-control. Thank you. -charm-tools>=2.4.4 coverage>=3.6 mock>=1.2 flake8>=4.0.1; python_version >= '3.6' diff --git a/tox.ini b/tox.ini index 83be40a..4d26005 100644 --- a/tox.ini +++ b/tox.ini @@ -64,6 +64,11 @@ 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 diff --git a/unit_tests/test_cinder_infinidat_charm.py b/unit_tests/test_cinder_infinidat_charm.py index 9925f7d..9c9e615 100644 --- a/unit_tests/test_cinder_infinidat_charm.py +++ b/unit_tests/test_cinder_infinidat_charm.py @@ -13,39 +13,242 @@ # limitations under the License. import unittest -from src.charm import CinderCharmBase -from ops.model import ActiveStatus +from src.charm import CinderInfinidatCharm + +from ops.model import ActiveStatus, BlockedStatus from ops.testing import Harness +from unittest import mock + +from charmhelpers.core.host_factory.ubuntu import UBUNTU_RELEASES + +SOURCE = "deb https://repo.infinidat.com/packages/main-stable/apt/linux-ubuntu focal main" # noqa: E501 +KEY = """\ +-----BEGIN PGP PUBLIC KEY BLOCK----- +Version: GnuPG v1.4.11 (GNU/Linux) + +mQENBFESDRIBCADMR7MQMbH4GdCQqfrOMt35MhBwwH4wv9kb1WRSTxa0CmuzYaBB +1nJ0nLaMAwHsEr9CytPWDpMngm/3nt+4F2hJcsOEkQkqeJ31gScJewM+AOUV3DEl +qOeXXYLcP+jUY6pPjlZpOw0p7moUQPXHn+7amVrk7cXGQ8O3B+5a5wjN86LT2hlX +DlBlV5bX/DYluiPUbvQLOknmwO53KpaeDeZc4a8iIOCYWu2ntuAMddBkTps0El5n +JJZMTf6os2ZzngWMZRMDiVJgqVRi2b+8SgFQlQy0cAmne/mpgPrRq0ZMX3DokGG5 +hnIg1mF82laTxd+9qtiOxupzJqf8mncQHdaTABEBAAG0IWFwcF9yZXBvIChDb21t +ZW50KSA8bm9AZW1haWwuY29tPokBOAQTAQIAIgUCURINEgIbLwYLCQgHAwIGFQgC +CQoLBBYCAwECHgECF4AACgkQem2D/j05RYSrcggAsCc4KppV/SZX5XI/CWFXIAXw ++HaNsh2EwYKf9DhtoGbTOuwePvrPGcgFYM3Tu+m+rziPnnFl0bs0xwQyNEVQ9yDw +t465pSgmXwEHbBkoISV1e4WYtZAsnTNne9ieJ49Ob/WY4w3AkdPRK/41UP5Ct6lR +HHRXrSWJYHVq5Rh6BakRuMJyJLz/KvcJAaPkA4U6VrPD7PFtSecMTaONPjGCcomq +b7q84G5ZfeJWb742PWBTS8fJdC+Jd4y5fFdJS9fQwIo52Ff9In2QBpJt5Wdc02SI +fvQnuh37D2P8OcIfMxMfoFXpAMWjrMYc5veyQY1GXD/EOkfjjLne6qWPLfNojA== +=w5Os +-----END PGP PUBLIC KEY BLOCK----- +""" + +PROTOCOL_INVALID_VALUE = 'not_fc_or_iscsi' + class TestCinderInfinidatCharm(unittest.TestCase): def setUp(self): - self.harness = Harness(CinderCharmBase) + self.harness = Harness(CinderInfinidatCharm) self.addCleanup(self.harness.cleanup) self.harness.begin() self.harness.set_leader(True) backend = self.harness.add_relation('storage-backend', 'cinder') - self.harness.update_config({'volume-backend-name': 'test'}) self.harness.add_relation_unit(backend, 'cinder/0') + def _get_partial_config_sample(self): + """ + A config with all mandatory params set + """ + return { + 'infinibox-ip': '123.123.123.123', + 'infinibox-login': 'login', + 'infinibox-password': 'password', + 'pool-name': 'test', + } + + def _get_valid_config_sample(self): + """ + A minimal config that would transition the charm to ActiveState + """ + partial_config = self._get_partial_config_sample() + partial_config.update({ + 'protocol': 'iscsi', + 'iscsi-netspaces': 'A,B', + }) + return partial_config + + def _get_source(self, codename, pocket, baseurl=None): + if baseurl is None: + baseurl = self.harness.charm.DEFAULT_REPO_BASEURL + return ' '.join(( + 'deb', + baseurl, + codename, + pocket)) + def test_cinder_base(self): self.assertEqual( self.harness.framework.model.app.name, 'cinder-infinidat') - # Test that charm is active upon installation. + self.harness.update_config({}) + self.assertTrue(isinstance( - self.harness.model.unit.status, ActiveStatus)) + self.harness.model.unit.status, BlockedStatus)) - def test_multipath_config(self): - self.harness.update_config({'use-multipath': True}) - conf = dict(self.harness.charm.cinder_configuration( - dict(self.harness.model.config))) - self.assertEqual(conf['volume_backend_name'], 'test') - self.assertTrue(conf.get('use_multipath_for_image_xfer')) - self.assertTrue(conf.get('enforce_multipath_for_image_xfer')) + @mock.patch('src.charm.CinderInfinidatCharm.install_pkgs') + @mock.patch('src.charm.service_running') + @mock.patch('src.charm.add_source') + @mock.patch('src.charm.apt_update') + @mock.patch('src.charm.apt_install') + @mock.patch('ops_openstack.core.os_utils.ows_check_services_running') + def test_protocol_config_validation(self, check, apt_install, apt_update, + add_source, service_running, + install_pkgs): + install_pkgs.return_value = None + service_running.return_value = True + check.return_value = None, None - def test_cinder_configuration(self): - # Add check here that configuration is as expected. + add_source.return_value = None + apt_install.return_value = None + apt_update.return_value = None + # Make sure we won't let incorrect value + # for choice list on Cinder's side + + cfg = self._get_partial_config_sample() + cfg.update({'protocol': PROTOCOL_INVALID_VALUE}) + + self.harness.update_config(cfg) + + self.assertTrue(isinstance( + self.harness.model.unit.status, BlockedStatus + )) + + self.harness.update_config({'protocol': 'iscsi'}) + self.assertTrue(isinstance( + self.harness.model.unit.status, BlockedStatus, + )) + + self.harness.update_config({'iscsi-netspaces': 'A,B'}) + self.assertTrue(isinstance( + self.harness.model.unit.status, ActiveStatus + )) + + self.harness.update_config({'protocol': 'fc'}, unset='iscsi-netspaces') + self.assertTrue(isinstance( + self.harness.model.unit.status, ActiveStatus + )) + + def test_cinder_conf_static_params(self): + self.harness.update_config({}) + + config = self.harness.framework.model.config + + self.assertIn( + ('volume_driver', self.harness.charm.VOLUME_DRIVER), + self.harness.charm.cinder_configuration(config) + ) + + def test_cinder_conf_backend_name_param(self): + self.harness.update_config({}) + + config = self.harness.framework.model.config + self.assertIn( + ('volume_backend_name', self.harness.model.app.name), + self.harness.charm.cinder_configuration(config) + ) + + self.harness.update_config({'volume-backend-name': 'overridden'}) + self.assertIn( + ('volume_backend_name', 'overridden'), + self.harness.charm.cinder_configuration(config) + ) + + @mock.patch('src.charm.CinderInfinidatCharm.install_pkgs') + @mock.patch('ops_openstack.core.os_utils.ows_check_services_running') + def test_mandatory_config_params(self, check_services_running, + install_pkgs): + cfg = self._get_valid_config_sample() + check_services_running.return_value = None, None + install_pkgs.return_value = None + + self.harness.update_config(cfg) + + self.assertTrue(isinstance( + self.harness.model.unit.status, ActiveStatus + )) + + for p in self.harness.charm.MANDATORY_CONFIG: + self.harness.update_config(unset=[p]) + # If the MANDATORY_CONFIG param has a default value, + # it won't transition to the BlockedStatus because of + # MANDATORY_CONFIG validation, so we unset it: + if not self.harness.model.config.get(p): + self.assertTrue(isinstance( + self.harness.model.unit.status, BlockedStatus + )) + + self.harness.update_config(cfg) + + self.assertTrue(isinstance( + self.harness.model.unit.status, ActiveStatus + )) + + @mock.patch('src.charm.add_source') + @mock.patch('src.charm.apt_update') + @mock.patch('src.charm.apt_install') + @mock.patch('src.charm.service_running') + @mock.patch('src.charm.service_start') + @mock.patch('src.charm.service_resume') + @mock.patch('src.charm.lsb_release') + def test_repo_management(self, lsb_release, svc_resume, svc_start, + svc_running, apt_install, apt_update, add_source): + + add_source.return_value = None + apt_install.return_value = None + apt_update.return_value = None + svc_resume.return_value = None + svc_start.return_value = None + svc_running.return_value = None + + # we'll need the config the charm considers valid + # in order to test repo management: + cfg = self._get_valid_config_sample() + + dynamic_source = self._get_source('{distrib_codename}', 'main') + + # generate test data for both 'source' values that need substituion + # and for the static ones + + test_data = [] + + for release in UBUNTU_RELEASES: + static_source = self._get_source(release, 'main') + test_data.append( + (dynamic_source, release, + self._get_source(release, 'main')), + ) + test_data.append( + (static_source, release, static_source), + ) + + for i in test_data: + # distro codename the charm runs on + lsb_release.return_value = {'DISTRIB_CODENAME': i[1]} + + # configure to use specific repo version + cfg['install_sources'] = i[0] + cfg['install_keys'] = KEY + + # on_config calls package installation + self.harness.update_config(cfg) + + # make sure the repo management calls were correct + add_source.assert_called_with(i[2], KEY) + apt_install.assert_called_with(self.harness.charm.PACKAGES, + fatal=True) + + def test_enable_ssl(self): pass