create multiple cnv workloads #10673

avd-sagare · 2024-10-14T14:38:41Z

No description provided.

openshift-ci · 2024-10-14T14:38:47Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: avd-sagare

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Avdhoot <[email protected]>

tests/conftest.py

parikshithb · 2024-12-02T12:07:40Z

tests/conftest.py

+        vm_list_default_compr = []
+
+        """
+        Setup csi-kms-connection-details configmap


can be commented instead.

parikshithb · 2024-12-02T12:08:11Z

tests/conftest.py

+    def factory(namespace=None):
+        """
+        Args:
+            namespace:


elaborate the namespace.

parikshithb · 2024-12-02T12:10:11Z

tests/conftest.py

+                source_url=constants.CNV_FEDORA_SOURCE,  # Assuming source_url is the same for all VMs
+                namespace=namespace,
+            )
+            vm_obj = vm_obj[-1]


what is the need for last vm_obj in the cnv_vm_objs list?

Actually cnv_workload gives list and everytime new obj is append on vm-obj list..So I am picking up last added vm object.

can get the index of current iteration by enumerating

tests/functional/cnv/test_vms.py

Signed-off-by: Avdhoot <[email protected]>

parikshithb · 2024-12-05T06:35:20Z

tests/conftest.py

+        vm_list_agg_compr = []
+        vm_list_default_compr = []
+
+        namespace = (


namespace will be created in vm class if no namespace is provided. So no need to create a specific ns here.

I need to create namespace here because I need to create csi kms token where we are creating vm pvc.

PrasadDesala · 2024-12-09T12:44:03Z

tests/conftest.py

@@ -7092,6 +7092,112 @@ def teardown():
    return factory


+@pytest.fixture


Suggested change

@pytest.fixture

@pytest.fixture()

PrasadDesala · 2024-12-09T12:49:57Z

tests/conftest.py

+    Create a cnv factory. Calling this fixture Creates multiple VMs
+    with specified configurations using the cnv_workload fixture.


Please elaborate the fixture description as discussed

PrasadDesala · 2024-12-09T12:52:58Z

tests/conftest.py

+            namespace if namespace else create_unique_resource_name("vm", "namespace")
+        )
+
+        # Setup csi-kms-connection-details configmap


wouldn't this be done as part of the cluster deployment with KMS?

As we are creating new customize encrypted storageclass, we can't do at during cluster deployment.

PrasadDesala · 2024-12-09T12:57:22Z

tests/conftest.py

+        return (
+            vm_list_default_compr,
+            vm_list_agg_compr,
+            sc_obj_aggressive,
+            sc_obj_def_compr,


any reason for categ the VMs this way? can we return a single list of VM objects and get the required data from those?

PrasadDesala · 2024-12-09T12:57:52Z

tests/functional/cnv/test_vms.py

@@ -0,0 +1,144 @@
+import logging


Please provide a more meaningful test case name

PrasadDesala · 2024-12-09T13:26:49Z

tests/functional/cnv/test_vms.py

+        run_cmd(cmd)
+
+        # Calculate the MD5 checksum
+        if file_path:


throw exceptions when the file path and md5sum_on_loca are not found

PrasadDesala · 2024-12-09T13:29:02Z

tests/functional/cnv/test_vms.py

+            md5sum_on_vm = cal_md5sum_vm(vm_obj, vm_filepath, username=None)
+            assert (
+                md5sum_on_vm == md5sum_on_local
+            ), f"md5sum has not changed after copying file on {vm_obj.name}"


should this be, md5sum changed?

PrasadDesala · 2024-12-09T13:29:06Z

tests/functional/cnv/test_vms.py

+            md5sum_on_vm = cal_md5sum_vm(vm_obj, vm_filepath, username=None)
+            assert (
+                md5sum_on_vm == md5sum_on_local
+            ), f"md5sum has not changed after copying file on {vm_obj.name}"


should this be, md5sum changed?

PrasadDesala · 2024-12-09T13:43:28Z

tests/functional/cnv/test_vms.py

+            volume_name = vm.pvc_obj.get().get("spec").get("volumeName")
+            for line in run_oc_command(
+                f"describe pv {volume_name}", namespace=vm.namespace
+            ):
+                if "VolumeHandle:" in line:
+                    volume_handle = line.split()[1]
+                    break
+            if volume_handle is None:
+                logger.error(f"Cannot get volume handle for pv {volume_name}")
+                raise Exception("Cannot get volume handle")


this is for getting the pv name?

no for getting volume handle name of PV

PrasadDesala · 2024-12-09T13:44:27Z

tests/functional/cnv/test_vms.py

+        # 4.Stop all VMs
+        for vm_obj in self.vm_objs_def + self.vm_objs_aggr:
+            vm_obj.stop()


I don't this part of the test case? do we need this, shouldn't the fixture teardown take care of stopping the VM's?

No its not stopping. https://github.com/avd-sagare/ocs-ci/blob/d3862188710066bdb7bf02e6d6e6f1c424b2bbdb/ocs_ci/ocs/cnv/virtual_machine.py#L559

parikshithb · 2024-12-09T13:51:11Z

tests/conftest.py

+            compression="aggressive",
+            new_rbd_pool=True,
+        )
+        vm_configs = [


Imo, you can add these configs by create cnv_workload.yaml file

parikshithb · 2024-12-09T13:51:44Z

tests/conftest.py

+        log.info("csi-kms-connection-details setup successful")
+
+        # Create an encryption enabled storageclass for RBD
+        sc_obj_def_compr = storageclass_factory(


sc created should have ceph-rbd-virt sc params. Please check and patch if necessary.

parikshithb · 2024-12-09T13:51:53Z

tests/conftest.py

+            new_rbd_pool=True,
+        )
+
+        sc_obj_aggressive = storageclass_factory(


same as above

parikshithb · 2024-12-09T13:52:55Z

tests/conftest.py

+                source_url=constants.CNV_FEDORA_SOURCE,  # Assuming source_url is the same for all VMs
+                namespace=namespace,
+            )
+            vm_obj = vm_obj[-1]


can get the index of current iteration by enumerating

hnallurv · 2024-12-09T13:55:10Z

tests/conftest.py

+            compression="aggressive",
+            new_rbd_pool=True,
+        )
+        vm_configs = [


Please make sure you cover all RWX and at least one RWO configs from here

hnallurv · 2024-12-09T13:56:31Z

tests/conftest.py

+        kms = pv_encryption_kms_setup_factory(kv_version="v2")
+        log.info("csi-kms-connection-details setup successful")
+
+        # Create an encryption enabled storageclass for RBD


Please use rbd virtualization storageclass here.

tests/functional/cnv/test_vms.py

Signed-off-by: Avdhoot <[email protected]>

ocs-ci

PR validation on existing cluster

Cluster Name: asagare-cnv8
Cluster Configuration:
PR Test Suite:
PR Test Path: tests/functional/cnv/test_vms.py
Additional Test Params:
OCP VERSION: 4.18
OCS VERSION: 4.18
tested against branch: master

Job PASSED.

avd-sagare requested a review from a team as a code owner October 14, 2024 14:38

pull-request-size bot added the size/M PR that changes 30-99 lines label Oct 14, 2024

avd-sagare requested a review from parikshithb October 14, 2024 14:39

pull-request-size bot added size/L PR that changes 100-499 lines and removed size/M PR that changes 30-99 lines labels Oct 15, 2024

avd-sagare force-pushed the multi-cnv-workload branch 3 times, most recently from 8a1c795 to 5ded88a Compare November 19, 2024 05:54

avd-sagare force-pushed the multi-cnv-workload branch 2 times, most recently from 5d52616 to 616acfb Compare November 26, 2024 16:28

avd-sagare requested review from PrasadDesala and hnallurv December 2, 2024 05:22

avd-sagare added 7 commits December 2, 2024 10:54

create multiple cnv workloads

6aadc1a

Signed-off-by: Avdhoot <[email protected]>

upadte with storage class

620a8ea

Signed-off-by: Avdhoot <[email protected]>

Added fixture

ebbd1f9

Signed-off-by: Avdhoot <[email protected]>

changed fixure structure

a094fe4

Signed-off-by: Avdhoot <[email protected]>

Moved file

a098c55

Signed-off-by: Avdhoot <[email protected]>

Added testcase

1441c20

Signed-off-by: Avdhoot <[email protected]>

commeneted compression code

46283a5

Signed-off-by: Avdhoot <[email protected]>

avd-sagare force-pushed the multi-cnv-workload branch from 616acfb to 46283a5 Compare December 2, 2024 05:30

parikshithb reviewed Dec 2, 2024

View reviewed changes

Incorporated comments

d386218

Signed-off-by: Avdhoot <[email protected]>

parikshithb reviewed Dec 5, 2024

View reviewed changes

PrasadDesala reviewed Dec 9, 2024

View reviewed changes

parikshithb reviewed Dec 9, 2024

View reviewed changes

hnallurv reviewed Dec 9, 2024

View reviewed changes

tests/functional/cnv/test_vms.py Show resolved Hide resolved

Incorporated comments

4de4aca

Signed-off-by: Avdhoot <[email protected]>

Incorporated comments

6676e51

Signed-off-by: Avdhoot <[email protected]>

openshift-merge-robot added the needs-rebase label Dec 13, 2024

Merge branch 'master' into multi-cnv-workload

8ff8e3d

openshift-merge-robot removed the needs-rebase label Dec 13, 2024

avd-sagare force-pushed the multi-cnv-workload branch 2 times, most recently from e639641 to cd2b1bd Compare December 13, 2024 10:00

avd-sagare requested review from parikshithb, PrasadDesala and hnallurv December 13, 2024 10:01

Update conftest.py

77acbb7

Signed-off-by: Avdhoot <[email protected]>

avd-sagare force-pushed the multi-cnv-workload branch from 3ed5434 to 77acbb7 Compare December 13, 2024 13:19

This comment was marked as duplicate.

Sign in to view

This comment was marked as outdated.

Sign in to view

ocs-ci reviewed Dec 14, 2024

View reviewed changes

		@@ -7092,6 +7092,112 @@ def teardown():
		return factory


		@pytest.fixture

		Create a cnv factory. Calling this fixture Creates multiple VMs
		with specified configurations using the cnv_workload fixture.

create multiple cnv workloads #10673

Are you sure you want to change the base?

create multiple cnv workloads #10673

Conversation

avd-sagare commented Oct 14, 2024

openshift-ci bot commented Oct 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as duplicate.

This comment was marked as outdated.

ocs-ci left a comment

Choose a reason for hiding this comment