stop serving deleted APIs #90983

deads2k · 2020-05-11T15:44:27Z

This programatically enforces the removal of beta APIs that have expired. Alpha levels have a grace period so that we can chase down their removal smoothly with CI by disabling the affected tests. Controllers already had to handle the beta level detection, so they shouldn't be an issue.

/kind bug
/priority important-soon
@kubernetes/sig-api-machinery-bugs

/hold the factorization is whacky, but I want to record the spot

The kube-apiserver will no longer serve APIs that should have been deleted in GA non-alpha levels.  Alpha levels will continue to serve the removed APIs so that CI doesn't immediately break.

fedebongio · 2020-05-12T20:09:04Z

/assign @liggitt
/cc @lavalamp

sttts · 2020-11-05T10:11:20Z

Since when do we fail on unused private methods?

pkg/controlplane/deleted_kinds_test.go:96:6: func storageNeverRemoved is unused (U1000)

sttts · 2020-11-11T08:28:00Z

pkg/controlplane/deleted_kinds.go

+	// This is usually set by a cluster-admin looking for a short-term escape hatch after something bad happened.
+	// This should be made a flag before merge
+	// Set KUBE_APISERVER_SERVE_REMOVED_APIS_FOR_ONE_RELEASE to prevent removing APIs for one more release.
+	serveRemoveAPIsOneMoreRelease bool


sttts · 2020-11-11T08:49:25Z

pkg/controlplane/deleted_kinds.go

+			}
+		}
+
+		for resourceName := range versionedResourcesStorageMap[apiVersion] {


why this double loop, and versionedResourcesStorageMap[apiVersion] vs. versionToResource?

why this double loop, and versionedResourcesStorageMap[apiVersion] vs. versionToResource?

10 lines up, the godoc describes the args. This mirrors the structure in APIGroupInfo, so per the comment it's a map from version to resource to the storage.. If I change it here, the usage becomes more difficult and the results cannot be directly used.

sttts · 2020-11-11T08:51:42Z

pkg/controlplane/deleted_kinds.go

+	}
+}
+
+func shouldRemoveResource(resourcesToRemove sets.String, resourceName string) bool {


shouldRemoveResourceAndSubresources

sttts · 2020-11-11T08:51:59Z

pkg/controlplane/deleted_kinds.go

+			return true
+		}
+		// our API works on nesting, so you can have deployments, deployments/status, and deployments/scale.  Not all subresources
+		// server the parent type, but if the parent type (deployments in this case), has been removed, it's subresources should be removed too.


serve the parent type

sttts · 2020-11-11T08:52:39Z

pkg/controlplane/instance.go

@@ -25,6 +25,8 @@ import (
 	"strconv"
 	"time"

+	"k8s.io/component-base/version"


sttts · 2020-11-11T08:53:55Z

/lgtm
/approve
/hold

Just some nits. Feel free to unhold and retag after fixing.

k8s-ci-robot · 2020-11-11T08:54:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, sttts

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:

~~pkg/controlplane/OWNERS~~ [deads2k,sttts]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

deads2k · 2020-11-11T13:53:45Z

$ git fetch https://github.com/kubernetes/kubernetes.git pull/90983/head
From https://github.com/kubernetes/kubernetes
 * branch                    refs/pull/90983/head -> FETCH_HEAD
$ git merge --no-ff a6a6b17348909030b7563dd1c21fa166c668844b
merge: a6a6b17348909030b7563dd1c21fa166c668844b - not something we can merge
# Error: exit status 1

slow github repo update?

/retest

deads2k · 2020-11-11T18:20:31Z

/retest

deads2k · 2020-11-12T14:15:27Z

/retest

deads2k · 2020-11-12T20:13:54Z

/retest

sttts · 2020-11-13T07:39:29Z

/retest
/lgtm

k8s-ci-robot · 2020-11-13T07:41:20Z

@deads2k: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-kubernetes-e2e-gce-100-performance	`48ac4d4`	link	`/test pull-kubernetes-e2e-gce-100-performance`
pull-kubernetes-node-e2e	`48ac4d4`	link	`/test pull-kubernetes-node-e2e`
pull-kubernetes-e2e-gce-ubuntu-containerd	`48ac4d4`	link	`/test pull-kubernetes-e2e-gce-ubuntu-containerd`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

liggitt · 2020-11-18T14:23:46Z

replaced by #96525

/close

k8s-ci-robot · 2020-11-18T14:24:01Z

@liggitt: Closed this PR.

In response to this:

replaced by #96525

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot requested review from hongchaodeng and lavalamp May 11, 2020 15:47

deads2k force-pushed the beta-stop-serving branch from ff89261 to 8f65c80 Compare May 11, 2020 18:19

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 11, 2020

deads2k force-pushed the beta-stop-serving branch from 8f65c80 to 99fa877 Compare May 11, 2020 18:57

k8s-ci-robot assigned liggitt May 12, 2020

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 17, 2020

deads2k mentioned this pull request May 26, 2020

Require Transition from Beta kubernetes/enhancements#1635

Closed

deads2k force-pushed the beta-stop-serving branch from 99fa877 to 909412c Compare July 6, 2020 19:23

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 6, 2020

liggitt added this to the v1.20 milestone Jul 8, 2020

This was referenced Aug 13, 2020

1.22: stop serving admissionregistration.k8s.io/v1beta1 by default #82021

Closed

1.22: stop serving apiextensions.k8s.io/v1beta1 by default #82022

Closed

justaugustus modified the milestones: v1.20, v1.20-phase-bug Aug 27, 2020

justaugustus modified the milestones: v1.20-phase-bug, v1.20 Sep 8, 2020

deads2k force-pushed the beta-stop-serving branch 2 times, most recently from d05ae11 to 9d1e80c Compare November 4, 2020 21:20

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 4, 2020

deads2k force-pushed the beta-stop-serving branch from 9d1e80c to 6b72016 Compare November 5, 2020 14:19

sttts reviewed Nov 11, 2020

View reviewed changes

pkg/controlplane/instance.go Outdated

@@ -25,6 +25,8 @@ import (

"strconv"

"time"

"k8s.io/component-base/version"

Copy link

Contributor

sttts Nov 11, 2020

Choose a reason for hiding this comment

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

order

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 11, 2020

k8s-ci-robot assigned sttts Nov 11, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 11, 2020

deads2k force-pushed the beta-stop-serving branch from 6b72016 to a6a6b17 Compare November 11, 2020 13:03

k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 11, 2020

stop serving deleted APIs

48ac4d4

deads2k force-pushed the beta-stop-serving branch from a6a6b17 to 48ac4d4 Compare November 12, 2020 14:21

deads2k mentioned this pull request Nov 12, 2020

stop serving deleted APIs #96525

Merged

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 13, 2020

k8s-ci-robot closed this Nov 18, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stop serving deleted APIs #90983

stop serving deleted APIs #90983

deads2k commented May 11, 2020

fedebongio commented May 12, 2020

sttts commented Nov 5, 2020

sttts Nov 11, 2020

sttts Nov 11, 2020

deads2k Nov 11, 2020

sttts Nov 11, 2020

sttts Nov 11, 2020

sttts Nov 11, 2020

sttts commented Nov 11, 2020

k8s-ci-robot commented Nov 11, 2020

deads2k commented Nov 11, 2020

deads2k commented Nov 11, 2020

deads2k commented Nov 12, 2020

deads2k commented Nov 12, 2020

sttts commented Nov 13, 2020

k8s-ci-robot commented Nov 13, 2020 •

edited

liggitt commented Nov 18, 2020

k8s-ci-robot commented Nov 18, 2020

stop serving deleted APIs #90983

stop serving deleted APIs #90983

Conversation

deads2k commented May 11, 2020

fedebongio commented May 12, 2020

sttts commented Nov 5, 2020

sttts Nov 11, 2020

Choose a reason for hiding this comment

sttts Nov 11, 2020

Choose a reason for hiding this comment

deads2k Nov 11, 2020

Choose a reason for hiding this comment

sttts Nov 11, 2020

Choose a reason for hiding this comment

sttts Nov 11, 2020

Choose a reason for hiding this comment

sttts Nov 11, 2020

Choose a reason for hiding this comment

sttts commented Nov 11, 2020

k8s-ci-robot commented Nov 11, 2020

deads2k commented Nov 11, 2020

deads2k commented Nov 11, 2020

deads2k commented Nov 12, 2020

deads2k commented Nov 12, 2020

sttts commented Nov 13, 2020

k8s-ci-robot commented Nov 13, 2020 • edited

liggitt commented Nov 18, 2020

k8s-ci-robot commented Nov 18, 2020

k8s-ci-robot commented Nov 13, 2020 •

edited