[manila-csi-plugin] Updated external-snapshotter to v2.1.1 and external-provisioner to v2.0.2

[gman0]

What this PR does / why we need it:
This PR bumps csi-manila's the external-snapshotter to v2.1.1 and external-provisioner to v2.0.2. This is a backwards incompatible change.

Which issue this PR fixes(if applicable):
fixes #1194 , #1232

Special notes for reviewers:

Release note:

manila-csi-plugin: Updated external-provisioner to v2.0.2 and external-snapshotter to v2.1.1

[theopenlab-ci]

Build succeeded.

• cloud-provider-openstack-unittest : SUCCESS in 3m 20s

[theopenlab-ci]

Build succeeded.

• cloud-provider-openstack-format : SUCCESS in 3m 18s

[theopenlab-ci]

Build failed.

• cloud-provider-openstack-acceptance-test-csi-manila : POST_FAILURE in 56m 12s

[brtkwr]

This definitely needs a chart major version bump since its not backward compatible.

[theopenlab-ci]

Build succeeded.

• cloud-provider-openstack-format : SUCCESS in 2m 13s

[theopenlab-ci]

Build succeeded.

• cloud-provider-openstack-unittest : SUCCESS in 3m 20s

[gman0]

Lint and Test Charts / Helm chart (pull_request) Failing

Installing chart 'openstack-manila-csi => (version: "1.2.0", path: "charts/manila-csi-plugin")'...
Creating namespace 'manila-csi-plugin-zgpx4pgefw'...
namespace/manila-csi-plugin-zgpx4pgefw created
Error: ClusterRole.rbac.authorization.k8s.io "manila-csi-plugin-zgpx4pgefw-openstack-manila-csi-nodeplugin-rules" is invalid: metadata.labels: Invalid value: "rbac.manila.csi.openstack.org/aggregate-to-manila-csi-plugin-zgpx4pgefw-openstack-manila-csi-nodeplugin": name part must be no more than 63 characters

[theopenlab-ci]

Build succeeded.

• cloud-provider-openstack-format : SUCCESS in 2m 51s

[theopenlab-ci]

Build succeeded.

• cloud-provider-openstack-unittest : SUCCESS in 3m 48s

[theopenlab-ci]

Build succeeded.

• cloud-provider-openstack-acceptance-test-csi-manila : SUCCESS in 40m 40s

[gman0]

PTAL @gouthampacha @jichenjc @tombarron @brtknr

[brtkwr]

LGTM

[brtkwr]

Do you intend to backport to release-1.19 branch?

[gman0]

@brtknr I believe only bug fixes are eligible for backporting.

@ramineni would this qualify as a bug fix?

There are at least two scenarios where this might be considered as a bug:

• cinder-csi and manila-csi cannot be deployed in a cluster at the same time. external-snapshotter v1 installs its own CRDs and those are not compatible with v2's.
• manila-csi depends on cephfs-csi for CephFS shares. Latest cephfs-csi release deploys snapshotter v2, which means we're in the same situation as in the bulletpoint above. This has, regrettably, gone unnoticed in our CI as we're only testing with NFS - perhaps this should be improved as well.

[gouthampacha]

/lgtm

[gouthampacha]

@ramineni would this qualify as a bug fix?

There are at least two scenarios where this might be considered as a bug:

• cinder-csi and manila-csi cannot be deployed in a cluster at the same time. external-snapshotter v1 installs its own CRDs and those are not compatible with v2's.
• manila-csi depends on cephfs-csi for CephFS shares. Latest cephfs-csi release deploys snapshotter v2, which means we're in the same situation as in the bulletpoint above. This has, regrettably, gone unnoticed in our CI as we're only testing with NFS - perhaps this should be improved as well.

I concur, realistically we'll see a lot of users that want to use cinder-csi and manila-csi simultaneously, and this is a valuable bug fix for the 1.19 branch.

[tombarron]

/lgtm
/approve

[ramineni]

@gman0 @gouthampacha Few clarifications

1. I believe snapshot feature had some issues when we updated k8s to 1.19 . Does release-1.19 have workable snapshot feature?
2. Is cephfs-csi is a seperate csi driver? And manila csi driver supports Cephfs shares now? I dont see it added in 1.19 release notes, I thought its not supported till 1.18.

And on the backport to release1.19.

1. Its only a version bump of sidecars , so I'm ok to backport because of 2 reasons
   * helm charts are only released on release-1.19 branch, so incorporating to 1.19 would trigger stable chart release which could be used.
   * And I agree with @gman0 , it would be messy to deploy both csi drivers due to backward incompatible changes.

I believe you understand that , even if we backport we dont do minor releases of release branch, only we push updated docker image of respective plugin , which in this case is not needed as no code changes. Only helm release should suffice

[gman0]

@ramineni

Is cephfs-csi is a seperate csi driver? And manila csi driver supports Cephfs shares now? I dont see it added in 1.19 release notes, I thought its not supported till 1.18.

Yes, it is a separate CSI driver https://github.com/ceph/ceph-csi . manila-csi has supported CephFS shares since the very beginning, see this PR #536. It also explains why/how we rely on 3rd party CSI drivers.

We still don't support CREATE_DELETE_SNAPSHOT for CephFS shares though.

I believe snapshot feature had some issues when we updated k8s to 1.19 . Does release-1.19 have workable snapshot feature?

$ kubectl version
Client Version: version.Info{Major:"1", Minor:"19", GitVersion:"v1.19.0", GitCommit:"e19964183377d0ec2052d1f1fa930c4d7575bd50", GitTreeState:"clean", BuildDate:"2020-09-14T09:55:54Z", GoVersion:"go1.15.2", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"19", GitVersion:"v1.19.0", GitCommit:"e19964183377d0ec2052d1f1fa930c4d7575bd50", GitTreeState:"clean", BuildDate:"2020-09-08T11:12:27Z", GoVersion:"go1.15", Compiler:"gc", Platform:"linux/amd64"}

$ kubectl describe pods | grep "Image:"
    Image:         quay.io/k8scsi/nfsplugin:canary
    Image:         quay.io/k8scsi/snapshot-controller:v2.0.1
    Image:         quay.io/k8scsi/csi-provisioner:v2.0.2
    Image:         quay.io/k8scsi/csi-snapshotter:v2.1.1
    Image:         k8scloudprovider/manila-csi-plugin:v1.19.0
    Image:         quay.io/k8scsi/csi-node-driver-registrar:v1.1.0
    Image:         k8scloudprovider/manila-csi-plugin:v1.19.0

...
$ kubectl get volumesnapshot
NAME                 READYTOUSE   SOURCEPVC           SOURCESNAPSHOTCONTENT   RESTORESIZE   SNAPSHOTCLASS    SNAPSHOTCONTENT                                    CREATIONTIME   AGE
new-nfs-share-snap   true         new-nfs-share-pvc                           1Gi           csi-manila-nfs   snapcontent-70e66d75-fd64-4e3b-a804-f218a9eb0f8e   3m25s          3m26s
...
$ kubectl describe pvc new-nfs-share-snap-restore
Name:          new-nfs-share-snap-restore
Namespace:     default
StorageClass:  csi-manila-nfs
Status:        Bound
Volume:        pvc-b34618cc-174c-4087-b8b5-bcfbcc55966a
Labels:        <none>
Annotations:   pv.kubernetes.io/bind-completed: yes
               pv.kubernetes.io/bound-by-controller: yes
               volume.beta.kubernetes.io/storage-provisioner: nfs.manila.csi.openstack.org
Finalizers:    [kubernetes.io/pvc-protection]
Capacity:      1Gi
Access Modes:  RWX
VolumeMode:    Filesystem
DataSource:
  APIGroup:  snapshot.storage.k8s.io
  Kind:      VolumeSnapshot
  Name:      new-nfs-share-snap
Mounted By:  new-nfs-share-snap-restore-pod
Events:
  Type    Reason                 Age                 From                                                                                                          Message
  ----    ------                 ----                ----                                                                                                          -------
  Normal  Provisioning           3m                  nfs.manila.csi.openstack.org_ss-openstack-manila-csi-controllerplugin-0_f062a348-dece-4a66-a52a-b37b76a06c21  External provisioner is provisioning volume for claim "default/new-nfs-share-snap-restore"
  Normal  ExternalProvisioning   2m57s (x3 over 3m)  persistentvolume-controller                                                                                   waiting for a volume to be created, either by external provisioner "nfs.manila.csi.openstack.org" or manually created by system administrator
  Normal  ProvisioningSucceeded  2m53s               nfs.manila.csi.openstack.org_ss-openstack-manila-csi-controllerplugin-0_f062a348-dece-4a66-a52a-b37b76a06c21  Successfully provisioned volume pvc-b34618cc-174c-4087-b8b5-bcfbcc55966a

I've applied the changes from this PR to v1.19.0 chart and it seems to work fine.

I believe you understand that , even if we backport we dont do minor releases of release branch, only we push updated docker image of respective plugin , which in this case is not needed as no code changes. Only helm release should suffice

Yes, chart release is what we want and is sufficient.

[ramineni]

@gman0 Thanks for clarifications.

I've applied the changes from this PR to v1.19.0 chart and it seems to work fine.

No, I meant without this PR , does snapshot feature work on v1.19.0

[brtkwr]

cinder-csi and manila-csi cannot be deployed in a cluster at the same time. external-snapshotter v1 installs its own CRDs and those are not compatible with v2's.
manila-csi depends on cephfs-csi for CephFS shares. Latest cephfs-csi release deploys snapshotter v2, which means we're in the same situation as in the bulletpoint above. This has, regrettably, gone unnoticed in our CI as we're only testing with NFS - perhaps this should be improved as well.

Based on the points above, not very well.

[gman0]

@ramineni If there are only v1alpha1 snapshot CRDs installed in the cluster then yes. But there are cases where this is no longer true.

[ramineni]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ramineni, tombarron

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 [ramineni]

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

[ramineni]

@gman0 you can cherry-pick change to 1.19

[gman0]

@ramineni Will do, thanks! Thanks guys for the feedback.