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

LVMCluster CRD changes #115

Merged
merged 3 commits into from
Feb 21, 2022
Merged

LVMCluster CRD changes #115

merged 3 commits into from
Feb 21, 2022

Conversation

nbalacha
Copy link
Contributor

The LVMCluster.Spec has been updated to add a "storage" field which will contain storage related configuration.

This change introduces a storage field in the
LVMCluster CRD which contains the deviceClasses.
This commit contains the API changes and manifests.

Signed-off-by: N Balachandran <[email protected]>
Code changes to use the updated LVMCluster definition.

Signed-off-by: N Balachandran <[email protected]>
The tests have been updated to use the updated LVMCluster
CRD definition.

Signed-off-by: N Balachandran <[email protected]>
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 17, 2022
@nbalacha nbalacha requested review from leelavg and sp98 and removed request for leelavg February 17, 2022 13:32
@@ -31,9 +31,9 @@ type LVMClusterSpec struct {
// Tolerations to apply to nodes to act on
// +optional
Tolerations []corev1.Toleration `json:"tolerations,omitempty"`
// DeviceClasses are a rules that assign local storage devices to volumegroups that are used for creating lvm based PVs
// Storage describes the deviceClass configuration for local storage devices

Choose a reason for hiding this comment

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

To keep it future-ready, should we update the description of the Storage field to be more generic or less aligned with DeviceClass; Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A good point. How about "Storage describes the configuration for local storage devices" ?

Choose a reason for hiding this comment

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

Sounds Great!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 21, 2022
@openshift-merge-robot openshift-merge-robot merged commit 3b3bac0 into openshift:main Feb 21, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 21, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leelavg, nbalacha, sp98

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

The pull request process is described 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

@nbalacha nbalacha deleted the crd-changes branch February 22, 2022 12:37
@nbalacha
Copy link
Contributor Author

/cherry-pick release-4.10

@openshift-cherrypick-robot

@nbalacha: new pull request created: #117

In response to this:

/cherry-pick release-4.10

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/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants