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

csi: change the fsgrouppolicy for CephFS driver to "File" mode #10503

Merged
merged 1 commit into from Aug 23, 2022

Conversation

humblec
Copy link
Contributor

@humblec humblec commented Jun 24, 2022

At present the CephFS CSI driver works with default mode ie,
ReadWriteOncewithFsType. However File type is more apt for
the CephFS CSI driver and this commit bring that change. The
similar change is also introduced in ceph csi driver here:

ceph/ceph-csi#3204

considering the File mode has been GAd in 1.23 kubernetes version
and also we are lifting one of the problematic code path in this
area via Ceph CSI driver changes, it is good to move the
fsgrouppolicy to File.

Signed-off-by: Humble Chirammal hchiramm@redhat.com

Description of your changes:

Which issue is resolved by this Pull Request:
Resolves #

Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide).
  • Skip Tests for Docs: If this is only a documentation change, add the label skip-ci on the PR.
  • Reviewed the developer guide on Submitting a Pull Request
  • Pending release notes updated with breaking and/or notable changes for the next minor release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

At present the CephFS CSI driver works with default mode ie,
`ReadWriteOncewithFsType`. However `File` type is more apt for
the CephFS CSI driver and this commit bring that change. The
similar change is also introduced in ceph csi driver here:

ceph/ceph-csi#3204

considering the `File` mode has been GAd in 1.23 kubernetes version
and also we are lifting one of the problematic code path in this
area via Ceph CSI driver changes, it is good to move the
fsgrouppolicy to `File`.

Signed-off-by: Humble Chirammal <hchiramm@redhat.com>
@humblec
Copy link
Contributor Author

humblec commented Jun 24, 2022

Cc @travisn

@travisn
Copy link
Member

travisn commented Jun 24, 2022

considering the File mode has been GAd in 1.23 kubernetes version

Are there any concerns with File mode before it GA'd? Just wanting to confirm if it's really a good option before 1.23.

@humblec
Copy link
Contributor Author

humblec commented Jun 27, 2022

considering the File mode has been GAd in 1.23 kubernetes version

Are there any concerns with File mode before it GA'd? Just wanting to confirm if it's really a good option before 1.23.

Not that I am aware of. afaict, It looked to be on spot since its introduction. The feature gate got enabled in 1.20 by default where it was alpha in 1.19. considering we are falling back to v1beta1 driver in that version , we are good there as well. @travisn

@travisn
Copy link
Member

travisn commented Jun 27, 2022

@humblec Do we need ceph/ceph-csi#3204 in a csi release before we merge this?

@humblec
Copy link
Contributor Author

humblec commented Jul 12, 2022

@humblec Do we need ceph/ceph-csi#3204 in a csi release before we merge this?

@travisn true.. its good to have it in before this . However unfortunately that PR in merge queue for last 2 weeks with CI failing spuriously, pretty much the same case with all other PRs in the CSI repo !! Will notify once the CI passes and merged..

@humblec
Copy link
Contributor Author

humblec commented Jul 13, 2022

@travisn jfyi , finally its merged..

@travisn
Copy link
Member

travisn commented Jul 13, 2022

@travisn jfyi , finally its merged..

Ok, when is the next ceph-csi release planned? I assume we need to wait for that before merging this?

@humblec
Copy link
Contributor Author

humblec commented Jul 27, 2022

@travisn jfyi , finally its merged..

Ok, when is the next ceph-csi release planned? I assume we need to wait for that before merging this?

@travisn it was planned a couple of weeks back, however on further discussion to align with odf release it has been postponed to 3rd week of August. So we can wait till then.

Additionally, I was thinking, cant we keep Rook master in parity or consuming canary images of ceph csi driver ? and while we cut the release of rook, may be tag the csi image to latest available version ? which also allow us to go on the edge and correct issues without a CSI release ...etc ?

@travisn
Copy link
Member

travisn commented Jul 27, 2022

Per discussion with csi team, we will plan on keeping the csi version label even in rook master, and we can add a daily CI test against the csi canary tag.

@humblec
Copy link
Contributor Author

humblec commented Aug 23, 2022

Per discussion with csi team, we will plan on keeping the csi version label even in rook master, and we can add a daily CI test against the csi canary tag.

@travisn considering Ceph CSI 3.7 is out can we please merge this ?

@travisn
Copy link
Member

travisn commented Aug 23, 2022

Per discussion with csi team, we will plan on keeping the csi version label even in rook master, and we can add a daily CI test against the csi canary tag.

@travisn considering Ceph CSI 3.7 is out can we please merge this ?

@humblec Remind me, what happens if this File setting is applied and a previous version of ceph-csi (e.g. 3.6) is deployed? Do we need a version check to change it just like for K8s 1.19?

@humblec
Copy link
Contributor Author

humblec commented Aug 23, 2022

Per discussion with csi team, we will plan on keeping the csi version label even in rook master, and we can add a daily CI test against the csi canary tag.

@travisn considering Ceph CSI 3.7 is out can we please merge this ?

@humblec Remind me, what happens if this File setting is applied and a previous version of ceph-csi (e.g. 3.6) is deployed? Do we need a version check to change it just like for K8s 1.19?

@travisn I dont anticipate an issue if its operating on file + 3.6 as were opening full permission on mount in that version. Additionally, 3.6 driver deployment should also bring its own csiDriver object.

@travisn travisn merged commit f02ffef into rook:master Aug 23, 2022
47 of 48 checks passed
@humblec
Copy link
Contributor Author

humblec commented Aug 24, 2022

Thanks @travisn 👍

travisn added a commit that referenced this pull request Aug 31, 2022
csi: change the fsgrouppolicy for CephFS driver to "File" mode (backport #10503)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants