Do not make multiple requests to the same URL for balancer healthcheck

pkg/healthcheck/healthcheck.go

@@ -375,17 +375,31 @@ func (lb *LbStatusUpdater) UpsertServer(u *url.URL, options ...roundrobin.Server
 // Balancers is a list of Balancers(s) that implements the Balancer interface.
 type Balancers []Balancer
 
-// Servers returns the servers url from all the BalancerHandler.
+// Servers returns the deduplicated server URLs from all the Balancer.
+// Note that the deduplication is only possible because all the underlying
+// balancers are of the same kind (the oxy implementation).
+// The comparison property is the same as the one found at:
+// https://github.com/vulcand/oxy/blob/fb2728c857b7973a27f8de2f2190729c0f22cf49/roundrobin/rr.go#L347.
 func (b Balancers) Servers() []*url.URL {
+	seen := make(map[string]struct{})
+
 	var servers []*url.URL
 	for _, lb := range b {
-		servers = append(servers, lb.Servers()...)
+		for _, server := range lb.Servers() {
+			key := serverKey(server)
+			if _, ok := seen[key]; ok {
+				continue
+			}
+
+			servers = append(servers, server)
+			seen[key] = struct{}{}
+		}
 	}
 
 	return servers
 }
 
-// RemoveServer removes the given server from all the BalancerHandler,
+// RemoveServer removes the given server from all the Balancer,
 // and updates the status of the server to "DOWN".
 func (b Balancers) RemoveServer(u *url.URL) error {
 	for _, lb := range b {
@@ -396,7 +410,7 @@ func (b Balancers) RemoveServer(u *url.URL) error {
 	return nil
 }
 
-// UpsertServer adds the given server to all the BalancerHandler,
+// UpsertServer adds the given server to all the Balancer,
 // and updates the status of the server to "UP".
 func (b Balancers) UpsertServer(u *url.URL, options ...roundrobin.ServerOption) error {
 	for _, lb := range b {
@@ -406,3 +420,7 @@ func (b Balancers) UpsertServer(u *url.URL, options ...roundrobin.ServerOption)
 	}
 	return nil
 }
+
+func serverKey(u *url.URL) string {
+	return u.Path + u.Host + u.Scheme
+}

pkg/healthcheck/healthcheck_test.go

@@ -362,6 +362,81 @@ func TestAddHeadersAndHost(t *testing.T) {
 	}
 }
 
+func TestBalancers_Servers(t *testing.T) {
+	server1, err := url.Parse("http://foo.com")
+	require.NoError(t, err)
+
+	balancer1, err := roundrobin.New(nil)
+	require.NoError(t, err)
+
+	err = balancer1.UpsertServer(server1)
+	require.NoError(t, err)
+
+	server2, err := url.Parse("http://foo.com")
+	require.NoError(t, err)
+
+	balancer2, err := roundrobin.New(nil)
+	require.NoError(t, err)
+
+	err = balancer2.UpsertServer(server2)
+	require.NoError(t, err)
+
+	balancers := Balancers([]Balancer{balancer1, balancer2})
+
+	want, err := url.Parse("http://foo.com")
+	require.NoError(t, err)
+
+	assert.Equal(t, 1, len(balancers.Servers()))
+	assert.Equal(t, want, balancers.Servers()[0])
+}
+
+func TestBalancers_UpsertServer(t *testing.T) {
+	balancer1, err := roundrobin.New(nil)
+	require.NoError(t, err)
+
+	balancer2, err := roundrobin.New(nil)
+	require.NoError(t, err)
+
+	want, err := url.Parse("http://foo.com")
+	require.NoError(t, err)
+
+	balancers := Balancers([]Balancer{balancer1, balancer2})
+
+	err = balancers.UpsertServer(want)
+	require.NoError(t, err)
+
+	assert.Equal(t, 1, len(balancer1.Servers()))
+	assert.Equal(t, want, balancer1.Servers()[0])
+
+	assert.Equal(t, 1, len(balancer2.Servers()))
+	assert.Equal(t, want, balancer2.Servers()[0])
+}
+
+func TestBalancers_RemoveServer(t *testing.T) {
+	server, err := url.Parse("http://foo.com")
+	require.NoError(t, err)
+
+	balancer1, err := roundrobin.New(nil)
+	require.NoError(t, err)
+
+	err = balancer1.UpsertServer(server)
+	require.NoError(t, err)
+
+	balancer2, err := roundrobin.New(nil)
+	require.NoError(t, err)
+
+	err = balancer2.UpsertServer(server)
+	require.NoError(t, err)
+
+	balancers := Balancers([]Balancer{balancer1, balancer2})
+
+	err = balancers.RemoveServer(server)
+	require.NoError(t, err)
+
+	assert.Equal(t, 0, len(balancer1.Servers()))
+	assert.Equal(t, 0, len(balancer2.Servers()))
+}
+
 type testLoadBalancer struct {
 	// RWMutex needed due to parallel test execution: Both the system-under-test
 	// and the test assertions reference the counters.