From 4d675e2aa88caed5e5d18fc7ce329792def2d2b2 Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Tue, 12 Jun 2018 08:07:09 -0400 Subject: [PATCH 01/10] web: restore old path prefix behavior Signed-off-by: Fabian Reinartz --- web/web.go | 8 +------- web/web_test.go | 36 ------------------------------------ 2 files changed, 1 insertion(+), 43 deletions(-) diff --git a/web/web.go b/web/web.go index 4e671e5ce..2d7cc3874 100644 --- a/web/web.go +++ b/web/web.go @@ -770,13 +770,7 @@ func tmplFuncs(consolesPath string, opts *Options) template_text.FuncMap { return time.Since(t) / time.Millisecond * time.Millisecond }, "consolesPath": func() string { return consolesPath }, - "pathPrefix": func() string { - if opts.RoutePrefix == "/" { - return "" - } else { - return opts.RoutePrefix - } - }, + "pathPrefix": func() string { return opts.ExternalURL.Path }, "buildVersion": func() string { return opts.Version.Revision }, "stripLabels": func(lset map[string]string, labels ...string) map[string]string { for _, ln := range labels { diff --git a/web/web_test.go b/web/web_test.go index 359a65073..31c1e0020 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -319,42 +319,6 @@ func TestRoutePrefix(t *testing.T) { testutil.Ok(t, err) testutil.Equals(t, http.StatusOK, resp.StatusCode) } - -func TestPathPrefix(t *testing.T) { - - tests := []struct { - routePrefix string - pathPrefix string - }{ - { - routePrefix: "/", - // If we have pathPrefix as "/", URL in UI gets "//"" as prefix, - // hence doesn't remain relative path anymore. - pathPrefix: "", - }, - { - routePrefix: "/prometheus", - pathPrefix: "/prometheus", - }, - { - routePrefix: "/p1/p2/p3/p4", - pathPrefix: "/p1/p2/p3/p4", - }, - } - - for _, test := range tests { - opts := &Options{ - RoutePrefix: test.routePrefix, - } - - pathPrefix := tmplFuncs("", opts)["pathPrefix"].(func() string) - pp := pathPrefix() - - testutil.Equals(t, test.pathPrefix, pp) - } - -} - func TestDebugHandler(t *testing.T) { for _, tc := range []struct { prefix, url string From 8cd59da857d99b831738a6a43f5ad10d5b0631b7 Mon Sep 17 00:00:00 2001 From: Simon Pasquier Date: Thu, 14 Jun 2018 16:49:43 +0200 Subject: [PATCH 02/10] kubernetes_sd: fix namespace filtering (#4273) Signed-off-by: Simon Pasquier --- discovery/kubernetes/endpoints_test.go | 135 +++++++++++++++++++++++++ discovery/kubernetes/ingress_test.go | 36 +++++-- discovery/kubernetes/kubernetes.go | 30 +++--- discovery/kubernetes/pod_test.go | 98 +++++++++--------- discovery/kubernetes/service_test.go | 47 +++++++++ 5 files changed, 282 insertions(+), 64 deletions(-) diff --git a/discovery/kubernetes/endpoints_test.go b/discovery/kubernetes/endpoints_test.go index aebb067f1..35dd5b52a 100644 --- a/discovery/kubernetes/endpoints_test.go +++ b/discovery/kubernetes/endpoints_test.go @@ -19,6 +19,7 @@ import ( "github.com/prometheus/common/model" "github.com/prometheus/prometheus/discovery/targetgroup" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/pkg/api/v1" ) @@ -468,3 +469,137 @@ func TestEndpointsDiscoveryWithServiceUpdate(t *testing.T) { }, }.Run(t) } + +func TestEndpointsDiscoveryNamespaces(t *testing.T) { + epOne := makeEndpoints() + epOne.Namespace = "ns1" + objs := []runtime.Object{ + epOne, + &v1.Endpoints{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testendpoints", + Namespace: "ns2", + }, + Subsets: []v1.EndpointSubset{ + { + Addresses: []v1.EndpointAddress{ + { + IP: "4.3.2.1", + TargetRef: &v1.ObjectReference{ + Kind: "Pod", + Name: "testpod", + Namespace: "ns2", + }, + }, + }, + Ports: []v1.EndpointPort{ + { + Name: "testport", + Port: 9000, + Protocol: v1.ProtocolTCP, + }, + }, + }, + }, + }, + &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testendpoints", + Namespace: "ns1", + Labels: map[string]string{ + "app": "app1", + }, + }, + }, + &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testpod", + Namespace: "ns2", + UID: types.UID("deadbeef"), + }, + Spec: v1.PodSpec{ + NodeName: "testnode", + Containers: []v1.Container{ + { + Name: "c1", + Ports: []v1.ContainerPort{ + { + Name: "mainport", + ContainerPort: 9000, + Protocol: v1.ProtocolTCP, + }, + }, + }, + }, + }, + Status: v1.PodStatus{ + HostIP: "2.3.4.5", + PodIP: "4.3.2.1", + }, + }, + } + n, _, _ := makeDiscovery(RoleEndpoint, NamespaceDiscovery{Names: []string{"ns1", "ns2"}}, objs...) + + k8sDiscoveryTest{ + discovery: n, + expectedMaxItems: 2, + expectedRes: map[string]*targetgroup.Group{ + "endpoints/ns1/testendpoints": { + Targets: []model.LabelSet{ + { + "__address__": "1.2.3.4:9000", + "__meta_kubernetes_endpoint_port_name": "testport", + "__meta_kubernetes_endpoint_port_protocol": "TCP", + "__meta_kubernetes_endpoint_ready": "true", + }, + { + "__address__": "2.3.4.5:9001", + "__meta_kubernetes_endpoint_port_name": "testport", + "__meta_kubernetes_endpoint_port_protocol": "TCP", + "__meta_kubernetes_endpoint_ready": "true", + }, + { + "__address__": "2.3.4.5:9001", + "__meta_kubernetes_endpoint_port_name": "testport", + "__meta_kubernetes_endpoint_port_protocol": "TCP", + "__meta_kubernetes_endpoint_ready": "false", + }, + }, + Labels: model.LabelSet{ + "__meta_kubernetes_namespace": "ns1", + "__meta_kubernetes_endpoints_name": "testendpoints", + "__meta_kubernetes_service_label_app": "app1", + "__meta_kubernetes_service_name": "testendpoints", + }, + Source: "endpoints/ns1/testendpoints", + }, + "endpoints/ns2/testendpoints": { + Targets: []model.LabelSet{ + { + "__address__": "4.3.2.1:9000", + "__meta_kubernetes_endpoint_port_name": "testport", + "__meta_kubernetes_endpoint_port_protocol": "TCP", + "__meta_kubernetes_endpoint_ready": "true", + "__meta_kubernetes_endpoint_address_target_kind": "Pod", + "__meta_kubernetes_endpoint_address_target_name": "testpod", + "__meta_kubernetes_pod_name": "testpod", + "__meta_kubernetes_pod_ip": "4.3.2.1", + "__meta_kubernetes_pod_ready": "unknown", + "__meta_kubernetes_pod_node_name": "testnode", + "__meta_kubernetes_pod_host_ip": "2.3.4.5", + "__meta_kubernetes_pod_container_name": "c1", + "__meta_kubernetes_pod_container_port_name": "mainport", + "__meta_kubernetes_pod_container_port_number": "9000", + "__meta_kubernetes_pod_container_port_protocol": "TCP", + "__meta_kubernetes_pod_uid": "deadbeef", + }, + }, + Labels: model.LabelSet{ + "__meta_kubernetes_namespace": "ns2", + "__meta_kubernetes_endpoints_name": "testendpoints", + }, + Source: "endpoints/ns2/testendpoints", + }, + }, + }.Run(t) +} diff --git a/discovery/kubernetes/ingress_test.go b/discovery/kubernetes/ingress_test.go index c6cd1c9b8..b3832ebce 100644 --- a/discovery/kubernetes/ingress_test.go +++ b/discovery/kubernetes/ingress_test.go @@ -14,6 +14,7 @@ package kubernetes import ( + "fmt" "testing" "github.com/prometheus/common/model" @@ -64,13 +65,14 @@ func makeIngress(tls []v1beta1.IngressTLS) *v1beta1.Ingress { } } -func expectedTargetGroups(tls bool) map[string]*targetgroup.Group { +func expectedTargetGroups(ns string, tls bool) map[string]*targetgroup.Group { scheme := "http" if tls { scheme = "https" } + key := fmt.Sprintf("ingress/%s/testingress", ns) return map[string]*targetgroup.Group{ - "ingress/default/testingress": { + key: { Targets: []model.LabelSet{ { "__meta_kubernetes_ingress_scheme": lv(scheme), @@ -93,11 +95,11 @@ func expectedTargetGroups(tls bool) map[string]*targetgroup.Group { }, Labels: model.LabelSet{ "__meta_kubernetes_ingress_name": "testingress", - "__meta_kubernetes_namespace": "default", + "__meta_kubernetes_namespace": lv(ns), "__meta_kubernetes_ingress_label_testlabel": "testvalue", "__meta_kubernetes_ingress_annotation_testannotation": "testannotationvalue", }, - Source: "ingress/default/testingress", + Source: key, }, } } @@ -113,7 +115,7 @@ func TestIngressDiscoveryAdd(t *testing.T) { w.Ingresses().Add(obj) }, expectedMaxItems: 1, - expectedRes: expectedTargetGroups(false), + expectedRes: expectedTargetGroups("default", false), }.Run(t) } @@ -128,6 +130,28 @@ func TestIngressDiscoveryAddTLS(t *testing.T) { w.Ingresses().Add(obj) }, expectedMaxItems: 1, - expectedRes: expectedTargetGroups(true), + expectedRes: expectedTargetGroups("default", true), + }.Run(t) +} + +func TestIngressDiscoveryNamespaces(t *testing.T) { + n, c, w := makeDiscovery(RoleIngress, NamespaceDiscovery{Names: []string{"ns1", "ns2"}}) + + expected := expectedTargetGroups("ns1", false) + for k, v := range expectedTargetGroups("ns2", false) { + expected[k] = v + } + k8sDiscoveryTest{ + discovery: n, + afterStart: func() { + for _, ns := range []string{"ns1", "ns2"} { + obj := makeIngress(nil) + obj.Namespace = ns + c.ExtensionsV1beta1().Ingresses(obj.Namespace).Create(obj) + w.Ingresses().Add(obj) + } + }, + expectedMaxItems: 2, + expectedRes: expected, }.Run(t) } diff --git a/discovery/kubernetes/kubernetes.go b/discovery/kubernetes/kubernetes.go index a55afde07..637a4eb1d 100644 --- a/discovery/kubernetes/kubernetes.go +++ b/discovery/kubernetes/kubernetes.go @@ -250,28 +250,31 @@ func (d *Discovery) Run(ctx context.Context, ch chan<- []*targetgroup.Group) { switch d.role { case RoleEndpoint: for _, namespace := range namespaces { + e := d.client.CoreV1().Endpoints(namespace) elw := &cache.ListWatch{ ListFunc: func(options metav1.ListOptions) (runtime.Object, error) { - return d.client.CoreV1().Endpoints(namespace).List(options) + return e.List(options) }, WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) { - return d.client.CoreV1().Endpoints(namespace).Watch(options) + return e.Watch(options) }, } + s := d.client.CoreV1().Services(namespace) slw := &cache.ListWatch{ ListFunc: func(options metav1.ListOptions) (runtime.Object, error) { - return d.client.CoreV1().Services(namespace).List(options) + return s.List(options) }, WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) { - return d.client.CoreV1().Services(namespace).Watch(options) + return s.Watch(options) }, } + p := d.client.CoreV1().Pods(namespace) plw := &cache.ListWatch{ ListFunc: func(options metav1.ListOptions) (runtime.Object, error) { - return d.client.CoreV1().Pods(namespace).List(options) + return p.List(options) }, WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) { - return d.client.CoreV1().Pods(namespace).Watch(options) + return p.Watch(options) }, } eps := NewEndpoints( @@ -287,12 +290,13 @@ func (d *Discovery) Run(ctx context.Context, ch chan<- []*targetgroup.Group) { } case RolePod: for _, namespace := range namespaces { + p := d.client.CoreV1().Pods(namespace) plw := &cache.ListWatch{ ListFunc: func(options metav1.ListOptions) (runtime.Object, error) { - return d.client.CoreV1().Pods(namespace).List(options) + return p.List(options) }, WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) { - return d.client.CoreV1().Pods(namespace).Watch(options) + return p.Watch(options) }, } pod := NewPod( @@ -304,12 +308,13 @@ func (d *Discovery) Run(ctx context.Context, ch chan<- []*targetgroup.Group) { } case RoleService: for _, namespace := range namespaces { + s := d.client.CoreV1().Services(namespace) slw := &cache.ListWatch{ ListFunc: func(options metav1.ListOptions) (runtime.Object, error) { - return d.client.CoreV1().Services(namespace).List(options) + return s.List(options) }, WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) { - return d.client.CoreV1().Services(namespace).Watch(options) + return s.Watch(options) }, } svc := NewService( @@ -321,12 +326,13 @@ func (d *Discovery) Run(ctx context.Context, ch chan<- []*targetgroup.Group) { } case RoleIngress: for _, namespace := range namespaces { + i := d.client.ExtensionsV1beta1().Ingresses(namespace) ilw := &cache.ListWatch{ ListFunc: func(options metav1.ListOptions) (runtime.Object, error) { - return d.client.ExtensionsV1beta1().Ingresses(namespace).List(options) + return i.List(options) }, WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) { - return d.client.ExtensionsV1beta1().Ingresses(namespace).Watch(options) + return i.Watch(options) }, } ingress := NewIngress( diff --git a/discovery/kubernetes/pod_test.go b/discovery/kubernetes/pod_test.go index a9e1a9838..b921bf9c7 100644 --- a/discovery/kubernetes/pod_test.go +++ b/discovery/kubernetes/pod_test.go @@ -14,6 +14,7 @@ package kubernetes import ( + "fmt" "testing" "github.com/prometheus/common/model" @@ -114,6 +115,33 @@ func makePods() *v1.Pod { } } +func expectedPodTargetGroups(ns string) map[string]*targetgroup.Group { + key := fmt.Sprintf("pod/%s/testpod", ns) + return map[string]*targetgroup.Group{ + key: { + Targets: []model.LabelSet{ + { + "__address__": "1.2.3.4:9000", + "__meta_kubernetes_pod_container_name": "testcontainer", + "__meta_kubernetes_pod_container_port_name": "testport", + "__meta_kubernetes_pod_container_port_number": "9000", + "__meta_kubernetes_pod_container_port_protocol": "TCP", + }, + }, + Labels: model.LabelSet{ + "__meta_kubernetes_pod_name": "testpod", + "__meta_kubernetes_namespace": lv(ns), + "__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_uid": "abc123", + }, + Source: key, + }, + } +} + func TestPodDiscoveryBeforeRun(t *testing.T) { n, c, w := makeDiscovery(RolePod, NamespaceDiscovery{}) @@ -177,29 +205,7 @@ func TestPodDiscoveryAdd(t *testing.T) { w.Pods().Add(obj) }, expectedMaxItems: 1, - expectedRes: map[string]*targetgroup.Group{ - "pod/default/testpod": { - Targets: []model.LabelSet{ - { - "__address__": "1.2.3.4:9000", - "__meta_kubernetes_pod_container_name": "testcontainer", - "__meta_kubernetes_pod_container_port_name": "testport", - "__meta_kubernetes_pod_container_port_number": "9000", - "__meta_kubernetes_pod_container_port_protocol": "TCP", - }, - }, - Labels: model.LabelSet{ - "__meta_kubernetes_pod_name": "testpod", - "__meta_kubernetes_namespace": "default", - "__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_uid": "abc123", - }, - Source: "pod/default/testpod", - }, - }, + expectedRes: expectedPodTargetGroups("default"), }.Run(t) } @@ -260,29 +266,7 @@ func TestPodDiscoveryUpdate(t *testing.T) { w.Pods().Modify(obj) }, expectedMaxItems: 2, - expectedRes: map[string]*targetgroup.Group{ - "pod/default/testpod": { - Targets: []model.LabelSet{ - { - "__address__": "1.2.3.4:9000", - "__meta_kubernetes_pod_container_name": "testcontainer", - "__meta_kubernetes_pod_container_port_name": "testport", - "__meta_kubernetes_pod_container_port_number": "9000", - "__meta_kubernetes_pod_container_port_protocol": "TCP", - }, - }, - Labels: model.LabelSet{ - "__meta_kubernetes_pod_name": "testpod", - "__meta_kubernetes_namespace": "default", - "__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_uid": "abc123", - }, - Source: "pod/default/testpod", - }, - }, + expectedRes: expectedPodTargetGroups("default"), }.Run(t) } @@ -311,3 +295,25 @@ func TestPodDiscoveryUpdateEmptyPodIP(t *testing.T) { }, }.Run(t) } + +func TestPodDiscoveryNamespaces(t *testing.T) { + n, c, w := makeDiscovery(RolePod, NamespaceDiscovery{Names: []string{"ns1", "ns2"}}) + + expected := expectedPodTargetGroups("ns1") + for k, v := range expectedPodTargetGroups("ns2") { + expected[k] = v + } + k8sDiscoveryTest{ + discovery: n, + beforeRun: func() { + for _, ns := range []string{"ns1", "ns2"} { + pod := makePods() + pod.Namespace = ns + c.CoreV1().Pods(pod.Namespace).Create(pod) + w.Pods().Add(pod) + } + }, + expectedMaxItems: 2, + expectedRes: expected, + }.Run(t) +} diff --git a/discovery/kubernetes/service_test.go b/discovery/kubernetes/service_test.go index db5feab72..e300c850a 100644 --- a/discovery/kubernetes/service_test.go +++ b/discovery/kubernetes/service_test.go @@ -155,3 +155,50 @@ func TestServiceDiscoveryUpdate(t *testing.T) { }, }.Run(t) } + +func TestServiceDiscoveryNamespaces(t *testing.T) { + n, c, w := makeDiscovery(RoleService, NamespaceDiscovery{Names: []string{"ns1", "ns2"}}) + + k8sDiscoveryTest{ + discovery: n, + afterStart: func() { + for _, ns := range []string{"ns1", "ns2"} { + obj := makeService() + obj.Namespace = ns + c.CoreV1().Services(obj.Namespace).Create(obj) + w.Services().Add(obj) + } + }, + expectedMaxItems: 2, + expectedRes: map[string]*targetgroup.Group{ + "svc/ns1/testservice": { + Targets: []model.LabelSet{ + { + "__meta_kubernetes_service_port_protocol": "TCP", + "__address__": "testservice.ns1.svc:30900", + "__meta_kubernetes_service_port_name": "testport", + }, + }, + Labels: model.LabelSet{ + "__meta_kubernetes_service_name": "testservice", + "__meta_kubernetes_namespace": "ns1", + }, + Source: "svc/ns1/testservice", + }, + "svc/ns2/testservice": { + Targets: []model.LabelSet{ + { + "__meta_kubernetes_service_port_protocol": "TCP", + "__address__": "testservice.ns2.svc:30900", + "__meta_kubernetes_service_port_name": "testport", + }, + }, + Labels: model.LabelSet{ + "__meta_kubernetes_service_name": "testservice", + "__meta_kubernetes_namespace": "ns2", + }, + Source: "svc/ns2/testservice", + }, + }, + }.Run(t) +} From 78efdc6d6b283efeab27fc1ed77b37d9b8339e84 Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Mon, 18 Jun 2018 17:34:08 +0100 Subject: [PATCH 03/10] Avoid infinite loop on duplicate NaN values. (#4275) Fixes #4254 NaNs don't equal themselves, so a duplicate NaN would always hit the break statement and never get popped. We should not be returning multiple data point for the same timestamp, so don't compare values at all. Signed-off-by: Brian Brazil --- storage/fanout.go | 6 +++--- storage/fanout_test.go | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/storage/fanout.go b/storage/fanout.go index 328287159..3ab994391 100644 --- a/storage/fanout.go +++ b/storage/fanout.go @@ -450,10 +450,10 @@ func (c *mergeIterator) Next() bool { return false } - currt, currv := c.At() + currt, _ := c.At() for len(c.h) > 0 { - nextt, nextv := c.h[0].At() - if nextt != currt || nextv != currv { + nextt, _ := c.h[0].At() + if nextt != currt { break } diff --git a/storage/fanout_test.go b/storage/fanout_test.go index f6298c859..3328673ef 100644 --- a/storage/fanout_test.go +++ b/storage/fanout_test.go @@ -15,6 +15,7 @@ package storage import ( "fmt" + "math" "testing" "github.com/stretchr/testify/require" @@ -97,6 +98,16 @@ func TestMergeSeriesSet(t *testing.T) { newMockSeries(labels.FromStrings("foo", "bar"), []sample{{0, 0}, {1, 1}, {2, 2}, {3, 3}}), ), }, + { + input: []SeriesSet{newMockSeriesSet( + newMockSeries(labels.FromStrings("foo", "bar"), []sample{{0, math.NaN()}}), + ), newMockSeriesSet( + newMockSeries(labels.FromStrings("foo", "bar"), []sample{{0, math.NaN()}}), + )}, + expected: newMockSeriesSet( + newMockSeries(labels.FromStrings("foo", "bar"), []sample{{0, math.NaN()}}), + ), + }, } { merged := NewMergeSeriesSet(tc.input) for merged.Next() { @@ -197,6 +208,10 @@ func drainSamples(iter SeriesIterator) []sample { result := []sample{} for iter.Next() { t, v := iter.At() + // NaNs can't be compared normally, so substitute for another value. + if math.IsNaN(v) { + v = -42 + } result = append(result, sample{t, v}) } return result From 5c70213f9f1d2eb5c326e22f1945c92e7f73fa69 Mon Sep 17 00:00:00 2001 From: Paul Gier Date: Wed, 13 Jun 2018 10:34:59 -0500 Subject: [PATCH 04/10] config: set target group source index during unmarshalling (#4245) * config: set target group source index during unmarshalling Fixes issue #4214 where the scrape pool is unnecessarily reloaded for a config reload where the config hasn't changed. Previously, the discovery manager changed the static config after loading which caused the in-memory config to differ from a freshly reloaded config. Signed-off-by: Paul Gier * [issue #4214] Test that static targets are not modified by discovery manager Signed-off-by: Paul Gier --- config/config.go | 14 ++++++++++++++ config/config_test.go | 4 ++++ discovery/manager.go | 11 +---------- discovery/manager_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 58 insertions(+), 10 deletions(-) diff --git a/config/config.go b/config/config.go index 746aae3bf..b7a566764 100644 --- a/config/config.go +++ b/config/config.go @@ -370,6 +370,13 @@ func (c *ScrapeConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { } } } + + // Add index to the static config target groups for unique identification + // within scrape pool. + for i, tg := range c.ServiceDiscoveryConfig.StaticConfigs { + tg.Source = fmt.Sprintf("%d", i) + } + return nil } @@ -432,6 +439,13 @@ func (c *AlertmanagerConfig) UnmarshalYAML(unmarshal func(interface{}) error) er } } } + + // Add index to the static config target groups for unique identification + // within scrape pool. + for i, tg := range c.ServiceDiscoveryConfig.StaticConfigs { + tg.Source = fmt.Sprintf("%d", i) + } + return nil } diff --git a/config/config_test.go b/config/config_test.go index 8caaebd55..549a4f909 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -128,6 +128,7 @@ var expectedConf = &Config{ "my": "label", "your": "label", }, + Source: "0", }, }, @@ -484,6 +485,7 @@ var expectedConf = &Config{ Targets: []model.LabelSet{ {model.AddressLabel: "localhost:9090"}, }, + Source: "0", }, }, }, @@ -503,6 +505,7 @@ var expectedConf = &Config{ Targets: []model.LabelSet{ {model.AddressLabel: "localhost:9090"}, }, + Source: "0", }, }, }, @@ -548,6 +551,7 @@ var expectedConf = &Config{ {model.AddressLabel: "1.2.3.5:9093"}, {model.AddressLabel: "1.2.3.6:9093"}, }, + Source: "0", }, }, }, diff --git a/discovery/manager.go b/discovery/manager.go index 669a91dc5..97468a549 100644 --- a/discovery/manager.go +++ b/discovery/manager.go @@ -285,7 +285,7 @@ func (m *Manager) providersFromConfig(cfg sd_config.ServiceDiscoveryConfig) map[ app("triton", i, t) } if len(cfg.StaticConfigs) > 0 { - app("static", 0, NewStaticProvider(cfg.StaticConfigs)) + app("static", 0, &StaticProvider{cfg.StaticConfigs}) } return providers @@ -296,15 +296,6 @@ type StaticProvider struct { TargetGroups []*targetgroup.Group } -// NewStaticProvider returns a StaticProvider configured with the given -// target groups. -func NewStaticProvider(groups []*targetgroup.Group) *StaticProvider { - for i, tg := range groups { - tg.Source = fmt.Sprintf("%d", i) - } - return &StaticProvider{groups} -} - // Run implements the Worker interface. func (sd *StaticProvider) Run(ctx context.Context, ch chan<- []*targetgroup.Group) { // We still have to consider that the consumer exits right away in which case diff --git a/discovery/manager_test.go b/discovery/manager_test.go index c57351270..19efb7d62 100644 --- a/discovery/manager_test.go +++ b/discovery/manager_test.go @@ -774,6 +774,45 @@ scrape_configs: verifyPresence(discoveryManager.targets, poolKey{setName: "prometheus", provider: "static/0"}, "{__address__=\"bar:9090\"}", false) } +func TestApplyConfigDoesNotModifyStaticProviderTargets(t *testing.T) { + cfgText := ` +scrape_configs: + - job_name: 'prometheus' + static_configs: + - targets: ["foo:9090"] + - targets: ["bar:9090"] + - targets: ["baz:9090"] +` + originalConfig := &config.Config{} + if err := yaml.UnmarshalStrict([]byte(cfgText), originalConfig); err != nil { + t.Fatalf("Unable to load YAML config cfgYaml: %s", err) + } + origScrpCfg := originalConfig.ScrapeConfigs[0] + + processedConfig := &config.Config{} + if err := yaml.UnmarshalStrict([]byte(cfgText), processedConfig); err != nil { + t.Fatalf("Unable to load YAML config cfgYaml: %s", err) + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + discoveryManager := NewManager(ctx, nil) + go discoveryManager.Run() + + c := make(map[string]sd_config.ServiceDiscoveryConfig) + for _, v := range processedConfig.ScrapeConfigs { + c[v.JobName] = v.ServiceDiscoveryConfig + } + discoveryManager.ApplyConfig(c) + <-discoveryManager.SyncCh() + + for _, sdcfg := range c { + if !reflect.DeepEqual(origScrpCfg.ServiceDiscoveryConfig.StaticConfigs, sdcfg.StaticConfigs) { + t.Fatalf("discovery manager modified static config \n expected: %v\n got: %v\n", + origScrpCfg.ServiceDiscoveryConfig.StaticConfigs, sdcfg.StaticConfigs) + } + } +} + type update struct { targetGroups []targetgroup.Group interval time.Duration From dacb6c530a2b36d81542df7cf4868d27d45346a3 Mon Sep 17 00:00:00 2001 From: Simon Pasquier Date: Tue, 12 Jun 2018 13:45:59 +0200 Subject: [PATCH 05/10] discovery/file: fix logging (#4178) Signed-off-by: Simon Pasquier --- discovery/file/file.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/discovery/file/file.go b/discovery/file/file.go index 780e25819..be6337822 100644 --- a/discovery/file/file.go +++ b/discovery/file/file.go @@ -279,7 +279,7 @@ func (d *Discovery) deleteTimestamp(filename string) { // stop shuts down the file watcher. func (d *Discovery) stop() { - level.Debug(d.logger).Log("msg", "Stopping file discovery...", "paths", d.paths) + level.Debug(d.logger).Log("msg", "Stopping file discovery...", "paths", fmt.Sprintf("%v", d.paths)) done := make(chan struct{}) defer close(done) @@ -299,10 +299,10 @@ func (d *Discovery) stop() { } }() if err := d.watcher.Close(); err != nil { - level.Error(d.logger).Log("msg", "Error closing file watcher", "paths", d.paths, "err", err) + level.Error(d.logger).Log("msg", "Error closing file watcher", "paths", fmt.Sprintf("%v", d.paths), "err", err) } - level.Debug(d.logger).Log("File discovery stopped", "paths", d.paths) + level.Debug(d.logger).Log("msg", "File discovery stopped") } // refresh reads all files matching the discovery's patterns and sends the respective From db9dbeeaec8a1e32615cdc6ff7efaf63f55645ed Mon Sep 17 00:00:00 2001 From: Corentin Chary Date: Wed, 13 Jun 2018 09:19:17 +0200 Subject: [PATCH 06/10] federation: nil pointer deference when using remove read ``` level=error ts=2018-06-13T07:19:04.515149169Z caller=stdlib.go:89 component=web caller="http: panic serving [::1" msg="]:56202: runtime error: invalid memory address or nil pointer dereference" level=error ts=2018-06-13T07:19:04.516199547Z caller=stdlib.go:89 component=web caller="http: panic serving [::1" msg="]:56204: runtime error: invalid memory address or nil pointer dereference" level=error ts=2018-06-13T07:19:04.51717692Z caller=stdlib.go:89 component=web caller="http: panic serving [::1" msg="]:56206: runtime error: invalid memory address or nil pointer dereference" level=error ts=2018-06-13T07:19:04.564952878Z caller=stdlib.go:89 component=web caller="http: panic serving [::1" msg="]:56208: runtime error: invalid memory address or nil pointer dereference" level=error ts=2018-06-13T07:19:04.566575791Z caller=stdlib.go:89 component=web caller="http: panic serving [::1" msg="]:56210: runtime error: invalid memory address or nil pointer dereference" level=error ts=2018-06-13T07:19:04.567106063Z caller=stdlib.go:89 component=web caller="http: panic serving [::1" msg="]:56212: runtime error: invalid memory address or nil pointer dereference" ``` When remove read is enabled, federation will call `q.Select(nil, mset...)` which will break remote reads because it currently doesn't handle empty SelectParams. Signed-off-by: Corentin Chary --- storage/remote/codec.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/storage/remote/codec.go b/storage/remote/codec.go index d68c442e5..cf43c390f 100644 --- a/storage/remote/codec.go +++ b/storage/remote/codec.go @@ -91,10 +91,13 @@ func ToQuery(from, to int64, matchers []*labels.Matcher, p *storage.SelectParams if err != nil { return nil, err } + var rp *prompb.ReadHints = nil - rp := &prompb.ReadHints{ - StepMs: p.Step, - Func: p.Func, + if p != nil { + rp = &prompb.ReadHints{ + StepMs: p.Step, + Func: p.Func, + } } return &prompb.Query{ From e518f51a999647f77a2245114825083ad8e60632 Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Sat, 16 Jun 2018 18:26:37 +0100 Subject: [PATCH 07/10] Extend API tests to cover remote read API. Signed-off-by: Tom Wilkie --- web/api/v1/api_test.go | 192 +++++++++++++++++++++++++++++++---------- 1 file changed, 147 insertions(+), 45 deletions(-) diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index 06e1a4ac6..35e27d5e9 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -31,7 +31,9 @@ import ( "github.com/gogo/protobuf/proto" "github.com/golang/snappy" + config_util "github.com/prometheus/common/config" "github.com/prometheus/common/model" + "github.com/prometheus/common/promlog" "github.com/prometheus/common/route" "github.com/prometheus/prometheus/config" @@ -128,30 +130,125 @@ func TestEndpoints(t *testing.T) { now := time.Now() - var tr testTargetRetriever + t.Run("local", func(t *testing.T) { + api := &API{ + Queryable: suite.Storage(), + QueryEngine: suite.QueryEngine(), + targetRetriever: testTargetRetriever{}, + alertmanagerRetriever: testAlertmanagerRetriever{}, + now: func() time.Time { return now }, + config: func() config.Config { return samplePrometheusCfg }, + flagsMap: sampleFlagMap, + ready: func(f http.HandlerFunc) http.HandlerFunc { return f }, + } - var ar testAlertmanagerRetriever + testEndpoints(t, api, true) + }) - api := &API{ - Queryable: suite.Storage(), - QueryEngine: suite.QueryEngine(), - targetRetriever: tr, - alertmanagerRetriever: ar, - now: func() time.Time { return now }, - config: func() config.Config { return samplePrometheusCfg }, - flagsMap: sampleFlagMap, - ready: func(f http.HandlerFunc) http.HandlerFunc { return f }, - } + // Run all the API tests against a API that is wired to forward queries via + // the remote read client to a test server, which in turn sends them to the + // date from the test suite. + t.Run("remote", func(t *testing.T) { + server := setupRemote(suite.Storage()) + defer server.Close() + + u, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + al := promlog.AllowedLevel{} + al.Set("debug") + remote := remote.NewStorage(promlog.New(al), func() (int64, error) { + return 0, nil + }, 1*time.Second) + + err = remote.ApplyConfig(&config.Config{ + RemoteReadConfigs: []*config.RemoteReadConfig{ + { + URL: &config_util.URL{URL: u}, + RemoteTimeout: model.Duration(1 * time.Second), + ReadRecent: true, + }, + }, + }) + if err != nil { + t.Fatal(err) + } + + api := &API{ + Queryable: remote, + QueryEngine: suite.QueryEngine(), + targetRetriever: testTargetRetriever{}, + alertmanagerRetriever: testAlertmanagerRetriever{}, + now: func() time.Time { return now }, + config: func() config.Config { return samplePrometheusCfg }, + flagsMap: sampleFlagMap, + ready: func(f http.HandlerFunc) http.HandlerFunc { return f }, + } + + testEndpoints(t, api, false) + }) +} + +func setupRemote(s storage.Storage) *httptest.Server { + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + req, err := remote.DecodeReadRequest(r) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + resp := prompb.ReadResponse{ + Results: make([]*prompb.QueryResult, len(req.Queries)), + } + for i, query := range req.Queries { + from, through, matchers, err := remote.FromQuery(query) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + querier, err := s.Querier(r.Context(), from, through) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + defer querier.Close() + + set, err := querier.Select(nil, matchers...) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + resp.Results[i], err = remote.ToQueryResult(set) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + } + + if err := remote.EncodeReadResponse(&resp, w); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + }) + + return httptest.NewServer(handler) +} + +func testEndpoints(t *testing.T, api *API, testLabelAPI bool) { start := time.Unix(0, 0) - var tests = []struct { + type test struct { endpoint apiFunc params map[string]string query url.Values response interface{} errType errorType - }{ + } + + var tests = []test{ { endpoint: api.query, query: url.Values{ @@ -203,7 +300,7 @@ func TestEndpoints(t *testing.T) { ResultType: promql.ValueTypeScalar, Result: promql.Scalar{ V: 0.333, - T: timestamp.FromTime(now), + T: timestamp.FromTime(api.now()), }, }, }, @@ -309,34 +406,6 @@ func TestEndpoints(t *testing.T) { }, errType: errorBadData, }, - { - endpoint: api.labelValues, - params: map[string]string{ - "name": "__name__", - }, - response: []string{ - "test_metric1", - "test_metric2", - }, - }, - { - endpoint: api.labelValues, - params: map[string]string{ - "name": "foo", - }, - response: []string{ - "bar", - "boo", - }, - }, - // Bad name parameter. - { - endpoint: api.labelValues, - params: map[string]string{ - "name": "not!!!allowed", - }, - errType: errorBadData, - }, { endpoint: api.series, query: url.Values{ @@ -500,6 +569,39 @@ func TestEndpoints(t *testing.T) { }, } + if testLabelAPI { + tests = append(tests, []test{ + { + endpoint: api.labelValues, + params: map[string]string{ + "name": "__name__", + }, + response: []string{ + "test_metric1", + "test_metric2", + }, + }, + { + endpoint: api.labelValues, + params: map[string]string{ + "name": "foo", + }, + response: []string{ + "bar", + "boo", + }, + }, + // Bad name parameter. + { + endpoint: api.labelValues, + params: map[string]string{ + "name": "not!!!allowed", + }, + errType: errorBadData, + }, + }...) + } + methods := func(f apiFunc) []string { fp := reflect.ValueOf(f).Pointer() if fp == reflect.ValueOf(api.query).Pointer() || fp == reflect.ValueOf(api.queryRange).Pointer() { @@ -517,14 +619,14 @@ func TestEndpoints(t *testing.T) { return http.NewRequest(m, fmt.Sprintf("http://example.com?%s", q.Encode()), nil) } - for _, test := range tests { + for i, test := range tests { for _, method := range methods(test.endpoint) { // Build a context with the correct request params. ctx := context.Background() for p, v := range test.params { ctx = route.WithParam(ctx, p, v) } - t.Logf("run %s\t%q", method, test.query.Encode()) + t.Logf("run %d\t%s\t%q", i, method, test.query.Encode()) req, err := request(method, test.query) if err != nil { From b8217720ac5c0a8f270d49147e91bd0e8e932b3d Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Mon, 18 Jun 2018 16:33:04 +0100 Subject: [PATCH 08/10] Review feedback. Signed-off-by: Tom Wilkie --- storage/remote/codec.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/remote/codec.go b/storage/remote/codec.go index cf43c390f..3063b64f9 100644 --- a/storage/remote/codec.go +++ b/storage/remote/codec.go @@ -91,8 +91,8 @@ func ToQuery(from, to int64, matchers []*labels.Matcher, p *storage.SelectParams if err != nil { return nil, err } - var rp *prompb.ReadHints = nil + var rp *prompb.ReadHints if p != nil { rp = &prompb.ReadHints{ StepMs: p.Step, From 4e4f0d4e41dd7b44cd91dbeea06762451ed2db7e Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Mon, 18 Jun 2018 17:32:44 +0100 Subject: [PATCH 09/10] spelling. Signed-off-by: Tom Wilkie --- web/api/v1/api_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index 35e27d5e9..1cb7fdd02 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -147,7 +147,7 @@ func TestEndpoints(t *testing.T) { // Run all the API tests against a API that is wired to forward queries via // the remote read client to a test server, which in turn sends them to the - // date from the test suite. + // data from the test suite. t.Run("remote", func(t *testing.T) { server := setupRemote(suite.Storage()) defer server.Close() From 141799da6e1a251774da2c06554c5366a3f889ce Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Tue, 19 Jun 2018 13:12:11 +0100 Subject: [PATCH 10/10] Release 2.3.1 Signed-off-by: Brian Brazil --- CHANGELOG.md | 10 ++++++++++ VERSION | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eeebc1a2f..11699d053 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,13 @@ +## 2.3.1 / 2018-06-19 + +* [BUGFIX] Avoid infinite loop on duplicate NaN values. #4275 +* [BUGFIX] Fix nil pointer deference when using various API endpoints #4282 +* [BUGFIX] config: set target group source index during unmarshalling #4245 +* [BUGFIX] discovery/file: fix logging #4178 +* [BUGFIX] kubernetes_sd: fix namespace filtering #4285 +* [BUGFIX] web: restore old path prefix behavior #4273 +* [BUGFIX] web: remove security headers added in 2.3.0 #4259 + ## 2.3.0 / 2018-06-05 * [CHANGE] `marathon_sd`: use `auth_token` and `auth_token_file` for token-based authentication instead of `bearer_token` and `bearer_token_file` respectively. diff --git a/VERSION b/VERSION index 276cbf9e2..2bf1c1ccf 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.3.0 +2.3.1