Merge pull request #97429 from sachinkumarsingh092/refactor-command-kubectl-create-pdb

Remove the dependency between create poddisruptionbudget and generators

staging/src/k8s.io/kubectl/pkg/cmd/create/BUILD

@@ -30,6 +30,7 @@ go_library(
         "//staging/src/k8s.io/api/batch/v1beta1:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/api/networking/v1:go_default_library",
+        "//staging/src/k8s.io/api/policy/v1beta1:go_default_library",
         "//staging/src/k8s.io/api/rbac/v1:go_default_library",
         "//staging/src/k8s.io/api/scheduling/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library",
@@ -51,6 +52,7 @@ go_library(
         "//staging/src/k8s.io/client-go/kubernetes/typed/batch/v1beta1:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes/typed/networking/v1:go_default_library",
+        "//staging/src/k8s.io/client-go/kubernetes/typed/policy/v1beta1:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes/typed/rbac/v1:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes/typed/scheduling/v1:go_default_library",
         "//staging/src/k8s.io/component-base/cli/flag:go_default_library",
@@ -97,6 +99,7 @@ go_test(
         "//staging/src/k8s.io/api/batch/v1beta1:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/api/networking/v1:go_default_library",
+        "//staging/src/k8s.io/api/policy/v1beta1:go_default_library",
         "//staging/src/k8s.io/api/rbac/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",

staging/src/k8s.io/kubectl/pkg/cmd/create/create_pdb.go

@@ -17,12 +17,22 @@ limitations under the License.
 package create
 
 import (
+	"context"
+	"fmt"
+	"regexp"
+
 	"github.com/spf13/cobra"
 
+	policyv1beta1 "k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
+	resourcecli "k8s.io/cli-runtime/pkg/resource"
+	policyclient "k8s.io/client-go/kubernetes/typed/policy/v1beta1"
 	cmdutil "k8s.io/kubectl/pkg/cmd/util"
-	"k8s.io/kubectl/pkg/generate"
+	"k8s.io/kubectl/pkg/scheme"
-	generateversioned "k8s.io/kubectl/pkg/generate/versioned"
+	"k8s.io/kubectl/pkg/util"
 	"k8s.io/kubectl/pkg/util/i18n"
 	"k8s.io/kubectl/pkg/util/templates"
 )
@@ -43,14 +53,40 @@ var (
 
 // PodDisruptionBudgetOpts holds the command-line options for poddisruptionbudget sub command
 type PodDisruptionBudgetOpts struct {
-	CreateSubcommandOptions *CreateSubcommandOptions
+	// PrintFlags holds options necessary for obtaining a printer
+	PrintFlags *genericclioptions.PrintFlags
+	PrintObj   func(obj runtime.Object) error
+	// Name of resource being created
+	Name string
+
+	MinAvailable   string
+	MaxUnavailable string
+
+	// A label selector to use for this budget
+	Selector         string
+	CreateAnnotation bool
+	FieldManager     string
+	Namespace        string
+	EnforceNamespace bool
+
+	Client         *policyclient.PolicyV1beta1Client
+	DryRunStrategy cmdutil.DryRunStrategy
+	DryRunVerifier *resourcecli.DryRunVerifier
+
+	genericclioptions.IOStreams
+}
+
+// NewPodDisruptionBudgetOpts creates a new *PodDisruptionBudgetOpts with sane defaults
+func NewPodDisruptionBudgetOpts(ioStreams genericclioptions.IOStreams) *PodDisruptionBudgetOpts {
+	return &PodDisruptionBudgetOpts{
+		PrintFlags: genericclioptions.NewPrintFlags("created").WithTypeSetter(scheme.Scheme),
+		IOStreams:  ioStreams,
+	}
 }
 
 // NewCmdCreatePodDisruptionBudget is a macro command to create a new pod disruption budget.
 func NewCmdCreatePodDisruptionBudget(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobra.Command {
-	options := &PodDisruptionBudgetOpts{
+	o := NewPodDisruptionBudgetOpts(ioStreams)
-		CreateSubcommandOptions: NewCreateSubcommandOptions(ioStreams),
-	}
 
 	cmd := &cobra.Command{
 		Use:                   "poddisruptionbudget NAME --selector=SELECTOR --min-available=N [--dry-run=server|client|none]",
@@ -60,54 +96,172 @@ func NewCmdCreatePodDisruptionBudget(f cmdutil.Factory, ioStreams genericcliopti
 		Long:                  pdbLong,
 		Example:               pdbExample,
 		Run: func(cmd *cobra.Command, args []string) {
-			cmdutil.CheckErr(options.Complete(f, cmd, args))
+			cmdutil.CheckErr(o.Complete(f, cmd, args))
-			cmdutil.CheckErr(options.Run())
+			cmdutil.CheckErr(o.Validate())
+			cmdutil.CheckErr(o.Run())
 		},
 	}
 
-	options.CreateSubcommandOptions.PrintFlags.AddFlags(cmd)
+	o.PrintFlags.AddFlags(cmd)
 
 	cmdutil.AddApplyAnnotationFlags(cmd)
 	cmdutil.AddValidateFlags(cmd)
-	cmdutil.AddGeneratorFlags(cmd, generateversioned.PodDisruptionBudgetV2GeneratorName)
+	cmdutil.AddDryRunFlag(cmd)
 
-	cmd.Flags().String("min-available", "", i18n.T("The minimum number or percentage of available pods this budget requires."))
+	cmd.Flags().StringVar(&o.MinAvailable, "min-available", o.MinAvailable, 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().StringVar(&o.MaxUnavailable, "max-unavailable", o.MaxUnavailable, 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."))
+	cmd.Flags().StringVar(&o.Selector, "selector", o.Selector, i18n.T("A label selector to use for this budget. Only equality-based selector requirements are supported."))
-	cmdutil.AddFieldManagerFlagVar(cmd, &options.CreateSubcommandOptions.FieldManager, "kubectl-create")
+	cmdutil.AddFieldManagerFlagVar(cmd, &o.FieldManager, "kubectl-create")
 	return cmd
 }
 
 // Complete completes all the required options
 func (o *PodDisruptionBudgetOpts) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error {
-	name, err := NameFromCommandArgs(cmd, args)
+	var err error
+	o.Name, err = NameFromCommandArgs(cmd, args)
 	if err != nil {
 		return err
 	}
 
-	var generator generate.StructuredGenerator
+	restConfig, err := f.ToRESTConfig()
-	switch generatorName := cmdutil.GetFlagString(cmd, "generator"); generatorName {
+	if err != nil {
-	case generateversioned.PodDisruptionBudgetV1GeneratorName:
+		return err
-		generator = &generateversioned.PodDisruptionBudgetV1Generator{
-			Name:         name,
-			MinAvailable: cmdutil.GetFlagString(cmd, "min-available"),
-			Selector:     cmdutil.GetFlagString(cmd, "selector"),
 	}
-	case generateversioned.PodDisruptionBudgetV2GeneratorName:
+	o.Client, err = policyclient.NewForConfig(restConfig)
-		generator = &generateversioned.PodDisruptionBudgetV2Generator{
+	if err != nil {
-			Name:           name,
+		return err
-			MinAvailable:   cmdutil.GetFlagString(cmd, "min-available"),
-			MaxUnavailable: cmdutil.GetFlagString(cmd, "max-unavailable"),
-			Selector:       cmdutil.GetFlagString(cmd, "selector"),
-		}
-	default:
-		return errUnsupportedGenerator(cmd, generatorName)
 	}
 
-	return o.CreateSubcommandOptions.Complete(f, cmd, args, generator)
+	o.CreateAnnotation = cmdutil.GetFlagBool(cmd, cmdutil.ApplyAnnotationsFlag)
+
+	o.DryRunStrategy, err = cmdutil.GetDryRunStrategy(cmd)
+	if err != nil {
+		return err
+	}
+	dynamicClient, err := f.DynamicClient()
+	if err != nil {
+		return err
+	}
+	discoveryClient, err := f.ToDiscoveryClient()
+	if err != nil {
+		return err
+	}
+	o.DryRunVerifier = resourcecli.NewDryRunVerifier(dynamicClient, discoveryClient)
+
+	o.Namespace, o.EnforceNamespace, err = f.ToRawKubeConfigLoader().Namespace()
+	if err != nil {
+		return err
+	}
+
+	cmdutil.PrintFlagsWithDryRunStrategy(o.PrintFlags, o.DryRunStrategy)
+
+	printer, err := o.PrintFlags.ToPrinter()
+	if err != nil {
+		return err
+	}
+
+	o.PrintObj = func(obj runtime.Object) error {
+		return printer.PrintObj(obj, o.Out)
+	}
+
+	return nil
+}
+
+// Validate checks to the PodDisruptionBudgetOpts to see if there is sufficient information run the command
+func (o *PodDisruptionBudgetOpts) Validate() error {
+	if len(o.Name) == 0 {
+		return fmt.Errorf("name must be specified")
+	}
+
+	if len(o.Selector) == 0 {
+		return fmt.Errorf("a selector must be specified")
+	}
+
+	if len(o.MaxUnavailable) == 0 && len(o.MinAvailable) == 0 {
+		return fmt.Errorf("one of min-available or max-unavailable must be specified")
+	}
+
+	if len(o.MaxUnavailable) > 0 && len(o.MinAvailable) > 0 {
+		return fmt.Errorf("min-available and max-unavailable cannot be both specified")
+	}
+
+	// The following regex matches the following values:
+	// 10, 20, 30%, 50% (number and percentage)
+	// but not 10Gb, 20Mb
+	re := regexp.MustCompile(`^[0-9]+%?$`)
+
+	switch {
+	case len(o.MinAvailable) > 0 && !re.MatchString(o.MinAvailable):
+		return fmt.Errorf("invalid format specified for min-available")
+	case len(o.MaxUnavailable) > 0 && !re.MatchString(o.MaxUnavailable):
+		return fmt.Errorf("invalid format specified for max-unavailable")
+	}
+
+	return nil
 }
 
 // Run calls the CreateSubcommandOptions.Run in PodDisruptionBudgetOpts instance
 func (o *PodDisruptionBudgetOpts) Run() error {
-	return o.CreateSubcommandOptions.Run()
+	podDisruptionBudget, err := o.createPodDisruptionBudgets()
+	if err != nil {
+		return err
+	}
+
+	if err := util.CreateOrUpdateAnnotation(o.CreateAnnotation, podDisruptionBudget, scheme.DefaultJSONEncoder()); err != nil {
+		return err
+	}
+
+	if o.DryRunStrategy != cmdutil.DryRunClient {
+		createOptions := metav1.CreateOptions{}
+		if o.FieldManager != "" {
+			createOptions.FieldManager = o.FieldManager
+		}
+		if o.DryRunStrategy == cmdutil.DryRunServer {
+			if err := o.DryRunVerifier.HasSupport(podDisruptionBudget.GroupVersionKind()); err != nil {
+				return err
+			}
+			createOptions.DryRun = []string{metav1.DryRunAll}
+		}
+		podDisruptionBudget, err = o.Client.PodDisruptionBudgets(o.Namespace).Create(context.TODO(), podDisruptionBudget, createOptions)
+		if err != nil {
+			return fmt.Errorf("failed to create poddisruptionbudgets: %v", err)
+		}
+	}
+	return o.PrintObj(podDisruptionBudget)
+}
+
+func (o *PodDisruptionBudgetOpts) createPodDisruptionBudgets() (*policyv1beta1.PodDisruptionBudget, error) {
+	namespace := ""
+	if o.EnforceNamespace {
+		namespace = o.Namespace
+	}
+
+	podDisruptionBudget := &policyv1beta1.PodDisruptionBudget{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: policyv1beta1.SchemeGroupVersion.String(),
+			Kind:       "PodDisruptionBudget",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      o.Name,
+			Namespace: namespace,
+		},
+	}
+
+	selector, err := metav1.ParseToLabelSelector(o.Selector)
+	if err != nil {
+		return nil, err
+	}
+
+	podDisruptionBudget.Spec.Selector = selector
+
+	switch {
+	case len(o.MinAvailable) > 0:
+		minAvailable := intstr.Parse(o.MinAvailable)
+		podDisruptionBudget.Spec.MinAvailable = &minAvailable
+	case len(o.MaxUnavailable) > 0:
+		maxUnavailable := intstr.Parse(o.MaxUnavailable)
+		podDisruptionBudget.Spec.MaxUnavailable = &maxUnavailable
+	}
+
+	return podDisruptionBudget, nil
 }

staging/src/k8s.io/kubectl/pkg/cmd/create/create_pdb_test.go

@@ -17,69 +17,221 @@ limitations under the License.
 package create
 
 import (
-	"bytes"
-	"io/ioutil"
-	"net/http"
 	"testing"
 
-	"k8s.io/apimachinery/pkg/runtime/schema"
+	policyv1beta1 "k8s.io/api/policy/v1beta1"
-	"k8s.io/cli-runtime/pkg/genericclioptions"
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
-	restclient "k8s.io/client-go/rest"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/client-go/rest/fake"
+	"k8s.io/apimachinery/pkg/util/intstr"
-	cmdtesting "k8s.io/kubectl/pkg/cmd/testing"
-	"k8s.io/kubectl/pkg/scheme"
 )
 
-func TestCreatePdb(t *testing.T) {
+func TestCreatePdbValidation(t *testing.T) {
-	pdbName := "my-pdb"
+	selectorOpts := "app=nginx"
-	tf := cmdtesting.NewTestFactory().WithNamespace("test")
+	podAmountNumber := "3"
-	defer tf.Cleanup()
+	podAmountPercent := "50%"
 
-	ns := scheme.Codecs.WithoutConversion()
+	tests := map[string]struct {
-
+		options  *PodDisruptionBudgetOpts
-	tf.Client = &fake.RESTClient{
+		expected string
-		GroupVersion:         schema.GroupVersion{Group: "policy", Version: "v1beta1"},
+	}{
-		NegotiatedSerializer: ns,
+		"test-missing-name-param": {
-		Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
+			options: &PodDisruptionBudgetOpts{
-			return &http.Response{
+				Selector:     selectorOpts,
-				StatusCode: http.StatusOK,
+				MinAvailable: podAmountNumber,
-				Body:       ioutil.NopCloser(&bytes.Buffer{}),
+			},
-			}, nil
+			expected: "name must be specified",
-		}),
+		},
-	}
+		"test-missing-selector-param": {
-	tf.ClientConfigVal = &restclient.Config{}
+			options: &PodDisruptionBudgetOpts{
-
+				Name:         "my-pdb",
-	outputFormat := "name"
+				MinAvailable: podAmountNumber,
-
+			},
-	ioStreams, _, buf, _ := genericclioptions.NewTestIOStreams()
+			expected: "a selector must be specified",
-	cmd := NewCmdCreatePodDisruptionBudget(tf, ioStreams)
+		},
-	cmd.Flags().Set("min-available", "1")
+		"test-missing-max-unavailable-max-unavailable-param": {
-	cmd.Flags().Set("selector", "app=rails")
+			options: &PodDisruptionBudgetOpts{
-	cmd.Flags().Set("dry-run", "client")
+				Name:     "my-pdb",
-	cmd.Flags().Set("output", outputFormat)
+				Selector: selectorOpts,
-
+			},
-	printFlags := genericclioptions.NewPrintFlags("created").WithTypeSetter(scheme.Scheme)
+			expected: "one of min-available or max-unavailable must be specified",
-	printFlags.OutputFormat = &outputFormat
+		},
-
+		"test-both-min-available-max-unavailable-param": {
-	options := &PodDisruptionBudgetOpts{
+			options: &PodDisruptionBudgetOpts{
-		CreateSubcommandOptions: &CreateSubcommandOptions{
+				Name:           "my-pdb",
-			PrintFlags: printFlags,
+				Selector:       selectorOpts,
-			Name:       pdbName,
+				MinAvailable:   podAmountNumber,
-			IOStreams:  ioStreams,
+				MaxUnavailable: podAmountPercent,
+			},
+			expected: "min-available and max-unavailable cannot be both specified",
+		},
+		"test-invalid-min-available-format": {
+			options: &PodDisruptionBudgetOpts{
+				Name:         "my-pdb",
+				Selector:     selectorOpts,
+				MinAvailable: "10GB",
+			},
+			expected: "invalid format specified for min-available",
+		},
+		"test-invalid-max-unavailable-format": {
+			options: &PodDisruptionBudgetOpts{
+				Name:           "my-pdb",
+				Selector:       selectorOpts,
+				MaxUnavailable: "10GB",
+			},
+			expected: "invalid format specified for max-unavailable",
+		},
+		"test-valid-min-available-format": {
+			options: &PodDisruptionBudgetOpts{
+				Name:           "my-pdb",
+				Selector:       selectorOpts,
+				MaxUnavailable: podAmountNumber,
+			},
+			expected: "",
+		},
+		"test-valid-max-unavailable-format": {
+			options: &PodDisruptionBudgetOpts{
+				Name:           "my-pdb",
+				Selector:       selectorOpts,
+				MaxUnavailable: podAmountPercent,
+			},
+			expected: "",
 		},
 	}
-	err := options.Complete(tf, cmd, []string{pdbName})
+
-	if err != nil {
+	for name, tc := range tests {
-		t.Fatalf("unexpected error: %v", err)
+		t.Run(name, func(t *testing.T) {
+
+			o := &PodDisruptionBudgetOpts{
+				Name:           tc.options.Name,
+				Selector:       tc.options.Selector,
+				MinAvailable:   tc.options.MinAvailable,
+				MaxUnavailable: tc.options.MaxUnavailable,
 			}
 
-	err = options.Run()
+			err := o.Validate()
-	if err != nil {
+			if err != nil && err.Error() != tc.expected {
-		t.Fatalf("unexpected error: %v", err)
+				t.Errorf("unexpected error: %v", err)
 			}
-
+			if tc.expected != "" && err == nil {
-	expectedOutput := "poddisruptionbudget.policy/" + pdbName + "\n"
+				t.Errorf("expected error, got no error")
-	if buf.String() != expectedOutput {
+			}
-		t.Errorf("expected output: %s, but got: %s", expectedOutput, buf.String())
+		})
+	}
+}
+
+func TestCreatePdb(t *testing.T) {
+	selectorOpts := "app=nginx"
+	podAmountNumber := "3"
+	podAmountPercent := "50%"
+
+	selector, err := metav1.ParseToLabelSelector(selectorOpts)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
+	minAvailableNumber := intstr.Parse(podAmountNumber)
+	minAvailablePercent := intstr.Parse(podAmountPercent)
+
+	minUnavailableNumber := intstr.Parse(podAmountNumber)
+	minUnavailablePercent := intstr.Parse(podAmountPercent)
+
+	tests := map[string]struct {
+		options  *PodDisruptionBudgetOpts
+		expected *policyv1beta1.PodDisruptionBudget
+	}{
+		"test-valid-min-available-pods-number": {
+			options: &PodDisruptionBudgetOpts{
+				Name:         "my-pdb",
+				Selector:     selectorOpts,
+				MinAvailable: podAmountNumber,
+			},
+			expected: &policyv1beta1.PodDisruptionBudget{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "PodDisruptionBudget",
+					APIVersion: "policy/v1beta1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "my-pdb",
+				},
+				Spec: policyv1beta1.PodDisruptionBudgetSpec{
+					Selector:     selector,
+					MinAvailable: &minAvailableNumber,
+				},
+			},
+		},
+		"test-valid-min-available-pods-percentage": {
+			options: &PodDisruptionBudgetOpts{
+				Name:         "my-pdb",
+				Selector:     selectorOpts,
+				MinAvailable: podAmountPercent,
+			},
+			expected: &policyv1beta1.PodDisruptionBudget{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "PodDisruptionBudget",
+					APIVersion: "policy/v1beta1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "my-pdb",
+				},
+				Spec: policyv1beta1.PodDisruptionBudgetSpec{
+					Selector:     selector,
+					MinAvailable: &minAvailablePercent,
+				},
+			},
+		},
+		"test-valid-max-unavailable-pods-number": {
+			options: &PodDisruptionBudgetOpts{
+				Name:         "my-pdb",
+				Selector:     selectorOpts,
+				MinAvailable: podAmountNumber,
+			},
+			expected: &policyv1beta1.PodDisruptionBudget{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "PodDisruptionBudget",
+					APIVersion: "policy/v1beta1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "my-pdb",
+				},
+				Spec: policyv1beta1.PodDisruptionBudgetSpec{
+					Selector:     selector,
+					MinAvailable: &minUnavailableNumber,
+				},
+			},
+		},
+		"test-valid-max-unavailable-pods-percentage": {
+			options: &PodDisruptionBudgetOpts{
+				Name:         "my-pdb",
+				Selector:     selectorOpts,
+				MinAvailable: podAmountPercent,
+			},
+			expected: &policyv1beta1.PodDisruptionBudget{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "PodDisruptionBudget",
+					APIVersion: "policy/v1beta1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "my-pdb",
+				},
+				Spec: policyv1beta1.PodDisruptionBudgetSpec{
+					Selector:     selector,
+					MinAvailable: &minUnavailablePercent,
+				},
+			},
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+
+			podDisruptionBudget, err := tc.options.createPodDisruptionBudgets()
+			if err != nil {
+				t.Errorf("unexpected error:\n%#v\n", err)
+				return
+			}
+			if !apiequality.Semantic.DeepEqual(podDisruptionBudget, tc.expected) {
+				t.Errorf("expected:\n%#v\ngot:\n%#v", tc.expected, podDisruptionBudget)
+			}
+		})
 	}
 }