PDB MaxUnavailable: kubectl changes

hack/make-rules/test-cmd-util.sh

@@ -431,16 +431,29 @@ run_pod_tests() {
   # Post-condition: configmap exists and has expected values
   kube::test::get_object_assert 'configmap/test-configmap --namespace=test-kubectl-describe-pod' "{{$id_field}}" 'test-configmap'
 
-  ### Create a pod disruption budget
+  ### Create a pod disruption budget with minAvailable
   # Command
-  kubectl create pdb test-pdb --selector=app=rails --min-available=2 --namespace=test-kubectl-describe-pod
+  kubectl create pdb test-pdb-1 --selector=app=rails --min-available=2 --namespace=test-kubectl-describe-pod
   # Post-condition: pdb exists and has expected values
-  kube::test::get_object_assert 'pdb/test-pdb --namespace=test-kubectl-describe-pod' "{{$pdb_min_available}}" '2'
+  kube::test::get_object_assert 'pdb/test-pdb-1 --namespace=test-kubectl-describe-pod' "{{$pdb_min_available}}" '2'
   # Command
   kubectl create pdb test-pdb-2 --selector=app=rails --min-available=50% --namespace=test-kubectl-describe-pod
   # Post-condition: pdb exists and has expected values
   kube::test::get_object_assert 'pdb/test-pdb-2 --namespace=test-kubectl-describe-pod' "{{$pdb_min_available}}" '50%'
 
+  ### Create a pod disruption budget with maxUnavailable
+  # Command
+  kubectl create pdb test-pdb-3 --selector=app=rails --max-unavailable=2 --namespace=test-kubectl-describe-pod
+  # Post-condition: pdb exists and has expected values
+  kube::test::get_object_assert 'pdb/test-pdb-3 --namespace=test-kubectl-describe-pod' "{{$pdb_max_unavailable}}" '2'
+  # Command
+  kubectl create pdb test-pdb-4 --selector=app=rails --max-unavailable=50% --namespace=test-kubectl-describe-pod
+  # Post-condition: pdb exists and has expected values
+  kube::test::get_object_assert 'pdb/test-pdb-4 --namespace=test-kubectl-describe-pod' "{{$pdb_max_unavailable}}" '50%'
+
+  ### Fail creating a pod disruption budget if both maxUnavailable and minAvailable specified
+  ! kubectl create pdb test-pdb --selector=app=rails --min-available=2 --max-unavailable=3 --namespace=test-kubectl-describe-pod
+
   # Create a pod that consumes secret, configmap, and downward API keys as envs
   kube::test::get_object_assert 'pods --namespace=test-kubectl-describe-pod' "{{range.items}}{{$id_field}}:{{end}}" ''
   kubectl create -f hack/testdata/pod-with-api-env.yaml --namespace=test-kubectl-describe-pod
@@ -453,7 +466,7 @@ run_pod_tests() {
   kubectl delete pod env-test-pod --namespace=test-kubectl-describe-pod
   kubectl delete secret test-secret --namespace=test-kubectl-describe-pod
   kubectl delete configmap test-configmap --namespace=test-kubectl-describe-pod
-  kubectl delete pdb/test-pdb pdb/test-pdb-2 --namespace=test-kubectl-describe-pod
+  kubectl delete pdb/test-pdb-1 pdb/test-pdb-2 pdb/test-pdb-3 pdb/test-pdb-4 --namespace=test-kubectl-describe-pod
   kubectl delete namespace test-kubectl-describe-pod
 
   ### Create two PODs
@@ -2990,6 +3003,7 @@ runTests() {
   deployment_second_image_field="(index .spec.template.spec.containers 1).image"
   change_cause_annotation='.*kubernetes.io/change-cause.*'
   pdb_min_available=".spec.minAvailable"
+  pdb_max_unavailable=".spec.maxUnavailable"
   template_generation_field=".spec.templateGeneration"
 
   # Make sure "default" namespace exists.

hack/verify-flags/known-flags.txt

@@ -457,6 +457,7 @@ max-outgoing-burst
 max-outgoing-qps
 max-pods
 max-requests-inflight
+max-unavailable
 mesos-authentication-principal
 mesos-authentication-provider
 mesos-authentication-secret-file

pkg/kubectl/cmd/create_pdb.go

@@ -59,8 +59,10 @@ func NewCmdCreatePodDisruptionBudget(f cmdutil.Factory, cmdOut io.Writer) *cobra
 	cmdutil.AddApplyAnnotationFlags(cmd)
 	cmdutil.AddValidateFlags(cmd)
 	cmdutil.AddPrinterFlags(cmd)
-	cmdutil.AddGeneratorFlags(cmd, cmdutil.PodDisruptionBudgetV1GeneratorName)
-	cmd.Flags().String("min-available", "1", i18n.T("The minimum number or percentage of available pods this budget requires."))
+	cmdutil.AddGeneratorFlags(cmd, cmdutil.PodDisruptionBudgetV2GeneratorName)
+
+	cmd.Flags().String("min-available", "", i18n.T("The minimum number or percentage of available pods this budget requires."))
+	cmd.Flags().String("max-unavailable", "", i18n.T("The maximum number or percentage of unavailable pods this budget requires."))
 	cmd.Flags().String("selector", "", i18n.T("A label selector to use for this budget. Only equality-based selector requirements are supported."))
 	return cmd
 }
@@ -79,6 +81,13 @@ func CreatePodDisruptionBudget(f cmdutil.Factory, cmdOut io.Writer, cmd *cobra.C
 			MinAvailable: cmdutil.GetFlagString(cmd, "min-available"),
 			Selector:     cmdutil.GetFlagString(cmd, "selector"),
 		}
+	case cmdutil.PodDisruptionBudgetV2GeneratorName:
+		generator = &kubectl.PodDisruptionBudgetV2Generator{
+			Name:           name,
+			MinAvailable:   cmdutil.GetFlagString(cmd, "min-available"),
+			MaxUnavailable: cmdutil.GetFlagString(cmd, "max-unavailable"),
+			Selector:       cmdutil.GetFlagString(cmd, "selector"),
+		}
 	default:
 		return cmdutil.UsageError(cmd, fmt.Sprintf("Generator: %s not supported.", generatorName))
 	}

pkg/kubectl/cmd/util/factory_client_access.go

@@ -455,6 +455,9 @@ func (f *ring0Factory) DefaultNamespace() (string, bool, error) {
 }
 
 const (
+	// TODO(sig-cli): Enforce consistent naming for generators here.
+	// See discussion in https://github.com/kubernetes/kubernetes/issues/46237
+	// before you add any more.
 	RunV1GeneratorName                      = "run/v1"
 	RunPodV1GeneratorName                   = "run-pod/v1"
 	ServiceV1GeneratorName                  = "service/v1"
@@ -482,6 +485,7 @@ const (
 	RoleBindingV1GeneratorName              = "rolebinding.rbac.authorization.k8s.io/v1alpha1"
 	ClusterV1Beta1GeneratorName             = "cluster/v1beta1"
 	PodDisruptionBudgetV1GeneratorName      = "poddisruptionbudget/v1beta1"
+	PodDisruptionBudgetV2GeneratorName      = "poddisruptionbudget/v1beta1/v2"
 )
 
 // DefaultGenerators returns the set of default generators for use in Factory instances

pkg/kubectl/pdb.go

@@ -18,6 +18,7 @@ package kubectl
 
 import (
 	"fmt"
+	"os"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -38,7 +39,7 @@ var _ StructuredGenerator = &PodDisruptionBudgetV1Generator{}
 func (PodDisruptionBudgetV1Generator) ParamNames() []GeneratorParam {
 	return []GeneratorParam{
 		{"name", true},
-		{"mim-available", true},
+		{"min-available", true},
 		{"selector", true},
 	}
 }
@@ -52,11 +53,11 @@ func (s PodDisruptionBudgetV1Generator) Generate(params map[string]interface{})
 	if !isString {
 		return nil, fmt.Errorf("expected string, saw %v for 'name'", name)
 	}
-	minAvailable, isString := params["mim-available"].(string)
+	minAvailable, isString := params["min-available"].(string)
 	if !isString {
 		return nil, fmt.Errorf("expected string, found %v", minAvailable)
 	}
-	selector, isString := params["selecor"].(string)
+	selector, isString := params["selector"].(string)
 	if !isString {
 		return nil, fmt.Errorf("expected string, found %v", selector)
 	}
@@ -66,6 +67,11 @@ func (s PodDisruptionBudgetV1Generator) Generate(params map[string]interface{})
 
 // StructuredGenerate outputs a pod disruption budget object using the configured fields.
 func (s *PodDisruptionBudgetV1Generator) StructuredGenerate() (runtime.Object, error) {
+	if len(s.MinAvailable) == 0 {
+		// defaulting behavior seen in Kubernetes 1.6 and below.
+		s.MinAvailable = "1"
+	}
+
 	if err := s.validate(); err != nil {
 		return nil, err
 	}
@@ -75,12 +81,13 @@ func (s *PodDisruptionBudgetV1Generator) StructuredGenerate() (runtime.Object, e
 		return nil, err
 	}
 
+	minAvailable := intstr.Parse(s.MinAvailable)
 	return &policy.PodDisruptionBudget{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: s.Name,
 		},
 		Spec: policy.PodDisruptionBudgetSpec{
-			MinAvailable: intstr.Parse(s.MinAvailable),
+			MinAvailable: &minAvailable,
 			Selector:     selector,
 		},
 	}, nil
@@ -95,7 +102,119 @@ func (s *PodDisruptionBudgetV1Generator) validate() error {
 		return fmt.Errorf("a selector must be specified")
 	}
 	if len(s.MinAvailable) == 0 {
-		return fmt.Errorf("the minimim number of available pods required must be specified")
+		return fmt.Errorf("the minimum number of available pods required must be specified")
+	}
+	return nil
+}
+
+// PodDisruptionBudgetV2Generator supports stable generation of a pod disruption budget.
+type PodDisruptionBudgetV2Generator struct {
+	Name           string
+	MinAvailable   string
+	MaxUnavailable string
+	Selector       string
+}
+
+// Ensure it supports the generator pattern that uses parameters specified during construction.
+var _ StructuredGenerator = &PodDisruptionBudgetV2Generator{}
+
+func (PodDisruptionBudgetV2Generator) ParamNames() []GeneratorParam {
+	return []GeneratorParam{
+		{"name", true},
+		{"min-available", false},
+		{"max-unavailable", false},
+		{"selector", false},
+	}
+}
+
+func (s PodDisruptionBudgetV2Generator) Generate(params map[string]interface{}) (runtime.Object, error) {
+	err := ValidateParams(s.ParamNames(), params)
+	if err != nil {
+		return nil, err
+	}
+
+	name, isString := params["name"].(string)
+	if !isString {
+		return nil, fmt.Errorf("expected string, saw %v for 'name'", name)
+	}
+
+	minAvailable, isString := params["min-available"].(string)
+	if !isString {
+		return nil, fmt.Errorf("expected string, found %v", minAvailable)
+	}
+
+	maxUnavailable, isString := params["max-available"].(string)
+	if !isString {
+		return nil, fmt.Errorf("expected string, found %v", maxUnavailable)
+	}
+
+	selector, isString := params["selector"].(string)
+	if !isString {
+		return nil, fmt.Errorf("expected string, found %v", selector)
+	}
+	delegate := &PodDisruptionBudgetV2Generator{Name: name, MinAvailable: minAvailable, MaxUnavailable: maxUnavailable, Selector: selector}
+	return delegate.StructuredGenerate()
+}
+
+// StructuredGenerate outputs a pod disruption budget object using the configured fields.
+func (s *PodDisruptionBudgetV2Generator) StructuredGenerate() (runtime.Object, error) {
+	if err := s.validate(); err != nil {
+		return nil, err
+	}
+
+	selector, err := metav1.ParseToLabelSelector(s.Selector)
+	if err != nil {
+		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{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: s.Name,
+			},
+			Spec: policy.PodDisruptionBudgetSpec{
+				MaxUnavailable: &maxUnavailable,
+				Selector:       selector,
+			},
+		}, nil
+	}
+
+	if len(s.MinAvailable) > 0 {
+		minAvailable := intstr.Parse(s.MinAvailable)
+		return &policy.PodDisruptionBudget{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: s.Name,
+			},
+			Spec: policy.PodDisruptionBudgetSpec{
+				MinAvailable: &minAvailable,
+				Selector:     selector,
+			},
+		}, nil
+	}
+
+	return nil, err
+}
+
+// validate validates required fields are set to support structured generation.
+func (s *PodDisruptionBudgetV2Generator) validate() error {
+	if len(s.Name) == 0 {
+		return fmt.Errorf("name must be specified")
+	}
+	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("exactly one of min-available and max-available must be specified")
 	}
 	return nil
 }

pkg/printers/internalversion/describe.go

@@ -2881,7 +2881,13 @@ func describePodDisruptionBudget(pdb *policy.PodDisruptionBudget, events *api.Ev
 	return tabbedString(func(out io.Writer) error {
 		w := NewPrefixWriter(out)
 		w.Write(LEVEL_0, "Name:\t%s\n", pdb.Name)
-		w.Write(LEVEL_0, "Min available:\t%s\n", pdb.Spec.MinAvailable.String())
+
+		if pdb.Spec.MinAvailable != nil {
+			w.Write(LEVEL_0, "Min available:\t%s\n", pdb.Spec.MinAvailable.String())
+		} else if pdb.Spec.MaxUnavailable != nil {
+			w.Write(LEVEL_0, "Max unavailable:\t%s\n", pdb.Spec.MaxUnavailable.String())
+		}
+
 		if pdb.Spec.Selector != nil {
 			w.Write(LEVEL_0, "Selector:\t%s\n", metav1.FormatLabelSelector(pdb.Spec.Selector))
 		} else {

pkg/printers/internalversion/describe_test.go

@@ -774,6 +774,7 @@ func TestDescribeStorageClass(t *testing.T) {
 }
 
 func TestDescribePodDisruptionBudget(t *testing.T) {
+	minAvailable := intstr.FromInt(22)
 	f := fake.NewSimpleClientset(&policy.PodDisruptionBudget{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace:         "ns1",
@@ -781,7 +782,7 @@ func TestDescribePodDisruptionBudget(t *testing.T) {
 			CreationTimestamp: metav1.Time{Time: time.Now().Add(1.9e9)},
 		},
 		Spec: policy.PodDisruptionBudgetSpec{
-			MinAvailable: intstr.FromInt(22),
+			MinAvailable: &minAvailable,
 		},
 		Status: policy.PodDisruptionBudgetStatus{
 			PodDisruptionsAllowed: 5,

pkg/printers/internalversion/printers.go

@@ -55,7 +55,7 @@ var (
 	podColumns                       = []string{"NAME", "READY", "STATUS", "RESTARTS", "AGE"}
 	podWideColumns                   = []string{"IP", "NODE"}
 	podTemplateColumns               = []string{"TEMPLATE", "CONTAINER(S)", "IMAGE(S)", "PODLABELS"}
-	podDisruptionBudgetColumns       = []string{"NAME", "MIN-AVAILABLE", "ALLOWED-DISRUPTIONS", "AGE"}
+	podDisruptionBudgetColumns       = []string{"NAME", "MIN-AVAILABLE", "MAX-UNAVAILABLE", "ALLOWED-DISRUPTIONS", "AGE"}
 	replicationControllerColumns     = []string{"NAME", "DESIRED", "CURRENT", "READY", "AGE"}
 	replicationControllerWideColumns = []string{"CONTAINER(S)", "IMAGE(S)", "SELECTOR"}
 	replicaSetColumns                = []string{"NAME", "DESIRED", "CURRENT", "READY", "AGE"}
@@ -397,7 +397,7 @@ func printPodTemplateList(podList *api.PodTemplateList, w io.Writer, options pri
 }
 
 func printPodDisruptionBudget(pdb *policy.PodDisruptionBudget, w io.Writer, options printers.PrintOptions) error {
-	// name, minavailable, selector
+	// name, minavailable, maxUnavailable, selector
 	name := printers.FormatResourceName(options.Kind, pdb.Name, options.WithKind)
 	namespace := pdb.Namespace
 
@@ -406,9 +406,25 @@ func printPodDisruptionBudget(pdb *policy.PodDisruptionBudget, w io.Writer, opti
 			return err
 		}
 	}
-	if _, err := fmt.Fprintf(w, "%s\t%s\t%d\t%s\n",
+
+	var minAvailable string
+	var maxUnavailable string
+	if pdb.Spec.MinAvailable != nil {
+		minAvailable = pdb.Spec.MinAvailable.String()
+	} else {
+		minAvailable = "N/A"
+	}
+
+	if pdb.Spec.MaxUnavailable != nil {
+		maxUnavailable = pdb.Spec.MaxUnavailable.String()
+	} else {
+		maxUnavailable = "N/A"
+	}
+
+	if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%d\t%s\n",
 		name,
-		pdb.Spec.MinAvailable.String(),
+		minAvailable,
+		maxUnavailable,
 		pdb.Status.PodDisruptionsAllowed,
 		translateTimestamp(pdb.CreationTimestamp),
 	); err != nil {

pkg/printers/internalversion/printers_test.go

@@ -2186,6 +2186,7 @@ func TestPrintService(t *testing.T) {
 }
 
 func TestPrintPodDisruptionBudget(t *testing.T) {
+	minAvailable := intstr.FromInt(22)
 	tests := []struct {
 		pdb    policy.PodDisruptionBudget
 		expect string
@@ -2198,13 +2199,13 @@ func TestPrintPodDisruptionBudget(t *testing.T) {
 					CreationTimestamp: metav1.Time{Time: time.Now().Add(1.9e9)},
 				},
 				Spec: policy.PodDisruptionBudgetSpec{
-					MinAvailable: intstr.FromInt(22),
+					MinAvailable: &minAvailable,
 				},
 				Status: policy.PodDisruptionBudgetStatus{
 					PodDisruptionsAllowed: 5,
 				},
 			},
-			"pdb1\t22\t5\t0s\n",
+			"pdb1\t22\tN/A\t5\t0s\n",
 		}}
 
 	buf := bytes.NewBuffer([]byte{})