diff --git a/Makefile b/Makefile index b806b6f..4955fad 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,7 @@ PYTHON := /usr/bin/python3 PROJECTPATH=$(dir $(realpath $(MAKEFILE_LIST))) ifndef CHARM_BUILD_DIR - CHARM_BUILD_DIR=${PROJECTPATH}.build + CHARM_BUILD_DIR=${PROJECTPATH} endif METADATA_FILE="metadata.yaml" CHARM_NAME=$(shell cat ${PROJECTPATH}/${METADATA_FILE} | grep -E '^name:' | awk '{print $$2}') @@ -16,8 +16,8 @@ help: @echo " make release - run clean and build targets" @echo " make lint - run flake8 and black" @echo " make proof - run charm proof" - @echo " make unit - run the tests defined in the unittest subdirectory" - @echo " make func - run the tests defined in the functional subdirectory" + @echo " make unittests - run the tests defined in the unittest subdirectory" + @echo " make functional - run the tests defined in the functional subdirectory" @echo " make test - run lint, proof, unittests and functional targets" @echo "" @@ -25,19 +25,14 @@ clean: @echo "Cleaning files" @git clean -fXd @echo "Cleaning existing build" - @rm -rf ${CHARM_BUILD_DIR}/${CHARM_NAME} + @rm -rf ${CHARM_BUILD_DIR}/${CHARM_NAME}.charm build: - @charmcraft build + @charmcraft pack --verbose + @bash -c ./rename.sh -# bypassing bug https://github.com/canonical/charmcraft/issues/109 -unpack: build - @mkdir -p ${CHARM_BUILD_DIR}/${CHARM_NAME} - @echo "Unpacking built .charm into ${CHARM_BUILD_DIR}/${CHARM_NAME}" - @cd ${CHARM_BUILD_DIR}/${CHARM_NAME} && unzip -q ${CHARM_BUILD_DIR}/${CHARM_NAME}.charm - -release: clean build unpack - @echo "Charm is built at ${CHARM_BUILD_DIR}/${CHARM_NAME}" +release: clean build + @echo "Charm is built at ${CHARM_BUILD_DIR}/${CHARM_NAME}.charm" lint: @echo "Running lint checks" diff --git a/charmcraft.yaml b/charmcraft.yaml new file mode 100644 index 0000000..89df11b --- /dev/null +++ b/charmcraft.yaml @@ -0,0 +1,19 @@ +type: charm +parts: + charm: + charm-python-packages: [setuptools] +bases: + - build-on: + - name: ubuntu + channel: "20.04" + architectures: ["amd64"] + run-on: + - name: ubuntu + channel: "20.04" + architectures: + - amd64 + - name: ubuntu + channel: "18.04" + architectures: + - amd64 + diff --git a/config.yaml b/config.yaml index b0a4840..b77ca5a 100644 --- a/config.yaml +++ b/config.yaml @@ -38,7 +38,7 @@ options: "hostname2": "iqn.yyyy-mm.naming-authority:uniquename2}' multipath-defaults: type: string - default: "{user_friendly_names: yes}" + default: '{"user_friendly_names": "yes"}' description: | In multipath config, sets the defaults configuration. String should be of JSON dictionary format. Double quotes are essential to the correct format of this JSON string. diff --git a/metadata.yaml b/metadata.yaml index 6da3ae4..36e07f4 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -13,9 +13,6 @@ tags: - fibrechannel - fiberchannel subordinate: true -series: - - bionic - - focal requires: host: interface: juju-info diff --git a/rename.sh b/rename.sh new file mode 100755 index 0000000..956a76b --- /dev/null +++ b/rename.sh @@ -0,0 +1,13 @@ +#!/bin/bash +charm=$(grep -E "^name:" metadata.yaml | awk '{print $2}') +echo "renaming ${charm}_*.charm to ${charm}.charm" +echo -n "pwd: " +pwd +ls -al +echo "Removing previous charm if it exists" +if [[ -e "${charm}.charm" ]]; +then + rm "${charm}.charm" +fi +echo "Renaming charm here." +mv ${charm}_*.charm ${charm}.charm diff --git a/src/charm.py b/src/charm.py index 3d3351e..d6d281a 100755 --- a/src/charm.py +++ b/src/charm.py @@ -40,17 +40,6 @@ class StorageConnectorCharm(CharmBase): ISCSI_SERVICES = ['iscsid', 'open-iscsi'] MULTIPATHD_SERVICE = 'multipathd' - # ISCSI_MANDATORY_CONFIG = [ - # 'storage-type', - # 'iscsi-target', - # 'iscsi-port', - # 'multipath-devices' - # ] - # FC_MANDATORY_CONFIG = [ - # 'storage-type', - # 'fc-lun-alias', - # 'multipath-devices' - # ] VALID_STORAGE_TYPES = ["fc", "iscsi"] MANDATORY_CONFIG = { "iscsi": [ @@ -320,7 +309,7 @@ def _multipath_configuration(self, tenv): config = charm_config.get('multipath-' + section) if config: logging.info("Gather information for the multipaths section " + section) - logging.debug("multipath-" + section + " data: " + config) + logging.debug("multipath-" + section + " data: " + str(config)) try: ctxt[section] = json.loads(config) except Exception as e: diff --git a/tests/functional/requirements.txt b/tests/functional/requirements.txt index b7c9112..6f7831a 100644 --- a/tests/functional/requirements.txt +++ b/tests/functional/requirements.txt @@ -1 +1,2 @@ git+https://github.com/openstack-charmers/zaza.git#egg=zaza +git+https://github.com/openstack-charmers/zaza-openstack-tests.git#egg=zaza.openstack diff --git a/tests/functional/storage-connector-func-tests/tests.py b/tests/functional/storage-connector-func-tests/tests.py index c42545e..bc6c7e9 100644 --- a/tests/functional/storage-connector-func-tests/tests.py +++ b/tests/functional/storage-connector-func-tests/tests.py @@ -45,8 +45,9 @@ def configure_iscsi_connector(self): 'iscsi-node-session-auth-password': 'password123', 'iscsi-node-session-auth-username-in': 'iscsi-target', 'iscsi-node-session-auth-password-in': 'secretpass', + 'multipath-devices': '{}' } - zaza.model.set_application_config('iscsi-connector', conf) + zaza.model.set_application_config('storage-connector', conf) def get_unit_full_hostname(self, unit_name): """Retrieve the full hostname of a unit.""" @@ -59,7 +60,22 @@ def test_iscsi_connector(self): """Test iscsi configuration and wait for idle status.""" self.configure_iscsi_connector() logging.info('Wait for idle/ready status...') - zaza.model.wait_for_application_states() + zaza.model.wait_for_application_states( + states={ + "storage-connector": { + "workload-status": "active", + "workload-status-message-prefix": "Unit is ready" + }, + "ubuntu": { + "workload-status": "active", + "workload-status-message-regex": "^$" + }, + "ubuntu-target": { + "workload-status": "active", + "workload-status-message-regex": "^$" + } + } + ) def test_validate_iscsi_session(self): """Validate that the iscsi session is active.""" diff --git a/tests/functional/tests/bundles/bionic.yaml b/tests/functional/tests/bundles/bionic.yaml index 1eadd1f..501be16 100644 --- a/tests/functional/tests/bundles/bionic.yaml +++ b/tests/functional/tests/bundles/bionic.yaml @@ -1,12 +1,11 @@ +local_overlay_enabled: True series: bionic applications: ubuntu-target: - charm: cs:ubuntu + charm: ch:ubuntu num_units: 1 - storage-connector: - charm: #path is defined in overlay because of bug https://github.com/openstack-charmers/zaza/issues/379 ubuntu: - charm: cs:ubuntu + charm: ch:ubuntu num_units: 1 relations: - - 'ubuntu:juju-info' diff --git a/tests/functional/tests/bundles/focal-fc.yaml b/tests/functional/tests/bundles/focal-fc.yaml index d6f3ac9..9411c8c 100644 --- a/tests/functional/tests/bundles/focal-fc.yaml +++ b/tests/functional/tests/bundles/focal-fc.yaml @@ -1,14 +1,14 @@ +local_overlay_enabled: True series: focal applications: storage-connector: - charm: #path is defined in overlay because of bug https://github.com/openstack-charmers/zaza/issues/379 options: storage-type: 'fc' fc-lun-alias: 'data1' multipath-defaults: '{"user_friendly_names":"yes", "find_multipaths":"yes", "polling_interval":"10"}' multipath-devices: '{"vendor":"PURE", "product":"FlashArray", "fast_io_fail_tmo":"10", "path_selector":"queue-length 0", "path_grouping_policy":"group_by_prio", "rr_min_io":"1", "path_checker":"tur", "fast_io_fail_tmo":"1", "dev_loss_tmo":"infinity", "no_path_retry":"5", "failback":"immediate", "prio":"alua", "hardware_handler":"1 alua", "max_sectors_kb":"4096"}' ubuntu: - charm: cs:ubuntu + charm: ch:ubuntu num_units: 1 to: - 0 diff --git a/tests/functional/tests/bundles/focal.yaml b/tests/functional/tests/bundles/focal.yaml index aad2b90..6d8edee 100644 --- a/tests/functional/tests/bundles/focal.yaml +++ b/tests/functional/tests/bundles/focal.yaml @@ -1,12 +1,11 @@ +local_overlay_enabled: True series: focal applications: ubuntu-target: - charm: cs:ubuntu + charm: ch:ubuntu num_units: 1 - storage-connector: - charm: #path is defined in overlay because of bug https://github.com/openstack-charmers/zaza/issues/379 ubuntu: - charm: cs:ubuntu + charm: ch:ubuntu num_units: 1 relations: - - 'ubuntu:juju-info' diff --git a/tests/functional/tests/bundles/overlays/bionic.yaml.j2 b/tests/functional/tests/bundles/overlays/bionic.yaml.j2 deleted file mode 100644 index 552fefb..0000000 --- a/tests/functional/tests/bundles/overlays/bionic.yaml.j2 +++ /dev/null @@ -1,4 +0,0 @@ -applications: - storage-connector: - charm: {{ CHARM_BUILD_DIR }}/{{ CHARM_NAME }}.charm - diff --git a/tests/functional/tests/bundles/overlays/focal-fc.yaml.j2 b/tests/functional/tests/bundles/overlays/focal-fc.yaml.j2 deleted file mode 100644 index 552fefb..0000000 --- a/tests/functional/tests/bundles/overlays/focal-fc.yaml.j2 +++ /dev/null @@ -1,4 +0,0 @@ -applications: - storage-connector: - charm: {{ CHARM_BUILD_DIR }}/{{ CHARM_NAME }}.charm - diff --git a/tests/functional/tests/bundles/overlays/focal.yaml.j2 b/tests/functional/tests/bundles/overlays/focal.yaml.j2 deleted file mode 100644 index 552fefb..0000000 --- a/tests/functional/tests/bundles/overlays/focal.yaml.j2 +++ /dev/null @@ -1,4 +0,0 @@ -applications: - storage-connector: - charm: {{ CHARM_BUILD_DIR }}/{{ CHARM_NAME }}.charm - diff --git a/tests/functional/tests/bundles/overlays/local-charm-overlay.yaml.j2 b/tests/functional/tests/bundles/overlays/local-charm-overlay.yaml.j2 new file mode 100644 index 0000000..d19c84a --- /dev/null +++ b/tests/functional/tests/bundles/overlays/local-charm-overlay.yaml.j2 @@ -0,0 +1,3 @@ +applications: + {{ charm_name }}: + charm: "{{ CHARM_BUILD_DIR }}/{{ charm_name }}.charm" diff --git a/tests/functional/tests/tests.yaml b/tests/functional/tests/tests.yaml index cbc79df..dfdad6f 100644 --- a/tests/functional/tests/tests.yaml +++ b/tests/functional/tests/tests.yaml @@ -17,4 +17,10 @@ tests: target_deploy_status: storage-connector: workload-status: blocked - workload-status-message: "Missing" + workload-status-message-prefix: "Missing" + ubuntu: + workload-status: active + workload-status-message-regex: "^$" + ubuntu-target: + workload-status: active + workload-status-message-regex: "^$" diff --git a/tests/unit/__init__.py b/tests/unit/__init__.py index a2f9161..f4a529f 100644 --- a/tests/unit/__init__.py +++ b/tests/unit/__init__.py @@ -2,9 +2,11 @@ """Init mocking for unit tests.""" import sys +from unittest import mock -import mock +from ops import testing +testing.SIMULATE_CAN_CONNECT = True sys.path.append('src') diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 86eeeec..6a33054 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -3,63 +3,70 @@ import os import shutil -import subprocess import tempfile import unittest from pathlib import Path -from unittest.mock import Mock, create_autospec, patch +from unittest.mock import PropertyMock, call, mock_open, patch -from charm import StorageConnectorCharm +import charm from ops.framework import EventBase from ops.testing import Harness -from src import charm - class TestCharm(unittest.TestCase): """Charm Unit Tests.""" - subprocess_mock = create_autospec(subprocess.check_call, return_value='True') - subprocess.check_call = subprocess_mock - def setUp(self): """Test setup.""" - self.tempdir = tempfile.mkdtemp() - self.harness = Harness(StorageConnectorCharm) + self.tmp_iscsi_conf_path = Path(tempfile.mkdtemp()) + self.tmp_multipath_conf_dir = Path(tempfile.mkdtemp()) + self.harness = Harness(charm.StorageConnectorCharm) self.harness.set_leader(is_leader=True) self.harness.begin() def tearDown(self): """Remove testing artifacts.""" - shutil.rmtree(self.tempdir) + shutil.rmtree(self.tmp_iscsi_conf_path) + shutil.rmtree(self.tmp_multipath_conf_dir) - def test__init__works_without_a_hitch(self): - """Test init.""" - - def test_abort_if_host_is_container(self): + @patch("charm.utils.is_container") + def test_abort_if_host_is_container(self, m_is_container): """Test if charm stops when deployed on a container.""" - charm.utils.is_container = Mock(return_value=True) + m_is_container.return_value = True + self.assertFalse(self.harness.charm._stored.installed) + self.harness.charm.on.install.emit() self.assertFalse(self.harness.charm._stored.installed) - @patch("charm.StorageConnectorCharm._iscsi_initiator") + @patch("charm.subprocess.check_call") + @patch("charm.subprocess.getoutput") @patch("charm.utils.is_container") - @patch("charm.StorageConnectorCharm.MULTIPATH_CONF_PATH") - @patch("charm.StorageConnectorCharm.MULTIPATH_CONF_DIR") - @patch("charm.StorageConnectorCharm.ISCSI_INITIATOR_NAME") - @patch("charm.StorageConnectorCharm.ISCSI_CONF") - @patch("charm.StorageConnectorCharm.ISCSI_CONF_PATH") - def test_on_iscsi_install(self, iscsi_conf_path, iscsi_conf, iscsi_initiator_name, - multipath_conf_dir, multipath_conf_path, - is_container, iscsi_initiator): + @patch("charm.StorageConnectorCharm.MULTIPATH_CONF_PATH", new_callable=PropertyMock) + @patch("charm.StorageConnectorCharm.MULTIPATH_CONF_DIR", new_callable=PropertyMock) + @patch( + "charm.StorageConnectorCharm.ISCSI_INITIATOR_NAME", new_callable=PropertyMock + ) + @patch("charm.StorageConnectorCharm.ISCSI_CONF", new_callable=PropertyMock) + @patch("charm.StorageConnectorCharm.ISCSI_CONF_PATH", new_callable=PropertyMock) + def test_on_iscsi_install(self, + m_iscsi_conf_path, + m_iscsi_conf, + m_iscsi_initiator_name, + m_multipath_conf_dir, + m_multipath_conf_path, + m_is_container, + m_getoutput, + m_check_call): """Test installation.""" - is_container.return_value = False - iscsi_conf_path.return_value = Path(tempfile.mkdtemp()) - iscsi_conf.return_value = iscsi_conf_path / 'iscsid.conf' - iscsi_initiator_name.return_value = iscsi_conf / 'initiatorname.iscsi' - multipath_conf_dir.return_value = Path(tempfile.mkdtemp()) - multipath_conf_path.return_value = multipath_conf_dir / 'conf.d' - iscsi_initiator.return_value = None + m_is_container.return_value = False + m_iscsi_conf_path.return_value = self.tmp_iscsi_conf_path + m_iscsi_conf.return_value = self.tmp_iscsi_conf_path / 'iscsid.conf' + m_iscsi_initiator_name.return_value = \ + self.tmp_iscsi_conf_path / 'initiatorname.iscsi' + m_multipath_conf_dir.return_value = self.tmp_multipath_conf_dir + m_multipath_conf_path.return_value = self.tmp_multipath_conf_dir / 'conf.d' + m_getoutput.return_value = "iqn.2020-07.canonical.com:lun1" + self.harness.update_config({ "storage-type": "iscsi", "iscsi-target": "abc", @@ -72,17 +79,40 @@ def test_on_iscsi_install(self, iscsi_conf_path, iscsi_conf, iscsi_initiator_nam self.assertTrue(os.path.exists(self.harness.charm.ISCSI_CONF)) self.assertTrue(os.path.exists(self.harness.charm.MULTIPATH_CONF_PATH)) + m_getoutput.assert_called_with("/sbin/iscsi-iname") + m_check_call.assert_has_calls( + [ + call("systemctl restart iscsid".split()), + call("systemctl restart open-iscsi".split()), + call("iscsiadm -m discovery -t sendtargets -p abc:443".split()), + call("iscsiadm -m node --login".split()) + ], + any_order=False + ) self.assertTrue(self.harness.charm._stored.installed) + @patch("charm.subprocess.getoutput") + @patch("builtins.open", new_callable=mock_open) @patch("charm.utils.is_container") - @patch("charm.StorageConnectorCharm.MULTIPATH_CONF_PATH") - @patch("charm.StorageConnectorCharm.MULTIPATH_CONF_DIR") - def test_on_fiberchannel_install(self, multipath_conf_dir, - multipath_conf_path, is_container): + @patch("charm.StorageConnectorCharm.MULTIPATH_CONF_PATH", new_callable=PropertyMock) + @patch("charm.StorageConnectorCharm.MULTIPATH_CONF_DIR", new_callable=PropertyMock) + @patch("charm.StorageConnectorCharm.ISCSI_CONF", new_callable=PropertyMock) + @patch("charm.StorageConnectorCharm.ISCSI_CONF_PATH", new_callable=PropertyMock) + def test_on_fiberchannel_install(self, + m_iscsi_conf_path, + m_iscsi_conf, + m_multipath_conf_dir, + m_multipath_conf_path, + m_is_container, + m_open, + m_getoutput): """Test installation.""" - is_container.return_value = False - multipath_conf_dir.return_value = Path(tempfile.mkdtemp()) - multipath_conf_path.return_value = multipath_conf_dir / 'conf.d' + m_is_container.return_value = False + m_iscsi_conf_path.return_value = self.tmp_iscsi_conf_path + m_iscsi_conf.return_value = self.tmp_iscsi_conf_path / "fc-iscsid.conf" + m_multipath_conf_dir.return_value = self.tmp_multipath_conf_dir + m_multipath_conf_path.return_value = self.tmp_multipath_conf_dir / "conf.d" + m_getoutput.return_value = "host0" self.harness.update_config({ "storage-type": "fc", "fc-lun-alias": "data1", @@ -94,6 +124,10 @@ def test_on_fiberchannel_install(self, multipath_conf_dir, self.assertFalse(os.path.exists(self.harness.charm.ISCSI_CONF)) self.assertTrue(os.path.exists(self.harness.charm.MULTIPATH_CONF_PATH)) self.assertTrue(self.harness.charm._stored.installed) + m_open.assert_called_once_with("/sys/class/scsi_host/host0/scan", "w") + self.assertTrue( + call(self.harness.charm.ISCSI_CONF, "w") not in m_open.mock_calls + ) def test_on_start(self): """Test on start hook.""" @@ -106,16 +140,27 @@ def test_on_start(self): self.harness.charm.on.start.emit() self.assertTrue(self.harness.charm._stored.started) - def test_on_restart_iscsi_services_action(self): + @patch("charm.subprocess.check_call") + def test_on_restart_iscsi_services_action(self, m_check_call): """Test on restart action.""" action_event = FakeActionEvent() + m_check_call.return_value = True self.harness.charm.on_restart_iscsi_services_action(action_event) + m_check_call.assert_has_calls( + [ + call(["systemctl", "restart", "iscsid"]), + call(["systemctl", "restart", "open-iscsi"]) + ], + any_order=False + ) self.assertEqual(action_event.results['success'], 'True') - def test_on_reload_multipathd_service_action(self): + @patch("charm.subprocess.check_call") + def test_on_reload_multipathd_service_action(self, m_check_call): """Test on reload action.""" action_event = FakeActionEvent() self.harness.charm.on_reload_multipathd_service_action(action_event) + m_check_call.assert_called_once_with(["systemctl", "reload", "multipathd"]) self.assertEqual(action_event.results['success'], 'True') diff --git a/tox.ini b/tox.ini index 75cba87..442c9ff 100644 --- a/tox.ini +++ b/tox.ini @@ -8,12 +8,10 @@ skip_missing_interpreters = False basepython = python3 setenv = PYTHONPATH = {toxinidir}/lib/:{toxinidir} - CHARM_BUILD_DIR = {toxinidir}/.build - CHARM_NAME = storage-connector passenv = HOME CHARM_BUILD_DIR - CHARM_NAME + OS_* whitelist_externals = charmcraft [testenv:unit]