Skip to content

Conversation

@aaron-prindle
Copy link
Contributor

@aaron-prindle aaron-prindle commented Feb 12, 2025

PR related to docs changes necessary for KEP 5073

KEP Issue: kubernetes/enhancements#5073
KEP PR: kubernetes/enhancements#5074

KEP Implementation PRs:

(related: kubernetes/enhancements#5073 (comment))

@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 Feb 12, 2025
@k8s-ci-robot k8s-ci-robot added the language/en Issues or PRs related to English language label Feb 12, 2025
@k8s-ci-robot k8s-ci-robot requested a review from sftim February 12, 2025 22:18
@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Feb 12, 2025
@netlify
Copy link

netlify bot commented Feb 12, 2025

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

Name Link
🔨 Latest commit fc8270a
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/67e19ff822dcde0009a29568

@netlify
Copy link

netlify bot commented Feb 12, 2025

Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit fc8270a
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/67e19ff822dcde0009a2956d
😎 Deploy Preview https://deploy-preview-49732--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@aaron-prindle aaron-prindle force-pushed the kep-5073-add-feature-gate-pr branch from f5d6b34 to 5f3a129 Compare March 13, 2025 20:49
@k8s-ci-robot k8s-ci-robot added area/blog Issues or PRs related to the Kubernetes Blog subproject area/localization General issues or PRs related to localization language/ko Issues or PRs related to Korean language language/pl Issues or PRs related to Polish language language/zh Issues or PRs related to Chinese language sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 13, 2025
@aaron-prindle aaron-prindle force-pushed the kep-5073-add-feature-gate-pr branch 2 times, most recently from bf7a05d to 5b0e9a8 Compare March 17, 2025 18:28
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 17, 2025
@hacktivist123
Copy link
Contributor

Hello @aaron-prindle 👋 please take a look at Documenting for a release - PR Ready for Review to get your PR ready for review before Tuesday 25th March 2025 18:00 PDT. Thank you!

@aaron-prindle aaron-prindle marked this pull request as ready for review March 20, 2025 21:02
@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 20, 2025
@aaron-prindle
Copy link
Contributor Author

@hacktivist123 thanks for the information here. This PR is ready for review and I have changed the status to such. The docs mention:

When you complete your content, the documentation person assigned to your feature reviews it. To ensure technical accuracy, the content may also require a technical review from corresponding SIG(s). Use their suggestions to get the content to a release ready state.

I am not sure if our feature has an assigned documentation person, do you happen to know who that might be or if we assign that person? I'm going to tag @sftim for now to attempt to get into a review queue but happy to change it if there is a more appropriate person for review. Thanks!

/assign @sftim

@aaron-prindle aaron-prindle force-pushed the kep-5073-add-feature-gate-pr branch 4 times, most recently from bf4af71 to fb7aa40 Compare March 21, 2025 18:57
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 21, 2025
@aaron-prindle aaron-prindle force-pushed the kep-5073-add-feature-gate-pr branch from fb7aa40 to 64d9c00 Compare March 21, 2025 19:03
@aaron-prindle
Copy link
Contributor Author

aaron-prindle commented Mar 21, 2025

I think we should merge this PR (which adds descriptions for feature gates) after the docs have merged into the v1.33, so that the feature gate descriptions can link to those docs.

@aaron-prindle do you have plans to write documentation about declarative validation rules? I imagine some will be needed.

There was not plans to write this documentation for the v1.33 release, initially the idea was to only add the feature gate documentation in v1.33. Declarative Validation is a maintainer+developer facing feature primarily, not an end-user facing feature (it unlocks some end users features but not in the near term). In v1.34 the plan was to add a full set of kubernetes developer facing documentation that explains:

The plan was to do this in v1.34 when we have upstreamed more of the validators to k/k and have documented the plan for how we want developers using declarative validation for new k8s native APIs/fields as well as where we want to focus the k8s dev community on the migration (we are hoping to get the community to aid in the migration and document how they can be most effective)

@aaron-prindle aaron-prindle force-pushed the kep-5073-add-feature-gate-pr branch from 64d9c00 to e193bee Compare March 21, 2025 22:35
@aaron-prindle
Copy link
Contributor Author

/cc @jpbetz

@k8s-ci-robot k8s-ci-robot requested a review from jpbetz March 24, 2025 17:57
…ve Validation of K8s Native Types With validation-gen
@aaron-prindle aaron-prindle force-pushed the kep-5073-add-feature-gate-pr branch from e193bee to fc8270a Compare March 24, 2025 18:09
@jpbetz
Copy link
Contributor

jpbetz commented Mar 24, 2025

/approve
/lgtm
For technical content

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

LGTM label has been added.

Git tree hash: 3e8bf15161b422ea05926784183524a01d45a8cb

@aaron-prindle
Copy link
Contributor Author

aaron-prindle commented Mar 25, 2025

@sftim would it be possible to merge the feature gate PR as is for now w/o the additional documentation mentioned? Currently the plan from folks working on Declarative Validation was for additional documentation to land once we have added some additional Validators upstream and then we can document the usage of those additional validators in our initial documentation piece. We plan to add the documentation noted in the comments above in a future PR

@sftim
Copy link
Contributor

sftim commented Mar 26, 2025

In v1.34 the plan was to add a full set of kubernetes developer facing documentation that explains

We ask for documentation the first time a feature gate is added, unless the feature gate doesn't have an effect (in which case, usually the project should revert the feature gate in the code).

If someone is writing an extension API server, and they want to use this, what do they need to know? We should aim to document a minimum useful level of detail.

@jpbetz
Copy link
Contributor

jpbetz commented Mar 26, 2025

In v1.34 the plan was to add a full set of kubernetes developer facing documentation that explains

We ask for documentation the first time a feature gate is added, unless the feature gate doesn't have an effect (in which case, usually the project should revert the feature gate in the code).

If someone is writing an extension API server, and they want to use this, what do they need to know? We should aim to document a minimum useful level of detail.

This one is tricky. The feature gate switches the implementation of validation (for only 2 fields in ReplicationController in 1.33). When the feature gates are on there is an additional metric. The feature gate docs plus the metrics docs feel like they explain this reasonably well.

The apiserver, as a library, cannot reasonably use it yet because the validation tags implemented are only sufficient for the fields we've migrated so far.

I agree with the sentiment that we should document features as we go. I'm struggling a bit to find a somewhere we could write something meaningful. We're adding validation-gen next to defaulter-gen and others. Most of the documentation for the adjacent code exists in the readme in package docs and readme files. https://kubernetes.io/docs/tasks/extend-kubernetes/setup-extension-api-server/ does not feel like a good place to put a page.

I was expecting us to do the bulk of declarative validation in the code and in the contrib repository (for guides on how to migrate).

Should we introduce a new page for "apiserver development" or "extension apiserver internals"? Does that fit into the website?

@sftim
Copy link
Contributor

sftim commented Mar 26, 2025

I agree that this doesn't seem to need much documentation, but I think it needs some.

Should we introduce a new page for "apiserver development" or "extension apiserver internals"? Does that fit into the website?

Maybe just a stub. We already have https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/apiserver-aggregation/ and that page could link to the stub.
[Ideally: we eventually document more than a stub, but that's a separate improvement]

We don't need any documentation aimed at people contributing to Kubernetes itself (but if we did, it would live within https://k8s.dev/docs/ anyway and not on this site).

  • When this change is stable, what differences will an end-user cluster administrator see?
  • During the journey to stability, what do people need to know about declarative validation around cluster upgrades?
  • Who might need to turn off this feature gate (v1.33 and also our expectations for future versions) and why.

@aaron-prindle
Copy link
Contributor Author

Maybe just a stub. We already have https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/apiserver-aggregation/ and that page could link to the stub.
[Ideally: we eventually document more than a stub, but that's a separate improvement]

We don't need any documentation aimed at people contributing to Kubernetes itself (but if we did, it would live within https://k8s.dev/docs/ anyway and not on this site).

When this change is stable, what differences will an end-user cluster administrator see?
During the journey to stability, what do people need to know about declarative validation around cluster upgrades?
Who might need to turn off this feature gate (v1.33 and also our expectations for future versions) and why.

@sftim - I have submitted a PR to add documentation that addresses the questions in the quoted comment and links this documentation in the suggested page. Link to the PR is here: #50265

@aaron-prindle
Copy link
Contributor Author

aaron-prindle commented Mar 31, 2025

@sftim I have addressed all feedback from the initial round of reviews and the related PR for a separate Declarative Validation docs page spawned from comments here (#50265) is merged now. I believe this is ready for another round of reviews now, thanks.

@rayandas
Copy link
Member

rayandas commented Apr 4, 2025

Hello @aaron-prindle 👋! I'm reaching out from the Docs team. Just checking in as we approach Docs Freeze on 8th April, 2025 18:00 PDT.
This documentation appears to still be under review. To meet the Docs Freeze, this PR must have a technical review as well as lgtm and approve labels applied, without any unaddressed comments or concerns from SIG Docs.
The status of this enhancement is marked as At risk for docs freeze. Thank you!

@tengqm
Copy link
Contributor

tengqm commented Apr 5, 2025

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jpbetz, tengqm

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 5, 2025
@k8s-ci-robot k8s-ci-robot merged commit 49dd495 into kubernetes:dev-1.33 Apr 5, 2025
6 checks passed
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/blog Issues or PRs related to the Kubernetes Blog subproject 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/docs Categorizes an issue or PR as relevant to SIG Docs. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants