Release 1.2.0 #102

pohly · 2019-10-15T10:33:05Z

What type of PR is this?
/kind feature

What this PR does / why we need it:

This in preparation for the v1.2.0 release.

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

updated sidecars to latest stable releases

pohly · 2019-10-15T15:18:16Z

deploy/kubernetes-1.15/hostpath/csi-hostpath-provisioner.yaml

@@ -40,7 +40,7 @@ spec:
      serviceAccountName: csi-provisioner
      containers:
        - name: csi-provisioner
-          image: quay.io/k8scsi/csi-provisioner:v1.3.0
+          image: quay.io/k8scsi/csi-provisioner:v1.4.0


I am unsure which deployments should be using 1.4.0. It's marked as compatible with Kubernetes >= 1.13 (https://github.com/kubernetes-csi/external-provisioner/releases/tag/v1.4.0). Do we still need any of the older releases? What for?

It could technically use 1.4.0. The main argument I can see for keeping older versions of sidecars is so that they still get CI testing.

That depends on whether anyone really needs to use those older versions. If that is not the case, then we don't need to support and test anything besides 1.4.0.

See also my questions in kubernetes-csi/docs#212.

BTW, the inverse is also true: because we claim that external-provisioner is compatible with Kubernetes >= 1.13, we need to test that, in particular because it is a combination that hasn't been tested before.

IMHO the "kubernetes-1.14" deployment should cover the recommended set of sidecars for that Kubernetes release. I'm just not sure at the moment what the recommendation is 🤷‍♂️

If we want to support also older releases of the sidecars, then we need additional deployments and Prow jobs.

The recommended should be documented in the sidecar pages. Let's stick with that for now.

Agree that we have a testing gap with what we claim to support. We can brainstorm ideas on how to improve the coverage in a separate issue for now.

The recommended should be documented in the sidecar pages.

You mean the "recommended K8s version" column? That's not quite the same.

For example, external-provisioner 1.4.0 may have some features that depend on 1.16 (hypothetically speaking, I have no idea whether that is the case). According to https://kubernetes-csi.github.io/docs/kubernetes-compatibility.html that means it's "recommended K8s version" will list 1.16 because "recommended Kubernetes version specifies the lowest Kubernetes version needed where all features of a sidecar will function correctly". That column basically answers the question "I want external-provisioner 1.4.0, which Kubernetes version will work best".

The other question is "I have Kubernetes 1.15, which sidecar versions should I use for that".

For example, suppose 1.4.0 can fully replace 1.3.1 on Kubernetes 1.15. Does that make it the recommended release for 1.15? This is not documented. The closest we get to documenting that are the reference deployments in this repo.

I think we should make the answers to both questions the same for simplicity

As discussed in the CI standup meeting, the deployments should use the sidecar versions on those (and only) those Kubernetes releases which are "recommended", i.e. we don't attempt to replace older sidecar versions if there have been API changes even if that would technically work - we simply don't test it and thus don't support it. Instead, older sidecar releases get maintained for older Kubernetes releases.

https://kubernetes-csi.github.io/docs/livenessprobe.html doesn't list "recommended Kubernetes". I made the assumption that there have been no API changes and thus all Kubernetes releases should be using the latest (1.1.0).

Same with https://kubernetes-csi.github.io/docs/node-driver-registrar.html (latest v1.2.0).

https://kubernetes-csi.github.io/docs/external-snapshotter.html lists 1.14 for 1.2.2, so it gets used on 1.14, 1.15 and 1.16.

msau42 · 2019-10-15T22:02:54Z

CHANGELOG-2.0.md

+
+## Other Notable Changes
+
+- The deployment uses `hostpath.csi.k8s.io` as driver name ([#64](https://github.com/kubernetes-csi/csi-driver-host-path/pull/64), [@pohly](https://github.com/pohly)).


Is this technically a breaking change? existing PVs using the old driver name won't work anymore?

True. I can move that into a new "breaking changes" section.

msau42 · 2019-10-15T22:03:54Z

deploy/kubernetes-1.13/hostpath/csi-hostpath-plugin.yaml

@@ -37,7 +37,7 @@ spec:
      hostNetwork: true
      containers:
        - name: node-driver-registrar
-          image: quay.io/k8scsi/csi-node-driver-registrar:v1.1.0
+          image: quay.io/k8scsi/csi-node-driver-registrar:v1.2.0


Shall we remove the entire 1.13 deployment? 1.13 is out of K8s support policy technically and we don't run tests on 1.13 anymore

Fine with me.

do you plan to remove 1.13 specs in this pr?

msau42 · 2019-10-15T22:07:02Z

deploy/kubernetes-1.15/hostpath/csi-hostpath-provisioner.yaml

@@ -40,7 +40,7 @@ spec:
      serviceAccountName: csi-provisioner
      containers:
        - name: csi-provisioner
-          image: quay.io/k8scsi/csi-provisioner:v1.3.0
+          image: quay.io/k8scsi/csi-provisioner:v1.4.0


It could technically use 1.4.0. The main argument I can see for keeping older versions of sidecars is so that they still get CI testing.

msau42 · 2019-10-16T19:06:44Z

CHANGELOG-2.0.md

+
+## Breaking Changes
+
+- The deployment uses `hostpath.csi.k8s.io` as driver name ([#64](https://github.com/kubernetes-csi/csi-driver-host-path/pull/64), [@pohly](https://github.com/pohly)).


Can you add a note about what might break and what they can do to mitigate/resolve it?

msau42 · 2019-10-16T19:07:55Z

deploy/kubernetes-1.13/hostpath/csi-hostpath-plugin.yaml

@@ -37,7 +37,7 @@ spec:
      hostNetwork: true
      containers:
        - name: node-driver-registrar
-          image: quay.io/k8scsi/csi-node-driver-registrar:v1.1.0
+          image: quay.io/k8scsi/csi-node-driver-registrar:v1.2.0


do you plan to remove 1.13 specs in this pr?

Deployments for older Kubernetes releases get updated if the release notes of a sidecar declare the sidecar release as "recommended" for that release of Kubernetes. The driver itself still needs to be tagged once this change is merged.

That release is no longer supported.

pohly · 2019-10-16T19:35:50Z

do you plan to remove 1.13 specs in this pr?

Done, also mentioned in changelog.

msau42 · 2019-10-16T21:20:06Z

CHANGELOG-1.2.md

+## Breaking Changes
+
+- The deployment uses `hostpath.csi.k8s.io` as driver name ([#64](https://github.com/kubernetes-csi/csi-driver-host-path/pull/64), [@pohly](https://github.com/pohly)).
+  Make sure that there are no active ephemeral inline volumes using the old `csi-hostpath` name before updating because otherwise


Also existing PVs created using the old driver name won't work either.

Aren't they tied to the storage class, and if the storage class gets updated the driver is still found under the new name? That at least was my hope; I haven't tried any of this.

One provisioned, the PV object contains the name of the driver in it. So if you change the driver name, it will no longer be able to process existing PVs

msau42 · 2019-10-17T14:49:41Z

CHANGELOG-1.2.md

+
+- The deployment uses `hostpath.csi.k8s.io` as driver name ([#64](https://github.com/kubernetes-csi/csi-driver-host-path/pull/64), [@pohly](https://github.com/pohly)).
+  Make sure that there are no persistent or ephemeral volumes using the old `csi-hostpath` name before updating because otherwise
+  those volumes cannot be removed. Pods with ephemeral volumes will be stuck in "terminating" state.


And any new pods cannot start

"Any new pod" or "pods using the old name"? I suppose the later.

msau42 · 2019-10-18T14:33:06Z

CHANGELOG-1.2.md

+- The deployment uses `hostpath.csi.k8s.io` as driver name ([#64](https://github.com/kubernetes-csi/csi-driver-host-path/pull/64), [@pohly](https://github.com/pohly)).
+  Make sure that there are no persistent or ephemeral volumes using the old `csi-hostpath` name before updating because otherwise
+  those volumes cannot be removed. Pods with such ephemeral volumes will be stuck in "terminating" state. New pods using
+  the old name in an inline volume will not be able to start.


This is true for pvs too

Updated.

I hereby promise to never, ever change the name of a CSI driver again. 😬

msau42 · 2019-10-18T16:15:22Z

/lgtm

msau42 · 2019-10-18T16:15:34Z

/approve

k8s-ci-robot · 2019-10-18T16:15:37Z

[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

msau42 · 2019-10-18T23:55:57Z

deploy/kubernetes-1.15/hostpath/csi-hostpath-attacher.yaml

@@ -40,7 +40,7 @@ spec:
      serviceAccountName: csi-attacher
      containers:
        - name: csi-attacher
-          image: quay.io/k8scsi/csi-attacher:v1.2.0
+          image: quay.io/k8scsi/csi-attacher:v2.0.0


Hm, one reason though to update 1.14 deployment to use attacher 2.0.0 is that currently our canary 1.14 tests are failing because 2.0 had added new rbac rules. We would need to update the 1.14 deployment here to pick up the new rbacs.

currently our canary 1.14 tests are failing because 2.0 had added new rbac rules

But isn't that as it should be? attacher 2.0.0 is not meant to run on Kubernetes 1.14, so we shouldn't be testing that combination (anymore).

This is where it gets complicated. I think it's still valuable to test 1.14 canary for all the other sidecars, but not attacher. Or we make an exception in this case

pohly · 2019-10-21T15:40:04Z

Michelle Au <[email protected]> writes:

This is where it gets complicated. I think it's still valuable to test 1.14 canary for all the other sidecars, but not attacher. Or we make an exception in this case

When it comes to testing, we can always introduce an alternative deployment and use that for canary testing or tweak the test setup script. Which Prow job exactly are we talking about?

msau42 · 2019-10-21T16:00:50Z

https://k8s-testgrid.appspot.com/sig-storage-csi-ci#canary-on-1.14

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 15, 2019

k8s-ci-robot requested review from jsafrane and vladimirvivien October 15, 2019 10:33

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

pohly force-pushed the release-1.2.0 branch 2 times, most recently from 789e535 to 199dabd Compare October 15, 2019 15:16

pohly commented Oct 15, 2019

View reviewed changes

pohly force-pushed the release-1.2.0 branch from 199dabd to e28cb91 Compare October 15, 2019 15:20

pohly mentioned this pull request Oct 15, 2019

e2e storage: bump revision of sidecars and driver kubernetes/kubernetes#83957

Merged

msau42 reviewed Oct 15, 2019

View reviewed changes

pohly force-pushed the release-1.2.0 branch from e28cb91 to 10f12e8 Compare October 16, 2019 17:43

msau42 reviewed Oct 16, 2019

View reviewed changes

pohly added 2 commits October 16, 2019 21:32

deploy: bump version to latest releases

32a353e

Deployments for older Kubernetes releases get updated if the release notes of a sidecar declare the sidecar release as "recommended" for that release of Kubernetes. The driver itself still needs to be tagged once this change is merged.

deploy: remove deployment for Kubernetes 1.13

9c8a9db

That release is no longer supported.

pohly force-pushed the release-1.2.0 branch from 10f12e8 to 00f4de0 Compare October 16, 2019 19:35

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 16, 2019

msau42 reviewed Oct 16, 2019

View reviewed changes

pohly force-pushed the release-1.2.0 branch from 00f4de0 to 096ceb5 Compare October 17, 2019 13:42

msau42 reviewed Oct 17, 2019

View reviewed changes

pohly force-pushed the release-1.2.0 branch from 096ceb5 to bf47ccc Compare October 18, 2019 14:22

msau42 reviewed Oct 18, 2019

View reviewed changes

CHANGELOG-1.2.md: initial version for v1.2.0

7ff8fa7

pohly force-pushed the release-1.2.0 branch from bf47ccc to 7ff8fa7 Compare October 18, 2019 15:05

k8s-ci-robot assigned msau42 Oct 18, 2019

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

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

k8s-ci-robot merged commit 8359099 into kubernetes-csi:master Oct 18, 2019

msau42 reviewed Oct 18, 2019

View reviewed changes


		## Other Notable Changes

		- The deployment uses `hostpath.csi.k8s.io` as driver name ([#64](https://github.com/kubernetes-csi/csi-driver-host-path/pull/64), [@pohly](https://github.com/pohly)).


		## Breaking Changes

		- The deployment uses `hostpath.csi.k8s.io` as driver name ([#64](https://github.com/kubernetes-csi/csi-driver-host-path/pull/64), [@pohly](https://github.com/pohly)).

Release 1.2.0 #102

Release 1.2.0 #102

Conversation

pohly commented Oct 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pohly commented Oct 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msau42 commented Oct 18, 2019

msau42 commented Oct 18, 2019

k8s-ci-robot commented Oct 18, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pohly commented Oct 21, 2019 via email

msau42 commented Oct 21, 2019