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

Modify the deprecation policy to disallow removal of stable API versions #31389

Merged
merged 2 commits into from
Jan 25, 2022

Conversation

liggitt
Copy link
Member

@liggitt liggitt commented Jan 18, 2022

Context: sig-architecture mailing list discussion and 1/13 meeting

/assign @derekwaynecarr @dims @johnbelamaric
for sig-arch

/assign @thockin @smarterclayton
for API approvers

/sig architecture
/hold for discussion/reviews

@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 18, 2022
@k8s-ci-robot k8s-ci-robot added the sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. label Jan 18, 2022
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 18, 2022
@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Jan 18, 2022
* **GA: 12 months or 3 releases (whichever is longer)**
* **Beta: 9 months or 3 releases (whichever is longer)**
* **Alpha: 0 releases**
* **GA: may be marked as deprecated, but must not be removed**
Copy link
Contributor

Choose a reason for hiding this comment

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

The X Window System ended up really struggling with its legacy of APIs. The risk here is of enduring success, with Kubernetes version 5.42 still advertising APIs that no client actually wants to use, and haven't wanted to for many years.

With that long term view in mind, I'm opposed to a flat MUST NOT here.

Copy link
Member Author

@liggitt liggitt Jan 18, 2022

Choose a reason for hiding this comment

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

in the mailing list thread, you suggested referencing a Kubernetes major version (e.g. Kubernetes v2.x) as a point at which stable APIs could be dropped. My concerns with language like that are:

  • there are no current plans for a v2.x stream
  • I wouldn't want this to serve as a motivation for a Kubernetes v2 that drops stable APIs (I fear a python2/python3-style split of the ecosystem)

I'm curious what others think here

Copy link
Contributor

@smarterclayton smarterclayton Jan 18, 2022

Choose a reason for hiding this comment

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

i tend to think that any kubernetes 2.x is going to be a layer around, above, or alongside kube v1. I highly doubt that Kube as a project will break continuity of stable API in such a dramatic way. Linux syscall has been fairly stable for 10+ years, and I tend to view our core stable APIs in the same vein.

I'd probably lean more towards creating the space for people to build their own v2 abstractions AND offer side by side alternatives to v1 (endpoint to endpoint slice is such an example) because that preserves the momentum of the project. Jordan's python2/3 point I think is the relevant cautionary tale - if someone wants to build to kube v2 AND decides turns off v1, that's great. But that won't necessarily be our focus and I would argue against prioritizing that.

Copy link
Contributor

@sftim sftim Jan 18, 2022

Choose a reason for hiding this comment

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

Rather than fork Kubernetes, I'd like to see the 1.x line of minor releases end (still with patch releases), and a new v2.x series start. What I'm imagining is that the last v1.x release and the v2.x release would be really similar, but the shift to v2.x would allow for some (well-publicized) removals.

So it'd be a new major version that looked a lot like the previous release.

As an example of what I'd turn off in a putative Kubernetes v2

  • componentstatuses API endpoints (this was the one I had in mind with my reservation about the change)
    • Gone, farewell, not served, not in the OpenAPI.
    • kubectl etc deny all knowledge
    • An error if you try to access them anyway

Leaving things like this as part of Kubernetes., even if they are deprecated, means some amount of maintenance cost and documentation effort forever. That's not so bad if it's only ever ComponentStatus but can we be sure there'll never be another API we fully move away from?

As an example of what I, personally, wouldn't want to turn off in a putative Kubernetes v2

  • endpoints API endpoints
    • Widely used at the moment; I don't relish the deprecation / migration work
    • Handling beta API deprecations and removal is already hard enough!

I am neutral on whether or not we remove, say, HorizontalPodAutoscaler v1 now that a new v2 API is stable. I'm sure other people have a better perspective on that/

If removing ComponentStatus splits the ecosystem, I'd be shocked. The overall idea is to drop the APIs that people have already actually stopped using.

PS. We could, exceptionally, make a commitment to support the final v1.x minor release for a longer period, say 18 months.

Copy link
Member

Choose a reason for hiding this comment

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

i wonder if k8s, long term would be comparable to the kernel ABIs stability. the surface there is much smaller, lower level and with less user facing exposure due to wrapping around system libraries. while some of the k8s stable APIs are directly user facing and much higher level.

the python fragmentation story is something i don't see happening in a mostly corporate driven development model as in k8s. a company choosing to remain on an old version of k8s while there rest of the project moves forward would not be a good business decision.

i think etcd is an example where they released a new incompatible API layer v3 (incompatible with v2) and that version of the etcd v3.x.x only works with the new layer. then the project established guides for migrating users to v3. if k8s ends up making major changes in v2.x.x, that new version may or may not be desirably compatible with the v1...but according to the policy - it must be.

i think it's worth leaving a small "backdoor" in the language here that this rule would apply to the current state of k8s versioning...if k8s moves to v2.x or has an unforeseen significant redesign, k8s v1.x would continue supporting the old set of stable APIs, but v2.x might not.

Copy link
Contributor

@smarterclayton smarterclayton Jan 18, 2022

Choose a reason for hiding this comment

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

componentstatuses API endpoints (this was the one I had in mind with my reservation about the change)

Ultimately the point made in the email thread is that no matter what, people contributing to Kube have to spend time dealing with implications of removing this API. Creating a v2 version of Kube, debating what is part of v2, writing blog posts, calming panicked users, setting up build infrastructure, having the release team deal with 2 completely separate streams is a hugely expensive way of removing the API. The default option is always "do nothing" and that is cheaper than any other approach. The next cheapest is adding a selective disablement flag. Once you get past that, the cost goes up significantly.

I really think the best way to measure these sorts of deprecations is focusing on the benefit you get out of it in measurable units, and imagine spending those measurable units on something you personally care about even more (like the Kubelet being 5% more efficient or a new flag on stateful sets). I'd love componentstatus to disappear, but I don't have any data suggesting it's continued existence costs engineering teams or contributors anything.

if k8s moves to v2.x or has an unforeseen significant redesign, k8s v1.x would continue supporting the old set of stable APIs, but v2.x might not.

I think it's reasonable to describe this effort as applying to k8s v1.x (because we have adopted semantic versioning, and have implicitly chosen to honor that mindset via no breakages to stable APIs). If we were to consider a k8s v2, we would need to revisit these topics anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to clarify one point, I don't want to advocate for an immediate v2.x release; I do only want to leave room for it to happen at some future point, when an API that's no longer used is hurting the project enough to justify that. (I don't think ComponentStatus is that API, so maybe it was a misleading example).

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @sftim - if we wrap all of these rules with "within a major version of Kubernetes", we at least leave room to discuss a v2 and whether it would be worth the effort to EOL some APIs. I don't even mind saying "as of Q1 2022, there are no plans for a v2.x.x of Kubernetes"

Copy link
Member Author

Choose a reason for hiding this comment

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

bounding this to v1.x and clearly stating there aren't current plans for a v2.x seems reasonable

Copy link
Member Author

Choose a reason for hiding this comment

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

added a commit to that effect

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.

LGTM
for SIG Docs

Concern about long-term API legacy is a personal viewpoint.

@netlify
Copy link

netlify bot commented Jan 18, 2022

✔️ Deploy Preview for kubernetes-io-main-staging ready!

🔨 Explore the source changes: b828365

🔍 Inspect the deploy log: https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/61eed730769f260008ac6b84

😎 Browse the preview: https://deploy-preview-31389--kubernetes-io-main-staging.netlify.app

**Rule #4a: Other than the most recent API versions in each track, older API
versions must be supported after their announced deprecation for a duration of
no less than:**
**Rule #4a: minimum API lifetime is determined by the API stability level**
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to update it similarly for metrics?
Removal to "Stable" metrics would also be surprise for users...

Copy link
Member Author

Choose a reason for hiding this comment

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

that sounds like it makes sense, but I'd like the instrumentation leads to propose that change (in a separate PR)

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

https://kubernetes.io/docs/reference/using-api/deprecation-policy/#deprecating-a-metric still says they can be removed after 4 releases / 12 months

Copy link
Member

Choose a reason for hiding this comment

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

the latter..

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Stable metrics cannot be removed, but they can become deprecated and then removed.

That... doesn't seem very stable :)

I'd agree with @wojtek-t that this should be similar to the stability guarantees for our stable APIs, but I still think that change should be proposed in a follow up PR

Copy link
Member

Choose a reason for hiding this comment

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

That... doesn't seem very stable :)

+1

[Followup PR sounds fine to me of course - those are tightly coupled.]

Copy link
Member

Choose a reason for hiding this comment

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

Stable metrics cannot be removed, but they can become deprecated and then removed.

That... doesn't seem very stable :)

You talked me into 4 releases (it was proposed to be 3 initially). Forever is a much bigger commitment...

@thockin
Copy link
Member

thockin commented Jan 21, 2022 via email

Copy link
Member

@thockin thockin left a comment

Choose a reason for hiding this comment

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

+1 from me

* **GA: 12 months or 3 releases (whichever is longer)**
* **Beta: 9 months or 3 releases (whichever is longer)**
* **Alpha: 0 releases**
* **GA: may be marked as deprecated, but must not be removed**
Copy link
Member

Choose a reason for hiding this comment

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

I agree with @sftim - if we wrap all of these rules with "within a major version of Kubernetes", we at least leave room to discuss a v2 and whether it would be worth the effort to EOL some APIs. I don't even mind saying "as of Q1 2022, there are no plans for a v2.x.x of Kubernetes"

@thockin
Copy link
Member

thockin commented Jan 22, 2022

LGTM

Not tagging it yet, in case we want other approvals.

@dims
Copy link
Member

dims commented Jan 24, 2022

/approve
/lgtm

thanks for leading the discussion and building consensus @liggitt

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

LGTM label has been added.

Git tree hash: d92091cfe92b8c41f5361f4387344d3cc9f390e6

@smarterclayton
Copy link
Contributor

LGTM as well

@sftim
Copy link
Contributor

sftim commented Jan 24, 2022

/approve

I'll leave the hold in place for now. Happy for anyone representing SIG Architecture to /unhold, or otherwise a SIG Docs approver is welcome to /unhold this in a few days.

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 24, 2022
@liggitt
Copy link
Member Author

liggitt commented Jan 24, 2022

thanks, would like @derekwaynecarr and @johnbelamaric to ack as well for sig-arch, then I can unhold

@johnbelamaric
Copy link
Member

/approve

@derekwaynecarr
Copy link
Member

/approve
/lgtm

@derekwaynecarr
Copy link
Member

i think all have acked on this.

/hold cancel

@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 Jan 25, 2022
@k8s-ci-robot k8s-ci-robot merged commit 7012f76 into kubernetes:main Jan 25, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: derekwaynecarr, dims, johnbelamaric, sftim

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

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/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
Status: API review completed, 1.24
Development

Successfully merging this pull request may close these issues.