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

Add resource constraints and oldSelf context into validation rule #32425

Merged
merged 3 commits into from
Apr 12, 2022

Conversation

cici37
Copy link
Contributor

@cici37 cici37 commented Mar 22, 2022

This is the placeholder pr to add resource constraints and oldSelf context into validation rule section.

The related PR:
kubernetes/enhancements#3160;
kubernetes/enhancements#3132;
kubernetes/enhancements#3144;
kubernetes/enhancements#3232

@k8s-ci-robot k8s-ci-robot added this to the 1.24 milestone Mar 22, 2022
@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 Mar 22, 2022
@netlify
Copy link

netlify bot commented Mar 22, 2022

👷 Deploy Preview for kubernetes-io-vnext-staging processing.

Name Link
🔨 Latest commit 40157c8
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/6255e3b0161df8000927414d

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 22, 2022
@k8s-ci-robot k8s-ci-robot added the language/en Issues or PRs related to English language label Mar 22, 2022
@k8s-ci-robot k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Mar 22, 2022
@cici37 cici37 force-pushed the celDoc branch 2 times, most recently from 4dadd28 to f7f1b73 Compare March 29, 2022 21:45
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 29, 2022
@cici37 cici37 changed the title [WIP] Add resource constraints and oldSelf context into validation rule Add resource constraints and oldSelf context into validation rule Mar 29, 2022
@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 Mar 29, 2022
@TurMitaev

This comment was marked as off-topic.

@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 Apr 5, 2022
@cici37
Copy link
Contributor Author

cici37 commented Apr 6, 2022

@kubernetes/sig-docs-en-owners
For approval. Thanks

Copy link
Contributor

@sftim sftim 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 PR. I've made a few suggestions from a technical writing perspective; I hope they're helpful.

- CEL standard functions, defined in the[list of standard definitions](https://github.com/google/cel-spec/blob/v0.7.0/doc/langdef.md#list-of-standard-definitions)
- CEL standard [macros](https://github.com/google/cel-spec/blob/v0.7.0/doc/langdef.md#macros)
- CEL [extended string function library](https://pkg.go.dev/github.com/google/cel-go@v0.11.2/ext#Strings)
- Kubernetes [CEL extension library](https://pkg.go.dev/k8s.io/apiextensions-apiserver@v0.24.0-alpha.4/pkg/apiserver/schema/cel/library#pkg-functions)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is v0.24.0-alpha.4 the right version to hyperlink to?

(there's a wider question of how we find a way to link to the right version of a package and keep that current with each Kubernetes release; the best approach for now is that each SIG is responsible for docs updates to any links to packages that the SIG maintains).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We plan to update the link to the formal release version after 1.24 were cut. Thanks for the reminder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The version is updated to beta.0 now which is the latest release tag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for my previous confusion. I have updated the link which will be available with 1.24 release

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 6, 2022
@k8s-ci-robot k8s-ci-robot requested a review from jpbetz April 6, 2022 23:08
@cici37
Copy link
Contributor Author

cici37 commented Apr 6, 2022

Hi @sftim , Thank you so much for your review. Your comments are addressed. Please feel free to take a look when have time. Thank you

@cici37
Copy link
Contributor Author

cici37 commented Apr 11, 2022

Gentle reminder^ @sftim

@cici37
Copy link
Contributor Author

cici37 commented Apr 11, 2022

@kubernetes/sig-docs-en-owners Gentle reminder on this pr. Thank you^

@sftim
Copy link
Contributor

sftim commented Apr 11, 2022

Based on https://github.com/kubernetes/website/pull/32425/files#r844493647
/hold

Hyperlink is not yet quite right.

@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 Apr 11, 2022
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

A couple of nits; otherwise, this is ready for tech review.

@sftim
Copy link
Contributor

sftim commented Apr 11, 2022

Hi @jpbetz (or SIG API Machinery more widely). With these latest changes, is this proposed text technically correct?

@jpbetz
Copy link
Contributor

jpbetz commented Apr 11, 2022

Hi @jpbetz (or SIG API Machinery more widely). With these latest changes, is this proposed text technically correct?

I just did a full read through a few minutes ago and it all made sense to me technically.

@cici37
Copy link
Contributor Author

cici37 commented Apr 11, 2022

Based on https://github.com/kubernetes/website/pull/32425/files#r844493647 /hold

Hyperlink is not yet quite right.

Regarding with this one, I have updated the link to beta.0 which is the latest release tag. We will update again after formal 1.24.0 released. I have updated the link to 0.24.0 which will be available when we publish doc in 1.24 release. Thank you

@cici37
Copy link
Contributor Author

cici37 commented Apr 12, 2022

Friendly reminder since approaching the doc deadline :). Would it be possible to get approval on this pr when you have time? Thank you @sftim

@reylejano
Copy link
Member

Preview of updated section on the CRD page:
https://deploy-preview-32425--kubernetes-io-vnext-staging.netlify.app/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#available-validation-functions

Link in Kubernetes CEL extension library is for the link that will be available when v1.24.0 is released.
The changes in this PR will be merged into main and live on kubernetes.io when v1.24.0 is released when the dev-1.24 branch is merged into main

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: reylejano

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 Apr 12, 2022
@reylejano
Copy link
Member

/sig api-machinery

@k8s-ci-robot k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Apr 12, 2022
@reylejano
Copy link
Member

Re-applying lgtm from @jpbetz and confirmation after changes
/lgtm

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

LGTM label has been added.

Git tree hash: ee17371d9e3b4dfd5550ea4929c71964a81ca9b2

@cici37
Copy link
Contributor Author

cici37 commented Apr 12, 2022

/unhold
Since the above issue has been addressed. Thank you!

@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 Apr 12, 2022
@k8s-ci-robot k8s-ci-robot merged commit d200945 into kubernetes:dev-1.24 Apr 12, 2022
@cici37 cici37 deleted the celDoc branch April 12, 2022 22:25
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. language/en Issues or PRs related to English language 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. sig/docs Categorizes an issue or PR as relevant to SIG Docs. 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.

None yet

7 participants