Merge pull request #32 from zalando-incubator/fix/last-record-not-del…

…eted process all hosted zones
linki · Dec 22, 2016 · f513a6d · f513a6d
2 parents a61d0d1 + fbe0df5
commit f513a6d
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 20 deletions.
diff --git a/consumers/aws.go b/consumers/aws.go
@@ -78,19 +78,16 @@ func (a *awsClient) Sync(endpoints []*pkg.Endpoint) error {
 
 	var wg sync.WaitGroup
 	for zoneName, zoneID := range hostedZonesMap {
-		if len(inputByZoneID[zoneID]) > 0 {
-			wg.Add(1)
-			zoneID := zoneID
-			go func() {
-				defer wg.Done()
-				err := a.syncPerHostedZone(inputByZoneID[zoneID], zoneID)
-				if err != nil {
-					//should pass the err down the error channel
-					//for now just log
-					log.Errorf("Error changing records per zone: %s", zoneName)
-				}
-			}()
-		}
+		wg.Add(1)
+		go func(zoneName, zoneID string) {
+			defer wg.Done()
+			err := a.syncPerHostedZone(inputByZoneID[zoneID], zoneID)
+			if err != nil {
+				//should pass the err down the error channel
+				//for now just log
+				log.Errorf("Error changing records per zone: %s", zoneName)
+			}
+		}(zoneName, zoneID)
 	}
 	wg.Wait()
 	return nil
@@ -150,7 +147,7 @@ func (a *awsClient) syncPerHostedZone(newAliasRecords []*route53.ResourceRecordS
 		return a.client.ChangeRecordSets(upsert, del, nil, zoneID)
 	}
 
-	log.Infoln("No changes submitted")
+	log.Infoln("No changes submitted for zone: ", zoneID)
 	return nil
 }
 

diff --git a/consumers/aws_test.go b/consumers/aws_test.go
@@ -63,10 +63,19 @@ func checkEndpointSlices(got []*route53.ResourceRecordSet, expect []*route53.Res
 			return false
 		}
 	}
-
 	return true
 }
 
+func NonEmptyMapLength(Map map[string][]*route53.ResourceRecordSet) int {
+	ans := 0
+	for key := range Map {
+		if len(Map[key]) > 0 {
+			ans++
+		}
+	}
+	return ans
+}
+
 func testAWSConsumer(t *testing.T, ti awsTestItem) {
 	groupID := "testing-group-id"
 	client := awstest.NewClient(groupID)
@@ -87,24 +96,24 @@ func testAWSConsumer(t *testing.T, ti awsTestItem) {
 			return
 		}
 	}
-	if len(client.LastUpsert) != len(ti.expectUpsert) {
+	if NonEmptyMapLength(client.LastUpsert) != NonEmptyMapLength(ti.expectUpsert) {
 		t.Error("failed to post the right upsert items. Number of hosted zones is different.", client.LastUpsert, ti.expectUpsert)
 	}
 	for zoneName := range ti.expectUpsert {
 		if !checkEndpointSlices(client.LastUpsert[zoneName], ti.expectUpsert[zoneName]) {
 			t.Error("failed to post the right upsert items", client.LastUpsert[zoneName], ti.expectUpsert[zoneName])
 		}
 	}
-	if len(client.LastCreate) != len(ti.expectCreate) {
-		t.Error("failed to post the right upsert items. Number of hosted zones is different.", client.LastUpsert, ti.expectUpsert)
+	if NonEmptyMapLength(client.LastCreate) != NonEmptyMapLength(ti.expectCreate) {
+		t.Error("failed to post the right create items. Number of hosted zones is different.", client.LastCreate, ti.expectCreate)
 	}
 	for zoneName := range ti.expectCreate {
 		if !checkEndpointSlices(client.LastCreate[zoneName], ti.expectCreate[zoneName]) {
 			t.Error("failed to post the right create items", client.LastCreate[zoneName], ti.expectCreate[zoneName])
 		}
 	}
-	if len(client.LastDelete) != len(ti.expectDelete) {
-		t.Error("failed to post the right upsert items. Number of hosted zones is different.", client.LastUpsert, ti.expectUpsert)
+	if NonEmptyMapLength(client.LastDelete) != NonEmptyMapLength(ti.expectDelete) {
+		t.Error("failed to post the right delete items. Number of hosted zones is different.", client.LastDelete, ti.expectDelete)
 	}
 	for zoneName := range ti.expectDelete {
 		if !checkEndpointSlices(client.LastDelete[zoneName], ti.expectDelete[zoneName]) {
@@ -161,6 +170,22 @@ func TestAWSConsumer(t *testing.T) { //exclude IP endpoints for now only Alias w
 					},
 				},
 			},
+			expectDelete: map[string][]*route53.ResourceRecordSet{
+				"foo.com.": []*route53.ResourceRecordSet{
+					&route53.ResourceRecordSet{
+						Type: aws.String("A"),
+						Name: aws.String("update.foo.com."),
+						AliasTarget: &route53.AliasTarget{
+							DNSName:      aws.String("404.elb.com"),
+							HostedZoneId: aws.String("123"),
+						},
+					},
+					&route53.ResourceRecordSet{
+						Type: aws.String("TXT"),
+						Name: aws.String("update.foo.com."),
+					},
+				},
+			},
 		},
 		{
 			msg: "no initial, sync new ones",
@@ -200,6 +225,20 @@ func TestAWSConsumer(t *testing.T) { //exclude IP endpoints for now only Alias w
 						Name: aws.String("update.example.com."),
 					},
 				},
+				"foo.com.": []*route53.ResourceRecordSet{
+					&route53.ResourceRecordSet{
+						Type: aws.String("A"),
+						Name: aws.String("update.foo.com."),
+						AliasTarget: &route53.AliasTarget{
+							DNSName:      aws.String("404.elb.com"),
+							HostedZoneId: aws.String("123"),
+						},
+					},
+					&route53.ResourceRecordSet{
+						Type: aws.String("TXT"),
+						Name: aws.String("update.foo.com."),
+					},
+				},
 			},
 		},
 		{
@@ -236,6 +275,20 @@ func TestAWSConsumer(t *testing.T) { //exclude IP endpoints for now only Alias w
 						Name: aws.String("update.example.com."),
 					},
 				},
+				"foo.com.": []*route53.ResourceRecordSet{
+					&route53.ResourceRecordSet{
+						Type: aws.String("A"),
+						Name: aws.String("update.foo.com."),
+						AliasTarget: &route53.AliasTarget{
+							DNSName:      aws.String("404.elb.com"),
+							HostedZoneId: aws.String("123"),
+						},
+					},
+					&route53.ResourceRecordSet{
+						Type: aws.String("TXT"),
+						Name: aws.String("update.foo.com."),
+					},
+				},
 			},
 		}, {
 			msg: "insert, update, delete, leave",