Update to use csi proxy beta #607
Conversation
k8s-ci-robot
added
do-not-merge/release-note-label-needed
k8s-ci-robot
added
approved
jingxu97
force-pushed
the
sep/vendor
branch
2 times, most recently
from
a8b7683 to
441a01d
Compare
| @@ -43,6 +43,7 @@ spec: | |||
| # Don't change base image without changing pdImagePlaceholder in | |||
| # test/k8s-integration/main.go | |||
| image: gke.gcr.io/gcp-compute-persistent-disk-csi-driver-win | |||
| imagePullPolicy: Always | |||
There was a problem hiding this comment.
We usually only set to always for dev environments where we're rebuilding the same image, but in general, we leave it unset (default)
|
|
||
| diskv1alpha1 "github.com/kubernetes-csi/csi-proxy/client/api/disk/v1alpha1" | ||
| diskclientv1alpha1 "github.com/kubernetes-csi/csi-proxy/client/groups/disk/v1alpha1" | ||
| diskv1beta1 "github.com/kubernetes-csi/csi-proxy/client/api/disk/v1beta1" |
There was a problem hiding this comment.
if we alias it to some common name, like "diskapi", then there's potentially less lines that need changing if we have to change api versions again
| diskv1alpha1 "github.com/kubernetes-csi/csi-proxy/client/api/disk/v1alpha1" | ||
| diskclientv1alpha1 "github.com/kubernetes-csi/csi-proxy/client/groups/disk/v1alpha1" | ||
| diskv1beta1 "github.com/kubernetes-csi/csi-proxy/client/api/disk/v1beta1" | ||
| diskclientv1beta1 "github.com/kubernetes-csi/csi-proxy/client/groups/disk/v1beta1" | ||
|
|
||
| fsv1alpha1 "github.com/kubernetes-csi/csi-proxy/client/api/filesystem/v1alpha1" |
There was a problem hiding this comment.
These apis don't need to be updated to beta?
There was a problem hiding this comment.
so because filesystem and smb api groups do not have any changes since alpha, so we didn't move it to beta, considering they not as mature as others. We think the version of the software/code is independent of the version of the API groups.
I discuss with @ddebroy and we will plan on moving them to beta too as post-beta action.
There was a problem hiding this comment.
From an API perspective, alpha has no guarantee of backwards compatibility or upgrade support. I would be wary of using an alpha api in production.
There was a problem hiding this comment.
right, i agree. I think I should not say "they not as mature as others. " because there is no change. I support moving them to beta, the APIs are working fine with all the current drivers for windows. I will address it in csi-windows meetup.
| for diskNum, diskInfo := range diskIDsMap { | ||
| klog.V(4).Infof("found disknum %s, diskInfo %v", diskNum, diskInfo) | ||
| idValue, found := diskInfo.Identifiers[id] | ||
| if found && strings.Contains(idValue, deviceName) { |
There was a problem hiding this comment.
Can you add a comment with an example format of idValue and deviceName?
strings.Contains is prone to substring matching. Do we have that issue here?
There was a problem hiding this comment.
I added comment. From the tests, it seems working fine.
There was a problem hiding this comment.
What if the idValue is something like "pvc-123", but the device name is "pvc-12"?
There was a problem hiding this comment.
good point. I should use exact match
yes, my change to install beta csi proxy is in 1.20. |
This PR updates driver to use csi-proxy beta version.
|
/retest |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jingxu97, msau42 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:
Approvers can indicate their approval by writing |
|
k8s-ci-robot
added
release-note-none
This PR updates driver to use csi-proxy beta version.
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: