Fix Traefik reload if Consul Catalog tags change

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)
 

integration/resources/compose/consul_catalog.yml

@@ -1,6 +1,6 @@
 consul:
-  image: progrium/consul
+  image: consul
-  command: -server -bootstrap -log-level debug -ui-dir /ui
+  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

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) {
+func hasChanged(current map[string]Service, previous map[string]Service) bool {
-	currKeySet := fun.Set(fun.Keys(currState).([]string)).(map[string]bool)
+	addedServiceKeys, removedServiceKeys := getChangedServiceKeys(current, previous)
-	prevKeySet := fun.Set(fun.Keys(prevState).([]string)).(map[string]bool)
+	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) {
+func hasNodeOrTagsChanged(current map[string]Service, previous map[string]Service) bool {
-	var addedNodeKeys []string
+	var added []string
-	var removedNodeKeys []string
+	var removed []string
-	for key, value := range currState {
+	for key, value := range current {
-		if prevValue, ok := prevState[key]; ok {
+		if prevValue, ok := previous[key]; ok {
-			addedKeys, removedKeys := getChangedHealthyKeys(value.Nodes, prevValue.Nodes)
+			addedNodesKeys, removedNodesKeys := getChangedStringKeys(value.Nodes, prevValue.Nodes)
-			addedNodeKeys = append(addedKeys)
+			added = append(added, addedNodesKeys...)
-			removedNodeKeys = append(removedKeys)
+			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)
+				if hasChanged(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.")
 					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)

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 {
+	for _, test := range testCases {
-		actual := provider.getFrontendRule(e.service)
+		test := test
-		if actual != e.expected {
+		t.Run(test.desc, func(t *testing.T) {
-			t.Fatalf("expected %s, got %s", e.expected, actual)
+			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"})
+	assert.Equal(t, true, provider.hasTag("management", []string{"management"}))
-	if !actual {
+	assert.Equal(t, true, provider.hasTag("management", []string{"management=yes"}))
-		t.Fatalf("expected %v, got %v", true, actual)
-	}
 
-	actual = provider.hasTag("management", []string{"management=yes"})
+	for _, test := range testCases {
-	if !actual {
+		test := test
-		t.Fatalf("expected %v, got %v", true, actual)
+		t.Run(test.desc, func(t *testing.T) {
-	}
+			t.Parallel()
 
-	for _, e := range services {
+			actual := provider.getTag(test.key, test.tags, test.defaultValue)
-		actual := provider.getTag(e.key, e.tags, e.defaultValue)
+			assert.Equal(t, test.expected, actual)
-		if actual != e.expected {
+		})
-			t.Fatalf("expected %s, got %s", e.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"
+	assert.Equal(t, provider.Prefix+".foo", provider.getPrefixedName("foo"))
-	actual := provider.getPrefixedName("foo")
-	if actual != expected {
-		t.Fatalf("expected %s, got %s", expected, actual)
-	}
 
-	for _, e := range services {
+	for _, test := range testCases {
-		actual := provider.getAttribute(e.key, e.tags, e.defaultValue)
+		test := test
-		if actual != e.expected {
+		t.Run(test.desc, func(t *testing.T) {
-			t.Fatalf("expected %s, got %s", e.expected, actual)
+			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"
+	assert.Equal(t, "foo", provider.getPrefixedName("foo"))
-	actual := provider.getPrefixedName("foo")
-	if actual != expected {
-		t.Fatalf("expected %s, got %s", expected, actual)
-	}
 
-	for _, e := range services {
+	for _, test := range testCases {
-		actual := provider.getAttribute(e.key, e.tags, e.defaultValue)
+		test := test
-		if actual != e.expected {
+		t.Run(test.desc, func(t *testing.T) {
-			t.Fatalf("expected %s, got %s", e.expected, actual)
+			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 {
+	for _, test := range testCases {
-		actual := provider.getBackendAddress(e.node)
+		test := test
-		if actual != e.expected {
+		t.Run(test.desc, func(t *testing.T) {
-			t.Fatalf("expected %s, got %s", e.expected, actual)
+			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 {
+	for i, test := range testCases {
-		actual := provider.getBackendName(e.node, i)
+		test := test
-		if actual != e.expected {
+		i := i
-			t.Fatalf("expected %s, got %s", e.expected, actual)
+		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 {
+	for _, test := range testCases {
-		actualConfig := provider.buildConfig(c.nodes)
+		test := test
-		if !reflect.DeepEqual(actualConfig.Backends, c.expectedBackends) {
+		t.Run(test.desc, func(t *testing.T) {
-			t.Fatalf("expected %#v, got %#v", c.expectedBackends, actualConfig.Backends)
+			t.Parallel()
-		}
+
-		if !reflect.DeepEqual(actualConfig.Frontends, c.expectedFrontends) {
+			actualConfig := provider.buildConfig(test.nodes)
-			t.Fatalf("expected %#v, got %#v", c.expectedFrontends["frontend-test"].BasicAuth, actualConfig.Frontends["frontend-test"].BasicAuth)
+			assert.Equal(t, test.expectedBackends, actualConfig.Backends)
-			t.Fatalf("expected %#v, got %#v", c.expectedFrontends, actualConfig.Frontends)
+			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 {
+	for _, test := range testCases {
-		sort.Sort(nodeSorter(c.nodes))
+		test := test
-		actual := c.nodes
+		t.Run(test.desc, func(t *testing.T) {
-		if !reflect.DeepEqual(actual, c.expected) {
+			t.Parallel()
-			t.Fatalf("expected %q, got %q", c.expected, actual)
+
-		}
+			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 {
+	for _, test := range testCases {
-		addedKeys, removedKeys := getChangedServiceKeys(c.input.currState, c.input.prevState)
+		test := test
+		t.Run(test.desc, func(t *testing.T) {
+			t.Parallel()
 
-		if !reflect.DeepEqual(fun.Set(addedKeys), fun.Set(c.output.addedKeys)) {
+			addedKeys, removedKeys := getChangedServiceKeys(test.input.currState, test.input.prevState)
-			t.Fatalf("Added keys comparison results: got %q, want %q", addedKeys, c.output.addedKeys)
+			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)
-
+		})
-		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)
-		}
 	}
 }
 
 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 {
+	for _, test := range testCases {
-		c := c
+		test := test
-		t.Run(c.desc, func(t *testing.T) {
+		t.Run(test.desc, func(t *testing.T) {
 			t.Parallel()
 			provider := &CatalogProvider{
 				Domain:           "localhost",
 				Prefix:           "traefik",
-				ExposedByDefault: c.exposedByDefault,
+				ExposedByDefault: test.exposedByDefault,
-			}
-			if provider.nodeFilter("test", c.node) != c.expected {
-				t.Errorf("got unexpected filtering = %t", !c.expected)
 			}
+			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 {
+	for _, test := range testCases {
-		c := c
+		test := test
-		t.Run(c.desc, func(t *testing.T) {
+		t.Run(test.desc, func(t *testing.T) {
 			t.Parallel()
 			provider := &CatalogProvider{
 				Prefix: "traefik",
 			}
-			actual := provider.getBasicAuth(c.tags)
+			actual := provider.getBasicAuth(test.tags)
-			if !reflect.DeepEqual(actual, c.expected) {
+			assert.Equal(t, test.expected, actual)
-				t.Errorf("actual %q, expected %q", actual, c.expected)
-			}
 		})
 	}
 }
@@ -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)
 		})
 	}
 }