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 <gabrioux@redhat.com>

diff --git a/library/ceph_dashboard_user.py b/library/ceph_dashboard_user.py
index 929c0d71d325bef948d910b1b7e5b48b875df433..e63e6c13379178bd356f647e1351253f3e4ab14d 100644 (file)
--- 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 cba4c4026c1a939fc63ce863b949ca6745ec3135..a9e84099b7ce7db75c3cd7b981a4383e4258aed9 100644 (file)
--- 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 3d676583b854fd4743bbb46517256df423495e14..4c972c635b409b4875d4f468ca7608b7e3361177 100644 (file)
--- 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 0562a793ebc93fe79656af6aec2aee97dfd3540f..14ae4e3b63031ca77cfb6fdbd9a500fad55dcb56 100644 (file)
--- 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 3ad9905fb08346594e627e0a254eb463fe4a93fa..0a7da58bcec27653573f5c149ff1dccb216522e0 100644 (file)
--- 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 f52fdbd11c0a0ff3725c343a87f41b76c82b58c7..23c8508573300da77009489395270e0ba2e4d615 100644 (file)
--- 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