diff --git a/hack/make-rules/test-cmd-util.sh b/hack/make-rules/test-cmd-util.sh index d849c9643c4..5fe2ee218d5 100644 --- a/hack/make-rules/test-cmd-util.sh +++ b/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. diff --git a/hack/verify-flags/known-flags.txt b/hack/verify-flags/known-flags.txt index f1b8b8d82f8..ced48ffa7b7 100644 --- a/hack/verify-flags/known-flags.txt +++ b/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 diff --git a/pkg/kubectl/cmd/create_pdb.go b/pkg/kubectl/cmd/create_pdb.go index fcbced118d1..dd282a4647f 100644 --- a/pkg/kubectl/cmd/create_pdb.go +++ b/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)) } diff --git a/pkg/kubectl/cmd/util/factory_client_access.go b/pkg/kubectl/cmd/util/factory_client_access.go index 30cab5206d9..186a0235c16 100644 --- a/pkg/kubectl/cmd/util/factory_client_access.go +++ b/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 diff --git a/pkg/kubectl/pdb.go b/pkg/kubectl/pdb.go index 04bc7688c0f..43f54c4a7c6 100644 --- a/pkg/kubectl/pdb.go +++ b/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 } diff --git a/pkg/printers/internalversion/describe.go b/pkg/printers/internalversion/describe.go index 7385344f87e..5135b231dae 100644 --- a/pkg/printers/internalversion/describe.go +++ b/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 { diff --git a/pkg/printers/internalversion/describe_test.go b/pkg/printers/internalversion/describe_test.go index 7671177df80..f9202866a58 100644 --- a/pkg/printers/internalversion/describe_test.go +++ b/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, diff --git a/pkg/printers/internalversion/printers.go b/pkg/printers/internalversion/printers.go index 4a393eef637..2f5228bda30 100644 --- a/pkg/printers/internalversion/printers.go +++ b/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 { diff --git a/pkg/printers/internalversion/printers_test.go b/pkg/printers/internalversion/printers_test.go index d801c452e46..ff940fb278e 100644 --- a/pkg/printers/internalversion/printers_test.go +++ b/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{})