Fix

Signed-off-by: Pavol Loffay <[email protected]>

Author: pavolloffay

apis/v1beta1/collector_webhook.go

@@ -330,15 +330,13 @@ func (c CollectorWebhook) validateTargetAllocatorConfig(ctx context.Context, r *
 		return nil, fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the target allocation deployment", r.Spec.Mode)
 	}
 
-	// TODO ask Mikolaj https://github.com/open-telemetry/opentelemetry-operator/issues/2860
-	//if r.Spec.Mode == ModeDaemonSet && r.Spec.TargetAllocator.AllocationStrategy != TargetAllocatorAllocationStrategyPerNode {
-	//	return nil, fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which must be used with target allocation strategy %s ", r.Spec.Mode, OpenTelemetryTargetAllocatorAllocationStrategyPerNode)
-	//}
-
-	//// TODO ask Mikolaj https://github.com/open-telemetry/opentelemetry-operator/issues/2860
-	//if r.Spec.TargetAllocator.AllocationStrategy == OpenTelemetryTargetAllocatorAllocationStrategyPerNode && r.Spec.Mode != ModeDaemonSet {
-	//	return nil, fmt.Errorf("target allocation strategy %s is only supported in OpenTelemetry Collector mode %s", OpenTelemetryTargetAllocatorAllocationStrategyPerNode, ModeDaemonSet)
-	//}
+	if r.Spec.Mode == ModeDaemonSet && r.Spec.TargetAllocator.AllocationStrategy != TargetAllocatorAllocationStrategyPerNode {
+		return nil, fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which must be used with target allocation strategy %s ", r.Spec.Mode, TargetAllocatorAllocationStrategyPerNode)
+	}
+
+	if r.Spec.TargetAllocator.AllocationStrategy == TargetAllocatorAllocationStrategyPerNode && r.Spec.Mode != ModeDaemonSet {
+		return nil, fmt.Errorf("target allocation strategy %s is only supported in OpenTelemetry Collector mode %s", TargetAllocatorAllocationStrategyPerNode, ModeDaemonSet)
+	}
 
 	cfgYaml, err := r.Spec.Config.Yaml()
 	if err != nil {

apis/v1beta1/collector_webhook_test.go

@@ -482,9 +482,6 @@ var cfgYaml = `receivers:
        endpoint: 0.0.0.0:15268
 `
 
-// TODO: a lot of these tests use .Spec.MaxReplicas and .Spec.MinReplicas. These fields are
-// deprecated and moved to .Spec.Autoscaler. Fine to use these fields to test that old CRD is
-// still supported but should eventually be updated.
 func TestOTELColValidatingWebhook(t *testing.T) {
 	minusOne := int32(-1)
 	zero := int32(0)
@@ -704,20 +701,19 @@ func TestOTELColValidatingWebhook(t *testing.T) {
 			},
 			expectedErr: "the OpenTelemetry Spec Prometheus configuration is incorrect",
 		},
-		// TODO https://github.com/open-telemetry/opentelemetry-operator/issues/2860
-		//{
-		//	name: "invalid target allocation strategy",
-		//	otelcol: OpenTelemetryCollector{
-		//		Spec: OpenTelemetryCollectorSpec{
-		//			Mode: ModeDaemonSet,
-		//			TargetAllocator: TargetAllocatorEmbedded{
-		//				Enabled:            true,
-		//				AllocationStrategy: TargetAllocatorAllocationStrategyLeastWeighted,
-		//			},
-		//		},
-		//	},
-		//	expectedErr: "mode is set to daemonset, which must be used with target allocation strategy per-node",
-		//},
+		{
+			name: "invalid target allocation strategy",
+			otelcol: OpenTelemetryCollector{
+				Spec: OpenTelemetryCollectorSpec{
+					Mode: ModeDaemonSet,
+					TargetAllocator: TargetAllocatorEmbedded{
+						Enabled:            true,
+						AllocationStrategy: TargetAllocatorAllocationStrategyLeastWeighted,
+					},
+				},
+			},
+			expectedErr: "mode is set to daemonset, which must be used with target allocation strategy per-node",
+		},
 		{
 			name: "invalid port name",
 			otelcol: OpenTelemetryCollector{