providers/aws: elb updates

builtin/providers/aws/resource_aws_elb.go

@@ -81,21 +81,90 @@ func resource_aws_elb_update(
 	s *terraform.ResourceState,
 	d *terraform.ResourceDiff,
 	meta interface{}) (*terraform.ResourceState, error) {
-	// p := meta.(*ResourceProvider)
-	// elbconn := p.elbconn
+	p := meta.(*ResourceProvider)
+	elbconn := p.elbconn
 
 	rs := s.MergeDiff(d)
-	log.Printf("ResourceDiff: %s", d)
-	log.Printf("ResourceState: %s", s)
-	log.Printf("Merged: %s", rs)
 
-	// If we have any instances, we need to register them
-	v := flatmap.Expand(rs.Attributes, "instances").([]interface{})
-	instances := expandStringList(v)
+	// If we currently have instances, or did have instances,
+	// we want to figure out what to add and remove from the load
+	// balancer
+	if attr, ok := d.Attributes["instances.#"]; ok && attr.Old != "" {
+		// The new state of instances merged with the diff
+		mergedInstances := expandStringList(flatmap.Expand(
+			rs.Attributes, "instances").([]interface{}))
 
-	log.Println(instances)
+		// The state before the diff merge
+		previousInstances := expandStringList(flatmap.Expand(
+			s.Attributes, "instances").([]interface{}))
 
-	return nil, fmt.Errorf("Did not update")
+		// keep track of what instances we are removing, and which
+		// we are adding
+		var toRemove []string
+		var toAdd []string
+
+		for _, instanceId := range mergedInstances {
+		   for _, prevId := range previousInstances {
+				// If the merged instance ID existed
+				// previously, we don't have to do anything
+				if instanceId == prevId {
+					continue
+				// Otherwise, we need to add it to the load balancer
+				} else {
+					toAdd = append(toAdd, instanceId)
+				}
+			}
+		}
+
+		for i, instanceId := range toAdd {
+		   for _, prevId := range previousInstances {
+				// If the instance ID we are adding existed
+				// previously, we want to not add it, but rather remove
+				// it
+				if instanceId == prevId {
+					toRemove = append(toRemove, instanceId)
+					toAdd = append(toAdd[:i], toAdd[i+1:]...)
+				// Otherwise, we continue adding it to the ELB
+				} else {
+					continue
+				}
+			}
+		}
+
+		if len(toAdd) > 0 {
+			registerInstancesOpts := elb.RegisterInstancesWithLoadBalancer{
+				LoadBalancerName: rs.ID,
+				Instances:        toAdd,
+			}
+
+			_, err := elbconn.RegisterInstancesWithLoadBalancer(&registerInstancesOpts)
+
+			if err != nil {
+				return s, fmt.Errorf("Failure registering instances: %s", err)
+			}
+		}
+
+		if len(toRemove) > 0 {
+			deRegisterInstancesOpts := elb.DeregisterInstancesFromLoadBalancer{
+				LoadBalancerName: rs.ID,
+				Instances:        toRemove,
+			}
+
+			_, err := elbconn.DeregisterInstancesFromLoadBalancer(&deRegisterInstancesOpts)
+
+			if err != nil {
+				return s, fmt.Errorf("Failure deregistering instances: %s", err)
+			}
+		}
+	}
+
+	loadBalancer, err := resource_aws_elb_retrieve_balancer(rs.ID, elbconn)
+
+	if err != nil {
+		return s, err
+	}
+
+	return resource_aws_elb_update_state(rs, loadBalancer)
 }
 
 func resource_aws_elb_destroy(
@@ -157,8 +226,20 @@ func resource_aws_elb_diff(
 func resource_aws_elb_update_state(
 	s *terraform.ResourceState,
 	balancer *elb.LoadBalancer) (*terraform.ResourceState, error) {
+
 	s.Attributes["name"] = balancer.LoadBalancerName
 	s.Attributes["dns_name"] = balancer.DNSName
+
+	// Flatten our group values
+	toFlatten := make(map[string]interface{})
+
+	if len(balancer.Instances) > 0 && balancer.Instances[0].InstanceId != "" {
+		toFlatten["instances"] = flattenInstances(balancer.Instances)
+		for k, v := range flatmap.Flatten(toFlatten) {
+			s.Attributes[k] = v
+		}
+	}
+
 	return s, nil
 }
 

builtin/providers/aws/resource_aws_elb_test.go

@@ -10,7 +10,7 @@ import (
 	"github.com/mitchellh/goamz/elb"
 )
 
-func TestAccAWSELB(t *testing.T) {
+func TestAccAWSELB_basic(t *testing.T) {
 	var conf elb.LoadBalancer
 
 	resource.Test(t, resource.TestCase{
@@ -23,6 +23,58 @@ func TestAccAWSELB(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAWSELBExists("aws_elb.bar", &conf),
 					testAccCheckAWSELBAttributes(&conf),
+					resource.TestCheckResourceAttr(
+						"aws_elb.bar", "name", "foobar-terraform-test"),
+					resource.TestCheckResourceAttr(
+						"aws_elb.bar", "availability_zones.0", "us-west-2a"),
+					resource.TestCheckResourceAttr(
+						"aws_elb.bar", "availability_zones.1", "us-west-2b"),
+					resource.TestCheckResourceAttr(
+						"aws_elb.bar", "availability_zones.2", "us-west-2c"),
+					resource.TestCheckResourceAttr(
+						"aws_elb.bar", "listener.0.instance_port", "8000"),
+					resource.TestCheckResourceAttr(
+						"aws_elb.bar", "listener.0.instance_protocol", "http"),
+					resource.TestCheckResourceAttr(
+						"aws_elb.bar", "listener.0.lb_port", "80"),
+					resource.TestCheckResourceAttr(
+						"aws_elb.bar", "listener.0.lb_protocol", "http"),
+				),
+			},
+		},
+	})
+}
+
+func TestAccAWSELB_InstanceAttaching(t *testing.T) {
+	var conf elb.LoadBalancer
+
+	testCheckInstanceAttached := func(count int) resource.TestCheckFunc {
+		return func(*terraform.State) error {
+			if len(conf.Instances) != count {
+				return fmt.Errorf("instance count does not match")
+			}
+			return nil
+		}
+	}
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSELBDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccAWSELBConfig,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSELBExists("aws_elb.bar", &conf),
+					testAccCheckAWSELBAttributes(&conf),
+				),
+			},
+
+			resource.TestStep{
+				Config: testAccAWSELBConfigNewInstance,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSELBExists("aws_elb.bar", &conf),
+					testCheckInstanceAttached(1),
 				),
 			},
 		},
@@ -64,7 +116,7 @@ func testAccCheckAWSELBDestroy(s *terraform.State) error {
 
 func testAccCheckAWSELBAttributes(conf *elb.LoadBalancer) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
-		if conf.AvailabilityZones[0].AvailabilityZone != "us-east-1a" {
+		if conf.AvailabilityZones[0].AvailabilityZone != "us-west-2a" {
 			return fmt.Errorf("bad availability_zones")
 		}
 
@@ -129,7 +181,7 @@ func testAccCheckAWSELBExists(n string, res *elb.LoadBalancer) resource.TestChec
 const testAccAWSELBConfig = `
 resource "aws_elb" "bar" {
   name = "foobar-terraform-test"
-  availability_zones = ["us-east-1a"]
+  availability_zones = ["us-west-2a", "us-west-2b", "us-west-2c"]
 
   listener {
     instance_port = 8000
@@ -141,3 +193,25 @@ resource "aws_elb" "bar" {
   instances = []
 }
 `
+
+const testAccAWSELBConfigNewInstance = `
+resource "aws_elb" "bar" {
+  name = "foobar-terraform-test"
+  availability_zones = ["us-west-2a", "us-west-2b", "us-west-2c"]
+
+  listener {
+    instance_port = 8000
+    instance_protocol = "http"
+    lb_port = 80
+    lb_protocol = "http"
+  }
+
+  instances = ["${aws_instance.foo.id}"]
+}
+
+resource "aws_instance" "foo" {
+	# us-west-2
+	ami = "ami-043a5034"
+	instance_type = "t1.micro"
+}
+`

builtin/providers/aws/structure.go

@@ -122,6 +122,15 @@ func flattenLoadBalancers(list []autoscaling.LoadBalancerName) []string {
 	return result
 }
 
+// Flattens an array of Instances into a []string
+func flattenInstances(list []elb.Instance) []string {
+	result := make([]string, 0, len(list))
+	for _, i := range list {
+		result = append(result, i.InstanceId)
+	}
+	return result
+}
+
 // Takes the result of flatmap.Expand for an array of strings
 // and returns a []string
 func expandStringList(configured []interface{}) []string {