From 850dc6123afbeabf9d47d2882e3598f9e89c5177 Mon Sep 17 00:00:00 2001 From: carlory Date: Fri, 28 Apr 2023 14:40:14 +0800 Subject: [PATCH] Remove ability to re-enable serving deprecated policyv1beta1 APIs --- pkg/controlplane/instance.go | 2 - pkg/registry/policy/rest/storage_policy.go | 23 ----- .../integration/disruption/disruption_test.go | 94 ++++++++----------- test/integration/etcd/data.go | 8 -- 4 files changed, 38 insertions(+), 89 deletions(-) diff --git a/pkg/controlplane/instance.go b/pkg/controlplane/instance.go index 396633f1da2..c40f3849266 100644 --- a/pkg/controlplane/instance.go +++ b/pkg/controlplane/instance.go @@ -47,7 +47,6 @@ import ( networkingapiv1alpha1 "k8s.io/api/networking/v1alpha1" nodev1 "k8s.io/api/node/v1" policyapiv1 "k8s.io/api/policy/v1" - policyapiv1beta1 "k8s.io/api/policy/v1beta1" rbacv1 "k8s.io/api/rbac/v1" resourcev1alpha2 "k8s.io/api/resource/v1alpha2" schedulingapiv1 "k8s.io/api/scheduling/v1" @@ -719,7 +718,6 @@ var ( // betaAPIGroupVersionsDisabledByDefault is for all future beta groupVersions. betaAPIGroupVersionsDisabledByDefault = []schema.GroupVersion{ authenticationv1beta1.SchemeGroupVersion, - policyapiv1beta1.SchemeGroupVersion, storageapiv1beta1.SchemeGroupVersion, flowcontrolv1beta1.SchemeGroupVersion, flowcontrolv1beta2.SchemeGroupVersion, diff --git a/pkg/registry/policy/rest/storage_policy.go b/pkg/registry/policy/rest/storage_policy.go index 1eabc0dda64..deb61929e81 100644 --- a/pkg/registry/policy/rest/storage_policy.go +++ b/pkg/registry/policy/rest/storage_policy.go @@ -18,7 +18,6 @@ package rest import ( policyapiv1 "k8s.io/api/policy/v1" - policyapiv1beta1 "k8s.io/api/policy/v1beta1" "k8s.io/apiserver/pkg/registry/generic" "k8s.io/apiserver/pkg/registry/rest" genericapiserver "k8s.io/apiserver/pkg/server" @@ -35,12 +34,6 @@ func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorag // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. // TODO refactor the plumbing to provide the information in the APIGroupInfo - if storageMap, err := p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { - return genericapiserver.APIGroupInfo{}, err - } else if len(storageMap) > 0 { - apiGroupInfo.VersionedResourcesStorageMap[policyapiv1beta1.SchemeGroupVersion.Version] = storageMap - } - if storageMap, err := p.v1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { return genericapiserver.APIGroupInfo{}, err } else if len(storageMap) > 0 { @@ -50,22 +43,6 @@ func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorag return apiGroupInfo, nil } -func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { - storage := map[string]rest.Storage{} - - // poddisruptionbudgets - if resource := "poddisruptionbudgets"; apiResourceConfigSource.ResourceEnabled(policyapiv1beta1.SchemeGroupVersion.WithResource(resource)) { - poddisruptionbudgetStorage, poddisruptionbudgetStatusStorage, err := poddisruptionbudgetstore.NewREST(restOptionsGetter) - if err != nil { - return storage, err - } - storage[resource] = poddisruptionbudgetStorage - storage[resource+"/status"] = poddisruptionbudgetStatusStorage - } - - return storage, nil -} - func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} diff --git a/test/integration/disruption/disruption_test.go b/test/integration/disruption/disruption_test.go index 04c9d65a320..bb10fe4c2d0 100644 --- a/test/integration/disruption/disruption_test.go +++ b/test/integration/disruption/disruption_test.go @@ -17,14 +17,17 @@ limitations under the License. package disruption import ( + "bytes" "context" "fmt" + "path" "reflect" "testing" "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + clientv3 "go.etcd.io/etcd/client/v3" v1 "k8s.io/api/core/v1" policyv1 "k8s.io/api/policy/v1" "k8s.io/api/policy/v1beta1" @@ -34,9 +37,12 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/runtime/serializer/protobuf" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/wait" + genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + "k8s.io/apiserver/pkg/registry/rest" cacheddiscovery "k8s.io/client-go/discovery/cached/memory" "k8s.io/client-go/dynamic" "k8s.io/client-go/informers" @@ -46,6 +52,7 @@ import ( "k8s.io/client-go/scale" "k8s.io/client-go/tools/cache" kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" + "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/controller/disruption" "k8s.io/kubernetes/test/integration/etcd" "k8s.io/kubernetes/test/integration/framework" @@ -201,12 +208,12 @@ func TestPDBWithScaleSubresource(t *testing.T) { func TestEmptySelector(t *testing.T) { testcases := []struct { name string - createPDBFunc func(ctx context.Context, clientSet clientset.Interface, name, nsName string, minAvailable intstr.IntOrString) error + createPDBFunc func(ctx context.Context, clientSet clientset.Interface, etcdClient *clientv3.Client, etcdStoragePrefix, name, nsName string, minAvailable intstr.IntOrString) error expectedCurrentHealthy int32 }{ { name: "v1beta1 should not target any pods", - createPDBFunc: func(ctx context.Context, clientSet clientset.Interface, name, nsName string, minAvailable intstr.IntOrString) error { + createPDBFunc: func(ctx context.Context, clientSet clientset.Interface, etcdClient *clientv3.Client, etcdStoragePrefix, name, nsName string, minAvailable intstr.IntOrString) error { pdb := &v1beta1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -216,14 +223,13 @@ func TestEmptySelector(t *testing.T) { Selector: &metav1.LabelSelector{}, }, } - _, err := clientSet.PolicyV1beta1().PodDisruptionBudgets(nsName).Create(ctx, pdb, metav1.CreateOptions{}) - return err + return createPDBUsingRemovedAPI(ctx, etcdClient, etcdStoragePrefix, nsName, pdb) }, expectedCurrentHealthy: 0, }, { name: "v1 should target all pods", - createPDBFunc: func(ctx context.Context, clientSet clientset.Interface, name, nsName string, minAvailable intstr.IntOrString) error { + createPDBFunc: func(ctx context.Context, clientSet clientset.Interface, etcdClient *clientv3.Client, etcdStoragePrefix, name, nsName string, minAvailable intstr.IntOrString) error { pdb := &policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -266,7 +272,7 @@ func TestEmptySelector(t *testing.T) { waitToObservePods(t, informers.Core().V1().Pods().Informer(), 4, v1.PodRunning) pdbName := "test-pdb" - if err := tc.createPDBFunc(ctx, clientSet, pdbName, nsName, minAvailable); err != nil { + if err := tc.createPDBFunc(ctx, clientSet, s.EtcdClient, s.EtcdStoragePrefix, pdbName, nsName, minAvailable); err != nil { t.Errorf("Error creating PodDisruptionBudget: %v", err) } @@ -287,12 +293,12 @@ func TestEmptySelector(t *testing.T) { func TestSelectorsForPodsWithoutLabels(t *testing.T) { testcases := []struct { name string - createPDBFunc func(ctx context.Context, clientSet clientset.Interface, name, nsName string, minAvailable intstr.IntOrString) error + createPDBFunc func(ctx context.Context, clientSet clientset.Interface, etcdClient *clientv3.Client, etcdStoragePrefix, name, nsName string, minAvailable intstr.IntOrString) error expectedCurrentHealthy int32 }{ { name: "pods with no labels can be targeted by v1 PDBs with empty selector", - createPDBFunc: func(ctx context.Context, clientSet clientset.Interface, name, nsName string, minAvailable intstr.IntOrString) error { + createPDBFunc: func(ctx context.Context, clientSet clientset.Interface, etcdClient *clientv3.Client, etcdStoragePrefix, name, nsName string, minAvailable intstr.IntOrString) error { pdb := &policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -309,7 +315,7 @@ func TestSelectorsForPodsWithoutLabels(t *testing.T) { }, { name: "pods with no labels can be targeted by v1 PDBs with DoesNotExist selector", - createPDBFunc: func(ctx context.Context, clientSet clientset.Interface, name, nsName string, minAvailable intstr.IntOrString) error { + createPDBFunc: func(ctx context.Context, clientSet clientset.Interface, etcdClient *clientv3.Client, etcdStoragePrefix, name, nsName string, minAvailable intstr.IntOrString) error { pdb := &policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -333,7 +339,7 @@ func TestSelectorsForPodsWithoutLabels(t *testing.T) { }, { name: "pods with no labels can be targeted by v1beta1 PDBs with DoesNotExist selector", - createPDBFunc: func(ctx context.Context, clientSet clientset.Interface, name, nsName string, minAvailable intstr.IntOrString) error { + createPDBFunc: func(ctx context.Context, clientSet clientset.Interface, etcdClient *clientv3.Client, etcdStoragePrefix, name, nsName string, minAvailable intstr.IntOrString) error { pdb := &v1beta1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -350,8 +356,7 @@ func TestSelectorsForPodsWithoutLabels(t *testing.T) { }, }, } - _, err := clientSet.PolicyV1beta1().PodDisruptionBudgets(nsName).Create(ctx, pdb, metav1.CreateOptions{}) - return err + return createPDBUsingRemovedAPI(ctx, etcdClient, etcdStoragePrefix, nsName, pdb) }, expectedCurrentHealthy: 1, }, @@ -376,7 +381,7 @@ func TestSelectorsForPodsWithoutLabels(t *testing.T) { // Create the PDB first and wait for it to settle. pdbName := "test-pdb" - if err := tc.createPDBFunc(ctx, clientSet, pdbName, nsName, minAvailable); err != nil { + if err := tc.createPDBFunc(ctx, clientSet, s.EtcdClient, s.EtcdStoragePrefix, pdbName, nsName, minAvailable); err != nil { t.Errorf("Error creating PodDisruptionBudget: %v", err) } waitPDBStable(ctx, t, clientSet, 0, nsName, pdbName) @@ -513,6 +518,26 @@ func waitToObservePods(t *testing.T, podInformer cache.SharedIndexInformer, podN } } +// createPDBUsingRemovedAPI creates a PDB directly using etcd. This is must *ONLY* be used for checks of compatibility +// with removed data. Do not use this just because you don't want to update your test to use v1. Only use this +// when it actually matters. +func createPDBUsingRemovedAPI(ctx context.Context, etcdClient *clientv3.Client, etcdStoragePrefix, nsName string, betaPDB *v1beta1.PodDisruptionBudget) error { + betaPDB.APIVersion = v1beta1.SchemeGroupVersion.Group + "/" + v1beta1.SchemeGroupVersion.Version + betaPDB.Kind = "PodDisruptionBudget" + betaPDB.Namespace = nsName + betaPDB.Generation = 1 + rest.FillObjectMetaSystemFields(betaPDB) + ctx = genericapirequest.WithNamespace(ctx, nsName) + key := path.Join("/", etcdStoragePrefix, "poddisruptionbudgets", nsName, betaPDB.Name) + protoSerializer := protobuf.NewSerializer(legacyscheme.Scheme, legacyscheme.Scheme) + buffer := bytes.NewBuffer(nil) + if err := protoSerializer.Encode(betaPDB, buffer); err != nil { + return err + } + _, err := etcdClient.Put(ctx, key, buffer.String()) + return err +} + func TestPatchCompatibility(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) @@ -533,17 +558,6 @@ func TestPatchCompatibility(t *testing.T) { fieldManager string expectSelector *metav1.LabelSelector }{ - { - name: "v1beta1-smp", - version: "v1beta1", - patchType: types.StrategicMergePatchType, - patch: `{"spec":{"selector":{"matchLabels":{"patchmatch":"true"},"matchExpressions":[{"key":"patchexpression","operator":"In","values":["true"]}]}}}`, - // matchLabels portion is merged, matchExpressions portion is replaced (because it's a list with no patchStrategy defined) - expectSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"basematch": "true", "patchmatch": "true"}, - MatchExpressions: []metav1.LabelSelectorRequirement{{Key: "patchexpression", Operator: "In", Values: []string{"true"}}}, - }, - }, { name: "v1-smp", version: "v1", @@ -555,18 +569,6 @@ func TestPatchCompatibility(t *testing.T) { MatchExpressions: []metav1.LabelSelectorRequirement{{Key: "patchexpression", Operator: "In", Values: []string{"true"}}}, }, }, - - { - name: "v1beta1-mergepatch", - version: "v1beta1", - patchType: types.MergePatchType, - patch: `{"spec":{"selector":{"matchLabels":{"patchmatch":"true"},"matchExpressions":[{"key":"patchexpression","operator":"In","values":["true"]}]}}}`, - // matchLabels portion is merged, matchExpressions portion is replaced (because it's a list) - expectSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"basematch": "true", "patchmatch": "true"}, - MatchExpressions: []metav1.LabelSelectorRequirement{{Key: "patchexpression", Operator: "In", Values: []string{"true"}}}, - }, - }, { name: "v1-mergepatch", version: "v1", @@ -578,20 +580,6 @@ func TestPatchCompatibility(t *testing.T) { MatchExpressions: []metav1.LabelSelectorRequirement{{Key: "patchexpression", Operator: "In", Values: []string{"true"}}}, }, }, - - { - name: "v1beta1-apply", - version: "v1beta1", - patchType: types.ApplyPatchType, - patch: `{"apiVersion":"policy/v1beta1","kind":"PodDisruptionBudget","spec":{"selector":{"matchLabels":{"patchmatch":"true"},"matchExpressions":[{"key":"patchexpression","operator":"In","values":["true"]}]}}}`, - force: pointer.Bool(true), - fieldManager: "test", - // entire selector is replaced (because structType=atomic) - expectSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"patchmatch": "true"}, - MatchExpressions: []metav1.LabelSelectorRequirement{{Key: "patchexpression", Operator: "In", Values: []string{"true"}}}, - }, - }, { name: "v1-apply", version: "v1", @@ -641,12 +629,6 @@ func TestPatchCompatibility(t *testing.T) { t.Fatal(err) } resultSelector = result.Spec.Selector - case "v1beta1": - result, err := clientSet.PolicyV1beta1().PodDisruptionBudgets(ns).Patch(context.TODO(), pdb.Name, tc.patchType, []byte(tc.patch), metav1.PatchOptions{Force: tc.force, FieldManager: tc.fieldManager}) - if err != nil { - t.Fatal(err) - } - resultSelector = result.Spec.Selector default: t.Error("unknown version") } diff --git a/test/integration/etcd/data.go b/test/integration/etcd/data.go index c4d2cae7587..c6359f6f448 100644 --- a/test/integration/etcd/data.go +++ b/test/integration/etcd/data.go @@ -224,14 +224,6 @@ func GetEtcdStorageDataForNamespace(namespace string) map[schema.GroupVersionRes }, // -- - // k8s.io/kubernetes/pkg/apis/policy/v1beta1 - gvr("policy", "v1beta1", "poddisruptionbudgets"): { - Stub: `{"metadata": {"name": "pdb1"}, "spec": {"selector": {"matchLabels": {"anokkey": "anokvalue"}}}}`, - ExpectedEtcdPath: "/registry/poddisruptionbudgets/" + namespace + "/pdb1", - ExpectedGVK: gvkP("policy", "v1", "PodDisruptionBudget"), - }, - // -- - // k8s.io/kubernetes/pkg/apis/storage/v1alpha1 gvr("storage.k8s.io", "v1alpha1", "csistoragecapacities"): { Stub: `{"metadata": {"name": "csc-12345-1"}, "storageClassName": "sc1"}`,