Add snapshot beta CRD deployment for 1.17 by xing-yang · Pull Request #98 · kubernetes-csi/csi-driver-host-path

xing-yang · 2019-09-18T02:08:48Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
This PR adds snapshot beta CRD deployment for 1.17.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:
Note: This PR is not ready for review.

Does this PR introduce a user-facing change?:

Adds snapshot beta CRD deployment for 1.17

xing-yang · 2019-10-23T16:58:23Z

+: ${CSI_ATTACHER_RBAC:=https://raw.githubusercontent.com/kubernetes-csi/external-attacher/$(rbac_version "${BASE_DIR}/hostpath/csi-hostpath-attacher.yaml" csi-attacher "${UPDATE_RBAC_RULES}")/deploy/kubernetes/rbac.yaml}
+# TODO: Change back to dynamic path after image is released officially
+CSI_SNAPSHOTTER_RBAC_YAML="https://raw.githubusercontent.com/kubernetes-csi/external-snapshotter/master/deploy/kubernetes/rbac.yaml"
+: ${CSI_SNAPSHOTTER_RBAC:=https://raw.githubusercontent.com/kubernetes-csi/external-snapshotter/master/deploy/kubernetes/rbac.yaml}


So before the snapshot beta PR is merged, the rbac.yaml here won't work. You can create a rbac.yaml temporarily so you can test it manually.

Add the following to the existing rbac.yaml:

apiGroups: ["snapshot.storage.k8s.io"]
resources: ["volumesnapshotcontents/status"]
verbs: ["update"]

We also need to link kubernetes-latest to the 1.17 directory:
https://github.com/kubernetes-csi/csi-driver-host-path/blob/master/deploy/kubernetes-latest

Updated this symlink

xing-yang · 2019-10-23T17:00:14Z

+kind: VolumeSnapshotClass
+metadata:
+  name: csi-hostpath-snapclass
+snapshotter: hostpath.csi.k8s.io


apiVersion: snapshot.storage.k8s.io/v1beta1
kind: VolumeSnapshotClass
metadata:
name: csi-hostpath-snapclass
driver: hostpath.csi.k8s.io
deletionPolicy: Delete

I wonder if we still need to pre-create snapshotclass with the new apis.

Removed this

xing-yang · 2019-10-23T17:05:04Z

storageclass.yaml:

apiVersion: storage.k8s.io/v1
kind: StorageClass
metadata:
  name: csi-hostpath-sc
provisioner: hostpath.csi.k8s.io
reclaimPolicy: Delete
volumeBindingMode: Immediate

snapshot.yaml

apiVersion: snapshot.storage.k8s.io/v1beta1
kind: VolumeSnapshot
metadata:
  name: new-snapshot-demo
spec:
  volumeSnapshotClassName: csi-hostpath-snapclass
  source:
    persistentVolumeClaimName: hpvc

xing-yang · 2019-10-25T21:28:29Z

We also need to link kubernetes-latest to the 1.17 directory:
https://github.com/kubernetes-csi/csi-driver-host-path/blob/master/deploy/kubernetes-latest

xing-yang · 2019-10-27T21:00:23Z

+#: ${CSI_ATTACHER_RBAC:=https://raw.githubusercontent.com/kubernetes-csi/external-attacher/$(rbac_version "${BASE_DIR}/hostpath/csi-hostpath-attacher.yaml" csi-attacher "${UPDATE_RBAC_RULES}")/deploy/kubernetes/rbac.yaml}
+# TODO: Change back to dynamic path after image is released officially
+#CSI_SNAPSHOTTER_RBAC_YAML="https://raw.githubusercontent.com/kubernetes-csi/external-snapshotter/master/deploy/kubernetes/rbac.yaml"
+#: ${CSI_SNAPSHOTTER_RBAC:=https://raw.githubusercontent.com/kubernetes-csi/external-snapshotter/master/deploy/kubernetes/rbac.yaml}


This one should work now. The snapshot beta API PR is merged and its rbac.yaml is updated.

Updated this

xing-yang · 2019-10-27T21:04:12Z

+      containers:
+        - name: csi-snapshotter
+          # TODO: change to official image when released
+          image: quay.io/k8scsi/csi-snapshotter:testsnapshotbeta


You can use the following image which is build from the master code:
image: quay.io/k8scsi/csi-snapshotter:canary

xing-yang · 2019-10-27T21:05:23Z

+      serviceAccountName: csi-provisioner
+      containers:
+        - name: csi-provisioner
+          image: quay.io/k8scsi/csi-provisioner:testsnapshotbeta #v1.4.0-rc1


Please use the following image which uses the merged snapshot beta APIs:
quay.io/k8scsi/csi-provisioner:testsplitcontroller

xing-yang · 2019-10-27T21:08:33Z

@@ -0,0 +1,74 @@
+---


Please update these 3 CRDs using the lates ones here. They are merged into master branch of external-snapshotter now:
https://github.com/kubernetes-csi/external-snapshotter/tree/master/config/crd

Updated with the latest CRDs

xing-yang · 2019-10-27T21:15:12Z

+The sidecars depend on 1.15 API changes for migration and resizing,
+and 1.14 API changes for CSIDriver and CSINode.
+However the hostpath driver doesn't use those features, so this
+deployment can work on older Kubernetes versions.


This Readme does not apply any more. Replace this readme with following:

The deployment for Kubernetes 1.17 installs VolumeSnapshot Beta CRDs and thus is imcompatible
with Kubernetes < 1.17 when the VolumeSnapshot CRDs were Alpha.

xing-yang · 2019-10-28T20:16:54Z

+#CSI_PROVISIONER_RBAC_YAML="https://raw.githubusercontent.com/kubernetes-csi/external-provisioner/$(rbac_version "${BASE_DIR}/hostpath/csi-hostpath-provisioner.yaml" csi-provisioner false)/deploy/kubernetes/rbac.yaml"
+#: ${CSI_PROVISIONER_RBAC:=https://raw.githubusercontent.com/kubernetes-csi/external-provisioner/$(rbac_version "${BASE_DIR}/hostpath/csi-hostpath-provisioner.yaml" csi-provisioner "${UPDATE_RBAC_RULES}")/deploy/kubernetes/rbac.yaml}
+#CSI_ATTACHER_RBAC_YAML="https://raw.githubusercontent.com/kubernetes-csi/external-attacher/$(rbac_version "${BASE_DIR}/hostpath/csi-hostpath-attacher.yaml" csi-attacher false)/deploy/kubernetes/rbac.yaml"
+#: ${CSI_ATTACHER_RBAC:=https://raw.githubusercontent.com/kubernetes-csi/external-attacher/$(rbac_version "${BASE_DIR}/hostpath/csi-hostpath-attacher.yaml" csi-attacher "${UPDATE_RBAC_RULES}")/deploy/kubernetes/rbac.yaml}


We didn't change these RBAC's. These should just work?

xing-yang · 2019-10-28T20:17:35Z

+CSI_PROVISIONER_RBAC_YAML="https://raw.githubusercontent.com/kubernetes-csi/external-provisioner/master/deploy/kubernetes/rbac.yaml"
+: ${CSI_PROVISIONER_RBAC:=https://raw.githubusercontent.com/kubernetes-csi/external-provisioner/master/deploy/kubernetes/rbac.yaml}
+CSI_ATTACHER_RBAC_YAML="https://raw.githubusercontent.com/kubernetes-csi/external-attacher/master/deploy/kubernetes/rbac.yaml"
+: ${CSI_ATTACHER_RBAC:=https://raw.githubusercontent.com/kubernetes-csi/external-attacher/master/deploy/kubernetes/rbac.yaml}


I don't think we need these temporary rbac files any more.

Removed these

xing-yang · 2019-10-28T20:26:17Z

+          args:
+            - -v=5
+            - --csi-address=/csi/csi.sock
+            - --connection-timeout=15s


This flag is deprecated.

Ok, removed

Use kind v0.6.0

…ure can be tested. Test cases that test accessing volumes from multiple nodes need to be skipped

ggriffiths · 2019-12-03T22:42:18Z

Addressed most recent round of feedback @xing-yang @msau42

/test pull-kubernetes-csi-csi-driver-host-path-1-17-on-kubernetes-master

msau42 · 2019-12-03T23:15:19Z

+
+  kubectl apply -f "https://raw.githubusercontent.com/kubernetes-csi/external-snapshotter/${CSI_SNAPSHOTTER_VERSION}/deploy/kubernetes/snapshot-controller/setup-snapshot-controller.yaml"
+  cnt=0
+  expected_running_pods=3


Can we pull the "replicas" value from the deployment file? (It's currently one)

Updated this.
/test pull-kubernetes-csi-csi-driver-host-path-1-17-on-kubernetes-master

ggriffiths · 2019-12-03T23:51:39Z

Was missing the snapshotclass file in the k8s 1.17 directory. Just re-added it.
/test pull-kubernetes-csi-csi-driver-host-path-1-17-on-kubernetes-master

msau42 · 2019-12-04T01:49:17Z

lgtm! thanks for all your work on this!
/approve

k8s-ci-robot · 2019-12-04T01:49:21Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: msau42, xing-yang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

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

ggriffiths · 2019-12-04T06:38:59Z

lgtm! thanks for all your work on this!
/approve

Thanks! I'll update my PR to the release tools repo with the latest prow.sh changes. Once that's merged, I'll update the PR with the final commits.

Signed-off-by: Grant Griffiths <grant@portworx.com>

ggriffiths · 2019-12-04T07:52:01Z

Here is the PR for these prow.sh changes. Updated it with my latest changes to release-tools/prow.sh
kubernetes-csi/csi-release-tools#49
@xing-yang @msau42

Enable topology testing with hostpath driver

…ersion_gt Improve snapshot-controller running check and version_gt to support multiple formats

Signed-off-by: Grant Griffiths <grant@portworx.com>

…_beta

ggriffiths · 2019-12-04T21:48:29Z

Tests seem to failing after I did
git subtree pull --prefix=release-tools https://github.com/kubernetes-csi/csi-release-tools.git master

if [ "$ARCH" = "amd64" ]; then \
	CGO_ENABLED=0 GOOS=windows go build -mod=vendor -a -ldflags '-X main.version=v1.3.0-rc1-40-ga068236c -extldflags "-static"' -o ./bin/hostpathplugin.exe ./cmd/hostpathplugin ; \
	CGO_ENABLED=0 GOOS=linux GOARCH=ppc64le go build -mod=vendor -a -ldflags '-X main.version=v1.3.0-rc1-40-ga068236c -extldflags "-static"' -o ./bin/hostpathplugin-ppc64le ./cmd/hostpathplugin
/bin/sh: 3: Syntax error: end of file unexpected (expecting "fi")
make: *** [release-tools/build.make:71: build-hostpathplugin] Error 2

msau42 · 2019-12-04T22:08:36Z

My bad, I think it's missing a "\" at the end

fix syntax for ppc64le build

…_beta

ggriffiths · 2019-12-04T22:36:35Z

Updated with the fix to build.make
/test pull-kubernetes-csi-csi-driver-host-path-1-17-on-kubernetes-master

ggriffiths · 2019-12-04T23:17:34Z

/test pull-kubernetes-csi-csi-driver-host-path-1-17-on-kubernetes-master

ggriffiths · 2019-12-05T01:42:44Z

Seems like a change is incompatible with pull-kubernetes-csi-csi-driver-host-path-1-17-on-kubernetes-master.

I just re-ran the following on a local k8s 1.16 cluster
install_snapshot_crds
install_snapshot_controller
deploy/kubernetes-1.17/deploy-hostpath.sh
and tested snapshots manually, and everything seems fine.

@msau42 do you know of any other prow.sh or test-infra changes that may cause this test to fail?
https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/kubernetes-csi_csi-driver-host-path/98/pull-kubernetes-csi-csi-driver-host-path-1-17-on-kubernetes-master/1202366525122547714

msau42 · 2019-12-05T02:02:41Z

Dec  4 23:41:34.186: INFO: At 2019-12-04 23:36:35 +0000 UTC - event for pod-subpath-test-hostpath-csi-k8s-io-dynamicpv-7ntl: {kubelet csi-prow-worker} FailedMount: MountVolume.MountDevice failed for volume "pvc-0200512e-849b-4df5-b90e-547ba8e30c64" : kubernetes.io/csi: attacher.MountDevice failed to create newCsiDriverClient: driver name hostpath.csi.k8s.io not found in the list of registered CSI drivers

Looks like the volume got provisioned on node csi-prow-worker2, but the Pod got scheduled on csi-prow-worker:

I1204 23:36:19.514673       1 event.go:258] Event(v1.ObjectReference{Kind:"PersistentVolumeClaim", Namespace:"provisioning-2145", Name:"hostpath.csi.k8s.iov56w5", UID:"0200512e-849b-4df5-b90e-547ba8e30c64", APIVersion:"v1", ResourceVersion:"2248", FieldPath:""}): type: 'Normal' reason: 'Provisioning' External provisioner is provisioning volume for claim "provisioning-2145/hostpath.csi.k8s.iov56w5"
I1204 23:36:19.522595       1 connection.go:183] GRPC response: {"volume":{"accessible_topology":[{"segments":{"topology.hostpath.csi/node":"csi-prow-worker2"}}],"capacity_bytes":1048576,"volume_id":"df74845e-16ee-11ea-8c4c-3edb8610010e"}}

msau42 · 2019-12-05T02:04:38Z

+          image: quay.io/k8scsi/csi-provisioner:v1.5.0-rc1
+          args:
+            - -v=5
+            - --csi-address=/csi/csi.sock


Ah you're missing the "--feature-gates=Topology=true" flag

Nice catch 👍 Will add now.

Signed-off-by: Grant Griffiths <grant@portworx.com>

ggriffiths · 2019-12-05T02:08:51Z

/test pull-kubernetes-csi-csi-driver-host-path-1-17-on-kubernetes-master

ggriffiths · 2019-12-05T02:38:49Z

Looks like that did the job! Thanks @msau42 !

All tests are passing now.
/lgtm

k8s-ci-robot requested review from jsafrane and vladimirvivien September 18, 2019 02:08

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Sep 18, 2019

ggriffiths force-pushed the snapshot_beta branch 2 times, most recently from 579ea03 to ee5fd27 Compare October 22, 2019 22:45

xing-yang commented Oct 23, 2019

View reviewed changes

ggriffiths force-pushed the snapshot_beta branch from ee5fd27 to 7296f9c Compare October 25, 2019 20:40

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 25, 2019

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 25, 2019

ggriffiths force-pushed the snapshot_beta branch from dd5bf59 to 350eb18 Compare October 25, 2019 22:16

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 25, 2019

xing-yang commented Oct 27, 2019

View reviewed changes

xing-yang commented Oct 28, 2019

View reviewed changes

xing-yang changed the title ~~WIP: Add snapshot beta CRD deployment for 1.17~~ Add snapshot beta CRD deployment for 1.17 Oct 29, 2019

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 29, 2019

k8s-ci-robot and others added 2 commits December 2, 2019 06:35

Merge pull request kubernetes-csi#50 from darkowlzz/kind-0.6.0

4ff2f5f

Use kind v0.6.0

Create a kind cluster with two worker nodes so that the topology feat…

9a7a685

…ure can be tested. Test cases that test accessing volumes from multiple nodes need to be skipped

msau42 reviewed Dec 3, 2019

View reviewed changes

Improve snapshot pod running checks and improve version_gt

4ad6949

Signed-off-by: Grant Griffiths <grant@portworx.com>

k8s-ci-robot and others added 4 commits December 4, 2019 06:38

Merge pull request kubernetes-csi#51 from msau42/enable-multinode

d7c69d2

Enable topology testing with hostpath driver

Merge pull request kubernetes-csi#49 from ggriffiths/prowsh_improve_v…

771ca6f

…ersion_gt Improve snapshot-controller running check and version_gt to support multiple formats

Split snapshotter changes and beta images update for k8s 1.17

7fedb9d

Signed-off-by: Grant Griffiths <grant@portworx.com>

Merge commit '771ca6f26271f3135cf1634bbd05b02bb09b4fff' into snapshot…

fc49ed4

…_beta

msau42 and others added 3 commits December 4, 2019 14:12

fix syntax for ppc64le build

a4e6299

Merge pull request kubernetes-csi#53 from msau42/fix-make

540599b

fix syntax for ppc64le build

Merge commit '540599ba3c92c37f3bd7c5fdd8585e110ccab8af' into snapshot…

8931eaf

…_beta

msau42 reviewed Dec 5, 2019

View reviewed changes

Add topology feature gate to provisioner for k8s 1.17

c10d111

Signed-off-by: Grant Griffiths <grant@portworx.com>

pohly mentioned this pull request Dec 9, 2019

system:serviceaccount:cert-manager:csi-snapshotter cannot create resource ... #127

Closed

ggriffiths mentioned this pull request Dec 11, 2019

REQUEST: New membership for ggriffiths kubernetes/org#1480

Closed

6 tasks

pohly mentioned this pull request Oct 12, 2020

master: update release-tools #208

Merged

Conversation

xing-yang commented Sep 18, 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!

Choose a reason for hiding this comment

Uh oh!

xing-yang commented Oct 23, 2019

Uh oh!

xing-yang commented Oct 25, 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!

Choose a reason for hiding this comment

Uh oh!

ggriffiths Oct 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

ggriffiths Oct 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

ggriffiths commented Dec 3, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggriffiths commented Dec 3, 2019

Uh oh!

msau42 commented Dec 4, 2019

Uh oh!

k8s-ci-robot commented Dec 4, 2019

Uh oh!

ggriffiths commented Dec 4, 2019

Uh oh!

ggriffiths commented Dec 4, 2019

Uh oh!

ggriffiths commented Dec 4, 2019

Uh oh!

msau42 commented Dec 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ggriffiths commented Dec 4, 2019

Uh oh!

ggriffiths commented Dec 4, 2019

Uh oh!

ggriffiths commented Dec 5, 2019

Uh oh!

msau42 commented Dec 5, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggriffiths commented Dec 5, 2019

Uh oh!

ggriffiths Oct 28, 2019 •

edited

Loading

ggriffiths Oct 28, 2019 •

edited

Loading

msau42 commented Dec 4, 2019 •

edited

Loading