diff --git a/integration/consul_catalog_test.go b/integration/consul_catalog_test.go index fdd132122..0241d8ab3 100644 --- a/integration/consul_catalog_test.go +++ b/integration/consul_catalog_test.go @@ -36,7 +36,7 @@ func (s *ConsulCatalogSuite) SetUpSuite(c *check.C) { } func (s *ConsulCatalogSuite) waitToElectConsulLeader() error { - return try.Do(3*time.Second, func() error { + return try.Do(15*time.Second, func() error { leader, err := s.consulClient.Status().Leader() if err != nil || len(leader) == 0 { @@ -344,8 +344,82 @@ func (s *ConsulCatalogSuite) TestBasicAuthSimpleService(c *check.C) { c.Assert(err, checker.IsNil) } -func (s *ConsulCatalogSuite) TestRetryWithConsulServer(c *check.C) { +func (s *ConsulCatalogSuite) TestRefreshConfigTagChange(c *check.C) { + cmd, display := s.traefikCmd( + withConfigFile("fixtures/consul_catalog/simple.toml"), + "--consulCatalog", + "--consulCatalog.exposedByDefault=false", + "--consulCatalog.watch=true", + "--consulCatalog.endpoint="+s.consulIP+":8500", + "--consulCatalog.domain=consul.localhost") + defer display(c) + err := cmd.Start() + c.Assert(err, checker.IsNil) + defer cmd.Process.Kill() + nginx := s.composeProject.Container(c, "nginx1") + + err = s.registerService("test", nginx.NetworkSettings.IPAddress, 80, []string{"name=nginx1", "traefik.enable=false", "traefik.backend.circuitbreaker=NetworkErrorRatio() > 0.5"}) + c.Assert(err, checker.IsNil, check.Commentf("Error registering service")) + defer s.deregisterService("test", nginx.NetworkSettings.IPAddress) + + err = try.GetRequest("http://127.0.0.1:8080/api/providers/consul_catalog/backends", 5*time.Second, try.BodyContains("nginx1")) + c.Assert(err, checker.NotNil) + + err = s.registerService("test", nginx.NetworkSettings.IPAddress, 80, []string{"name=nginx1", "traefik.enable=true", "traefik.backend.circuitbreaker=ResponseCodeRatio(500, 600, 0, 600) > 0.5"}) + c.Assert(err, checker.IsNil, check.Commentf("Error registering service")) + + req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1:8000/", nil) + c.Assert(err, checker.IsNil) + req.Host = "test.consul.localhost" + + err = try.Request(req, 20*time.Second, try.StatusCodeIs(http.StatusOK), try.HasBody()) + c.Assert(err, checker.IsNil) + + err = try.GetRequest("http://127.0.0.1:8080/api/providers/consul_catalog/backends", 60*time.Second, try.BodyContains("nginx1")) + c.Assert(err, checker.IsNil) +} + +func (s *ConsulCatalogSuite) TestCircuitBreaker(c *check.C) { + cmd, display := s.traefikCmd( + withConfigFile("fixtures/consul_catalog/simple.toml"), + "--retry", + "--retry.attempts=1", + "--forwardingTimeouts.dialTimeout=5s", + "--forwardingTimeouts.responseHeaderTimeout=10s", + "--consulCatalog", + "--consulCatalog.exposedByDefault=false", + "--consulCatalog.watch=true", + "--consulCatalog.endpoint="+s.consulIP+":8500", + "--consulCatalog.domain=consul.localhost") + defer display(c) + err := cmd.Start() + c.Assert(err, checker.IsNil) + defer cmd.Process.Kill() + + nginx := s.composeProject.Container(c, "nginx1") + nginx2 := s.composeProject.Container(c, "nginx2") + nginx3 := s.composeProject.Container(c, "nginx3") + + err = s.registerService("test", nginx.NetworkSettings.IPAddress, 80, []string{"name=nginx1", "traefik.enable=true", "traefik.backend.circuitbreaker=NetworkErrorRatio() > 0.5"}) + c.Assert(err, checker.IsNil, check.Commentf("Error registering service")) + defer s.deregisterService("test", nginx.NetworkSettings.IPAddress) + err = s.registerService("test", nginx2.NetworkSettings.IPAddress, 42, []string{"name=nginx2", "traefik.enable=true", "traefik.backend.circuitbreaker=NetworkErrorRatio() > 0.5"}) + c.Assert(err, checker.IsNil, check.Commentf("Error registering service")) + defer s.deregisterService("test", nginx2.NetworkSettings.IPAddress) + err = s.registerService("test", nginx3.NetworkSettings.IPAddress, 42, []string{"name=nginx3", "traefik.enable=true", "traefik.backend.circuitbreaker=NetworkErrorRatio() > 0.5"}) + c.Assert(err, checker.IsNil, check.Commentf("Error registering service")) + defer s.deregisterService("test", nginx3.NetworkSettings.IPAddress) + + req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1:8000/", nil) + c.Assert(err, checker.IsNil) + req.Host = "test.consul.localhost" + + err = try.Request(req, 20*time.Second, try.StatusCodeIs(http.StatusServiceUnavailable), try.HasBody()) + c.Assert(err, checker.IsNil) +} + +func (s *ConsulCatalogSuite) TestRetryWithConsulServer(c *check.C) { //Scale consul to 0 to be able to start traefik before and test retry s.composeProject.Scale(c, "consul", 0) diff --git a/integration/resources/compose/consul_catalog.yml b/integration/resources/compose/consul_catalog.yml index b755d2ad5..9569a32c4 100644 --- a/integration/resources/compose/consul_catalog.yml +++ b/integration/resources/compose/consul_catalog.yml @@ -1,6 +1,6 @@ consul: - image: progrium/consul - command: -server -bootstrap -log-level debug -ui-dir /ui + image: consul + command: agent -server -bootstrap-expect 1 -client 0.0.0.0 -log-level debug -ui ports: - "8400:8400" - "8500:8500" @@ -15,3 +15,5 @@ nginx1: image: nginx:alpine nginx2: image: nginx:alpine +nginx3: + image: nginx:alpine diff --git a/provider/consul/consul_catalog.go b/provider/consul/consul_catalog.go index db9c7bbc2..8387e72fa 100644 --- a/provider/consul/consul_catalog.go +++ b/provider/consul/consul_catalog.go @@ -77,9 +77,14 @@ func (a nodeSorter) Less(i int, j int) bool { return lentr.Service.Port < rentr.Service.Port } -func getChangedServiceKeys(currState map[string]Service, prevState map[string]Service) ([]string, []string) { - currKeySet := fun.Set(fun.Keys(currState).([]string)).(map[string]bool) - prevKeySet := fun.Set(fun.Keys(prevState).([]string)).(map[string]bool) +func hasChanged(current map[string]Service, previous map[string]Service) bool { + addedServiceKeys, removedServiceKeys := getChangedServiceKeys(current, previous) + return len(removedServiceKeys) > 0 || len(addedServiceKeys) > 0 || hasNodeOrTagsChanged(current, previous) +} + +func getChangedServiceKeys(current map[string]Service, previous map[string]Service) ([]string, []string) { + currKeySet := fun.Set(fun.Keys(current).([]string)).(map[string]bool) + prevKeySet := fun.Set(fun.Keys(previous).([]string)).(map[string]bool) addedKeys := fun.Difference(currKeySet, prevKeySet).(map[string]bool) removedKeys := fun.Difference(prevKeySet, currKeySet).(map[string]bool) @@ -87,20 +92,23 @@ func getChangedServiceKeys(currState map[string]Service, prevState map[string]Se return fun.Keys(addedKeys).([]string), fun.Keys(removedKeys).([]string) } -func getChangedServiceNodeKeys(currState map[string]Service, prevState map[string]Service) ([]string, []string) { - var addedNodeKeys []string - var removedNodeKeys []string - for key, value := range currState { - if prevValue, ok := prevState[key]; ok { - addedKeys, removedKeys := getChangedHealthyKeys(value.Nodes, prevValue.Nodes) - addedNodeKeys = append(addedKeys) - removedNodeKeys = append(removedKeys) +func hasNodeOrTagsChanged(current map[string]Service, previous map[string]Service) bool { + var added []string + var removed []string + for key, value := range current { + if prevValue, ok := previous[key]; ok { + addedNodesKeys, removedNodesKeys := getChangedStringKeys(value.Nodes, prevValue.Nodes) + added = append(added, addedNodesKeys...) + removed = append(removed, removedNodesKeys...) + addedTagsKeys, removedTagsKeys := getChangedStringKeys(value.Tags, prevValue.Tags) + added = append(added, addedTagsKeys...) + removed = append(removed, removedTagsKeys...) } } - return addedNodeKeys, removedNodeKeys + return len(added) > 0 || len(removed) > 0 } -func getChangedHealthyKeys(currState []string, prevState []string) ([]string, []string) { +func getChangedStringKeys(currState []string, prevState []string) ([]string, []string) { currKeySet := fun.Set(currState).(map[string]bool) prevKeySet := fun.Set(prevState).(map[string]bool) @@ -163,7 +171,7 @@ func (p *CatalogProvider) watchHealthState(stopCh <-chan struct{}, watchCh chan< // A critical note is that the return of a blocking request is no guarantee of a change. // It is possible that there was an idempotent write that does not affect the result of the query. // Thus it is required to do extra check for changes... - addedKeys, removedKeys := getChangedHealthyKeys(current, flashback) + addedKeys, removedKeys := getChangedStringKeys(current, flashback) if len(addedKeys) > 0 { log.WithField("DiscoveredServices", addedKeys).Debug("Health State change detected.") @@ -242,12 +250,7 @@ func (p *CatalogProvider) watchCatalogServices(stopCh <-chan struct{}, watchCh c // A critical note is that the return of a blocking request is no guarantee of a change. // It is possible that there was an idempotent write that does not affect the result of the query. // Thus it is required to do extra check for changes... - addedServiceKeys, removedServiceKeys := getChangedServiceKeys(current, flashback) - - addedServiceNodeKeys, removedServiceNodeKeys := getChangedServiceNodeKeys(current, flashback) - - if len(removedServiceKeys) > 0 || len(removedServiceNodeKeys) > 0 || len(addedServiceKeys) > 0 || len(addedServiceNodeKeys) > 0 { - log.WithField("MissingServices", removedServiceKeys).WithField("DiscoveredServices", addedServiceKeys).Debug("Catalog Services change detected.") + if hasChanged(current, flashback) { watchCh <- data flashback = current } @@ -255,6 +258,7 @@ func (p *CatalogProvider) watchCatalogServices(stopCh <-chan struct{}, watchCh c } }) } + func getServiceIds(services []*api.CatalogService) []string { var serviceIds []string for _, service := range services { @@ -271,7 +275,6 @@ func (p *CatalogProvider) healthyNodes(service string) (catalogUpdate, error) { log.WithError(err).Errorf("Failed to fetch details of %s", service) return catalogUpdate{}, err } - nodes := fun.Filter(func(node *api.ServiceEntry) bool { return p.nodeFilter(service, node) }, data).([]*api.ServiceEntry) diff --git a/provider/consul/consul_catalog_test.go b/provider/consul/consul_catalog_test.go index 8ffdd3e0d..3e6899f80 100644 --- a/provider/consul/consul_catalog_test.go +++ b/provider/consul/consul_catalog_test.go @@ -1,7 +1,6 @@ package consul import ( - "reflect" "sort" "testing" "text/template" @@ -21,11 +20,13 @@ func TestConsulCatalogGetFrontendRule(t *testing.T) { } provider.setupFrontEndTemplate() - services := []struct { + testCases := []struct { + desc string service serviceUpdate expected string }{ { + desc: "Should return default host foo.localhost", service: serviceUpdate{ ServiceName: "foo", Attributes: []string{}, @@ -33,6 +34,7 @@ func TestConsulCatalogGetFrontendRule(t *testing.T) { expected: "Host:foo.localhost", }, { + desc: "Should return host *.example.com", service: serviceUpdate{ ServiceName: "foo", Attributes: []string{ @@ -42,6 +44,7 @@ func TestConsulCatalogGetFrontendRule(t *testing.T) { expected: "Host:*.example.com", }, { + desc: "Should return host foo.example.com", service: serviceUpdate{ ServiceName: "foo", Attributes: []string{ @@ -51,6 +54,7 @@ func TestConsulCatalogGetFrontendRule(t *testing.T) { expected: "Host:foo.example.com", }, { + desc: "Should return path prefix /bar", service: serviceUpdate{ ServiceName: "foo", Attributes: []string{ @@ -62,11 +66,14 @@ func TestConsulCatalogGetFrontendRule(t *testing.T) { }, } - for _, e := range services { - actual := provider.getFrontendRule(e.service) - if actual != e.expected { - t.Fatalf("expected %s, got %s", e.expected, actual) - } + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + actual := provider.getFrontendRule(test.service) + assert.Equal(t, test.expected, actual) + }) } } @@ -76,13 +83,15 @@ func TestConsulCatalogGetTag(t *testing.T) { Prefix: "traefik", } - services := []struct { + testCases := []struct { + desc string tags []string key string defaultValue string expected string }{ { + desc: "Should return value of foo.bar key", tags: []string{ "foo.bar=random", "traefik.backend.weight=42", @@ -94,21 +103,17 @@ func TestConsulCatalogGetTag(t *testing.T) { }, } - actual := provider.hasTag("management", []string{"management"}) - if !actual { - t.Fatalf("expected %v, got %v", true, actual) - } + assert.Equal(t, true, provider.hasTag("management", []string{"management"})) + assert.Equal(t, true, provider.hasTag("management", []string{"management=yes"})) - actual = provider.hasTag("management", []string{"management=yes"}) - if !actual { - t.Fatalf("expected %v, got %v", true, actual) - } + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() - for _, e := range services { - actual := provider.getTag(e.key, e.tags, e.defaultValue) - if actual != e.expected { - t.Fatalf("expected %s, got %s", e.expected, actual) - } + actual := provider.getTag(test.key, test.tags, test.defaultValue) + assert.Equal(t, test.expected, actual) + }) } } @@ -118,13 +123,15 @@ func TestConsulCatalogGetAttribute(t *testing.T) { Prefix: "traefik", } - services := []struct { + testCases := []struct { + desc string tags []string key string defaultValue string expected string }{ { + desc: "Should return tag value 42", tags: []string{ "foo.bar=ramdom", "traefik.backend.weight=42", @@ -134,6 +141,7 @@ func TestConsulCatalogGetAttribute(t *testing.T) { expected: "42", }, { + desc: "Should return tag default value 0", tags: []string{ "foo.bar=ramdom", "traefik.backend.wei=42", @@ -144,17 +152,16 @@ func TestConsulCatalogGetAttribute(t *testing.T) { }, } - expected := provider.Prefix + ".foo" - actual := provider.getPrefixedName("foo") - if actual != expected { - t.Fatalf("expected %s, got %s", expected, actual) - } + assert.Equal(t, provider.Prefix+".foo", provider.getPrefixedName("foo")) - for _, e := range services { - actual := provider.getAttribute(e.key, e.tags, e.defaultValue) - if actual != e.expected { - t.Fatalf("expected %s, got %s", e.expected, actual) - } + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + actual := provider.getAttribute(test.key, test.tags, test.defaultValue) + assert.Equal(t, test.expected, actual) + }) } } @@ -164,13 +171,15 @@ func TestConsulCatalogGetAttributeWithEmptyPrefix(t *testing.T) { Prefix: "", } - services := []struct { + testCases := []struct { + desc string tags []string key string defaultValue string expected string }{ { + desc: "Should return tag value 42", tags: []string{ "foo.bar=ramdom", "backend.weight=42", @@ -180,6 +189,7 @@ func TestConsulCatalogGetAttributeWithEmptyPrefix(t *testing.T) { expected: "42", }, { + desc: "Should return default value 0", tags: []string{ "foo.bar=ramdom", "backend.wei=42", @@ -189,6 +199,7 @@ func TestConsulCatalogGetAttributeWithEmptyPrefix(t *testing.T) { expected: "0", }, { + desc: "Should return for.bar key value random", tags: []string{ "foo.bar=ramdom", "backend.wei=42", @@ -199,17 +210,16 @@ func TestConsulCatalogGetAttributeWithEmptyPrefix(t *testing.T) { }, } - expected := "foo" - actual := provider.getPrefixedName("foo") - if actual != expected { - t.Fatalf("expected %s, got %s", expected, actual) - } + assert.Equal(t, "foo", provider.getPrefixedName("foo")) - for _, e := range services { - actual := provider.getAttribute(e.key, e.tags, e.defaultValue) - if actual != e.expected { - t.Fatalf("expected %s, got %s", e.expected, actual) - } + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + actual := provider.getAttribute(test.key, test.tags, test.defaultValue) + assert.Equal(t, test.expected, actual) + }) } } @@ -219,11 +229,13 @@ func TestConsulCatalogGetBackendAddress(t *testing.T) { Prefix: "traefik", } - services := []struct { + testCases := []struct { + desc string node *api.ServiceEntry expected string }{ { + desc: "Should return the address of the service", node: &api.ServiceEntry{ Node: &api.Node{ Address: "10.1.0.1", @@ -235,6 +247,7 @@ func TestConsulCatalogGetBackendAddress(t *testing.T) { expected: "10.2.0.1", }, { + desc: "Should return the address of the node", node: &api.ServiceEntry{ Node: &api.Node{ Address: "10.1.0.1", @@ -247,11 +260,14 @@ func TestConsulCatalogGetBackendAddress(t *testing.T) { }, } - for _, e := range services { - actual := provider.getBackendAddress(e.node) - if actual != e.expected { - t.Fatalf("expected %s, got %s", e.expected, actual) - } + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + actual := provider.getBackendAddress(test.node) + assert.Equal(t, test.expected, actual) + }) } } @@ -261,11 +277,13 @@ func TestConsulCatalogGetBackendName(t *testing.T) { Prefix: "traefik", } - services := []struct { + testCases := []struct { + desc string node *api.ServiceEntry expected string }{ { + desc: "Should create backend name without tags", node: &api.ServiceEntry{ Service: &api.AgentService{ Service: "api", @@ -277,6 +295,7 @@ func TestConsulCatalogGetBackendName(t *testing.T) { expected: "api--10-0-0-1--80--0", }, { + desc: "Should create backend name with multiple tags", node: &api.ServiceEntry{ Service: &api.AgentService{ Service: "api", @@ -288,6 +307,7 @@ func TestConsulCatalogGetBackendName(t *testing.T) { expected: "api--10-0-0-1--80--traefik-weight-42--traefik-enable-true--1", }, { + desc: "Should create backend name with one tag", node: &api.ServiceEntry{ Service: &api.AgentService{ Service: "api", @@ -300,11 +320,15 @@ func TestConsulCatalogGetBackendName(t *testing.T) { }, } - for i, e := range services { - actual := provider.getBackendName(e.node, i) - if actual != e.expected { - t.Fatalf("expected %s, got %s", e.expected, actual) - } + for i, test := range testCases { + test := test + i := i + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + actual := provider.getBackendName(test.node, i) + assert.Equal(t, test.expected, actual) + }) } } @@ -317,17 +341,20 @@ func TestConsulCatalogBuildConfig(t *testing.T) { frontEndRuleTemplate: template.New("consul catalog frontend rule"), } - cases := []struct { + testCases := []struct { + desc string nodes []catalogUpdate expectedFrontends map[string]*types.Frontend expectedBackends map[string]*types.Backend }{ { + desc: "Should build config of nothing", nodes: []catalogUpdate{}, expectedFrontends: map[string]*types.Frontend{}, expectedBackends: map[string]*types.Backend{}, }, { + desc: "Should build config with no frontend and backend", nodes: []catalogUpdate{ { Service: &serviceUpdate{ @@ -339,6 +366,7 @@ func TestConsulCatalogBuildConfig(t *testing.T) { expectedBackends: map[string]*types.Backend{}, }, { + desc: "Should build config who contains one frontend and one backend", nodes: []catalogUpdate{ { Service: &serviceUpdate{ @@ -408,28 +436,31 @@ func TestConsulCatalogBuildConfig(t *testing.T) { }, } - for _, c := range cases { - actualConfig := provider.buildConfig(c.nodes) - if !reflect.DeepEqual(actualConfig.Backends, c.expectedBackends) { - t.Fatalf("expected %#v, got %#v", c.expectedBackends, actualConfig.Backends) - } - if !reflect.DeepEqual(actualConfig.Frontends, c.expectedFrontends) { - t.Fatalf("expected %#v, got %#v", c.expectedFrontends["frontend-test"].BasicAuth, actualConfig.Frontends["frontend-test"].BasicAuth) - t.Fatalf("expected %#v, got %#v", c.expectedFrontends, actualConfig.Frontends) - } + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + actualConfig := provider.buildConfig(test.nodes) + assert.Equal(t, test.expectedBackends, actualConfig.Backends) + assert.Equal(t, test.expectedFrontends, actualConfig.Frontends) + }) } } func TestConsulCatalogNodeSorter(t *testing.T) { - cases := []struct { + testCases := []struct { + desc string nodes []*api.ServiceEntry expected []*api.ServiceEntry }{ { + desc: "Should sort nothing", nodes: []*api.ServiceEntry{}, expected: []*api.ServiceEntry{}, }, { + desc: "Should sort by node address", nodes: []*api.ServiceEntry{ { Service: &api.AgentService{ @@ -458,6 +489,7 @@ func TestConsulCatalogNodeSorter(t *testing.T) { }, }, { + desc: "Should sort by service name", nodes: []*api.ServiceEntry{ { Service: &api.AgentService{ @@ -552,6 +584,7 @@ func TestConsulCatalogNodeSorter(t *testing.T) { }, }, { + desc: "Should sort by node address", nodes: []*api.ServiceEntry{ { Service: &api.AgentService{ @@ -603,12 +636,15 @@ func TestConsulCatalogNodeSorter(t *testing.T) { }, } - for _, c := range cases { - sort.Sort(nodeSorter(c.nodes)) - actual := c.nodes - if !reflect.DeepEqual(actual, c.expected) { - t.Fatalf("expected %q, got %q", c.expected, actual) - } + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + sort.Sort(nodeSorter(test.nodes)) + actual := test.nodes + assert.Equal(t, test.expected, actual) + }) } } @@ -623,11 +659,13 @@ func TestConsulCatalogGetChangedKeys(t *testing.T) { removedKeys []string } - cases := []struct { + testCases := []struct { + desc string input Input output Output }{ { + desc: "Should add 0 services and removed 0", input: Input{ currState: map[string]Service{ "foo-service": {Name: "v1"}, @@ -668,6 +706,7 @@ func TestConsulCatalogGetChangedKeys(t *testing.T) { }, }, { + desc: "Should add 3 services and removed 0", input: Input{ currState: map[string]Service{ "foo-service": {Name: "v1"}, @@ -705,6 +744,7 @@ func TestConsulCatalogGetChangedKeys(t *testing.T) { }, }, { + desc: "Should add 2 services and removed 2", input: Input{ currState: map[string]Service{ "foo-service": {Name: "v1"}, @@ -742,21 +782,20 @@ func TestConsulCatalogGetChangedKeys(t *testing.T) { }, } - for _, c := range cases { - addedKeys, removedKeys := getChangedServiceKeys(c.input.currState, c.input.prevState) + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() - if !reflect.DeepEqual(fun.Set(addedKeys), fun.Set(c.output.addedKeys)) { - t.Fatalf("Added keys comparison results: got %q, want %q", addedKeys, c.output.addedKeys) - } - - if !reflect.DeepEqual(fun.Set(removedKeys), fun.Set(c.output.removedKeys)) { - t.Fatalf("Removed keys comparison results: got %q, want %q", removedKeys, c.output.removedKeys) - } + addedKeys, removedKeys := getChangedServiceKeys(test.input.currState, test.input.prevState) + assert.Equal(t, fun.Set(test.output.addedKeys), fun.Set(addedKeys), "Added keys comparison results: got %q, want %q", addedKeys, test.output.addedKeys) + assert.Equal(t, fun.Set(test.output.removedKeys), fun.Set(removedKeys), "Removed keys comparison results: got %q, want %q", removedKeys, test.output.removedKeys) + }) } } func TestConsulCatalogFilterEnabled(t *testing.T) { - cases := []struct { + testCases := []struct { desc string exposedByDefault bool node *api.ServiceEntry @@ -842,24 +881,23 @@ func TestConsulCatalogFilterEnabled(t *testing.T) { }, } - for _, c := range cases { - c := c - t.Run(c.desc, func(t *testing.T) { + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { t.Parallel() provider := &CatalogProvider{ Domain: "localhost", Prefix: "traefik", - ExposedByDefault: c.exposedByDefault, - } - if provider.nodeFilter("test", c.node) != c.expected { - t.Errorf("got unexpected filtering = %t", !c.expected) + ExposedByDefault: test.exposedByDefault, } + actual := provider.nodeFilter("test", test.node) + assert.Equal(t, test.expected, actual) }) } } func TestConsulCatalogGetBasicAuth(t *testing.T) { - cases := []struct { + testCases := []struct { desc string tags []string expected []string @@ -878,17 +916,15 @@ func TestConsulCatalogGetBasicAuth(t *testing.T) { }, } - for _, c := range cases { - c := c - t.Run(c.desc, func(t *testing.T) { + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { t.Parallel() provider := &CatalogProvider{ Prefix: "traefik", } - actual := provider.getBasicAuth(c.tags) - if !reflect.DeepEqual(actual, c.expected) { - t.Errorf("actual %q, expected %q", actual, c.expected) - } + actual := provider.getBasicAuth(test.tags) + assert.Equal(t, test.expected, actual) }) } } @@ -930,7 +966,276 @@ func TestConsulCatalogHasStickinessLabel(t *testing.T) { t.Parallel() actual := provider.hasStickinessLabel(test.tags) - assert.Equal(t, actual, test.expected) + assert.Equal(t, test.expected, actual) + }) + } +} + +func TestConsulCatalogGetChangedStringKeys(t *testing.T) { + testCases := []struct { + desc string + current []string + previous []string + expectedAdded []string + expectedRemoved []string + }{ + { + desc: "1 element added, 0 removed", + current: []string{"chou"}, + previous: []string{}, + expectedAdded: []string{"chou"}, + expectedRemoved: []string{}, + }, { + desc: "0 element added, 0 removed", + current: []string{"chou"}, + previous: []string{"chou"}, + expectedAdded: []string{}, + expectedRemoved: []string{}, + }, + { + desc: "0 element added, 1 removed", + current: []string{}, + previous: []string{"chou"}, + expectedAdded: []string{}, + expectedRemoved: []string{"chou"}, + }, + { + desc: "1 element added, 1 removed", + current: []string{"carotte"}, + previous: []string{"chou"}, + expectedAdded: []string{"carotte"}, + expectedRemoved: []string{"chou"}, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + actualAdded, actualRemoved := getChangedStringKeys(test.current, test.previous) + assert.Equal(t, test.expectedAdded, actualAdded) + assert.Equal(t, test.expectedRemoved, actualRemoved) + }) + } +} + +func TestConsulCatalogHasNodeOrTagschanged(t *testing.T) { + testCases := []struct { + desc string + current map[string]Service + previous map[string]Service + expected bool + }{ + { + desc: "Change detected due to change of nodes", + current: map[string]Service{ + "foo-service": { + Name: "foo", + Nodes: []string{"node1"}, + Tags: []string{}, + }, + }, + previous: map[string]Service{ + "foo-service": { + Name: "foo", + Nodes: []string{"node2"}, + Tags: []string{}, + }, + }, + expected: true, + }, + { + desc: "No change missing current service", + current: make(map[string]Service), + previous: map[string]Service{ + "foo-service": { + Name: "foo", + Nodes: []string{"node1"}, + Tags: []string{}, + }, + }, + expected: false, + }, + { + desc: "No change on nodes", + current: map[string]Service{ + "foo-service": { + Name: "foo", + Nodes: []string{"node1"}, + Tags: []string{}, + }, + }, + previous: map[string]Service{ + "foo-service": { + Name: "foo", + Nodes: []string{"node1"}, + Tags: []string{}, + }, + }, + expected: false, + }, + { + desc: "No change on nodes and tags", + current: map[string]Service{ + "foo-service": { + Name: "foo", + Nodes: []string{"node1"}, + Tags: []string{"foo=bar"}, + }, + }, + previous: map[string]Service{ + "foo-service": { + Name: "foo", + Nodes: []string{"node1"}, + Tags: []string{"foo=bar"}, + }, + }, + expected: false, + }, + { + desc: "Change detected con tags", + current: map[string]Service{ + "foo-service": { + Name: "foo", + Nodes: []string{"node1"}, + Tags: []string{"foo=bar"}, + }, + }, + previous: map[string]Service{ + "foo-service": { + Name: "foo", + Nodes: []string{"node1"}, + Tags: []string{"foo"}, + }, + }, + expected: true, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + actual := hasNodeOrTagsChanged(test.current, test.previous) + assert.Equal(t, test.expected, actual) + }) + } +} + +func TestConsulCatalogHasChanged(t *testing.T) { + testCases := []struct { + desc string + current map[string]Service + previous map[string]Service + expected bool + }{ + { + desc: "Change detected due to change new service", + current: map[string]Service{ + "foo-service": { + Name: "foo", + Nodes: []string{"node1"}, + Tags: []string{}, + }, + }, + previous: make(map[string]Service), + expected: true, + }, + { + desc: "Change detected due to change service removed", + current: make(map[string]Service), + previous: map[string]Service{ + "foo-service": { + Name: "foo", + Nodes: []string{"node1"}, + Tags: []string{}, + }, + }, + expected: true, + }, + { + desc: "Change detected due to change of nodes", + current: map[string]Service{ + "foo-service": { + Name: "foo", + Nodes: []string{"node1"}, + Tags: []string{}, + }, + }, + previous: map[string]Service{ + "foo-service": { + Name: "foo", + Nodes: []string{"node2"}, + Tags: []string{}, + }, + }, + expected: true, + }, + { + desc: "No change on nodes", + current: map[string]Service{ + "foo-service": { + Name: "foo", + Nodes: []string{"node1"}, + Tags: []string{}, + }, + }, + previous: map[string]Service{ + "foo-service": { + Name: "foo", + Nodes: []string{"node1"}, + Tags: []string{}, + }, + }, + expected: false, + }, + { + desc: "No change on nodes and tags", + current: map[string]Service{ + "foo-service": { + Name: "foo", + Nodes: []string{"node1"}, + Tags: []string{"foo=bar"}, + }, + }, + previous: map[string]Service{ + "foo-service": { + Name: "foo", + Nodes: []string{"node1"}, + Tags: []string{"foo=bar"}, + }, + }, + expected: false, + }, + { + desc: "Change detected on tags", + current: map[string]Service{ + "foo-service": { + Name: "foo", + Nodes: []string{"node1"}, + Tags: []string{"foo=bar"}, + }, + }, + previous: map[string]Service{ + "foo-service": { + Name: "foo", + Nodes: []string{"node1"}, + Tags: []string{"foo"}, + }, + }, + expected: true, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + actual := hasChanged(test.current, test.previous) + assert.Equal(t, test.expected, actual) }) } }