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-3716: add beta milestone for AdmissionWebhookMatchConditions #4059

Merged
merged 1 commit into from
Jun 8, 2023
Merged

KEP-3716: add beta milestone for AdmissionWebhookMatchConditions #4059

merged 1 commit into from
Jun 8, 2023

Conversation

andrewsykim
Copy link
Member

  • One-line PR description: Add beta milestone for AdmissionWebhookMatchConditions
  • Other comments:

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 6, 2023
@k8s-ci-robot k8s-ci-robot added the kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory label Jun 6, 2023
@k8s-ci-robot k8s-ci-robot added 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. labels Jun 6, 2023
@andrewsykim andrewsykim changed the title [WIP] KEP-3716: add beta milestone for AdmissionWebhookMatchConditions KEP-3716: add beta milestone for AdmissionWebhookMatchConditions Jun 6, 2023
@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 Jun 6, 2023
@andrewsykim andrewsykim force-pushed the kep-3716 branch 5 times, most recently from 50fde1e to 57c7ff4 Compare June 6, 2023 18:19
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 6, 2023
andrewsykim
andrewsykim commented Jun 6, 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 @deads2k

For PRR

@a-hilaly a-hilaly mentioned this pull request Jun 7, 2023
Closed
@andrewsykim
Copy link
Member Author

/assign @jpbetz

@jpbetz
Copy link
Contributor

jpbetz commented Jun 7, 2023 •
edited
Loading

@deads2k LGTM for PRR for beta.

Since this feature is intended as an optimization, if the matchCondition is not run (e.g. on old clusters that don't support the field) the behavior is expected to be equivalent-- the webhook is called, adding latency, but the admission outcome is expected be the same.

a-hilaly
a-hilaly approved these changes Jun 7, 2023
View reviewed changes
Copy link
Member

@a-hilaly a-hilaly left a comment

Choose a reason for hiding this comment

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

LGTM

a-hilaly
a-hilaly reviewed Jun 7, 2023
View reviewed changes
keps/sig-api-machinery/3716-admission-webhook-match-conditions/kep.yaml
Copy link
Member

Choose a reason for hiding this comment

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

nit: not sure if the metric names need to be added to line 55 in this file

Copy link
Member Author

Choose a reason for hiding this comment

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

done, thanks!

@deads2k
Copy link
Contributor

deads2k commented Jun 7, 2023

@deads2k LGTM for PRR for beta.

Agree
/lgtm
/approve
/hold

holding because I agree with this one too

nit: not sure if the metric names need to be added to line 55 in this file

fix up and it's good to go.

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Jun 7, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: A-Hilaly, andrewsykim, deads2k

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 Jun 7, 2023
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 8, 2023
@jpbetz
Copy link
Contributor

jpbetz commented Jun 8, 2023

/lgtm
/hold cancel

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jun 8, 2023
@k8s-ci-robot k8s-ci-robot merged commit 76dcd4f into kubernetes:master Jun 8, 2023
@k8s-ci-robot k8s-ci-robot added this to the v1.28 milestone Jun 8, 2023
@a-hilaly a-hilaly mentioned this pull request Jul 17, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@a-hilaly a-hilaly a-hilaly approved these changes

@apelisse apelisse Awaiting requested review from apelisse

@fedebongio fedebongio Awaiting requested review from fedebongio

Assignees

@jpbetz jpbetz

@deads2k deads2k

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/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.28
Development

Successfully merging this pull request may close these issues.
5 participants
@andrewsykim @jpbetz @deads2k @k8s-ci-robot @a-hilaly