diff --git a/pkg/healthcheck/healthcheck.go b/pkg/healthcheck/healthcheck.go index 000ee2d00..97a4264c9 100644 --- a/pkg/healthcheck/healthcheck.go +++ b/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 +} diff --git a/pkg/healthcheck/healthcheck_test.go b/pkg/healthcheck/healthcheck_test.go index 88972419e..fc71eafdd 100644 --- a/pkg/healthcheck/healthcheck_test.go +++ b/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.