Merge pull request #307 from justinsb/fix_208

If no changes are needed in an update, don't print a confusing message

cmd/kops/create_cluster.go

@@ -91,15 +91,16 @@ func (c *CreateClusterCmd) Run(args []string) error {
 
 	isDryrun := false
 	// direct requires --yes (others do not, because they don't make changes)
+	targetName := c.Target
 	if c.Target == cloudup.TargetDirect {
 		if !c.Yes {
 			isDryrun = true
-			c.Target = cloudup.TargetDryRun
+			targetName = cloudup.TargetDryRun
 		}
 	}
 	if c.Target == cloudup.TargetDryRun {
 		isDryrun = true
-		c.Target = cloudup.TargetDryRun
+		targetName = cloudup.TargetDryRun
 	}
 
 	clusterName := rootCommand.clusterName
@@ -374,7 +375,7 @@ func (c *CreateClusterCmd) Run(args []string) error {
 		InstanceGroups:  fullInstanceGroups,
 		Models:          strings.Split(c.Models, ","),
 		ClusterRegistry: clusterRegistry,
-		Target:          c.Target,
+		TargetName:      targetName,
 		OutDir:          c.OutDir,
 		DryRun:          isDryrun,
 	}

cmd/kops/root.go

@@ -127,7 +127,7 @@ func readKubectlClusterConfig() (*kutil.KubectlClusterWithName, error) {
 	}
 
 	// Minify should have done this
-	if  len(config.Clusters) != 1 {
+	if len(config.Clusters) != 1 {
 		return nil, fmt.Errorf("expected exactly one cluster in kubectl config, found %d", len(config.Clusters))
 	}
 

cmd/kops/update_cluster.go

@@ -53,16 +53,18 @@ func (c *UpdateClusterCmd) Run(args []string) error {
 	}
 
 	isDryrun := false
+	targetName := c.Target
+
 	// direct requires --yes (others do not, because they don't do anything!)
 	if c.Target == cloudup.TargetDirect {
 		if !c.Yes {
 			isDryrun = true
-			c.Target = cloudup.TargetDryRun
+			targetName = cloudup.TargetDryRun
 		}
 	}
 	if c.Target == cloudup.TargetDryRun {
 		isDryrun = true
-		c.Target = cloudup.TargetDryRun
+		targetName = cloudup.TargetDryRun
 	}
 
 	if c.OutDir == "" {
@@ -118,7 +120,7 @@ func (c *UpdateClusterCmd) Run(args []string) error {
 		InstanceGroups:  fullInstanceGroups,
 		Models:          strings.Split(c.Models, ","),
 		ClusterRegistry: clusterRegistry,
-		Target:          c.Target,
+		TargetName:      targetName,
 		OutDir:          c.OutDir,
 		DryRun:          isDryrun,
 	}
@@ -128,7 +130,12 @@ func (c *UpdateClusterCmd) Run(args []string) error {
 	}
 
 	if isDryrun {
-		fmt.Printf("Must specify --yes to apply changes\n")
+		target := applyCmd.Target.(*fi.DryRunTarget)
+		if target.HasChanges() {
+			fmt.Printf("Must specify --yes to apply changes\n")
+		} else {
+			fmt.Printf("No changes need to be applied\n")
+		}
 		return nil
 	}
 
@@ -196,4 +203,4 @@ func hasKubecfg(contextName string) (bool, error) {
 		}
 	}
 	return false, nil
-}
+}

upup/pkg/fi/cloudup/apply_cluster.go

@@ -32,8 +32,11 @@ type ApplyClusterCmd struct {
 	// Models is a list of cloudup models to apply
 	Models []string
 
-	// Target specifies how we are operating e.g. direct to GCE, or AWS, or dry-run, or terraform
-	Target string
+	// TargetName specifies how we are operating e.g. direct to GCE, or AWS, or dry-run, or terraform
+	TargetName string
+
+	// Target is the fi.Target we will operate against
+	Target fi.Target
 
 	// OutDir is a local directory in which we place output, can cache files etc
 	OutDir string
@@ -413,7 +416,7 @@ func (c *ApplyClusterCmd) Run() error {
 
 	var target fi.Target
 
-	switch c.Target {
+	switch c.TargetName {
 	case TargetDirect:
 		switch cluster.Spec.CloudProvider {
 		case "gce":
@@ -432,8 +435,9 @@ func (c *ApplyClusterCmd) Run() error {
 	case TargetDryRun:
 		target = fi.NewDryRunTarget(os.Stdout)
 	default:
-		return fmt.Errorf("unsupported target type %q", c.Target)
+		return fmt.Errorf("unsupported target type %q", c.TargetName)
 	}
+	c.Target = target
 
 	context, err := fi.NewContext(target, cloud, keyStore, secretStore, checkExisting)
 	if err != nil {

upup/pkg/fi/dryrun_target.go

@@ -257,3 +257,8 @@ func ValueAsString(value reflect.Value) string {
 func (t *DryRunTarget) Finish(taskMap map[string]Task) error {
 	return t.PrintReport(taskMap, t.out)
 }
+
+// HasChanges returns true iff any changes would have been made
+func (t *DryRunTarget) HasChanges() bool {
+	return len(t.changes) != 0
+}