Skip to content
This repository has been archived by the owner on Nov 5, 2021. It is now read-only.

Commit

Permalink
De-duplicate the targets list while refreshing state.
Browse files Browse the repository at this point in the history
We anyway keep the target information in a map by name, i.e. we lose the information for duplicate targets anyway.

Also, log a warning if we get a duplicate target as it should ideally never happen.

Duplicate targets can lead to spurious behavior from probes:
#436

PiperOrigin-RevId: 325081938
  • Loading branch information
manugarg committed Aug 5, 2020
1 parent dd0bb3e commit 831ccc7
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 4 deletions.
15 changes: 14 additions & 1 deletion rds/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,23 @@ func (client *Client) updateState(response *pb.ListResourcesResponse) {
defer client.mu.Unlock()

client.names = make([]string, len(response.GetResources()))
for i, res := range response.GetResources() {
oldcache := client.cache
client.cache = make(map[string]*cacheRecord, len(response.GetResources()))

i := 0
for _, res := range response.GetResources() {
if oldRes, ok := client.cache[res.GetName()]; ok {
client.l.Warningf("Got resource (%s) again, ignoring this instance: {%v}. Previous record: %+v.", res.GetName(), res, *oldRes)
continue
}
if oldcache[res.GetName()] != nil && res.GetIp() != oldcache[res.GetName()].ip {
client.l.Infof("Resource (%s) ip has changed: %s -> %s.", res.GetName(), oldcache[res.GetName()].ip, res.GetIp())
}
client.cache[res.GetName()] = &cacheRecord{res.GetIp(), int(res.GetPort()), res.Labels}
client.names[i] = res.GetName()
i++
}
client.names = client.names[:i]
}

// ListEndpoints returns the list of resources.
Expand Down
15 changes: 12 additions & 3 deletions rds/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,13 @@ var testResourcesMap = map[string][]*pb.Resource{
Port: proto.Int32(80),
Labels: map[string]string{"zone": "us-central1-c"},
},
// Duplicate resource, it should be removed from the result.
{
Name: proto.String("testR3"),
Ip: proto.String("testR3.test.com"),
Port: proto.Int32(80),
Labels: map[string]string{"zone": "us-central1-c"},
},
},
}

Expand Down Expand Up @@ -136,9 +143,11 @@ func TestListAndResolve(t *testing.T) {

client.refreshState(time.Second)

// Test ListEndpoint()
// Test ListEndpoint(), note that we remove the duplicate resource from the
// exepected output.
epList := client.ListEndpoints()
for i, res := range testResources {
expectedList := testResources[:len(testResources)-1]
for i, res := range expectedList {
if epList[i].Name != res.GetName() {
t.Errorf("Resource name: got=%s, want=%s", epList[i].Name, res.GetName())
}
Expand All @@ -153,7 +162,7 @@ func TestListAndResolve(t *testing.T) {
}

// Test Resolve()
for _, res := range testResources {
for _, res := range expectedList {
for _, ipVer := range []int{0, 4, 6} {
t.Run(fmt.Sprintf("resolve_%s_for_IPv%d", res.GetName(), ipVer), func(t *testing.T) {
expectedIP := expectedIPByVersion[res.GetName()][ipVer]
Expand Down

0 comments on commit 831ccc7

Please sign in to comment.