Merge pull request #195 from justinsb/fix_194

Fix bug where edit cluster was computing with the old spec

cmd/kops/edit_cluster.go

@@ -40,7 +40,7 @@ func (c *EditClusterCmd) Run(args []string) error {
 		return err
 	}
 
-	clusterRegistry, cluster, err := rootCommand.Cluster()
+	clusterRegistry, oldCluster, err := rootCommand.Cluster()
 	if err != nil {
 		return err
 	}
@@ -60,7 +60,7 @@ func (c *EditClusterCmd) Run(args []string) error {
 	)
 
 	ext := "yaml"
-	raw, err := api.ToYaml(cluster)
+	raw, err := api.ToYaml(oldCluster)
 	if err != nil {
 		return fmt.Errorf("error reading config file: %v", err)
 	}
@@ -92,7 +92,7 @@ func (c *EditClusterCmd) Run(args []string) error {
 		return fmt.Errorf("error populating configuration: %v", err)
 	}
 
-	fullCluster, err := cloudup.PopulateClusterSpec(cluster, clusterRegistry)
+	fullCluster, err := cloudup.PopulateClusterSpec(newCluster, clusterRegistry)
 	if err != nil {
 		return err
 	}

cmd/kops/edit_instancegroup.go

@@ -65,11 +65,11 @@ func (c *EditInstanceGroupCmd) Run(groupName string) error {
 		return fmt.Errorf("name is required")
 	}
 
-	group, err := registry.Find(groupName)
+	oldGroup, err := registry.Find(groupName)
 	if err != nil {
 		return fmt.Errorf("error reading InstanceGroup %q: %v", groupName, err)
 	}
-	if group == nil {
+	if oldGroup == nil {
 		return fmt.Errorf("InstanceGroup %q not found", groupName)
 	}
 
@@ -78,7 +78,7 @@ func (c *EditInstanceGroupCmd) Run(groupName string) error {
 	)
 
 	ext := "yaml"
-	raw, err := api.ToYaml(group)
+	raw, err := api.ToYaml(oldGroup)
 	if err != nil {
 		return fmt.Errorf("error parsing InstanceGroup: %v", err)
 	}

upup/pkg/fi/cloudup/populatecluster_test.go

@@ -3,6 +3,7 @@ package cloudup
 import (
 	"fmt"
 	"k8s.io/kops/upup/pkg/api"
+	"k8s.io/kubernetes/pkg/util/sets"
 	"strings"
 	"testing"
 )
@@ -26,6 +27,85 @@ func buildMinimalCluster() *api.Cluster {
 	return c
 }
 
+func TestPopulateCluster_Default_NoError(t *testing.T) {
+	c := buildMinimalCluster()
+
+	err := c.PerformAssignments()
+	if err != nil {
+		t.Fatalf("error from PerformAssignments: %v", err)
+	}
+
+	if len(c.Spec.EtcdClusters) == 0 {
+		zones := sets.NewString()
+		for _, z := range c.Spec.Zones {
+			zones.Insert(z.Name)
+		}
+		etcdZones := zones.List()
+
+		for _, etcdCluster := range EtcdClusters {
+			etcd := &api.EtcdClusterSpec{}
+			etcd.Name = etcdCluster
+			for _, zone := range etcdZones {
+				m := &api.EtcdMemberSpec{}
+				m.Name = zone
+				m.Zone = zone
+				etcd.Members = append(etcd.Members, m)
+			}
+			c.Spec.EtcdClusters = append(c.Spec.EtcdClusters, etcd)
+		}
+	}
+
+	registry := buildInmemoryClusterRegistry()
+	_, err = PopulateClusterSpec(c, registry)
+	if err != nil {
+		t.Fatalf("Unexpected error from PopulateCluster: %v", err)
+	}
+}
+
+func TestPopulateCluster_Custom_CIDR(t *testing.T) {
+	c := buildMinimalCluster()
+	c.Spec.NetworkCIDR = "172.20.2.0/24"
+	c.Spec.Zones = []*api.ClusterZoneSpec{
+		{Name: "us-east-1a", CIDR: "172.20.2.0/27"},
+		{Name: "us-east-1b", CIDR: "172.20.2.32/27"},
+		{Name: "us-east-1c", CIDR: "172.20.2.64/27"},
+	}
+
+	err := c.PerformAssignments()
+	if err != nil {
+		t.Fatalf("error from PerformAssignments: %v", err)
+	}
+
+	if len(c.Spec.EtcdClusters) == 0 {
+		zones := sets.NewString()
+		for _, z := range c.Spec.Zones {
+			zones.Insert(z.Name)
+		}
+		etcdZones := zones.List()
+
+		for _, etcdCluster := range EtcdClusters {
+			etcd := &api.EtcdClusterSpec{}
+			etcd.Name = etcdCluster
+			for _, zone := range etcdZones {
+				m := &api.EtcdMemberSpec{}
+				m.Name = zone
+				m.Zone = zone
+				etcd.Members = append(etcd.Members, m)
+			}
+			c.Spec.EtcdClusters = append(c.Spec.EtcdClusters, etcd)
+		}
+	}
+
+	registry := buildInmemoryClusterRegistry()
+	full, err := PopulateClusterSpec(c, registry)
+	if err != nil {
+		t.Fatalf("Unexpected error from PopulateCluster: %v", err)
+	}
+	if full.Spec.NetworkCIDR != "172.20.2.0/24" {
+		t.Fatalf("Unexpected NetworkCIDR: %v", full.Spec.NetworkCIDR)
+	}
+}
+
 func TestPopulateCluster_Name_Required(t *testing.T) {
 	c := buildMinimalCluster()
 	c.Name = ""