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 new SC and Pool for RBD #1756

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rakeshgm
Copy link
Member

@rakeshgm rakeshgm commented Jan 20, 2025

This commit adds a new storage class and pool for RBD, which allows the ramen code base to exercise the filtering logic when there are multiple storage classes in the environment. This also prepares the environment for scenarios where more than two workloads use different storage classes.

@rakeshgm rakeshgm requested review from nirs and ShyamsundarR January 20, 2025 12:39
@rakeshgm rakeshgm force-pushed the drenv-multisc branch 8 times, most recently from 8b884a0 to 8760c02 Compare January 23, 2025 10:18
Copy link
Member

@nirs nirs left a comment

Choose a reason for hiding this comment

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

Missing the big picture but changes look very nice!


def deploy(cluster):
scs = [
Copy link
Member

Choose a reason for hiding this comment

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

storage_classes would be nicer and consistent with other names like POOL_NAMES

Copy link
Member Author

Choose a reason for hiding this comment

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

renamed the variable.

for sc in scs:
template = drenv.template("storage-class.yaml")
yaml = template.substitute(
cluster=cluster, name=sc.get("name"), pool=sc.get("pool")
Copy link
Member

Choose a reason for hiding this comment

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

sc.get() will return None if we have a typo. It should be used when you want to support missing key, possibly adding a default. Since missing key is a bug in this code we want to fail loudly in this case, and the right way is:

cluster=cluster, name=sc["name"], pool=sc["pool"]

Copy link
Member Author

Choose a reason for hiding this comment

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

i have made the changes.

This commit adds a new storage class and pool for RBD, which allows the
ramen code base to exercise the filtering logic when there are multiple
storage classes in the environment. This also prepares the environment
for scenarios where more than two workloads use different storage
classes.

Signed-off-by: rakeshgm <[email protected]>
@rakeshgm
Copy link
Member Author

Missing the big picture but changes look very nice!

I have updated the PR description and the commit message

@rakeshgm rakeshgm marked this pull request as ready for review January 24, 2025 11:02
@nirs nirs self-requested a review February 4, 2025 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants