Try harder to query all the possible ec2 instances, and filter on instance state / lack of IP address

provider/ecs.go

@@ -275,37 +275,48 @@ func (provider *ECS) listInstances(ctx context.Context, client *awsClient) ([]ec
 }
 
 func (provider *ECS) lookupEc2Instances(ctx context.Context, client *awsClient, containerArns []*string) ([]*ec2.Instance, error) {
-	req, containerResp := client.ecs.DescribeContainerInstancesRequest(&ecs.DescribeContainerInstancesInput{
-		ContainerInstances: containerArns,
-		Cluster:            &provider.Cluster,
-	})
-
-	if err := wrapAws(ctx, req); err != nil {
-		return nil, err
-	}
 
 	order := make(map[string]int)
+	instanceIds := make([]*string, len(containerArns))
+	instances := make([]*ec2.Instance, len(containerArns))
 	for i, arn := range containerArns {
 		order[*arn] = i
 	}
 
-	instanceIds := make([]*string, len(containerArns))
+	req, _ := client.ecs.DescribeContainerInstancesRequest(&ecs.DescribeContainerInstancesInput{
-	for i, container := range containerResp.ContainerInstances {
+		ContainerInstances: containerArns,
-		order[*container.Ec2InstanceId] = order[*container.ContainerInstanceArn]
+		Cluster:            &provider.Cluster,
-		instanceIds[i] = container.Ec2InstanceId
-	}
-
-	req, instancesResp := client.ec2.DescribeInstancesRequest(&ec2.DescribeInstancesInput{
-		InstanceIds: instanceIds,
 	})
 
+	for ; req != nil; req = req.NextPage() {
 		if err := wrapAws(ctx, req); err != nil {
 			return nil, err
 		}
 
-	instances := make([]*ec2.Instance, len(containerArns))
+		containerResp := req.Data.(*ecs.DescribeContainerInstancesOutput)
+		for i, container := range containerResp.ContainerInstances {
+			order[*container.Ec2InstanceId] = order[*container.ContainerInstanceArn]
+			instanceIds[i] = container.Ec2InstanceId
+		}
+	}
+
+	req, _ = client.ec2.DescribeInstancesRequest(&ec2.DescribeInstancesInput{
+		InstanceIds: instanceIds,
+	})
+
+	for ; req != nil; req = req.NextPage() {
+		if err := wrapAws(ctx, req); err != nil {
+			return nil, err
+		}
+
+		instancesResp := req.Data.(*ec2.DescribeInstancesOutput)
 		for _, r := range instancesResp.Reservations {
-		instances[order[*r.Instances[0].InstanceId]] = r.Instances[0]
+			for _, i := range r.Instances {
+				if i.InstanceId != nil {
+					instances[order[*i.InstanceId]] = i
+				}
+			}
+		}
 	}
 	return instances, nil
 }
@@ -340,6 +351,23 @@ func (provider *ECS) filterInstance(i ecsInstance) bool {
 		return false
 	}
 
+	if i.machine == nil ||
+		i.machine.State == nil ||
+		i.machine.State.Name == nil {
+		log.Debugf("Filtering ecs instance in an missing ec2 information %s (%s)", i.Name, i.ID)
+		return false
+	}
+
+	if *i.machine.State.Name != ec2.InstanceStateNameRunning {
+		log.Debugf("Filtering ecs instance in an incorrect state %s (%s) (state = %s)", i.Name, i.ID, *i.machine.State.Name)
+		return false
+	}
+
+	if i.machine.PrivateIpAddress == nil {
+		log.Debugf("Filtering ecs instance without an ip address %s (%s)", i.Name, i.ID)
+		return false
+	}
+
 	label := i.label("traefik.enable")
 	enabled := provider.ExposedByDefault && label != "false" || label == "true"
 	if !enabled {

provider/ecs_test.go

@@ -37,6 +37,9 @@ func makeEcsInstance(containerDef *ecs.ContainerDefinition) ecsInstance {
 		containerDefinition: containerDef,
 		machine: &ec2.Instance{
 			PrivateIpAddress: aws.String("10.0.0.0"),
+			State: &ec2.InstanceState{
+				Name: aws.String(ec2.InstanceStateNameRunning),
+			},
 		},
 	}
 }
@@ -70,10 +73,10 @@ func TestEcsProtocol(t *testing.T) {
 		},
 	}
 
-	for _, c := range cases {
+	for i, c := range cases {
 		value := c.instanceInfo.Protocol()
 		if value != c.expected {
-			t.Fatalf("Should have been %s, got %s", c.expected, value)
+			t.Fatalf("Should have been %v, got %v (case %d)", c.expected, value, i)
 		}
 	}
 }
@@ -89,10 +92,10 @@ func TestEcsHost(t *testing.T) {
 		},
 	}
 
-	for _, c := range cases {
+	for i, c := range cases {
 		value := c.instanceInfo.Host()
 		if value != c.expected {
-			t.Fatalf("Should have been %s, got %s", c.expected, value)
+			t.Fatalf("Should have been %v, got %v (case %d)", c.expected, value, i)
 		}
 	}
 }
@@ -108,10 +111,10 @@ func TestEcsPort(t *testing.T) {
 		},
 	}
 
-	for _, c := range cases {
+	for i, c := range cases {
 		value := c.instanceInfo.Port()
 		if value != c.expected {
-			t.Fatalf("Should have been %s, got %s", c.expected, value)
+			t.Fatalf("Should have been %v, got %v (case %d)", c.expected, value, i)
 		}
 	}
 }
@@ -133,10 +136,10 @@ func TestEcsWeight(t *testing.T) {
 		},
 	}
 
-	for _, c := range cases {
+	for i, c := range cases {
 		value := c.instanceInfo.Weight()
 		if value != c.expected {
-			t.Fatalf("Should have been %s, got %s", c.expected, value)
+			t.Fatalf("Should have been %v, got %v (case %d)", c.expected, value, i)
 		}
 	}
 }
@@ -158,10 +161,10 @@ func TestEcsPassHostHeader(t *testing.T) {
 		},
 	}
 
-	for _, c := range cases {
+	for i, c := range cases {
 		value := c.instanceInfo.PassHostHeader()
 		if value != c.expected {
-			t.Fatalf("Should have been %s, got %s", c.expected, value)
+			t.Fatalf("Should have been %v, got %v (case %d)", c.expected, value, i)
 		}
 	}
 }
@@ -183,10 +186,10 @@ func TestEcsPriority(t *testing.T) {
 		},
 	}
 
-	for _, c := range cases {
+	for i, c := range cases {
 		value := c.instanceInfo.Priority()
 		if value != c.expected {
-			t.Fatalf("Should have been %s, got %s", c.expected, value)
+			t.Fatalf("Should have been %v, got %v (case %d)", c.expected, value, i)
 		}
 	}
 }
@@ -214,10 +217,94 @@ func TestEcsEntryPoints(t *testing.T) {
 		},
 	}
 
-	for _, c := range cases {
+	for i, c := range cases {
 		value := c.instanceInfo.EntryPoints()
 		if !reflect.DeepEqual(value, c.expected) {
-			t.Fatalf("Should have been %s, got %s", c.expected, value)
+			t.Fatalf("Should have been %v, got %v (case %d)", c.expected, value, i)
+		}
+	}
+}
+
+func TestFilterInstance(t *testing.T) {
+
+	nilPrivateIP := simpleEcsInstance(map[string]*string{})
+	nilPrivateIP.machine.PrivateIpAddress = nil
+
+	nilMachine := simpleEcsInstance(map[string]*string{})
+	nilMachine.machine = nil
+
+	nilMachineState := simpleEcsInstance(map[string]*string{})
+	nilMachineState.machine.State = nil
+
+	nilMachineStateName := simpleEcsInstance(map[string]*string{})
+	nilMachineStateName.machine.State.Name = nil
+
+	invalidMachineState := simpleEcsInstance(map[string]*string{})
+	invalidMachineState.machine.State.Name = aws.String(ec2.InstanceStateNameStopped)
+
+	cases := []struct {
+		expected         bool
+		exposedByDefault bool
+		instanceInfo     ecsInstance
+	}{
+		{
+			expected:         true,
+			exposedByDefault: true,
+			instanceInfo:     simpleEcsInstance(map[string]*string{}),
+		},
+		{
+			expected:         false,
+			exposedByDefault: false,
+			instanceInfo:     simpleEcsInstance(map[string]*string{}),
+		},
+		{
+			expected:         false,
+			exposedByDefault: true,
+			instanceInfo: simpleEcsInstance(map[string]*string{
+				"traefik.enable": aws.String("false"),
+			}),
+		},
+		{
+			expected:         true,
+			exposedByDefault: false,
+			instanceInfo: simpleEcsInstance(map[string]*string{
+				"traefik.enable": aws.String("true"),
+			}),
+		},
+		{
+			expected:         false,
+			exposedByDefault: true,
+			instanceInfo:     nilPrivateIP,
+		},
+		{
+			expected:         false,
+			exposedByDefault: true,
+			instanceInfo:     nilMachine,
+		},
+		{
+			expected:         false,
+			exposedByDefault: true,
+			instanceInfo:     nilMachineState,
+		},
+		{
+			expected:         false,
+			exposedByDefault: true,
+			instanceInfo:     nilMachineStateName,
+		},
+		{
+			expected:         false,
+			exposedByDefault: true,
+			instanceInfo:     invalidMachineState,
+		},
+	}
+
+	for i, c := range cases {
+		provider := &ECS{
+			ExposedByDefault: c.exposedByDefault,
+		}
+		value := provider.filterInstance(c.instanceInfo)
+		if value != c.expected {
+			t.Fatalf("Should have been %v, got %v (case %d)", c.expected, value, i)
 		}
 	}
 }