-
Notifications
You must be signed in to change notification settings - Fork 187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update vendor dependencies #221
Conversation
Hi @humblec. Thanks for your PR. I'm waiting for a kubernetes-csi member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
/release-note-none |
@humblec: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
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. |
Signed-off-by: Humble Chirammal <[email protected]>
/ok-to-test |
go.mod
Outdated
github.com/imdario/mergo v0.3.7 // indirect | ||
github.com/golang/mock v1.4.3 | ||
github.com/golang/protobuf v1.3.5 | ||
github.com/imdario/mergo v0.3.9 // indirect | ||
github.com/kubernetes-csi/csi-lib-utils v0.7.0 | ||
github.com/kubernetes-csi/csi-test v2.0.0+incompatible |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also update to csi-test v3.x?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done..thx.
Signed-off-by: Humble Chirammal <[email protected]>
Signed-off-by: Humble Chirammal <[email protected]>
Signed-off-by: Humble Chirammal <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, but before we merge I'd like to hear from @msau42 or @xing-yang how we want to handle the transition to client-go 1.18: we have some shared libraries (https://github.com/kubernetes-csi/csi-lib-utils, https://github.com/kubernetes-sigs/sig-storage-lib-external-provisioner) which still use the older client-go API. Shall we update all repos after the next release?
Yes, let's update all our repos to 1.18 client-go |
kubernetes-sigs/sig-storage-lib-external-provisioner#73 cover s-s-l-external provisioner. |
pkg/controller/util.go
Outdated
"k8s.io/apimachinery/pkg/types" | ||
"k8s.io/client-go/kubernetes" | ||
"k8s.io/klog" | ||
) | ||
|
||
var ctx context.Context = context.Background() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not use global variables. Use context.TODO()
in all places where you use this.
Or even better, find a good API server timeout and use that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, let me start with context.TODO() local variable. Will do a follow up PR for configurable timeout . thanks !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: Humble Chirammal <[email protected]>
/lgtm This may be worth adding a release note since the client-go update was signficant. Please add one and then unhold |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: humblec, 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 |
@humblec: the
In response to this:
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. |
The release note is for end users (admins), who do not care about internals of golang API calls. IMO something like "Updated client-go to v0.18" would be sufficient. |
@sure.. done.. ! |
/hold cancel |
Fixing failed travis by closing and reopening... |
I was wondering why this was stuck.. thanks @jsafrane 👍 |
travis is still stuck :( going to force merge. everything else is passing fine. |
6613c398 Merge pull request kubernetes-csi#223 from sunnylovestiramisu/update 0e7ae993 Update k8s image repo url 77e47cce Merge pull request kubernetes-csi#222 from xinydev/fix-dep-version 155854b0 Fix dep version mismatch 8f839056 Merge pull request kubernetes-csi#221 from sunnylovestiramisu/go-update 1d3f94dd Update go version to 1.20 to match k/k v1.27 e322ce5e Merge pull request kubernetes-csi#220 from andyzhangx/fix-golint-error b74a5120 test: fix golint error aa61bfd0 Merge pull request kubernetes-csi#218 from xing-yang/update_csi_driver 7563d196 Update CSI_PROW_DRIVER_VERSION to v1.11.0 a2171bef Merge pull request kubernetes-csi#216 from msau42/process cb987826 Merge pull request kubernetes-csi#217 from msau42/owners a11216e4 add new reviewers and remove inactive reviewers dd986754 Add step for checking builds b66c0824 Merge pull request kubernetes-csi#214 from pohly/junit-fixes b9b6763b filter-junit.go: fix loss of testcases when parsing Ginkgo v2 JUnit d4277839 filter-junit.go: preserve system error log 38e11468 prow.sh: publish individual JUnit files as separate artifacts git-subtree-dir: release-tools git-subtree-split: 6613c3980d1e418bebb7bc49d64c977cfff85671
Signed-off-by: Humble Chirammal [email protected]