From f28e9e13118b45a2ba6797f5bf319f8b3e5a3c5e Mon Sep 17 00:00:00 2001 From: Stanislav Khalash Date: Fri, 6 Dec 2024 17:06:00 +0100 Subject: [PATCH] Remove single-field configs --- .../telemetry/logpipeline_controller.go | 6 +-- .../telemetry/metricpipeline_controller.go | 8 +-- .../telemetry/tracepipeline_controller.go | 6 +-- .../reconciler/logpipeline/otel/reconciler.go | 10 ++-- .../logpipeline/otel/reconciler_test.go | 10 ++-- .../reconciler/logpipeline/otel/status.go | 2 +- .../reconciler/metricpipeline/reconciler.go | 23 ++++----- .../metricpipeline/reconciler_test.go | 50 ++++++++++++------- internal/reconciler/metricpipeline/status.go | 4 +- .../reconciler/tracepipeline/reconciler.go | 10 ++-- .../tracepipeline/reconciler_test.go | 28 +++++------ internal/reconciler/tracepipeline/status.go | 2 +- 12 files changed, 74 insertions(+), 85 deletions(-) diff --git a/controllers/telemetry/logpipeline_controller.go b/controllers/telemetry/logpipeline_controller.go index 4fef1d518..9d672877a 100644 --- a/controllers/telemetry/logpipeline_controller.go +++ b/controllers/telemetry/logpipeline_controller.go @@ -200,17 +200,13 @@ func configureFluentBitReconciler(client client.Client, config LogPipelineContro //nolint:unparam // error is always nil: An error could be returned after implementing the IstioStatusChecker (TODO) func configureOtelReconciler(client client.Client, config LogPipelineControllerConfig, _ *prober.LogPipelineProber) (*logpipelineotel.Reconciler, error) { - otelConfig := logpipelineotel.Config{ - TelemetryNamespace: config.TelemetryNamespace, - } - pipelineValidator := &logpipelineotel.Validator{ // TODO: Add validators } otelReconciler := logpipelineotel.New( client, - otelConfig, + config.TelemetryNamespace, otelcollector.NewLogGatewayApplierDeleter(config.OTelCollectorImage, config.TelemetryNamespace, config.LogGatewayPriorityClassName), &gateway.Builder{Reader: client}, &workloadstatus.DeploymentProber{Client: client}, diff --git a/controllers/telemetry/metricpipeline_controller.go b/controllers/telemetry/metricpipeline_controller.go index 4595f331e..115aeccf6 100644 --- a/controllers/telemetry/metricpipeline_controller.go +++ b/controllers/telemetry/metricpipeline_controller.go @@ -111,14 +111,10 @@ func NewMetricPipelineController(client client.Client, reconcileTriggerChan <-ch gatewayConfigBuilder := &gateway.Builder{Reader: client} - reconcilerConfig := metricpipeline.Config{ - ModuleVersion: config.ModuleVersion, - TelemetryNamespace: config.TelemetryNamespace, - } - reconciler := metricpipeline.New( client, - reconcilerConfig, + config.TelemetryNamespace, + config.ModuleVersion, otelcollector.NewMetricAgentApplierDeleter(config.OTelCollectorImage, config.TelemetryNamespace, config.MetricAgentPriorityClassName), agentConfigBuilder, &workloadstatus.DaemonSetProber{Client: client}, diff --git a/controllers/telemetry/tracepipeline_controller.go b/controllers/telemetry/tracepipeline_controller.go index 475a25110..ca141a832 100644 --- a/controllers/telemetry/tracepipeline_controller.go +++ b/controllers/telemetry/tracepipeline_controller.go @@ -99,13 +99,9 @@ func NewTracePipelineController(client client.Client, reconcileTriggerChan <-cha return nil, err } - reconcilerConfig := tracepipeline.Config{ - TelemetryNamespace: config.TelemetryNamespace, - } - reconciler := tracepipeline.New( client, - reconcilerConfig, + config.TelemetryNamespace, flowHealthProber, otelcollector.NewTraceGatewayApplierDeleter(config.OTelCollectorImage, config.TelemetryNamespace, config.TraceGatewayPriorityClassName), &gateway.Builder{Reader: client}, diff --git a/internal/reconciler/logpipeline/otel/reconciler.go b/internal/reconciler/logpipeline/otel/reconciler.go index 4a40610ba..c744b76ad 100644 --- a/internal/reconciler/logpipeline/otel/reconciler.go +++ b/internal/reconciler/logpipeline/otel/reconciler.go @@ -25,10 +25,6 @@ import ( const defaultReplicaCount int32 = 2 -type Config struct { - TelemetryNamespace string -} - type GatewayConfigBuilder interface { Build(ctx context.Context, pipelines []telemetryv1alpha1.LogPipeline) (*gateway.Config, otlpexporter.EnvVars, error) } @@ -47,7 +43,7 @@ var _ logpipeline.LogPipelineReconciler = &Reconciler{} type Reconciler struct { client.Client - config Config + telemetryNamespace string // Dependencies gatewayApplierDeleter GatewayApplierDeleter @@ -59,7 +55,7 @@ type Reconciler struct { func New( client client.Client, - config Config, + telemetryNamespace string, gatewayApplierDeleter GatewayApplierDeleter, gatewayConfigBuilder GatewayConfigBuilder, gatewayProber commonstatus.Prober, @@ -68,7 +64,7 @@ func New( ) *Reconciler { return &Reconciler{ Client: client, - config: config, + telemetryNamespace: telemetryNamespace, gatewayApplierDeleter: gatewayApplierDeleter, gatewayConfigBuilder: gatewayConfigBuilder, gatewayProber: gatewayProber, diff --git a/internal/reconciler/logpipeline/otel/reconciler_test.go b/internal/reconciler/logpipeline/otel/reconciler_test.go index 1e873ffa5..1759c49f2 100644 --- a/internal/reconciler/logpipeline/otel/reconciler_test.go +++ b/internal/reconciler/logpipeline/otel/reconciler_test.go @@ -32,9 +32,7 @@ func TestReconcile(t *testing.T) { overridesHandlerStub := &logpipelinemocks.OverridesHandler{} overridesHandlerStub.On("LoadOverrides", context.Background()).Return(&overrides.Config{}, nil) - testConfig := Config{ - TelemetryNamespace: "default", - } + telemetryNamespace := "default" t.Run("log gateway probing failed", func(t *testing.T) { pipeline := testutils.NewLogPipelineBuilder().WithName("pipeline").WithOTLPOutput().Build() @@ -61,7 +59,7 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, gatewayApplierDeleterMock, gatewayConfigBuilderMock, gatewayProberStub, @@ -109,7 +107,7 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, gatewayApplierDeleterMock, gatewayConfigBuilderMock, gatewayProberStub, @@ -157,7 +155,7 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, gatewayApplierDeleterMock, gatewayConfigBuilderMock, gatewayProberStub, diff --git a/internal/reconciler/logpipeline/otel/status.go b/internal/reconciler/logpipeline/otel/status.go index 27350e00a..7a1449d33 100644 --- a/internal/reconciler/logpipeline/otel/status.go +++ b/internal/reconciler/logpipeline/otel/status.go @@ -50,7 +50,7 @@ func (r *Reconciler) updateStatus(ctx context.Context, pipelineName string) erro func (r *Reconciler) setGatewayHealthyCondition(ctx context.Context, pipeline *telemetryv1alpha1.LogPipeline) { condition := commonstatus.GetGatewayHealthyCondition(ctx, - r.gatewayProber, types.NamespacedName{Name: otelcollector.LogGatewayName, Namespace: r.config.TelemetryNamespace}, + r.gatewayProber, types.NamespacedName{Name: otelcollector.LogGatewayName, Namespace: r.telemetryNamespace}, r.errToMessageConverter, commonstatus.SignalTypeLogs) condition.ObservedGeneration = pipeline.Generation diff --git a/internal/reconciler/metricpipeline/reconciler.go b/internal/reconciler/metricpipeline/reconciler.go index 2f4ca02a7..204758e1c 100644 --- a/internal/reconciler/metricpipeline/reconciler.go +++ b/internal/reconciler/metricpipeline/reconciler.go @@ -28,11 +28,6 @@ import ( const defaultReplicaCount int32 = 2 -type Config struct { - ModuleVersion string - TelemetryNamespace string -} - type AgentConfigBuilder interface { Build(pipelines []telemetryv1alpha1.MetricPipeline, options agent.BuildOptions) *agent.Config } @@ -71,7 +66,9 @@ type IstioStatusChecker interface { type Reconciler struct { client.Client - config Config + telemetryNamespace string + // TODO(skhalash): introduce an embed pkg exposing the module version set by go build + moduleVersion string agentApplierDeleter AgentApplierDeleter agentConfigBuilder AgentConfigBuilder @@ -89,7 +86,8 @@ type Reconciler struct { func New( client client.Client, - config Config, + telemetryNamespace string, + moduleVersion string, agentApplierDeleter AgentApplierDeleter, agentConfigBuilder AgentConfigBuilder, agentProber commonstatus.Prober, @@ -105,7 +103,8 @@ func New( ) *Reconciler { return &Reconciler{ Client: client, - config: config, + telemetryNamespace: telemetryNamespace, + moduleVersion: moduleVersion, agentApplierDeleter: agentApplierDeleter, agentConfigBuilder: agentConfigBuilder, agentProber: agentProber, @@ -244,8 +243,8 @@ func isMetricAgentRequired(pipeline *telemetryv1alpha1.MetricPipeline) bool { func (r *Reconciler) reconcileMetricGateway(ctx context.Context, pipeline *telemetryv1alpha1.MetricPipeline, allPipelines []telemetryv1alpha1.MetricPipeline) error { collectorConfig, collectorEnvVars, err := r.gatewayConfigBuilder.Build(ctx, allPipelines, gateway.BuildOptions{ - GatewayNamespace: r.config.TelemetryNamespace, - InstrumentationScopeVersion: r.config.ModuleVersion, + GatewayNamespace: r.telemetryNamespace, + InstrumentationScopeVersion: r.moduleVersion, }) if err != nil { @@ -290,8 +289,8 @@ func (r *Reconciler) reconcileMetricAgents(ctx context.Context, pipeline *teleme agentConfig := r.agentConfigBuilder.Build(allPipelines, agent.BuildOptions{ IstioEnabled: isIstioActive, IstioCertPath: otelcollector.IstioCertPath, - InstrumentationScopeVersion: r.config.ModuleVersion, - AgentNamespace: r.config.TelemetryNamespace, + InstrumentationScopeVersion: r.moduleVersion, + AgentNamespace: r.telemetryNamespace, }) agentConfigYAML, err := yaml.Marshal(agentConfig) diff --git a/internal/reconciler/metricpipeline/reconciler_test.go b/internal/reconciler/metricpipeline/reconciler_test.go index 64db39993..6f4ed3bee 100644 --- a/internal/reconciler/metricpipeline/reconciler_test.go +++ b/internal/reconciler/metricpipeline/reconciler_test.go @@ -46,9 +46,8 @@ func TestReconcile(t *testing.T) { istioStatusCheckerStub := &stubs.IstioStatusChecker{IsActive: false} - testConfig := Config{ - TelemetryNamespace: "default", - } + telemetryNamespace := "default" + moduleVersion := "1.0.0" t.Run("metric gateway deployment is not ready", func(t *testing.T) { pipeline := testutils.NewMetricPipelineBuilder().Build() @@ -81,7 +80,8 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, + moduleVersion, &mocks.AgentApplierDeleter{}, &mocks.AgentConfigBuilder{}, agentProberStub, @@ -141,7 +141,8 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, + moduleVersion, &mocks.AgentApplierDeleter{}, &mocks.AgentConfigBuilder{}, agentProberStub, @@ -201,7 +202,8 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, + moduleVersion, &mocks.AgentApplierDeleter{}, &mocks.AgentConfigBuilder{}, agentProberStub, @@ -267,7 +269,8 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, + moduleVersion, agentApplierDeleterMock, agentConfigBuilderMock, agentProberStub, @@ -335,7 +338,8 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, + moduleVersion, agentApplierDeleterMock, agentConfigBuilderMock, agentProberStub, @@ -403,7 +407,8 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, + moduleVersion, agentApplierDeleterMock, agentConfigBuilderMock, agentProberStub, @@ -472,7 +477,8 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, + moduleVersion, &mocks.AgentApplierDeleter{}, &mocks.AgentConfigBuilder{}, agentProberStub, @@ -535,7 +541,8 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, + moduleVersion, agentApplierDeleterMock, &mocks.AgentConfigBuilder{}, agentProberStub, @@ -599,7 +606,8 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, + moduleVersion, &mocks.AgentApplierDeleter{}, &mocks.AgentConfigBuilder{}, agentProberStub, @@ -761,7 +769,8 @@ func TestReconcile(t *testing.T) { errToMsg := &conditions.ErrorToMessageConverter{} sut := New( fakeClient, - testConfig, + telemetryNamespace, + moduleVersion, &mocks.AgentApplierDeleter{}, &mocks.AgentConfigBuilder{}, agentProberStub, @@ -905,7 +914,8 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, + moduleVersion, agentApplierDeleterMock, &mocks.AgentConfigBuilder{}, agentProberStub, @@ -990,7 +1000,8 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, + moduleVersion, &mocks.AgentApplierDeleter{}, &mocks.AgentConfigBuilder{}, agentProberStub, @@ -1060,7 +1071,8 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, + moduleVersion, &mocks.AgentApplierDeleter{}, &mocks.AgentConfigBuilder{}, agentProberStub, @@ -1134,7 +1146,8 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, + moduleVersion, agentApplierDeleterMock, &mocks.AgentConfigBuilder{}, agentProberStub, @@ -1259,7 +1272,8 @@ func TestReconcile(t *testing.T) { sut := Reconciler{ Client: fakeClient, - config: testConfig, + telemetryNamespace: telemetryNamespace, + moduleVersion: moduleVersion, agentConfigBuilder: agentConfigBuilderMock, gatewayConfigBuilder: gatewayConfigBuilderMock, agentApplierDeleter: agentApplierDeleterMock, diff --git a/internal/reconciler/metricpipeline/status.go b/internal/reconciler/metricpipeline/status.go index 2d759c4f2..d57de2f17 100644 --- a/internal/reconciler/metricpipeline/status.go +++ b/internal/reconciler/metricpipeline/status.go @@ -61,7 +61,7 @@ func (r *Reconciler) setAgentHealthyCondition(ctx context.Context, pipeline *tel if isMetricAgentRequired(pipeline) { condition = commonstatus.GetAgentHealthyCondition(ctx, r.agentProber, - types.NamespacedName{Name: otelcollector.MetricAgentName, Namespace: r.config.TelemetryNamespace}, + types.NamespacedName{Name: otelcollector.MetricAgentName, Namespace: r.telemetryNamespace}, r.errToMsgConverter, commonstatus.SignalTypeMetrics) } @@ -72,7 +72,7 @@ func (r *Reconciler) setAgentHealthyCondition(ctx context.Context, pipeline *tel func (r *Reconciler) setGatewayHealthyCondition(ctx context.Context, pipeline *telemetryv1alpha1.MetricPipeline) { condition := commonstatus.GetGatewayHealthyCondition(ctx, - r.gatewayProber, types.NamespacedName{Name: otelcollector.MetricGatewayName, Namespace: r.config.TelemetryNamespace}, + r.gatewayProber, types.NamespacedName{Name: otelcollector.MetricGatewayName, Namespace: r.telemetryNamespace}, r.errToMsgConverter, commonstatus.SignalTypeMetrics) condition.ObservedGeneration = pipeline.Generation diff --git a/internal/reconciler/tracepipeline/reconciler.go b/internal/reconciler/tracepipeline/reconciler.go index 4d91dd22a..60686541e 100644 --- a/internal/reconciler/tracepipeline/reconciler.go +++ b/internal/reconciler/tracepipeline/reconciler.go @@ -43,10 +43,6 @@ import ( const defaultReplicaCount int32 = 2 -type Config struct { - TelemetryNamespace string -} - type GatewayConfigBuilder interface { Build(ctx context.Context, pipelines []telemetryv1alpha1.TracePipeline) (*gateway.Config, otlpexporter.EnvVars, error) } @@ -76,7 +72,7 @@ type IstioStatusChecker interface { type Reconciler struct { client.Client - config Config + telemetryNamespace string // Dependencies flowHealthProber FlowHealthProber @@ -92,7 +88,7 @@ type Reconciler struct { func New( client client.Client, - config Config, + telemetryNamespace string, flowHealthProber FlowHealthProber, gatewayApplierDeleter GatewayApplierDeleter, gatewayConfigBuilder GatewayConfigBuilder, @@ -105,7 +101,7 @@ func New( ) *Reconciler { return &Reconciler{ Client: client, - config: config, + telemetryNamespace: telemetryNamespace, flowHealthProber: flowHealthProber, gatewayApplierDeleter: gatewayApplierDeleter, gatewayConfigBuilder: gatewayConfigBuilder, diff --git a/internal/reconciler/tracepipeline/reconciler_test.go b/internal/reconciler/tracepipeline/reconciler_test.go index 815a25351..4cd1a7ee6 100644 --- a/internal/reconciler/tracepipeline/reconciler_test.go +++ b/internal/reconciler/tracepipeline/reconciler_test.go @@ -45,9 +45,7 @@ func TestReconcile(t *testing.T) { istioStatusCheckerStub := &stubs.IstioStatusChecker{IsActive: false} - testConfig := Config{ - TelemetryNamespace: "default", - } + telemetryNamespace := "default" t.Run("trace gateway probing failed", func(t *testing.T) { pipeline := testutils.NewTracePipelineBuilder().WithName("pipeline").Build() @@ -79,7 +77,7 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, flowHealthProberStub, gatewayApplierDeleterMock, gatewayConfigBuilderMock, @@ -136,7 +134,7 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, flowHealthProberStub, gatewayApplierDeleterMock, gatewayConfigBuilderMock, @@ -193,7 +191,7 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, flowHealthProberStub, gatewayApplierDeleterMock, gatewayConfigBuilderMock, @@ -249,7 +247,7 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, flowHealthProberStub, gatewayApplierDeleterMock, gatewayConfigBuilderMock, @@ -323,7 +321,7 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, flowHealthProberStub, gatewayApplierDeleterMock, gatewayConfigBuilderMock, @@ -376,7 +374,7 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, flowHealthProberStub, &mocks.GatewayApplierDeleter{}, gatewayConfigBuilderMock, @@ -533,7 +531,7 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, flowHealthProberStub, gatewayApplierDeleterMock, gatewayConfigBuilderMock, @@ -668,7 +666,7 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, flowHealthProberStub, gatewayApplierDeleterMock, gatewayConfigBuilderMock, @@ -751,7 +749,7 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, flowHealthProberStub, gatewayApplierDeleterMock, gatewayConfigBuilderMock, @@ -816,7 +814,7 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, flowHealthProberStub, gatewayApplierDeleterMock, gatewayConfigBuilderMock, @@ -879,7 +877,7 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, flowHealthProberStub, gatewayApplierDeleterMock, gatewayConfigBuilderMock, @@ -970,7 +968,7 @@ func TestReconcile(t *testing.T) { sut := New( fakeClient, - testConfig, + telemetryNamespace, flowHealthProberStub, gatewayApplierDeleterMock, gatewayConfigBuilderMock, diff --git a/internal/reconciler/tracepipeline/status.go b/internal/reconciler/tracepipeline/status.go index 2e29ae555..688716c30 100644 --- a/internal/reconciler/tracepipeline/status.go +++ b/internal/reconciler/tracepipeline/status.go @@ -51,7 +51,7 @@ func (r *Reconciler) updateStatus(ctx context.Context, pipelineName string) erro func (r *Reconciler) setGatewayHealthyCondition(ctx context.Context, pipeline *telemetryv1alpha1.TracePipeline) { condition := commonstatus.GetGatewayHealthyCondition(ctx, - r.gatewayProber, types.NamespacedName{Name: otelcollector.TraceGatewayName, Namespace: r.config.TelemetryNamespace}, + r.gatewayProber, types.NamespacedName{Name: otelcollector.TraceGatewayName, Namespace: r.telemetryNamespace}, r.errToMsgConverter, commonstatus.SignalTypeTraces) condition.ObservedGeneration = pipeline.Generation