Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding support in creation of bucket class with mcg cli and a new test case for creation of rgw namespacestore using mcg-cli #11119

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ypersky1980
Copy link
Contributor

This PR implements the following enhancement to cli_create_bucketclass method : in the past it was creating only backingstore with mcg-cli and from now on it will be able to create a bucketclass with mcg-cli command also for namespacestore.

Following this enhancement it was possible to add a new test case which implements creation of namespacestore and bucketclass for rgw using mcg-cli commands.

In the next PR I will implement automation of https://issues.redhat.com/browse/DFBUGS-1035 ( creating namespacestore and bucketclass on rgw bucket and writing IO to this bucket).

…t case for creation of rgw namespace using mcg-cli

Signed-off-by: Yulia Persky <[email protected]>
@ypersky1980 ypersky1980 requested review from a team as code owners January 7, 2025 13:19
@pull-request-size pull-request-size bot added the size/M PR that changes 30-99 lines label Jan 7, 2025
Copy link

openshift-ci bot commented Jan 7, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ypersky1980

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

Needs approval from an approver in each of these files:

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: Yulia Persky <[email protected]>
Signed-off-by: Yulia Persky <[email protected]>
Signed-off-by: Yulia Persky <[email protected]>
f"bucketclass create {placement_type}{name}{backingstores}{placement_policy}{replication_policy}"
)
elif namespacestores: # bucket class is over namespacestores
namestores_name_list = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could hijack the existing logic for backingstores if we replace "backingstores" with "stores", and check if we're here for namespaces based on whether namespace_policy is not None.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sagihirshfeld Following our conversation, I'm going to resolve this comment by separating cli_create_bucketclass method to two methods:
cli_create_bucketclass_over_backingspacestores and cli_create_bucketclass_over_namespacestores .

@@ -209,10 +209,12 @@ def _create_bucket_class(bucket_class_dict):
interfaces[interface.lower()](
name=bucket_class_name,
backingstores=backingstores,
namespacestores=namespacestores,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will cause an error when interface.lower == 'oc' because mcg.py::MCG::oc_create_bucketclass doesn't have this argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sagihirshfeld Thank you for pointing this out. The method mcg.py::MCG::oc_create_bucketclass will be called now ( after the last commit) with the correct arguments.

…ucketclass_over_backingspacestores and cli_create_bucketclass_over_namespacestores

Signed-off-by: Yulia Persky <[email protected]>
replication_policy=None,
):
"""
Creates a new NooBaa bucket class using the noobaa cli
Creates a new NooBaa bucket class using the noobaa cli over either backingstores
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Creates a new NooBaa bucket class using the noobaa cli over either backingstores
Creates a new NooBaa bucket class using the noobaa cli over backingstores

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thanks

Comment on lines +761 to +764
self.exec_mcg_cmd(
f"bucketclass create namespace-bucketclass {namespace_policy_type} "
f"--resource={namestores_name_str} {name}"
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is correct for the single NSS type, but please also address the multi and cache options.

For the scope of this PR, just raise a NotImplementedError for the other two types. We'll implement those in follow-up PRs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M PR that changes 30-99 lines Squad/Red
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants