diff --git a/src/actions/actions.py b/src/actions/actions.py index 5784516..8da5fb0 100755 --- a/src/actions/actions.py +++ b/src/actions/actions.py @@ -35,7 +35,7 @@ import charms_openstack.charm as charm import charmhelpers.core as ch_core -import charm.openstack.ironic.clients as clients +import charm.openstack.ironic.api_utils as api_utils charms_openstack.bus.discover() @@ -52,12 +52,12 @@ def set_temp_url_secret(*args): identity_service = reactive.endpoint_from_flag( 'identity-credentials.available') try: - keystone_session = clients.create_keystone_session(identity_service) + keystone_session = api_utils.create_keystone_session(identity_service) except Exception as e: ch_core.hookenv.action_fail('Failed to create keystone session ("{}")' .format(e)) - os_cli = clients.OSClients(keystone_session) + os_cli = api_utils.OSClients(keystone_session) if os_cli.has_swift() is False: ch_core.hookenv.action_fail( 'Swift not yet available. Please wait for deployment to finish') @@ -65,7 +65,7 @@ def set_temp_url_secret(*args): if os_cli.has_glance() is False: ch_core.hookenv.action_fail( 'Glance not yet available. Please wait for deployment to finish') - + if "swift" not in os_cli.glance_stores: ch_core.hookenv.action_fail( 'Glance does not support Swift storage backend. ' diff --git a/src/lib/charm/openstack/ironic/clients.py b/src/lib/charm/openstack/ironic/api_utils.py similarity index 88% rename from src/lib/charm/openstack/ironic/clients.py rename to src/lib/charm/openstack/ironic/api_utils.py index c3905f8..971fd3b 100644 --- a/src/lib/charm/openstack/ironic/clients.py +++ b/src/lib/charm/openstack/ironic/api_utils.py @@ -19,7 +19,7 @@ def create_keystone_session(keystone, verify=True): project_name = keystone.credentials_project() - auth_url = "%s://%s:%s" % ( + auth_url = "%s://%s:%s" % ( keystone.auth_protocol(), keystone.auth_host(), keystone.credentials_port()) @@ -30,7 +30,7 @@ def create_keystone_session(keystone, verify=True): }) keystone_version = keystone.api_version() - if keystone_version and int(keystone_version) == 3: + if keystone_version and str(keystone_version) == "3": plugin_name = "v3" + plugin_name project_domain_name = keystone.credentials_project_domain_name() @@ -60,7 +60,8 @@ class OSClients(object): def _stores_info(self): if self._stores: return self._stores - self._stores = self._img_cli.images.get_stores_info().get( + store = self._img_cli.images.get_stores_info() + self._stores = store.get( "stores", []) return self._stores @@ -79,7 +80,7 @@ class OSClients(object): props = {} for prop, val in acct[0].items(): if prop.startswith('x-account-meta-'): - props[prop.replace("x-account-meta-","")] = val + props[prop.replace("x-account-meta-", "")] = val return props def set_object_account_property(self, prop, value): @@ -101,16 +102,12 @@ class OSClients(object): def _has_service_type(self, svc_type, interface="public"): try: - svc_id = self._ks.services.find(type=svc_type) - except ks_exc.http.NotFound: - return False - - try: + svc = self._ks.services.find(type=svc_type) + print(svc.id) self._ks.endpoints.find( - service_id=svc_id.id, interface=interface) + service_id=svc.id, interface=interface) except ks_exc.http.NotFound: return False - return True def has_swift(self): @@ -118,5 +115,3 @@ class OSClients(object): def has_glance(self): return self._has_service_type("image") - - diff --git a/src/lib/charm/openstack/ironic/controller_utils.py b/src/lib/charm/openstack/ironic/controller_utils.py index 87e7921..98afa7d 100644 --- a/src/lib/charm/openstack/ironic/controller_utils.py +++ b/src/lib/charm/openstack/ironic/controller_utils.py @@ -1,9 +1,7 @@ import os import shutil -from charmhelpers.core.templating import render import charmhelpers.core.host as ch_host -import charmhelpers.fetch as fetch _IRONIC_USER = "ironic" @@ -11,7 +9,7 @@ _IRONIC_GROUP = "ironic" class PXEBootBase(object): - + TFTP_ROOT = "/tftpboot" HTTP_ROOT = "/httpboot" HTTP_SERVER_CONF = "/etc/nginx/nginx.conf" @@ -51,10 +49,10 @@ class PXEBootBase(object): def get_restart_map(self): return { - self.TFTP_CONFIG: [self.TFTPD_SERVICE,], - self.MAP_FILE: [self.TFTPD_SERVICE,], - self.GRUB_CFG: [self.TFTPD_SERVICE,], - self.HTTP_SERVER_CONF: [self.HTTPD_SERVICE_NAME,], + self.TFTP_CONFIG: [self.TFTPD_SERVICE, ], + self.MAP_FILE: [self.TFTPD_SERVICE, ], + self.GRUB_CFG: [self.TFTPD_SERVICE, ], + self.HTTP_SERVER_CONF: [self.HTTPD_SERVICE_NAME, ], } def determine_packages(self): @@ -77,13 +75,13 @@ class PXEBootBase(object): def _ensure_folders(self): if os.path.isdir(self.TFTP_ROOT) is False: os.makedirs(self.TFTP_ROOT) - + if os.path.isdir(self.HTTP_ROOT) is False: os.makedirs(self.HTTP_ROOT) if os.path.isdir(self.GRUB_DIR) is False: os.makedirs(self.GRUB_DIR) - + ch_host.chownr( self.TFTP_ROOT, _IRONIC_USER, _IRONIC_GROUP, chowntopdir=True) ch_host.chownr( @@ -114,11 +112,3 @@ def get_pxe_config_class(charm_config): if series == "bionic": return PXEBootBionic(charm_config) return PXEBootBase(charm_config) - -# TODO: Create keystone session -# TODO: create swift client -# TODO: generate secret -# TODO: set tmp-url-key to secret -# TODO: Config property function that returns the secret from leader data -def set_temp_url_secret(keystone): - pass \ No newline at end of file diff --git a/src/lib/charm/openstack/ironic/ironic.py b/src/lib/charm/openstack/ironic/ironic.py index d4daac9..5b0ef3b 100644 --- a/src/lib/charm/openstack/ironic/ironic.py +++ b/src/lib/charm/openstack/ironic/ironic.py @@ -5,7 +5,6 @@ import os import charms_openstack.charm import charms_openstack.adapters -import charms_openstack.ip as os_ip from charms_openstack.adapters import ( RabbitMQRelationAdapter, DatabaseRelationAdapter, @@ -46,6 +45,10 @@ IRONIC_UTILS_FILTERS = os.path.join( FILTERS_DIR, "ironic-utils.filters") TFTP_CONF = "/etc/default/tftpd-hpa" HTTP_SERVER_CONF = "/etc/nginx/nginx.conf" +VALID_NETWORK_INTERFACES = ["neutron", "flat", "noop"] +VALID_DEPLOY_INTERFACES = ["direct", "iscsi"] +DEFAULT_DEPLOY_IFACE = "flat" +DEFAULT_NET_IFACE = "direct" OPENSTACK_RELEASE_KEY = 'ironic-charm.openstack-release-version' @@ -130,10 +133,10 @@ class IronicConductorCharm(charms_openstack.charm.OpenStackCharm): def _configure_defaults(self): net_iface = self.config.get("default-network-interface", None) if not net_iface: - self.config["default-network-interface"] = "flat" + self.config["default-network-interface"] = DEFAULT_NET_IFACE iface = self.config.get("default-deploy-interface", None) if not iface: - self.config["default-deploy-interface"] = "direct" + self.config["default-deploy-interface"] = DEFAULT_DEPLOY_IFACE def _setup_pxe_config(self, cfg): self.packages.extend(cfg.determine_packages()) @@ -166,14 +169,14 @@ class IronicConductorCharm(charms_openstack.charm.OpenStackCharm): ] def _validate_network_interfaces(self, interfaces): - valid_interfaces = ["flat", "neutron", "noop"] + valid_interfaces = VALID_NETWORK_INTERFACES for interface in interfaces: if interface not in valid_interfaces: raise ValueError( 'Network interface "%s" is not valid. Valid ' 'interfaces are: %s' % ( interface, ", ".join(valid_interfaces))) - + def _validate_default_net_interface(self): net_iface = self.config["default-network-interface"] if net_iface not in self.enabled_network_interfaces: @@ -183,7 +186,7 @@ class IronicConductorCharm(charms_openstack.charm.OpenStackCharm): self.enabled_network_interfaces)) def _validate_deploy_interfaces(self, interfaces): - valid_interfaces = ["direct", "iscsi"] + valid_interfaces = VALID_DEPLOY_INTERFACES has_secret = reactive.is_flag_set("leadership.set.temp_url_secret") for interface in interfaces: if interface not in valid_interfaces: @@ -196,7 +199,7 @@ class IronicConductorCharm(charms_openstack.charm.OpenStackCharm): raise ValueError( 'run "set-temp-url-secret" action on leader to ' 'enable "direct" deploy method') - + def _validate_default_deploy_interface(self): iface = self.config["default-deploy-interface"] if iface not in self.enabled_deploy_interfaces: @@ -204,7 +207,7 @@ class IronicConductorCharm(charms_openstack.charm.OpenStackCharm): "default-deploy-interface (%s) is not enabled " "in enabled-deploy-interfaces: %s" % ", ".join( self.enabled_deploy_interfaces)) - + @property def enabled_network_interfaces(self): network_interfaces = self.config.get( @@ -242,5 +245,4 @@ class IronicConductorCharm(charms_openstack.charm.OpenStackCharm): except Exception as err: msg = ("invalid default-deploy-interface config: %s" % err) return ('blocked', msg) - - return (None, None) \ No newline at end of file + return (None, None) diff --git a/src/reactive/ironic_handlers.py b/src/reactive/ironic_handlers.py index 51e1ab1..7de1ee6 100644 --- a/src/reactive/ironic_handlers.py +++ b/src/reactive/ironic_handlers.py @@ -2,10 +2,8 @@ from __future__ import absolute_import import charms.reactive as reactive import charmhelpers.core.hookenv as hookenv -import charms.leadership as leadership import charms_openstack.charm as charm -import charm.openstack.ironic.ironic as ironic # Use the charms.openstack defaults for common states and hooks charm.use_defaults( diff --git a/unit_tests/__init__.py b/unit_tests/__init__.py index 186c1d9..3af8c51 100644 --- a/unit_tests/__init__.py +++ b/unit_tests/__init__.py @@ -9,3 +9,39 @@ import charms_openstack.test_mocks # noqa charms_openstack.test_mocks.mock_charmhelpers() sys.modules['charmhelpers.core.decorators'] = ( charms_openstack.test_mocks.charmhelpers.core.decorators) + + +class _fake_decorator(object): + + def __init__(self, *args): + pass + + def __call__(self, f): + return f + + +charms = mock.MagicMock() +sys.modules['charms'] = charms +charms.leadership = mock.MagicMock() +sys.modules['charms.leadership'] = charms.leadership +charms.reactive = mock.MagicMock() +charms.reactive.when = _fake_decorator +charms.reactive.when_all = _fake_decorator +charms.reactive.when_any = _fake_decorator +charms.reactive.when_not = _fake_decorator +charms.reactive.when_none = _fake_decorator +charms.reactive.when_not_all = _fake_decorator +charms.reactive.not_unless = _fake_decorator +charms.reactive.when_file_changed = _fake_decorator +charms.reactive.collect_metrics = _fake_decorator +charms.reactive.meter_status_changed = _fake_decorator +charms.reactive.only_once = _fake_decorator +charms.reactive.hook = _fake_decorator +charms.reactive.bus = mock.MagicMock() +charms.reactive.flags = mock.MagicMock() +charms.reactive.relations = mock.MagicMock() +sys.modules['charms.reactive'] = charms.reactive +sys.modules['charms.reactive.bus'] = charms.reactive.bus +sys.modules['charms.reactive.bus'] = charms.reactive.decorators +sys.modules['charms.reactive.flags'] = charms.reactive.flags +sys.modules['charms.reactive.relations'] = charms.reactive.relations diff --git a/unit_tests/test_api_utils.py b/unit_tests/test_api_utils.py new file mode 100644 index 0000000..5404f6f --- /dev/null +++ b/unit_tests/test_api_utils.py @@ -0,0 +1,184 @@ +# Copyright 2020 Canonical Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import mock + +import charms_openstack.test_utils as test_utils +from keystoneauth1 import exceptions as ks_exc + +import charm.openstack.ironic.api_utils as api_utils + + +class TestGetKeystoneSession(test_utils.PatchHelper): + + def setUp(self): + super().setUp() + self.ks_int = mock.MagicMock() + self.ks_int.credentials_username.return_value = "ironic" + self.ks_int.credentials_password.return_value = "super_secret" + self.ks_int.credentials_project.return_value = "services" + self.ks_int.auth_protocol.return_value = "https" + self.ks_int.auth_host.return_value = "example.com" + self.ks_int.credentials_port.return_value = "5000" + self.ks_int.api_version.return_value = "3" + self.ks_int.credentials_project_domain_name.return_value = "default" + self.ks_int.credentials_user_domain_name.return_value = "default" + self.ks_expect = { + "username": "ironic", + "password": "super_secret", + "auth_url": "https://example.com:5000", + "project_name": "services", + "project_domain_name": "default", + "user_domain_name": "default", + } + + def test_create_keystone_session(self): + self.patch_object(api_utils, 'loading') + self.patch_object(api_utils, 'ks_session') + loader = mock.MagicMock() + auth = mock.MagicMock() + loader.load_from_options.return_value = auth + self.loading.get_plugin_loader.return_value = loader + api_utils.create_keystone_session(self.ks_int) + + self.loading.get_plugin_loader.assert_called_with("v3password") + loader.load_from_options.assert_called_with(**self.ks_expect) + self.ks_session.Session.assert_called_with(auth=auth, verify=True) + + def test_create_keystone_session_v2(self): + self.patch_object(api_utils, 'loading') + self.patch_object(api_utils, 'ks_session') + loader = mock.MagicMock() + auth = mock.MagicMock() + loader.load_from_options.return_value = auth + self.loading.get_plugin_loader.return_value = loader + self.ks_int.api_version.return_value = "v2.0" + api_utils.create_keystone_session(self.ks_int) + del self.ks_expect["project_domain_name"] + del self.ks_expect["user_domain_name"] + + self.loading.get_plugin_loader.assert_called_with("password") + loader.load_from_options.assert_called_with(**self.ks_expect) + self.ks_session.Session.assert_called_with(auth=auth, verify=True) + + +class TestOSClients(test_utils.PatchHelper): + + def setUp(self): + super().setUp() + self.session = mock.MagicMock() + self.stores = { + "stores": [ + {"id": "swift"}, + {"id": "local"}, + {"id": "ceph", "default": True} + ] + } + + self.patch_object( + api_utils.glanceclient, + 'Client', name="glance_client") + self.patch_object( + api_utils.swiftclient, + 'Connection', name="swift_con") + self.patch_object( + api_utils.keystoneclient.v3, + 'Client', name="ks_client") + + self.mocked_glance = mock.MagicMock() + self.glance_client.return_value = self.mocked_glance + self.mocked_glance.images.get_stores_info.return_value = self.stores + + self.mocked_swift = mock.MagicMock() + self.swift_con.return_value = self.mocked_swift + + self.mocked_ks = mock.MagicMock() + self.ks_client.return_value = self.mocked_ks + + self.target = api_utils.OSClients(self.session) + self.glance_client.assert_called_with(session=self.session, version=2) + self.swift_con.assert_called_with(session=self.session, cacert=None) + self.ks_client.assert_called_with(session=self.session) + + def test_stores_info(self): + self.assertEqual(self.target._stores_info, self.stores["stores"]) + + def test_glance_stores(self): + self.assertEqual( + self.target.glance_stores, + [i["id"] for i in self.stores["stores"]]) + + def test_default_glance_store(self): + self.assertEqual(self.target.get_default_glance_store(), "ceph") + del self.stores["stores"][2]["default"] + self.stores["stores"][1]["default"] = True + self.assertEqual(self.target.get_default_glance_store(), "local") + + def test_get_object_account_properties(self): + props = { + "x-account-meta-fakeprop": "hi there", + "x-account-meta-fakeprop2": "bye there", + "bogus": "won't be here in result" + } + expected_result = { + "fakeprop": "hi there", + "fakeprop2": "bye there", + } + self.mocked_swift.get_account.return_value = (props, "") + result = self.target.get_object_account_properties() + self.assertEqual(result, expected_result) + + def test_set_object_account_property(self): + props = { + "x-account-meta-fakeprop": "hi there", + } + self.mocked_swift.get_account.return_value = (props, "") + + self.target.set_object_account_property("FaKePrOp", "hi there") + self.mocked_swift.post_account.assert_not_called() + + self.target.set_object_account_property("FaKePrOp2", "bye there") + self.mocked_swift.post_account.assert_called_with( + {"x-account-meta-fakeprop2": "bye there"}) + + def test_delete_object_account_property(self): + self.target.delete_object_account_property("FaKePrOp2") + self.mocked_swift.post_account.assert_called_with( + {"x-account-meta-fakeprop2": ""}) + + def test_has_service_type(self): + mocked_svc_find = mock.MagicMock() + mocked_svc_find.id = "fakeid" + + self.ks_client.services.find.return_value = mocked_svc_find + self.target._ks = self.ks_client + + result = self.target._has_service_type( + "object-store", interface="public") + self.ks_client.services.find.assert_called_with( + type="object-store") + self.ks_client.endpoints.find.assert_called_with( + service_id="fakeid", interface="public") + self.assertTrue(result) + + def test_does_not_have_service_type(self): + self.ks_client.services.find.side_effect = ks_exc.http.NotFound() + self.target._ks = self.ks_client + + result = self.target._has_service_type( + "object-store", interface="public") + self.ks_client.services.find.assert_called_with( + type="object-store") + self.ks_client.endpoints.find.assert_not_called() + self.assertFalse(result) diff --git a/unit_tests/test_controller_utils.py b/unit_tests/test_controller_utils.py new file mode 100644 index 0000000..25c947a --- /dev/null +++ b/unit_tests/test_controller_utils.py @@ -0,0 +1,122 @@ +# Copyright 2020 Canonical Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import mock +import shutil + +import charms_openstack.test_utils as test_utils +import charmhelpers.core.host as ch_host + +import charm.openstack.ironic.controller_utils as controller_utils + + +class TestGetPXEBootClass(test_utils.PatchHelper): + + def test_get_pxe_config_class_bionic(self): + self.patch_object( + ch_host, 'get_distrib_codename') + self.get_distrib_codename.return_value = "bionic" + charm_config = {} + pxe_class = controller_utils.get_pxe_config_class(charm_config) + self.assertTrue( + isinstance( + pxe_class, controller_utils.PXEBootBionic)) + + def test_get_pxe_config_class(self): + self.patch_object( + ch_host, 'get_distrib_codename') + self.get_distrib_codename.return_value = "focal" + charm_config = {} + pxe_class = controller_utils.get_pxe_config_class(charm_config) + self.assertTrue( + isinstance( + pxe_class, controller_utils.PXEBootBase)) + + +class TestPXEBootBase(test_utils.PatchHelper): + + def setUp(self): + super().setUp() + self.target = controller_utils.PXEBootBase({}) + + def test_ensure_folders(self): + global _TEST_FOLDERS + self.patch_object(os.path, 'isdir') + self.isdir.side_effect = [False, False, False] + self.patch_object(os, 'makedirs') + self.patch_object(ch_host, 'chownr') + self.target._ensure_folders() + chown_call_list = [ + mock.call( + controller_utils.PXEBootBase.TFTP_ROOT, + controller_utils._IRONIC_USER, + controller_utils._IRONIC_GROUP, + chowntopdir=True), + mock.call( + controller_utils.PXEBootBase.HTTP_ROOT, + controller_utils._IRONIC_USER, + controller_utils._IRONIC_GROUP, + chowntopdir=True), + ] + isdir_call_list = [ + mock.call(controller_utils.PXEBootBase.TFTP_ROOT), + mock.call(controller_utils.PXEBootBase.HTTP_ROOT), + mock.call(controller_utils.PXEBootBase.GRUB_DIR), + ] + + makedirs_call_list = [ + mock.call(controller_utils.PXEBootBase.TFTP_ROOT), + mock.call(controller_utils.PXEBootBase.HTTP_ROOT), + mock.call(controller_utils.PXEBootBase.GRUB_DIR), + ] + self.isdir.assert_has_calls(isdir_call_list) + self.makedirs.assert_has_calls(makedirs_call_list) + ch_host.chownr.assert_has_calls(chown_call_list) + + def test_copy_resources_missing_file(self): + self.patch_object(self.target, '_ensure_folders') + self.patch_object(os.path, 'isfile') + is_file_returns = list([ + True for i in controller_utils.PXEBootBase.FILE_MAP]) + is_file_returns[0] = False + self.isfile.side_effect = is_file_returns + with self.assertRaises(ValueError): + self.target._copy_resources() + + def test_copy_resources(self): + shutil_calls = [ + mock.call( + i, + os.path.join( + controller_utils.PXEBootBase.TFTP_ROOT, + controller_utils.PXEBootBase.FILE_MAP[i]), + follow_symlinks=True + ) for i in controller_utils.PXEBootBase.FILE_MAP + ] + self.patch_object(self.target, '_ensure_folders') + self.patch_object(os.path, 'isfile') + self.patch_object(shutil, 'copy') + self.patch_object(ch_host, 'chownr') + self.isfile.side_effect = [ + True for i in controller_utils.PXEBootBase.FILE_MAP] + + self.target._copy_resources() + self._ensure_folders.assert_called_with() + self.copy.assert_has_calls(shutil_calls) + self.chownr.assert_called_with( + controller_utils.PXEBootBase.TFTP_ROOT, + controller_utils._IRONIC_USER, + controller_utils._IRONIC_GROUP, + chowntopdir=True) diff --git a/unit_tests/test_ironic_conductor_handlers.py b/unit_tests/test_ironic_conductor_handlers.py index bc53b15..832d04b 100644 --- a/unit_tests/test_ironic_conductor_handlers.py +++ b/unit_tests/test_ironic_conductor_handlers.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import json import mock from charm.openstack.ironic import ironic @@ -92,7 +91,7 @@ class TestIronicHandlers(test_utils.PatchHelper): 'rabbit-vhost': 'openstack', } self.ironic_charm.get_amqp_credentials.return_value = list( - config.values()) + config.values()) handlers.request_amqp_access(amqp) amqp.request_access.assert_called_once_with( username=config['rabbit-user'], @@ -101,16 +100,16 @@ class TestIronicHandlers(test_utils.PatchHelper): def test_request_database_access(self): database = mock.MagicMock() - dbs = [{ - "database": "ironic", - "username": "ironic", - }, - # Ironic only needs one DB, but the code can handle more, - # so we test it. - { - "database": "second_db", - "username": "second_user", - }] + dbs = [ + { + "database": "ironic", + "username": "ironic", + }, + { + "database": "second_db", + "username": "second_user", + } + ] self.ironic_charm.get_database_setup.return_value = dbs calls = [mock.call(**i) for i in dbs]