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

cephfs: go with default permissions while creating subvolumes #3204

Merged
merged 1 commit into from Jul 13, 2022

Conversation

humblec
Copy link
Collaborator

@humblec humblec commented Jun 23, 2022

While creating subvolumes, CephFS driver set the mode to 777
and pass it along to go ceph apis which cause the subvolume
permission to be on 777, however if we create a subvolume
directly in the ceph cluster, the default permission bits are
set which is 755 for the subvolume. This commit try to stick
to the default behaviour even while creating the subvolume.

This also means that we can work with fsgrouppolicy set to
File in csiDriver object which is also addressed in this commit.

Additional Note for reviewer:

Rook Changes: rook/rook#10503

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

@mergify mergify bot added the component/cephfs Issues related to CephFS label Jun 23, 2022
@humblec humblec changed the title cephfs: go with default permissions while creating subvolumes [wip] cephfs: go with default permissions while creating subvolumes Jun 23, 2022
@humblec humblec changed the title [wip] cephfs: go with default permissions while creating subvolumes cephfs: go with default permissions while creating subvolumes Jun 23, 2022
@humblec humblec requested review from Madhu-1 and nixpanic June 23, 2022 13:26
@humblec
Copy link
Collaborator Author

humblec commented Jun 23, 2022

Jun 23 15:21:45.090: INFO: ExecWithOptions: execute(POST https://192.168.39.23:8443/api/v1/namespaces/rook-ceph/pods/rook-ceph-tools-f87879d85-7c26z/exec?command=%2Fbin%2Fsh&command=-c&command=rbd+ls+--format%3Djson+--pool%3Dreplicapool&container=rook-ceph-tools&container=rook-ceph-tools&stderr=true&stdout=true)
Jun 23 15:21:45.470: FAIL: backend images not matching kubernetes resource count,image count 22 kubernetes resource count 21

known one.

@humblec
Copy link
Collaborator Author

humblec commented Jun 24, 2022

/retest ci/centos/mini-e2e-helm/k8s-1.21

@humblec
Copy link
Collaborator Author

humblec commented Jun 24, 2022

/retest ci/centos/mini-e2e/k8s-1.22

@humblec humblec requested a review from a team June 24, 2022 05:27
@humblec humblec added this to the release-3.7 milestone Jun 24, 2022
@humblec
Copy link
Collaborator Author

humblec commented Jun 24, 2022

@Madhu-1ptal.. thanks

@@ -9,3 +9,4 @@ metadata:
spec:
attachRequired: false
podInfoOnMount: false
fsGroupPolicy: File
Copy link
Collaborator

Choose a reason for hiding this comment

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

@humblec can you provide details on why it should be File here, we have set fsGroupPolicy to ReadWriteOnceWithFSType in Rook for cephfs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Madhu-1 dropping the PR for Rook to change that.. pretty much ready .. I will link the same here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Madhu-1 rook/rook#10503 address the same

humblec added a commit to humblec/rook that referenced this pull request 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>
@humblec
Copy link
Collaborator Author

humblec commented Jun 24, 2022

/retest ci/centos/mini-e2e-helm/k8s-1.23

@humblec
Copy link
Collaborator Author

humblec commented Jun 24, 2022

/retest ci/centos/mini-e2e/k8s-1.22

@humblec
Copy link
Collaborator Author

humblec commented Jun 24, 2022

/retest ci/centos/mini-e2e-helm/k8s-1.21

@humblec humblec requested a review from a team June 24, 2022 10:13
@humblec
Copy link
Collaborator Author

humblec commented Jun 27, 2022

/retest ci/centos/mini-e2e-helm/k8s-1.21

1 similar comment
@humblec
Copy link
Collaborator Author

humblec commented Jun 27, 2022

/retest ci/centos/mini-e2e-helm/k8s-1.21

@humblec
Copy link
Collaborator Author

humblec commented Jun 29, 2022

@Mergifyio refresh

@humblec
Copy link
Collaborator Author

humblec commented Jun 29, 2022

/retest ci/centos/mini-e2e-helm/k8s-1.21

@mergify
Copy link
Contributor

mergify bot commented Jun 29, 2022

refresh

✅ Pull request refreshed

@humblec
Copy link
Collaborator Author

humblec commented Jun 29, 2022

/retest ci/centos/mini-e2e/k8s-1.22

@humblec
Copy link
Collaborator Author

humblec commented Jun 29, 2022

/retest ci/centos/mini-e2e-helm/k8s-1.21

@humblec
Copy link
Collaborator Author

humblec commented Jun 29, 2022

/retest ci/centos/mini-e2e-helm/k8s-1.23

@humblec
Copy link
Collaborator Author

humblec commented Jul 12, 2022

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Jul 12, 2022

rebase

✅ Branch has been successfully rebased

@humblec
Copy link
Collaborator Author

humblec commented Jul 12, 2022

/retest ci/centos/k8s-e2e-external-storage/1.23

@humblec
Copy link
Collaborator Author

humblec commented Jul 12, 2022

/retest ci/centos/k8s-e2e-external-storage/1.22

@humblec
Copy link
Collaborator Author

humblec commented Jul 12, 2022

2 weeks to get the CI pass with merge commit !!! still not the PR in..

@humblec
Copy link
Collaborator Author

humblec commented Jul 12, 2022

[Show complete log](https://jenkins-ceph-csi.apps.ocp.ci.centos.org/blue/rest/organizations/jenkins/pipelines/mini-e2e-helm_k8s-1.22/runs/3737/nodes/99/steps/102/log/?start=0)

ls Stdin:<nil> CaptureStdout:true CaptureStderr:true PreserveWhitespace:true Quiet:false}

Jul 12 10:34:59.038: INFO: >>> kubeConfig: /root/.kube/config

Jul 12 10:34:59.039: INFO: ExecWithOptions: Clientset creation

Jul 12 10:34:59.040: INFO: ExecWithOptions: execute(POST https://192.168.39.249:8443/api/v1/namespaces/rook-ceph/pods/rook-ceph-tools-6944779c47-s9xtt/exec?command=%2Fbin%2Fsh&command=-c&command=rbd+image-meta+get+--pool%3Dreplicapool+--image%3Dcsi-vol-d9824c79-01c5-11ed-b9d0-da8054cf23ee+csi.storage.k8s.io%2Fvolumesnapshot%2Fnamespace&container=rook-ceph-tools&container=rook-ceph-tools&stderr=true&stdout=true)

Jul 12 10:35:00.093: INFO: stdErr occurred: failed to get metadata csi.storage.k8s.io/volumesnapshot/namespace of image : (2) No such file or directory

rbd: getting metadata failed: (2) No such file or directory

https://jenkins-ceph-csi.apps.ocp.ci.centos.org/blue/organizations/jenkins/mini-e2e-helm_k8s-1.22/detail/mini-e2e-helm_k8s-1.22/3737/pipeline

@humblec
Copy link
Collaborator Author

humblec commented Jul 12, 2022

/retest ci/centos/mini-e2e/k8s-1.21

@humblec
Copy link
Collaborator Author

humblec commented Jul 12, 2022

/retest ci/centos/mini-e2e-helm/k8s-1.23

@humblec
Copy link
Collaborator Author

humblec commented Jul 12, 2022

/retest ci/centos/mini-e2e-helm/k8s-1.22

@humblec
Copy link
Collaborator Author

humblec commented Jul 12, 2022

/retest ci/centos/k8s-e2e-external-storage/1.23

@humblec
Copy link
Collaborator Author

humblec commented Jul 12, 2022

/retest ci/centos/mini-e2e-helm/k8s-1.22

While creating subvolumes, CephFS driver set the mode to `777`
and pass it along to go ceph apis which cause the subvolume
permission to be on 777, however if we create a subvolume
directly in the ceph cluster, the default permission bits are
set which is 755 for the subvolume. This commit try to stick
to the default behaviour even while creating the subvolume.

This also means that we can work with fsgrouppolicy set to
`File` in csiDriver object which is also addressed in this commit.

Signed-off-by: Humble Chirammal <hchiramm@redhat.com>
@humblec humblec added the ci/retry/e2e Label to retry e2e retesting on approved PR's label Jul 13, 2022
@ceph-csi-bot
Copy link
Collaborator

/retest ci/centos/mini-e2e-helm/k8s-1.22

@ceph-csi-bot
Copy link
Collaborator

@humblec "ci/centos/mini-e2e-helm/k8s-1.22" test failed. Logs are available at location for debugging

@ceph-csi-bot
Copy link
Collaborator

/retest ci/centos/k8s-e2e-external-storage/1.22

@ceph-csi-bot
Copy link
Collaborator

@humblec "ci/centos/k8s-e2e-external-storage/1.22" test failed. Logs are available at location for debugging

@ceph-csi-bot
Copy link
Collaborator

/retest ci/centos/k8s-e2e-external-storage/1.21

@ceph-csi-bot
Copy link
Collaborator

@humblec "ci/centos/k8s-e2e-external-storage/1.21" test failed. Logs are available at location for debugging

@ceph-csi-bot
Copy link
Collaborator

@Mergifyio requeue

@mergify
Copy link
Contributor

mergify bot commented Jul 13, 2022

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

@ceph-csi-bot
Copy link
Collaborator

@Mergifyio requeue

@mergify
Copy link
Contributor

mergify bot commented Jul 13, 2022

requeue

☑️ This pull request is already queued

@humblec
Copy link
Collaborator Author

humblec commented Jul 13, 2022

/retest ci/centos/mini-e2e-helm/k8s-1.21

@mergify mergify bot merged commit 1856647 into ceph:devel Jul 13, 2022
25 checks passed
mergify bot pushed a commit to rook/rook that referenced this pull request Aug 23, 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>
(cherry picked from commit 58a2f66)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/retry/e2e Label to retry e2e retesting on approved PR's component/cephfs Issues related to CephFS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants