[target allocator] Allow setting collector namespace via the config file

swiatekm · swiatekm · commit ffce405a7dbb · 2025-03-07T18:27:14.000+01:00
Make this namespace part of the config and only read the environment
variable during config loading.
diff --git a/.chloggen/feat_ta-namespace-setting.yaml b/.chloggen/feat_ta-namespace-setting.yaml
@@ -0,0 +1,16 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, 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: Allow setting the collector namespace via the config file
+
+# One or more tracking issues related to the change
+issues: [3782]
+
+# (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:
diff --git a/cmd/otel-allocator/internal/collector/collector.go b/cmd/otel-allocator/internal/collector/collector.go
@@ -4,7 +4,6 @@
 package collector
 
 import (
-	"os"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -25,7 +24,6 @@ const (
 )
 
 var (
-	ns                   = os.Getenv("OTELCOL_NAMESPACE")
 	collectorsDiscovered = promauto.NewGauge(prometheus.GaugeOpts{
 		Name: "opentelemetry_allocator_collectors_discovered",
 		Help: "Number of collectors discovered.",
@@ -53,7 +51,11 @@ func NewCollectorWatcher(logger logr.Logger, kubeConfig *rest.Config) (*Watcher,
 	}, nil
 }
 
-func (k *Watcher) Watch(labelSelector *metav1.LabelSelector, fn func(collectors map[string]*allocation.Collector)) error {
+func (k *Watcher) Watch(
+	collectorNamespace string,
+	labelSelector *metav1.LabelSelector,
+	fn func(collectors map[string]*allocation.Collector),
+) error {
 	selector, err := metav1.LabelSelectorAsSelector(labelSelector)
 	if err != nil {
 		return err
@@ -65,7 +67,7 @@ func (k *Watcher) Watch(labelSelector *metav1.LabelSelector, fn func(collectors
 	informerFactory := informers.NewSharedInformerFactoryWithOptions(
 		k.k8sClient,
 		time.Second*30,
-		informers.WithNamespace(ns),
+		informers.WithNamespace(collectorNamespace),
 		informers.WithTweakListOptions(listOptionsFunc))
 	informer := informerFactory.Core().V1().Pods().Informer()
 
diff --git a/cmd/otel-allocator/internal/collector/collector_test.go b/cmd/otel-allocator/internal/collector/collector_test.go
@@ -53,6 +53,7 @@ func pod(name string) *v1.Pod {
 }
 
 func Test_runWatch(t *testing.T) {
+	namespace := "test-ns"
 	type args struct {
 		kubeFn       func(t *testing.T, podWatcher Watcher)
 		collectorMap map[string]*allocation.Collector
@@ -68,7 +69,7 @@ func Test_runWatch(t *testing.T) {
 				kubeFn: func(t *testing.T, podWatcher Watcher) {
 					for _, k := range []string{"test-pod1", "test-pod2", "test-pod3"} {
 						p := pod(k)
-						_, err := podWatcher.k8sClient.CoreV1().Pods("test-ns").Create(context.Background(), p, metav1.CreateOptions{})
+						_, err := podWatcher.k8sClient.CoreV1().Pods(namespace).Create(context.Background(), p, metav1.CreateOptions{})
 						assert.NoError(t, err)
 					}
 				},
@@ -94,7 +95,7 @@ func Test_runWatch(t *testing.T) {
 			args: args{
 				kubeFn: func(t *testing.T, podWatcher Watcher) {
 					for _, k := range []string{"test-pod2", "test-pod3"} {
-						err := podWatcher.k8sClient.CoreV1().Pods("test-ns").Delete(context.Background(), k, metav1.DeleteOptions{})
+						err := podWatcher.k8sClient.CoreV1().Pods(namespace).Delete(context.Background(), k, metav1.DeleteOptions{})
 						assert.NoError(t, err)
 					}
 				},
@@ -135,7 +136,7 @@ func Test_runWatch(t *testing.T) {
 				assert.NoError(t, err)
 			}
 			go func(podWatcher Watcher) {
-				err := podWatcher.Watch(&labelSelector, func(colMap map[string]*allocation.Collector) {
+				err := podWatcher.Watch(namespace, &labelSelector, func(colMap map[string]*allocation.Collector) {
 					mapMutex.Lock()
 					defer mapMutex.Unlock()
 					actual = colMap
@@ -165,7 +166,7 @@ func Test_closeChannel(t *testing.T) {
 
 	go func(podWatcher Watcher) {
 		defer wg.Done()
-		err := podWatcher.Watch(&labelSelector, func(colMap map[string]*allocation.Collector) {})
+		err := podWatcher.Watch("default", &labelSelector, func(colMap map[string]*allocation.Collector) {})
 		require.NoError(t, err)
 	}(podWatcher)
 
diff --git a/cmd/otel-allocator/internal/config/config.go b/cmd/otel-allocator/internal/config/config.go
@@ -42,6 +42,7 @@ type Config struct {
 	ClusterConfig              *rest.Config          `yaml:"-"`
 	RootLogger                 logr.Logger           `yaml:"-"`
 	CollectorSelector          *metav1.LabelSelector `yaml:"collector_selector,omitempty"`
+	CollectorNamespace         string                `yaml:"collector_namespace,omitempty"`
 	PromConfig                 *promconfig.Config    `yaml:"config"`
 	AllocationStrategy         string                `yaml:"allocation_strategy,omitempty"`
 	AllocationFallbackStrategy string                `yaml:"allocation_fallback_strategy,omitempty"`
@@ -239,6 +240,12 @@ func LoadFromCLI(target *Config, flagSet *pflag.FlagSet) error {
 	return nil
 }
 
+// LoadFromEnv loads configuration from environment variables.
+func LoadFromEnv(target *Config) error {
+	target.CollectorNamespace = os.Getenv("OTELCOL_NAMESPACE")
+	return nil
+}
+
 // unmarshal decodes the contents of the configFile into the cfg argument, using a
 // mapstructure decoder with the following notable behaviors.
 // Decodes time.Duration from strings (see StringToModelDurationHookFunc).
@@ -309,6 +316,11 @@ func Load() (*Config, error) {
 		return nil, err
 	}
 
+	err = LoadFromEnv(&config)
+	if err != nil {
+		return nil, err
+	}
+
 	err = LoadFromCLI(&config, flagSet)
 	if err != nil {
 		return nil, err
@@ -323,6 +335,9 @@ func ValidateConfig(config *Config) error {
 	if !(config.PrometheusCR.Enabled || scrapeConfigsPresent) {
 		return fmt.Errorf("at least one scrape config must be defined, or Prometheus CR watching must be enabled")
 	}
+	if config.CollectorNamespace == "" {
+		return fmt.Errorf("collector namespace must be set")
+	}
 	return nil
 }
 
diff --git a/cmd/otel-allocator/internal/config/config_test.go b/cmd/otel-allocator/internal/config/config_test.go
@@ -5,6 +5,7 @@ package config
 
 import (
 	"fmt"
+	"os"
 	"testing"
 	"time"
 
@@ -14,6 +15,7 @@ import (
 	"github.com/prometheus/prometheus/discovery"
 	"github.com/prometheus/prometheus/discovery/file"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -40,6 +42,7 @@ func TestLoad(t *testing.T) {
 			},
 			want: Config{
 				AllocationStrategy: DefaultAllocationStrategy,
+				CollectorNamespace: "default",
 				CollectorSelector: &metav1.LabelSelector{
 					MatchLabels: map[string]string{
 						"app.kubernetes.io/instance":   "default.test",
@@ -120,6 +123,7 @@ func TestLoad(t *testing.T) {
 			},
 			want: Config{
 				AllocationStrategy: DefaultAllocationStrategy,
+				CollectorNamespace: "default",
 				CollectorSelector: &metav1.LabelSelector{
 					MatchLabels: map[string]string{
 						"app.kubernetes.io/instance":   "default.test",
@@ -190,6 +194,7 @@ func TestLoad(t *testing.T) {
 			},
 			want: Config{
 				AllocationStrategy: DefaultAllocationStrategy,
+				CollectorNamespace: "default",
 				CollectorSelector: &metav1.LabelSelector{
 					MatchLabels: map[string]string{
 						"app.kubernetes.io/instance":   "default.test",
@@ -260,6 +265,7 @@ func TestLoad(t *testing.T) {
 			},
 			want: Config{
 				AllocationStrategy: DefaultAllocationStrategy,
+				CollectorNamespace: "default",
 				CollectorSelector: &metav1.LabelSelector{
 					MatchExpressions: []metav1.LabelSelectorRequirement{
 						{
@@ -354,6 +360,7 @@ func TestLoad(t *testing.T) {
 			},
 			want: Config{
 				AllocationStrategy: DefaultAllocationStrategy,
+				CollectorNamespace: "default",
 				CollectorSelector: &metav1.LabelSelector{
 					MatchExpressions: []metav1.LabelSelectorRequirement{
 						{
@@ -454,15 +461,34 @@ func TestLoad(t *testing.T) {
 	}
 }
 
+func TestLoadFromEnv(t *testing.T) {
+	current := os.Getenv("OTELCOL_NAMESPACE")
+	t.Cleanup(func() {
+		err := os.Setenv("OTELCOL_NAMESPACE", current)
+		assert.NoError(t, err)
+	})
+	namespace := "default"
+	os.Setenv("OTELCOL_NAMESPACE", namespace)
+	cfg := &Config{}
+	err := LoadFromEnv(cfg)
+	require.NoError(t, err)
+	assert.Equal(t, namespace, cfg.CollectorNamespace)
+}
+
 func TestValidateConfig(t *testing.T) {
 	testCases := []struct {
 		name        string
 		fileConfig  Config
 		expectedErr error
 	}{
+		{
+			name:        "no namespace",
+			fileConfig:  Config{PrometheusCR: PrometheusCRConfig{Enabled: true}},
+			expectedErr: fmt.Errorf("collector namespace must be set"),
+		},
 		{
 			name:        "promCR enabled, no Prometheus config",
-			fileConfig:  Config{PromConfig: nil, PrometheusCR: PrometheusCRConfig{Enabled: true}},
+			fileConfig:  Config{PromConfig: nil, PrometheusCR: PrometheusCRConfig{Enabled: true}, CollectorNamespace: "default"},
 			expectedErr: nil,
 		},
 		{
@@ -478,15 +504,17 @@ func TestValidateConfig(t *testing.T) {
 		{
 			name: "promCR disabled, Prometheus config present, scrapeConfigs present",
 			fileConfig: Config{
-				PromConfig: &promconfig.Config{ScrapeConfigs: []*promconfig.ScrapeConfig{{}}},
+				PromConfig:         &promconfig.Config{ScrapeConfigs: []*promconfig.ScrapeConfig{{}}},
+				CollectorNamespace: "default",
 			},
 			expectedErr: nil,
 		},
 		{
 			name: "promCR enabled, Prometheus config present, scrapeConfigs present",
 			fileConfig: Config{
-				PromConfig:   &promconfig.Config{ScrapeConfigs: []*promconfig.ScrapeConfig{{}}},
-				PrometheusCR: PrometheusCRConfig{Enabled: true},
+				PromConfig:         &promconfig.Config{ScrapeConfigs: []*promconfig.ScrapeConfig{{}}},
+				PrometheusCR:       PrometheusCRConfig{Enabled: true},
+				CollectorNamespace: "default",
 			},
 			expectedErr: nil,
 		},
diff --git a/cmd/otel-allocator/internal/config/testdata/config_test.yaml b/cmd/otel-allocator/internal/config/testdata/config_test.yaml
@@ -1,3 +1,4 @@
+collector_namespace: default
 collector_selector:
   matchlabels:
     app.kubernetes.io/instance: default.test
diff --git a/cmd/otel-allocator/internal/config/testdata/pod_service_selector_camelcase_expressions_test.yaml b/cmd/otel-allocator/internal/config/testdata/pod_service_selector_camelcase_expressions_test.yaml
@@ -1,3 +1,4 @@
+collector_namespace: default
 collector_selector:
   matchExpressions:
   - key: "app.kubernetes.io/instance"
diff --git a/cmd/otel-allocator/internal/config/testdata/pod_service_selector_camelcase_test.yaml b/cmd/otel-allocator/internal/config/testdata/pod_service_selector_camelcase_test.yaml
@@ -1,3 +1,4 @@
+collector_namespace: default
 collector_selector:
   matchLabels:
     app.kubernetes.io/instance: default.test
diff --git a/cmd/otel-allocator/internal/config/testdata/pod_service_selector_expressions_test.yaml b/cmd/otel-allocator/internal/config/testdata/pod_service_selector_expressions_test.yaml
@@ -1,3 +1,4 @@
+collector_namespace: default
 collector_selector:
   matchexpressions:
   - key: "app.kubernetes.io/instance"
diff --git a/cmd/otel-allocator/internal/config/testdata/pod_service_selector_test.yaml b/cmd/otel-allocator/internal/config/testdata/pod_service_selector_test.yaml
@@ -1,3 +1,4 @@
+collector_namespace: default
 collector_selector:
   matchlabels:
     app.kubernetes.io/instance: default.test
diff --git a/cmd/otel-allocator/main.go b/cmd/otel-allocator/main.go
@@ -168,7 +168,7 @@ func main() {
 		})
 	runGroup.Add(
 		func() error {
-			err := collectorWatcher.Watch(cfg.CollectorSelector, allocator.SetCollectors)
+			err := collectorWatcher.Watch(cfg.CollectorNamespace, cfg.CollectorSelector, allocator.SetCollectors)
 			setupLog.Info("Collector watcher exited")
 			return err
 		},

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,4 @@`
	`1`	`+collector_namespace: default`
`1`	`2`	`collector_selector:`
`2`	`3`	`matchlabels:`
`3`	`4`	`app.kubernetes.io/instance: default.test`