Update dependency go modules for k8s v1.26.0 #123

sunnylovestiramisu · 2022-12-19T18:30:37Z

Ran kubernetes-csi/csi-release-tools go-get-kubernetes.sh -p 1.26.0

sunnylovestiramisu · 2022-12-19T18:31:14Z

/release-note-none

sunnylovestiramisu · 2022-12-19T19:05:39Z

make test failed with TestConnectMetrics

The diffs are:

connection_test.go:404: Running testcase Regular connection test
connection_test.go:432: Expected client metrics not found -- 

-csi_sidecar_operations_seconds_sum{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo"} 0

+csi_sidecar_operations_seconds_sum{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo"} 0.000649487

=======================

-csi_plugin_operations_seconds_sum{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo"} 0

+csi_plugin_operations_seconds_sum{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo"} 1.075e-06

=======================

-csi_sidecar_operations_seconds_sum{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",migrated="true"} 0

+csi_sidecar_operations_seconds_sum{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",migrated="true"} 0.000252969

Seems like the type conversion in between changed from int to some doubles?

sunnylovestiramisu · 2022-12-19T19:28:10Z

Also noticed that the unit test is saying we should ignore the csi_sidecar_operations_seconds_sum metrics?

https://github.com/kubernetes-csi/csi-lib-utils/blob/master/connection/connection_test.go#L429

sunnylovestiramisu · 2022-12-19T19:47:00Z

/assign @Jiawei0227

Do you have any context on this?

sunnylovestiramisu · 2022-12-20T19:21:01Z

1.25.0 release is not broken, the breaking change started at 1.26.0: #124

sunnylovestiramisu · 2022-12-20T19:34:22Z

I am not sure what the magic is but i copied the dependencies from the 1.25.0 pr and then ran go mod tidy/vendor etc. and now this pr is passing as well.

sunnylovestiramisu · 2022-12-20T19:35:26Z

/assign @xing-yang

xing-yang · 2022-12-20T19:50:09Z

go.mod

-	k8s.io/api v0.22.0
-	k8s.io/client-go v0.22.0
+	github.com/stretchr/testify v1.8.0
+	golang.org/x/net v0.3.1-0.20221206200815-1e63c2f08a10


Can you change this to v0.4.0? There's a CVE in earlier version.

Sure let me do that

You are going to update this to v0.4.0, right?

This one has been constantly downgraded again. Let me update.

xing-yang · 2022-12-20T19:51:02Z

go.mod

 	k8s.io/component-base v0.22.0
-	k8s.io/klog/v2 v2.9.0
+	k8s.io/klog/v2 v2.80.1


k8s.io/component-base v0.22.0?

Can this be updated to 0.26.0?

Update to 0.26.0 break the unit tests as we see above.

I see. Can you add a comment about this in the PR description?

I think we should update the component-base and fix the tests. I'm not sure if there will be any side effects if the sidecars and the library use two different versions of component-base: https://github.com/kubernetes-csi/external-provisioner/blob/master/go.mod#L24

xing-yang · 2022-12-20T19:55:49Z

go.mod

@@ -1,23 +1,62 @@
 module github.com/kubernetes-csi/csi-lib-utils

-go 1.16
+go 1.19

 require (
 	github.com/container-storage-interface/spec v1.5.0


Can you update this to 1.7.0?

Why 1.17? k8s 1.26.0 is at 1.19

github.com/container-storage-interface/spec v1.5.0 -> v1.7.0 which is the latest CSI spec release. This is not K8s release. Also not go version.

this comment is about github.com/container-storage-interface/spec

Then I need to fix the previous 1.25.0 first

But the go version for github.com/container-storage-interface is 1.18? https://github.com/container-storage-interface/spec/blob/release-1.7/go.mod

This should be fine. It's not possible for all dependency libs to use the exact same go version.

I see this is still github.com/container-storage-interface/spec v1.5.0. Is there any issue updating to github.com/container-storage-interface/spec v1.7.0?

Oh sorry I somehow was thinking you want me to update the go version to 1.17.0, changing it now. And i am still trying to fix the component base.

xing-yang · 2022-12-21T01:24:01Z

go.mod

@@ -1,34 +1,33 @@
 module github.com/kubernetes-csi/csi-lib-utils

-go 1.18
+go 1.17


This should still be updated to go 1.19.

pohly · 2022-12-21T17:26:47Z

go.mod

@@ -1,34 +1,33 @@
 module github.com/kubernetes-csi/csi-lib-utils

-go 1.18
+go 1.19


Please keep it at 1.18. 1.19 would indicate that the code in this repo needs 1.19, which isn't the case.

sunnylovestiramisu · 2022-12-22T01:01:21Z

The error has changed from this line after upgraded to component-base 0.26.0

Diff:
--- metric output does not match expectation; want
+++ got:
@@ -16,3 +16,3 @@
 csi_plugin_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="+Inf"} 1
-csi_plugin_operations_seconds_sum{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo"} 0
+csi_plugin_operations_seconds_sum{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo"} 4.81e-07
 csi_plugin_operations_seconds_count{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo"} 1
connection_test.go:458:

This results in the want split does not have the expected string to compare on this line

The want string is:

Diff:
--- metric output does not match expectation; want
+++

We need to change the split rule to something without the ":":

wantArr := strings.Split(err.Error(), "want")

This also ends up in certain situations the want string contains one line extra, so we need to remove that line so we can scan later:

want = strings.ReplaceAll(want, "+++ got:", "")

msau42 · 2022-12-22T01:24:22Z

/lgtm
/approve

k8s-ci-robot · 2022-12-22T01:24:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: msau42, sunnylovestiramisu

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

k8s-ci-robot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 19, 2022

k8s-ci-robot requested review from Jiawei0227 and pohly December 19, 2022 18:30

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Dec 19, 2022

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Dec 19, 2022

sunnylovestiramisu force-pushed the module-update-master branch from 69992a9 to 45c7c68 Compare December 19, 2022 18:38

k8s-ci-robot assigned Jiawei0227 Dec 19, 2022

sunnylovestiramisu force-pushed the module-update-master branch from 45c7c68 to 348c028 Compare December 20, 2022 19:29

k8s-ci-robot assigned xing-yang Dec 20, 2022

xing-yang reviewed Dec 20, 2022

View reviewed changes

sunnylovestiramisu force-pushed the module-update-master branch 2 times, most recently from 2515761 to 4d068b7 Compare December 21, 2022 00:25

xing-yang reviewed Dec 21, 2022

View reviewed changes

sunnylovestiramisu force-pushed the module-update-master branch from 4d068b7 to 5f590ee Compare December 21, 2022 17:22

pohly reviewed Dec 21, 2022

View reviewed changes

Update dependency go modules for k8s v1.26.0

d827e9d

sunnylovestiramisu force-pushed the module-update-master branch from 5f590ee to d827e9d Compare December 21, 2022 17:50

Fix unit test TestConnectMetrics for k8s.io/component-base v0.26.0

19b8406

Update golang.org/x/net to v0.4.0 avoid CVE

2690c64

k8s-ci-robot assigned msau42 Dec 22, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 22, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 22, 2022

k8s-ci-robot merged commit 975107d into kubernetes-csi:master Dec 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update dependency go modules for k8s v1.26.0 #123

Update dependency go modules for k8s v1.26.0 #123

sunnylovestiramisu commented Dec 19, 2022

sunnylovestiramisu commented Dec 19, 2022

sunnylovestiramisu commented Dec 19, 2022 •

edited

sunnylovestiramisu commented Dec 19, 2022

sunnylovestiramisu commented Dec 19, 2022

sunnylovestiramisu commented Dec 20, 2022 •

edited

sunnylovestiramisu commented Dec 20, 2022

sunnylovestiramisu commented Dec 20, 2022

xing-yang Dec 20, 2022

sunnylovestiramisu Dec 20, 2022

xing-yang Dec 21, 2022

sunnylovestiramisu Dec 22, 2022

xing-yang Dec 20, 2022

sunnylovestiramisu Dec 20, 2022

xing-yang Dec 20, 2022

msau42 Dec 20, 2022

xing-yang Dec 20, 2022

sunnylovestiramisu Dec 20, 2022

xing-yang Dec 20, 2022 •

edited

msau42 Dec 20, 2022

sunnylovestiramisu Dec 21, 2022

sunnylovestiramisu Dec 21, 2022

xing-yang Dec 21, 2022

xing-yang Dec 21, 2022 •

edited

sunnylovestiramisu Dec 21, 2022

xing-yang Dec 21, 2022 •

edited

pohly Dec 21, 2022

sunnylovestiramisu commented Dec 22, 2022

msau42 commented Dec 22, 2022

k8s-ci-robot commented Dec 22, 2022

Update dependency go modules for k8s v1.26.0 #123

Update dependency go modules for k8s v1.26.0 #123

Conversation

sunnylovestiramisu commented Dec 19, 2022

sunnylovestiramisu commented Dec 19, 2022

sunnylovestiramisu commented Dec 19, 2022 • edited

sunnylovestiramisu commented Dec 19, 2022

sunnylovestiramisu commented Dec 19, 2022

sunnylovestiramisu commented Dec 20, 2022 • edited

sunnylovestiramisu commented Dec 20, 2022

sunnylovestiramisu commented Dec 20, 2022

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

xing-yang Dec 20, 2022 • edited

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

xing-yang Dec 21, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xing-yang Dec 21, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunnylovestiramisu commented Dec 22, 2022

msau42 commented Dec 22, 2022

k8s-ci-robot commented Dec 22, 2022

sunnylovestiramisu commented Dec 19, 2022 •

edited

sunnylovestiramisu commented Dec 20, 2022 •

edited

xing-yang Dec 20, 2022 •

edited

xing-yang Dec 21, 2022 •

edited

xing-yang Dec 21, 2022 •

edited