diff --git a/pkg/kubectl/BUILD b/pkg/kubectl/BUILD index af8ea4a70b3..ff86f12f78e 100644 --- a/pkg/kubectl/BUILD +++ b/pkg/kubectl/BUILD @@ -17,6 +17,7 @@ go_test( "env_file_test.go", "generate_test.go", "namespace_test.go", + "pdb_test.go", "quota_test.go", "resource_filter_test.go", "rolebinding_test.go", @@ -57,6 +58,7 @@ go_test( "//vendor/k8s.io/api/batch/v2alpha1:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/api/extensions/v1beta1:go_default_library", + "//vendor/k8s.io/api/policy/v1beta1:go_default_library", "//vendor/k8s.io/api/rbac/v1:go_default_library", "//vendor/k8s.io/api/rbac/v1beta1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library", diff --git a/pkg/kubectl/pdb.go b/pkg/kubectl/pdb.go index 0c93b27b57c..24632b0c3ce 100644 --- a/pkg/kubectl/pdb.go +++ b/pkg/kubectl/pdb.go @@ -18,7 +18,6 @@ package kubectl import ( "fmt" - "os" policy "k8s.io/api/policy/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -167,15 +166,6 @@ func (s *PodDisruptionBudgetV2Generator) StructuredGenerate() (runtime.Object, e return nil, err } - if len(s.MaxUnavailable) == 0 && len(s.MinAvailable) == 0 { - s.MinAvailable = "1" - - // This behavior is intended for backward compatibility. - // TODO: remove in Kubernetes 1.8 - fmt.Fprintln(os.Stderr, "Deprecated behavior in kubectl create pdb: Defaulting min-available to 1. "+ - "Kubernetes 1.8 will remove this default, and one of min-available/max-available must be specified. ") - } - if len(s.MaxUnavailable) > 0 { maxUnavailable := intstr.Parse(s.MaxUnavailable) return &policy.PodDisruptionBudget{ @@ -213,6 +203,9 @@ func (s *PodDisruptionBudgetV2Generator) validate() error { if len(s.Selector) == 0 { return fmt.Errorf("a selector must be specified") } + if len(s.MaxUnavailable) == 0 && len(s.MinAvailable) == 0 { + return fmt.Errorf("one of min-available/max-available must be specified") + } if len(s.MaxUnavailable) > 0 && len(s.MinAvailable) > 0 { return fmt.Errorf("min-available and max-unavailable cannot be both specified") } diff --git a/pkg/kubectl/pdb_test.go b/pkg/kubectl/pdb_test.go new file mode 100644 index 00000000000..f821e9e81ef --- /dev/null +++ b/pkg/kubectl/pdb_test.go @@ -0,0 +1,130 @@ +/* +Copyright 2017 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 kubectl + +import ( + policy "k8s.io/api/policy/v1beta1" + apiequality "k8s.io/apimachinery/pkg/api/equality" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "testing" +) + +func TestPodDisruptionBudgetV2Generator(t *testing.T) { + minAvailableNumber := intstr.Parse("2") + minAvailablePercent := intstr.Parse("50%") + + tests := map[string]struct { + params map[string]interface{} + expected *policy.PodDisruptionBudget + expectErr bool + }{ + "test valid case with number min-available": { + params: map[string]interface{}{ + "name": "foo", + "selector": "app=nginx", + "min-available": "2", + "max-available": "", + }, + expected: &policy.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: policy.PodDisruptionBudgetSpec{ + MinAvailable: &minAvailableNumber, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "nginx", + }, + }, + }, + }, + expectErr: false, + }, + "test valid case with percent min-available": { + params: map[string]interface{}{ + "name": "foo", + "selector": "app=nginx", + "min-available": "50%", + "max-available": "", + }, + expected: &policy.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: policy.PodDisruptionBudgetSpec{ + MinAvailable: &minAvailablePercent, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "nginx", + }, + }, + }, + }, + expectErr: false, + }, + "test missing required param": { + params: map[string]interface{}{ + "name": "foo", + "min-available": "2", + "max-available": "", + }, + expectErr: true, + }, + "test with invalid format params": { + params: map[string]interface{}{ + "name": "foo", + "selector": "app=nginx", + "min-available": 2, + "max-available": "", + }, + expectErr: true, + }, + "test min-available/max-available all not be specified": { + params: map[string]interface{}{ + "name": "foo", + "selector": "app=nginx", + "min-available": "", + "max-available": "", + }, + expectErr: true, + }, + "test min-available and max-unavailable cannot be both specified": { + params: map[string]interface{}{ + "name": "foo", + "selector": "app=nginx", + "min-available": "2", + "max-available": "5", + }, + expectErr: true, + }, + } + + generator := PodDisruptionBudgetV2Generator{} + for name, test := range tests { + obj, err := generator.Generate(test.params) + if !test.expectErr && err != nil { + t.Errorf("%s: unexpected error: %v", name, err) + } + if test.expectErr && err != nil { + continue + } + if !apiequality.Semantic.DeepEqual(obj.(*policy.PodDisruptionBudget), test.expected) { + t.Errorf("%s:\nexpected:\n%#v\nsaw:\n%#v", name, test.expected, obj.(*policy.PodDisruptionBudget)) + } + } +}