From 21fba79d453b0bab7153f46916126c754d10341e Mon Sep 17 00:00:00 2001 From: Morten Torkildsen Date: Sat, 20 Feb 2021 12:56:31 -0800 Subject: [PATCH] Promote PDBs to GA --- cmd/kube-apiserver/app/aggregator.go | 1 + cmd/kube-controller-manager/app/policy.go | 13 +- hack/.import-aliases | 1 + hack/lib/init.sh | 1 + pkg/apis/policy/helper.go | 51 ++++ pkg/apis/policy/install/install.go | 5 +- pkg/apis/policy/v1/conversion.go | 42 +++ pkg/apis/policy/v1/conversion_test.go | 127 +++++++++ pkg/apis/policy/v1/doc.go | 24 ++ pkg/apis/policy/v1/register.go | 45 ++++ pkg/apis/policy/v1beta1/conversion.go | 66 +++++ pkg/apis/policy/v1beta1/conversion_test.go | 197 ++++++++++++++ pkg/controller/disruption/disruption.go | 11 +- pkg/controller/disruption/disruption_test.go | 16 +- pkg/controlplane/instance.go | 2 + .../storageversionhashdata/data.go | 1 + pkg/features/kube_features.go | 2 +- pkg/registry/core/pod/storage/eviction.go | 14 +- .../core/pod/storage/eviction_test.go | 148 +++++------ pkg/registry/core/pod/storage/storage.go | 2 +- pkg/registry/core/rest/storage_core.go | 12 +- .../policy/poddisruptionbudget/strategy.go | 4 +- .../poddisruptionbudget/strategy_test.go | 62 +++++ pkg/registry/policy/rest/storage_policy.go | 22 ++ .../defaultpreemption/default_preemption.go | 6 +- .../default_preemption_test.go | 2 +- staging/src/k8s.io/api/policy/v1/doc.go | 24 ++ staging/src/k8s.io/api/policy/v1/register.go | 51 ++++ staging/src/k8s.io/api/policy/v1/types.go | 150 +++++++++++ .../src/k8s.io/api/policy/v1beta1/types.go | 14 +- staging/src/k8s.io/api/roundtrip_test.go | 2 + .../v1/poddisruptionbudget_expansion.go | 70 +++++ .../v1beta1/poddisruptionbudget_expansion.go | 4 - .../apps/poddisruptionbudget/helpers.go | 2 +- .../src/k8s.io/kubectl/pkg/scheme/install.go | 3 +- .../admissionwebhook/admission_test.go | 6 +- .../apiserver/apply/status_test.go | 1 + .../integration/disruption/disruption_test.go | 242 ++++++++++++++++-- test/integration/etcd/data.go | 8 + test/integration/evictions/evictions_test.go | 24 +- test/integration/scheduler/util.go | 2 +- 41 files changed, 1306 insertions(+), 174 deletions(-) create mode 100644 pkg/apis/policy/helper.go create mode 100644 pkg/apis/policy/v1/conversion.go create mode 100644 pkg/apis/policy/v1/conversion_test.go create mode 100644 pkg/apis/policy/v1/doc.go create mode 100644 pkg/apis/policy/v1/register.go create mode 100644 pkg/apis/policy/v1beta1/conversion.go create mode 100644 pkg/apis/policy/v1beta1/conversion_test.go create mode 100644 staging/src/k8s.io/api/policy/v1/doc.go create mode 100644 staging/src/k8s.io/api/policy/v1/register.go create mode 100644 staging/src/k8s.io/api/policy/v1/types.go create mode 100644 staging/src/k8s.io/client-go/listers/policy/v1/poddisruptionbudget_expansion.go diff --git a/cmd/kube-apiserver/app/aggregator.go b/cmd/kube-apiserver/app/aggregator.go index c674470945b..457cefd027c 100644 --- a/cmd/kube-apiserver/app/aggregator.go +++ b/cmd/kube-apiserver/app/aggregator.go @@ -262,6 +262,7 @@ var apiVersionPriorities = map[schema.GroupVersion]priority{ {Group: "networking.k8s.io", Version: "v1"}: {group: 17200, version: 15}, {Group: "networking.k8s.io", Version: "v1beta1"}: {group: 17200, version: 9}, {Group: "extensions", Version: "v1beta1"}: {group: 17150, version: 1}, // prioritize below networking.k8s.io, which contains the GA version of Ingress, the only resource remaining in extensions/v1beta1 + {Group: "policy", Version: "v1"}: {group: 17100, version: 15}, {Group: "policy", Version: "v1beta1"}: {group: 17100, version: 9}, {Group: "rbac.authorization.k8s.io", Version: "v1"}: {group: 17000, version: 15}, {Group: "rbac.authorization.k8s.io", Version: "v1beta1"}: {group: 17000, version: 12}, diff --git a/cmd/kube-controller-manager/app/policy.go b/cmd/kube-controller-manager/app/policy.go index 30acb48ddf0..ed471902452 100644 --- a/cmd/kube-controller-manager/app/policy.go +++ b/cmd/kube-controller-manager/app/policy.go @@ -25,7 +25,6 @@ import ( "k8s.io/klog/v2" - "k8s.io/apimachinery/pkg/runtime/schema" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/dynamic" "k8s.io/client-go/scale" @@ -34,16 +33,6 @@ import ( ) func startDisruptionController(ctx ControllerContext) (http.Handler, bool, error) { - var group = "policy" - var version = "v1beta1" - var resource = "poddisruptionbudgets" - - if !ctx.AvailableResources[schema.GroupVersionResource{Group: group, Version: version, Resource: resource}] { - klog.Infof( - "Refusing to start disruption because resource %q in group %q is not available.", - resource, group+"/"+version) - return nil, false, nil - } if !utilfeature.DefaultFeatureGate.Enabled(kubefeatures.PodDisruptionBudget) { klog.Infof("Refusing to start disruption because the PodDisruptionBudget feature is disabled") return nil, false, nil @@ -59,7 +48,7 @@ func startDisruptionController(ctx ControllerContext) (http.Handler, bool, error go disruption.NewDisruptionController( ctx.InformerFactory.Core().V1().Pods(), - ctx.InformerFactory.Policy().V1beta1().PodDisruptionBudgets(), + ctx.InformerFactory.Policy().V1().PodDisruptionBudgets(), ctx.InformerFactory.Core().V1().ReplicationControllers(), ctx.InformerFactory.Apps().V1().ReplicaSets(), ctx.InformerFactory.Apps().V1().Deployments(), diff --git a/hack/.import-aliases b/hack/.import-aliases index 66b2b98d959..600d65d241c 100644 --- a/hack/.import-aliases +++ b/hack/.import-aliases @@ -28,6 +28,7 @@ "k8s.io/api/node/v1alpha1": "nodev1alpha1", "k8s.io/api/node/v1beta1": "nodev1beta1", "k8s.io/api/node/v1": "nodev1", + "k8s.io/api/policy/v1": "policyv1", "k8s.io/api/policy/v1beta1": "policyv1beta1", "k8s.io/api/rbac/v1": "rbacv1", "k8s.io/api/rbac/v1alpha1": "rbacv1alpha1", diff --git a/hack/lib/init.sh b/hack/lib/init.sh index f85e4e9523a..6c601b5662b 100755 --- a/hack/lib/init.sh +++ b/hack/lib/init.sh @@ -95,6 +95,7 @@ networking.k8s.io/v1beta1 \ node.k8s.io/v1 \ node.k8s.io/v1alpha1 \ node.k8s.io/v1beta1 \ +policy/v1 \ policy/v1beta1 \ rbac.authorization.k8s.io/v1 \ rbac.authorization.k8s.io/v1beta1 \ diff --git a/pkg/apis/policy/helper.go b/pkg/apis/policy/helper.go new file mode 100644 index 00000000000..4dd1658abc7 --- /dev/null +++ b/pkg/apis/policy/helper.go @@ -0,0 +1,51 @@ +/* +Copyright 2021 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 policy + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +const ( + PDBV1beta1Label = "pdb.kubernetes.io/deprecated-v1beta1-empty-selector-match" +) + +var ( + NonV1beta1MatchAllSelector = &metav1.LabelSelector{} + NonV1beta1MatchNoneSelector = &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{{Key: PDBV1beta1Label, Operator: metav1.LabelSelectorOpExists}}, + } + + V1beta1MatchNoneSelector = &metav1.LabelSelector{} + V1beta1MatchAllSelector = &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{{Key: PDBV1beta1Label, Operator: metav1.LabelSelectorOpDoesNotExist}}, + } +) + +func StripPDBV1beta1Label(selector *metav1.LabelSelector) { + if selector == nil { + return + } + + trimmedMatchExpressions := selector.MatchExpressions[:0] + for _, exp := range selector.MatchExpressions { + if exp.Key != PDBV1beta1Label { + trimmedMatchExpressions = append(trimmedMatchExpressions, exp) + } + } + selector.MatchExpressions = trimmedMatchExpressions +} diff --git a/pkg/apis/policy/install/install.go b/pkg/apis/policy/install/install.go index 0d91720e4d8..b3e2475e910 100644 --- a/pkg/apis/policy/install/install.go +++ b/pkg/apis/policy/install/install.go @@ -23,6 +23,7 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/apis/policy" + "k8s.io/kubernetes/pkg/apis/policy/v1" "k8s.io/kubernetes/pkg/apis/policy/v1beta1" ) @@ -34,5 +35,7 @@ func init() { func Install(scheme *runtime.Scheme) { utilruntime.Must(policy.AddToScheme(scheme)) utilruntime.Must(v1beta1.AddToScheme(scheme)) - utilruntime.Must(scheme.SetVersionPriority(v1beta1.SchemeGroupVersion)) + utilruntime.Must(v1.AddToScheme(scheme)) + // TODO (mortent): priority should change after 1.21. See https://github.com/kubernetes/kubernetes/pull/95718#discussion_r520969477 + utilruntime.Must(scheme.SetVersionPriority(v1beta1.SchemeGroupVersion, v1.SchemeGroupVersion)) } diff --git a/pkg/apis/policy/v1/conversion.go b/pkg/apis/policy/v1/conversion.go new file mode 100644 index 00000000000..d3aedbd634d --- /dev/null +++ b/pkg/apis/policy/v1/conversion.go @@ -0,0 +1,42 @@ +/* +Copyright 2021 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 v1 + +import ( + "k8s.io/api/policy/v1" + apiequality "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/conversion" + "k8s.io/kubernetes/pkg/apis/policy" +) + +func Convert_v1_PodDisruptionBudget_To_policy_PodDisruptionBudget(in *v1.PodDisruptionBudget, out *policy.PodDisruptionBudget, s conversion.Scope) error { + if err := autoConvert_v1_PodDisruptionBudget_To_policy_PodDisruptionBudget(in, out, s); err != nil { + return err + } + + switch { + case apiequality.Semantic.DeepEqual(in.Spec.Selector, policy.NonV1beta1MatchNoneSelector): + // no-op, preserve + case apiequality.Semantic.DeepEqual(in.Spec.Selector, policy.NonV1beta1MatchAllSelector): + // no-op, preserve + default: + // otherwise, make sure the label intended to be used in a match-all or match-none selector + // never gets combined with user-specified fields + policy.StripPDBV1beta1Label(out.Spec.Selector) + } + return nil +} diff --git a/pkg/apis/policy/v1/conversion_test.go b/pkg/apis/policy/v1/conversion_test.go new file mode 100644 index 00000000000..7cde200a0af --- /dev/null +++ b/pkg/apis/policy/v1/conversion_test.go @@ -0,0 +1,127 @@ +/* +Copyright 2021 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 v1 + +import ( + "reflect" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "k8s.io/api/policy/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/kubernetes/pkg/apis/policy" +) + +func TestConversion(t *testing.T) { + testcases := []struct { + Name string + In runtime.Object + Out runtime.Object + ExpectOut runtime.Object + ExpectErr string + }{ + { + Name: "v1 to internal with match none selector", + In: &v1.PodDisruptionBudget{ + Spec: v1.PodDisruptionBudgetSpec{ + Selector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "pdb.kubernetes.io/deprecated-v1beta1-empty-selector-match", + Operator: metav1.LabelSelectorOpExists, + }, + }, + }, + }, + }, + Out: &policy.PodDisruptionBudget{}, + ExpectOut: &policy.PodDisruptionBudget{ + Spec: policy.PodDisruptionBudgetSpec{ + Selector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "pdb.kubernetes.io/deprecated-v1beta1-empty-selector-match", + Operator: metav1.LabelSelectorOpExists, + }, + }, + }, + }, + }, + }, + { + Name: "v1 to internal with nil selector", + In: &v1.PodDisruptionBudget{}, + Out: &policy.PodDisruptionBudget{}, + ExpectOut: &policy.PodDisruptionBudget{}, + }, + { + Name: "v1 to internal with match all selector", + In: &v1.PodDisruptionBudget{ + Spec: v1.PodDisruptionBudgetSpec{ + Selector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "pdb.kubernetes.io/deprecated-v1beta1-empty-selector-match", + Operator: metav1.LabelSelectorOpDoesNotExist, + }, + }, + }, + }, + }, + Out: &policy.PodDisruptionBudget{}, + ExpectOut: &policy.PodDisruptionBudget{ + Spec: policy.PodDisruptionBudgetSpec{ + Selector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{}, + }, + }, + }, + }, + } + + scheme := runtime.NewScheme() + if err := policy.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + + if err := AddToScheme(scheme); err != nil { + t.Fatal(err) + } + + for _, tc := range testcases { + t.Run(tc.Name, func(t *testing.T) { + err := scheme.Convert(tc.In, tc.Out, nil) + if err != nil { + if len(tc.ExpectErr) == 0 { + t.Fatalf("unexpected error %v", err) + } + if !strings.Contains(err.Error(), tc.ExpectErr) { + t.Fatalf("expected error %s, got %v", tc.ExpectErr, err) + } + return + } + if len(tc.ExpectErr) > 0 { + t.Fatalf("expected error %s, got none", tc.ExpectErr) + } + if !reflect.DeepEqual(tc.Out, tc.ExpectOut) { + t.Fatalf("unexpected result:\n %s", cmp.Diff(tc.ExpectOut, tc.Out)) + } + }) + } +} diff --git a/pkg/apis/policy/v1/doc.go b/pkg/apis/policy/v1/doc.go new file mode 100644 index 00000000000..f5d07a38d53 --- /dev/null +++ b/pkg/apis/policy/v1/doc.go @@ -0,0 +1,24 @@ +/* +Copyright 2021 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. +*/ + +// +k8s:conversion-gen=k8s.io/kubernetes/pkg/apis/policy +// +k8s:conversion-gen-external-types=k8s.io/api/policy/v1 +// +k8s:defaulter-gen=TypeMeta +// +k8s:defaulter-gen-input=../../../../vendor/k8s.io/api/policy/v1 + +// Package policy is for any kind of policy object. Currently, this only +// includes policyv1.PodDisruptionBudget +package v1 // import "k8s.io/kubernetes/pkg/apis/policy/v1" diff --git a/pkg/apis/policy/v1/register.go b/pkg/apis/policy/v1/register.go new file mode 100644 index 00000000000..bd3f8f379a2 --- /dev/null +++ b/pkg/apis/policy/v1/register.go @@ -0,0 +1,45 @@ +/* +Copyright 2021 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 v1 + +import ( + policyv1 "k8s.io/api/policy/v1" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +// GroupName is the group name use in this package +const GroupName = "policy" + +// SchemeGroupVersion is group version used to register these objects +var SchemeGroupVersion = schema.GroupVersion{Group: GroupName, Version: "v1"} + +// Resource takes an unqualified resource and returns a Group qualified GroupResource +func Resource(resource string) schema.GroupResource { + return SchemeGroupVersion.WithResource(resource).GroupResource() +} + +var ( + localSchemeBuilder = &policyv1.SchemeBuilder + AddToScheme = localSchemeBuilder.AddToScheme +) + +func init() { + // We only register manually written functions here. The registration of the + // generated functions takes place in the generated files. The separation + // makes the code compile even when the generated files are missing. + localSchemeBuilder.Register(RegisterDefaults) +} diff --git a/pkg/apis/policy/v1beta1/conversion.go b/pkg/apis/policy/v1beta1/conversion.go new file mode 100644 index 00000000000..26638f978a9 --- /dev/null +++ b/pkg/apis/policy/v1beta1/conversion.go @@ -0,0 +1,66 @@ +/* +Copyright 2021 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 v1beta1 + +import ( + "k8s.io/api/policy/v1beta1" + apiequality "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/conversion" + "k8s.io/kubernetes/pkg/apis/policy" +) + +func Convert_v1beta1_PodDisruptionBudget_To_policy_PodDisruptionBudget(in *v1beta1.PodDisruptionBudget, out *policy.PodDisruptionBudget, s conversion.Scope) error { + if err := autoConvert_v1beta1_PodDisruptionBudget_To_policy_PodDisruptionBudget(in, out, s); err != nil { + return err + } + + switch { + case apiequality.Semantic.DeepEqual(in.Spec.Selector, policy.V1beta1MatchNoneSelector): + // If the v1beta1 version has a non-nil but empty selector, it should be + // selecting no pods, even when used with the internal or v1 api. We + // add a selector that is non-empty but will never match any pods. + out.Spec.Selector = policy.NonV1beta1MatchNoneSelector.DeepCopy() + case apiequality.Semantic.DeepEqual(in.Spec.Selector, policy.V1beta1MatchAllSelector): + // If the v1beta1 version has our v1beta1-specific "match-all" selector, + // swap that out for a simpler empty "match-all" selector for v1 + out.Spec.Selector = policy.NonV1beta1MatchAllSelector.DeepCopy() + default: + // otherwise, make sure the label intended to be used in a match-all or match-none selector + // never gets combined with user-specified fields + policy.StripPDBV1beta1Label(out.Spec.Selector) + } + return nil +} + +func Convert_policy_PodDisruptionBudget_To_v1beta1_PodDisruptionBudget(in *policy.PodDisruptionBudget, out *v1beta1.PodDisruptionBudget, s conversion.Scope) error { + if err := autoConvert_policy_PodDisruptionBudget_To_v1beta1_PodDisruptionBudget(in, out, s); err != nil { + return err + } + + switch { + case apiequality.Semantic.DeepEqual(in.Spec.Selector, policy.NonV1beta1MatchNoneSelector): + // If the internal version has our v1beta1-specific "match-none" selector, + // swap that out for a simpler empty "match-none" selector for v1beta1 + out.Spec.Selector = policy.V1beta1MatchNoneSelector.DeepCopy() + case apiequality.Semantic.DeepEqual(in.Spec.Selector, policy.NonV1beta1MatchAllSelector): + // If the internal version has a non-nil but empty selector, we want it to + // select all pods. We make sure this happens even with the v1beta1 api by + // adding a non-empty selector that selects all pods. + out.Spec.Selector = policy.V1beta1MatchAllSelector.DeepCopy() + } + return nil +} \ No newline at end of file diff --git a/pkg/apis/policy/v1beta1/conversion_test.go b/pkg/apis/policy/v1beta1/conversion_test.go new file mode 100644 index 00000000000..6a40a75a899 --- /dev/null +++ b/pkg/apis/policy/v1beta1/conversion_test.go @@ -0,0 +1,197 @@ +/* +Copyright 2021 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 v1beta1 + +import ( + "reflect" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "k8s.io/api/policy/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/kubernetes/pkg/apis/policy" +) + +func TestConversion(t *testing.T) { + testcases := []struct { + Name string + In runtime.Object + Out runtime.Object + ExpectOut runtime.Object + ExpectErr string + }{ + { + Name: "v1beta1 to internal with empty selector", + In: &v1beta1.PodDisruptionBudget{ + Spec: v1beta1.PodDisruptionBudgetSpec{ + Selector: &metav1.LabelSelector{}, + }, + }, + Out: &policy.PodDisruptionBudget{}, + ExpectOut: &policy.PodDisruptionBudget{ + Spec: policy.PodDisruptionBudgetSpec{ + Selector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "pdb.kubernetes.io/deprecated-v1beta1-empty-selector-match", + Operator: metav1.LabelSelectorOpExists, + }, + }, + }, + }, + }, + }, + { + Name: "v1beta1 to internal with nil selector", + In: &v1beta1.PodDisruptionBudget{}, + Out: &policy.PodDisruptionBudget{}, + ExpectOut: &policy.PodDisruptionBudget{}, + }, + { + Name: "v1 to internal with existing selector", + In: &v1beta1.PodDisruptionBudget{ + Spec: v1beta1.PodDisruptionBudgetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "foo": "bar", + }, + }, + }, + }, + Out: &policy.PodDisruptionBudget{}, + ExpectOut: &policy.PodDisruptionBudget{ + Spec: policy.PodDisruptionBudgetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "foo": "bar", + }, + }, + }, + }, + }, + { + Name: "v1beta1 to internal with existing pdb selector", + In: &v1beta1.PodDisruptionBudget{ + Spec: v1beta1.PodDisruptionBudgetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "foo": "bar", + }, + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "pdb.kubernetes.io/deprecated-v1beta1-empty-selector-match", + Operator: metav1.LabelSelectorOpDoesNotExist, + }, + }, + }, + }, + }, + Out: &policy.PodDisruptionBudget{}, + ExpectOut: &policy.PodDisruptionBudget{ + Spec: policy.PodDisruptionBudgetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "foo": "bar", + }, + MatchExpressions: []metav1.LabelSelectorRequirement{}, + }, + }, + }, + }, + { + Name: "internal to v1beta1 with empty selector", + In: &policy.PodDisruptionBudget{ + Spec: policy.PodDisruptionBudgetSpec{ + Selector: &metav1.LabelSelector{}, + }, + }, + Out: &v1beta1.PodDisruptionBudget{}, + ExpectOut: &v1beta1.PodDisruptionBudget{ + Spec: v1beta1.PodDisruptionBudgetSpec{ + Selector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "pdb.kubernetes.io/deprecated-v1beta1-empty-selector-match", + Operator: metav1.LabelSelectorOpDoesNotExist, + }, + }, + }, + }, + }, + }, + { + Name: "internal to v1beta1 with nil selector", + In: &policy.PodDisruptionBudget{}, + Out: &v1beta1.PodDisruptionBudget{}, + ExpectOut: &v1beta1.PodDisruptionBudget{}, + }, + { + Name: "internal to v1beta1 with existing selector", + In: &policy.PodDisruptionBudget{ + Spec: policy.PodDisruptionBudgetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "foo": "bar", + }, + }, + }, + }, + Out: &v1beta1.PodDisruptionBudget{}, + ExpectOut: &v1beta1.PodDisruptionBudget{ + Spec: v1beta1.PodDisruptionBudgetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "foo": "bar", + }, + }, + }, + }, + }, + } + + scheme := runtime.NewScheme() + if err := policy.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + + if err := AddToScheme(scheme); err != nil { + t.Fatal(err) + } + + for _, tc := range testcases { + t.Run(tc.Name, func(t *testing.T) { + err := scheme.Convert(tc.In, tc.Out, nil) + if err != nil { + if len(tc.ExpectErr) == 0 { + t.Fatalf("unexpected error %v", err) + } + if !strings.Contains(err.Error(), tc.ExpectErr) { + t.Fatalf("expected error %s, got %v", tc.ExpectErr, err) + } + return + } + if len(tc.ExpectErr) > 0 { + t.Fatalf("expected error %s, got none", tc.ExpectErr) + } + if !reflect.DeepEqual(tc.Out, tc.ExpectOut) { + t.Fatalf("unexpected result:\n %s", cmp.Diff(tc.ExpectOut, tc.Out)) + } + }) + } +} \ No newline at end of file diff --git a/pkg/controller/disruption/disruption.go b/pkg/controller/disruption/disruption.go index 2b7abf2780d..24266da425c 100644 --- a/pkg/controller/disruption/disruption.go +++ b/pkg/controller/disruption/disruption.go @@ -25,7 +25,7 @@ import ( apps "k8s.io/api/apps/v1beta1" v1 "k8s.io/api/core/v1" "k8s.io/api/extensions/v1beta1" - policy "k8s.io/api/policy/v1beta1" + policy "k8s.io/api/policy/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" @@ -38,13 +38,13 @@ import ( "k8s.io/client-go/discovery" appsv1informers "k8s.io/client-go/informers/apps/v1" coreinformers "k8s.io/client-go/informers/core/v1" - policyinformers "k8s.io/client-go/informers/policy/v1beta1" + policyinformers "k8s.io/client-go/informers/policy/v1" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" v1core "k8s.io/client-go/kubernetes/typed/core/v1" appsv1listers "k8s.io/client-go/listers/apps/v1" corelisters "k8s.io/client-go/listers/core/v1" - policylisters "k8s.io/client-go/listers/policy/v1beta1" + policylisters "k8s.io/client-go/listers/policy/v1" scaleclient "k8s.io/client-go/scale" "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" @@ -506,9 +506,6 @@ func (dc *DisruptionController) getPodsForPdb(pdb *policy.PodDisruptionBudget) ( if err != nil { return []*v1.Pod{}, err } - if sel.Empty() { - return []*v1.Pod{}, nil - } pods, err := dc.podLister.Pods(pdb.Namespace).List(sel) if err != nil { return []*v1.Pod{}, err @@ -835,6 +832,6 @@ func (dc *DisruptionController) updatePdbStatus(pdb *policy.PodDisruptionBudget, func (dc *DisruptionController) writePdbStatus(pdb *policy.PodDisruptionBudget) error { // If this update fails, don't retry it. Allow the failure to get handled & // retried in `processNextWorkItem()`. - _, err := dc.kubeClient.PolicyV1beta1().PodDisruptionBudgets(pdb.Namespace).UpdateStatus(context.TODO(), pdb, metav1.UpdateOptions{}) + _, err := dc.kubeClient.PolicyV1().PodDisruptionBudgets(pdb.Namespace).UpdateStatus(context.TODO(), pdb, metav1.UpdateOptions{}) return err } diff --git a/pkg/controller/disruption/disruption_test.go b/pkg/controller/disruption/disruption_test.go index 8e47266964b..d542bef235f 100644 --- a/pkg/controller/disruption/disruption_test.go +++ b/pkg/controller/disruption/disruption_test.go @@ -29,7 +29,7 @@ import ( apps "k8s.io/api/apps/v1" autoscalingapi "k8s.io/api/autoscaling/v1" v1 "k8s.io/api/core/v1" - policy "k8s.io/api/policy/v1beta1" + policy "k8s.io/api/policy/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" @@ -151,7 +151,7 @@ func newFakeDisruptionController() (*disruptionController, *pdbStates) { dc := NewDisruptionController( informerFactory.Core().V1().Pods(), - informerFactory.Policy().V1beta1().PodDisruptionBudgets(), + informerFactory.Policy().V1().PodDisruptionBudgets(), informerFactory.Core().V1().ReplicationControllers(), informerFactory.Apps().V1().ReplicaSets(), informerFactory.Apps().V1().Deployments(), @@ -175,7 +175,7 @@ func newFakeDisruptionController() (*disruptionController, *pdbStates) { return &disruptionController{ dc, informerFactory.Core().V1().Pods().Informer().GetStore(), - informerFactory.Policy().V1beta1().PodDisruptionBudgets().Informer().GetStore(), + informerFactory.Policy().V1().PodDisruptionBudgets().Informer().GetStore(), informerFactory.Core().V1().ReplicationControllers().Informer().GetStore(), informerFactory.Apps().V1().ReplicaSets().Informer().GetStore(), informerFactory.Apps().V1().Deployments().Informer().GetStore(), @@ -402,7 +402,7 @@ func add(t *testing.T, store cache.Store, obj interface{}) { } } -// Create one with no selector. Verify it matches 0 pods. +// Create one with no selector. Verify it matches all pods func TestNoSelector(t *testing.T) { dc, ps := newFakeDisruptionController() @@ -416,7 +416,7 @@ func TestNoSelector(t *testing.T) { add(t, dc.podStore, pod) dc.sync(pdbName) - ps.VerifyPdbStatus(t, pdbName, 0, 0, 3, 0, map[string]metav1.Time{}) + ps.VerifyPdbStatus(t, pdbName, 0, 1, 3, 1, map[string]metav1.Time{}) } // Verify that available/expected counts go up as we add pods, then verify that @@ -1151,7 +1151,7 @@ func TestUpdatePDBStatusRetries(t *testing.T) { // Create a PDB and 3 pods that match it. pdb, pdbKey := newMinAvailablePodDisruptionBudget(t, intstr.FromInt(1)) - pdb, err := dc.coreClient.PolicyV1beta1().PodDisruptionBudgets(pdb.Namespace).Create(context.TODO(), pdb, metav1.CreateOptions{}) + pdb, err := dc.coreClient.PolicyV1().PodDisruptionBudgets(pdb.Namespace).Create(context.TODO(), pdb, metav1.CreateOptions{}) if err != nil { t.Fatalf("Failed to create PDB: %v", err) } @@ -1186,7 +1186,7 @@ func TestUpdatePDBStatusRetries(t *testing.T) { // These GVRs are copied from the generated fake code because they are not exported. var ( podsResource = schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"} - poddisruptionbudgetsResource = schema.GroupVersionResource{Group: "policy", Version: "v1beta1", Resource: "poddisruptionbudgets"} + poddisruptionbudgetsResource = schema.GroupVersionResource{Group: "policy", Version: "v1", Resource: "poddisruptionbudgets"} ) // Bypass the coreClient.Fake and write directly to the ObjectTracker, because @@ -1248,7 +1248,7 @@ func TestUpdatePDBStatusRetries(t *testing.T) { // (C) Whether or not sync() returned an error, the PDB status should reflect // the evictions that took place. - finalPDB, err := dc.coreClient.PolicyV1beta1().PodDisruptionBudgets("default").Get(context.TODO(), pdb.Name, metav1.GetOptions{}) + finalPDB, err := dc.coreClient.PolicyV1().PodDisruptionBudgets("default").Get(context.TODO(), pdb.Name, metav1.GetOptions{}) if err != nil { t.Fatalf("Failed to get PDB: %v", err) } diff --git a/pkg/controlplane/instance.go b/pkg/controlplane/instance.go index 45951dfe683..9bba2ebd51b 100644 --- a/pkg/controlplane/instance.go +++ b/pkg/controlplane/instance.go @@ -54,6 +54,7 @@ import ( nodev1 "k8s.io/api/node/v1" nodev1alpha1 "k8s.io/api/node/v1alpha1" nodev1beta1 "k8s.io/api/node/v1beta1" + policyapiv1 "k8s.io/api/policy/v1" policyapiv1beta1 "k8s.io/api/policy/v1beta1" rbacv1 "k8s.io/api/rbac/v1" rbacv1alpha1 "k8s.io/api/rbac/v1alpha1" @@ -688,6 +689,7 @@ func DefaultAPIResourceConfigSource() *serverstorage.ResourceConfig { networkingapiv1beta1.SchemeGroupVersion, nodev1.SchemeGroupVersion, nodev1beta1.SchemeGroupVersion, + policyapiv1.SchemeGroupVersion, policyapiv1beta1.SchemeGroupVersion, rbacv1.SchemeGroupVersion, rbacv1beta1.SchemeGroupVersion, diff --git a/pkg/controlplane/storageversionhashdata/data.go b/pkg/controlplane/storageversionhashdata/data.go index cfe1f14cf0b..cf817a1ab01 100644 --- a/pkg/controlplane/storageversionhashdata/data.go +++ b/pkg/controlplane/storageversionhashdata/data.go @@ -77,6 +77,7 @@ var GVRToStorageVersionHash = map[string]string{ "networking.k8s.io/v1/ingressclasses": "l/iqIbDgFyQ=", "node.k8s.io/v1/runtimeclasses": "WQTu1GL3T2Q=", "node.k8s.io/v1beta1/runtimeclasses": "WQTu1GL3T2Q=", + "policy/v1/poddisruptionbudgets": "6BGBu0kpHtk=", "policy/v1beta1/poddisruptionbudgets": "6BGBu0kpHtk=", "policy/v1beta1/podsecuritypolicies": "khBLobUXkqA=", "rbac.authorization.k8s.io/v1/clusterrolebindings": "48tpQ8gZHFc=", diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 5b2ce6fa433..6029abdc504 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -805,7 +805,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS WindowsEndpointSliceProxying: {Default: true, PreRelease: featuregate.Beta}, StartupProbe: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.23 AllowInsecureBackendProxy: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.23 - PodDisruptionBudget: {Default: true, PreRelease: featuregate.Beta}, + PodDisruptionBudget: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.25 CronJobControllerV2: {Default: true, PreRelease: featuregate.Beta}, DaemonSetUpdateSurge: {Default: false, PreRelease: featuregate.Alpha}, ServiceTopology: {Default: false, PreRelease: featuregate.Alpha}, diff --git a/pkg/registry/core/pod/storage/eviction.go b/pkg/registry/core/pod/storage/eviction.go index ec1899b614d..a4aa690cf8e 100644 --- a/pkg/registry/core/pod/storage/eviction.go +++ b/pkg/registry/core/pod/storage/eviction.go @@ -22,7 +22,7 @@ import ( "reflect" "time" - policyv1beta1 "k8s.io/api/policy/v1beta1" + policyv1 "k8s.io/api/policy/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -31,7 +31,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/registry/rest" "k8s.io/apiserver/pkg/util/dryrun" - policyclient "k8s.io/client-go/kubernetes/typed/policy/v1beta1" + policyclient "k8s.io/client-go/kubernetes/typed/policy/v1" "k8s.io/client-go/util/retry" pdbhelper "k8s.io/component-helpers/apps/poddisruptionbudget" podutil "k8s.io/kubernetes/pkg/api/pod" @@ -314,7 +314,7 @@ func createTooManyRequestsError(name string) error { } // checkAndDecrement checks if the provided PodDisruptionBudget allows any disruption. -func (r *EvictionREST) checkAndDecrement(namespace string, podName string, pdb policyv1beta1.PodDisruptionBudget, dryRun bool) error { +func (r *EvictionREST) checkAndDecrement(namespace string, podName string, pdb policyv1.PodDisruptionBudget, dryRun bool) error { if pdb.Status.ObservedGeneration < pdb.Generation { return createTooManyRequestsError(pdb.Name) @@ -358,17 +358,13 @@ func (r *EvictionREST) checkAndDecrement(namespace string, podName string, pdb p } // getPodDisruptionBudgets returns any PDBs that match the pod or err if there's an error. -func (r *EvictionREST) getPodDisruptionBudgets(ctx context.Context, pod *api.Pod) ([]policyv1beta1.PodDisruptionBudget, error) { - if len(pod.Labels) == 0 { - return nil, nil - } - +func (r *EvictionREST) getPodDisruptionBudgets(ctx context.Context, pod *api.Pod) ([]policyv1.PodDisruptionBudget, error) { pdbList, err := r.podDisruptionBudgetClient.PodDisruptionBudgets(pod.Namespace).List(context.TODO(), metav1.ListOptions{}) if err != nil { return nil, err } - var pdbs []policyv1beta1.PodDisruptionBudget + var pdbs []policyv1.PodDisruptionBudget for _, pdb := range pdbList.Items { if pdb.Namespace != pod.Namespace { continue diff --git a/pkg/registry/core/pod/storage/eviction_test.go b/pkg/registry/core/pod/storage/eviction_test.go index b15cea5daee..e1d705c4f19 100644 --- a/pkg/registry/core/pod/storage/eviction_test.go +++ b/pkg/registry/core/pod/storage/eviction_test.go @@ -21,7 +21,7 @@ import ( "errors" "testing" - policyv1beta1 "k8s.io/api/policy/v1beta1" + policyv1"k8s.io/api/policy/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" @@ -52,10 +52,10 @@ func TestEviction(t *testing.T) { }{ { name: "matching pdbs with no disruptions allowed, pod running", - pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{ + pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, }}, eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t1", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)}, expectError: true, @@ -64,10 +64,10 @@ func TestEviction(t *testing.T) { }, { name: "matching pdbs with no disruptions allowed, pod pending", - pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{ + pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, }}, eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t2", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)}, expectError: false, @@ -77,10 +77,10 @@ func TestEviction(t *testing.T) { }, { name: "matching pdbs with no disruptions allowed, pod succeeded", - pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{ + pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, }}, eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t3", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)}, expectError: false, @@ -90,10 +90,10 @@ func TestEviction(t *testing.T) { }, { name: "matching pdbs with no disruptions allowed, pod failed", - pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{ + pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, }}, eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t4", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)}, expectError: false, @@ -103,10 +103,10 @@ func TestEviction(t *testing.T) { }, { name: "matching pdbs with disruptions allowed", - pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{ + pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{DisruptionsAllowed: 1}, + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 1}, }}, eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t5", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)}, expectDeleted: true, @@ -114,10 +114,10 @@ func TestEviction(t *testing.T) { }, { name: "non-matching pdbs", - pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{ + pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"b": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"b": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, }}, eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t6", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)}, expectDeleted: true, @@ -125,10 +125,10 @@ func TestEviction(t *testing.T) { }, { name: "matching pdbs with disruptions allowed but bad name in Url", - pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{ + pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{DisruptionsAllowed: 1}, + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 1}, }}, badNameInURL: true, eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t7", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)}, @@ -161,7 +161,7 @@ func TestEviction(t *testing.T) { } client := fake.NewSimpleClientset(tc.pdbs...) - evictionRest := newEvictionStorage(storage.Store, client.PolicyV1beta1()) + evictionRest := newEvictionStorage(storage.Store, client.PolicyV1()) name := pod.Name if tc.badNameInURL { @@ -225,10 +225,10 @@ func TestEvictionIngorePDB(t *testing.T) { }{ { name: "pdbs No disruptions allowed, pod pending, first delete conflict, pod still pending, pod deleted successfully", - pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{ + pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, }}, eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t1", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)}, expectError: false, @@ -241,10 +241,10 @@ func TestEvictionIngorePDB(t *testing.T) { // pod should not be deleted. { name: "pdbs No disruptions allowed, pod pending, first delete conflict, pod becomes running, continueToPDBs", - pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{ + pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, }}, eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t2", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)}, expectError: true, @@ -254,10 +254,10 @@ func TestEvictionIngorePDB(t *testing.T) { }, { name: "pdbs disruptions allowed, pod pending, first delete conflict, pod becomes running, continueToPDBs", - pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{ + pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{DisruptionsAllowed: 1}, + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 1}, }}, eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t3", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)}, expectError: false, @@ -267,10 +267,10 @@ func TestEvictionIngorePDB(t *testing.T) { }, { name: "pod pending, always conflict on delete", - pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{ + pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, }}, eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t4", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)}, expectError: true, @@ -280,10 +280,10 @@ func TestEvictionIngorePDB(t *testing.T) { }, { name: "pod pending, always conflict on delete, user provided ResourceVersion constraint", - pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{ + pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, }}, eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t5", Namespace: "default"}, DeleteOptions: metav1.NewRVDeletionPrecondition("userProvided")}, expectError: true, @@ -293,10 +293,10 @@ func TestEvictionIngorePDB(t *testing.T) { }, { name: "matching pdbs with no disruptions allowed, pod terminating", - pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{ + pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0}, }}, eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t6", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(300)}, expectError: false, @@ -306,10 +306,10 @@ func TestEvictionIngorePDB(t *testing.T) { }, { name: "matching pdbs with no disruptions allowed, pod running, pod healthy, unhealthy pod not ours", - pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{ + pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{ + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{ // This simulates 3 pods desired, our pod healthy, unhealthy pod is not ours. DisruptionsAllowed: 0, CurrentHealthy: 2, @@ -329,10 +329,10 @@ func TestEvictionIngorePDB(t *testing.T) { }, { name: "matching pdbs with no disruptions allowed, pod running, pod unhealthy, unhealthy pod ours", - pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{ + pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{ + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{ // This simulates 3 pods desired, our pod unhealthy DisruptionsAllowed: 0, CurrentHealthy: 2, @@ -353,10 +353,10 @@ func TestEvictionIngorePDB(t *testing.T) { { // This case should return the 529 retry error. name: "matching pdbs with no disruptions allowed, pod running, pod unhealthy, unhealthy pod ours, resource version conflict", - pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{ + pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{ + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{ // This simulates 3 pods desired, our pod unhealthy DisruptionsAllowed: 0, CurrentHealthy: 2, @@ -377,10 +377,10 @@ func TestEvictionIngorePDB(t *testing.T) { { // This case should return the 529 retry error. name: "matching pdbs with no disruptions allowed, pod running, pod unhealthy, unhealthy pod ours, other error on delete", - pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{ + pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{ + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{ // This simulates 3 pods desired, our pod unhealthy DisruptionsAllowed: 0, CurrentHealthy: 2, @@ -428,7 +428,7 @@ func TestEvictionIngorePDB(t *testing.T) { } client := fake.NewSimpleClientset(tc.pdbs...) - evictionRest := newEvictionStorage(ms, client.PolicyV1beta1()) + evictionRest := newEvictionStorage(ms, client.PolicyV1()) name := pod.Name ms.pod = pod @@ -473,10 +473,10 @@ func TestEvictionDryRun(t *testing.T) { name: "with pdbs", evictionOptions: &metav1.DeleteOptions{DryRun: []string{"All"}}, requestOptions: &metav1.CreateOptions{DryRun: []string{"All"}}, - pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{ + pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{DisruptionsAllowed: 1}, + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 1}, }}, }, } @@ -496,7 +496,7 @@ func TestEvictionDryRun(t *testing.T) { } client := fake.NewSimpleClientset(tc.pdbs...) - evictionRest := newEvictionStorage(storage.Store, client.PolicyV1beta1()) + evictionRest := newEvictionStorage(storage.Store, client.PolicyV1()) eviction := &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, DeleteOptions: tc.evictionOptions} _, err := evictionRest.Create(testContext, pod.Name, eviction, nil, tc.requestOptions) if err != nil { @@ -509,47 +509,47 @@ func TestEvictionDryRun(t *testing.T) { func TestEvictionPDBStatus(t *testing.T) { testcases := []struct { name string - pdb *policyv1beta1.PodDisruptionBudget + pdb *policyv1.PodDisruptionBudget expectedDisruptionsAllowed int32 expectedReason string }{ { name: "pdb status is updated after eviction", - pdb: &policyv1beta1.PodDisruptionBudget{ + pdb: &policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{ + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{ DisruptionsAllowed: 1, Conditions: []metav1.Condition{ { - Type: policyv1beta1.DisruptionAllowedCondition, - Reason: policyv1beta1.SufficientPodsReason, + Type: policyv1.DisruptionAllowedCondition, + Reason: policyv1.SufficientPodsReason, Status: metav1.ConditionTrue, }, }, }, }, expectedDisruptionsAllowed: 0, - expectedReason: policyv1beta1.InsufficientPodsReason, + expectedReason: policyv1.InsufficientPodsReason, }, { name: "condition reason is only updated if AllowedDisruptions becomes 0", - pdb: &policyv1beta1.PodDisruptionBudget{ + pdb: &policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, - Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, - Status: policyv1beta1.PodDisruptionBudgetStatus{ + Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1.PodDisruptionBudgetStatus{ DisruptionsAllowed: 3, Conditions: []metav1.Condition{ { - Type: policyv1beta1.DisruptionAllowedCondition, - Reason: policyv1beta1.SufficientPodsReason, + Type: policyv1.DisruptionAllowedCondition, + Reason: policyv1.SufficientPodsReason, Status: metav1.ConditionTrue, }, }, }, }, expectedDisruptionsAllowed: 2, - expectedReason: policyv1beta1.SufficientPodsReason, + expectedReason: policyv1.SufficientPodsReason, }, } @@ -578,14 +578,14 @@ func TestEvictionPDBStatus(t *testing.T) { } } - evictionRest := newEvictionStorage(storage.Store, client.PolicyV1beta1()) + evictionRest := newEvictionStorage(storage.Store, client.PolicyV1()) eviction := &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "foo-1", Namespace: "default"}, DeleteOptions: &metav1.DeleteOptions{}} _, err := evictionRest.Create(testContext, "foo-1", eviction, nil, &metav1.CreateOptions{}) if err != nil { t.Fatalf("Failed to run eviction: %v", err) } - existingPDB, err := client.PolicyV1beta1().PodDisruptionBudgets(metav1.NamespaceDefault).Get(context.TODO(), tc.pdb.Name, metav1.GetOptions{}) + existingPDB, err := client.PolicyV1().PodDisruptionBudgets(metav1.NamespaceDefault).Get(context.TODO(), tc.pdb.Name, metav1.GetOptions{}) if err != nil { t.Errorf("%#v", err) return @@ -595,7 +595,7 @@ func TestEvictionPDBStatus(t *testing.T) { t.Errorf("expected DisruptionsAllowed to be %d, but got %d", want, got) } - cond := apimeta.FindStatusCondition(existingPDB.Status.Conditions, policyv1beta1.DisruptionAllowedCondition) + cond := apimeta.FindStatusCondition(existingPDB.Status.Conditions, policyv1.DisruptionAllowedCondition) if want, got := tc.expectedReason, cond.Reason; want != got { t.Errorf("expected Reason to be %q, but got %q", want, got) } diff --git a/pkg/registry/core/pod/storage/storage.go b/pkg/registry/core/pod/storage/storage.go index e82bb4c5db9..abdde48e2b9 100644 --- a/pkg/registry/core/pod/storage/storage.go +++ b/pkg/registry/core/pod/storage/storage.go @@ -33,7 +33,7 @@ import ( storeerr "k8s.io/apiserver/pkg/storage/errors" "k8s.io/apiserver/pkg/util/dryrun" utilfeature "k8s.io/apiserver/pkg/util/feature" - policyclient "k8s.io/client-go/kubernetes/typed/policy/v1beta1" + policyclient "k8s.io/client-go/kubernetes/typed/policy/v1" podutil "k8s.io/kubernetes/pkg/api/pod" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/validation" diff --git a/pkg/registry/core/rest/storage_core.go b/pkg/registry/core/rest/storage_core.go index 40aed6226c7..f9323413ea2 100644 --- a/pkg/registry/core/rest/storage_core.go +++ b/pkg/registry/core/rest/storage_core.go @@ -35,7 +35,7 @@ import ( serverstorage "k8s.io/apiserver/pkg/server/storage" "k8s.io/apiserver/pkg/storage/etcd3" utilfeature "k8s.io/apiserver/pkg/util/feature" - policyclient "k8s.io/client-go/kubernetes/typed/policy/v1beta1" + policyclient "k8s.io/client-go/kubernetes/typed/policy/v1" restclient "k8s.io/client-go/rest" "k8s.io/kubernetes/pkg/api/legacyscheme" api "k8s.io/kubernetes/pkg/apis/core" @@ -110,13 +110,9 @@ func (c LegacyRESTStorageProvider) NewLegacyRESTStorage(restOptionsGetter generi NegotiatedSerializer: legacyscheme.Codecs, } - var podDisruptionClient policyclient.PodDisruptionBudgetsGetter - if policyGroupVersion := (schema.GroupVersion{Group: "policy", Version: "v1beta1"}); legacyscheme.Scheme.IsVersionRegistered(policyGroupVersion) { - var err error - podDisruptionClient, err = policyclient.NewForConfig(c.LoopbackClientConfig) - if err != nil { - return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, err - } + podDisruptionClient, err := policyclient.NewForConfig(c.LoopbackClientConfig) + if err != nil { + return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, err } restStorage := LegacyRESTStorage{} diff --git a/pkg/registry/policy/poddisruptionbudget/strategy.go b/pkg/registry/policy/poddisruptionbudget/strategy.go index b9ed2bf40c1..66e1cb80beb 100644 --- a/pkg/registry/policy/poddisruptionbudget/strategy.go +++ b/pkg/registry/policy/poddisruptionbudget/strategy.go @@ -114,8 +114,8 @@ func (podDisruptionBudgetStatusStrategy) ValidateUpdate(ctx context.Context, obj var apiVersion schema.GroupVersion if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found { apiVersion = schema.GroupVersion{ - Group: requestInfo.APIVersion, - Version: requestInfo.APIGroup, + Group: requestInfo.APIGroup, + Version: requestInfo.APIVersion, } } return validation.ValidatePodDisruptionBudgetStatusUpdate(obj.(*policy.PodDisruptionBudget).Status, diff --git a/pkg/registry/policy/poddisruptionbudget/strategy_test.go b/pkg/registry/policy/poddisruptionbudget/strategy_test.go index d553780b339..3aef68575c0 100644 --- a/pkg/registry/policy/poddisruptionbudget/strategy_test.go +++ b/pkg/registry/policy/poddisruptionbudget/strategy_test.go @@ -148,3 +148,65 @@ func TestPodDisruptionBudgetStatusStrategy(t *testing.T) { t.Errorf("Unexpected error %v", errs) } } + +func TestPodDisruptionBudgetStatusValidationByApiVersion(t *testing.T) { + testCases := map[string]struct { + apiVersion string + validation bool + }{ + "policy/v1beta1 should not do update validation": { + apiVersion: "v1beta1", + validation: false, + }, + "policy/v1 should do update validation": { + apiVersion: "v1", + validation: true, + }, + "policy/some-version should do update validation": { + apiVersion: "some-version", + validation: true, + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + ctx := genericapirequest.WithRequestInfo(genericapirequest.NewDefaultContext(), + &genericapirequest.RequestInfo{ + APIGroup: "policy", + APIVersion: tc.apiVersion, + }) + + oldMaxUnavailable := intstr.FromInt(2) + newMaxUnavailable := intstr.FromInt(3) + oldPdb := &policy.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault, ResourceVersion: "10"}, + Spec: policy.PodDisruptionBudgetSpec{ + Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "b"}}, + MaxUnavailable: &oldMaxUnavailable, + }, + Status: policy.PodDisruptionBudgetStatus{ + DisruptionsAllowed: 1, + }, + } + newPdb := &policy.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault, ResourceVersion: "9"}, + Spec: policy.PodDisruptionBudgetSpec{ + Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "b"}}, + MinAvailable: &newMaxUnavailable, + }, + Status: policy.PodDisruptionBudgetStatus{ + DisruptionsAllowed: -1, // This is not allowed, so should trigger validation error. + }, + } + + errs := StatusStrategy.ValidateUpdate(ctx, newPdb, oldPdb) + hasErrors := len(errs) > 0 + if !tc.validation && hasErrors { + t.Errorf("Validation failed when no validation should happen") + } + if tc.validation && !hasErrors { + t.Errorf("Expected validation errors but didn't get any") + } + }) + } +} diff --git a/pkg/registry/policy/rest/storage_policy.go b/pkg/registry/policy/rest/storage_policy.go index f71b523e4ee..a3c34079608 100644 --- a/pkg/registry/policy/rest/storage_policy.go +++ b/pkg/registry/policy/rest/storage_policy.go @@ -17,6 +17,7 @@ limitations under the License. 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" @@ -42,6 +43,14 @@ func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorag apiGroupInfo.VersionedResourcesStorageMap[policyapiv1beta1.SchemeGroupVersion.Version] = storageMap } } + + if apiResourceConfigSource.VersionEnabled(policyapiv1.SchemeGroupVersion) { + if storageMap, err := p.v1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[policyapiv1.SchemeGroupVersion.Version] = storageMap + } + } return apiGroupInfo, true, nil } @@ -64,6 +73,19 @@ func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorag return storage, err } +func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { + storage := map[string]rest.Storage{} + + poddisruptionbudgetStorage, poddisruptionbudgetStatusStorage, err := poddisruptionbudgetstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } + storage["poddisruptionbudgets"] = poddisruptionbudgetStorage + storage["poddisruptionbudgets/status"] = poddisruptionbudgetStatusStorage + + return storage, err +} + func (p RESTStorageProvider) GroupName() string { return policy.GroupName } diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go index 6437e09883c..7b34162ca4f 100644 --- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go +++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go @@ -29,7 +29,7 @@ import ( "k8s.io/klog/v2" v1 "k8s.io/api/core/v1" - policy "k8s.io/api/policy/v1beta1" + policy "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -37,7 +37,7 @@ import ( "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" corelisters "k8s.io/client-go/listers/core/v1" - policylisters "k8s.io/client-go/listers/policy/v1beta1" + policylisters "k8s.io/client-go/listers/policy/v1" corev1helpers "k8s.io/component-helpers/scheduling/corev1" extenderv1 "k8s.io/kube-scheduler/extender/v1" kubefeatures "k8s.io/kubernetes/pkg/features" @@ -799,7 +799,7 @@ func filterPodsWithPDBViolation(podInfos []*framework.PodInfo, pdbs []*policy.Po func getPDBLister(informerFactory informers.SharedInformerFactory) policylisters.PodDisruptionBudgetLister { if utilfeature.DefaultFeatureGate.Enabled(kubefeatures.PodDisruptionBudget) { - return informerFactory.Policy().V1beta1().PodDisruptionBudgets().Lister() + return informerFactory.Policy().V1().PodDisruptionBudgets().Lister() } return nil } diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go index 9952c315fb5..9e5f0fbe0a3 100644 --- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go +++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go @@ -27,7 +27,7 @@ import ( "github.com/google/go-cmp/cmp" v1 "k8s.io/api/core/v1" - policy "k8s.io/api/policy/v1beta1" + policy "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" diff --git a/staging/src/k8s.io/api/policy/v1/doc.go b/staging/src/k8s.io/api/policy/v1/doc.go new file mode 100644 index 00000000000..b46af58e43c --- /dev/null +++ b/staging/src/k8s.io/api/policy/v1/doc.go @@ -0,0 +1,24 @@ +/* +Copyright 2021 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. +*/ + +// +k8s:deepcopy-gen=package +// +k8s:protobuf-gen=package +// +k8s:openapi-gen=true + +// Package policy is for any kind of policy object. Suitable examples, even if +// they aren't all here, are PodDisruptionBudget, PodSecurityPolicy, +// NetworkPolicy, etc. +package v1 // import "k8s.io/api/policy/v1" diff --git a/staging/src/k8s.io/api/policy/v1/register.go b/staging/src/k8s.io/api/policy/v1/register.go new file mode 100644 index 00000000000..603c49b9cef --- /dev/null +++ b/staging/src/k8s.io/api/policy/v1/register.go @@ -0,0 +1,51 @@ +/* +Copyright 2021 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 v1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +// GroupName is the group name use in this package +const GroupName = "policy" + +// SchemeGroupVersion is group version used to register these objects +var SchemeGroupVersion = schema.GroupVersion{Group: GroupName, Version: "v1"} + +// Resource takes an unqualified resource and returns a Group qualified GroupResource +func Resource(resource string) schema.GroupResource { + return SchemeGroupVersion.WithResource(resource).GroupResource() +} + +var ( + SchemeBuilder = runtime.NewSchemeBuilder(addKnownTypes) + localSchemeBuilder = &SchemeBuilder + AddToScheme = localSchemeBuilder.AddToScheme +) + +// Adds the list of known types to the given scheme. +func addKnownTypes(scheme *runtime.Scheme) error { + scheme.AddKnownTypes(SchemeGroupVersion, + &PodDisruptionBudget{}, + &PodDisruptionBudgetList{}, + ) + // Add the watch version that applies + metav1.AddToGroupVersion(scheme, SchemeGroupVersion) + return nil +} diff --git a/staging/src/k8s.io/api/policy/v1/types.go b/staging/src/k8s.io/api/policy/v1/types.go new file mode 100644 index 00000000000..7a62a040133 --- /dev/null +++ b/staging/src/k8s.io/api/policy/v1/types.go @@ -0,0 +1,150 @@ +/* +Copyright 2021 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 v1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +// PodDisruptionBudgetSpec is a description of a PodDisruptionBudget. +type PodDisruptionBudgetSpec struct { + // An eviction is allowed if at least "minAvailable" pods selected by + // "selector" will still be available after the eviction, i.e. even in the + // absence of the evicted pod. So for example you can prevent all voluntary + // evictions by specifying "100%". + // +optional + MinAvailable *intstr.IntOrString `json:"minAvailable,omitempty" protobuf:"bytes,1,opt,name=minAvailable"` + + // Label query over pods whose evictions are managed by the disruption + // budget. + // A null selector will match no pods, while an empty ({}) selector will select + // all pods within the namespace. + // +patchStrategy=replace + // +optional + Selector *metav1.LabelSelector `json:"selector,omitempty" patchStrategy:"replace" protobuf:"bytes,2,opt,name=selector"` + + // An eviction is allowed if at most "maxUnavailable" pods selected by + // "selector" are unavailable after the eviction, i.e. even in absence of + // the evicted pod. For example, one can prevent all voluntary evictions + // by specifying 0. This is a mutually exclusive setting with "minAvailable". + // +optional + MaxUnavailable *intstr.IntOrString `json:"maxUnavailable,omitempty" protobuf:"bytes,3,opt,name=maxUnavailable"` +} + +// PodDisruptionBudgetStatus represents information about the status of a +// PodDisruptionBudget. Status may trail the actual state of a system. +type PodDisruptionBudgetStatus struct { + // Most recent generation observed when updating this PDB status. DisruptionsAllowed and other + // status information is valid only if observedGeneration equals to PDB's object generation. + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty" protobuf:"varint,1,opt,name=observedGeneration"` + + // DisruptedPods contains information about pods whose eviction was + // processed by the API server eviction subresource handler but has not + // yet been observed by the PodDisruptionBudget controller. + // A pod will be in this map from the time when the API server processed the + // eviction request to the time when the pod is seen by PDB controller + // as having been marked for deletion (or after a timeout). The key in the map is the name of the pod + // and the value is the time when the API server processed the eviction request. If + // the deletion didn't occur and a pod is still there it will be removed from + // the list automatically by PodDisruptionBudget controller after some time. + // If everything goes smooth this map should be empty for the most of the time. + // Large number of entries in the map may indicate problems with pod deletions. + // +optional + DisruptedPods map[string]metav1.Time `json:"disruptedPods,omitempty" protobuf:"bytes,2,rep,name=disruptedPods"` + + // Number of pod disruptions that are currently allowed. + DisruptionsAllowed int32 `json:"disruptionsAllowed" protobuf:"varint,3,opt,name=disruptionsAllowed"` + + // current number of healthy pods + CurrentHealthy int32 `json:"currentHealthy" protobuf:"varint,4,opt,name=currentHealthy"` + + // minimum desired number of healthy pods + DesiredHealthy int32 `json:"desiredHealthy" protobuf:"varint,5,opt,name=desiredHealthy"` + + // total number of pods counted by this disruption budget + ExpectedPods int32 `json:"expectedPods" protobuf:"varint,6,opt,name=expectedPods"` + + // Conditions contain conditions for PDB. The disruption controller sets the + // DisruptionAllowed condition. The following are known values for the reason field + // (additional reasons could be added in the future): + // - SyncFailed: The controller encountered an error and wasn't able to compute + // the number of allowed disruptions. Therefore no disruptions are + // allowed and the status of the condition will be False. + // - InsufficientPods: The number of pods are either at or below the number + // required by the PodDisruptionBudget. No disruptions are + // allowed and the status of the condition will be False. + // - SufficientPods: There are more pods than required by the PodDisruptionBudget. + // The condition will be True, and the number of allowed + // disruptions are provided by the disruptionsAllowed property. + // + // +optional + // +patchMergeKey=type + // +patchStrategy=merge + // +listType=map + // +listMapKey=type + Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,7,rep,name=conditions"` +} + +const ( + // DisruptionAllowedCondition is a condition set by the disruption controller + // that signal whether any of the pods covered by the PDB can be disrupted. + DisruptionAllowedCondition = "DisruptionAllowed" + + // SyncFailedReason is set on the DisruptionAllowed condition if reconcile + // of the PDB failed and therefore disruption of pods are not allowed. + SyncFailedReason = "SyncFailed" + // SufficientPodsReason is set on the DisruptionAllowed condition if there are + // more pods covered by the PDB than required and at least one can be disrupted. + SufficientPodsReason = "SufficientPods" + // InsufficientPodsReason is set on the DisruptionAllowed condition if the number + // of pods are equal to or fewer than required by the PDB. + InsufficientPodsReason = "InsufficientPods" +) + +// +genclient +// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object + +// PodDisruptionBudget is an object to define the max disruption that can be caused to a collection of pods +type PodDisruptionBudget struct { + metav1.TypeMeta `json:",inline"` + // Standard object's metadata. + // More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata + // +optional + metav1.ObjectMeta `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` + + // Specification of the desired behavior of the PodDisruptionBudget. + // +optional + Spec PodDisruptionBudgetSpec `json:"spec,omitempty" protobuf:"bytes,2,opt,name=spec"` + // Most recently observed status of the PodDisruptionBudget. + // +optional + Status PodDisruptionBudgetStatus `json:"status,omitempty" protobuf:"bytes,3,opt,name=status"` +} + +// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object + +// PodDisruptionBudgetList is a collection of PodDisruptionBudgets. +type PodDisruptionBudgetList struct { + metav1.TypeMeta `json:",inline"` + // Standard object's metadata. + // More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata + // +optional + metav1.ListMeta `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` + // Items is a list of PodDisruptionBudgets + Items []PodDisruptionBudget `json:"items" protobuf:"bytes,2,rep,name=items"` +} diff --git a/staging/src/k8s.io/api/policy/v1beta1/types.go b/staging/src/k8s.io/api/policy/v1beta1/types.go index 84c9eacf7f0..2811044518e 100644 --- a/staging/src/k8s.io/api/policy/v1beta1/types.go +++ b/staging/src/k8s.io/api/policy/v1beta1/types.go @@ -33,8 +33,12 @@ type PodDisruptionBudgetSpec struct { // Label query over pods whose evictions are managed by the disruption // budget. + // A null selector selects no pods. + // An empty selector ({}) also selects no pods, which differs from standard behavior of selecting all pods. + // In policy/v1, an empty selector will select all pods in the namespace. + // +patchStrategy=replace // +optional - Selector *metav1.LabelSelector `json:"selector,omitempty" protobuf:"bytes,2,opt,name=selector"` + Selector *metav1.LabelSelector `json:"selector,omitempty" patchStrategy:"replace" protobuf:"bytes,2,opt,name=selector"` // An eviction is allowed if at most "maxUnavailable" pods selected by // "selector" are unavailable after the eviction, i.e. even in absence of @@ -118,7 +122,9 @@ const ( // +genclient // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // +k8s:prerelease-lifecycle-gen:introduced=1.5 -// +k8s:prerelease-lifecycle-gen:deprecated=1.22 +// +k8s:prerelease-lifecycle-gen:deprecated=1.21 +// +k8s:prerelease-lifecycle-gen:removed=1.25 +// +k8s:prerelease-lifecycle-gen:replacement=policy,v1,PodDisruptionBudget // PodDisruptionBudget is an object to define the max disruption that can be caused to a collection of pods type PodDisruptionBudget struct { @@ -136,7 +142,9 @@ type PodDisruptionBudget struct { // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // +k8s:prerelease-lifecycle-gen:introduced=1.5 -// +k8s:prerelease-lifecycle-gen:deprecated=1.22 +// +k8s:prerelease-lifecycle-gen:deprecated=1.21 +// +k8s:prerelease-lifecycle-gen:removed=1.25 +// +k8s:prerelease-lifecycle-gen:replacement=policy,v1,PodDisruptionBudgetList // PodDisruptionBudgetList is a collection of PodDisruptionBudgets. type PodDisruptionBudgetList struct { diff --git a/staging/src/k8s.io/api/roundtrip_test.go b/staging/src/k8s.io/api/roundtrip_test.go index d4db75daf4d..257767c648b 100644 --- a/staging/src/k8s.io/api/roundtrip_test.go +++ b/staging/src/k8s.io/api/roundtrip_test.go @@ -20,6 +20,7 @@ import ( "math/rand" "testing" + policyv1 "k8s.io/api/policy/v1" admissionv1 "k8s.io/api/admission/v1" admissionv1beta1 "k8s.io/api/admission/v1beta1" admissionregv1 "k8s.io/api/admissionregistration/v1" @@ -110,6 +111,7 @@ var groups = []runtime.SchemeBuilder{ nodev1.SchemeBuilder, nodev1alpha1.SchemeBuilder, nodev1beta1.SchemeBuilder, + policyv1.SchemeBuilder, policyv1beta1.SchemeBuilder, rbacv1alpha1.SchemeBuilder, rbacv1beta1.SchemeBuilder, diff --git a/staging/src/k8s.io/client-go/listers/policy/v1/poddisruptionbudget_expansion.go b/staging/src/k8s.io/client-go/listers/policy/v1/poddisruptionbudget_expansion.go new file mode 100644 index 00000000000..cd9e1225d89 --- /dev/null +++ b/staging/src/k8s.io/client-go/listers/policy/v1/poddisruptionbudget_expansion.go @@ -0,0 +1,70 @@ +/* +Copyright 2021 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 v1 + +import ( + "fmt" + + "k8s.io/api/core/v1" + policy "k8s.io/api/policy/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/klog/v2" +) + +// PodDisruptionBudgetListerExpansion allows custom methods to be added to +// PodDisruptionBudgetLister. +type PodDisruptionBudgetListerExpansion interface { + GetPodPodDisruptionBudgets(pod *v1.Pod) ([]*policy.PodDisruptionBudget, error) +} + +// PodDisruptionBudgetNamespaceListerExpansion allows custom methods to be added to +// PodDisruptionBudgetNamespaceLister. +type PodDisruptionBudgetNamespaceListerExpansion interface{} + +// GetPodPodDisruptionBudgets returns a list of PodDisruptionBudgets matching a pod. +func (s *podDisruptionBudgetLister) GetPodPodDisruptionBudgets(pod *v1.Pod) ([]*policy.PodDisruptionBudget, error) { + var selector labels.Selector + + list, err := s.PodDisruptionBudgets(pod.Namespace).List(labels.Everything()) + if err != nil { + return nil, err + } + + var pdbList []*policy.PodDisruptionBudget + for i := range list { + pdb := list[i] + selector, err = metav1.LabelSelectorAsSelector(pdb.Spec.Selector) + if err != nil { + klog.Warningf("invalid selector: %v", err) + continue + } + + // Unlike the v1beta version, here we let an empty selector match everything. + if !selector.Matches(labels.Set(pod.Labels)) { + continue + } + pdbList = append(pdbList, pdb) + } + + if len(pdbList) == 0 { + return nil, fmt.Errorf("could not find PodDisruptionBudget for pod %s in namespace %s with labels: %v", pod.Name, pod.Namespace, pod.Labels) + } + + return pdbList, nil +} \ No newline at end of file diff --git a/staging/src/k8s.io/client-go/listers/policy/v1beta1/poddisruptionbudget_expansion.go b/staging/src/k8s.io/client-go/listers/policy/v1beta1/poddisruptionbudget_expansion.go index e93c3647b52..dce5dca8208 100644 --- a/staging/src/k8s.io/client-go/listers/policy/v1beta1/poddisruptionbudget_expansion.go +++ b/staging/src/k8s.io/client-go/listers/policy/v1beta1/poddisruptionbudget_expansion.go @@ -40,10 +40,6 @@ type PodDisruptionBudgetNamespaceListerExpansion interface{} func (s *podDisruptionBudgetLister) GetPodPodDisruptionBudgets(pod *v1.Pod) ([]*policy.PodDisruptionBudget, error) { var selector labels.Selector - if len(pod.Labels) == 0 { - return nil, fmt.Errorf("no PodDisruptionBudgets found for pod %v because it has no labels", pod.Name) - } - list, err := s.PodDisruptionBudgets(pod.Namespace).List(labels.Everything()) if err != nil { return nil, err diff --git a/staging/src/k8s.io/component-helpers/apps/poddisruptionbudget/helpers.go b/staging/src/k8s.io/component-helpers/apps/poddisruptionbudget/helpers.go index d14dc71ca11..f6008c76d76 100644 --- a/staging/src/k8s.io/component-helpers/apps/poddisruptionbudget/helpers.go +++ b/staging/src/k8s.io/component-helpers/apps/poddisruptionbudget/helpers.go @@ -17,7 +17,7 @@ limitations under the License. package poddisruptionbudget import ( - policy "k8s.io/api/policy/v1beta1" + policy "k8s.io/api/policy/v1" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) diff --git a/staging/src/k8s.io/kubectl/pkg/scheme/install.go b/staging/src/k8s.io/kubectl/pkg/scheme/install.go index d7b9dc5e002..0aa436eeb9f 100644 --- a/staging/src/k8s.io/kubectl/pkg/scheme/install.go +++ b/staging/src/k8s.io/kubectl/pkg/scheme/install.go @@ -38,6 +38,7 @@ import ( extensionsv1beta1 "k8s.io/api/extensions/v1beta1" imagepolicyv1alpha1 "k8s.io/api/imagepolicy/v1alpha1" networkingv1 "k8s.io/api/networking/v1" + policyv1 "k8s.io/api/policy/v1" policyv1beta1 "k8s.io/api/policy/v1beta1" rbacv1 "k8s.io/api/rbac/v1" rbacv1alpha1 "k8s.io/api/rbac/v1alpha1" @@ -74,7 +75,7 @@ func init() { utilruntime.Must(Scheme.SetVersionPriority(extensionsv1beta1.SchemeGroupVersion)) utilruntime.Must(Scheme.SetVersionPriority(imagepolicyv1alpha1.SchemeGroupVersion)) utilruntime.Must(Scheme.SetVersionPriority(networkingv1.SchemeGroupVersion)) - utilruntime.Must(Scheme.SetVersionPriority(policyv1beta1.SchemeGroupVersion)) + utilruntime.Must(Scheme.SetVersionPriority(policyv1beta1.SchemeGroupVersion, policyv1.SchemeGroupVersion)) utilruntime.Must(Scheme.SetVersionPriority(rbacv1.SchemeGroupVersion, rbacv1beta1.SchemeGroupVersion, rbacv1alpha1.SchemeGroupVersion)) utilruntime.Must(Scheme.SetVersionPriority(schedulingv1alpha1.SchemeGroupVersion)) utilruntime.Must(Scheme.SetVersionPriority(storagev1.SchemeGroupVersion, storagev1beta1.SchemeGroupVersion)) diff --git a/test/integration/apiserver/admissionwebhook/admission_test.go b/test/integration/apiserver/admissionwebhook/admission_test.go index d88d6fa08b6..9140de559e4 100644 --- a/test/integration/apiserver/admissionwebhook/admission_test.go +++ b/test/integration/apiserver/admissionwebhook/admission_test.go @@ -119,9 +119,9 @@ var ( gvr("", "v1", "pods/exec"): {"create": testPodConnectSubresource}, gvr("", "v1", "pods/portforward"): {"create": testPodConnectSubresource}, - gvr("", "v1", "bindings"): {"create": testPodBindingEviction}, - gvr("", "v1", "pods/binding"): {"create": testPodBindingEviction}, - gvr("", "v1", "pods/eviction"): {"create": testPodBindingEviction}, + gvr("", "v1", "bindings"): {"create": testPodBindingEviction}, + gvr("", "v1", "pods/binding"): {"create": testPodBindingEviction}, + gvr("", "v1", "pods/eviction"): {"create": testPodBindingEviction}, gvr("", "v1", "nodes/proxy"): {"*": testSubresourceProxy}, gvr("", "v1", "pods/proxy"): {"*": testSubresourceProxy}, diff --git a/test/integration/apiserver/apply/status_test.go b/test/integration/apiserver/apply/status_test.go index e8f0b4e54b4..9cba6664575 100644 --- a/test/integration/apiserver/apply/status_test.go +++ b/test/integration/apiserver/apply/status_test.go @@ -54,6 +54,7 @@ var statusData = map[schema.GroupVersionResource]string{ gvr("batch", "v1", "cronjobs"): `{"status": {"lastScheduleTime": null}}`, gvr("batch", "v1beta1", "cronjobs"): `{"status": {"lastScheduleTime": null}}`, gvr("storage.k8s.io", "v1", "volumeattachments"): `{"status": {"attached": true}}`, + gvr("policy", "v1", "poddisruptionbudgets"): `{"status": {"currentHealthy": 5}}`, gvr("policy", "v1beta1", "poddisruptionbudgets"): `{"status": {"currentHealthy": 5}}`, gvr("certificates.k8s.io", "v1beta1", "certificatesigningrequests"): `{"status": {"conditions": [{"type": "MyStatus"}]}}`, gvr("certificates.k8s.io", "v1", "certificatesigningrequests"): `{"status": {"conditions": [{"type": "MyStatus", "status": "True"}]}}`, diff --git a/test/integration/disruption/disruption_test.go b/test/integration/disruption/disruption_test.go index 0a74a916e49..911dc9be9b7 100644 --- a/test/integration/disruption/disruption_test.go +++ b/test/integration/disruption/disruption_test.go @@ -27,6 +27,7 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" v1 "k8s.io/api/core/v1" + policyv1 "k8s.io/api/policy/v1" "k8s.io/api/policy/v1beta1" apiextensionsclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -81,7 +82,7 @@ func setup(t *testing.T) (*kubeapiservertesting.TestServer, *disruption.Disrupti pdbc := disruption.NewDisruptionController( informers.Core().V1().Pods(), - informers.Policy().V1beta1().PodDisruptionBudgets(), + informers.Policy().V1().PodDisruptionBudgets(), informers.Core().V1().ReplicationControllers(), informers.Apps().V1().ReplicaSets(), informers.Apps().V1().Deployments(), @@ -113,7 +114,6 @@ func TestPDBWithScaleSubresource(t *testing.T) { replicas := 4 maxUnavailable := int32(2) - podLabelValue := "test-crd" resource := &unstructured.Unstructured{ Object: map[string]interface{}{ @@ -134,40 +134,42 @@ func TestPDBWithScaleSubresource(t *testing.T) { } trueValue := true - ownerRef := metav1.OwnerReference{ - Name: resource.GetName(), - Kind: crdDefinition.Spec.Names.Kind, - APIVersion: crdDefinition.Spec.Group + "/" + crdDefinition.Spec.Versions[0].Name, - UID: createdResource.GetUID(), - Controller: &trueValue, + ownerRefs := []metav1.OwnerReference{ + { + Name: resource.GetName(), + Kind: crdDefinition.Spec.Names.Kind, + APIVersion: crdDefinition.Spec.Group + "/" + crdDefinition.Spec.Versions[0].Name, + UID: createdResource.GetUID(), + Controller: &trueValue, + }, } for i := 0; i < replicas; i++ { - createPod(t, fmt.Sprintf("pod-%d", i), nsName, podLabelValue, clientSet, ownerRef) + createPod(t, fmt.Sprintf("pod-%d", i), nsName, map[string]string{"app": "test-crd"}, clientSet, ownerRefs) } waitToObservePods(t, informers.Core().V1().Pods().Informer(), 4, v1.PodRunning) - pdb := &v1beta1.PodDisruptionBudget{ + pdb := &policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{ Name: "test-pdb", }, - Spec: v1beta1.PodDisruptionBudgetSpec{ + Spec: policyv1.PodDisruptionBudgetSpec{ MaxUnavailable: &intstr.IntOrString{ Type: intstr.Int, IntVal: maxUnavailable, }, Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"app": podLabelValue}, + MatchLabels: map[string]string{"app": "test-crd"}, }, }, } - if _, err := clientSet.PolicyV1beta1().PodDisruptionBudgets(nsName).Create(context.TODO(), pdb, metav1.CreateOptions{}); err != nil { + if _, err := clientSet.PolicyV1().PodDisruptionBudgets(nsName).Create(context.TODO(), pdb, metav1.CreateOptions{}); err != nil { t.Errorf("Error creating PodDisruptionBudget: %v", err) } waitPDBStable(t, clientSet, 4, nsName, pdb.Name) - newPdb, err := clientSet.PolicyV1beta1().PodDisruptionBudgets(nsName).Get(context.TODO(), pdb.Name, metav1.GetOptions{}) + newPdb, err := clientSet.PolicyV1().PodDisruptionBudgets(nsName).Get(context.TODO(), pdb.Name, metav1.GetOptions{}) if err != nil { t.Errorf("Error getting PodDisruptionBudget: %v", err) } @@ -183,15 +185,211 @@ func TestPDBWithScaleSubresource(t *testing.T) { } } -func createPod(t *testing.T, name, namespace, labelValue string, clientSet clientset.Interface, ownerRef metav1.OwnerReference) { +func TestEmptySelector(t *testing.T) { + testcases := []struct { + name string + createPDBFunc func(clientSet clientset.Interface, name, nsName string, minAvailable intstr.IntOrString) error + expectedCurrentHealthy int32 + }{ + { + name: "v1beta1 should not target any pods", + createPDBFunc: func(clientSet clientset.Interface, name, nsName string, minAvailable intstr.IntOrString) error { + pdb := &v1beta1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: v1beta1.PodDisruptionBudgetSpec{ + MinAvailable: &minAvailable, + Selector: &metav1.LabelSelector{}, + }, + } + _, err := clientSet.PolicyV1beta1().PodDisruptionBudgets(nsName).Create(context.TODO(), pdb, metav1.CreateOptions{}) + return err + }, + expectedCurrentHealthy: 0, + }, + { + name: "v1 should target all pods", + createPDBFunc: func(clientSet clientset.Interface, name, nsName string, minAvailable intstr.IntOrString) error { + pdb := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: policyv1.PodDisruptionBudgetSpec{ + MinAvailable: &minAvailable, + Selector: &metav1.LabelSelector{}, + }, + } + _, err := clientSet.PolicyV1().PodDisruptionBudgets(nsName).Create(context.TODO(), pdb, metav1.CreateOptions{}) + return err + }, + expectedCurrentHealthy: 4, + }, + } + + for i, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + s, pdbc, informers, clientSet, _, _ := setup(t) + defer s.TearDownFn() + + nsName := fmt.Sprintf("pdb-empty-selector-%d", i) + createNs(t, nsName, clientSet) + + stopCh := make(chan struct{}) + informers.Start(stopCh) + go pdbc.Run(stopCh) + defer close(stopCh) + + replicas := 4 + minAvailable := intstr.FromInt(2) + + for j := 0; j < replicas; j++ { + createPod(t, fmt.Sprintf("pod-%d", j), nsName, map[string]string{"app": "test-crd"}, + clientSet, []metav1.OwnerReference{}) + } + + waitToObservePods(t, informers.Core().V1().Pods().Informer(), 4, v1.PodRunning) + + pdbName := "test-pdb" + if err := tc.createPDBFunc(clientSet, pdbName, nsName, minAvailable); err != nil { + t.Errorf("Error creating PodDisruptionBudget: %v", err) + } + + waitPDBStable(t, clientSet, tc.expectedCurrentHealthy, nsName, pdbName) + + newPdb, err := clientSet.PolicyV1().PodDisruptionBudgets(nsName).Get(context.TODO(), pdbName, metav1.GetOptions{}) + if err != nil { + t.Errorf("Error getting PodDisruptionBudget: %v", err) + } + + if expected, found := tc.expectedCurrentHealthy, newPdb.Status.CurrentHealthy; expected != found { + t.Errorf("Expected %d, but found %d", expected, found) + } + }) + } +} + +func TestSelectorsForPodsWithoutLabels(t *testing.T) { + testcases := []struct { + name string + createPDBFunc func(clientSet clientset.Interface, 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(clientSet clientset.Interface, name, nsName string, minAvailable intstr.IntOrString) error { + pdb := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: policyv1.PodDisruptionBudgetSpec{ + MinAvailable: &minAvailable, + Selector: &metav1.LabelSelector{}, + }, + } + _, err := clientSet.PolicyV1().PodDisruptionBudgets(nsName).Create(context.TODO(), pdb, metav1.CreateOptions{}) + return err + }, + expectedCurrentHealthy: 1, + }, + { + name: "pods with no labels can be targeted by v1 PDBs with DoesNotExist selector", + createPDBFunc: func(clientSet clientset.Interface, name, nsName string, minAvailable intstr.IntOrString) error { + pdb := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: policyv1.PodDisruptionBudgetSpec{ + MinAvailable: &minAvailable, + Selector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "DoesNotExist", + Operator: metav1.LabelSelectorOpDoesNotExist, + }, + }, + }, + }, + } + _, err := clientSet.PolicyV1().PodDisruptionBudgets(nsName).Create(context.TODO(), pdb, metav1.CreateOptions{}) + return err + }, + expectedCurrentHealthy: 1, + }, + { + name: "pods with no labels can be targeted by v1beta1 PDBs with DoesNotExist selector", + createPDBFunc: func(clientSet clientset.Interface, name, nsName string, minAvailable intstr.IntOrString) error { + pdb := &v1beta1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: v1beta1.PodDisruptionBudgetSpec{ + MinAvailable: &minAvailable, + Selector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "DoesNotExist", + Operator: metav1.LabelSelectorOpDoesNotExist, + }, + }, + }, + }, + } + _, err := clientSet.PolicyV1beta1().PodDisruptionBudgets(nsName).Create(context.TODO(), pdb, metav1.CreateOptions{}) + return err + }, + expectedCurrentHealthy: 1, + }, + } + + for i, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + s, pdbc, informers, clientSet, _, _ := setup(t) + defer s.TearDownFn() + + nsName := fmt.Sprintf("pdb-selectors-%d", i) + createNs(t, nsName, clientSet) + + stopCh := make(chan struct{}) + informers.Start(stopCh) + go pdbc.Run(stopCh) + defer close(stopCh) + + minAvailable := intstr.FromInt(1) + + // Create the PDB first and wait for it to settle. + pdbName := "test-pdb" + if err := tc.createPDBFunc(clientSet, pdbName, nsName, minAvailable); err != nil { + t.Errorf("Error creating PodDisruptionBudget: %v", err) + } + waitPDBStable(t, clientSet, 0, nsName, pdbName) + + // Create a pod and wait for it be reach the running phase. + createPod(t,"pod", nsName, map[string]string{}, clientSet, []metav1.OwnerReference{}) + waitToObservePods(t, informers.Core().V1().Pods().Informer(), 1, v1.PodRunning) + + // Then verify that the added pod are picked up by the disruption controller. + waitPDBStable(t, clientSet, 1, nsName, pdbName) + + newPdb, err := clientSet.PolicyV1().PodDisruptionBudgets(nsName).Get(context.TODO(), pdbName, metav1.GetOptions{}) + if err != nil { + t.Errorf("Error getting PodDisruptionBudget: %v", err) + } + + if expected, found := tc.expectedCurrentHealthy, newPdb.Status.CurrentHealthy; expected != found { + t.Errorf("Expected %d, but found %d", expected, found) + } + }) + } +} + +func createPod(t *testing.T, name, namespace string, labels map[string]string, clientSet clientset.Interface, ownerRefs []metav1.OwnerReference) { pod := &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - Labels: map[string]string{"app": labelValue}, - OwnerReferences: []metav1.OwnerReference{ - ownerRef, - }, + Name: name, + Namespace: namespace, + Labels: labels, + OwnerReferences: ownerRefs, }, Spec: v1.PodSpec{ Containers: []v1.Container{ @@ -271,7 +469,7 @@ func waitPDBStable(t *testing.T, clientSet clientset.Interface, podNum int32, ns if err != nil { return false, err } - if pdb.Status.CurrentHealthy != podNum { + if pdb.Status.ObservedGeneration == 0 || pdb.Status.CurrentHealthy != podNum { return false, nil } return true, nil diff --git a/test/integration/etcd/data.go b/test/integration/etcd/data.go index befff1e2ee5..9287620176e 100644 --- a/test/integration/etcd/data.go +++ b/test/integration/etcd/data.go @@ -265,6 +265,14 @@ func GetEtcdStorageDataForNamespace(namespace string) map[schema.GroupVersionRes }, // -- + // k8s.io/kubernetes/pkg/apis/policy/v1 + gvr("policy", "v1", "poddisruptionbudgets"): { + Stub: `{"metadata": {"name": "pdbv1"}, "spec": {"selector": {"matchLabels": {"anokkey": "anokvalue"}}}}`, + ExpectedEtcdPath: "/registry/poddisruptionbudgets/" + namespace + "/pdbv1", + ExpectedGVK: gvkP("policy", "v1beta1", "PodDisruptionBudget"), + }, + // -- + // k8s.io/kubernetes/pkg/apis/policy/v1beta1 gvr("policy", "v1beta1", "poddisruptionbudgets"): { Stub: `{"metadata": {"name": "pdb1"}, "spec": {"selector": {"matchLabels": {"anokkey": "anokvalue"}}}}`, diff --git a/test/integration/evictions/evictions_test.go b/test/integration/evictions/evictions_test.go index ae78b4676ff..1c592c09dfe 100644 --- a/test/integration/evictions/evictions_test.go +++ b/test/integration/evictions/evictions_test.go @@ -19,7 +19,6 @@ package evictions import ( "context" "fmt" - "net/http/httptest" "reflect" "sync" @@ -28,7 +27,8 @@ import ( "time" v1 "k8s.io/api/core/v1" - "k8s.io/api/policy/v1beta1" + policyv1 "k8s.io/api/policy/v1" + policyv1beta1 "k8s.io/api/policy/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" @@ -95,7 +95,7 @@ func TestConcurrentEvictionRequests(t *testing.T) { waitToObservePods(t, informers.Core().V1().Pods().Informer(), numOfEvictions, v1.PodRunning) pdb := newPDB() - if _, err := clientSet.PolicyV1beta1().PodDisruptionBudgets(ns.Name).Create(context.TODO(), pdb, metav1.CreateOptions{}); err != nil { + if _, err := clientSet.PolicyV1().PodDisruptionBudgets(ns.Name).Create(context.TODO(), pdb, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create PodDisruptionBudget: %v", err) } @@ -110,7 +110,7 @@ func TestConcurrentEvictionRequests(t *testing.T) { go func(id int, errCh chan error) { defer wg.Done() podName := fmt.Sprintf(podNameFormat, id) - eviction := newEviction(ns.Name, podName, deleteOption) + eviction := newV1beta1Eviction(ns.Name, podName, deleteOption) err := wait.PollImmediate(5*time.Second, 60*time.Second, func() (bool, error) { e := clientSet.PolicyV1beta1().Evictions(ns.Name).Evict(context.TODO(), eviction) @@ -208,7 +208,7 @@ func TestTerminalPodEviction(t *testing.T) { waitToObservePods(t, informers.Core().V1().Pods().Informer(), 1, v1.PodSucceeded) pdb := newPDB() - if _, err := clientSet.PolicyV1beta1().PodDisruptionBudgets(ns.Name).Create(context.TODO(), pdb, metav1.CreateOptions{}); err != nil { + if _, err := clientSet.PolicyV1().PodDisruptionBudgets(ns.Name).Create(context.TODO(), pdb, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create PodDisruptionBudget: %v", err) } @@ -219,7 +219,7 @@ func TestTerminalPodEviction(t *testing.T) { t.Fatalf("Error while listing pod disruption budget") } oldPdb := pdbList.Items[0] - eviction := newEviction(ns.Name, pod.Name, deleteOption) + eviction := newV1beta1Eviction(ns.Name, pod.Name, deleteOption) err = wait.PollImmediate(5*time.Second, 60*time.Second, func() (bool, error) { e := clientSet.PolicyV1beta1().Evictions(ns.Name).Evict(context.TODO(), eviction) switch { @@ -292,12 +292,12 @@ func addPodConditionReady(pod *v1.Pod) { } } -func newPDB() *v1beta1.PodDisruptionBudget { - return &v1beta1.PodDisruptionBudget{ +func newPDB() *policyv1.PodDisruptionBudget { + return &policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{ Name: "test-pdb", }, - Spec: v1beta1.PodDisruptionBudgetSpec{ + Spec: policyv1.PodDisruptionBudgetSpec{ MinAvailable: &intstr.IntOrString{ Type: intstr.Int, IntVal: 0, @@ -309,8 +309,8 @@ func newPDB() *v1beta1.PodDisruptionBudget { } } -func newEviction(ns, evictionName string, deleteOption metav1.DeleteOptions) *v1beta1.Eviction { - return &v1beta1.Eviction{ +func newV1beta1Eviction(ns, evictionName string, deleteOption metav1.DeleteOptions) *policyv1beta1.Eviction { + return &policyv1beta1.Eviction{ TypeMeta: metav1.TypeMeta{ APIVersion: "Policy/v1beta1", Kind: "Eviction", @@ -348,7 +348,7 @@ func rmSetup(t *testing.T) (*httptest.Server, framework.CloseFunc, *disruption.D rm := disruption.NewDisruptionController( informers.Core().V1().Pods(), - informers.Policy().V1beta1().PodDisruptionBudgets(), + informers.Policy().V1().PodDisruptionBudgets(), informers.Core().V1().ReplicationControllers(), informers.Apps().V1().ReplicaSets(), informers.Apps().V1().Deployments(), diff --git a/test/integration/scheduler/util.go b/test/integration/scheduler/util.go index c4776c6c376..ce457acaa86 100644 --- a/test/integration/scheduler/util.go +++ b/test/integration/scheduler/util.go @@ -62,7 +62,7 @@ func initDisruptionController(t *testing.T, testCtx *testutils.TestContext) *dis dc := disruption.NewDisruptionController( informers.Core().V1().Pods(), - informers.Policy().V1beta1().PodDisruptionBudgets(), + informers.Policy().V1().PodDisruptionBudgets(), informers.Core().V1().ReplicationControllers(), informers.Apps().V1().ReplicaSets(), informers.Apps().V1().Deployments(),