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

CRI API development policies #49455

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SergeyKanzhelev
Copy link
Member

Description

Policies of CRI API development.

The consensus was recieved via google doc: https://docs.google.com/document/d/1y42XrUPrm-6DZby1RQjexYYoNn822IRR6igsOiy_62c/edit?tab=t.0#heading=h.nzjfadot7nt7 and presented on both - SIG Node and SIG Atchitecture.

I am following the example of https://kubernetes.io/docs/reference/using-api/deprecation-policy/ placing it under the "Reference" section of docs.

/sig node

@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language labels Jan 15, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign divya-mohan0209 for approval. For more information see the 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

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 15, 2025
Copy link

netlify bot commented Jan 15, 2025

Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit a8b66df
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/67883182395a760008c442e7
😎 Deploy Preview https://deploy-preview-49455--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.

@@ -6,6 +6,8 @@ no_list: true

This section contains the following reference topics about nodes:

* Policies on [Kubernetes feature development and Container runtimes](content/en/docs/reference/node/cri-api-development-policies.md)
Copy link
Member

Choose a reason for hiding this comment

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

The link will need correction.

Suggested change
* Policies on [Kubernetes feature development and Container runtimes](content/en/docs/reference/node/cri-api-development-policies.md)
* Policies on [Kubernetes feature development and Container runtimes](/docs/reference/node/cri-api-development-policies)

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.

/hold

Looking at this change, I think it's in the wrong Git repository and would publish to the wrong place. It's OK to publish this policy to https://k8s.dev/ and then have https://kubernetes.io/docs/ hyperlink to that new policy page.

OK to unhold if SIG Docs tech leadership confirms that this repo is the right home.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this document belong within https://k8s.dev/docs/?

If not, https://kubernetes.io/docs/reference/using-api/deprecation-policy/ should change (maybe with a new URL, and a redirect from its old home) to link to this new document.

## Same maturity level (for beta and GA)

Implementation of an API needed for a Kubernetes feature in a container runtime
MUST be at least the same maturity as in k8s at a moment of Kubernetes release.
Copy link
Contributor

Choose a reason for hiding this comment

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

use bold for emphasis; avoid using un-bolded ALL CAPS to achieve the same thing.

Also, avoid the abbreviation “k8s” (especially all lowercase) within reference pages, unless the API or whatever specifically uses the abbreviation.

Comment on lines +16 to +23
Features and CRI API are supposed to be portable and generic and not limited to
a specific container runtime. However at this moment we require every feature to
work on two container runtimes: [Containerd](https://containerd.io/) and
[CRI-O](https://cri-o.io/). These are two runtimes that are tested as part of a
kubernetes development and release process. When this document refers to two
container runtimes, it assumes both - Containerd and CRI-O. If any other
container runtimes begin working actively with the Kubernetes community, this
document will need to be updated.
Copy link
Contributor

Choose a reason for hiding this comment

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

This sounds like details aimed at contributors, not end users.

Also also, avoid using “we“

experience for people trying the new feature early, adopting it when it is
enabled by default, and relying on it as a GA functionality.

## Supported container runtimes
Copy link
Contributor

Choose a reason for hiding this comment

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

We cannot risk implying that runtimes not on this list are unsupported. I think we must change this text.

I think this is a list of runtimes that this project uses for its own release testing. It's not related to conformance, for example.

Comment on lines +7 to +8
The mechanics of a feature development that requires new CRI APIs is covered in
documentation on CRI API [feature-development](https://github.com/kubernetes/cri-api?tab=readme-ov-file#feature-development).
Copy link
Contributor

Choose a reason for hiding this comment

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

In end user docs we avoid hyperlinking to pages on GitHub, especially anything in-project.

---
content_type: "reference"
title: Kubernetes feature development and Container runtimes
weight: 10
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit) should be a larger number, ie lower priority

Comment on lines +53 to +55
Both Containerd and CRI-O MUST have a GA release with the implementations of an
API needed for a Kubernetes feature before this Kubernetes feature can be
promoted to GA.
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid making this sound like a commitment. Imagine if the containerd project closed and everyone used to a rival thing (Docker maybe?)

We would still release Kubernetes and wouldn't require eg a bump to major version 2 to bypass this. It's not a commitment.

@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 Jan 16, 2025
Copy link
Contributor

@divya-mohan0209 divya-mohan0209 left a comment

Choose a reason for hiding this comment

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

Adding this as an agenda item for our next SIG Docs meeting that's held every fortnight.

The next occurrence is Tue 21st Jan.

@sftim
Copy link
Contributor

sftim commented Jan 16, 2025

Just to be clear, I've no disagreement with the policy as drafted; my concern is about where we publish it

We should make an implication that "must work with CRI-O and containerd" is part of conformance if and only if we're sure that's the message we're stating publicly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. language/en Issues or PRs related to English language sig/node Categorizes an issue or PR as relevant to SIG Node. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Development

Successfully merging this pull request may close these issues.

5 participants