From 6a00bbb2c6a3ef5d296b1e5b762a65fb07663b65 Mon Sep 17 00:00:00 2001 From: Guillaume Demonet Date: Mon, 26 Sep 2022 09:07:57 +0200 Subject: [PATCH] salt,tests: Allow forcing the creation of an LV In case a formatted LV was removed without wiping its FS signature, creation from storage-operator will fail. We add support for an annotation (metalk8s.scality.com/force-lvcreate) to force the creation of the LV, at the risk of also wiping the previous FS contents. --- CHANGELOG.md | 4 +++ docs/_infos/volumes.yaml | 8 +++++ .../volume_creation_deletion_cli.rst | 2 ++ salt/_modules/metalk8s_volumes.py | 10 +++++- .../modules/files/test_metalk8s_volumes.yaml | 24 +++++++++++++- .../unit/modules/test_metalk8s_volumes.py | 14 +++++++- tests/post/features/volume.feature | 33 +++++++++++++++++++ tests/post/steps/test_volume.py | 31 +++++++++++++++++ 8 files changed, 123 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6c1456b5c..d950e95d8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Add `metalk8s-operator` to manage Workload Plane Ingress virtual IPs (PR[#3864](https://github.com/scality/metalk8s/pull/3864)) +- Add support for a `metalk8s.scality.com/force-lvcreate` annotation on `Volume` + objects of type `LVMLogicalVolume` to force the creation of their LV (use with + caution) (PR[#3877](https://github.com/scality/metalk8s/pull/3877)) + ### Removals - The deprecated long name `--archive` for the "add" option to the `iso-manager.sh` diff --git a/docs/_infos/volumes.yaml b/docs/_infos/volumes.yaml index b9d3d8b2f1..32c6881b35 100644 --- a/docs/_infos/volumes.yaml +++ b/docs/_infos/volumes.yaml @@ -55,3 +55,11 @@ volume_types: lvmLogicalVolume: vgName: size: 10Gi + notes: | + .. tip:: + + You can add the ``metalk8s.scality.com/force-lvcreate`` annotation + (value does not matter) to LVMLogicalVolume objects to force the LV + creation. This will wipe any existing FS signature on the created LV, + so use with caution. + If the LV already exists however, it will just attempt to use it as is. diff --git a/docs/operation/volume_management/volume_creation_deletion_cli.rst b/docs/operation/volume_management/volume_creation_deletion_cli.rst index da4398acd6..f41bfb22c7 100644 --- a/docs/operation/volume_management/volume_creation_deletion_cli.rst +++ b/docs/operation/volume_management/volume_creation_deletion_cli.rst @@ -38,6 +38,8 @@ Creating a Volume {% for key, info in volume_info["fields"].items() %} - **{{ key }}**: {{ info }}. {% endfor %} + + {{ volume_info.get("notes", "") }} {% endfor %} #. Create the Volume. diff --git a/salt/_modules/metalk8s_volumes.py b/salt/_modules/metalk8s_volumes.py index 2b2918e716..6c3071d56a 100644 --- a/salt/_modules/metalk8s_volumes.py +++ b/salt/_modules/metalk8s_volumes.py @@ -484,9 +484,17 @@ def exists(self): return bool(lv_info and lv_info.get(self.path)) def create(self): + force_annotation = ( + self.get("metadata") + .get("annotations", {}) + .get("metalk8s.scality.com/force-lvcreate") + ) try: ret = __salt__["lvm.lvcreate"]( - lvname=self.lv_name, vgname=self.vg_name, size="{}b".format(self.size) + lvname=self.lv_name, + vgname=self.vg_name, + size="{}b".format(self.size), + force=force_annotation is not None, ) except Exception as exc: raise CommandExecutionError( diff --git a/salt/tests/unit/modules/files/test_metalk8s_volumes.yaml b/salt/tests/unit/modules/files/test_metalk8s_volumes.yaml index 5322c95a09..6c75f34037 100644 --- a/salt/tests/unit/modules/files/test_metalk8s_volumes.yaml +++ b/salt/tests/unit/modules/files/test_metalk8s_volumes.yaml @@ -137,7 +137,7 @@ _volumes_details: &volumes_details lastUpdateTime: '2020-07-11T13:55:26Z' status: 'True' type: Ready - my-lvm-lv-volume: + my-lvm-lv-volume: &_lv_volume apiVersion: storage.metalk8s.scality.com/v1alpha1 kind: Volume metadata: @@ -542,18 +542,37 @@ create: of: informations "Output from lvcreate": All good pillar_volumes: *volumes_details + lvcreate_forced: false # unable to create the LVM LV (raise when calling `lvcreate`) - name: my-lvm-lv-volume pillar_volumes: *volumes_details lvcreate: null raise_msg: "cannot create LVM LogicalVolume my-lvm-lv-volume in VG my_vg" + lvcreate_forced: false # unable to create the LVM LV (usual "vg does not exist") - name: my-lvm-lv-volume pillar_volumes: *volumes_details lvcreate: 'Volume group "my_vg" not found Penguin' raise_msg: 'cannot create LVM LogicalVolume my-lvm-lv-volume in VG my_vg: Volume group "my_vg" not found Penguin' + lvcreate_forced: false + + # annotation to force the call to lvcreate + - name: example + pillar_volumes: + example: + <<: *_lv_volume + metadata: + name: example + uid: 6474cda7-0dbe-40fc-9842-3cb0404a725a + annotations: + metalk8s.scality.com/force-lvcreate: "" + lvcreate: + /dev/my_vg/example: + all: good + Output from lvcreate: All good + lvcreate_forced: true ## LVM LogicalVolume block volume # create a simple LVM LV volume @@ -564,18 +583,21 @@ create: of: informations "Output from lvcreate": All good pillar_volumes: *volumes_details + lvcreate_forced: false # unable to create the LVM LV (raise when calling `lvcreate`) - name: my-lvm-lv-block-volume pillar_volumes: *volumes_details lvcreate: null raise_msg: "cannot create LVM LogicalVolume my-lvm-lv-block-volume in VG my_vg" + lvcreate_forced: false # unable to create the LVM LV (usual "vg does not exist") - name: my-lvm-lv-block-volume pillar_volumes: *volumes_details lvcreate: 'Volume group "my_vg" not found Penguin' raise_msg: 'cannot create LVM LogicalVolume my-lvm-lv-block-volume in VG my_vg: Volume group "my_vg" not found Penguin' + lvcreate_forced: false ## Invalid volumes # specified volume is not in the pillar diff --git a/salt/tests/unit/modules/test_metalk8s_volumes.py b/salt/tests/unit/modules/test_metalk8s_volumes.py index 24fadeb497..e3679e8578 100644 --- a/salt/tests/unit/modules/test_metalk8s_volumes.py +++ b/salt/tests/unit/modules/test_metalk8s_volumes.py @@ -84,7 +84,13 @@ def test_exists( @utils.parameterized_from_cases(YAML_TESTS_CASES["create"]) def test_create( - self, name, raise_msg=None, pillar_volumes=None, ftruncate=True, lvcreate=None + self, + name, + raise_msg=None, + pillar_volumes=None, + ftruncate=True, + lvcreate=None, + lvcreate_forced=None, ): """ Tests the return of `create` function @@ -123,6 +129,12 @@ def test_create( # This function does not return anything metalk8s_volumes.create(name) + if lvcreate_forced is not None: + lvcreate_mock.assert_called_once() + call_kwargs = lvcreate_mock.call_args[1] + self.assertIn("force", call_kwargs) + self.assertEqual(call_kwargs["force"], lvcreate_forced) + @utils.parameterized_from_cases(YAML_TESTS_CASES["is_prepared"]) def test_is_prepared( self, diff --git a/tests/post/features/volume.feature b/tests/post/features/volume.feature index 2bf6a0dc00..a593dd6891 100644 --- a/tests/post/features/volume.feature +++ b/tests/post/features/volume.feature @@ -269,3 +269,36 @@ Feature: Volume management Then the Volume 'test-volume12-lvmlv' is 'Available' And the PersistentVolume 'test-volume12-lvmlv' has size '10Gi' And the device '/dev/test-vg-12/test-volume12-lvmlv' exists + + Scenario: Test volume re-creation (lvmLogicalVolume force-lvcreate) + Given the Kubernetes API is available + And a LVM VG 'test-vg-13' exists + And a LVM LV 'test-lv-13' in VG 'test-vg-13' was created, formatted, then removed + When I create the following Volume: + apiVersion: storage.metalk8s.scality.com/v1alpha1 + kind: Volume + metadata: + name: test-lv-13 + spec: + nodeName: bootstrap + storageClassName: metalk8s + lvmLogicalVolume: + vgName: test-vg-13 + size: 1Gi + Then the Volume 'test-lv-13' is 'Failed' with code 'CreationError' and message matches 'signature detected on /dev/test-vg-13/test-lv-13' + When I delete the Volume 'test-lv-13' + Then the Volume 'test-lv-13' does not exist + When I create the following Volume: + apiVersion: storage.metalk8s.scality.com/v1alpha1 + kind: Volume + metadata: + name: test-lv-13 + annotations: + metalk8s.scality.com/force-lvcreate: "" + spec: + nodeName: bootstrap + storageClassName: metalk8s + lvmLogicalVolume: + vgName: test-vg-13 + size: 1Gi + Then the Volume 'test-lv-13' is 'Available' diff --git a/tests/post/steps/test_volume.py b/tests/post/steps/test_volume.py index ed418077ff..d9ce9f6eee 100644 --- a/tests/post/steps/test_volume.py +++ b/tests/post/steps/test_volume.py @@ -125,6 +125,14 @@ def test_volume_creation_lvmlv_block(host, teardown): pass +@scenario( + "../features/volume.feature", + "Test volume re-creation (lvmLogicalVolume force-lvcreate)", +) +def test_volume_creation_lvmlv_block(host, teardown): + pass + + # }}} # Given {{{ @@ -206,6 +214,29 @@ def storage_class_exist(name, sc_client): sc_client.create_from_yaml(kube_utils.DEFAULT_SC.format(name=name)) +@given( + parsers.parse( + "a LVM LV '{name}' in VG '{vg_name}' was created, formatted, then removed" + ) +) +def lvm_lv_was_used(host, name, vg_name): + with host.sudo(): + host.run_test(f"vgdisplay {vg_name}") + host.run_test(f"lvcreate -L 1G -n {name} {vg_name}") + host.run_test(f"mkfs.ext4 /dev/{vg_name}/{name}") + host.run_test(f"lvremove -y /dev/{vg_name}/{name}") + + yield + + with host.sudo(): + lv_exists = host.run(f"lvdisplay /dev/{vg_name}/{name}") + if lv_exists.rc != 0: + # LV does not exist, attempt to recreate for wiping the signature + host.run_test(f"lvcreate -L 1G -n {name} -y {vg_name}") + host.run_test(f"wipefs -a /dev/{vg_name}/{name}") + host.run_test(f"lvremove -y /dev/{vg_name}/{name}") + + # }}} # When {{{