Merge pull request #96525 from deads2k/beta-stop-serving-2

stop serving deleted APIs
This commit is contained in:
Kubernetes Prow Robot 2020-11-13 10:53:14 -08:00 committed by GitHub
commit ec734aced7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 547 additions and 2 deletions

View File

@ -5,6 +5,7 @@ go_library(
srcs = [
"client_util.go",
"controller.go",
"deleted_kinds.go",
"doc.go",
"import_known_versions.go",
"instance.go",
@ -115,10 +116,13 @@ go_library(
"//staging/src/k8s.io/apimachinery/pkg/util/intstr:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/net:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/version:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/endpoints/discovery:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/features:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/registry/generic:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/registry/rest:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/server:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/server/healthz:go_default_library",
@ -130,6 +134,7 @@ go_library(
"//staging/src/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library",
"//staging/src/k8s.io/client-go/kubernetes/typed/discovery/v1beta1:go_default_library",
"//staging/src/k8s.io/client-go/rest:go_default_library",
"//staging/src/k8s.io/component-base/version:go_default_library",
"//staging/src/k8s.io/component-helpers/apimachinery/lease:go_default_library",
"//vendor/k8s.io/klog/v2:go_default_library",
"//vendor/k8s.io/utils/integer:go_default_library",
@ -143,6 +148,7 @@ go_test(
timeout = "long",
srcs = [
"controller_test.go",
"deleted_kinds_test.go",
"import_known_versions_test.go",
"instance_openapi_test.go",
"instance_test.go",
@ -166,6 +172,7 @@ go_test(
"//staging/src/k8s.io/api/discovery/v1beta1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/apitesting/naming:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/intstr:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/net:go_default_library",
@ -173,6 +180,7 @@ go_test(
"//staging/src/k8s.io/apimachinery/pkg/version:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/authorization/authorizerfactory:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/endpoints/openapi:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/registry/rest:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/server:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/server/options:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/server/resourceconfig:go_default_library",
@ -185,6 +193,7 @@ go_test(
"//staging/src/k8s.io/client-go/rest:go_default_library",
"//staging/src/k8s.io/client-go/testing:go_default_library",
"//staging/src/k8s.io/component-base/version:go_default_library",
"//vendor/github.com/davecgh/go-spew/spew:go_default_library",
"//vendor/github.com/go-openapi/loads:go_default_library",
"//vendor/github.com/go-openapi/spec:go_default_library",
"//vendor/github.com/go-openapi/strfmt:go_default_library",

View File

@ -0,0 +1,167 @@
/*
Copyright 2020 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package controlplane
import (
"os"
"strconv"
"strings"
"k8s.io/apimachinery/pkg/util/sets"
apimachineryversion "k8s.io/apimachinery/pkg/version"
"k8s.io/apiserver/pkg/registry/rest"
"k8s.io/klog/v2"
)
// resourceExpirationEvaluator holds info for deciding if a particular rest.Storage needs to excluded from the API
type resourceExpirationEvaluator struct {
currentMajor int
currentMinor int
isAlpha bool
// This is usually set for testing for which tests need to be removed. This prevent insta-failing CI.
// Set KUBE_APISERVER_STRICT_REMOVED_API_HANDLING_IN_ALPHA to see what will be removed when we tag beta
strictRemovedHandlingInAlpha bool
// 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.
serveRemovedAPIsOneMoreRelease bool
}
func newResourceExpirationEvaluator(currentVersion apimachineryversion.Info) (*resourceExpirationEvaluator, error) {
ret := &resourceExpirationEvaluator{}
if len(currentVersion.Major) > 0 {
currentMajor64, err := strconv.ParseInt(currentVersion.Major, 10, 32)
if err != nil {
return nil, err
}
ret.currentMajor = int(currentMajor64)
}
if len(currentVersion.Minor) > 0 {
// split the "normal" + and - for semver stuff
minorString := strings.Split(currentVersion.Minor, "+")[0]
minorString = strings.Split(minorString, "-")[0]
minorString = strings.Split(minorString, ".")[0]
currentMinor64, err := strconv.ParseInt(minorString, 10, 32)
if err != nil {
return nil, err
}
ret.currentMinor = int(currentMinor64)
}
ret.isAlpha = strings.Contains(currentVersion.GitVersion, "alpha")
if envString, ok := os.LookupEnv("KUBE_APISERVER_STRICT_REMOVED_API_HANDLING_IN_ALPHA"); !ok {
// do nothing
} else if envBool, err := strconv.ParseBool(envString); err != nil {
return nil, err
} else {
ret.strictRemovedHandlingInAlpha = envBool
}
if envString, ok := os.LookupEnv("KUBE_APISERVER_SERVE_REMOVED_APIS_FOR_ONE_RELEASE"); !ok {
// do nothing
} else if envBool, err := strconv.ParseBool(envString); err != nil {
return nil, err
} else {
ret.serveRemovedAPIsOneMoreRelease = envBool
}
return ret, nil
}
func (e *resourceExpirationEvaluator) shouldServe(resourceServingInfo rest.Storage) bool {
versionedPtr := resourceServingInfo.New()
removed, ok := versionedPtr.(removedInterface)
if !ok {
return true
}
majorRemoved, minorRemoved := removed.APILifecycleRemoved()
if e.currentMajor < majorRemoved {
return true
}
if e.currentMajor > majorRemoved {
return false
}
if e.currentMinor < minorRemoved {
return true
}
if e.currentMinor > minorRemoved {
return false
}
// at this point major and minor are equal, so this API should be removed when the current release GAs.
// If this is an alpha tag, don't remove by default, but allow the option.
// If the cluster-admin has requested serving one more release, allow it.
if e.isAlpha && e.strictRemovedHandlingInAlpha { // don't serve in alpha if we want strict handling
return false
}
if e.isAlpha { // alphas are allowed to continue serving expired betas while we clean up the test
return true
}
if e.serveRemovedAPIsOneMoreRelease { // cluster-admins are allowed to kick the can one release down the road
return true
}
return false
}
type removedInterface interface {
APILifecycleRemoved() (major, minor int)
}
// removeDeletedKinds inspects the storage map and modifies it in place by removing storage for kinds that have been deleted.
// versionedResourcesStorageMap mirrors the field on APIGroupInfo, it's a map from version to resource to the storage.
func (e *resourceExpirationEvaluator) removeDeletedKinds(groupName string, versionedResourcesStorageMap map[string]map[string]rest.Storage) {
versionsToRemove := sets.NewString()
for apiVersion, versionToResource := range versionedResourcesStorageMap {
resourcesToRemove := sets.NewString()
for resourceName, resourceServingInfo := range versionToResource {
if !e.shouldServe(resourceServingInfo) {
resourcesToRemove.Insert(resourceName)
}
}
for resourceName := range versionedResourcesStorageMap[apiVersion] {
if !shouldRemoveResourceAndSubresources(resourcesToRemove, resourceName) {
continue
}
klog.V(1).Infof("Removing resource %v.%v.%v because it is time to stop serving it per APILifecycle.", resourceName, apiVersion, groupName)
delete(versionedResourcesStorageMap[apiVersion], resourceName)
}
if len(versionedResourcesStorageMap[apiVersion]) == 0 {
versionsToRemove.Insert(apiVersion)
}
}
for _, apiVersion := range versionsToRemove.List() {
klog.V(1).Infof("Removing version %v.%v because it is time to stop serving it because it has no resources per APILifecycle.", apiVersion, groupName)
delete(versionedResourcesStorageMap, apiVersion)
}
}
func shouldRemoveResourceAndSubresources(resourcesToRemove sets.String, resourceName string) bool {
for _, resourceToRemove := range resourcesToRemove.List() {
if resourceName == resourceToRemove {
return true
}
// our API works on nesting, so you can have deployments, deployments/status, and deployments/scale. Not all subresources
// serve the parent type, but if the parent type (deployments in this case), has been removed, it's subresources should be removed too.
if strings.HasPrefix(resourceName, resourceToRemove+"/") {
return true
}
}
return false
}

View File

@ -0,0 +1,353 @@
/*
Copyright 2020 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package controlplane
import (
"reflect"
"strings"
"testing"
"github.com/davecgh/go-spew/spew"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/version"
"k8s.io/apiserver/pkg/registry/rest"
)
func Test_newResourceExpirationEvaluator(t *testing.T) {
tests := []struct {
name string
currentVersion version.Info
expected resourceExpirationEvaluator
expectedErr string
}{
{
name: "beta",
currentVersion: version.Info{
Major: "1",
Minor: "20+",
GitVersion: "v1.20.0-beta.0.62+a5d22854a2ac21",
},
expected: resourceExpirationEvaluator{currentMajor: 1, currentMinor: 20},
},
{
name: "alpha",
currentVersion: version.Info{
Major: "1",
Minor: "20+",
GitVersion: "v1.20.0-alpha.0.62+a5d22854a2ac21",
},
expected: resourceExpirationEvaluator{currentMajor: 1, currentMinor: 20, isAlpha: true},
},
{
name: "maintenance",
currentVersion: version.Info{
Major: "1",
Minor: "20+",
GitVersion: "v1.20.1",
},
expected: resourceExpirationEvaluator{currentMajor: 1, currentMinor: 20},
},
{
name: "bad",
currentVersion: version.Info{
Major: "1",
Minor: "20something+",
GitVersion: "v1.20.1",
},
expectedErr: `strconv.ParseInt: parsing "20something": invalid syntax`,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
actual, actualErr := newResourceExpirationEvaluator(tt.currentVersion)
checkErr(t, actualErr, tt.expectedErr)
if actualErr != nil {
return
}
if !reflect.DeepEqual(tt.expected, *actual) {
t.Fatal(actual)
}
})
}
}
func storageRemovedIn(major, minor int) removedInStorage {
return removedInStorage{major: major, minor: minor}
}
func storageNeverRemoved() removedInStorage {
return removedInStorage{neverRemoved: true}
}
type removedInStorage struct {
major, minor int
neverRemoved bool
}
func (r removedInStorage) New() runtime.Object {
if r.neverRemoved {
return neverRemovedObj{}
}
return removedInObj{major: r.major, minor: r.minor}
}
type neverRemovedObj struct {
}
func (r neverRemovedObj) GetObjectKind() schema.ObjectKind {
panic("don't do this")
}
func (r neverRemovedObj) DeepCopyObject() runtime.Object {
panic("don't do this either")
}
type removedInObj struct {
major, minor int
}
func (r removedInObj) GetObjectKind() schema.ObjectKind {
panic("don't do this")
}
func (r removedInObj) DeepCopyObject() runtime.Object {
panic("don't do this either")
}
func (r removedInObj) APILifecycleRemoved() (major, minor int) {
return r.major, r.minor
}
func Test_resourceExpirationEvaluator_shouldServe(t *testing.T) {
tests := []struct {
name string
resourceExpirationEvaluator resourceExpirationEvaluator
restStorage rest.Storage
expected bool
}{
{
name: "removed-in-curr",
resourceExpirationEvaluator: resourceExpirationEvaluator{
currentMajor: 1,
currentMinor: 20,
},
restStorage: storageRemovedIn(1, 20),
expected: false,
},
{
name: "removed-in-curr-but-deferred",
resourceExpirationEvaluator: resourceExpirationEvaluator{
currentMajor: 1,
currentMinor: 20,
serveRemovedAPIsOneMoreRelease: true,
},
restStorage: storageRemovedIn(1, 20),
expected: true,
},
{
name: "removed-in-curr-but-alpha",
resourceExpirationEvaluator: resourceExpirationEvaluator{
currentMajor: 1,
currentMinor: 20,
isAlpha: true,
},
restStorage: storageRemovedIn(1, 20),
expected: true,
},
{
name: "removed-in-curr-but-alpha-but-strict",
resourceExpirationEvaluator: resourceExpirationEvaluator{
currentMajor: 1,
currentMinor: 20,
isAlpha: true,
strictRemovedHandlingInAlpha: true,
},
restStorage: storageRemovedIn(1, 20),
expected: false,
},
{
name: "removed-in-prev-deferral-does-not-help",
resourceExpirationEvaluator: resourceExpirationEvaluator{
currentMajor: 1,
currentMinor: 21,
serveRemovedAPIsOneMoreRelease: true,
},
restStorage: storageRemovedIn(1, 20),
expected: false,
},
{
name: "removed-in-prev-major",
resourceExpirationEvaluator: resourceExpirationEvaluator{
currentMajor: 2,
currentMinor: 20,
serveRemovedAPIsOneMoreRelease: true,
},
restStorage: storageRemovedIn(1, 20),
expected: false,
},
{
name: "removed-in-future",
resourceExpirationEvaluator: resourceExpirationEvaluator{
currentMajor: 1,
currentMinor: 20,
},
restStorage: storageRemovedIn(1, 21),
expected: true,
},
{
name: "never-removed",
resourceExpirationEvaluator: resourceExpirationEvaluator{
currentMajor: 1,
currentMinor: 20,
},
restStorage: storageNeverRemoved(),
expected: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if actual := tt.resourceExpirationEvaluator.shouldServe(tt.restStorage); actual != tt.expected {
t.Errorf("shouldServe() = %v, want %v", actual, tt.expected)
}
})
}
}
func checkErr(t *testing.T, actual error, expected string) {
t.Helper()
switch {
case len(expected) == 0 && actual == nil:
case len(expected) == 0 && actual != nil:
t.Fatal(actual)
case len(expected) != 0 && actual == nil:
t.Fatalf("missing %q, <nil>", expected)
case len(expected) != 0 && actual != nil && !strings.Contains(actual.Error(), expected):
t.Fatalf("missing %q, %v", expected, actual)
}
}
func Test_removeDeletedKinds(t *testing.T) {
tests := []struct {
name string
resourceExpirationEvaluator resourceExpirationEvaluator
versionedResourcesStorageMap map[string]map[string]rest.Storage
expectedStorage map[string]map[string]rest.Storage
}{
{
name: "remove-one-of-two",
resourceExpirationEvaluator: resourceExpirationEvaluator{
currentMajor: 1,
currentMinor: 20,
},
versionedResourcesStorageMap: map[string]map[string]rest.Storage{
"v1": {
"twenty": storageRemovedIn(1, 20),
"twentyone": storageRemovedIn(1, 21),
},
},
expectedStorage: map[string]map[string]rest.Storage{
"v1": {
"twentyone": storageRemovedIn(1, 21),
},
},
},
{
name: "remove-nested-not-expired",
resourceExpirationEvaluator: resourceExpirationEvaluator{
currentMajor: 1,
currentMinor: 20,
},
versionedResourcesStorageMap: map[string]map[string]rest.Storage{
"v1": {
"twenty": storageRemovedIn(1, 20),
"twenty/scale": storageRemovedIn(1, 21),
"twentyone": storageRemovedIn(1, 21),
},
},
expectedStorage: map[string]map[string]rest.Storage{
"v1": {
"twentyone": storageRemovedIn(1, 21),
},
},
},
{
name: "remove-all-of-version",
resourceExpirationEvaluator: resourceExpirationEvaluator{
currentMajor: 1,
currentMinor: 20,
},
versionedResourcesStorageMap: map[string]map[string]rest.Storage{
"v1": {
"twenty": storageRemovedIn(1, 20),
},
"v2": {
"twenty": storageRemovedIn(1, 20),
"twentyone": storageRemovedIn(1, 21),
},
},
expectedStorage: map[string]map[string]rest.Storage{
"v2": {
"twentyone": storageRemovedIn(1, 21),
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tt.resourceExpirationEvaluator.removeDeletedKinds("group.name", tt.versionedResourcesStorageMap)
if !reflect.DeepEqual(tt.expectedStorage, tt.versionedResourcesStorageMap) {
t.Fatal(spew.Sdump(tt.versionedResourcesStorageMap))
}
})
}
}
func Test_shouldRemoveResource(t *testing.T) {
tests := []struct {
name string
resourcesToRemove sets.String
resourceName string
want bool
}{
{
name: "prefix-matches",
resourcesToRemove: sets.NewString("foo"),
resourceName: "foo/scale",
want: true,
},
{
name: "exact-matches",
resourcesToRemove: sets.NewString("foo"),
resourceName: "foo",
want: true,
},
{
name: "no-match",
resourcesToRemove: sets.NewString("foo"),
resourceName: "bar",
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if actual := shouldRemoveResourceAndSubresources(tt.resourcesToRemove, tt.resourceName); actual != tt.want {
t.Errorf("shouldRemoveResourceAndSubresources() = %v, want %v", actual, tt.want)
}
})
}
}

View File

@ -82,7 +82,9 @@ import (
"k8s.io/client-go/kubernetes"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
discoveryclient "k8s.io/client-go/kubernetes/typed/discovery/v1beta1"
"k8s.io/component-base/version"
"k8s.io/component-helpers/apimachinery/lease"
"k8s.io/klog/v2"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/controlplane/controller/apiserverleasegc"
"k8s.io/kubernetes/pkg/controlplane/controller/clusterauthenticationtrust"
@ -95,8 +97,6 @@ import (
"k8s.io/kubernetes/pkg/serviceaccount"
nodeutil "k8s.io/kubernetes/pkg/util/node"
"k8s.io/klog/v2"
// RESTStorage installers
admissionregistrationrest "k8s.io/kubernetes/pkg/registry/admissionregistration/rest"
apiserverinternalrest "k8s.io/kubernetes/pkg/registry/apiserverinternal/rest"
@ -581,6 +581,12 @@ type RESTStorageProvider interface {
func (m *Instance) InstallAPIs(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter, restStorageProviders ...RESTStorageProvider) error {
apiGroupsInfo := []*genericapiserver.APIGroupInfo{}
// used later in the loop to filter the served resource by those that have expired.
resourceExpirationEvaluator, err := newResourceExpirationEvaluator(version.Get())
if err != nil {
return err
}
for _, restStorageBuilder := range restStorageProviders {
groupName := restStorageBuilder.GroupName()
if !apiResourceConfigSource.AnyVersionForGroupEnabled(groupName) {
@ -595,6 +601,16 @@ func (m *Instance) InstallAPIs(apiResourceConfigSource serverstorage.APIResource
klog.Warningf("API group %q is not enabled, skipping.", groupName)
continue
}
// Remove resources that serving kinds that are removed.
// We do this here so that we don't accidentally serve versions without resources or openapi information that for kinds we don't serve.
// This is a spot above the construction of individual storage handlers so that no sig accidentally forgets to check.
resourceExpirationEvaluator.removeDeletedKinds(groupName, apiGroupInfo.VersionedResourcesStorageMap)
if len(apiGroupInfo.VersionedResourcesStorageMap) == 0 {
klog.V(1).Infof("Removing API group %v because it is time to stop serving it because it has no versions per APILifecycle.", groupName)
continue
}
klog.V(1).Infof("Enabling API group %q.", groupName)
if postHookProvider, ok := restStorageBuilder.(genericapiserver.PostStartHookProvider); ok {