Rename old upgrade command; make new upgrade intuitive

cmd/kops/rollingupdate_cluster.go

@@ -5,7 +5,6 @@ import (
 	"os"
 	"strconv"
 
-	"github.com/golang/glog"
 	"github.com/spf13/cobra"
 	"k8s.io/kops/upup/pkg/fi/cloudup"
 	"k8s.io/kops/upup/pkg/kutil"
@@ -34,7 +33,7 @@ func init() {
 	cmd.Run = func(cmd *cobra.Command, args []string) {
 		err := rollingupdateCluster.Run()
 		if err != nil {
-			glog.Exitf("%v", err)
+			exitWithError(err)
 		}
 	}
 }

cmd/kops/toolbox.go

@@ -0,0 +1,15 @@
+package main
+
+import (
+	"github.com/spf13/cobra"
+)
+
+// toolboxCmd represents the toolbox command
+var toolboxCmd = &cobra.Command{
+	Use:   "toolbox",
+	Short: "Misc infrequently used commands",
+}
+
+func init() {
+	rootCommand.AddCommand(toolboxCmd)
+}

cmd/kops/toolbox_convert_imported.go

@@ -0,0 +1,100 @@
+package main
+
+import (
+	"fmt"
+
+	"github.com/spf13/cobra"
+	"k8s.io/kops/upup/pkg/api"
+	"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
+	"k8s.io/kops/upup/pkg/kutil"
+)
+
+type ConvertImportedCmd struct {
+	NewClusterName string
+}
+
+var convertImported ConvertImportedCmd
+
+func init() {
+	cmd := &cobra.Command{
+		Use:   "convert-imported",
+		Short: "Convert an imported cluster into a kops cluster",
+		Run: func(cmd *cobra.Command, args []string) {
+			err := convertImported.Run()
+			if err != nil {
+				exitWithError(err)
+			}
+		},
+	}
+
+	toolboxCmd.AddCommand(cmd)
+
+	cmd.Flags().StringVar(&convertImported.NewClusterName, "newname", "", "new cluster name")
+}
+
+func (c *ConvertImportedCmd) Run() error {
+	clusterRegistry, cluster, err := rootCommand.Cluster()
+	if err != nil {
+		return err
+	}
+
+	instanceGroupRegistry, err := rootCommand.InstanceGroupRegistry()
+	if err != nil {
+		return err
+	}
+
+	instanceGroups, err := instanceGroupRegistry.ReadAll()
+
+	if cluster.Annotations[api.AnnotationNameManagement] != api.AnnotationValueManagementImported {
+		return fmt.Errorf("cluster %q does not appear to be a cluster imported using kops import", cluster.Name)
+	}
+
+	if c.NewClusterName == "" {
+		return fmt.Errorf("--newname is required for converting an imported cluster")
+	}
+
+	oldClusterName := cluster.Name
+	if oldClusterName == "" {
+		return fmt.Errorf("(Old) ClusterName must be set in configuration")
+	}
+
+	// TODO: Switch to cloudup.BuildCloud
+	if len(cluster.Spec.Zones) == 0 {
+		return fmt.Errorf("Configuration must include Zones")
+	}
+
+	region := ""
+	for _, zone := range cluster.Spec.Zones {
+		if len(zone.Name) <= 2 {
+			return fmt.Errorf("Invalid AWS zone: %q", zone.Name)
+		}
+
+		zoneRegion := zone.Name[:len(zone.Name)-1]
+		if region != "" && zoneRegion != region {
+			return fmt.Errorf("Clusters cannot span multiple regions")
+		}
+
+		region = zoneRegion
+	}
+
+	tags := map[string]string{"KubernetesCluster": oldClusterName}
+	cloud, err := awsup.NewAWSCloud(region, tags)
+	if err != nil {
+		return fmt.Errorf("error initializing AWS client: %v", err)
+	}
+
+	d := &kutil.ConvertKubeupCluster{}
+	d.NewClusterName = c.NewClusterName
+	d.OldClusterName = oldClusterName
+	d.Cloud = cloud
+	d.ClusterConfig = cluster
+	d.InstanceGroups = instanceGroups
+	d.ClusterRegistry = clusterRegistry
+
+	err = d.Upgrade()
+	if err != nil {
+		return err
+	}
+
+	return nil
+}

cmd/kops/upgrade_cluster.go

@@ -3,13 +3,15 @@ package main
 import (
 	"fmt"
 
-	"github.com/golang/glog"
 	"github.com/spf13/cobra"
-	"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
-	"k8s.io/kops/upup/pkg/kutil"
+	"k8s.io/kops/upup/pkg/api"
+	"k8s.io/kops/upup/pkg/fi/cloudup"
+	"os"
 )
 
 type UpgradeClusterCmd struct {
+	Yes bool
+
 	NewClusterName string
 }
 
@@ -23,21 +25,26 @@ func init() {
 		Run: func(cmd *cobra.Command, args []string) {
 			err := upgradeCluster.Run()
 			if err != nil {
-				glog.Exitf("%v", err)
+				exitWithError(err)
 			}
 		},
 	}
 
+	cmd.Flags().BoolVar(&upgradeCluster.Yes, "yes", false, "Apply update")
+
 	upgradeCmd.AddCommand(cmd)
+}
+
+type upgradeAction struct {
+	Item     string
+	Property string
+	Old      string
+	New      string
 
-	cmd.Flags().StringVar(&upgradeCluster.NewClusterName, "newname", "", "new cluster name")
+	apply func()
 }
 
 func (c *UpgradeClusterCmd) Run() error {
-	if c.NewClusterName == "" {
-		return fmt.Errorf("--newname is required")
-	}
-
 	clusterRegistry, cluster, err := rootCommand.Cluster()
 	if err != nil {
 		return err
@@ -50,46 +57,94 @@ func (c *UpgradeClusterCmd) Run() error {
 
 	instanceGroups, err := instanceGroupRegistry.ReadAll()
 
-	oldClusterName := cluster.Name
-	if oldClusterName == "" {
-		return fmt.Errorf("(Old) ClusterName must be set in configuration")
+	if cluster.Annotations[api.AnnotationNameManagement] == api.AnnotationValueManagementImported {
+		return fmt.Errorf("upgrade is not for use with imported clusters (did you mean `kops toolbox convert-imported`?)")
+	}
+
+	latestKubernetesVersion, err := api.FindLatestKubernetesVersion()
+	if err != nil {
+		return err
+	}
+
+	var actions []*upgradeAction
+	if cluster.Spec.KubernetesVersion != latestKubernetesVersion {
+		actions = append(actions, &upgradeAction{
+			Item:     "Cluster",
+			Property: "KubernetesVersion",
+			Old:      cluster.Spec.KubernetesVersion,
+			New:      latestKubernetesVersion,
+			apply: func() {
+				cluster.Spec.KubernetesVersion = latestKubernetesVersion
+			},
+		})
+	}
+
+	if len(actions) == 0 {
+		// TODO: Allow --force option to force even if not needed?
+		fmt.Printf("\nNo upgrade required\n")
+		return nil
 	}
 
-	if len(cluster.Spec.Zones) == 0 {
-		return fmt.Errorf("Configuration must include Zones")
+	{
+		t := &Table{}
+		t.AddColumn("ITEM", func(a *upgradeAction) string {
+			return a.Item
+		})
+		t.AddColumn("PROPERTY", func(a *upgradeAction) string {
+			return a.Property
+		})
+		t.AddColumn("OLD", func(a *upgradeAction) string {
+			return a.Old
+		})
+		t.AddColumn("NEW", func(a *upgradeAction) string {
+			return a.New
+		})
+
+		err := t.Render(actions, os.Stdout, "ITEM", "PROPERTY", "OLD", "NEW")
+		if err != nil {
+			return err
+		}
 	}
 
-	region := ""
-	for _, zone := range cluster.Spec.Zones {
-		if len(zone.Name) <= 2 {
-			return fmt.Errorf("Invalid AWS zone: %q", zone.Name)
+	if !c.Yes {
+		fmt.Printf("\nMust specify --yes to perform upgrade\n")
+		return nil
+	} else {
+		for _, action := range actions {
+			action.apply()
 		}
 
-		zoneRegion := zone.Name[:len(zone.Name)-1]
-		if region != "" && zoneRegion != region {
-			return fmt.Errorf("Clusters cannot span multiple regions")
+		// TODO: DRY this chunk
+		err = cluster.PerformAssignments()
+		if err != nil {
+			return fmt.Errorf("error populating configuration: %v", err)
 		}
 
-		region = zoneRegion
-	}
+		fullCluster, err := cloudup.PopulateClusterSpec(cluster, clusterRegistry)
+		if err != nil {
+			return err
+		}
 
-	tags := map[string]string{"KubernetesCluster": oldClusterName}
-	cloud, err := awsup.NewAWSCloud(region, tags)
-	if err != nil {
-		return fmt.Errorf("error initializing AWS client: %v", err)
-	}
+		err = api.DeepValidate(fullCluster, instanceGroups, true)
+		if err != nil {
+			return err
+		}
 
-	d := &kutil.UpgradeCluster{}
-	d.NewClusterName = c.NewClusterName
-	d.OldClusterName = oldClusterName
-	d.Cloud = cloud
-	d.ClusterConfig = cluster
-	d.InstanceGroups = instanceGroups
-	d.ClusterRegistry = clusterRegistry
+		// Note we perform as much validation as we can, before writing a bad config
+		err = clusterRegistry.Update(cluster)
+		if err != nil {
+			return err
+		}
 
-	err = d.Upgrade()
-	if err != nil {
-		return err
+		err = clusterRegistry.WriteCompletedConfig(fullCluster)
+		if err != nil {
+			return fmt.Errorf("error writing completed cluster spec: %v", err)
+		}
+
+		fmt.Printf("\nUpdates applied to configuration.\n")
+
+		// TODO: automate this step
+		fmt.Printf("You can now apply these changes, using `kops update cluster %s`\n", cluster.Name)
 	}
 
 	return nil

docs/upgrade_from_k8s_12.md

@@ -44,7 +44,7 @@ Now have a look at the cluster configuration, to make sure it looks right.  If i
 open an issue.
 
 ```
-kops edit cluster ${OLD_NAME}
+kops get cluster ${OLD_NAME} -oyaml
 ````
 
 ## Move resources to a new cluster
@@ -62,7 +62,7 @@ The upgrade procedure forces you to choose a new cluster name (e.g. `k8s.mydomai
 
 ```
 export NEW_NAME=k8s.mydomain.com
-kops upgrade cluster --newname ${NEW_NAME} --name ${OLD_NAME}
+kops toolbox convert-imported --newname ${NEW_NAME} --name ${OLD_NAME}
 ```
 
 If you now list the clusters, you should see both the old cluster & the new cluster

upup/pkg/api/cluster.go

@@ -319,7 +319,7 @@ func (c *Cluster) FillDefaults() error {
 // It will be populated with the latest stable kubernetes version
 func (c *Cluster) ensureKubernetesVersion() error {
 	if c.Spec.KubernetesVersion == "" {
-		latestVersion, err := findLatestKubernetesVersion()
+		latestVersion, err := FindLatestKubernetesVersion()
 		if err != nil {
 			return err
 		}
@@ -329,9 +329,9 @@ func (c *Cluster) ensureKubernetesVersion() error {
 	return nil
 }
 
-// findLatestKubernetesVersion returns the latest kubernetes version,
+// FindLatestKubernetesVersion returns the latest kubernetes version,
 // as stored at https://storage.googleapis.com/kubernetes-release/release/stable.txt
-func findLatestKubernetesVersion() (string, error) {
+func FindLatestKubernetesVersion() (string, error) {
 	stableURL := "https://storage.googleapis.com/kubernetes-release/release/stable.txt"
 	b, err := vfs.Context.ReadFile(stableURL)
 	if err != nil {

upup/pkg/api/labels.go

@@ -0,0 +1,7 @@
+package api
+
+// AnnotationNameManagement is the annotation that indicates that a cluster is under external or non-standard management
+const AnnotationNameManagement = "kops.kubernetes.io/management"
+
+// AnnotationValueManagementImported is the annotation value that indicates a cluster was imported, typically as part of an upgrade
+const AnnotationValueManagementImported = "imported"

upup/pkg/api/validation_test.go

@@ -1,8 +1,8 @@
 package api
 
 import (
-	"testing"
 	"k8s.io/kubernetes/pkg/util/validation"
+	"testing"
 )
 
 func Test_Validate_DNS(t *testing.T) {

upup/pkg/kutil/upgrade_cluster.go → upup/pkg/kutil/convert_kubeup_cluster.go

@@ -13,8 +13,8 @@ import (
 	"time"
 )
 
-// UpgradeCluster performs an upgrade of a k8s cluster
-type UpgradeCluster struct {
+// ConvertKubeupCluster performs a conversion of a cluster that was imported from kube-up
+type ConvertKubeupCluster struct {
 	OldClusterName string
 	NewClusterName string
 	Cloud          fi.Cloud
@@ -25,7 +25,7 @@ type UpgradeCluster struct {
 	InstanceGroups []*api.InstanceGroup
 }
 
-func (x *UpgradeCluster) Upgrade() error {
+func (x *ConvertKubeupCluster) Upgrade() error {
 	awsCloud := x.Cloud.(*awsup.AWSCloud)
 
 	cluster := x.ClusterConfig
@@ -54,6 +54,11 @@ func (x *UpgradeCluster) Upgrade() error {
 		return fmt.Errorf("error populating cluster defaults: %v", err)
 	}
 
+	if cluster.Annotations != nil {
+		// Remove the management annotation for the new cluster
+		delete(cluster.Annotations, api.AnnotationNameManagement)
+	}
+
 	fullCluster, err := cloudup.PopulateClusterSpec(cluster, x.ClusterRegistry)
 	if err != nil {
 		return err