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

fsgroupolicy changes for powerscale #159

Closed
wants to merge 4 commits into from

Conversation

nitesh3108
Copy link
Contributor

@nitesh3108 nitesh3108 commented Feb 25, 2022

Description

A few sentences describing the overall goals of the pull request's commits.

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
dell/csm#167

Checklist:

  • Have you run a grammar and spell checks against your submission?
  • Have you tested the changes locally?
  • Have you tested whether the hyperlinks are working properly?
  • Did you add the examples wherever applicable?
  • Have you added high-resolution images?

prablr79
prablr79 previously approved these changes Feb 25, 2022
@prablr79
Copy link
Collaborator

@nitesh3108 Please move this PR release branch

@nitesh3108 nitesh3108 closed this Feb 25, 2022
@nitesh3108 nitesh3108 reopened this Feb 25, 2022
@nitesh3108 nitesh3108 changed the base branch from main to release-1.2 February 25, 2022 10:05
@nitesh3108 nitesh3108 dismissed prablr79’s stale review February 25, 2022 10:05

The base branch was changed.

@nitesh3108 nitesh3108 requested a review from prablr79 February 25, 2022 10:05
prablr79
prablr79 previously approved these changes Feb 25, 2022
Copy link
Collaborator

@prablr79 prablr79 left a comment

Choose a reason for hiding this comment

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

lgtm

walker2
walker2 previously approved these changes Feb 25, 2022
@prablr79
Copy link
Collaborator

@nitesh3108 please resolve the conflicts

Copy link
Collaborator

@shanmydell shanmydell left a comment

Choose a reason for hiding this comment

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

Pls rebase ur branch


fsGroupPolicy will be default to "ReadWriteOnceWithFSType", keeping the previous behavior.

Note: FSGroupPolicy may not work as expected with "root_squash", to get the desired behavior "no_root_squash" has to be enabled.
Copy link
Contributor

Choose a reason for hiding this comment

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

Pls mention in terms of RootClientEnabled and not root squash.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

root_squash and no_root_squash must be enough.
i.e.
no_root_squash --> RootClient=true
root_squash --> RootClient=false

Copy link
Contributor

Choose a reason for hiding this comment

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

Still we should go with the option what we provide.

@randeepdell
Copy link
Contributor

I see multiple changes for other drivers, it that expected to be in this PR ?

@shanmydell shanmydell dismissed stale reviews from walker2 and prablr79 via e62e399 February 25, 2022 10:50
@shanmydell shanmydell force-pushed the fsgrouppolicy-powerscale branch from e3d32ef to e62e399 Compare February 25, 2022 10:50
@@ -565,3 +565,19 @@ When this feature is enabled, the existing `ReadWriteOnce(RWO)` access mode rest

To migrate existing PersistentVolumes to use `ReadWriteOncePod`, please follow the instruction from [here](https://kubernetes.io/blog/2021/09/13/read-write-once-pod-access-mode-alpha/#migrating-existing-persistentvolumes).

## FSGroupPolicy
Copy link
Contributor

Choose a reason for hiding this comment

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

@nitesh3108 I don't think we need to mention this under feature since this is already available by default even in the previous release right? Also, we have not implemented anything and we just gave the option to configure with values file
I haven't mentioned this as a feature in csi-powerstore docs.

@nitesh3108 nitesh3108 closed this Feb 25, 2022
@shanmydell shanmydell deleted the fsgrouppolicy-powerscale branch March 14, 2022 06:43
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.

8 participants