Merge pull request #61288 from ayushpateria/deployment_apps_v1_gen

Automatic merge from submit-queue. 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>.

Add generators for apps/v1 deployments

**What this PR does / why we need it**:
- Adds basic generator `DeploymentBasicAppsGeneratorV1` that is used on running `kubectl create deployment`. 
- Changes default basic generator for `create deployment` to `deployment-basic/apps.v1`.
- Don't fall back in case generator is not available for `create deployment`, instead, return an error.

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

**Special notes for your reviewer**:

**Release note**:
```release-note
Add generators for `apps/v1` deployments.
```

pkg/kubectl/BUILD

@@ -49,6 +49,7 @@ go_test(
         "//pkg/kubectl/util:go_default_library",
         "//pkg/util/pointer:go_default_library",
         "//vendor/github.com/spf13/cobra:go_default_library",
+        "//vendor/k8s.io/api/apps/v1:go_default_library",
         "//vendor/k8s.io/api/apps/v1beta1:go_default_library",
         "//vendor/k8s.io/api/autoscaling/v1:go_default_library",
         "//vendor/k8s.io/api/batch/v1:go_default_library",

pkg/kubectl/cmd/BUILD

@@ -117,7 +117,6 @@ go_library(
         "//vendor/k8s.io/api/batch/v1:go_default_library",
         "//vendor/k8s.io/api/batch/v1beta1: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/apimachinery/pkg/api/errors:go_default_library",

pkg/kubectl/cmd/create_deployment.go

@@ -55,7 +55,7 @@ func NewCmdCreateDeployment(f cmdutil.Factory, cmdOut, cmdErr io.Writer) *cobra.
 	cmdutil.AddApplyAnnotationFlags(cmd)
 	cmdutil.AddValidateFlags(cmd)
 	cmdutil.AddPrinterFlags(cmd)
-	cmdutil.AddGeneratorFlags(cmd, cmdutil.DeploymentBasicV1Beta1GeneratorName)
+	cmdutil.AddGeneratorFlags(cmd, "")
 	cmd.Flags().StringSlice("image", []string{}, "Image name to run.")
 	cmd.MarkFlagRequired("image")
 	return cmd
@@ -71,7 +71,7 @@ func generatorFromName(
 ) (kubectl.StructuredGenerator, bool) {
 
 	switch generatorName {
-	case cmdutil.DeploymentBasicAppsV1Beta1GeneratorName:
+	case cmdutil.DeploymentBasicAppsV1GeneratorName:
 		generator := &kubectl.DeploymentBasicAppsGeneratorV1{
 			BaseDeploymentGenerator: kubectl.BaseDeploymentGenerator{
 				Name:   deploymentName,
@@ -80,6 +80,15 @@ func generatorFromName(
 		}
 		return generator, true
 
+	case cmdutil.DeploymentBasicAppsV1Beta1GeneratorName:
+		generator := &kubectl.DeploymentBasicAppsGeneratorV1Beta1{
+			BaseDeploymentGenerator: kubectl.BaseDeploymentGenerator{
+				Name:   deploymentName,
+				Images: imageNames,
+			},
+		}
+		return generator, true
+
 	case cmdutil.DeploymentBasicV1Beta1GeneratorName:
 		generator := &kubectl.DeploymentBasicGeneratorV1{
 			BaseDeploymentGenerator: kubectl.BaseDeploymentGenerator{
@@ -112,11 +121,17 @@ func createDeployment(f cmdutil.Factory, cmdOut, cmdErr io.Writer,
 
 	generatorName := cmdutil.GetFlagString(cmd, "generator")
 
-	// It is possible we have to modify the user-provided generator name if
-	// the server does not have support for the requested generator.
-	generatorName, err = cmdutil.FallbackGeneratorNameIfNecessary(generatorName, clientset.Discovery(), cmdErr)
-	if err != nil {
-		return err
+	if len(generatorName) == 0 {
+		generatorName = cmdutil.DeploymentBasicAppsV1GeneratorName
+		generatorNameTemp, err := cmdutil.FallbackGeneratorNameIfNecessary(generatorName, clientset.Discovery(), cmdErr)
+		if err != nil {
+			return err
+		}
+		if generatorNameTemp != generatorName {
+			cmdutil.Warning(cmdErr, generatorName, generatorNameTemp)
+		} else {
+			generatorName = generatorNameTemp
+		}
 	}
 
 	imageNames := cmdutil.GetFlagStringSlice(cmd, "image")

pkg/kubectl/cmd/create_deployment_test.go

@@ -34,10 +34,11 @@ import (
 
 func Test_generatorFromName(t *testing.T) {
 	const (
-		nonsenseName   = "not-a-real-generator-name"
-		basicName      = cmdutil.DeploymentBasicV1Beta1GeneratorName
-		basicAppsName  = cmdutil.DeploymentBasicAppsV1Beta1GeneratorName
-		deploymentName = "deployment-name"
+		nonsenseName         = "not-a-real-generator-name"
+		basicName            = cmdutil.DeploymentBasicV1Beta1GeneratorName
+		basicAppsV1Beta1Name = cmdutil.DeploymentBasicAppsV1Beta1GeneratorName
+		basicAppsV1Name      = cmdutil.DeploymentBasicAppsV1GeneratorName
+		deploymentName       = "deployment-name"
 	)
 	imageNames := []string{"image-1", "image-2"}
 
@@ -58,7 +59,20 @@ func Test_generatorFromName(t *testing.T) {
 		assert.Equal(t, expectedGenerator, generator)
 	}
 
-	generator, ok = generatorFromName(basicAppsName, imageNames, deploymentName)
+	generator, ok = generatorFromName(basicAppsV1Beta1Name, imageNames, deploymentName)
+	assert.True(t, ok)
+
+	{
+		expectedGenerator := &kubectl.DeploymentBasicAppsGeneratorV1Beta1{
+			BaseDeploymentGenerator: kubectl.BaseDeploymentGenerator{
+				Name:   deploymentName,
+				Images: imageNames,
+			},
+		}
+		assert.Equal(t, expectedGenerator, generator)
+	}
+
+	generator, ok = generatorFromName(basicAppsV1Name, imageNames, deploymentName)
 	assert.True(t, ok)
 
 	{
@@ -78,13 +92,13 @@ func TestCreateDeployment(t *testing.T) {
 	defer tf.Cleanup()
 
 	ns := legacyscheme.Codecs
-
+	fakeDiscovery := "{\"kind\":\"APIResourceList\",\"apiVersion\":\"v1\",\"groupVersion\":\"apps/v1\",\"resources\":[{\"name\":\"deployments\",\"singularName\":\"\",\"namespaced\":true,\"kind\":\"Deployment\",\"verbs\":[\"create\",\"delete\",\"deletecollection\",\"get\",\"list\",\"patch\",\"update\",\"watch\"],\"shortNames\":[\"deploy\"],\"categories\":[\"all\"]}]}"
 	tf.Client = &fake.RESTClient{
 		NegotiatedSerializer: ns,
 		Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
 			return &http.Response{
 				StatusCode: http.StatusOK,
-				Body:       ioutil.NopCloser(bytes.NewBuffer([]byte("{}"))),
+				Body:       ioutil.NopCloser(bytes.NewBuffer([]byte(fakeDiscovery))),
 			}, nil
 		}),
 	}
@@ -97,7 +111,7 @@ func TestCreateDeployment(t *testing.T) {
 	cmd.Flags().Set("output", "name")
 	cmd.Flags().Set("image", "hollywood/jonny.depp:v2")
 	cmd.Run(cmd, []string{depName})
-	expectedOutput := "deployment.extensions/" + depName + "\n"
+	expectedOutput := "deployment.apps/" + depName + "\n"
 	if buf.String() != expectedOutput {
 		t.Errorf("expected output: %s, but got: %s", expectedOutput, buf.String())
 	}

pkg/kubectl/cmd/run.go

@@ -23,9 +23,6 @@ import (
 	"github.com/docker/distribution/reference"
 	"github.com/spf13/cobra"
 
-	batchv1 "k8s.io/api/batch/v1"
-	batchv1beta1 "k8s.io/api/batch/v1beta1"
-	extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -218,48 +215,28 @@ func RunRun(f cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer, cmd *c
 	generatorName := cmdutil.GetFlagString(cmd, "generator")
 	schedule := cmdutil.GetFlagString(cmd, "schedule")
 	if len(schedule) != 0 && len(generatorName) == 0 {
-		hasResource, err := cmdutil.HasResource(clientset.Discovery(), batchv1beta1.SchemeGroupVersion.WithResource("cronjobs"))
-		if err != nil {
-			return err
-		}
-		if hasResource {
-			generatorName = cmdutil.CronJobV1Beta1GeneratorName
-		} else {
-			generatorName = cmdutil.CronJobV2Alpha1GeneratorName
-		}
+		generatorName = cmdutil.CronJobV1Beta1GeneratorName
 	}
 	if len(generatorName) == 0 {
 		switch restartPolicy {
 		case api.RestartPolicyAlways:
-			// TODO: we need to deprecate this along with extensions/v1beta1.Deployments
-			// in favor of the new generator for apps/v1beta1.Deployments
-			hasResource, err := cmdutil.HasResource(clientset.Discovery(), extensionsv1beta1.SchemeGroupVersion.WithResource("deployments"))
-			if err != nil {
-				return err
-			}
-			if hasResource {
-				generatorName = cmdutil.DeploymentV1Beta1GeneratorName
-			} else {
-				generatorName = cmdutil.RunV1GeneratorName
-			}
+			generatorName = cmdutil.DeploymentV1Beta1GeneratorName
 		case api.RestartPolicyOnFailure:
-			hasResource, err := cmdutil.HasResource(clientset.Discovery(), batchv1.SchemeGroupVersion.WithResource("jobs"))
-			if err != nil {
-				return err
-			}
-			if hasResource {
-				generatorName = cmdutil.JobV1GeneratorName
-			} else {
-				generatorName = cmdutil.RunPodV1GeneratorName
-			}
+			generatorName = cmdutil.JobV1GeneratorName
 		case api.RestartPolicyNever:
 			generatorName = cmdutil.RunPodV1GeneratorName
 		}
-	}
 
-	generatorName, err = cmdutil.FallbackGeneratorNameIfNecessary(generatorName, clientset.Discovery(), cmdErr)
-	if err != nil {
-		return err
+		// Falling back because the generator was not provided and the default one could be unavailable.
+		generatorNameTemp, err := cmdutil.FallbackGeneratorNameIfNecessary(generatorName, clientset.Discovery(), cmdErr)
+		if err != nil {
+			return err
+		}
+		if generatorNameTemp != generatorName {
+			cmdutil.Warning(cmdErr, generatorName, generatorNameTemp)
+		} else {
+			generatorName = generatorNameTemp
+		}
 	}
 
 	generators := f.Generators("run")

pkg/kubectl/cmd/util/factory_client_access.go

@@ -465,6 +465,7 @@ const (
 	DeploymentAppsV1Beta1GeneratorName      = "deployment/apps.v1beta1"
 	DeploymentBasicV1Beta1GeneratorName     = "deployment-basic/v1beta1"
 	DeploymentBasicAppsV1Beta1GeneratorName = "deployment-basic/apps.v1beta1"
+	DeploymentBasicAppsV1GeneratorName      = "deployment-basic/apps.v1"
 	JobV1GeneratorName                      = "job/v1"
 	CronJobV2Alpha1GeneratorName            = "cronjob/v2alpha1"
 	CronJobV1Beta1GeneratorName             = "cronjob/v1beta1"
@@ -506,9 +507,10 @@ func DefaultGenerators(cmdName string) map[string]kubectl.Generator {
 	case "deployment":
 		// Create Deployment has only StructuredGenerators and no
 		// param-based Generators.
-		// The StructuredGenerators are as follows (as of 2017-07-17):
+		// The StructuredGenerators are as follows (as of 2018-03-16):
 		// DeploymentBasicV1Beta1GeneratorName -> kubectl.DeploymentBasicGeneratorV1
-		// DeploymentBasicAppsV1Beta1GeneratorName -> kubectl.DeploymentBasicAppsGeneratorV1
+		// DeploymentBasicAppsV1Beta1GeneratorName -> kubectl.DeploymentBasicAppsGeneratorV1Beta1
+		// DeploymentBasicAppsV1GeneratorName -> kubectl.DeploymentBasicAppsGeneratorV1
 		generator = map[string]kubectl.Generator{}
 	case "run":
 		generator = map[string]kubectl.Generator{
@@ -557,30 +559,68 @@ func FallbackGeneratorNameIfNecessary(
 	cmdErr io.Writer,
 ) (string, error) {
 	switch generatorName {
+	case DeploymentAppsV1Beta1GeneratorName:
+		hasResource, err := HasResource(discoveryClient, appsv1beta1.SchemeGroupVersion.WithResource("deployments"))
+		if err != nil {
+			return "", err
+		}
+		if !hasResource {
+			return FallbackGeneratorNameIfNecessary(DeploymentV1Beta1GeneratorName, discoveryClient, cmdErr)
+		}
+	case DeploymentV1Beta1GeneratorName:
+		hasResource, err := HasResource(discoveryClient, extensionsv1beta1.SchemeGroupVersion.WithResource("deployments"))
+		if err != nil {
+			return "", err
+		}
+		if !hasResource {
+			return RunV1GeneratorName, nil
+		}
+	case DeploymentBasicAppsV1GeneratorName:
+		hasResource, err := HasResource(discoveryClient, appsv1.SchemeGroupVersion.WithResource("deployments"))
+		if err != nil {
+			return "", err
+		}
+		if !hasResource {
+			return FallbackGeneratorNameIfNecessary(DeploymentBasicAppsV1Beta1GeneratorName, discoveryClient, cmdErr)
+		}
 	case DeploymentBasicAppsV1Beta1GeneratorName:
 		hasResource, err := HasResource(discoveryClient, appsv1beta1.SchemeGroupVersion.WithResource("deployments"))
 		if err != nil {
 			return "", err
 		}
 		if !hasResource {
-			warning(cmdErr, DeploymentBasicAppsV1Beta1GeneratorName, DeploymentBasicV1Beta1GeneratorName)
 			return DeploymentBasicV1Beta1GeneratorName, nil
 		}
+	case JobV1GeneratorName:
+		hasResource, err := HasResource(discoveryClient, batchv1.SchemeGroupVersion.WithResource("jobs"))
+		if err != nil {
+			return "", err
+		}
+		if !hasResource {
+			return RunPodV1GeneratorName, nil
+		}
+	case CronJobV1Beta1GeneratorName:
+		hasResource, err := HasResource(discoveryClient, batchv1beta1.SchemeGroupVersion.WithResource("cronjobs"))
+		if err != nil {
+			return "", err
+		}
+		if !hasResource {
+			return FallbackGeneratorNameIfNecessary(CronJobV2Alpha1GeneratorName, discoveryClient, cmdErr)
+		}
 	case CronJobV2Alpha1GeneratorName:
 		hasResource, err := HasResource(discoveryClient, batchv2alpha1.SchemeGroupVersion.WithResource("cronjobs"))
 		if err != nil {
 			return "", err
 		}
 		if !hasResource {
-			warning(cmdErr, CronJobV2Alpha1GeneratorName, JobV1GeneratorName)
 			return JobV1GeneratorName, nil
 		}
 	}
 	return generatorName, nil
 }
 
-func warning(cmdErr io.Writer, newGeneratorName, oldGeneratorName string) {
-	fmt.Fprintf(cmdErr, "WARNING: New deployments generator %q specified, "+
+func Warning(cmdErr io.Writer, newGeneratorName, oldGeneratorName string) {
+	fmt.Fprintf(cmdErr, "WARNING: New generator %q specified, "+
 		"but it isn't available. "+
 		"Falling back to %q.\n",
 		newGeneratorName,

pkg/kubectl/deployment.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"strings"
 
+	appsv1 "k8s.io/api/apps/v1"
 	appsv1beta1 "k8s.io/api/apps/v1beta1"
 	"k8s.io/api/core/v1"
 	extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
@@ -28,7 +29,7 @@ import (
 )
 
 // BaseDeploymentGenerator: implement the common functionality of
-// DeploymentBasicGeneratorV1 and DeploymentBasicAppsGeneratorV1. To reduce
+// DeploymentBasicGeneratorV1, DeploymentBasicAppsGeneratorV1Beta1 and DeploymentBasicAppsGeneratorV1. To reduce
 // confusion, it's best to keep this struct in the same file as those
 // generators.
 type BaseDeploymentGenerator struct {
@@ -117,16 +118,16 @@ func (s *DeploymentBasicGeneratorV1) StructuredGenerate() (runtime.Object, error
 	}, err
 }
 
-// DeploymentBasicAppsGeneratorV1 supports stable generation of a deployment under apps/v1beta1 endpoint
-type DeploymentBasicAppsGeneratorV1 struct {
+// DeploymentBasicAppsGeneratorV1Beta1 supports stable generation of a deployment under apps/v1beta1 endpoint
+type DeploymentBasicAppsGeneratorV1Beta1 struct {
 	BaseDeploymentGenerator
 }
 
 // Ensure it supports the generator pattern that uses parameters specified during construction
-var _ StructuredGenerator = &DeploymentBasicAppsGeneratorV1{}
+var _ StructuredGenerator = &DeploymentBasicAppsGeneratorV1Beta1{}
 
 // StructuredGenerate outputs a deployment object using the configured fields
-func (s *DeploymentBasicAppsGeneratorV1) StructuredGenerate() (runtime.Object, error) {
+func (s *DeploymentBasicAppsGeneratorV1Beta1) StructuredGenerate() (runtime.Object, error) {
 	podSpec, labels, selector, err := s.structuredGenerate()
 	one := int32(1)
 	return &appsv1beta1.Deployment{
@@ -146,3 +147,33 @@ func (s *DeploymentBasicAppsGeneratorV1) StructuredGenerate() (runtime.Object, e
 		},
 	}, err
 }
+
+// DeploymentBasicAppsGeneratorV1 supports stable generation of a deployment under apps/v1 endpoint
+type DeploymentBasicAppsGeneratorV1 struct {
+	BaseDeploymentGenerator
+}
+
+// Ensure it supports the generator pattern that uses parameters specified during construction
+var _ StructuredGenerator = &DeploymentBasicAppsGeneratorV1{}
+
+// StructuredGenerate outputs a deployment object using the configured fields
+func (s *DeploymentBasicAppsGeneratorV1) StructuredGenerate() (runtime.Object, error) {
+	podSpec, labels, selector, err := s.structuredGenerate()
+	one := int32(1)
+	return &appsv1.Deployment{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:   s.Name,
+			Labels: labels,
+		},
+		Spec: appsv1.DeploymentSpec{
+			Replicas: &one,
+			Selector: &selector,
+			Template: v1.PodTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: labels,
+				},
+				Spec: podSpec,
+			},
+		},
+	}, err
+}

pkg/kubectl/deployment_test.go

@@ -20,8 +20,8 @@ import (
 	"reflect"
 	"testing"
 
+	appsv1 "k8s.io/api/apps/v1"
 	"k8s.io/api/core/v1"
-	extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -31,19 +31,19 @@ func TestDeploymentBasicGenerate(t *testing.T) {
 		name           string
 		deploymentName string
 		images         []string
-		expected       *extensionsv1beta1.Deployment
+		expected       *appsv1.Deployment
 		expectErr      bool
 	}{
 		{
 			name:           "deployment name and images ok",
 			deploymentName: "images-name-ok",
 			images:         []string{"nn/image1", "registry/nn/image2", "nn/image3:tag", "nn/image4@digest", "nn/image5@sha256:digest"},
-			expected: &extensionsv1beta1.Deployment{
+			expected: &appsv1.Deployment{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:   "images-name-ok",
 					Labels: map[string]string{"app": "images-name-ok"},
 				},
-				Spec: extensionsv1beta1.DeploymentSpec{
+				Spec: appsv1.DeploymentSpec{
 					Replicas: &one,
 					Selector: &metav1.LabelSelector{
 						MatchLabels: map[string]string{"app": "images-name-ok"},
@@ -83,7 +83,7 @@ func TestDeploymentBasicGenerate(t *testing.T) {
 		},
 	}
 	for _, test := range tests {
-		generator := &DeploymentBasicGeneratorV1{
+		generator := &DeploymentBasicAppsGeneratorV1{
 			BaseDeploymentGenerator{
 				Name:   test.deploymentName,
 				Images: test.images,
@@ -96,8 +96,8 @@ func TestDeploymentBasicGenerate(t *testing.T) {
 		if test.expectErr && err != nil {
 			continue
 		}
-		if !reflect.DeepEqual(obj.(*extensionsv1beta1.Deployment), test.expected) {
-			t.Errorf("test: %v\nexpected:\n%#v\nsaw:\n%#v", test.name, test.expected, obj.(*extensionsv1beta1.Deployment))
+		if !reflect.DeepEqual(obj.(*appsv1.Deployment), test.expected) {
+			t.Errorf("test: %v\nexpected:\n%#v\nsaw:\n%#v", test.name, test.expected, obj.(*appsv1.Deployment))
 		}
 	}
 }