Merge pull request #54790 from wackxu/fixTODOPdb

Automatic merge from submit-queue (batch tested with PRs 53190, 54790, 54445, 52607, 54801). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

one of min-available/max-available must be specified when create pdb

**What this PR does / why we need it**:

fix TODO one of min-available/max-available must be specified when create pdb

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```

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",

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")
 	}

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))
+		}
+	}
+}