kubeadm: Resolve tech debt; move commonly used funcs to a general package instead of duplicating

cmd/kubeadm/app/BUILD

@@ -46,6 +46,7 @@ filegroup(
         "//cmd/kubeadm/app/phases/markmaster:all-srcs",
         "//cmd/kubeadm/app/phases/selfhosting:all-srcs",
         "//cmd/kubeadm/app/phases/token:all-srcs",
+        "//cmd/kubeadm/app/phases/upgrade:all-srcs",
         "//cmd/kubeadm/app/phases/uploadconfig:all-srcs",
         "//cmd/kubeadm/app/preflight:all-srcs",
         "//cmd/kubeadm/app/util:all-srcs",

cmd/kubeadm/app/cmd/BUILD

@@ -23,6 +23,8 @@ go_library(
         "//cmd/kubeadm/app/apis/kubeadm/v1alpha1:go_default_library",
         "//cmd/kubeadm/app/apis/kubeadm/validation:go_default_library",
         "//cmd/kubeadm/app/cmd/phases:go_default_library",
+        "//cmd/kubeadm/app/cmd/upgrade:go_default_library",
+        "//cmd/kubeadm/app/cmd/util:go_default_library",
         "//cmd/kubeadm/app/constants:go_default_library",
         "//cmd/kubeadm/app/discovery:go_default_library",
         "//cmd/kubeadm/app/features:go_default_library",
@@ -95,6 +97,8 @@ filegroup(
     srcs = [
         ":package-srcs",
         "//cmd/kubeadm/app/cmd/phases:all-srcs",
+        "//cmd/kubeadm/app/cmd/upgrade:all-srcs",
+        "//cmd/kubeadm/app/cmd/util:all-srcs",
     ],
     tags = ["automanaged"],
 )

cmd/kubeadm/app/cmd/cmd.go

@@ -17,7 +17,6 @@ limitations under the License.
 package cmd
 
 import (
-	"fmt"
 	"io"
 
 	"github.com/renstrom/dedent"
@@ -25,6 +24,7 @@ import (
 
 	"k8s.io/apiserver/pkg/util/flag"
 	"k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases"
+	"k8s.io/kubernetes/cmd/kubeadm/app/cmd/upgrade"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 )
 
@@ -75,6 +75,7 @@ func NewKubeadmCommand(f cmdutil.Factory, in io.Reader, out, err io.Writer) *cob
 	cmds.AddCommand(NewCmdReset(out))
 	cmds.AddCommand(NewCmdVersion(out))
 	cmds.AddCommand(NewCmdToken(out, err))
+	cmds.AddCommand(upgrade.NewCmdUpgrade(out))
 
 	// Wrap not yet fully supported commands in an alpha subcommand
 	experimentalCmd := &cobra.Command{
@@ -86,18 +87,3 @@ func NewKubeadmCommand(f cmdutil.Factory, in io.Reader, out, err io.Writer) *cob
 
 	return cmds
 }
-
-// subCmdRunE returns a function that handles a case where a subcommand must be specified
-// Without this callback, if a user runs just the command without a subcommand,
-// or with an invalid subcommand, cobra will print usage information, but still exit cleanly.
-// We want to return an error code in these cases so that the
-// user knows that their command was invalid.
-func subCmdRunE(name string) func(*cobra.Command, []string) error {
-	return func(_ *cobra.Command, args []string) error {
-		if len(args) < 1 {
-			return fmt.Errorf("missing subcommand; %q is not meant to be run on its own", name)
-		}
-
-		return fmt.Errorf("invalid subcommand: %q", args[0])
-	}
-}

cmd/kubeadm/app/cmd/config.go

@@ -26,6 +26,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clientset "k8s.io/client-go/kubernetes"
 	kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
+	cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util"
 	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	"k8s.io/kubernetes/cmd/kubeadm/app/features"
 	"k8s.io/kubernetes/cmd/kubeadm/app/phases/uploadconfig"
@@ -52,7 +53,7 @@ func NewCmdConfig(out io.Writer) *cobra.Command {
 		// cobra will print usage information, but still exit cleanly.
 		// We want to return an error code in these cases so that the
 		// user knows that their command was invalid.
-		RunE: subCmdRunE("config"),
+		RunE: cmdutil.SubCmdRunE("config"),
 	}
 
 	cmd.PersistentFlags().StringVar(&kubeConfigFile, "kubeconfig", "/etc/kubernetes/admin.conf", "The KubeConfig file to use for talking to the cluster")
@@ -67,7 +68,7 @@ func NewCmdConfigUpload(out io.Writer, kubeConfigFile *string) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "upload",
 		Short: "Upload configuration about the current state so 'kubeadm upgrade' later can know how to configure the upgraded cluster",
-		RunE:  subCmdRunE("upload"),
+		RunE:  cmdutil.SubCmdRunE("upload"),
 	}
 
 	cmd.AddCommand(NewCmdConfigUploadFromFile(out, kubeConfigFile))

cmd/kubeadm/app/cmd/phases/BUILD

@@ -25,6 +25,7 @@ go_library(
         "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
         "//cmd/kubeadm/app/apis/kubeadm/v1alpha1:go_default_library",
         "//cmd/kubeadm/app/apis/kubeadm/validation:go_default_library",
+        "//cmd/kubeadm/app/cmd/util:go_default_library",
         "//cmd/kubeadm/app/constants:go_default_library",
         "//cmd/kubeadm/app/features:go_default_library",
         "//cmd/kubeadm/app/phases/bootstraptoken/clusterinfo:go_default_library",
@@ -54,7 +55,6 @@ go_test(
         "controlplane_test.go",
         "etcd_test.go",
         "kubeconfig_test.go",
-        "phase_test.go",
     ],
     library = ":go_default_library",
     deps = [

cmd/kubeadm/app/cmd/phases/bootstraptoken.go

@@ -22,6 +22,7 @@ import (
 	"github.com/spf13/cobra"
 
 	clientset "k8s.io/client-go/kubernetes"
+	cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util"
 	"k8s.io/kubernetes/cmd/kubeadm/app/phases/bootstraptoken/clusterinfo"
 	"k8s.io/kubernetes/cmd/kubeadm/app/phases/bootstraptoken/node"
 	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
@@ -36,7 +37,7 @@ func NewCmdBootstrapToken() *cobra.Command {
 		Use:     "bootstrap-token",
 		Short:   "Manage kubeadm-specific Bootstrap Token functions.",
 		Aliases: []string{"bootstraptoken"},
-		RunE:    subCmdRunE("bootstrap-token"),
+		RunE:    cmdutil.SubCmdRunE("bootstrap-token"),
 	}
 
 	cmd.PersistentFlags().StringVar(&kubeConfigFile, "kubeconfig", "/etc/kubernetes/admin.conf", "The KubeConfig file to use for talking to the cluster")
@@ -55,7 +56,7 @@ func NewSubCmdClusterInfo(kubeConfigFile *string) *cobra.Command {
 		Short:   "Uploads and exposes the cluster-info ConfigMap publicly from the given cluster-info file",
 		Aliases: []string{"clusterinfo"},
 		Run: func(cmd *cobra.Command, args []string) {
-			err := validateExactArgNumber(args, []string{"clusterinfo-file"})
+			err := cmdutil.ValidateExactArgNumber(args, []string{"clusterinfo-file"})
 			kubeadmutil.CheckErr(err)
 
 			client, err := kubeconfigutil.ClientSetFromFile(*kubeConfigFile)
@@ -81,7 +82,7 @@ func NewSubCmdNodeBootstrapToken(kubeConfigFile *string) *cobra.Command {
 		Use:     "node",
 		Short:   "Manages Node Bootstrap Tokens",
 		Aliases: []string{"clusterinfo"},
-		RunE:    subCmdRunE("node"),
+		RunE:    cmdutil.SubCmdRunE("node"),
 	}
 
 	cmd.AddCommand(NewSubCmdNodeBootstrapTokenPostCSRs(kubeConfigFile))

cmd/kubeadm/app/cmd/phases/certs.go

@@ -22,6 +22,7 @@ import (
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
 	"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/validation"
+	cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util"
 	certsphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs"
 	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 	configutil "k8s.io/kubernetes/cmd/kubeadm/app/util/config"
@@ -34,7 +35,7 @@ func NewCmdCerts() *cobra.Command {
 		Use:     "certs",
 		Aliases: []string{"certificates"},
 		Short:   "Generate certificates for a Kubernetes cluster.",
-		RunE:    subCmdRunE("certs"),
+		RunE:    cmdutil.SubCmdRunE("certs"),
 	}
 
 	cmd.AddCommand(getCertsSubCommands()...)

cmd/kubeadm/app/cmd/phases/controlplane.go

@@ -21,6 +21,7 @@ import (
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
+	cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	controlplanephase "k8s.io/kubernetes/cmd/kubeadm/app/phases/controlplane"
 	"k8s.io/kubernetes/pkg/api"
@@ -31,7 +32,7 @@ func NewCmdControlplane() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "controlplane",
 		Short: "Generate all static pod manifest files necessary to establish the control plane.",
-		RunE:  subCmdRunE("controlplane"),
+		RunE:  cmdutil.SubCmdRunE("controlplane"),
 	}
 
 	manifestPath := kubeadmconstants.GetStaticPodDirectory()

cmd/kubeadm/app/cmd/phases/etcd.go

@@ -21,6 +21,7 @@ import (
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
+	cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	etcdphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/etcd"
 	"k8s.io/kubernetes/pkg/api"
@@ -31,7 +32,7 @@ func NewCmdEtcd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "etcd",
 		Short: "Generate static pod manifest file for etcd.",
-		RunE:  subCmdRunE("etcd"),
+		RunE:  cmdutil.SubCmdRunE("etcd"),
 	}
 
 	manifestPath := kubeadmconstants.GetStaticPodDirectory()

cmd/kubeadm/app/cmd/phases/kubeconfig.go

@@ -24,6 +24,7 @@ import (
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
+	cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	kubeconfigphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/kubeconfig"
 	"k8s.io/kubernetes/pkg/api"
@@ -34,7 +35,7 @@ func NewCmdKubeConfig(out io.Writer) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "kubeconfig",
 		Short: "Generate all kubeconfig files necessary to establish the control plane and the admin kubeconfig file.",
-		RunE:  subCmdRunE("kubeconfig"),
+		RunE:  cmdutil.SubCmdRunE("kubeconfig"),
 	}
 
 	cmd.AddCommand(getKubeConfigSubCommands(out, kubeadmconstants.KubernetesDir)...)

cmd/kubeadm/app/cmd/phases/markmaster.go

@@ -19,6 +19,7 @@ package phases
 import (
 	"github.com/spf13/cobra"
 
+	cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util"
 	markmasterphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/markmaster"
 	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 	kubeconfigutil "k8s.io/kubernetes/cmd/kubeadm/app/util/kubeconfig"
@@ -32,7 +33,7 @@ func NewCmdMarkMaster() *cobra.Command {
 		Short:   "Mark a node as master.",
 		Aliases: []string{"markmaster"},
 		RunE: func(_ *cobra.Command, args []string) error {
-			err := validateExactArgNumber(args, []string{"node-name"})
+			err := cmdutil.ValidateExactArgNumber(args, []string{"node-name"})
 			kubeadmutil.CheckErr(err)
 
 			client, err := kubeconfigutil.ClientSetFromFile(kubeConfigFile)

cmd/kubeadm/app/cmd/phases/phase.go

@@ -17,10 +17,10 @@ limitations under the License.
 package phases
 
 import (
-	"fmt"
 	"io"
 
 	"github.com/spf13/cobra"
+	cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util"
 )
 
 // NewCmdPhase returns the cobra command for the "kubeadm phase" command (currently alpha-gated)
@@ -28,7 +28,7 @@ func NewCmdPhase(out io.Writer) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "phase",
 		Short: "Invoke subsets of kubeadm functions separately for a manual install.",
-		RunE:  subCmdRunE("phase"),
+		RunE:  cmdutil.SubCmdRunE("phase"),
 	}
 
 	cmd.AddCommand(NewCmdBootstrapToken())
@@ -43,37 +43,3 @@ func NewCmdPhase(out io.Writer) *cobra.Command {
 
 	return cmd
 }
-
-// subCmdRunE returns a function that handles a case where a subcommand must be specified
-// Without this callback, if a user runs just the command without a subcommand,
-// or with an invalid subcommand, cobra will print usage information, but still exit cleanly.
-// We want to return an error code in these cases so that the
-// user knows that their command was invalid.
-func subCmdRunE(name string) func(*cobra.Command, []string) error {
-	return func(_ *cobra.Command, args []string) error {
-		if len(args) < 1 {
-			return fmt.Errorf("missing subcommand; %q is not meant to be run on its own", name)
-		}
-
-		return fmt.Errorf("invalid subcommand: %q", args[0])
-	}
-}
-
-// validateExactArgNumber validates that the required top-level arguments are specified
-func validateExactArgNumber(args []string, supportedArgs []string) error {
-	validArgs := 0
-	// Disregard possible "" arguments; they are invalid
-	for _, arg := range args {
-		if len(arg) > 0 {
-			validArgs++
-		}
-	}
-
-	if validArgs < len(supportedArgs) {
-		return fmt.Errorf("missing one or more required arguments. Required arguments: %v", supportedArgs)
-	}
-	if validArgs > len(supportedArgs) {
-		return fmt.Errorf("too many arguments, only %d argument(s) supported: %v", validArgs, supportedArgs)
-	}
-	return nil
-}

cmd/kubeadm/app/cmd/phases/preflight.go

@@ -20,6 +20,7 @@ import (
 	"github.com/spf13/cobra"
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
+	cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util"
 	"k8s.io/kubernetes/cmd/kubeadm/app/preflight"
 )
 
@@ -27,7 +28,7 @@ func NewCmdPreFlight() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "preflight",
 		Short: "Run pre-flight checks",
-		RunE:  subCmdRunE("preflight"),
+		RunE:  cmdutil.SubCmdRunE("preflight"),
 	}
 
 	cmd.AddCommand(NewCmdPreFlightMaster())

cmd/kubeadm/app/cmd/phases/selfhosting.go

@@ -29,6 +29,7 @@ import (
 	configutil "k8s.io/kubernetes/cmd/kubeadm/app/util/config"
 	kubeconfigutil "k8s.io/kubernetes/cmd/kubeadm/app/util/kubeconfig"
 	"k8s.io/kubernetes/pkg/api"
+	cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util"
 )
 
 // NewCmdSelfhosting returns the self-hosting Cobra command
@@ -37,7 +38,7 @@ func NewCmdSelfhosting() *cobra.Command {
 		Use:     "selfhosting",
 		Aliases: []string{"selfhosted"},
 		Short:   "Make a kubeadm cluster self-hosted.",
-		RunE:    subCmdRunE("selfhosting"),
+		RunE:    cmdutil.SubCmdRunE("selfhosting"),
 	}
 
 	cmd.AddCommand(getSelfhostingSubCommand())

cmd/kubeadm/app/cmd/token.go

@@ -33,6 +33,7 @@ import (
 	"k8s.io/apimachinery/pkg/fields"
 	clientset "k8s.io/client-go/kubernetes"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
+	cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	tokenphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/bootstraptoken/node"
 	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
@@ -77,7 +78,7 @@ func NewCmdToken(out io.Writer, errW io.Writer) *cobra.Command {
 		// cobra will print usage information, but still exit cleanly.
 		// We want to return an error code in these cases so that the
 		// user knows that their command was invalid.
-		RunE: subCmdRunE("token"),
+		RunE: cmdutil.SubCmdRunE("token"),
 	}
 
 	tokenCmd.PersistentFlags().StringVar(&kubeConfigFile,

cmd/kubeadm/app/cmd/util/BUILD

@@ -0,0 +1,28 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["cmdutil.go"],
+    visibility = ["//visibility:public"],
+    deps = ["//vendor/github.com/spf13/cobra:go_default_library"],
+)
+
+go_test(
+    name = "go_default_test",
+    srcs = ["cmdutil_test.go"],
+    library = ":go_default_library",
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [":package-srcs"],
+    tags = ["automanaged"],
+    visibility = ["//visibility:public"],
+)

cmd/kubeadm/app/cmd/util/cmdutil.go

@@ -0,0 +1,57 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package phases
+
+import (
+	"fmt"
+
+	"github.com/spf13/cobra"
+)
+
+// SubCmdRunE returns a function that handles a case where a subcommand must be specified
+// Without this callback, if a user runs just the command without a subcommand,
+// or with an invalid subcommand, cobra will print usage information, but still exit cleanly.
+// We want to return an error code in these cases so that the
+// user knows that their command was invalid.
+func SubCmdRunE(name string) func(*cobra.Command, []string) error {
+	return func(_ *cobra.Command, args []string) error {
+		if len(args) < 1 {
+			return fmt.Errorf("missing subcommand; %q is not meant to be run on its own", name)
+		}
+
+		return fmt.Errorf("invalid subcommand: %q", args[0])
+	}
+}
+
+// ValidateExactArgNumber validates that the required top-level arguments are specified
+func ValidateExactArgNumber(args []string, supportedArgs []string) error {
+	validArgs := 0
+	// Disregard possible "" arguments; they are invalid
+	for _, arg := range args {
+		if len(arg) > 0 {
+			validArgs++
+		}
+	}
+
+	if validArgs < len(supportedArgs) {
+		return fmt.Errorf("missing one or more required arguments. Required arguments: %v", supportedArgs)
+	}
+	if validArgs > len(supportedArgs) {
+		return fmt.Errorf("too many arguments, only %d argument(s) supported: %v", validArgs, supportedArgs)
+	}
+	return nil
+}

cmd/kubeadm/app/cmd/util/cmdutil_test.go

@@ -52,10 +52,10 @@ func TestValidateExactArgNumber(t *testing.T) {
 		},
 	}
 	for _, rt := range tests {
-		actual := validateExactArgNumber(rt.args, rt.supportedArgs)
+		actual := ValidateExactArgNumber(rt.args, rt.supportedArgs)
 		if (actual != nil) != rt.expectedErr {
 			t.Errorf(
-				"failed validateExactArgNumber:\n\texpected error: %t\n\t  actual error: %t",
+				"failed ValidateExactArgNumber:\n\texpected error: %t\n\t  actual error: %t",
 				rt.expectedErr,
 				(actual != nil),
 			)