cmd/create_deployment: refactor & test long function

pkg/kubectl/cmd/create_deployment.go

@@ -23,6 +23,7 @@ import (
 	"github.com/spf13/cobra"
 
 	appsv1beta1 "k8s.io/api/apps/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/kubernetes/pkg/kubectl"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
@@ -62,8 +63,69 @@ func NewCmdCreateDeployment(f cmdutil.Factory, cmdOut, cmdErr io.Writer) *cobra.
 	return cmd
 }
 
-func createDeployment(f cmdutil.Factory, cmdOut, cmdErr io.Writer, cmd *cobra.Command, args []string) error {
+// fallbackGeneratorNameIfNecessary returns the name of the old generator
-	name, err := NameFromCommandArgs(cmd, args)
+// (v1beta1) if server does not support apps/v1beta1 deployments. Otherwise, the
+// generator string is returned unchanged.
+//
+// If the generator name is changed, print a warning message to let the user
+// know.
+func fallbackGeneratorNameIfNecessary(
+	generatorName string,
+	resourcesList []*metav1.APIResourceList,
+	cmdErr io.Writer,
+) string {
+
+	if generatorName == cmdutil.DeploymentBasicAppsV1Beta1GeneratorName &&
+		!contains(resourcesList, appsv1beta1.SchemeGroupVersion.WithResource("deployments")) {
+
+		fmt.Fprintf(cmdErr,
+			"WARNING: New deployments generator %q specified, "+
+				"but apps/v1beta1.Deployments are not available. "+
+				"Falling back to %q.\n",
+			cmdutil.DeploymentBasicAppsV1Beta1GeneratorName,
+			cmdutil.DeploymentBasicV1Beta1GeneratorName,
+		)
+
+		return cmdutil.DeploymentBasicV1Beta1GeneratorName
+	}
+
+	return generatorName
+}
+
+// generatorFromName returns the appropriate StructuredGenerator based on the
+// generatorName. If the generatorName is unrecognized, then return (nil,
+// false).
+func generatorFromName(
+	generatorName string,
+	imageNames []string,
+	deploymentName string,
+) (kubectl.StructuredGenerator, bool) {
+
+	switch generatorName {
+	case cmdutil.DeploymentBasicAppsV1Beta1GeneratorName:
+		return &kubectl.DeploymentBasicAppsGeneratorV1{
+			Name:   deploymentName,
+			Images: imageNames,
+		}, true
+
+	case cmdutil.DeploymentBasicV1Beta1GeneratorName:
+		return &kubectl.DeploymentBasicGeneratorV1{
+			Name:   deploymentName,
+			Images: imageNames,
+		}, true
+	}
+
+	return nil, false
+}
+
+// createDeployment
+// 1. Reads user config values from Cobra.
+// 2. Sets up the correct Generator object.
+// 3. Calls RunCreateSubcommand.
+func createDeployment(f cmdutil.Factory, cmdOut, cmdErr io.Writer,
+	cmd *cobra.Command, args []string) error {
+
+	deploymentName, err := NameFromCommandArgs(cmd, args)
 	if err != nil {
 		return err
 	}
@@ -77,25 +139,21 @@ func createDeployment(f cmdutil.Factory, cmdOut, cmdErr io.Writer, cmd *cobra.Co
 	if err != nil {
 		return fmt.Errorf("failed to discover supported resources: %v", err)
 	}
+
 	generatorName := cmdutil.GetFlagString(cmd, "generator")
-	// fallback to the old generator if server does not support apps/v1beta1 deployments
+
-	if generatorName == cmdutil.DeploymentBasicAppsV1Beta1GeneratorName &&
+	// It is possible we have to modify the user-provided generator name if
-		!contains(resourcesList, appsv1beta1.SchemeGroupVersion.WithResource("deployments")) {
+	// the server does not have support for the requested generator.
-		fmt.Fprintf(cmdErr, "WARNING: New deployments generator specified (%s), but apps/v1beta1.Deployments are not available, falling back to the old one (%s).\n",
+	generatorName = fallbackGeneratorNameIfNecessary(generatorName, resourcesList, cmdErr)
-			cmdutil.DeploymentBasicAppsV1Beta1GeneratorName, cmdutil.DeploymentBasicV1Beta1GeneratorName)
+
-		generatorName = cmdutil.DeploymentBasicV1Beta1GeneratorName
+	imageNames := cmdutil.GetFlagStringSlice(cmd, "image")
-	}
+	generator, ok := generatorFromName(generatorName, imageNames, deploymentName)
-	var generator kubectl.StructuredGenerator
+	if !ok {
-	switch generatorName {
-	case cmdutil.DeploymentBasicAppsV1Beta1GeneratorName:
-		generator = &kubectl.DeploymentBasicAppsGeneratorV1{Name: name, Images: cmdutil.GetFlagStringSlice(cmd, "image")}
-	case cmdutil.DeploymentBasicV1Beta1GeneratorName:
-		generator = &kubectl.DeploymentBasicGeneratorV1{Name: name, Images: cmdutil.GetFlagStringSlice(cmd, "image")}
-	default:
 		return errUnsupportedGenerator(cmd, generatorName)
 	}
+
 	return RunCreateSubcommand(f, cmd, cmdOut, &CreateSubcommandOptions{
-		Name:                name,
+		Name:                deploymentName,
 		StructuredGenerator: generator,
 		DryRun:              cmdutil.GetDryRunFlag(cmd),
 		OutputFormat:        cmdutil.GetFlagString(cmd, "output"),

pkg/kubectl/cmd/create_deployment_test.go

@@ -27,9 +27,39 @@ import (
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/client-go/rest/fake"
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/kubectl"
 	cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing"
+	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 )
 
+func Test_generatorFromName(t *testing.T) {
+	const (
+		nonsenseName   = "not-a-real-generator-name"
+		basicName      = cmdutil.DeploymentBasicV1Beta1GeneratorName
+		basicAppsName  = cmdutil.DeploymentBasicAppsV1Beta1GeneratorName
+		deploymentName = "deployment-name"
+	)
+	imageNames := []string{"image-1", "image-2"}
+
+	generator, ok := generatorFromName(nonsenseName, imageNames, deploymentName)
+	assert.Nil(t, generator)
+	assert.False(t, ok)
+
+	generator, ok = generatorFromName(basicName, imageNames, deploymentName)
+	assert.True(t, ok)
+	assert.Equal(t, &kubectl.DeploymentBasicGeneratorV1{
+		Name:   deploymentName,
+		Images: imageNames,
+	}, generator)
+
+	generator, ok = generatorFromName(basicAppsName, imageNames, deploymentName)
+	assert.True(t, ok)
+	assert.Equal(t, &kubectl.DeploymentBasicAppsGeneratorV1{
+		Name:   deploymentName,
+		Images: imageNames,
+	}, generator)
+}
+
 func TestCreateDeployment(t *testing.T) {
 	depName := "jonny-dep"
 	f, tf, _, ns := cmdtesting.NewAPIFactory()