ephemeral volumes in Kubernetes 1.16#209
Conversation
|
@pohly: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
This assumes that csi-driver-host-path v1.2.0 gets released including kubernetes-csi/csi-driver-host-path#97. The updated |
|
/release-notes-none |
|
/release-note-none |
| spec: | ||
| attachRequired: true | ||
| podInfoOnMount: true | ||
| volumeLifecycleModes: |
There was a problem hiding this comment.
Add a comment here that it's added in 1.16
| * `"csi.storage.k8s.io/pod.uid": string(pod.UID)` | ||
| * For more information see [Pod Info on Mount](pod-info.md). | ||
| * `volumeLifecycleModes` | ||
| * This field was added in Kubernetes 1.16 and must not be set when using an older Kubernetes release. |
There was a problem hiding this comment.
I was thinking "must not be set in the .yaml file because it's invalid", but "cannot" is simpler. Changed.
| @@ -5,6 +5,7 @@ | |||
| Status | Min K8s Version | Max K8s Version | |||
| --|--|-- | |||
| Alpha | 1.15 | - | |||
0954e22 to
54c63ed
Compare
| spec: | ||
| attachRequired: true | ||
| podInfoOnMount: true | ||
| volumeLifecycleModes: |
| * `"csi.storage.k8s.io/pod.uid": string(pod.UID)` | ||
| * For more information see [Pod Info on Mount](pod-info.md). | ||
| * `volumeLifecycleModes` | ||
| * This field was added in Kubernetes 1.16 and must not be set when using an older Kubernetes release. |
There was a problem hiding this comment.
I was thinking "must not be set in the .yaml file because it's invalid", but "cannot" is simpler. Changed.
44bb0e3 to
65c3147
Compare
| its [`CSIDriverInfo`](csi-driver-object.md) object explicitly declares | ||
| that the driver supports that kind of usage in its | ||
| `volumeLifecycleModes` field. This is a safeguard against accidentally | ||
| using a driver the wrong way. |
There was a problem hiding this comment.
@msau42: this is basically the same comment as in csi-driver-object.md ("used incorrectly by Kubernetes").
Should I expand on that in ephemeral-local-volumes.md, in csi-driver-object.md, or link to https://github.com/kubernetes/enhancements/blob/master/keps/sig-storage/20190122-csi-inline-volumes.md#support-for-inline-csi-volumes?
I'm leaning towards keeping the current text and just turning it into a link to the KEP, but I'm also fine with everything else.
There was a problem hiding this comment.
I would just link to the KEP for now. I bring this up because there is an ask to remove this field if it's not really providing anything other than a nicer error message when someone tries to misuse a driver: kubernetes/kubernetes#82507
There was a problem hiding this comment.
Added the links and commented on that issue (which had escaped my attention).
If the reason for the field still isn't clear, we can update the KEP.
65c3147 to
7e8cf77
Compare
| * `volumeLifecycleModes` | ||
| * This field was added in Kubernetes 1.16 and cannot be set when using an older Kubernetes release. | ||
| * It informs Kubernetes about the volume modes that are supported by the driver. | ||
| This ensures that the driver [is not used incorrectly](https://github.com/kubernetes/enhancements/blob/master/keps/sig-storage/20190122-csi-inline-volumes.md#support-for-inline-csi-volumes) by Kubernetes. |
There was a problem hiding this comment.
"by Kubernetes" or "by users"?
There was a problem hiding this comment.
"by Kubernetes on behalf of a user". That's becoming complicated - perhaps the sentence should simply end with "is not used incorrectly".
There was a problem hiding this comment.
I think "by users" is fine
There was a problem hiding this comment.
"by Kubernetes" implies to me that we're protecting against Kubernetes bugs, which I don't think is the case here
There was a problem hiding this comment.
Changed to "by users".
The CSIDriverInfo object was extended and the feature changed from alpha to beta.
7e8cf77 to
3a0b17f
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: msau42, pohly The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
The CSIDriverInfo object was extended and the feature changed from
alpha to beta.