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.

Sign up for GitHub

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

Jump to bottom

set pinning field in CephSubvolumeGroup CR #2317

Merged
merged 4 commits into from
Dec 13, 2023
Merged

set pinning field in CephSubvolumeGroup CR #2317

merged 4 commits into from
Dec 13, 2023

Conversation

Nikhil-Ladha
Copy link
Member

Updated the CephSubvolumeGroup CR to include the pinning field with a default value of Distributed: 1

Ref: https://issues.redhat.com/browse/RHSTOR-5125

@Nikhil-Ladha Nikhil-Ladha force-pushed the add-pinning-support-to-svg branch 2 times, most recently from 0467033 to 6c88d8e Compare December 12, 2023 06:43
iamniting
iamniting requested changes Dec 12, 2023
View reviewed changes
Copy link
Member

@iamniting iamniting left a comment

Choose a reason for hiding this comment

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

Is it tested?

controllers/storagecluster/cephfilesystem.go Outdated Show resolved Hide resolved
@Nikhil-Ladha Nikhil-Ladha force-pushed the add-pinning-support-to-svg branch 2 times, most recently from 275cc14 to 2d16c4f Compare December 12, 2023 07:10
umangachapagain
umangachapagain requested changes Dec 12, 2023
View reviewed changes
Copy link
Contributor

@umangachapagain umangachapagain left a comment

Choose a reason for hiding this comment

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

  • Break the commits into smaller logical parts.
    • go dependency changes should be first commit.
    • StorageCluster API and CRD changes should be second commit.
    • cephfilesystem changes should be third commit.
  • Add clear description of changes to each commit.
  • StorageCluster CRD should not be edited manually. Change the api/*_type.go file to make changes.

parth-gr
parth-gr suggested changes Dec 12, 2023
View reviewed changes
go.mod Show resolved Hide resolved
controllers/storagecluster/generate.go Show resolved Hide resolved
controllers/storagecluster/generate.go Outdated Show resolved Hide resolved
controllers/storagecluster/cephfilesystem.go Outdated Show resolved Hide resolved
controllers/storagecluster/cephfilesystem.go Outdated Show resolved Hide resolved
controllers/storagecluster/cephfilesystem.go Outdated Show resolved Hide resolved
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Dec 12, 2023

@parth-gr: changing LGTM is restricted to collaborators

In response to this:

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.

@Nikhil-Ladha Nikhil-Ladha marked this pull request as draft December 12, 2023 09:12
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 12, 2023
@Nikhil-Ladha Nikhil-Ladha force-pushed the add-pinning-support-to-svg branch 2 times, most recently from 51d768a to 1eda24f Compare December 12, 2023 09:16
@Nikhil-Ladha
Updated rook dependencies
1af758a 
Updated rook dependencies to include latest API changes

Signed-off-by: Nikhil-Ladha <[email protected]>
@Nikhil-Ladha Nikhil-Ladha force-pushed the add-pinning-support-to-svg branch from 1eda24f to 1af758a Compare December 12, 2023 09:33
@Nikhil-Ladha
Updated latest rook image
45eca74 
Updated latest rook image and the corresponding rook CRDs

Signed-off-by: Nikhil-Ladha <[email protected]>
@Nikhil-Ladha Nikhil-Ladha force-pushed the add-pinning-support-to-svg branch from 7c250e1 to 45eca74 Compare December 12, 2023 10:16
@Nikhil-Ladha Nikhil-Ladha force-pushed the add-pinning-support-to-svg branch 2 times, most recently from 2314063 to 5187aae Compare December 12, 2023 13:41
@Nikhil-Ladha Nikhil-Ladha marked this pull request as ready for review December 12, 2023 14:21
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 12, 2023
@Nikhil-Ladha Nikhil-Ladha force-pushed the add-pinning-support-to-svg branch from 5187aae to 517f607 Compare December 12, 2023 14:25
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Dec 12, 2023

@parth-gr: changing LGTM is restricted to collaborators

In response to this:

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.

@Nikhil-Ladha Nikhil-Ladha force-pushed the add-pinning-support-to-svg branch from 517f607 to 4070fee Compare December 12, 2023 14:46
@Nikhil-Ladha Nikhil-Ladha force-pushed the add-pinning-support-to-svg branch from 4070fee to 284f42a Compare December 12, 2023 14:49
parth-gr
parth-gr approved these changes Dec 12, 2023
View reviewed changes
Copy link
Member

@parth-gr parth-gr left a comment

Choose a reason for hiding this comment

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

lgtm

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Dec 12, 2023

@parth-gr: changing LGTM is restricted to collaborators

In response to this:

lgtm

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.

@Nikhil-Ladha Nikhil-Ladha force-pushed the add-pinning-support-to-svg branch 2 times, most recently from 4442521 to a37b23a Compare December 12, 2023 16:39
@Nikhil-Ladha
set pinning field in CephFilesystemSubvolumeGroup CR
b352070 
Updated the CephSubvolumeGroup CR to include the pinning field
with a default value of Distributed: 1

Signed-off-by: Nikhil-Ladha <[email protected]>
@Nikhil-Ladha Nikhil-Ladha force-pushed the add-pinning-support-to-svg branch from 6897acb to b352070 Compare December 12, 2023 16:42
@Nikhil-Ladha
update golangci-lint to 1.51.2
2df5c6e 
updated golangci-lint to version 1.51.2 in github actions

Signed-off-by: Nikhil-Ladha <[email protected]>
@Nikhil-Ladha Nikhil-Ladha force-pushed the add-pinning-support-to-svg branch from 348fd5f to 2df5c6e Compare December 12, 2023 16:49
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 13, 2023
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Dec 13, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: iamniting, Nikhil-Ladha, parth-gr

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 13, 2023
@iamniting iamniting merged commit e8d575d into red-hat-storage:main Dec 13, 2023
13 of 15 checks passed
@Nikhil-Ladha Nikhil-Ladha deleted the add-pinning-support-to-svg branch February 28, 2024 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@parth-gr parth-gr parth-gr approved these changes

@iamniting iamniting iamniting approved these changes

@umangachapagain umangachapagain Awaiting requested review from umangachapagain

Assignees

@umangachapagain umangachapagain

@iamniting iamniting

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Nikhil-Ladha @umangachapagain @iamniting @parth-gr