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

Deprecate SelfLink and introduce feature gate to disable its propagation #80978

Merged
merged 3 commits into from Aug 8, 2019

Conversation

@wojtek-t
Copy link
Member

commented Aug 5, 2019

Ref kubernetes/enhancements#1164

API: the metadata.selfLink field is deprecated in individual and list objects. It will no longer be returned starting in v1.20, and the field will be removed entirely in v1.21.

@wojtek-t wojtek-t self-assigned this Aug 5, 2019

@wojtek-t wojtek-t changed the title [WIP Deprecate SelfLink and introduce feature gate to disable its propagation [WIP] Deprecate SelfLink and introduce feature gate to disable its propagation Aug 5, 2019

@k8s-ci-robot k8s-ci-robot requested review from apelisse and caesarxuchao Aug 5, 2019

@wojtek-t wojtek-t force-pushed the wojtek-t:selflink_deprecation branch 2 times, most recently from 03891b2 to 50ebc32 Aug 5, 2019

@wojtek-t wojtek-t force-pushed the wojtek-t:selflink_deprecation branch 2 times, most recently from cbcff20 to feaa295 Aug 5, 2019

@k8s-ci-robot k8s-ci-robot added size/L sig/apps and removed size/M labels Aug 5, 2019

@wojtek-t wojtek-t force-pushed the wojtek-t:selflink_deprecation branch from 70523d5 to f2bef76 Aug 7, 2019

@wojtek-t
Copy link
Member Author

left a comment

@liggitt - thanks a lot for comments; PTAL

@@ -270,6 +272,10 @@ func transformDecodeError(typer runtime.ObjectTyper, baseErr error, into runtime
// setSelfLink sets the self link of an object (or the child items in a list) to the base URL of the request
// plus the path and query generated by the provided linkFunc
func setSelfLink(obj runtime.Object, requestInfo *request.RequestInfo, namer ScopeNamer) error {
if utilfeature.DefaultFeatureGate.Enabled(features.RemoveSelfLink) {

This comment has been minimized.

Copy link
@wojtek-t

wojtek-t Aug 7, 2019

Author Member

great catch - refactored so that it's called once per request

} else {
version = parts[2] + "/" + parts[3]
if len(gvks) == 0 || gvks[0].Empty() {
return nil, fmt.Errorf("unexpected gvks registered for object %v: %v", obj, gvks)

This comment has been minimized.

Copy link
@liggitt

liggitt Aug 7, 2019

Member

don't log the full object, just %T

This comment has been minimized.

Copy link
@wojtek-t

wojtek-t Aug 7, 2019

Author Member

done

expectedRefVersion: "v1",
},
{
name: "foo.group/v3 from selflink",
name: "foo.group/v3 GV from scheme",
input: &TestRuntimeObj{
ObjectMeta: metav1.ObjectMeta{SelfLink: "/apis/foo.group/v3/namespaces"},

This comment has been minimized.

Copy link
@liggitt

liggitt Aug 7, 2019

Member

make the groupVersion and selfLink different so it is clear the data is extracted from the scheme groupVersion. same for the other test case

This comment has been minimized.

Copy link
@wojtek-t

wojtek-t Aug 7, 2019

Author Member

done

@liggitt

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019

a couple nits, then squash and lgtm

wojtek-t added some commits Aug 5, 2019

@wojtek-t wojtek-t force-pushed the wojtek-t:selflink_deprecation branch from f2bef76 to f0a8aec Aug 7, 2019

@wojtek-t

This comment has been minimized.

Copy link
Member Author

commented Aug 7, 2019

Thanks a lot Jordan - PTAL

@wojtek-t

This comment has been minimized.

Copy link
Member Author

commented Aug 7, 2019

Also - does the release-note look ok to you?

@liggitt

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019

updated the release note. lgtm, needs squash

@liggitt

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019

/lgtm
/approve
/retest

@k8s-ci-robot k8s-ci-robot added the lgtm label Aug 7, 2019

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Aug 7, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, wojtek-t

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

@fejta-bot

This comment has been minimized.

Copy link

commented Aug 7, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

1 similar comment
@fejta-bot

This comment has been minimized.

Copy link

commented Aug 8, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@k8s-ci-robot k8s-ci-robot merged commit ef88694 into kubernetes:master Aug 8, 2019

23 checks passed

cla/linuxfoundation wojtek-t authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-csi-serial Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-iscsi Skipped.
pull-kubernetes-e2e-gce-iscsi-serial Skipped.
pull-kubernetes-e2e-gce-storage-slow Job succeeded.
Details
pull-kubernetes-godeps Skipped.
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-node-e2e-containerd Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details

@k8s-ci-robot k8s-ci-robot added this to the v1.16 milestone Aug 8, 2019

k8s-ci-robot added a commit that referenced this pull request Aug 16, 2019

Merge pull request #81407 from wojtek-t/automated-cherry-pick-of-#809…
…78-upstream-release-1.14

Manual cherry pick of #80978 upstream release 1.14

k8s-ci-robot added a commit that referenced this pull request Aug 16, 2019

Merge pull request #81355 from wojtek-t/automated-cherry-pick-of-#809…
…78-upstream-release-1.15

Manual cherry pick of #80978 upstream release 1.15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.