From 67467fd87c96699d805b66dad3bf4dac6aa21911 Mon Sep 17 00:00:00 2001 From: Guillaume Abrioux Date: Wed, 25 Nov 2020 20:51:09 +0100 Subject: [PATCH] module_utils: refactor - update `generate_ceph_cmd()` so `user_key` is automatically built from `cluster` and `user` params. - update and add testing. Signed-off-by: Guillaume Abrioux --- library/ceph_dashboard_user.py | 10 +-- library/ceph_ec_profile.py | 12 ++-- library/ceph_fs.py | 10 +-- library/ceph_pool.py | 36 +++++------ module_utils/ca_common.py | 5 +- tests/library/test_ca_common.py | 107 +++++++++++++++++++++++++------- 6 files changed, 124 insertions(+), 56 deletions(-) diff --git a/library/ceph_dashboard_user.py b/library/ceph_dashboard_user.py index 929c0d71d..e63e6c133 100644 --- a/library/ceph_dashboard_user.py +++ b/library/ceph_dashboard_user.py @@ -123,7 +123,7 @@ def create_user(module, container_image=None): args = ['ac-user-create', name, password] - cmd = generate_ceph_cmd(cluster=cluster, sub_cmd=['dashboard'], args=args, container_image=container_image) + cmd = generate_ceph_cmd(sub_cmd=['dashboard'], args=args, cluster=cluster, container_image=container_image) return cmd @@ -141,7 +141,7 @@ def set_roles(module, container_image=None): args.extend(roles) - cmd = generate_ceph_cmd(cluster=cluster, sub_cmd=['dashboard'], args=args, container_image=container_image) + cmd = generate_ceph_cmd(sub_cmd=['dashboard'], args=args, cluster=cluster, container_image=container_image) return cmd @@ -157,7 +157,7 @@ def set_password(module, container_image=None): args = ['ac-user-set-password', name, password] - cmd = generate_ceph_cmd(cluster=cluster, sub_cmd=['dashboard'], args=args, container_image=container_image) + cmd = generate_ceph_cmd(sub_cmd=['dashboard'], args=args, cluster=cluster, container_image=container_image) return cmd @@ -172,7 +172,7 @@ def get_user(module, container_image=None): args = ['ac-user-show', name, '--format=json'] - cmd = generate_ceph_cmd(cluster=cluster, sub_cmd=['dashboard'], args=args, container_image=container_image) + cmd = generate_ceph_cmd(sub_cmd=['dashboard'], args=args, cluster=cluster, container_image=container_image) return cmd @@ -187,7 +187,7 @@ def remove_user(module, container_image=None): args = ['ac-user-delete', name] - cmd = generate_ceph_cmd(cluster=cluster, sub_cmd=['dashboard'], args=args, container_image=container_image) + cmd = generate_ceph_cmd(sub_cmd=['dashboard'], args=args, cluster=cluster, container_image=container_image) return cmd diff --git a/library/ceph_ec_profile.py b/library/ceph_ec_profile.py index cba4c4026..a9e84099b 100644 --- a/library/ceph_ec_profile.py +++ b/library/ceph_ec_profile.py @@ -113,9 +113,9 @@ def get_profile(module, name, cluster='ceph', container_image=None): args = ['get', name, '--format=json'] - cmd = generate_ceph_cmd(cluster=cluster, - sub_cmd=['osd', 'erasure-code-profile'], + cmd = generate_ceph_cmd(sub_cmd=['osd', 'erasure-code-profile'], args=args, + cluster=cluster, container_image=container_image) return cmd @@ -132,9 +132,9 @@ def create_profile(module, name, k, m, stripe_unit, cluster='ceph', force=False, if force: args.append('--force') - cmd = generate_ceph_cmd(cluster=cluster, - sub_cmd=['osd', 'erasure-code-profile'], + cmd = generate_ceph_cmd(sub_cmd=['osd', 'erasure-code-profile'], args=args, + cluster=cluster, container_image=container_image) return cmd @@ -147,9 +147,9 @@ def delete_profile(module, name, cluster='ceph', container_image=None): args = ['rm', name] - cmd = generate_ceph_cmd(cluster=cluster, - sub_cmd=['osd', 'erasure-code-profile'], + cmd = generate_ceph_cmd(sub_cmd=['osd', 'erasure-code-profile'], args=args, + cluster=cluster, container_image=container_image) return cmd diff --git a/library/ceph_fs.py b/library/ceph_fs.py index 3d676583b..4c972c635 100644 --- a/library/ceph_fs.py +++ b/library/ceph_fs.py @@ -119,7 +119,7 @@ def create_fs(module, container_image=None): args = ['new', name, metadata, data] - cmd = generate_ceph_cmd(cluster=cluster, sub_cmd=['fs'], args=args, container_image=container_image) + cmd = generate_ceph_cmd(sub_cmd=['fs'], args=args, cluster=cluster, container_image=container_image) return cmd @@ -134,7 +134,7 @@ def get_fs(module, container_image=None): args = ['get', name, '--format=json'] - cmd = generate_ceph_cmd(cluster=cluster, sub_cmd=['fs'], args=args, container_image=container_image) + cmd = generate_ceph_cmd(sub_cmd=['fs'], args=args, cluster=cluster, container_image=container_image) return cmd @@ -149,7 +149,7 @@ def remove_fs(module, container_image=None): args = ['rm', name, '--yes-i-really-mean-it'] - cmd = generate_ceph_cmd(cluster=cluster, sub_cmd=['fs'], args=args, container_image=container_image) + cmd = generate_ceph_cmd(sub_cmd=['fs'], args=args, cluster=cluster, container_image=container_image) return cmd @@ -164,7 +164,7 @@ def fail_fs(module, container_image=None): args = ['fail', name] - cmd = generate_ceph_cmd(cluster=cluster, sub_cmd=['fs'], args=args, container_image=container_image) + cmd = generate_ceph_cmd(sub_cmd=['fs'], args=args, cluster=cluster, container_image=container_image) return cmd @@ -180,7 +180,7 @@ def set_fs(module, container_image=None): args = ['set', name, 'max_mds', str(max_mds)] - cmd = generate_ceph_cmd(cluster=cluster, sub_cmd=['fs'], args=args, container_image=container_image) + cmd = generate_ceph_cmd(sub_cmd=['fs'], args=args, cluster=cluster, container_image=container_image) return cmd diff --git a/library/ceph_pool.py b/library/ceph_pool.py index 0562a793e..14ae4e3b6 100644 --- a/library/ceph_pool.py +++ b/library/ceph_pool.py @@ -167,9 +167,9 @@ def check_pool_exist(cluster, args = ['stats', name, '-f', output_format] - cmd = generate_ceph_cmd(cluster=cluster, - sub_cmd=['osd', 'pool'], + cmd = generate_ceph_cmd(sub_cmd=['osd', 'pool'], args=args, + cluster=cluster, user=user, user_key=user_key, container_image=container_image) @@ -211,9 +211,9 @@ def get_application_pool(cluster, args = ['application', 'get', name, '-f', output_format] - cmd = generate_ceph_cmd(cluster=cluster, - sub_cmd=['osd', 'pool'], + cmd = generate_ceph_cmd(sub_cmd=['osd', 'pool'], args=args, + cluster=cluster, user=user, user_key=user_key, container_image=container_image) @@ -233,9 +233,9 @@ def enable_application_pool(cluster, args = ['application', 'enable', name, application] - cmd = generate_ceph_cmd(cluster=cluster, - sub_cmd=['osd', 'pool'], + cmd = generate_ceph_cmd(sub_cmd=['osd', 'pool'], args=args, + cluster=cluster, user=user, user_key=user_key, container_image=container_image) @@ -256,9 +256,9 @@ def disable_application_pool(cluster, args = ['application', 'disable', name, application, '--yes-i-really-mean-it'] - cmd = generate_ceph_cmd(cluster=cluster, - sub_cmd=['osd', 'pool'], + cmd = generate_ceph_cmd(sub_cmd=['osd', 'pool'], args=args, + cluster=cluster, user=user, user_key=user_key, container_image=container_image) @@ -279,9 +279,9 @@ def get_pool_details(module, args = ['ls', 'detail', '-f', output_format] - cmd = generate_ceph_cmd(cluster=cluster, - sub_cmd=['osd', 'pool'], + cmd = generate_ceph_cmd(sub_cmd=['osd', 'pool'], args=args, + cluster=cluster, user=user, user_key=user_key, container_image=container_image) @@ -367,9 +367,9 @@ def list_pools(cluster, args.extend(['-f', output_format]) - cmd = generate_ceph_cmd(cluster=cluster, - sub_cmd=['osd', 'pool'], + cmd = generate_ceph_cmd(sub_cmd=['osd', 'pool'], args=args, + cluster=cluster, user=user, user_key=user_key, container_image=container_image) @@ -418,9 +418,9 @@ def create_pool(cluster, '--autoscale-mode', user_pool_config['pg_autoscale_mode']['value']]) - cmd = generate_ceph_cmd(cluster=cluster, - sub_cmd=['osd', 'pool'], + cmd = generate_ceph_cmd(sub_cmd=['osd', 'pool'], args=args, + cluster=cluster, user=user, user_key=user_key, container_image=container_image) @@ -435,9 +435,9 @@ def remove_pool(cluster, name, user, user_key, container_image=None): args = ['rm', name, name, '--yes-i-really-really-mean-it'] - cmd = generate_ceph_cmd(cluster=cluster, - sub_cmd=['osd', 'pool'], + cmd = generate_ceph_cmd(sub_cmd=['osd', 'pool'], args=args, + cluster=cluster, user=user, user_key=user_key, container_image=container_image) @@ -460,9 +460,9 @@ def update_pool(module, cluster, name, delta[key]['cli_set_opt'], delta[key]['value']] - cmd = generate_ceph_cmd(cluster=cluster, - sub_cmd=['osd', 'pool'], + cmd = generate_ceph_cmd(sub_cmd=['osd', 'pool'], args=args, + cluster=cluster, user=user, user_key=user_key, container_image=container_image) diff --git a/module_utils/ca_common.py b/module_utils/ca_common.py index 3ad9905fb..0a7da58bc 100644 --- a/module_utils/ca_common.py +++ b/module_utils/ca_common.py @@ -2,11 +2,14 @@ import os import datetime -def generate_ceph_cmd(cluster, sub_cmd, args, user='client.admin', user_key='/etc/ceph/ceph.client.admin.keyring', container_image=None): +def generate_ceph_cmd(sub_cmd, args, user_key=None, cluster='ceph', user='client.admin', container_image=None): ''' Generate 'ceph' command line to execute ''' + if not user_key: + user_key = '/etc/ceph/{}.{}.keyring'.format(cluster, user) + cmd = pre_generate_ceph_cmd(container_image=container_image) base_cmd = [ diff --git a/tests/library/test_ca_common.py b/tests/library/test_ca_common.py index f52fdbd11..23c850857 100644 --- a/tests/library/test_ca_common.py +++ b/tests/library/test_ca_common.py @@ -3,29 +3,31 @@ import os import ca_common import pytest -fake_binary = 'ceph' -fake_cluster = 'ceph' fake_container_binary = 'podman' fake_container_image = 'docker.io/ceph/daemon:latest' -fake_container_cmd = [ - fake_container_binary, - 'run', - '--rm', - '--net=host', - '-v', '/etc/ceph:/etc/ceph:z', - '-v', '/var/lib/ceph/:/var/lib/ceph/:z', - '-v', '/var/log/ceph/:/var/log/ceph/:z', - '--entrypoint=' + fake_binary, - fake_container_image -] class TestCommon(object): + def setup_method(self): + self.fake_binary = 'ceph' + self.fake_cluster = 'ceph' + self.fake_container_cmd = [ + fake_container_binary, + 'run', + '--rm', + '--net=host', + '-v', '/etc/ceph:/etc/ceph:z', + '-v', '/var/lib/ceph/:/var/lib/ceph/:z', + '-v', '/var/log/ceph/:/var/log/ceph/:z', + '--entrypoint=' + self.fake_binary, + fake_container_image + ] + @patch.dict(os.environ, {'CEPH_CONTAINER_BINARY': fake_container_binary}) def test_container_exec(self): - cmd = ca_common.container_exec(fake_binary, fake_container_image) - assert cmd == fake_container_cmd + cmd = ca_common.container_exec(self.fake_binary, fake_container_image) + assert cmd == self.fake_container_cmd def test_not_is_containerized(self): assert ca_common.is_containerized() is None @@ -38,9 +40,9 @@ class TestCommon(object): @patch.dict(os.environ, {'CEPH_CONTAINER_BINARY': fake_container_binary}) def test_pre_generate_ceph_cmd(self, image): if image: - expected_cmd = fake_container_cmd + expected_cmd = self.fake_container_cmd else: - expected_cmd = [fake_binary] + expected_cmd = [self.fake_binary] assert ca_common.pre_generate_ceph_cmd(image) == expected_cmd @@ -50,16 +52,79 @@ class TestCommon(object): sub_cmd = ['osd', 'pool'] args = ['create', 'foo'] if image: - expected_cmd = fake_container_cmd + expected_cmd = self.fake_container_cmd else: - expected_cmd = [fake_binary] + expected_cmd = [self.fake_binary] expected_cmd.extend([ '-n', 'client.admin', '-k', '/etc/ceph/ceph.client.admin.keyring', '--cluster', - fake_cluster, + self.fake_cluster, + 'osd', 'pool', + 'create', 'foo' + ]) + assert ca_common.generate_ceph_cmd(sub_cmd, args, cluster=self.fake_cluster, container_image=image) == expected_cmd + + @pytest.mark.parametrize('image', [None, fake_container_image]) + @patch.dict(os.environ, {'CEPH_CONTAINER_BINARY': fake_container_binary}) + def test_generate_ceph_cmd_different_cluster_name(self, image): + sub_cmd = ['osd', 'pool'] + args = ['create', 'foo'] + if image: + expected_cmd = self.fake_container_cmd + else: + expected_cmd = [self.fake_binary] + + expected_cmd.extend([ + '-n', 'client.admin', + '-k', '/etc/ceph/foo.client.admin.keyring', + '--cluster', + 'foo', + 'osd', 'pool', + 'create', 'foo' + ]) + result = ca_common.generate_ceph_cmd(sub_cmd, args, cluster='foo', container_image=image) + assert result == expected_cmd + + @pytest.mark.parametrize('image', [None, fake_container_image]) + @patch.dict(os.environ, {'CEPH_CONTAINER_BINARY': fake_container_binary}) + def test_generate_ceph_cmd_different_cluster_name_and_user(self, image): + sub_cmd = ['osd', 'pool'] + args = ['create', 'foo'] + if image: + expected_cmd = self.fake_container_cmd + else: + expected_cmd = [self.fake_binary] + + expected_cmd.extend([ + '-n', 'client.foo', + '-k', '/etc/ceph/foo.client.foo.keyring', + '--cluster', + 'foo', + 'osd', 'pool', + 'create', 'foo' + ]) + result = ca_common.generate_ceph_cmd(sub_cmd, args, cluster='foo', user='client.foo', container_image=image) + assert result == expected_cmd + + @pytest.mark.parametrize('image', [None, fake_container_image]) + @patch.dict(os.environ, {'CEPH_CONTAINER_BINARY': fake_container_binary}) + def test_generate_ceph_cmd_different_user(self, image): + sub_cmd = ['osd', 'pool'] + args = ['create', 'foo'] + if image: + expected_cmd = self.fake_container_cmd + else: + expected_cmd = [self.fake_binary] + + expected_cmd.extend([ + '-n', 'client.foo', + '-k', '/etc/ceph/ceph.client.foo.keyring', + '--cluster', + 'ceph', 'osd', 'pool', 'create', 'foo' ]) - assert ca_common.generate_ceph_cmd(fake_cluster, sub_cmd, args, container_image=image) == expected_cmd + result = ca_common.generate_ceph_cmd(sub_cmd, args, user='client.foo', container_image=image) + assert result == expected_cmd -- 2.47.3