-
Notifications
You must be signed in to change notification settings - Fork 224
PVC mount support #462
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
PVC mount support #462
Conversation
42c0fb5 to
ee2dcdf
Compare
ee2dcdf to
0ba4c5f
Compare
0ba4c5f to
ecf4658
Compare
|
I got everything working here according to the CSI examples. I DO NOT expect anyone else to be able to reproduce the same environment, but if you want to try giving it a whirl... Here goes:
|
ecf4658 to
92c3d07
Compare
|
Note: this is blocked until kflansburg/k8s-csi#1 is merged and a new version of k8s-csi is released, as it will not compile until it's been re-compiled against tonic 0.3. I've pointed Krustlet at a temporary fork for the time being. |
thomastaylor312
left a comment
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.
This is looking pretty good! We definitely should document all the steps required (basically at least the components needed) and a bug "THIS IS VERY ALPHA" label somewhere
kflansburg
left a comment
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.
I don't understand this enough to approve but had some comments.
itowlson
left a comment
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.
I have basically no idea whether this is right or wrong, but I sure do have a lot of opinions about how to organise code I don't understand!
894c7ca to
8559b4b
Compare
bb4a0ca to
638031b
Compare
thomastaylor312
left a comment
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.
Docs look really good. I realized we were missing some things this second go around
This is a simple change that allows the registrar to be built for MacOS. All of the `unix` package being used works for Mac as well. The reason for enabling this is completely selfish I'll admit. We are adding [CSI Support](krustlet/krustlet#462) to the Krustlet project, which supports Mac as well. As most CSI implementations don't support registering themselves with the Kubelet directly, we wanted to unblock any users to need to register their plugins with a Krustlet Kubelet. This change shouldn't affect normal container-based usage of the registrar
480dd32 to
9acdac1
Compare
|
Looks like the doc tests are failing @bacongobbler. I also am seeing a lot of clippy errors, but I can fix those later |
|
Oh yeah I see that. Never run the doc tests locally... My laptop is having a hard enough time as it is with just Thanks for catching the clippy errors as well. |
9acdac1 to
a27b60d
Compare
|
One last doc add I just thought of. In some clusters, the apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: storageclass-reader
rules:
- apiGroups: ["storage.k8s.io"]
resources: ["storageclasses"]
verbs: ["get", "watch", "list"]
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: node-storageclass-reader
subjects:
- kind: Group
name: system:nodes
apiGroup: rbac.authorization.k8s.io
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: storageclass-reader |
Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>
a27b60d to
e42772c
Compare
|
Good call. I just added it to the HOWTO doc as an addendum. Let me know if you'd like me to make further changes/tweaks there. |
|
Thanks @thomastaylor312! @itowlson are there any further changes you'd like me to address here? |
itowlson
left a comment
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
This is a simple change that allows the registrar to be built for MacOS. All of the `unix` package being used works for Mac as well. The reason for enabling this is completely selfish I'll admit. We are adding [CSI Support](krustlet/krustlet#462) to the Krustlet project, which supports Mac as well. As most CSI implementations don't support registering themselves with the Kubelet directly, we wanted to unblock any users to need to register their plugins with a Krustlet Kubelet. This change shouldn't affect normal container-based usage of the registrar
This is a simple change that allows the registrar to be built for MacOS. All of the `unix` package being used works for Mac as well. The reason for enabling this is completely selfish I'll admit. We are adding [CSI Support](krustlet/krustlet#462) to the Krustlet project, which supports Mac as well. As most CSI implementations don't support registering themselves with the Kubelet directly, we wanted to unblock any users to need to register their plugins with a Krustlet Kubelet. This change shouldn't affect normal container-based usage of the registrar
Adds support for mounting PersistentVolumeClaims to a Pod via the Container Storage Interface.
Closes #182