Create new test for create multiple device classes #11096

yitzhak12 · 2025-01-02T17:12:37Z

See more details in the Jira issue: https://issues.redhat.com/browse/RHSTOR-4795 and in the test plan: https://docs.google.com/spreadsheets/d/1CL9rFM8kFcO4WNNOH2L_oTpH4qgGGTFm45GtoQshSDs/edit?gid=1729233278#gid=1729233278

The test will perform the following steps:
1. Get the osd nodes.
2. Create a new LocalVolumeSet for the new device class as defined in the function
'create_new_lvs_for_new_deviceclass'.
3. Wait for the PVS in the LocalVolumeSet above to be available.
4. Add a new device set in the storagecluster for the new LocalVolumeSet above, which will also create a new device class.
5. Wait for the storagecluster to be ready.
6. Create a new CephBlockPool for the device class above.
7. Wait for the CephBlockPool to be ready.
8. Create a new StorageClass for the pool.
9. Run the Verification steps defined in the function 'verification_steps_after_adding_new_deviceclass'.
10. Check the cluster and Ceph health.
11. Check basic cluster functionality by creating some resources.

To do in the following PR:

Add a few more ceph verification steps.
Test add a new SSD device class with FileSystem.

openshift-ci · 2025-01-02T17:12:43Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: yitzhak12

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

- Create new test for create multiple device classes - Create new Yaml files for multiple device classes - Create new helper file for multiple device classes - Create new functions for creating the LVS, add new deviceset in the storagecluster, create CephBlockpool, create deviceclass storageclass for multiple device classes Signed-off-by: Itzhak Kave <[email protected]>

yitzhak12 · 2025-01-20T15:07:35Z

The test passed in my local.

ebenahar

great work Itzhak!
If possible, suggest breaking this PR into several smaller PRs.
This way it will be easier to merge

ebenahar · 2025-01-22T20:34:45Z

ocs_ci/helpers/helpers.py

@@ -5630,3 +5630,158 @@ def apply_custom_taint_and_toleration(taint_label="xyz"):
            )
            for pod_obj in pod_list:
                pod_obj.delete(wait=False)
+
+
+def create_ceph_block_pool_for_deviceclass(


I suggest trying to integrate deviceclass into the existing helper function of create_ceph_block_pool() - https://github.com/red-hat-storage/ocs-ci/blob/3d8e5d8f68210023671c50e10f0ba3b81bca3035/ocs_ci/helpers/helpers.py#L576C5-L576C27

This will prevent duplicity

Yes, I thought of that, but the Yaml files have some diffs.

ebenahar · 2025-01-22T20:38:22Z

ocs_ci/helpers/helpers.py

+    return lvs_obj
+
+
+def create_deviceclass_storageclass(


same here. Please try to integrate the deviceclass aspect into the existing function of SC creation -

ocs-ci/ocs_ci/helpers/helpers.py

Line 745 in 3d8e5d8

def create_storage_class(

I will try to see what the differences are.

ebenahar · 2025-01-22T20:39:55Z

ocs_ci/helpers/multiple_device_classes.py

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


this file could be used for anything related to device classes and not only for the multiple aspect of it.
Hence, suggest renaming the file to device_class.py and better to place it under ocs_ci/ocs

Yes, I will change the name.

ebenahar · 2025-01-22T20:40:41Z

ocs_ci/helpers/multiple_device_classes.py

+    """
+    osd_size = get_storage_size()
+    log.info(f"the osd size is {osd_size}")
+    old_lvs_max_size = sum_of_two_storage_sizes(osd_size, "30Gi")


what's behind "30Gi"?

I thought it was a reasonable maximum limit—nothing special in that number.

ebenahar · 2025-01-22T20:42:52Z

ocs_ci/ocs/cluster.py

@@ -3823,3 +3823,51 @@ def get_age_of_cluster_in_days():
    seconds_per_day = 24 * 60 * 60
    time_diff_in_days = time_difference_in_sec / seconds_per_day
    return math.ceil(time_diff_in_days)
+
+
+def get_ceph_pool_list():


we have a similar function here

ocs-ci/ocs_ci/ocs/rados_utils.py

Line 124 in 3d8e5d8

def list_pools(self):

I will check

ebenahar · 2025-01-22T20:43:18Z

ocs_ci/ocs/resources/pod.py

+        if pvc:
+            return pvc.get("claimName")
+
+    return None


this is redundant

ebenahar · 2025-01-22T20:45:20Z

ocs_ci/ocs/resources/pod.py

@@ -4013,3 +4014,42 @@ def wait_for_ceph_cmd_execute_successfully(
        f"The ceph command failed to execute successfully after {num_of_retries} retries"
    )
    return False
+
+
+def get_osd_pod_pvc(osd_pod):


you can use this existing function instead

ocs-ci/ocs_ci/ocs/resources/pod.py

Line 1943 in 3d8e5d8

def get_pvc_name(pod_obj):

I saw the function get_pvc_name, but this will not work with getting the osd PVC. Should I update the function with my changes of searching the first occurrence of the persistentVolumeClaim?

ebenahar · 2025-01-22T20:46:40Z

tests/functional/z_cluster/cluster_expansion/test_create_multiple_device_classes.py

+
+
+@brown_squad
+@tier1


I don't think these tests are considered function for the product. I would say these are more advanced scenarios and we can mark them with tier2

Why not? This is something we expect from the client, like adding capacity or resizing the osd.

openshift-merge-robot · 2025-01-23T22:30:18Z

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

yitzhak12 added the new feature label Jan 2, 2025

yitzhak12 requested review from a team as code owners January 2, 2025 17:12

openshift-ci bot added the do-not-merge/work-in-progress label Jan 2, 2025

pull-request-size bot added the size/L PR that changes 100-499 lines label Jan 2, 2025

yitzhak12 force-pushed the test-create-multiple-device-classes branch from e150cdf to a42d51d Compare January 5, 2025 16:34

openshift-merge-robot added the needs-rebase label Jan 5, 2025

yitzhak12 force-pushed the test-create-multiple-device-classes branch from a42d51d to 2726dc2 Compare January 6, 2025 15:45

pull-request-size bot added size/XL and removed size/L PR that changes 100-499 lines labels Jan 6, 2025

openshift-merge-robot removed the needs-rebase label Jan 6, 2025

yitzhak12 force-pushed the test-create-multiple-device-classes branch 3 times, most recently from 9229e9a to b713c44 Compare January 20, 2025 14:03

yitzhak12 changed the title ~~WIP: Create new test for create multiple device classes~~ Create new test for create multiple device classes Jan 20, 2025

openshift-ci bot removed the do-not-merge/work-in-progress label Jan 20, 2025

yitzhak12 force-pushed the test-create-multiple-device-classes branch from b713c44 to 5b12522 Compare January 20, 2025 14:24

yitzhak12 added the Verified Mark when PR was verified and log provided label Jan 21, 2025

ebenahar reviewed Jan 22, 2025

View reviewed changes

openshift-merge-robot added the needs-rebase label Jan 23, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create new test for create multiple device classes #11096

Create new test for create multiple device classes #11096

yitzhak12 commented Jan 2, 2025 •

edited

Loading

openshift-ci bot commented Jan 2, 2025

yitzhak12 commented Jan 20, 2025

ebenahar left a comment

ebenahar Jan 22, 2025

yitzhak12 Jan 23, 2025

ebenahar Jan 22, 2025

yitzhak12 Jan 23, 2025

ebenahar Jan 22, 2025

yitzhak12 Jan 23, 2025

ebenahar Jan 22, 2025

yitzhak12 Jan 23, 2025

ebenahar Jan 22, 2025

yitzhak12 Jan 23, 2025

ebenahar Jan 22, 2025

yitzhak12 Jan 23, 2025

ebenahar Jan 22, 2025

yitzhak12 Jan 23, 2025

ebenahar Jan 22, 2025

yitzhak12 Jan 23, 2025

openshift-merge-robot commented Jan 23, 2025



		@brown_squad
		@tier1

Create new test for create multiple device classes #11096

Are you sure you want to change the base?

Create new test for create multiple device classes #11096

Conversation

yitzhak12 commented Jan 2, 2025 • edited Loading

To do in the following PR:

openshift-ci bot commented Jan 2, 2025

yitzhak12 commented Jan 20, 2025

ebenahar left a 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

openshift-merge-robot commented Jan 23, 2025

yitzhak12 commented Jan 2, 2025 •

edited

Loading