Bump cloud-provider-gcp to k8s v1.23 #305

Fedosin · 2021-12-21T15:46:25Z

Ran:
go mod tidy && ./tools/update_vendor.sh

Fixes #300

Fedosin · 2021-12-21T21:21:35Z

/test cloud-provider-gcp-e2e-full

jiahuif · 2021-12-21T23:12:18Z

May be another breaking change.
Suggested change:

diff --git a/cmd/gcp-controller-manager/service_account_verifier_test.go b/cmd/gcp-controller-manager/service_account_verifier_test.go
index cc19caf3..4db82ac5 100644
--- a/cmd/gcp-controller-manager/service_account_verifier_test.go
+++ b/cmd/gcp-controller-manager/service_account_verifier_test.go
@@ -20,6 +20,8 @@ import (
        "encoding/json"
        "fmt"
        "io/ioutil"
+       "net/http"
+       "net/http/httptest"
        "reflect"
        "sync"
        "testing"
@@ -32,8 +34,6 @@ import (
        "k8s.io/client-go/kubernetes/fake"
        ktesting "k8s.io/client-go/testing"
        "k8s.io/client-go/tools/cache"
-       "net/http"
-       "net/http/httptest"
 )
 
 func newKSA(sa serviceAccount, gsa gsaEmail) *core.ServiceAccount {
@@ -396,7 +396,6 @@ func TestConfigMapPersist(t *testing.T) {
 
        cmRes := schema.GroupVersionResource{Version: "v1", Resource: "configmaps"}
        wantCMKey := fmt.Sprintf("%s/%s", verifiedSAConfigMapNamespace, verifiedSAConfigMapName)
-
        tests := []struct {
                desc        string
                indexerObj  interface{}
@@ -469,7 +468,7 @@ func TestConfigMapPersist(t *testing.T) {
                        wantErr:    true,
                        wantActions: []ktesting.Action{
                                ktesting.NewUpdateAction(cmRes, verifiedSAConfigMapNamespace, newCMFromSAMap(t, &mapWithBothSA)),
-                               ktesting.NewDeleteAction(cmRes, verifiedSAConfigMapNamespace, wantCMKey),
+                               ktesting.NewDeleteActionWithOptions(cmRes, verifiedSAConfigMapNamespace, wantCMKey, *meta.NewDeleteOptions(0)),
                        },
                },
                {

Ref:

cloud-provider-gcp/cmd/gcp-controller-manager/service_account_verifier.go

Line 400 in 440cf63

    
           rmErr := sav.c.CoreV1().ConfigMaps(verifiedSAConfigMapNamespace).Delete(context.TODO(), key, *metav1.NewDeleteOptions(0))

cmd/cloud-controller-manager/nodeipamcontroller.go

Fedosin · 2021-12-22T16:12:03Z

@jiahuif thanks for the suggestion! I updated the code as you asked and now all test pass

jiahuif · 2021-12-22T17:30:43Z

/lgtm
/assign @cici37
How do you think?

go.mod

cici37 · 2021-12-28T17:58:25Z

Thanks for raising this PR! Have you tried to run Conformance test locally(with command kubetest2 gce -v 2 --repo-root $REPO_ROOT --build --up --down --test=ginkgo -- --test-package-version=v1.23.0 --focus-regex='\[Conformance\]')? We didn't include conformance test into presubmit so that is something we have to verify locally for this kind of changes. :)

go.mod

Fedosin · 2022-01-05T20:05:39Z

I got 2 errors with kubetest2 gce -v 2 --repo-root $REPO_ROOT --gcp-project openshift-gce-devel --build --up --down --test=ginkgo -- --test-package-version=v1.23.0 --focus-regex='\[Conformance\]' :

Summarizing 2 Failures:

[Fail] [sig-network] Services [It] should delete a collection of services [Conformance] 
/workspace/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/network/service.go:2808

[Fail] [sig-cli] Kubectl client Update Demo [It] should scale a replication controller  [Conformance] 
/workspace/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/framework/util.go:603

Ran 346 of 7042 Specs in 7822.170 seconds
FAIL! -- 344 Passed | 2 Failed | 0 Pending | 6696 Skipped
--- FAIL: TestE2E (7823.71s)
FAIL

Running this without my changes led to 3 failures:

Summarizing 3 Failures:

[Fail] [sig-network] Services [It] should delete a collection of services [Conformance] 
/workspace/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/network/service.go:2808

[Fail] [sig-node] KubeletManagedEtcHosts [AfterEach] should test kubelet managed /etc/hosts file [LinuxOnly] [NodeConformance] [Conformance] 
/workspace/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/framework/framework.go:481

[Fail] [sig-api-machinery] CustomResourcePublishOpenAPI [Privileged:ClusterAdmin] [It] works for CRD preserving unknown fields at the schema root [Conformance] 
/workspace/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/github.com/onsi/ginkgo/internal/leafnodes/runner.go:113

Ran 346 of 7042 Specs in 7886.095 seconds
FAIL! -- 343 Passed | 3 Failed | 0 Pending | 6696 Skipped

After a quick look at the failing tests it seems to me that they are not caused by my changes. Probably you may rerun it on your own to gather more statistics about these issues

cici37 · 2022-01-05T23:24:36Z

@Fedosin When you run it without your changes, did you run with --test-package-version=v1.22.0? The test package would be updated together with Kubernetes release. Testgrid shows the conformance test passes on current master.

jiahuif

Here is my result

Summarizing 1 Failure:                                                                                   
                                                                                                                                                                                                                   
[Fail] [sig-network] Services [It] should delete a collection of services [Conformance]                                                                                                                            
/workspace/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/network/service.go:2808
                                                                                                         
Ran 346 of 7042 Specs in 5709.385 seconds                                                                
FAIL! -- 345 Passed | 1 Failed | 0 Pending | 6696 Skipped
--- FAIL: TestE2E (5711.23s)
FAIL

jiahuif

Suggested change:

--- a/WORKSPACE
+++ b/WORKSPACE
@@ -100,11 +100,11 @@ load("//defs:repo_rules.bzl", "fetch_kube_release")
 fetch_kube_release(
     name = "io_k8s_release",
     archives = {
-        "kubernetes-server-linux-amd64.tar.gz": "af62692a5d5c6024a78d230061061b3fcd2f5422cea2f3d6aaad2fe20fb13746",
-        "kubernetes-manifests.tar.gz": "037d1a93edd2b9f54902adaca6060f9dd94911fc4c7122d06c26112ef5e72cce",
+        "kubernetes-server-linux-amd64.tar.gz": "9dc1be960e0f9fa41652aad90ea67c228e6b96b21f3d1833741d4362834df081",
+        "kubernetes-manifests.tar.gz": "a65cc4f74063b0d2d65b47b054bcc5d1bf3f00ae69563419530e374f414756b0",
         # we do not currently make modifications to these release tars below
-        "kubernetes-node-linux-amd64.tar.gz": "55e63ed2452693c1026307a14554e5b2fff549a97993400d178cd827c76ce128",
-        "kubernetes-node-windows-amd64.tar.gz": "17eea748c06137bc802d836a7db8e2d4fba379f3cea1d22b88fcfbbd7c8396ad",
+        "kubernetes-node-linux-amd64.tar.gz": "faa25a8a0b14742b2f48184ad04a3c32c357912a9cdf4a0988d97a70174d5eab",
+        "kubernetes-node-windows-amd64.tar.gz": "aec9dfbd64670bc1175577aa2538e8eefc770c85e6e8cc2ea6719c524a02adff",
     },
-    version = "v1.22.0",
+    version = "v1.23.1",
 )

jiahuif · 2022-01-06T00:24:57Z

Or use this PR
Fedosin#1

jiahuif · 2022-01-06T02:38:24Z

I can confirm that after applying the patch above, the conformance suite passes. Please resolve conflicts and it should be good enough.

                                                                                                                                             
Ran 346 of 7042 Specs in 6051.468 seconds                                                                                                    
SUCCESS! -- 346 Passed | 0 Failed | 0 Pending | 6696 Skipped                                                                                 
PASS                                                                  

Ginkgo ran 1 suite in 1h40m53.373588361s                                                                                                     
Test Suite Passed

Ran: go mod tidy && ./tools/update_vendor.sh && ./tools/update_bazel.sh

jiahuif · 2022-01-06T19:42:12Z

/lgtm
I can confirm on commit 8c2304c5717443889512692aff39b011d34c65dd the conformance suite passed.

kubetest2 gce -v 3 --gcp-project $PROJECT --repo-root `pwd` --build --up --down --test=ginkgo -- --test-package-version=v1.23.1 --focus-regex='\[Conformance\]'

End of ginkgo output.

{"msg":"Test Suite completed","total":346,"completed":346,"skipped":6696,"failed":0}                               │·······························································································
                                                                                                                   │·······························································································
Ran 346 of 7042 Specs in 6093.393 seconds                                                                          │·······························································································
SUCCESS! -- 346 Passed | 0 Failed | 0 Pending | 6696 Skipped                                                       │·······························································································
PASS                                                                                                               │·······························································································
                                                                                                                   │·······························································································
Ginkgo ran 1 suite in 1h41m35.321024187s                                                                           │·······························································································
Test Suite Passed

/assign @cheftako
for further approval.

cheftako · 2022-01-18T22:23:52Z

go.mod

 	sigs.k8s.io/controller-tools v0.7.0
 )

+require (
+	github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect


This seems suspect. Would like to understand why we're pulling this in.

$ go mod why github.com/Azure/go-ansiterm # github.com/Azure/go-ansiterm k8s.io/cloud-provider-gcp/cmd/cloud-controller-manager k8s.io/cloud-provider/app k8s.io/component-base/term github.com/moby/term github.com/moby/term/windows github.com/Azure/go-ansiterm

See also
https://github.com/kubernetes/kubernetes/blob/32f83b2b60a799020f8b512a923410270168ad94/go.mod#L149

This is an indirect dependency that we import via k8s.io/cloud-provider from Azure org, but it's not related to Azure cloud platform.
It's important to mention, that this dependency already exists in the current CCM version https://github.com/kubernetes/cloud-provider-gcp/blob/master/go.sum#L21-L34

The fact it appears in go.mod file now is because in golang 1.17 all indirect dependencies must be listed there.

In case this makes it easier to swallow, this dependency is already in the main branch of this project

cloud-provider-gcp/go.sum

Lines 22 to 25 in 285a94c

github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8=

github.com/Azure/go-ansiterm v0.0.0-20210608223527-2377c96fe795/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8=

github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 h1:UQHMgLO+TxOElx5B5HZ4hJQsoJ/PvUvKRhJHDQXO8P8=

github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E=

but the difference here is that the go.mod file has been bumped to Go 1.17, and in 1.17, when Go writes out the go.mod file, it lists first the required explicit dependencies, and then it creates a list of all of the implicit dependencies as well. That's why we are now seeing this as required where we weren't in Go 1.16

@bridgetkromhout going to look into why this is an "Azure" project rather than a more generic Microsoft contribution.

https://github.com/kubernetes/kubernetes/blob/v1.23.1/staging/src/k8s.io/cloud-provider/app/controllermanager.go#L127-L136

The only usage of k8s.io/component-base/term is to figure out how many columns the terminal has, which has the unfortunate side effect of introducing github.com/moby/term, github.com/moby/term/windows (for windows support), and finally github.com/Azure/go-ansiterm

cheftako · 2022-01-25T19:11:19Z

/lgtm
/approve

k8s-ci-robot · 2022-01-25T19:11:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cheftako, Fedosin

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~~ [cheftako]

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

jiahuif · 2022-01-25T19:57:01Z

/retest

After kubernetes#305 these dependencies should be bumped as well.

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Dec 21, 2021

k8s-ci-robot requested review from freehan and jpbetz December 21, 2021 15:49

Fedosin force-pushed the k8s_123 branch 5 times, most recently from ddea78b to 42ec38d Compare December 21, 2021 20:48

k8s-ci-robot assigned jiahuif Dec 21, 2021

jiahuif suggested changes Dec 21, 2021

View reviewed changes

cmd/cloud-controller-manager/nodeipamcontroller.go Outdated Show resolved Hide resolved

Fedosin force-pushed the k8s_123 branch 2 times, most recently from 5ca9610 to 4b1b0f9 Compare December 22, 2021 13:34

k8s-ci-robot assigned cici37 Dec 22, 2021

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

cici37 reviewed Dec 28, 2021

View reviewed changes

go.mod Outdated Show resolved Hide resolved

jiahuif suggested changes Dec 28, 2021

View reviewed changes

go.mod Outdated Show resolved Hide resolved

go.mod Outdated Show resolved Hide resolved

Fedosin force-pushed the k8s_123 branch from 4b1b0f9 to 05dd10f Compare December 30, 2021 21:06

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 30, 2021

jiahuif reviewed Jan 5, 2022

View reviewed changes

jiahuif suggested changes Jan 6, 2022

View reviewed changes

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

Fedosin added 3 commits January 6, 2022 11:55

Bump to golang 1.17

b57c295

Bump cloud-provider-gcp to k8s v1.23

b4c8c56

Ran: go mod tidy && ./tools/update_vendor.sh && ./tools/update_bazel.sh

Fix breaking changes introduced after upgrading to 1.23

b680576

Fedosin force-pushed the k8s_123 branch from 05dd10f to b680576 Compare January 6, 2022 11:20

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 6, 2022

uprade to Kubernetes 1.23.1 in WORKSPACE

8c2304c

k8s-ci-robot assigned cheftako Jan 6, 2022

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

cheftako reviewed Jan 18, 2022

View reviewed changes

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

k8s-ci-robot merged commit c2a51fc into kubernetes:master Jan 25, 2022

Fedosin added a commit to Fedosin/cloud-provider-gcp that referenced this pull request Jan 28, 2022

Bump k8s libraries for "providers" module to 1.23

7a58f43

After kubernetes#305 these dependencies should be bumped as well.

Fedosin mentioned this pull request Jan 28, 2022

Bump k8s libraries for "providers" module to 1.23 #314

Closed

Fedosin added a commit to Fedosin/cloud-provider-gcp that referenced this pull request Jan 28, 2022

Bump k8s libraries for "providers" module to 1.23

de1b15d

After kubernetes#305 these dependencies should be bumped as well.

Fedosin added a commit to Fedosin/cloud-provider-gcp that referenced this pull request Jan 28, 2022

Bump k8s libraries for "providers" module to 1.23

846673d

After kubernetes#305 these dependencies should be bumped as well.

Fedosin added a commit to Fedosin/cloud-provider-gcp that referenced this pull request Jan 28, 2022

Bump k8s libraries for "providers" module to 1.23

d27c376

After kubernetes#305 these dependencies should be bumped as well.

Fedosin added a commit to Fedosin/cloud-provider-gcp that referenced this pull request Mar 29, 2022

Bump k8s libraries for "providers" module to 1.23

2621aba

After kubernetes#305 these dependencies should be bumped as well.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump cloud-provider-gcp to k8s v1.23 #305

Bump cloud-provider-gcp to k8s v1.23 #305

Fedosin commented Dec 21, 2021

Fedosin commented Dec 21, 2021

jiahuif commented Dec 21, 2021 •

edited

Fedosin commented Dec 22, 2021

jiahuif commented Dec 22, 2021

cici37 commented Dec 28, 2021

Fedosin commented Jan 5, 2022

cici37 commented Jan 5, 2022

jiahuif left a comment

jiahuif left a comment

jiahuif commented Jan 6, 2022 •

edited

jiahuif commented Jan 6, 2022

jiahuif commented Jan 6, 2022 •

edited

cheftako Jan 18, 2022

jiahuif Jan 19, 2022

jiahuif Jan 19, 2022

Fedosin Jan 19, 2022

JoelSpeed Jan 19, 2022

cheftako Jan 19, 2022

jiahuif Jan 19, 2022

cheftako commented Jan 25, 2022

k8s-ci-robot commented Jan 25, 2022

jiahuif commented Jan 25, 2022

	github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8=
	github.com/Azure/go-ansiterm v0.0.0-20210608223527-2377c96fe795/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8=
	github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 h1:UQHMgLO+TxOElx5B5HZ4hJQsoJ/PvUvKRhJHDQXO8P8=
	github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E=

Bump cloud-provider-gcp to k8s v1.23 #305

Bump cloud-provider-gcp to k8s v1.23 #305

Conversation

Fedosin commented Dec 21, 2021

Fedosin commented Dec 21, 2021

jiahuif commented Dec 21, 2021 • edited

Fedosin commented Dec 22, 2021

jiahuif commented Dec 22, 2021

cici37 commented Dec 28, 2021

Fedosin commented Jan 5, 2022

cici37 commented Jan 5, 2022

jiahuif left a comment

Choose a reason for hiding this comment

jiahuif left a comment

Choose a reason for hiding this comment

jiahuif commented Jan 6, 2022 • edited

jiahuif commented Jan 6, 2022

jiahuif commented Jan 6, 2022 • edited

cheftako Jan 18, 2022

Choose a reason for hiding this comment

jiahuif Jan 19, 2022

Choose a reason for hiding this comment

jiahuif Jan 19, 2022

Choose a reason for hiding this comment

Fedosin Jan 19, 2022

Choose a reason for hiding this comment

JoelSpeed Jan 19, 2022

Choose a reason for hiding this comment

cheftako Jan 19, 2022

Choose a reason for hiding this comment

jiahuif Jan 19, 2022

Choose a reason for hiding this comment

cheftako commented Jan 25, 2022

k8s-ci-robot commented Jan 25, 2022

jiahuif commented Jan 25, 2022

jiahuif commented Dec 21, 2021 •

edited

jiahuif commented Jan 6, 2022 •

edited

jiahuif commented Jan 6, 2022 •

edited