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.

Sign up for GitHub

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

Jump to bottom

Graduate SelfLink deprecation to GA #31756

Merged
merged 1 commit into from Feb 28, 2022
Merged

Graduate SelfLink deprecation to GA #31756

merged 1 commit into from Feb 28, 2022

Conversation

wojtek-t
Copy link
Member

@k8s-ci-robot k8s-ci-robot added this to the 1.24 milestone Feb 15, 2022
@netlify
Copy link

netlify bot commented Feb 15, 2022 •
edited

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

🔨 Explore the source changes: 86af7c1

🔍 Inspect the deploy log: https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/62135acf722fb00008f18bd5

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 15, 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 Feb 15, 2022
@wojtek-t wojtek-t mentioned this pull request Feb 15, 2022
Closed
sftim
sftim previously requested changes Feb 15, 2022
View reviewed changes
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 @wojtek-t

Let's document which release deprecated selfLink, and edit the feature gate description to make that clear. The current text is a bit off because a literal reading implies the end user has control over whether the field is deprecated.

Additionally, we should describe the effect of this feature gate in terms of the API and its fields (camelCase), rather than the Golang code for our reference implementation (ListMeta, etc).

For example:

- - `RemoveSelfLink`: Deprecates and removes `selfLink` from ObjectMeta and-
-   ListMeta.
+ - `RemoveSelfLink`: Sets the `.metadata.selfLink` field to blank (empty string) for all
+   objects and collections. This field has been deprecated since the Kubernetes v1.xx
+   release. When this feature is enabled, the `.metadata.selfLink` field remains part of
+   the Kubernetes API, but is always unset.

@nate-double-u
Copy link
Contributor

/assign @mehabhalodiya

@wojtek-t
Copy link
Member Author

@sftim - thanks; PTAL

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 16, 2022
@sftim sftim dismissed their stale review February 16, 2022 10:24

Feedback incorporated

sftim
sftim reviewed Feb 16, 2022
View reviewed changes
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.

content/en/docs/reference/command-line-tools-reference/feature-gates.md LGTM

We'd usually omit the other changes (which are to generated files) and let the docs generation phase for the v1.24 release take care of those details.

content/en/docs/reference/kubernetes-api/common-definitions/list-meta.md Outdated



SelfLink is DEPRECATED read-only field that is no longer populated by the system.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I might call this an unused and / or legacy field; “deprecated” means “you should stop using this” (ahead of future removal, etc), whereas the truth now is that it already gone. The field is retained to meet API compatibility promises but the only field value is "".

Copy link
Member

Choose a reason for hiding this comment

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

noted in the k/k PR, but putting Deprecated: in the godoc will trigger some tools to issue warnings on use, which seems helpful

@wojtek-t
Copy link
Member Author

@sftim - thanks for comments; PTAL

@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 Feb 16, 2022
sftim
sftim reviewed Feb 16, 2022
View reviewed changes
content/en/docs/reference/kubernetes-api/common-definitions/list-meta.md Outdated



SelfLink is a legacy read-only field that is no longer populated by the system.
Copy link
Contributor

Choose a reason for hiding this comment

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

This change needs to happen upstream first.

Copy link
Member Author

Choose a reason for hiding this comment

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

I opened kubernetes/kubernetes#108163 for it.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is now merged in upstream.

@wojtek-t
Copy link
Member Author

@sftim - PTAL

@tengqm
Copy link
Contributor

tengqm commented Feb 21, 2022

@wojtek-t Please remove changes to content/en/docs/reference/kubernetes-api/common-definitions/ because those files are auto-generated. They would be taken care of when 1.24 is released.

Other than this, the PR LGTM.

@wojtek-t
Copy link
Member Author

@tengqm - thanks; PTAL

@tengqm
Copy link
Contributor

tengqm commented Feb 21, 2022

/lgtm

Defer approval to release maester.

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

LGTM label has been added.

Git tree hash: 664eccd09945b83e78de097ff511b0544691fb9a

@kbhawkey
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kbhawkey

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 Feb 28, 2022
@k8s-ci-robot k8s-ci-robot merged commit d02e2d6 into kubernetes:dev-1.24 Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liggitt liggitt liggitt left review comments

@sftim sftim sftim left review comments

@divya-mohan0209 divya-mohan0209 Awaiting requested review from divya-mohan0209

@onlydole onlydole Awaiting requested review from onlydole

Assignees

@liggitt liggitt

@tengqm tengqm

@mehabhalodiya mehabhalodiya

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/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
Milestone
1.24
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@wojtek-t @nate-double-u @tengqm @k8s-ci-robot @kbhawkey @liggitt @sftim @mehabhalodiya