Skip to content

🐛Use generate name for CP loadbalancer listeners #4990

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 3, 2024
Merged

🐛Use generate name for CP loadbalancer listeners #4990

merged 1 commit into from
Jun 3, 2024

Conversation

@alexander-demicev
Copy link
Contributor

@alexander-demicev alexander-demicev commented May 28, 2024 •
edited
Loading

What type of PR is this?
/kind bug

What this PR does / why we need it:

Currently, we use timestamps to generate names for target rules of additional CP load balancer listeners. This fails because the name is generated so quickly that the timestamp stays the same. In this PR I changed timestamps to GenerateName from the API server, this should generate a unique and valid name.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Checklist:

  • squashed commits
  • includes documentation
  • includes emojis
  • adds unit tests
  • adds or updates e2e tests

Release note:

Use generate name for CP loadbalancer listeners

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. 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. needs-priority size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. 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 May 28, 2024
@alexander-demicev alexander-demicev changed the title Use generate name for CP loadbalancer listeners 🐛Use generate name for CP loadbalancer listeners May 28, 2024
@alexander-demicev alexander-demicev force-pushed the fixadditionallisteners branch from 6779cf4 to 565effa Compare May 28, 2024 15:57
@nrb
Copy link
Contributor

nrb commented May 28, 2024

/test ?

@k8s-ci-robot
Copy link
Contributor

@nrb: 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-test
  • /test pull-cluster-api-provider-aws-verify

The following commands are available to trigger optional jobs:

  • /test pull-cluster-api-provider-aws-apidiff-main
  • /test pull-cluster-api-provider-aws-e2e
  • /test pull-cluster-api-provider-aws-e2e-blocking
  • /test pull-cluster-api-provider-aws-e2e-clusterclass
  • /test pull-cluster-api-provider-aws-e2e-conformance
  • /test pull-cluster-api-provider-aws-e2e-conformance-with-ci-artifacts
  • /test pull-cluster-api-provider-aws-e2e-eks
  • /test pull-cluster-api-provider-aws-e2e-eks-gc
  • /test pull-cluster-api-provider-aws-e2e-eks-testing

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

  • pull-cluster-api-provider-aws-apidiff-main
  • pull-cluster-api-provider-aws-build
  • pull-cluster-api-provider-aws-build-docker
  • pull-cluster-api-provider-aws-test
  • pull-cluster-api-provider-aws-verify

In response to this:

/test ?

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.

@nrb
Copy link
Contributor

nrb commented May 28, 2024

/test pull-cluster-api-provider-aws-e2e
/test pull-cluster-api-provider-aws-e2e-blocking
/test pull-cluster-api-provider-aws-e2e-clusterclass
/test pull-cluster-api-provider-aws-e2e-conformance
/test pull-cluster-api-provider-aws-e2e-eks

@alexander-demicev
Copy link
Contributor Author

@nrb E2E failures don't seem to be related to the PR

@nrb
Copy link
Contributor

nrb commented May 30, 2024

Yeah, looks like Boskos errors. Will re-run while waiting for a second review.

/retest
/approve
/hold
/assign @damdo

Feel free to unhold once you've reviewed @damdo

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 30, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

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 k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 30, 2024
@nrb
Copy link
Contributor

nrb commented May 31, 2024

/retest

damdo
damdo reviewed Jun 2, 2024
View reviewed changes
Copy link
Member

@damdo damdo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this Alex :)

@alexander-demicev alexander-demicev force-pushed the fixadditionallisteners branch from 565effa to 63db13a Compare June 3, 2024 08:33
damdo
damdo reviewed Jun 3, 2024
View reviewed changes
Copy link
Member

@damdo damdo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 3, 2024
@damdo
Copy link
Member

damdo commented Jun 3, 2024

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 3, 2024
@k8s-ci-robot k8s-ci-robot merged commit 1564094 into kubernetes-sigs:main Jun 3, 2024
@alexander-demicev alexander-demicev deleted the fixadditionallisteners branch June 6, 2024 13:46
@nrb
Copy link
Contributor

nrb commented Jun 6, 2024

/cherry-pick release-2.5

@k8s-infra-cherrypick-robot
Copy link

@nrb: new pull request created: #5006

In response to this:

/cherry-pick release-2.5

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.

@k8s-infra-cherrypick-robot k8s-infra-cherrypick-robot mentioned this pull request Jun 6, 2024
Merged
@damdo damdo added this to the v2.5.1 milestone Jun 14, 2024
@r4f4 r4f4 mentioned this pull request Jun 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@damdo damdo damdo left review comments

@AndiDog AndiDog Awaiting requested review from AndiDog

@luthermonson luthermonson Awaiting requested review from luthermonson

Assignees

@damdo damdo

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Milestone

v2.5.1

Development

Successfully merging this pull request may close these issues.

5 participants

@alexander-demicev @nrb @k8s-ci-robot @damdo @k8s-infra-cherrypick-robot