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
ceph: modify CephFS provisioner permission #8739
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After thoughts, I see you have a similar change in ceph/ceph-csi#2518. It's "a lot" of back and forth.
What if ceph-csi would provide its yaml files as code? So that Rook, when pulling a new version for ceph-csi (through go modules) can also get the updated yamls?
We don't have such a mechanism in cephcsi and we are not exposing any functions for external consumptions. |
Yes but you do have an examples directory right? And it needs to be synced with Rook all the time, right? |
Yes correct |
Honestly, it feels like an annoyance to me. @BlaineEXE @travisn thoughts? |
This would introduce a code dependency between rook and ceph-csi that we don't have today, right? But removing the duplication between repos would certainly be convenient. Anyways, a much bigger change separate from this PR |
I think we wouldn't need any code synchronization if the templates CSI needed were stored using Go 1.16's |
Actually, how would this change work? If the manifests are embedded in the CSI image rather than the Rook image, how would Rook create them? |
If manifest are stored with embed, the code still needs to parse and unmarshal in a known type, eg: |
But if rook is going to read the manifest stored with embed, doesn't it have to be embedded in the rook binary? In that case I don't see how we can move the manifest to the csi repo |
AFAIK yes we need to embed this to the Rook binary. again if we need that its adds more inter dependency between Rook and CSI releases and it's more work in cephcsi to support this one. |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for now, will bring the topic in the next CSI meeting.
@Madhu-1 The file test is failing consistently in the smoke suite:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI needs investigation
Am able to create PVC and mount to application pod locally with this change
Let me try to run CI locally. |
FYI, @Madhu-1 you can log into GitHub action runner |
Let me try to rerun the test tomorrow and see |
Yes, it has timeout of 1 hr.
|
ceph cluster is in error state. @travisn @subhamkrai anyone can help here? |
looks similar to this issue #8745 |
@leseb am frequently hitting the issue in the CI any idea what could be wrong? JFYI same template changes work fine in cephcsi ceph/ceph-csi#2518 and CI is passed |
As like RBD, CephFS provisioner pod need not to run as privileged. as its not doing any operation like plugin pods which does mounting and unmounting removing the permissions for the same. Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
fc61fe9
to
95775fd
Compare
Looks like you just needed to rebase. |
That helped Thank you. |
Hi, there, when will this be merged to release-1.7? Thank you. |
ceph: modify CephFS provisioner permission (backport #8739)
Merged now, and will be included in v1.7.4 planned today |
Description of your changes:
Like RBD, the CephFS provisioner pod need not run as privileged. as it's not doing any operation like plugin pods which does mounting and unmounting removing the permissions for the same.
Signed-off-by: Madhu Rajanna madhupr007@gmail.com
Which issue is resolved by this Pull Request:
Resolves #
Checklist:
make codegen
) has been run to update object specifications, if necessary.