Skip to content

Implement KEP-4420: Retry Generate Name#122887

Merged
k8s-ci-robot merged 1 commit into
kubernetes:masterfrom
jpbetz:retry-generate-name-create
Feb 15, 2024
Merged

Implement KEP-4420: Retry Generate Name#122887
k8s-ci-robot merged 1 commit into
kubernetes:masterfrom
jpbetz:retry-generate-name-create

Conversation

@jpbetz

@jpbetz jpbetz commented Jan 20, 2024

Copy link
Copy Markdown
Contributor

What type of PR is this?

/kind feature

What this PR does / why we need it:

Implements kubernetes/enhancements#4420

Special notes for your reviewer:

Does this PR introduce a user-facing change?

When the RetryGenerateName feature gate is enabled on the kube-apiserver,
create requests using generateName are retried automatically by the apiserver when the generated name conflicts with an existing resource name, up to a max limit of 7 retries.
This feature is in alpha.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

/sig api-machinery

@k8s-ci-robot

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. kind/feature Categorizes issue or PR as related to a new feature. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jan 20, 2024
@k8s-ci-robot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jpbetz

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

The pull request process is described here

Details 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 Jan 20, 2024
@jpbetz jpbetz changed the title Add retry around create Implement KEP-4420: Retry Generate Name Jan 20, 2024
@sftim

sftim commented Jan 25, 2024

Copy link
Copy Markdown
Contributor

Changelog suggestion

-When the RetryGenerateName feature gate is enabled on the kube-apiserver,
- create requests using generateName are retried automatically by the apiserver when the generated name conflicts with an existing resource name, up to a max limit of 7 retries.
-This feature is in alpha.
+Added an alpha feature behind the `RetryGenerateName` feature gate. When the feature gate is enabled,
+and the API server is handling a **create** request that uses `.metadata.generateName`, then any
+name conflict between the generated name and an existing resource name triggers an automatic server-side
+retry with a different generated name (up to a maximum of 8 attempts).

@sftim

sftim commented Jan 25, 2024

Copy link
Copy Markdown
Contributor

Once this new behavior is targeted at a release and merged / ready to merge, please document the feature gate.

@jpbetz

jpbetz commented Jan 29, 2024

Copy link
Copy Markdown
Contributor Author

/assign @deads2k

@deads2k

deads2k commented Feb 14, 2024

Copy link
Copy Markdown
Contributor

suggested doc updates seem fine.

This fell out shockingly cleanly. Thinking through failure modes

  1. not using generic registry storage: fails as before, but clients will react no differently compared to today. Also, I don't see any in kube
  2. someone tracking a per-attempt item in context: I'm not aware of any that should break. Warnings and audit stand out as places where the context holds an instance and repeated admission (for instance) would produce duplicate messages. I think this is ok since uniqueness isn't guaranteed.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 14, 2024
@k8s-ci-robot

Copy link
Copy Markdown
Contributor

LGTM label has been added.

DetailsGit tree hash: b44c14f515077e2ee13c7f66777fd6f4f995664f

@jpbetz jpbetz marked this pull request as ready for review February 14, 2024 20:30
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 14, 2024
@k8s-ci-robot k8s-ci-robot requested a review from deads2k February 14, 2024 20:31
@jpbetz

jpbetz commented Feb 14, 2024

Copy link
Copy Markdown
Contributor Author

/priority important-longterm

@k8s-ci-robot k8s-ci-robot added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Feb 14, 2024
@jpbetz

jpbetz commented Feb 14, 2024

Copy link
Copy Markdown
Contributor Author

/retest

@k8s-triage-robot

Copy link
Copy Markdown

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

  • The PR does have any do-not-merge/* labels
  • The PR does not have the needs-ok-to-test label
  • The PR is mergeable (does not have a needs-rebase label)
  • The PR is approved (has cncf-cla: yes, lgtm, approved labels)
  • The PR is failing tests required for merge

You can:

/retest

@jpbetz

jpbetz commented Feb 15, 2024

Copy link
Copy Markdown
Contributor Author

/retest

@k8s-ci-robot k8s-ci-robot merged commit 58c77d7 into kubernetes:master Feb 15, 2024
@k8s-ci-robot k8s-ci-robot added this to the v1.30 milestone Feb 15, 2024
@alexzielenski

Copy link
Copy Markdown
Member

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/apiserver cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants