Skip to content
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

KEP-1965: add GA criterias APIServerIdentity #4067

Closed
wants to merge 2 commits into from
Closed

KEP-1965: add GA criterias APIServerIdentity #4067

wants to merge 2 commits into from

Conversation

andrewsykim
Copy link
Member

  • One-line PR description: add GA criterias for APIServerIdentity
  • Other comments:

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory labels Jun 8, 2023
@k8s-ci-robot k8s-ci-robot requested review from apelisse and deads2k June 8, 2023 02:56
@k8s-ci-robot k8s-ci-robot added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 8, 2023
andrewsykim
andrewsykim commented Jun 8, 2023
View reviewed changes
keps/sig-api-machinery/1965-kube-apiserver-identity/README.md Outdated
@@ -179,7 +179,7 @@ Alpha should provide basic functionality covered with tests described above.

#### Beta -> GA Graduation

==TODO==
- support for aggregated apiservers
Copy link
Member Author

Choose a reason for hiding this comment

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

@enj last I checked this was the biggest thing that was left for GA, anything else come to mind?

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 8, 2023
andrewsykim
andrewsykim commented Jun 8, 2023
View reviewed changes
Copy link
Member Author

@andrewsykim andrewsykim left a comment

Choose a reason for hiding this comment

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

/assign @enj @deads2k @jpbetz

@jpbetz
Copy link
Contributor

jpbetz commented Jun 8, 2023

xref #4015 which will write kube-apiserver peer IPs to the v1.Lease used for APIServerIdentity. This doesn't directly impact this KEP, or it's graduation, but I wanted to keep everyone aware.

cc @richabanker

enj
enj requested changes Jun 9, 2023
View reviewed changes
Copy link
Member

@enj enj left a comment

Choose a reason for hiding this comment

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

BTW This KEP has not be opted into this release.

keps/sig-api-machinery/1965-kube-apiserver-identity/kep.yaml

# The milestone at which this feature was, or is targeted to be, at each stage.
milestone:
alpha: "v1.20"
beta: "v1.26"
stable: "v1.28"
Copy link
Member

Choose a reason for hiding this comment

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

IMO this is not ready for stable - IIUC nothing actually uses this functionality, so we have no proof or experience that it actually works. GA graduation criteria should include at least one, ideally two plus usages of this feature (and those usages need to beta level).

The conversation around kubernetes/kubernetes#114314 (comment) happened so long ago that I have forgotten all the things that we need to fix.

Copy link
Member Author

Choose a reason for hiding this comment

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

IMO this is not ready for stable - IIUC nothing actually uses this functionality, so we have no proof or experience that it actually works. GA graduation criteria should include at least one, ideally two plus usages of this feature (and those usages need to beta level).

I think it's reasonable to say this feature shouldn't GA until StorageVersion API is Beta, which is currently the primary use-case for it. I'll add this to the GA criteria.

Copy link
Contributor

Choose a reason for hiding this comment

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

For posterity: UVIP is using this but is still in alpha.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: andrewsykim
Once this PR has been reviewed and has the lgtm label, please ask for approval from deads2k. 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:

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

sftim
sftim reviewed Jul 2, 2023
View reviewed changes
keps/sig-api-machinery/1965-kube-apiserver-identity/README.md
@@ -179,7 +178,7 @@ Alpha should provide basic functionality covered with tests described above.

#### Beta -> GA Graduation

==TODO==
- support for aggregated apiservers (with sufficient integration/e2e test coverage)
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll need to document how to implement this in your aggregated API server. The docs should cover what's needed even if your implementation isn't in Go. It doesn't need to be in any great detail but I'm convinced we should make it feasible for an author to at least learn what they must implement themselves in order to conform.

@jpbetz
Copy link
Contributor

jpbetz commented Oct 3, 2023

Are we planning anything for 1.29 for this feature? I see the comments about waiting for SVM, are we in a holding pattern for a bit on this one?

@richabanker
Copy link
Contributor

To confirm: Is the only pending requirement for this to graduate to GA that - StorageVersion API needs graduating to beta first?

From what I understand, in order for StorageVersion API to graduate to beta, we need to

  1. Add support for SV API in CRDs (I have an open PR for it )
  2. Add support for SV API in aggregated apiservers.
    1. From the KEP it seems like aggregated apiservers should already have the support for this, so maybe we need more tests to verify that.

I am willing to work on this (graduating both StorageVersion API and APIServer Identity), if I can get some guidance in what's required for both. @enj @andrewsykim

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 11, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle rotten
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 11, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Reopen this PR with /reopen
  • Mark this PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

@k8s-ci-robot
Copy link
Contributor

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Reopen this PR with /reopen
  • Mark this PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sftim sftim sftim left review comments

@jpbetz jpbetz jpbetz left review comments

@enj enj enj requested changes

@apelisse apelisse Awaiting requested review from apelisse

@deads2k deads2k Awaiting requested review from deads2k

Assignees

@jpbetz jpbetz

@enj enj

@deads2k deads2k

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@andrewsykim @jpbetz @k8s-ci-robot @richabanker @k8s-triage-robot @enj @sftim @deads2k