fix: collector selection should not fail if there is a single sidecar (…

…#210) fixes #200 if there are multiple collectors, but a single sidecar, the collector selection should not fail. this fix ensure we filter the list of collectors first. Signed-off-by: Vincent Behar <[email protected]>
open-telemetry · Feb 23, 2021 · a13ca07 · a13ca07
1 parent 67a3403
commit a13ca07
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 29 deletions.
diff --git a/internal/podinjector/main.go b/internal/podinjector/main.go
@@ -161,22 +161,28 @@ func (p *podSidecarInjector) getCollectorInstance(ctx context.Context, ns corev1
 }
 
 func (p *podSidecarInjector) selectCollectorInstance(ctx context.Context, ns corev1.Namespace) (v1alpha1.OpenTelemetryCollector, error) {
-	otelcols := v1alpha1.OpenTelemetryCollectorList{}
+	var (
+		otelcols = v1alpha1.OpenTelemetryCollectorList{}
+		sidecars []v1alpha1.OpenTelemetryCollector
+	)
+
 	if err := p.client.List(ctx, &otelcols, client.InNamespace(ns.Name)); err != nil {
 		return v1alpha1.OpenTelemetryCollector{}, err
 	}
 
-	if len(otelcols.Items) == 0 {
-		return v1alpha1.OpenTelemetryCollector{}, ErrNoInstancesAvailable
-	}
-	if len(otelcols.Items) > 1 {
-		return v1alpha1.OpenTelemetryCollector{}, ErrMultipleInstancesPossible
+	for i := range otelcols.Items {
+		coll := otelcols.Items[i]
+		if coll.Spec.Mode == v1alpha1.ModeSidecar {
+			sidecars = append(sidecars, coll)
+		}
 	}
 
-	otelcol := otelcols.Items[0]
-	if otelcol.Spec.Mode != v1alpha1.ModeSidecar {
-		return v1alpha1.OpenTelemetryCollector{}, ErrInstanceNotSidecar
+	switch {
+	case len(sidecars) == 0:
+		return v1alpha1.OpenTelemetryCollector{}, ErrNoInstancesAvailable
+	case len(sidecars) > 1:
+		return v1alpha1.OpenTelemetryCollector{}, ErrMultipleInstancesPossible
+	default:
+		return sidecars[0], nil
 	}
-
-	return otelcol, nil
 }
diff --git a/internal/podinjector/main_test.go b/internal/podinjector/main_test.go
@@ -41,10 +41,10 @@ var logger = logf.Log.WithName("unit-tests")
 
 func TestShouldInjectSidecar(t *testing.T) {
 	for _, tt := range []struct {
-		name    string
-		ns      corev1.Namespace
-		pod     corev1.Pod
-		otelcol v1alpha1.OpenTelemetryCollector
+		name     string
+		ns       corev1.Namespace
+		pod      corev1.Pod
+		otelcols []v1alpha1.OpenTelemetryCollector
 	}{
 		{
 			// this is the simplest positive test: a pod is being created with an annotation
@@ -61,15 +61,15 @@ func TestShouldInjectSidecar(t *testing.T) {
 					Annotations: map[string]string{sidecar.Annotation: "my-instance"},
 				},
 			},
-			otelcol: v1alpha1.OpenTelemetryCollector{
+			otelcols: []v1alpha1.OpenTelemetryCollector{{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "my-instance",
 					Namespace: "my-namespace-simplest-positive-case",
 				},
 				Spec: v1alpha1.OpenTelemetryCollectorSpec{
 					Mode: v1alpha1.ModeSidecar,
 				},
-			},
+			}},
 		},
 		{
 			// in this case, the annotation is at the namespace instead of at the pod
@@ -81,18 +81,18 @@ func TestShouldInjectSidecar(t *testing.T) {
 				},
 			},
 			pod: corev1.Pod{},
-			otelcol: v1alpha1.OpenTelemetryCollector{
+			otelcols: []v1alpha1.OpenTelemetryCollector{{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "my-instance",
 					Namespace: "my-annotated-namespace",
 				},
 				Spec: v1alpha1.OpenTelemetryCollectorSpec{
 					Mode: v1alpha1.ModeSidecar,
 				},
-			},
+			}},
 		},
 		{
-			// now, we automatically select an existing otelcol
+			// now, we automatically select an existing sidecar otelcol
 			name: "auto-select based on the annotation's value",
 			ns: corev1.Namespace{
 				ObjectMeta: metav1.ObjectMeta{
@@ -101,13 +101,24 @@ func TestShouldInjectSidecar(t *testing.T) {
 				},
 			},
 			pod: corev1.Pod{},
-			otelcol: v1alpha1.OpenTelemetryCollector{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "my-instance",
-					Namespace: "my-namespace-with-autoselect",
+			otelcols: []v1alpha1.OpenTelemetryCollector{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "my-instance",
+						Namespace: "my-namespace-with-autoselect",
+					},
+					Spec: v1alpha1.OpenTelemetryCollectorSpec{
+						Mode: v1alpha1.ModeSidecar,
+					},
 				},
-				Spec: v1alpha1.OpenTelemetryCollectorSpec{
-					Mode: v1alpha1.ModeSidecar,
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "a-deployment-instance",
+						Namespace: "my-namespace-with-autoselect",
+					},
+					Spec: v1alpha1.OpenTelemetryCollectorSpec{
+						Mode: v1alpha1.ModeDeployment,
+					},
 				},
 			},
 		},
@@ -116,8 +127,10 @@ func TestShouldInjectSidecar(t *testing.T) {
 			err := k8sClient.Create(context.Background(), &tt.ns)
 			require.NoError(t, err)
 
-			err = k8sClient.Create(context.Background(), &tt.otelcol)
-			require.NoError(t, err)
+			for i := range tt.otelcols {
+				err := k8sClient.Create(context.Background(), &tt.otelcols[i])
+				require.NoError(t, err)
+			}
 
 			encoded, err := json.Marshal(tt.pod)
 			require.NoError(t, err)
@@ -163,7 +176,9 @@ func TestShouldInjectSidecar(t *testing.T) {
 			}
 
 			// cleanup
-			require.NoError(t, k8sClient.Delete(context.Background(), &tt.otelcol))
+			for i := range tt.otelcols {
+				require.NoError(t, k8sClient.Delete(context.Background(), &tt.otelcols[i]))
+			}
 			require.NoError(t, k8sClient.Delete(context.Background(), &tt.ns))
 		})
 	}