Take the lastest changes from upstream kuberneres gce provider code #422

cezarygerard · 2022-12-14T14:32:47Z

Take the lastest changes from upstream kuberneres gce provider code (staging/src/k8s.io/legacy-cloud-providers/gce)

upstream commit id 68f808e6db388b249dae8c6c9dfb078bc0947b49

cezarygerard · 2022-12-14T14:43:02Z

/hold

jprzychodzen · 2022-12-14T15:12:29Z

/assign @jprzychodzen

k8s-triage-robot · 2022-12-14T17:59:17Z

Unknown CLA label state. Rechecking for CLA labels.

Send feedback to sig-contributor-experience at kubernetes/community.

/check-cla
/easycla

…staging/src/k8s.io/legacy-cloud-providers/gce) commit id 68f808e6db388b249dae8c6c9dfb078bc0947b49

k8s-ci-robot · 2022-12-15T14:59:54Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cezarygerard
Once this PR has been reviewed and has the lgtm label, please ask for approval from jprzychodzen by writing /assign @jprzychodzen in a comment. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

providers/gce/OWNERS

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

aojea · 2022-12-15T15:07:03Z

providers/gce/gce_loadbalancer_external.go

@@ -51,6 +53,11 @@ const (
 // new load balancers and updating existing load balancers, recognizing when
 // each is needed.
 func (g *Cloud) ensureExternalLoadBalancer(clusterName string, clusterID string, apiService *v1.Service, existingFwdRule *compute.ForwardingRule, nodes []*v1.Node) (*v1.LoadBalancerStatus, error) {
+	// Skip service handling if it uses Regional Backend Services and handled by other controllers
+	if usesL4RBS(apiService, existingFwdRule) {


is this concerning?
in which versions is this controller running?
does it conflict with L4RBS implementation?

k8s-ci-robot · 2022-12-15T15:07:55Z

@cezarygerard: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
cloud-provider-gcp-tests	`7f63831`	link	true	`/test cloud-provider-gcp-tests`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

cezarygerard · 2022-12-15T15:08:16Z

/assign @sdmodi

k8s-ci-robot · 2022-12-15T15:08:18Z

@cezarygerard: GitHub didn't allow me to assign the following users: sdmodi.

Note that only kubernetes members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @sdmodi

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.

aojea · 2022-12-15T15:09:35Z

providers/gce/gce_loadbalancer_external.go

+
+	destinationRanges := []string{ipAddress}
+
+	if !reflect.DeepEqual(destinationRanges, fw.DestinationRanges) {


this comparison is a bit awkward since destination ranges only has an IP address, no?

if len(fw.DestinationRanges) == 1 && fw.DestinationRanges[0] == ipAddress

since this seems to be pulled from intree we can follow up later

cezarygerard · 2022-12-15T15:10:25Z

@jprzychodzen

I am not able to rerun the tests locally, and I will probably need to use the debugger.

Also I don't understand full why CCM has diverged form KCM in a way that KCM had some code added (by me and Slavik) and the CCM had some code added? What was the purpose of the new code in the CCM? It had never run on production.
@sdmodi maybe you know?

aojea · 2022-12-15T15:18:08Z

/assign @bowei

if the controllers drift it may cause problems on upgrades,

jprzychodzen · 2022-12-16T16:37:14Z

FYI @cheftako @andrewsykim

jprzychodzen · 2022-12-16T16:46:00Z

As per why specific changes were not picked up by /cloud-provider-gcp/providers - due to https://github.com/kubernetes/enhancements/tree/master/keps/sig-cloud-provider/2395-removing-in-tree-cloud-providers#phase-3---migrating-provider-code-to-provider-repos and freezeing of the legacy cloud providers, not feature work should happen in upstream code.

jprzychodzen · 2022-12-19T22:21:35Z

/test cloud-provider-gcp-verify-up-to-date

jprzychodzen · 2022-12-20T13:05:07Z

providers/gce/gce_loadbalancer_internal_test.go

@@ -492,7 +520,7 @@ func TestUpdateInternalLoadBalancerNodes(t *testing.T) {
 	assert.Contains(
 		t,
 		instances[0].Instance,
-		fmt.Sprintf("%s/zones/%s/instances/%s", vals.ProjectID, vals.ZoneName, node2Name),
+		fmt.Sprintf("projects/%s/zones/%s/instances/%s", vals.ProjectID, vals.ZoneName, node2Name),


Please don't change that. legacy-cloud-provider has too old version of the /api library, which has different behavior for listing instances.

This was fixed in #262 for cloud-provider-gcp repository.

k8s-ci-robot · 2023-02-09T18:36:22Z

PR needs rebase.

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.

jprzychodzen · 2023-02-24T14:21:50Z

This PR (with fixes) already happened.

See #466

jprzychodzen · 2023-02-24T14:21:57Z

/close

k8s-ci-robot · 2023-02-24T14:22:02Z

@jprzychodzen: Closed this PR.

In response to this:

/close

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.

k8s-ci-robot requested review from cici37 and jpbetz December 14, 2022 14:32

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

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 14, 2022

cezarygerard force-pushed the master branch from b9e5476 to 5d4d16c Compare December 14, 2022 15:10

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Dec 14, 2022

k8s-ci-robot assigned jprzychodzen Dec 14, 2022

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 14, 2022

Take the lastest changes from upstream kuberneres gce provider code (…

7f63831

…staging/src/k8s.io/legacy-cloud-providers/gce) commit id 68f808e6db388b249dae8c6c9dfb078bc0947b49

cezarygerard force-pushed the master branch from 5d4d16c to 7f63831 Compare December 15, 2022 14:59

aojea reviewed Dec 15, 2022

View reviewed changes

k8s-ci-robot assigned bowei Dec 15, 2022

jprzychodzen mentioned this pull request Dec 16, 2022

Automate cloud-controller-image-creation based on cloudbuild #424

Merged

jprzychodzen reviewed Dec 20, 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 Feb 9, 2023

k8s-ci-robot closed this Feb 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Take the lastest changes from upstream kuberneres gce provider code #422

Take the lastest changes from upstream kuberneres gce provider code #422

cezarygerard commented Dec 14, 2022

cezarygerard commented Dec 14, 2022

jprzychodzen commented Dec 14, 2022

k8s-triage-robot commented Dec 14, 2022

k8s-ci-robot commented Dec 15, 2022

aojea Dec 15, 2022

k8s-ci-robot commented Dec 15, 2022

cezarygerard commented Dec 15, 2022

k8s-ci-robot commented Dec 15, 2022

aojea Dec 15, 2022

cezarygerard commented Dec 15, 2022

aojea commented Dec 15, 2022

jprzychodzen commented Dec 16, 2022

jprzychodzen commented Dec 16, 2022

jprzychodzen commented Dec 19, 2022

jprzychodzen Dec 20, 2022

k8s-ci-robot commented Feb 9, 2023

jprzychodzen commented Feb 24, 2023

jprzychodzen commented Feb 24, 2023

k8s-ci-robot commented Feb 24, 2023


		destinationRanges := []string{ipAddress}

		if !reflect.DeepEqual(destinationRanges, fw.DestinationRanges) {

Take the lastest changes from upstream kuberneres gce provider code #422

Take the lastest changes from upstream kuberneres gce provider code #422

Conversation

cezarygerard commented Dec 14, 2022

cezarygerard commented Dec 14, 2022

jprzychodzen commented Dec 14, 2022

k8s-triage-robot commented Dec 14, 2022

k8s-ci-robot commented Dec 15, 2022

aojea Dec 15, 2022

Choose a reason for hiding this comment

k8s-ci-robot commented Dec 15, 2022

cezarygerard commented Dec 15, 2022

k8s-ci-robot commented Dec 15, 2022

aojea Dec 15, 2022

Choose a reason for hiding this comment

cezarygerard commented Dec 15, 2022

aojea commented Dec 15, 2022

jprzychodzen commented Dec 16, 2022

jprzychodzen commented Dec 16, 2022

jprzychodzen commented Dec 19, 2022

jprzychodzen Dec 20, 2022

Choose a reason for hiding this comment

k8s-ci-robot commented Feb 9, 2023

jprzychodzen commented Feb 24, 2023

jprzychodzen commented Feb 24, 2023

k8s-ci-robot commented Feb 24, 2023