[release-2.6] 🐛: elbv2: wait for LB active state instead of resolving DNS name #5226

k8s-infra-cherrypick-robot · 2024-11-22T14:31:52Z

This is an automated cherry-pick of #5093

/assign nrb

Check for the LB "active" status instead of trying to resolve the DNS name to validate the LB is ready.

Using DNS name resolution as a way to check the load balancer is working can cause problems that are dependent on the host running CAPA. In some systems, the DNS resolution can fail with very large TTLs cached DNS responses, causing very long provisioning times. Instead of DNS resolution, let's use the AWS API to check for the load balancer "active" state. Waiting for resolvable DNS names should be left for the clients to do.

AndiDog

/approve

damdo · 2024-11-23T09:53:38Z

/test pull-cluster-api-provider-aws-e2e

k8s-ci-robot · 2024-11-23T09:53:40Z

@damdo: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test pull-cluster-api-provider-aws-build
/test pull-cluster-api-provider-aws-build-docker
/test pull-cluster-api-provider-aws-build-docker-release-2-6
/test pull-cluster-api-provider-aws-build-release-2-6
/test pull-cluster-api-provider-aws-test-release-2-6
/test pull-cluster-api-provider-aws-verify-release-2-6

The following commands are available to trigger optional jobs:

/test pull-cluster-api-provider-aws-apidiff-release-2-6
/test pull-cluster-api-provider-aws-e2e-blocking-release-2-6
/test pull-cluster-api-provider-aws-e2e-conformance-release-2-6
/test pull-cluster-api-provider-aws-e2e-conformance-with-ci-artifacts-release-2-6
/test pull-cluster-api-provider-aws-e2e-eks-gc-release-2-6
/test pull-cluster-api-provider-aws-e2e-eks-release-2-6
/test pull-cluster-api-provider-aws-e2e-eks-testing-release-2-6
/test pull-cluster-api-provider-aws-e2e-release-2-6

Use /test all to run the following jobs that were automatically triggered:

pull-cluster-api-provider-aws-apidiff-release-2-6
pull-cluster-api-provider-aws-build
pull-cluster-api-provider-aws-build-docker
pull-cluster-api-provider-aws-build-docker-release-2-6
pull-cluster-api-provider-aws-build-release-2-6
pull-cluster-api-provider-aws-test-release-2-6
pull-cluster-api-provider-aws-verify-release-2-6

In response to this:

/test pull-cluster-api-provider-aws-e2e

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-sigs/prow repository.

r4f4 · 2024-11-25T12:37:47Z

/test pull-cluster-api-provider-aws-e2e-release-2-6

Ankitasw · 2024-11-26T12:36:57Z

/test pull-cluster-api-provider-aws-e2e-release-2-6

r4f4 · 2024-12-05T14:18:44Z

STEP: Searching for AMI: name=capa-ami-ubuntu-18.04-v1.26.6* @ 11/26/24 12:50:42.09

@richardcase it seems the deprecated AMIs issue needs to be solved for other branches as well?

r4f4 · 2025-03-03T07:51:51Z

/test pull-cluster-api-provider-aws-e2e-release-2-6

patrickdillon · 2025-03-03T16:27:46Z

Looking at pull-cluster-api-provider-aws-e2e-release-2-6 failure:


  �[38;5;9m[FAILED] Expected
      <int>: 0
  not to be zero-valued�[0m
  �[38;5;9mIn �[1m[SynchronizedBeforeSuite]�[0m�[38;5;9m at: �[1m/home/prow/go/src/sigs.k8s.io/cluster-api-provider-aws/test/e2e/shared/aws.go:868�[0m �[38;5;243m@ 03/03/25 08:06:32.008�[0m

  �[38;5;9mFull Stack Trace�[0m
    sigs.k8s.io/cluster-api-provider-aws/v2/test/e2e/shared.conformanceImageID(0xc000966580)
    	/home/prow/go/src/sigs.k8s.io/cluster-api-provider-aws/test/e2e/shared/aws.go:868 +0x47

It looks like the failure may correspond to missing creds?

cluster-api-provider-aws/test/e2e/shared/aws.go

Lines 865 to 868 in 79faf4c

    
           return &iam.AccessKey{ 
        
           	AccessKeyId:     out.AccessKey.AccessKeyId, 
        
           	SecretAccessKey: out.AccessKey.SecretAccessKey, 
        
           }

nrb · 2025-03-03T16:32:02Z

@patrickdillon That is a flake that happens sometimes, we haven't nailed it down.

/test pull-cluster-api-provider-aws-e2e-release-2-6
/approve

/assign @damdo

k8s-ci-robot · 2025-03-03T16:32:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AndiDog, nrb

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~~ [AndiDog,nrb]

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

damdo · 2025-03-04T10:02:42Z

/test pull-cluster-api-provider-aws-e2e-release-2-6

damdo · 2025-03-04T11:30:55Z

The e2e tests are failing because the capa-ami-ubuntu-18.04-v1.26.6 AMI doesn't exist anymore as it has been cleaned up.

@richardcase what do we suggest here? Switch to 22.04 also for release-2.6 or republish old images for this?

We might also need a better story around automated cleanup, we don't really want to not be able to run tests on older releases anymore.

richardcase · 2025-03-04T17:14:43Z

I'd say we need to support tests on previous release branches for a short while. May be 3 release branches?

We could update the tests to use newer versions of k8s/ubuntu? But how much effort should we but into fixing e2e on an old branch?

damdo · 2025-03-04T17:29:48Z

@richardcase Yeah I agree with the current CAPA staffing we might not be able to keep up with tests on releases too far back. This is n-1 w.r.t. the current n (latest), which is 2.7, so I think it would be well within reasonable ranges that we can define.

richardcase · 2025-03-04T17:46:30Z

@richardcase Yeah I agree with the current CAPA staffing we might not be able to keep up with tests on releases too far back. This is n-1 w.r.t. the current n (latest), which is 2.7, so I think it would be well within reasonable ranges that we can define.

Shall we just remove the e2e tests from all branches except:

main
release-2.7

Then i think when 2.8 is released we can keep it as:

main
release-2.7
release-2.8

I agree with you, with the number of active contributors to CAPA we need to pick where we put our effort.

Perhaps we can jump on a call with @nrb and we can come to a consensus.

nrb · 2025-03-04T19:06:43Z

I'm +1 to dropping the 2.6 tests. While it would be nice to have an updated, LTS OS for older branches, I don't think it's a valuable use of time or effort to keep these branches testing right now.

patrickdillon · 2025-03-06T17:50:00Z

@nrb @damdo are you ok moving forward with this or do we need to do anything else?

damdo · 2025-03-07T14:34:16Z

Yeah I agree with the current CAPA staffing we might not be able to keep up with tests on releases too far back. This is n-1 w.r.t. the current n (latest), which is 2.7, so I think it would be well within reasonable ranges that we can define.

Shall we just remove the e2e tests from all branches except:
main
release-2.7
Then i think when 2.8 is released we can keep it as:
main
release-2.7
release-2.8
I agree with you, with the number of active contributors to CAPA we need to pick where we put our effort.
Perhaps we can jump on a call with @nrb and we can come to a consensus.

Yes not an ideal situation but we need to be pragmatic with it, we can't maintain CI for older releases without more staffing.
As such I am happy with @richardcase proposal and considering @nrb's happy too let's do this.
We can always revisit this decision in the future if significant parties/contributions arise in this area.

damdo · 2025-03-07T14:34:43Z

I guess as far as this PR now concerned we can LGTM and merge as is.

damdo · 2025-03-07T14:38:59Z

Considering the above conversation and having @nrb @richardcase and @fiunchinho's green light, I'll proceed with merging this.

/lgtm

damdo · 2025-03-07T14:46:36Z

/test pull-cluster-api-provider-aws-test-release-2-6

k8s-ci-robot · 2025-03-07T14:51:47Z

@k8s-infra-cherrypick-robot: 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
pull-cluster-api-provider-aws-e2e-release-2-6	`a38f759`	link	false	`/test pull-cluster-api-provider-aws-e2e-release-2-6`

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

r4f4 · 2025-03-07T18:33:41Z

/test pull-cluster-api-provider-aws-test-release-2-6

damdo · 2025-03-07T19:03:12Z

@r4f4 it might need rebasing

r4f4 · 2025-03-07T19:10:10Z

It merged! Thanks for the help.

k8s-infra-cherrypick-robot mentioned this pull request Nov 22, 2024

🐛: elbv2: wait for LB active state instead of resolving DNS name #5093

Merged

5 tasks

k8s-ci-robot assigned nrb Nov 22, 2024

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 22, 2024

k8s-ci-robot requested review from damdo and faiq November 22, 2024 14:31

k8s-ci-robot added needs-priority size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 22, 2024

AndiDog reviewed Nov 23, 2024

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 23, 2024

k8s-ci-robot assigned damdo Mar 3, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 7, 2025

r4f4 mentioned this pull request Mar 7, 2025

🌱 [release-2.6] Bump golangci-lint to v1.62.2 and fix all lint errors #5377

Merged

5 tasks

k8s-ci-robot merged commit 2361956 into kubernetes-sigs:release-2.6 Mar 7, 2025
19 of 20 checks passed

[release-2.6] 🐛: elbv2: wait for LB active state instead of resolving DNS name #5226

[release-2.6] 🐛: elbv2: wait for LB active state instead of resolving DNS name #5226

Uh oh!

Conversation

k8s-infra-cherrypick-robot commented Nov 22, 2024

Uh oh!

AndiDog left a comment

Choose a reason for hiding this comment

Uh oh!

damdo commented Nov 23, 2024

Uh oh!

k8s-ci-robot commented Nov 23, 2024

Uh oh!

r4f4 commented Nov 25, 2024

Uh oh!

Ankitasw commented Nov 26, 2024

Uh oh!

r4f4 commented Dec 5, 2024

Uh oh!

r4f4 commented Mar 3, 2025

Uh oh!

patrickdillon commented Mar 3, 2025

Uh oh!

nrb commented Mar 3, 2025

Uh oh!

k8s-ci-robot commented Mar 3, 2025

Uh oh!

damdo commented Mar 4, 2025

Uh oh!

damdo commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

richardcase commented Mar 4, 2025

Uh oh!

damdo commented Mar 4, 2025

Uh oh!

richardcase commented Mar 4, 2025

Uh oh!

nrb commented Mar 4, 2025

Uh oh!

patrickdillon commented Mar 6, 2025

Uh oh!

damdo commented Mar 7, 2025

Uh oh!

damdo commented Mar 7, 2025

Uh oh!

damdo commented Mar 7, 2025

Uh oh!

damdo commented Mar 7, 2025

Uh oh!

k8s-ci-robot commented Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

r4f4 commented Mar 7, 2025

Uh oh!

damdo commented Mar 7, 2025

Uh oh!

Uh oh!

r4f4 commented Mar 7, 2025

Uh oh!

Uh oh!

damdo commented Mar 4, 2025 •

edited

Loading

k8s-ci-robot commented Mar 7, 2025 •

edited

Loading