From 9970e4e7e48b4277301bdbc54d21bc50f45a1aa1 Mon Sep 17 00:00:00 2001 From: Augustin Husson Date: Wed, 5 Sep 2018 15:12:07 +0200 Subject: [PATCH 1/6] unregister source when the target is empty Signed-off-by: Augustin Husson --- discovery/manager.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/discovery/manager.go b/discovery/manager.go index bb4409fea..ab8328924 100644 --- a/discovery/manager.go +++ b/discovery/manager.go @@ -232,6 +232,11 @@ func (m *Manager) updateGroup(poolKey poolKey, tgs []*targetgroup.Group) { m.targets[poolKey] = make(map[string]*targetgroup.Group) } m.targets[poolKey][tg.Source] = tg + + // Clear the key in the case where the targets is empty. + if tg.Targets == nil || len(tg.Targets) <= 0 { + delete(m.targets[poolKey], tg.Source) + } } } } From 97950a3faea3e88f643708f25cc88159e5441cce Mon Sep 17 00:00:00 2001 From: Augustin Husson Date: Sun, 16 Sep 2018 14:36:09 +0200 Subject: [PATCH 2/6] remove group if the target is empty at adapter level Signed-off-by: Augustin Husson --- discovery/manager.go | 5 ----- documentation/examples/custom-sd/adapter/adapter.go | 6 ++++++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/discovery/manager.go b/discovery/manager.go index ab8328924..bb4409fea 100644 --- a/discovery/manager.go +++ b/discovery/manager.go @@ -232,11 +232,6 @@ func (m *Manager) updateGroup(poolKey poolKey, tgs []*targetgroup.Group) { m.targets[poolKey] = make(map[string]*targetgroup.Group) } m.targets[poolKey][tg.Source] = tg - - // Clear the key in the case where the targets is empty. - if tg.Targets == nil || len(tg.Targets) <= 0 { - delete(m.targets[poolKey], tg.Source) - } } } } diff --git a/documentation/examples/custom-sd/adapter/adapter.go b/documentation/examples/custom-sd/adapter/adapter.go index 6452a50f9..95bf09163 100644 --- a/documentation/examples/custom-sd/adapter/adapter.go +++ b/documentation/examples/custom-sd/adapter/adapter.go @@ -60,6 +60,12 @@ func (a *Adapter) generateTargetGroups(allTargetGroups map[string][]*targetgroup tempGroups := make(map[string]*customSD) for k, sdTargetGroups := range allTargetGroups { for i, group := range sdTargetGroups { + + // There is no target, so no need to keep it + if len(group.Targets) <= 0 { + continue + } + newTargets := make([]string, 0) newLabels := make(map[string]string) From e03869de76fdb4fe8079641317f3c60c8a229e88 Mon Sep 17 00:00:00 2001 From: Augustin Husson Date: Sun, 16 Sep 2018 23:39:03 +0200 Subject: [PATCH 3/6] add unit test and isolate the method that generate the target Signed-off-by: Augustin Husson --- .../examples/custom-sd/adapter/adapter.go | 29 ++-- .../custom-sd/adapter/adapter_test.go | 151 ++++++++++++++++++ 2 files changed, 168 insertions(+), 12 deletions(-) create mode 100644 documentation/examples/custom-sd/adapter/adapter_test.go diff --git a/documentation/examples/custom-sd/adapter/adapter.go b/documentation/examples/custom-sd/adapter/adapter.go index 95bf09163..39cfea289 100644 --- a/documentation/examples/custom-sd/adapter/adapter.go +++ b/documentation/examples/custom-sd/adapter/adapter.go @@ -18,15 +18,14 @@ import ( "context" "encoding/json" "fmt" - "io/ioutil" - "os" - "path/filepath" - "reflect" - "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" "github.com/prometheus/prometheus/discovery" "github.com/prometheus/prometheus/discovery/targetgroup" + "io/ioutil" + "os" + "path/filepath" + "reflect" ) type customSD struct { @@ -54,10 +53,8 @@ func mapToArray(m map[string]*customSD) []customSD { return arr } -// Parses incoming target groups updates. If the update contains changes to the target groups -// Adapter already knows about, or new target groups, we Marshal to JSON and write to file. -func (a *Adapter) generateTargetGroups(allTargetGroups map[string][]*targetgroup.Group) { - tempGroups := make(map[string]*customSD) +func generateTargetGroups(allTargetGroups map[string][]*targetgroup.Group) map[string]*customSD { + groups := make(map[string]*customSD) for k, sdTargetGroups := range allTargetGroups { for i, group := range sdTargetGroups { @@ -80,12 +77,21 @@ func (a *Adapter) generateTargetGroups(allTargetGroups map[string][]*targetgroup } // Make a unique key, including the current index, in case the sd_type (map key) and group.Source is not unique. key := fmt.Sprintf("%s:%s:%d", k, group.Source, i) - tempGroups[key] = &customSD{ + groups[key] = &customSD{ Targets: newTargets, Labels: newLabels, } } } + + return groups +} + +// Parses incoming target groups updates. If the update contains changes to the target groups +// Adapter already knows about, or new target groups, we Marshal to JSON and write to file. +func (a *Adapter) refreshTargetGroups(allTargetGroups map[string][]*targetgroup.Group) { + tempGroups := generateTargetGroups(allTargetGroups) + if !reflect.DeepEqual(a.groups, tempGroups) { a.groups = tempGroups err := a.writeOutput() @@ -93,7 +99,6 @@ func (a *Adapter) generateTargetGroups(allTargetGroups map[string][]*targetgroup level.Error(log.With(a.logger, "component", "sd-adapter")).Log("err", err) } } - } // Writes JSON formatted targets to output file. @@ -131,7 +136,7 @@ func (a *Adapter) runCustomSD(ctx context.Context) { if !ok { return } - a.generateTargetGroups(allTargetGroups) + a.refreshTargetGroups(allTargetGroups) } } } diff --git a/documentation/examples/custom-sd/adapter/adapter_test.go b/documentation/examples/custom-sd/adapter/adapter_test.go new file mode 100644 index 000000000..291f2e082 --- /dev/null +++ b/documentation/examples/custom-sd/adapter/adapter_test.go @@ -0,0 +1,151 @@ +// Copyright 2018 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package adapter + +import ( + "github.com/prometheus/common/model" + "github.com/prometheus/prometheus/discovery/targetgroup" + "reflect" + "testing" +) + +// TestGenerateTargetGroups checks that the target is correctly generated. +// It covers the case when the target is empty +func TestGenerateTargetGroups(t *testing.T) { + testCases := []struct { + title string + targetGroup map[string][]*targetgroup.Group + expectedCustomSD map[string]*customSD + }{ + { + title: "Empty targetGroup", + targetGroup: map[string][]*targetgroup.Group{ + "customSD": { + { + Source: "Consul", + }, + { + Source: "Kubernetes", + }, + }, + }, + expectedCustomSD: map[string]*customSD{}, + }, + { + title: "targetGroup filled", + targetGroup: map[string][]*targetgroup.Group{ + "customSD": { + { + Source: "Azure", + Targets: []model.LabelSet{ + { + model.AddressLabel: "host1", + }, + { + model.AddressLabel: "host2", + }, + }, + Labels: model.LabelSet{ + model.LabelName("__meta_test_label"): model.LabelValue("label_test_1"), + }, + }, + { + Source: "Openshift", + Targets: []model.LabelSet{ + { + model.AddressLabel: "host3", + }, + { + model.AddressLabel: "host4", + }, + }, + Labels: model.LabelSet{ + model.LabelName("__meta_test_label"): model.LabelValue("label_test_2"), + }, + }, + }, + }, + expectedCustomSD: map[string]*customSD{ + "customSD:Azure:0": { + Targets: []string{ + "host1", + "host2", + }, + Labels: map[string]string{ + "__meta_test_label": "label_test_1", + }, + }, + "customSD:Openshift:1": { + Targets: []string{ + "host3", + "host4", + }, + Labels: map[string]string{ + "__meta_test_label": "label_test_2", + }, + }, + }, + }, + { + title: "Mixed between empty targetGroup and targetGroup filled", + targetGroup: map[string][]*targetgroup.Group{ + "customSD": { + { + Source: "GCE", + Targets: []model.LabelSet{ + { + model.AddressLabel: "host1", + }, + { + model.AddressLabel: "host2", + }, + }, + Labels: model.LabelSet{ + model.LabelName("__meta_test_label"): model.LabelValue("label_test_1"), + }, + }, + { + Source: "Kubernetes", + Labels: model.LabelSet{ + model.LabelName("__meta_test_label"): model.LabelValue("label_test_2"), + }, + }, + }, + }, + expectedCustomSD: map[string]*customSD{ + "customSD:GCE:0": { + Targets: []string{ + "host1", + "host2", + }, + Labels: map[string]string{ + "__meta_test_label": "label_test_1", + }, + }, + }, + }, + } + + for _, testCase := range testCases { + result := generateTargetGroups(testCase.targetGroup) + + if !reflect.DeepEqual(result, testCase.expectedCustomSD) { + t.Errorf("%v :\nresult produced %v\nmismatch the expected customSD: %v", + testCase.title, + result, + testCase.expectedCustomSD) + } + + } +} From 3c0b130e5e3918d90e86de17dd056b17482edf86 Mon Sep 17 00:00:00 2001 From: Augustin Husson Date: Tue, 18 Sep 2018 11:08:38 +0200 Subject: [PATCH 4/6] apply review Signed-off-by: Augustin Husson --- documentation/examples/custom-sd/adapter/adapter.go | 9 +++++---- documentation/examples/custom-sd/adapter/adapter_test.go | 7 ++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/documentation/examples/custom-sd/adapter/adapter.go b/documentation/examples/custom-sd/adapter/adapter.go index 39cfea289..048a7be34 100644 --- a/documentation/examples/custom-sd/adapter/adapter.go +++ b/documentation/examples/custom-sd/adapter/adapter.go @@ -18,14 +18,15 @@ import ( "context" "encoding/json" "fmt" - "github.com/go-kit/kit/log" - "github.com/go-kit/kit/log/level" - "github.com/prometheus/prometheus/discovery" - "github.com/prometheus/prometheus/discovery/targetgroup" "io/ioutil" "os" "path/filepath" "reflect" + + "github.com/go-kit/kit/log" + "github.com/go-kit/kit/log/level" + "github.com/prometheus/prometheus/discovery" + "github.com/prometheus/prometheus/discovery/targetgroup" ) type customSD struct { diff --git a/documentation/examples/custom-sd/adapter/adapter_test.go b/documentation/examples/custom-sd/adapter/adapter_test.go index 291f2e082..563e1c108 100644 --- a/documentation/examples/custom-sd/adapter/adapter_test.go +++ b/documentation/examples/custom-sd/adapter/adapter_test.go @@ -14,10 +14,11 @@ package adapter import ( - "github.com/prometheus/common/model" - "github.com/prometheus/prometheus/discovery/targetgroup" "reflect" "testing" + + "github.com/prometheus/common/model" + "github.com/prometheus/prometheus/discovery/targetgroup" ) // TestGenerateTargetGroups checks that the target is correctly generated. @@ -141,7 +142,7 @@ func TestGenerateTargetGroups(t *testing.T) { result := generateTargetGroups(testCase.targetGroup) if !reflect.DeepEqual(result, testCase.expectedCustomSD) { - t.Errorf("%v :\nresult produced %v\nmismatch the expected customSD: %v", + t.Errorf("%q failed\ngot: %#v\nexpected: %v", testCase.title, result, testCase.expectedCustomSD) From 9e6dc6f96c21700612a881323e635ce7ae6d3a10 Mon Sep 17 00:00:00 2001 From: Augustin Husson Date: Tue, 25 Sep 2018 19:05:02 +0200 Subject: [PATCH 5/6] fix targetGroup that disappear totally Signed-off-by: Augustin Husson --- .../examples/custom-sd/adapter-usage/main.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/documentation/examples/custom-sd/adapter-usage/main.go b/documentation/examples/custom-sd/adapter-usage/main.go index 9a5123eab..c8484891c 100644 --- a/documentation/examples/custom-sd/adapter-usage/main.go +++ b/documentation/examples/custom-sd/adapter-usage/main.go @@ -94,6 +94,7 @@ type discovery struct { clientDatacenter string tagSeparator string logger log.Logger + oldSourceList map[string]bool } func (d *discovery) parseServiceNodes(resp *http.Response, name string) (*targetgroup.Group, error) { @@ -180,6 +181,8 @@ func (d *discovery) Run(ctx context.Context, ch chan<- []*targetgroup.Group) { // list of targets simply because there may have been a timeout. If the service is actually // gone as far as consul is concerned, that will be picked up during the next iteration of // the outer loop. + + newSourceList := make(map[string]bool) for name := range srvs { if name == "consul" { continue @@ -195,7 +198,17 @@ func (d *discovery) Run(ctx context.Context, ch chan<- []*targetgroup.Group) { break } tgs = append(tgs, tg) + newSourceList[tg.Source] = true } + // when targetGroup disappear, send an update with empty targetList + for key := range d.oldSourceList { + if !newSourceList[key] { + tgs = append(tgs, &targetgroup.Group{ + Source: key, + }) + } + } + d.oldSourceList = newSourceList if err == nil { // We're returning all Consul services as a single targetgroup. ch <- tgs @@ -216,6 +229,7 @@ func newDiscovery(conf sdConfig) (*discovery, error) { refreshInterval: conf.RefreshInterval, tagSeparator: conf.TagSeparator, logger: logger, + oldSourceList: make(map[string]bool), } return cd, nil } From f60620ec0b65b0b18ad2af9feff5f59f8fda47b0 Mon Sep 17 00:00:00 2001 From: Augustin Husson Date: Wed, 26 Sep 2018 10:48:35 +0200 Subject: [PATCH 6/6] format comment Signed-off-by: Augustin Husson --- documentation/examples/custom-sd/adapter-usage/main.go | 2 +- documentation/examples/custom-sd/adapter/adapter.go | 2 +- documentation/examples/custom-sd/adapter/adapter_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/documentation/examples/custom-sd/adapter-usage/main.go b/documentation/examples/custom-sd/adapter-usage/main.go index c8484891c..40bb07e2a 100644 --- a/documentation/examples/custom-sd/adapter-usage/main.go +++ b/documentation/examples/custom-sd/adapter-usage/main.go @@ -200,7 +200,7 @@ func (d *discovery) Run(ctx context.Context, ch chan<- []*targetgroup.Group) { tgs = append(tgs, tg) newSourceList[tg.Source] = true } - // when targetGroup disappear, send an update with empty targetList + // When targetGroup disappear, send an update with empty targetList. for key := range d.oldSourceList { if !newSourceList[key] { tgs = append(tgs, &targetgroup.Group{ diff --git a/documentation/examples/custom-sd/adapter/adapter.go b/documentation/examples/custom-sd/adapter/adapter.go index 048a7be34..3d9e7afe1 100644 --- a/documentation/examples/custom-sd/adapter/adapter.go +++ b/documentation/examples/custom-sd/adapter/adapter.go @@ -59,7 +59,7 @@ func generateTargetGroups(allTargetGroups map[string][]*targetgroup.Group) map[s for k, sdTargetGroups := range allTargetGroups { for i, group := range sdTargetGroups { - // There is no target, so no need to keep it + // There is no target, so no need to keep it. if len(group.Targets) <= 0 { continue } diff --git a/documentation/examples/custom-sd/adapter/adapter_test.go b/documentation/examples/custom-sd/adapter/adapter_test.go index 563e1c108..812da09d0 100644 --- a/documentation/examples/custom-sd/adapter/adapter_test.go +++ b/documentation/examples/custom-sd/adapter/adapter_test.go @@ -22,7 +22,7 @@ import ( ) // TestGenerateTargetGroups checks that the target is correctly generated. -// It covers the case when the target is empty +// It covers the case when the target is empty. func TestGenerateTargetGroups(t *testing.T) { testCases := []struct { title string