ephemeral volumes in Kubernetes 1.16 #209

pohly · 2019-09-13T10:41:47Z

The CSIDriverInfo object was extended and the feature changed from
alpha to beta.

k8s-ci-robot · 2019-09-13T10:41:48Z

@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.

Instructions 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.

pohly · 2019-09-13T10:43:07Z

This assumes that csi-driver-host-path v1.2.0 gets released including kubernetes-csi/csi-driver-host-path#97. The updated CSIDriverInfo was created with kubelet+Kubernetes 1.16 4cb51f0d2d8392 and a locally compiled host path driver with the kubernetes-1.16 deployment from that PR.

pohly · 2019-09-13T11:23:46Z

/release-notes-none

pohly · 2019-09-13T11:24:24Z

/release-note-none

msau42 · 2019-09-13T18:21:30Z

book/src/csi-driver-object.md

@@ -26,6 +26,9 @@ metadata:
 spec:
  attachRequired: true
  podInfoOnMount: true
+  volumeLifecycleModes:


Add a comment here that it's added in 1.16

msau42 · 2019-09-13T18:21:42Z

book/src/csi-driver-object.md

@@ -45,6 +48,12 @@ There are three important fields:
    * `"csi.storage.k8s.io/pod.namespace": pod.Namespace`
    * `"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.


must => cannot

I was thinking "must not be set in the .yaml file because it's invalid", but "cannot" is simpler. Changed.

book/src/csi-driver-object.md

msau42 · 2019-09-13T18:23:13Z

book/src/ephemeral-local-volumes.md

@@ -5,6 +5,7 @@
 Status | Min K8s Version | Max K8s Version 
 --|--|--
 Alpha | 1.15 | - 


Alpha max is 1.15

pohly

@msau42: force-pushed the update

pohly · 2019-09-16T12:48:02Z

book/src/csi-driver-object.md

@@ -26,6 +26,9 @@ metadata:
 spec:
  attachRequired: true
  podInfoOnMount: true
+  volumeLifecycleModes:


pohly · 2019-09-16T12:49:07Z

book/src/csi-driver-object.md

@@ -45,6 +48,12 @@ There are three important fields:
    * `"csi.storage.k8s.io/pod.namespace": pod.Namespace`
    * `"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.


I was thinking "must not be set in the .yaml file because it's invalid", but "cannot" is simpler. Changed.

book/src/csi-driver-object.md

pohly · 2019-09-16T17:26:02Z

book/src/ephemeral-local-volumes.md

+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.


@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.

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

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.

msau42 · 2019-09-17T16:39:00Z

book/src/csi-driver-object.md

+* `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.


"by Kubernetes" or "by users"?

"by Kubernetes on behalf of a user". That's becoming complicated - perhaps the sentence should simply end with "is not used incorrectly".

I think "by users" is fine

"by Kubernetes" implies to me that we're protecting against Kubernetes bugs, which I don't think is the case here

Changed to "by users".

The CSIDriverInfo object was extended and the feature changed from alpha to beta.

msau42 · 2019-09-17T20:41:24Z

/lgtm
/approve

k8s-ci-robot · 2019-09-17T20:41:27Z

[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

Needs approval from an approver in each of these files:

~~OWNERS~~ [msau42]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 13, 2019

k8s-ci-robot requested review from lpabon and msau42 September 13, 2019 10:41

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 13, 2019

pohly mentioned this pull request Sep 13, 2019

Kubernetes 1.16 + ephemeral inline volumes kubernetes-csi/csi-driver-host-path#97

Merged

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Sep 13, 2019

msau42 reviewed Sep 13, 2019

View reviewed changes

pohly force-pushed the inline-volumes-beta branch from 0954e22 to 54c63ed Compare September 16, 2019 12:51

pohly commented Sep 16, 2019

View reviewed changes

pohly force-pushed the inline-volumes-beta branch 2 times, most recently from 44bb0e3 to 65c3147 Compare September 16, 2019 12:53

pohly commented Sep 16, 2019

View reviewed changes

msau42 mentioned this pull request Sep 17, 2019

Update cloning to Beta #210

Merged

pohly force-pushed the inline-volumes-beta branch from 65c3147 to 7e8cf77 Compare September 17, 2019 09:54

msau42 reviewed Sep 17, 2019

View reviewed changes

ephemeral volumes in Kubernetes 1.16

3a0b17f

The CSIDriverInfo object was extended and the feature changed from alpha to beta.

pohly force-pushed the inline-volumes-beta branch from 7e8cf77 to 3a0b17f Compare September 17, 2019 20:10

k8s-ci-robot assigned msau42 Sep 17, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 17, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 17, 2019

k8s-ci-robot merged commit 7c56815 into kubernetes-csi:master Sep 17, 2019

ephemeral volumes in Kubernetes 1.16 #209

ephemeral volumes in Kubernetes 1.16 #209

Uh oh!

Conversation

pohly commented Sep 13, 2019

Uh oh!

k8s-ci-robot commented Sep 13, 2019

Uh oh!

pohly commented Sep 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pohly commented Sep 13, 2019

Uh oh!

pohly commented Sep 13, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pohly left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msau42 commented Sep 17, 2019

Uh oh!

k8s-ci-robot commented Sep 17, 2019

Uh oh!

Uh oh!

pohly commented Sep 13, 2019 •

edited

Loading