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.

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

KEP-2876: Promote CRD Validation Expression Language to Beta #3266

Merged
merged 1 commit into from
Jun 1, 2022

Conversation

cici37
Copy link
Contributor

@cici37 cici37 commented Apr 5, 2022

  • One-line PR description: Promote CRD Validation Expression Language to Beta
  • Other comments:

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 5, 2022
@cici37 cici37 changed the title kep-2876: Promote CRD Validation Expression Language to Beta KEP-2876: Promote CRD Validation Expression Language to Beta Apr 5, 2022
@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Apr 5, 2022
@k8s-ci-robot k8s-ci-robot added kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Apr 5, 2022
@cici37 cici37 mentioned this pull request Apr 5, 2022
21 tasks
@cici37
Copy link
Contributor Author

cici37 commented Apr 5, 2022

cc @jpbetz

@jpbetz
Copy link
Contributor

jpbetz commented Apr 5, 2022

This looks right. We're on track for beta in 1.25 according to the plan recorded in the KEP.

@jpbetz
Copy link
Contributor

jpbetz commented Apr 21, 2022

/assign @deads2k
/cc @liggitt

@liggitt
Copy link
Member

liggitt commented Apr 25, 2022

are there any items in the list of remaining work that need to be reflected here as alpha → beta graduation criteria? thinking specifically of any of the optimization work need to avoid super-linear complexity growth (especially around deeper CRDs with repeatedly computing nested expressions)

@cici37
Copy link
Contributor Author

cici37 commented Apr 26, 2022

are there any items in the list of remaining work that need to be reflected here as alpha → beta graduation criteria? thinking specifically of any of the optimization work need to avoid super-linear complexity growth (especially around deeper CRDs with repeatedly computing nested expressions)

Thanks for pointing out, I have added this into beta graduation criteria^

@liggitt
Copy link
Member

liggitt commented May 3, 2022

one question on the e2e test of enablement/disablement, lgtm otherwise

@cici37
Copy link
Contributor Author

cici37 commented May 10, 2022

@liggitt Would you mind to take a look when have time? Wondering if we could get it merged ^^

@liggitt
Copy link
Member

liggitt commented May 10, 2022

sorry, didn't know this was waiting on me.
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 10, 2022
@cici37
Copy link
Contributor Author

cici37 commented May 11, 2022

/assign @deads2k
for final approval.
Would you mind to take a look when you have time? Thank you 😊

@kikisdeliveryservice
Copy link
Member

kikisdeliveryservice commented May 13, 2022

As a small reminder, we have updated our KEP template Test Plan section this enhancement is using the previous version. Please be sure to update relevant sections for acceptance into the release. :)

@k8s-ci-robot k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 24, 2022
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 24, 2022
@deads2k
Copy link
Contributor

deads2k commented May 25, 2022

It looks like the ### Troubleshooting section is missing some answers. In particular, the ###### What steps should be taken if SLOs are not being met to determine the problem? section probably ought to be clear on how to determine which particular expression is costly or failing. How to mitigate, and whether it means disabling the entire feature.

Otherwise, this looks fine to me.

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 26, 2022
@cici37
Copy link
Contributor Author

cici37 commented May 26, 2022

@deads2k I have updated Troubleshooting section. Would you mind to take a look when have time? Thank you


###### What steps should be taken if SLOs are not being met to determine the problem?

We have added resource constraints to the feature. That being said, the request will fail if any particular expression is costly or failing or the entire request exceed certain limit.
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a metric for this and is it per-resource or just in aggregate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@deads2k
Copy link
Contributor

deads2k commented Jun 1, 2022

Thanks, looks good.

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 1, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cici37, 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 1, 2022
@k8s-ci-robot k8s-ci-robot merged commit 29adeed into kubernetes:master Jun 1, 2022
@k8s-ci-robot k8s-ci-robot added this to the v1.25 milestone Jun 1, 2022
@cici37 cici37 deleted the celBeta branch June 1, 2022 21:22
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. 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/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants