From 87e23faea759fc9f895c1ec4565b0b50a2668996 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20=C5=9Awi=C4=85tek?= Date: Mon, 8 Jan 2024 19:51:39 +0100 Subject: [PATCH] Use standard collector selectors in target allocator config (#2466) * Use standard collector selectors in target allocator config * Use both collector selector formats in ta config This is to keep backwards compatibility with older target allocator versions, which makes upgrades easier. --- .chloggen/feat_targetallocator-selector.yaml | 18 +++++ cmd/otel-allocator/collector/collector.go | 9 ++- .../collector/collector_test.go | 13 ++-- cmd/otel-allocator/config/config.go | 24 +++---- cmd/otel-allocator/config/config_test.go | 17 +++-- .../config/testdata/config_test.yaml | 7 +- .../testdata/pod_service_selector_test.yaml | 7 +- cmd/otel-allocator/main.go | 2 +- cmd/otel-allocator/target/testdata/test.yaml | 7 +- .../target/testdata/test_update.yaml | 7 +- controllers/builder_test.go | 66 +++++++++++++++++-- controllers/reconcile_test.go | 8 +++ .../manifests/targetallocator/configmap.go | 8 ++- .../targetallocator/configmap_test.go | 18 +++++ 14 files changed, 166 insertions(+), 45 deletions(-) create mode 100755 .chloggen/feat_targetallocator-selector.yaml diff --git a/.chloggen/feat_targetallocator-selector.yaml b/.chloggen/feat_targetallocator-selector.yaml new file mode 100755 index 0000000000..13df1836c4 --- /dev/null +++ b/.chloggen/feat_targetallocator-selector.yaml @@ -0,0 +1,18 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: breaking + +# The name of the component, or a single word describing the area of concern, (e.g. operator, target allocator, github action) +component: target allocator + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Use standard K8s label selectors for collectors in target allocator config + +# One or more tracking issues related to the change +issues: [2422] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: | + This is a breaking change only for users of standalone target allocator. Operator users are unaffected. + The operator is still compatible with previous target allocator versions, and will be for the next 3 releases. diff --git a/cmd/otel-allocator/collector/collector.go b/cmd/otel-allocator/collector/collector.go index eaeee8e16c..a48797f587 100644 --- a/cmd/otel-allocator/collector/collector.go +++ b/cmd/otel-allocator/collector/collector.go @@ -24,7 +24,6 @@ import ( "github.com/prometheus/client_golang/prometheus/promauto" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" @@ -63,11 +62,15 @@ func NewClient(logger logr.Logger, kubeConfig *rest.Config) (*Client, error) { }, nil } -func (k *Client) Watch(ctx context.Context, labelMap map[string]string, fn func(collectors map[string]*allocation.Collector)) error { +func (k *Client) Watch(ctx context.Context, labelSelector *metav1.LabelSelector, fn func(collectors map[string]*allocation.Collector)) error { collectorMap := map[string]*allocation.Collector{} + selector, err := metav1.LabelSelectorAsSelector(labelSelector) + if err != nil { + return err + } opts := metav1.ListOptions{ - LabelSelector: labels.SelectorFromSet(labelMap).String(), + LabelSelector: selector.String(), } pods, err := k.k8sClient.CoreV1().Pods(ns).List(ctx, opts) if err != nil { diff --git a/cmd/otel-allocator/collector/collector_test.go b/cmd/otel-allocator/collector/collector_test.go index 5367828e23..3a652d491b 100644 --- a/cmd/otel-allocator/collector/collector_test.go +++ b/cmd/otel-allocator/collector/collector_test.go @@ -28,7 +28,6 @@ import ( "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" "k8s.io/client-go/kubernetes/fake" "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation" @@ -42,14 +41,16 @@ func getTestClient() (Client, watch.Interface) { close: make(chan struct{}), log: logger, } - - labelMap := map[string]string{ - "app.kubernetes.io/instance": "default.test", - "app.kubernetes.io/managed-by": "opentelemetry-operator", + labelSelector := metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app.kubernetes.io/instance": "default.test", + "app.kubernetes.io/managed-by": "opentelemetry-operator", + }, } + selector, _ := metav1.LabelSelectorAsSelector(&labelSelector) opts := metav1.ListOptions{ - LabelSelector: labels.SelectorFromSet(labelMap).String(), + LabelSelector: selector.String(), } watcher, err := kubeClient.k8sClient.CoreV1().Pods("test-ns").Watch(context.Background(), opts) if err != nil { diff --git a/cmd/otel-allocator/config/config.go b/cmd/otel-allocator/config/config.go index c2d716b9de..d7aea48d35 100644 --- a/cmd/otel-allocator/config/config.go +++ b/cmd/otel-allocator/config/config.go @@ -27,6 +27,7 @@ import ( _ "github.com/prometheus/prometheus/discovery/install" "github.com/spf13/pflag" "gopkg.in/yaml.v2" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" "k8s.io/klog/v2" @@ -39,17 +40,17 @@ const DefaultConfigFilePath string = "/conf/targetallocator.yaml" const DefaultCRScrapeInterval model.Duration = model.Duration(time.Second * 30) type Config struct { - ListenAddr string `yaml:"listen_addr,omitempty"` - KubeConfigFilePath string `yaml:"kube_config_file_path,omitempty"` - ClusterConfig *rest.Config `yaml:"-"` - RootLogger logr.Logger `yaml:"-"` - LabelSelector map[string]string `yaml:"label_selector,omitempty"` - PromConfig *promconfig.Config `yaml:"config"` - AllocationStrategy *string `yaml:"allocation_strategy,omitempty"` - FilterStrategy *string `yaml:"filter_strategy,omitempty"` - PrometheusCR PrometheusCRConfig `yaml:"prometheus_cr,omitempty"` - PodMonitorSelector map[string]string `yaml:"pod_monitor_selector,omitempty"` - ServiceMonitorSelector map[string]string `yaml:"service_monitor_selector,omitempty"` + ListenAddr string `yaml:"listen_addr,omitempty"` + KubeConfigFilePath string `yaml:"kube_config_file_path,omitempty"` + ClusterConfig *rest.Config `yaml:"-"` + RootLogger logr.Logger `yaml:"-"` + CollectorSelector *metav1.LabelSelector `yaml:"collector_selector,omitempty"` + PromConfig *promconfig.Config `yaml:"config"` + AllocationStrategy *string `yaml:"allocation_strategy,omitempty"` + FilterStrategy *string `yaml:"filter_strategy,omitempty"` + PrometheusCR PrometheusCRConfig `yaml:"prometheus_cr,omitempty"` + PodMonitorSelector map[string]string `yaml:"pod_monitor_selector,omitempty"` + ServiceMonitorSelector map[string]string `yaml:"service_monitor_selector,omitempty"` } type PrometheusCRConfig struct { @@ -114,7 +115,6 @@ func LoadFromCLI(target *Config, flagSet *pflag.FlagSet) error { } func unmarshal(cfg *Config, configFile string) error { - yamlFile, err := os.ReadFile(configFile) if err != nil { return err diff --git a/cmd/otel-allocator/config/config_test.go b/cmd/otel-allocator/config/config_test.go index 89f6307d85..b7ffcaf954 100644 --- a/cmd/otel-allocator/config/config_test.go +++ b/cmd/otel-allocator/config/config_test.go @@ -21,6 +21,7 @@ import ( commonconfig "github.com/prometheus/common/config" promconfig "github.com/prometheus/prometheus/config" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/prometheus/common/model" "github.com/prometheus/prometheus/discovery" @@ -44,9 +45,11 @@ func TestLoad(t *testing.T) { file: "./testdata/config_test.yaml", }, want: Config{ - LabelSelector: map[string]string{ - "app.kubernetes.io/instance": "default.test", - "app.kubernetes.io/managed-by": "opentelemetry-operator", + CollectorSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app.kubernetes.io/instance": "default.test", + "app.kubernetes.io/managed-by": "opentelemetry-operator", + }, }, PrometheusCR: PrometheusCRConfig{ ScrapeInterval: model.Duration(time.Second * 60), @@ -108,9 +111,11 @@ func TestLoad(t *testing.T) { file: "./testdata/pod_service_selector_test.yaml", }, want: Config{ - LabelSelector: map[string]string{ - "app.kubernetes.io/instance": "default.test", - "app.kubernetes.io/managed-by": "opentelemetry-operator", + CollectorSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app.kubernetes.io/instance": "default.test", + "app.kubernetes.io/managed-by": "opentelemetry-operator", + }, }, PrometheusCR: PrometheusCRConfig{ ScrapeInterval: DefaultCRScrapeInterval, diff --git a/cmd/otel-allocator/config/testdata/config_test.yaml b/cmd/otel-allocator/config/testdata/config_test.yaml index efdc27bc39..8d4a427133 100644 --- a/cmd/otel-allocator/config/testdata/config_test.yaml +++ b/cmd/otel-allocator/config/testdata/config_test.yaml @@ -1,6 +1,7 @@ -label_selector: - app.kubernetes.io/instance: default.test - app.kubernetes.io/managed-by: opentelemetry-operator +collector_selector: + matchlabels: + app.kubernetes.io/instance: default.test + app.kubernetes.io/managed-by: opentelemetry-operator prometheus_cr: scrape_interval: 60s config: diff --git a/cmd/otel-allocator/config/testdata/pod_service_selector_test.yaml b/cmd/otel-allocator/config/testdata/pod_service_selector_test.yaml index c0ff54ad36..dddef851cd 100644 --- a/cmd/otel-allocator/config/testdata/pod_service_selector_test.yaml +++ b/cmd/otel-allocator/config/testdata/pod_service_selector_test.yaml @@ -1,6 +1,7 @@ -label_selector: - app.kubernetes.io/instance: default.test - app.kubernetes.io/managed-by: opentelemetry-operator +collector_selector: + matchlabels: + app.kubernetes.io/instance: default.test + app.kubernetes.io/managed-by: opentelemetry-operator pod_monitor_selector: release: test service_monitor_selector: diff --git a/cmd/otel-allocator/main.go b/cmd/otel-allocator/main.go index 4f805519b1..5bda64003f 100644 --- a/cmd/otel-allocator/main.go +++ b/cmd/otel-allocator/main.go @@ -149,7 +149,7 @@ func main() { }) runGroup.Add( func() error { - err := collectorWatcher.Watch(ctx, cfg.LabelSelector, allocator.SetCollectors) + err := collectorWatcher.Watch(ctx, cfg.CollectorSelector, allocator.SetCollectors) setupLog.Info("Collector watcher exited") return err }, diff --git a/cmd/otel-allocator/target/testdata/test.yaml b/cmd/otel-allocator/target/testdata/test.yaml index d38b0183a7..47ecd566f8 100644 --- a/cmd/otel-allocator/target/testdata/test.yaml +++ b/cmd/otel-allocator/target/testdata/test.yaml @@ -1,6 +1,7 @@ -label_selector: - app.kubernetes.io/instance: default.test - app.kubernetes.io/managed-by: opentelemetry-operator +collector_selector: + matchlabels: + app.kubernetes.io/instance: default.test + app.kubernetes.io/managed-by: opentelemetry-operator config: scrape_configs: - job_name: prometheus diff --git a/cmd/otel-allocator/target/testdata/test_update.yaml b/cmd/otel-allocator/target/testdata/test_update.yaml index 3a7ec60f3b..682ca91473 100644 --- a/cmd/otel-allocator/target/testdata/test_update.yaml +++ b/cmd/otel-allocator/target/testdata/test_update.yaml @@ -1,6 +1,7 @@ -label_selector: - app.kubernetes.io/instance: default.test - app.kubernetes.io/managed-by: opentelemetry-operator +collector_selector: + matchlabels: + app.kubernetes.io/instance: default.test + app.kubernetes.io/managed-by: opentelemetry-operator config: scrape_configs: - job_name: prometheus diff --git a/controllers/builder_test.go b/controllers/builder_test.go index be8e635d7e..807d0f9d05 100644 --- a/controllers/builder_test.go +++ b/controllers/builder_test.go @@ -1316,7 +1316,36 @@ service: Annotations: nil, }, Data: map[string]string{ - "targetallocator.yaml": "allocation_strategy: least-weighted\nconfig:\n scrape_configs:\n - job_name: example\n metric_relabel_configs:\n - replacement: $1_$2\n source_labels:\n - job\n target_label: job\n relabel_configs:\n - replacement: my_service_$1\n source_labels:\n - __meta_service_id\n target_label: job\n - replacement: $1\n source_labels:\n - __meta_service_name\n target_label: instance\nlabel_selector:\n app.kubernetes.io/component: opentelemetry-collector\n app.kubernetes.io/instance: test.test\n app.kubernetes.io/managed-by: opentelemetry-operator\n app.kubernetes.io/part-of: opentelemetry\n", + "targetallocator.yaml": `allocation_strategy: least-weighted +collector_selector: + matchlabels: + app.kubernetes.io/component: opentelemetry-collector + app.kubernetes.io/instance: test.test + app.kubernetes.io/managed-by: opentelemetry-operator + app.kubernetes.io/part-of: opentelemetry +config: + scrape_configs: + - job_name: example + metric_relabel_configs: + - replacement: $1_$2 + source_labels: + - job + target_label: job + relabel_configs: + - replacement: my_service_$1 + source_labels: + - __meta_service_id + target_label: job + - replacement: $1 + source_labels: + - __meta_service_name + target_label: instance +label_selector: + app.kubernetes.io/component: opentelemetry-collector + app.kubernetes.io/instance: test.test + app.kubernetes.io/managed-by: opentelemetry-operator + app.kubernetes.io/part-of: opentelemetry +`, }, }, &appsv1.Deployment{ @@ -1348,7 +1377,7 @@ service: "app.kubernetes.io/version": "latest", }, Annotations: map[string]string{ - "opentelemetry-targetallocator-config/hash": "4d1911fd40106e9e2dd3d928f067a6c8c9eab0c569f737ba3701c6f5a9aad6d7", + "opentelemetry-targetallocator-config/hash": "20c09760c240d08287ff05bd2375985220b577d938e82efd85467e17174690e0", }, }, Spec: corev1.PodSpec{ @@ -1676,7 +1705,36 @@ service: Annotations: nil, }, Data: map[string]string{ - "targetallocator.yaml": "allocation_strategy: least-weighted\nconfig:\n scrape_configs:\n - job_name: example\n metric_relabel_configs:\n - replacement: $1_$2\n source_labels:\n - job\n target_label: job\n relabel_configs:\n - replacement: my_service_$1\n source_labels:\n - __meta_service_id\n target_label: job\n - replacement: $1\n source_labels:\n - __meta_service_name\n target_label: instance\nlabel_selector:\n app.kubernetes.io/component: opentelemetry-collector\n app.kubernetes.io/instance: test.test\n app.kubernetes.io/managed-by: opentelemetry-operator\n app.kubernetes.io/part-of: opentelemetry\n", + "targetallocator.yaml": `allocation_strategy: least-weighted +collector_selector: + matchlabels: + app.kubernetes.io/component: opentelemetry-collector + app.kubernetes.io/instance: test.test + app.kubernetes.io/managed-by: opentelemetry-operator + app.kubernetes.io/part-of: opentelemetry +config: + scrape_configs: + - job_name: example + metric_relabel_configs: + - replacement: $1_$2 + source_labels: + - job + target_label: job + relabel_configs: + - replacement: my_service_$1 + source_labels: + - __meta_service_id + target_label: job + - replacement: $1 + source_labels: + - __meta_service_name + target_label: instance +label_selector: + app.kubernetes.io/component: opentelemetry-collector + app.kubernetes.io/instance: test.test + app.kubernetes.io/managed-by: opentelemetry-operator + app.kubernetes.io/part-of: opentelemetry +`, }, }, &appsv1.Deployment{ @@ -1708,7 +1766,7 @@ service: "app.kubernetes.io/version": "latest", }, Annotations: map[string]string{ - "opentelemetry-targetallocator-config/hash": "4d1911fd40106e9e2dd3d928f067a6c8c9eab0c569f737ba3701c6f5a9aad6d7", + "opentelemetry-targetallocator-config/hash": "20c09760c240d08287ff05bd2375985220b577d938e82efd85467e17174690e0", }, }, Spec: corev1.PodSpec{ diff --git a/controllers/reconcile_test.go b/controllers/reconcile_test.go index 13fe816898..817da2eab1 100644 --- a/controllers/reconcile_test.go +++ b/controllers/reconcile_test.go @@ -443,6 +443,14 @@ func TestOpenTelemetryCollectorReconciler_Reconcile(t *testing.T) { assert.NoError(t, err) taConfig := make(map[interface{}]interface{}) + taConfig["collector_selector"] = map[string]any{ + "matchlabels": map[string]string{ + "app.kubernetes.io/instance": "default.test", + "app.kubernetes.io/managed-by": "opentelemetry-operator", + "app.kubernetes.io/component": "opentelemetry-collector", + "app.kubernetes.io/part-of": "opentelemetry", + }, + } taConfig["label_selector"] = map[string]string{ "app.kubernetes.io/instance": "default.test", "app.kubernetes.io/managed-by": "opentelemetry-operator", diff --git a/internal/manifests/targetallocator/configmap.go b/internal/manifests/targetallocator/configmap.go index ca267c7088..8f5fc76617 100644 --- a/internal/manifests/targetallocator/configmap.go +++ b/internal/manifests/targetallocator/configmap.go @@ -44,7 +44,13 @@ func ConfigMap(params manifests.Params) (*corev1.ConfigMap, error) { taConfig := make(map[interface{}]interface{}) prometheusCRConfig := make(map[interface{}]interface{}) - taConfig["label_selector"] = manifestutils.SelectorLabels(params.OtelCol.ObjectMeta, collector.ComponentOpenTelemetryCollector) + collectorSelectorLabels := manifestutils.SelectorLabels(params.OtelCol.ObjectMeta, collector.ComponentOpenTelemetryCollector) + taConfig["collector_selector"] = map[string]any{ + "matchlabels": collectorSelectorLabels, + } + // The below instruction is here for compatibility with the previous target allocator version + // TODO: Drop it after 3 more versions + taConfig["label_selector"] = collectorSelectorLabels // We only take the "config" from the returned object, if it's present if prometheusConfig, ok := prometheusReceiverConfig["config"]; ok { taConfig["config"] = prometheusConfig diff --git a/internal/manifests/targetallocator/configmap_test.go b/internal/manifests/targetallocator/configmap_test.go index 8baa202400..a73f9c17d9 100644 --- a/internal/manifests/targetallocator/configmap_test.go +++ b/internal/manifests/targetallocator/configmap_test.go @@ -40,6 +40,12 @@ func TestDesiredConfigMap(t *testing.T) { expectedData := map[string]string{ "targetallocator.yaml": `allocation_strategy: least-weighted +collector_selector: + matchlabels: + app.kubernetes.io/component: opentelemetry-collector + app.kubernetes.io/instance: default.my-instance + app.kubernetes.io/managed-by: opentelemetry-operator + app.kubernetes.io/part-of: opentelemetry config: scrape_configs: - job_name: otel-collector @@ -76,6 +82,12 @@ label_selector: expectedData := map[string]string{ "targetallocator.yaml": `allocation_strategy: least-weighted +collector_selector: + matchlabels: + app.kubernetes.io/component: opentelemetry-collector + app.kubernetes.io/instance: default.my-instance + app.kubernetes.io/managed-by: opentelemetry-operator + app.kubernetes.io/part-of: opentelemetry config: scrape_configs: - job_name: otel-collector @@ -122,6 +134,12 @@ service_monitor_selector: expectedData := map[string]string{ "targetallocator.yaml": `allocation_strategy: least-weighted +collector_selector: + matchlabels: + app.kubernetes.io/component: opentelemetry-collector + app.kubernetes.io/instance: default.my-instance + app.kubernetes.io/managed-by: opentelemetry-operator + app.kubernetes.io/part-of: opentelemetry config: scrape_configs: - job_name: otel-collector