From ef19ede14adbb601d9736369b517b20b446ffab7 Mon Sep 17 00:00:00 2001 From: Vasily Sliouniaev Date: Tue, 16 Apr 2019 11:25:19 +0100 Subject: [PATCH 1/3] Prevent reshard concurrent with calling stop (#5460) * Prevent reshard concurrent with calling stop Signed-off-by: Vasily --- storage/remote/queue_manager.go | 5 ++++- storage/remote/queue_manager_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/storage/remote/queue_manager.go b/storage/remote/queue_manager.go index e9a5dba87..9dd50ee60 100644 --- a/storage/remote/queue_manager.go +++ b/storage/remote/queue_manager.go @@ -322,9 +322,12 @@ func (t *QueueManager) Stop() { defer level.Info(t.logger).Log("msg", "Remote storage stopped.") close(t.quit) + t.wg.Wait() + // Wait for all QueueManager routines to end before stopping shards and WAL watcher. This + // is to ensure we don't end up executing a reshard and shards.stop() at the same time, which + // causes a closed channel panic. t.shards.stop() t.watcher.Stop() - t.wg.Wait() // On shutdown, release the strings in the labels from the intern pool. t.seriesMtx.Lock() diff --git a/storage/remote/queue_manager_test.go b/storage/remote/queue_manager_test.go index 48ecc5052..f709c452f 100644 --- a/storage/remote/queue_manager_test.go +++ b/storage/remote/queue_manager_test.go @@ -233,6 +233,30 @@ func TestReshard(t *testing.T) { c.waitForExpectedSamples(t) } +func TestReshardRaceWithStop(t *testing.T) { + c := NewTestStorageClient() + var m *QueueManager + h := sync.Mutex{} + + h.Lock() + + go func() { + for { + m = NewQueueManager(nil, "", newEWMARate(ewmaWeight, shardUpdateDuration), config.DefaultQueueConfig, nil, nil, c, defaultFlushDeadline) + m.Start() + h.Unlock() + h.Lock() + m.Stop() + } + }() + + for i := 1; i < 100; i++ { + h.Lock() + m.reshardChan <- i + h.Unlock() + } +} + func createTimeseries(n int) ([]tsdb.RefSample, []tsdb.RefSeries) { samples := make([]tsdb.RefSample, 0, n) series := make([]tsdb.RefSeries, 0, n) From b7169df0c1b84da0b6089b089d341eb28312391d Mon Sep 17 00:00:00 2001 From: Simon Pasquier Date: Mon, 15 Apr 2019 20:04:50 +0200 Subject: [PATCH 2/3] discovery/kubernetes: fix missing label sanitization (#5462) Signed-off-by: Simon Pasquier --- discovery/kubernetes/endpoints_test.go | 20 ++++++++-------- discovery/kubernetes/ingress_test.go | 16 ++++++------- discovery/kubernetes/node_test.go | 14 +++++------ discovery/kubernetes/pod.go | 2 +- discovery/kubernetes/pod_test.go | 32 +++++++++++++------------- discovery/kubernetes/service_test.go | 16 ++++++------- 6 files changed, 50 insertions(+), 50 deletions(-) diff --git a/discovery/kubernetes/endpoints_test.go b/discovery/kubernetes/endpoints_test.go index 04c0d01ff..bd7f072fd 100644 --- a/discovery/kubernetes/endpoints_test.go +++ b/discovery/kubernetes/endpoints_test.go @@ -357,7 +357,7 @@ func TestEndpointsDiscoveryWithService(t *testing.T) { Name: "testendpoints", Namespace: "default", Labels: map[string]string{ - "app": "test", + "app/name": "test", }, }, } @@ -387,11 +387,11 @@ func TestEndpointsDiscoveryWithService(t *testing.T) { }, }, Labels: model.LabelSet{ - "__meta_kubernetes_namespace": "default", - "__meta_kubernetes_endpoints_name": "testendpoints", - "__meta_kubernetes_service_label_app": "test", - "__meta_kubernetes_service_labelpresent_app": "true", - "__meta_kubernetes_service_name": "testendpoints", + "__meta_kubernetes_namespace": "default", + "__meta_kubernetes_endpoints_name": "testendpoints", + "__meta_kubernetes_service_label_app_name": "test", + "__meta_kubernetes_service_labelpresent_app_name": "true", + "__meta_kubernetes_service_name": "testendpoints", }, Source: "endpoints/default/testendpoints", }, @@ -410,7 +410,7 @@ func TestEndpointsDiscoveryWithServiceUpdate(t *testing.T) { Name: "testendpoints", Namespace: "default", Labels: map[string]string{ - "app": "test", + "app/name": "test", }, }, } @@ -422,7 +422,7 @@ func TestEndpointsDiscoveryWithServiceUpdate(t *testing.T) { Name: "testendpoints", Namespace: "default", Labels: map[string]string{ - "app": "svc", + "app/name": "svc", "component": "testing", }, }, @@ -455,8 +455,8 @@ func TestEndpointsDiscoveryWithServiceUpdate(t *testing.T) { Labels: model.LabelSet{ "__meta_kubernetes_namespace": "default", "__meta_kubernetes_endpoints_name": "testendpoints", - "__meta_kubernetes_service_label_app": "svc", - "__meta_kubernetes_service_labelpresent_app": "true", + "__meta_kubernetes_service_label_app_name": "svc", + "__meta_kubernetes_service_labelpresent_app_name": "true", "__meta_kubernetes_service_name": "testendpoints", "__meta_kubernetes_service_label_component": "testing", "__meta_kubernetes_service_labelpresent_component": "true", diff --git a/discovery/kubernetes/ingress_test.go b/discovery/kubernetes/ingress_test.go index d7973e8b1..ffe3bffc3 100644 --- a/discovery/kubernetes/ingress_test.go +++ b/discovery/kubernetes/ingress_test.go @@ -36,8 +36,8 @@ func makeIngress(tls TLSMode) *v1beta1.Ingress { ObjectMeta: metav1.ObjectMeta{ Name: "testingress", Namespace: "default", - Labels: map[string]string{"testlabel": "testvalue"}, - Annotations: map[string]string{"testannotation": "testannotationvalue"}, + Labels: map[string]string{"test/label": "testvalue"}, + Annotations: map[string]string{"test/annotation": "testannotationvalue"}, }, Spec: v1beta1.IngressSpec{ TLS: nil, @@ -118,12 +118,12 @@ func expectedTargetGroups(ns string, tls TLSMode) map[string]*targetgroup.Group }, }, Labels: model.LabelSet{ - "__meta_kubernetes_ingress_name": "testingress", - "__meta_kubernetes_namespace": lv(ns), - "__meta_kubernetes_ingress_label_testlabel": "testvalue", - "__meta_kubernetes_ingress_labelpresent_testlabel": "true", - "__meta_kubernetes_ingress_annotation_testannotation": "testannotationvalue", - "__meta_kubernetes_ingress_annotationpresent_testannotation": "true", + "__meta_kubernetes_ingress_name": "testingress", + "__meta_kubernetes_namespace": lv(ns), + "__meta_kubernetes_ingress_label_test_label": "testvalue", + "__meta_kubernetes_ingress_labelpresent_test_label": "true", + "__meta_kubernetes_ingress_annotation_test_annotation": "testannotationvalue", + "__meta_kubernetes_ingress_annotationpresent_test_annotation": "true", }, Source: key, }, diff --git a/discovery/kubernetes/node_test.go b/discovery/kubernetes/node_test.go index 2eaf49bf3..da92eefce 100644 --- a/discovery/kubernetes/node_test.go +++ b/discovery/kubernetes/node_test.go @@ -59,8 +59,8 @@ func TestNodeDiscoveryBeforeStart(t *testing.T) { obj := makeNode( "test", "1.2.3.4", - map[string]string{"testlabel": "testvalue"}, - map[string]string{"testannotation": "testannotationvalue"}, + map[string]string{"test-label": "testvalue"}, + map[string]string{"test-annotation": "testannotationvalue"}, ) c.CoreV1().Nodes().Create(obj) }, @@ -75,11 +75,11 @@ func TestNodeDiscoveryBeforeStart(t *testing.T) { }, }, Labels: model.LabelSet{ - "__meta_kubernetes_node_name": "test", - "__meta_kubernetes_node_label_testlabel": "testvalue", - "__meta_kubernetes_node_labelpresent_testlabel": "true", - "__meta_kubernetes_node_annotation_testannotation": "testannotationvalue", - "__meta_kubernetes_node_annotationpresent_testannotation": "true", + "__meta_kubernetes_node_name": "test", + "__meta_kubernetes_node_label_test_label": "testvalue", + "__meta_kubernetes_node_labelpresent_test_label": "true", + "__meta_kubernetes_node_annotation_test_annotation": "testannotationvalue", + "__meta_kubernetes_node_annotationpresent_test_annotation": "true", }, Source: "node/test", }, diff --git a/discovery/kubernetes/pod.go b/discovery/kubernetes/pod.go index baa531331..5ee8e959d 100644 --- a/discovery/kubernetes/pod.go +++ b/discovery/kubernetes/pod.go @@ -188,7 +188,7 @@ func podLabels(pod *apiv1.Pod) model.LabelSet { for k, v := range pod.Labels { ln := strutil.SanitizeLabelName(k) - ls[model.LabelName(podLabelPrefix+k)] = lv(v) + ls[model.LabelName(podLabelPrefix+ln)] = lv(v) ls[model.LabelName(podLabelPresentPrefix+ln)] = presentValue } diff --git a/discovery/kubernetes/pod_test.go b/discovery/kubernetes/pod_test.go index 0548716a3..fecdbffd9 100644 --- a/discovery/kubernetes/pod_test.go +++ b/discovery/kubernetes/pod_test.go @@ -33,8 +33,8 @@ func makeMultiPortPods() *v1.Pod { ObjectMeta: metav1.ObjectMeta{ Name: "testpod", Namespace: "default", - Labels: map[string]string{"testlabel": "testvalue"}, - Annotations: map[string]string{"testannotation": "testannotationvalue"}, + Labels: map[string]string{"test/label": "testvalue"}, + Annotations: map[string]string{"test/annotation": "testannotationvalue"}, UID: types.UID("abc123"), OwnerReferences: []metav1.OwnerReference{ { @@ -178,20 +178,20 @@ func TestPodDiscoveryBeforeRun(t *testing.T) { }, }, Labels: model.LabelSet{ - "__meta_kubernetes_pod_name": "testpod", - "__meta_kubernetes_namespace": "default", - "__meta_kubernetes_pod_label_testlabel": "testvalue", - "__meta_kubernetes_pod_labelpresent_testlabel": "true", - "__meta_kubernetes_pod_annotation_testannotation": "testannotationvalue", - "__meta_kubernetes_pod_annotationpresent_testannotation": "true", - "__meta_kubernetes_pod_node_name": "testnode", - "__meta_kubernetes_pod_ip": "1.2.3.4", - "__meta_kubernetes_pod_host_ip": "2.3.4.5", - "__meta_kubernetes_pod_ready": "true", - "__meta_kubernetes_pod_phase": "Running", - "__meta_kubernetes_pod_uid": "abc123", - "__meta_kubernetes_pod_controller_kind": "testcontrollerkind", - "__meta_kubernetes_pod_controller_name": "testcontrollername", + "__meta_kubernetes_pod_name": "testpod", + "__meta_kubernetes_namespace": "default", + "__meta_kubernetes_pod_label_test_label": "testvalue", + "__meta_kubernetes_pod_labelpresent_test_label": "true", + "__meta_kubernetes_pod_annotation_test_annotation": "testannotationvalue", + "__meta_kubernetes_pod_annotationpresent_test_annotation": "true", + "__meta_kubernetes_pod_node_name": "testnode", + "__meta_kubernetes_pod_ip": "1.2.3.4", + "__meta_kubernetes_pod_host_ip": "2.3.4.5", + "__meta_kubernetes_pod_ready": "true", + "__meta_kubernetes_pod_phase": "Running", + "__meta_kubernetes_pod_uid": "abc123", + "__meta_kubernetes_pod_controller_kind": "testcontrollerkind", + "__meta_kubernetes_pod_controller_name": "testcontrollername", }, Source: "pod/default/testpod", }, diff --git a/discovery/kubernetes/service_test.go b/discovery/kubernetes/service_test.go index 585bbd49a..783b5d06b 100644 --- a/discovery/kubernetes/service_test.go +++ b/discovery/kubernetes/service_test.go @@ -28,8 +28,8 @@ func makeMultiPortService() *v1.Service { ObjectMeta: metav1.ObjectMeta{ Name: "testservice", Namespace: "default", - Labels: map[string]string{"testlabel": "testvalue"}, - Annotations: map[string]string{"testannotation": "testannotationvalue"}, + Labels: map[string]string{"test-label": "testvalue"}, + Annotations: map[string]string{"test-annotation": "testannotationvalue"}, }, Spec: v1.ServiceSpec{ Ports: []v1.ServicePort{ @@ -186,12 +186,12 @@ func TestServiceDiscoveryUpdate(t *testing.T) { }, }, Labels: model.LabelSet{ - "__meta_kubernetes_service_name": "testservice", - "__meta_kubernetes_namespace": "default", - "__meta_kubernetes_service_label_testlabel": "testvalue", - "__meta_kubernetes_service_labelpresent_testlabel": "true", - "__meta_kubernetes_service_annotation_testannotation": "testannotationvalue", - "__meta_kubernetes_service_annotationpresent_testannotation": "true", + "__meta_kubernetes_service_name": "testservice", + "__meta_kubernetes_namespace": "default", + "__meta_kubernetes_service_label_test_label": "testvalue", + "__meta_kubernetes_service_labelpresent_test_label": "true", + "__meta_kubernetes_service_annotation_test_annotation": "testannotationvalue", + "__meta_kubernetes_service_annotationpresent_test_annotation": "true", }, Source: "svc/default/testservice", }, From a9309a2e1e1175104378a584b15e28492aac2e85 Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Tue, 16 Apr 2019 12:53:40 +0100 Subject: [PATCH 3/3] Prepare 2.9.1 Signed-off-by: Brian Brazil --- CHANGELOG.md | 5 +++++ VERSION | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66b0b4fb3..cdf961a1e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## 2.9.1 / 2019-04-16 + +* [BUGFIX] Discovery/kubernetes: fix missing label sanitization #5462 +* [BUGFIX] Remote_write: Prevent reshard concurrent with calling stop #5460 + ## 2.9.0 / 2019-04-15 This releases uses Go 1.12, which includes a change in how memory is released diff --git a/VERSION b/VERSION index c8e38b614..dedcc7d43 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.9.0 +2.9.1