Clean up signatures (#2621)

* mostly working signatures

* Clean up types to only marshal once

* linting

* rename methods

Author: jaronoff97

controllers/builder_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/go-logr/logr"
 	monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
 	"github.com/stretchr/testify/require"
+	go_yaml "gopkg.in/yaml.v3"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
@@ -33,6 +34,7 @@ import (
 	colfeaturegate "go.opentelemetry.io/collector/featuregate"
 
 	"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
+	"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha2"
 	"github.com/open-telemetry/opentelemetry-operator/internal/config"
 	"github.com/open-telemetry/opentelemetry-operator/internal/manifests"
 	"github.com/open-telemetry/opentelemetry-operator/pkg/featuregate"
@@ -73,7 +75,7 @@ var (
 )
 
 func TestBuildCollector(t *testing.T) {
-	var goodConfig = `receivers:
+	var goodConfigYaml = `receivers:
   examplereceiver:
     endpoint: "0.0.0.0:12345"
 exporters:
@@ -84,9 +86,13 @@ service:
       receivers: [examplereceiver]
       exporters: [logging]
 `
+
+	goodConfig := v1alpha2.Config{}
+	err := go_yaml.Unmarshal([]byte(goodConfigYaml), &goodConfig)
+	require.NoError(t, err)
 	one := int32(1)
 	type args struct {
-		instance v1alpha1.OpenTelemetryCollector
+		instance v1alpha2.OpenTelemetryCollector
 	}
 	tests := []struct {
 		name    string
@@ -97,16 +103,18 @@ service:
 		{
 			name: "base case",
 			args: args{
-				instance: v1alpha1.OpenTelemetryCollector{
+				instance: v1alpha2.OpenTelemetryCollector{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "test",
 						Namespace: "test",
 					},
-					Spec: v1alpha1.OpenTelemetryCollectorSpec{
-						Replicas: &one,
-						Mode:     "deployment",
-						Image:    "test",
-						Config:   goodConfig,
+					Spec: v1alpha2.OpenTelemetryCollectorSpec{
+						OpenTelemetryCommonFields: v1alpha2.OpenTelemetryCommonFields{
+							Image:    "test",
+							Replicas: &one,
+						},
+						Mode:   "deployment",
+						Config: goodConfig,
 					},
 				},
 			},
@@ -334,17 +342,19 @@ service:
 		{
 			name: "ingress",
 			args: args{
-				instance: v1alpha1.OpenTelemetryCollector{
+				instance: v1alpha2.OpenTelemetryCollector{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "test",
 						Namespace: "test",
 					},
-					Spec: v1alpha1.OpenTelemetryCollectorSpec{
-						Replicas: &one,
-						Mode:     "deployment",
-						Image:    "test",
-						Ingress: v1alpha1.Ingress{
-							Type:     v1alpha1.IngressTypeNginx,
+					Spec: v1alpha2.OpenTelemetryCollectorSpec{
+						OpenTelemetryCommonFields: v1alpha2.OpenTelemetryCommonFields{
+							Image:    "test",
+							Replicas: &one,
+						},
+						Mode: "deployment",
+						Ingress: v1alpha2.Ingress{
+							Type:     v1alpha2.IngressTypeNginx,
 							Hostname: "example.com",
 							Annotations: map[string]string{
 								"something": "true",
@@ -617,17 +627,19 @@ service:
 		{
 			name: "specified service account case",
 			args: args{
-				instance: v1alpha1.OpenTelemetryCollector{
+				instance: v1alpha2.OpenTelemetryCollector{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "test",
 						Namespace: "test",
 					},
-					Spec: v1alpha1.OpenTelemetryCollectorSpec{
-						Replicas:       &one,
-						Mode:           "deployment",
-						Image:          "test",
-						Config:         goodConfig,
-						ServiceAccount: "my-special-sa",
+					Spec: v1alpha2.OpenTelemetryCollectorSpec{
+						OpenTelemetryCommonFields: v1alpha2.OpenTelemetryCommonFields{
+							Image:          "test",
+							Replicas:       &one,
+							ServiceAccount: "my-special-sa",
+						},
+						Mode:   "deployment",
+						Config: goodConfig,
 					},
 				},
 			},
@@ -1083,7 +1095,7 @@ endpoint: ws://opamp-server:4320/v1/opamp
 }
 
 func TestBuildTargetAllocator(t *testing.T) {
-	var goodConfig = `
+	var goodConfigYaml = `
 receivers:
   prometheus:
     config:
@@ -1108,9 +1120,13 @@ service:
       receivers: [prometheus]
       exporters: [logging]
 `
+
+	goodConfig := v1alpha2.Config{}
+	err := go_yaml.Unmarshal([]byte(goodConfigYaml), &goodConfig)
+	require.NoError(t, err)
 	one := int32(1)
 	type args struct {
-		instance v1alpha1.OpenTelemetryCollector
+		instance v1alpha2.OpenTelemetryCollector
 	}
 	tests := []struct {
 		name         string
@@ -1122,16 +1138,18 @@ service:
 		{
 			name: "base case",
 			args: args{
-				instance: v1alpha1.OpenTelemetryCollector{
+				instance: v1alpha2.OpenTelemetryCollector{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "test",
 						Namespace: "test",
 					},
-					Spec: v1alpha1.OpenTelemetryCollectorSpec{
-						Replicas: &one,
-						Mode:     "statefulset",
-						Image:    "test",
-						Config:   goodConfig,
+					Spec: v1alpha2.OpenTelemetryCollectorSpec{
+						OpenTelemetryCommonFields: v1alpha2.OpenTelemetryCommonFields{
+							Image:    "test",
+							Replicas: &one,
+						},
+						Mode:   "statefulset",
+						Config: goodConfig,
 						TargetAllocator: v1alpha1.OpenTelemetryTargetAllocator{
 							Enabled:        true,
 							FilterStrategy: "relabel-config",
@@ -1516,16 +1534,18 @@ prometheus_cr:
 		{
 			name: "enable metrics case",
 			args: args{
-				instance: v1alpha1.OpenTelemetryCollector{
+				instance: v1alpha2.OpenTelemetryCollector{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "test",
 						Namespace: "test",
 					},
-					Spec: v1alpha1.OpenTelemetryCollectorSpec{
-						Replicas: &one,
-						Mode:     "statefulset",
-						Image:    "test",
-						Config:   goodConfig,
+					Spec: v1alpha2.OpenTelemetryCollectorSpec{
+						OpenTelemetryCommonFields: v1alpha2.OpenTelemetryCommonFields{
+							Image:    "test",
+							Replicas: &one,
+						},
+						Mode:   "statefulset",
+						Config: goodConfig,
 						TargetAllocator: v1alpha1.OpenTelemetryTargetAllocator{
 							Enabled: true,
 							PrometheusCR: v1alpha1.OpenTelemetryTargetAllocatorPrometheusCR{

controllers/opentelemetrycollector_controller.go

@@ -32,6 +32,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
+	"github.com/open-telemetry/opentelemetry-operator/internal/api/convert"
 	"github.com/open-telemetry/opentelemetry-operator/internal/config"
 	"github.com/open-telemetry/opentelemetry-operator/internal/manifests"
 	collectorStatus "github.com/open-telemetry/opentelemetry-operator/internal/status/collector"
@@ -56,15 +57,19 @@ type Params struct {
 	Config   config.Config
 }
 
-func (r *OpenTelemetryCollectorReconciler) getParams(instance v1alpha1.OpenTelemetryCollector) manifests.Params {
+func (r *OpenTelemetryCollectorReconciler) getParams(instance v1alpha1.OpenTelemetryCollector) (manifests.Params, error) {
+	otelCol, err := convert.V1Alpha1to2(instance)
+	if err != nil {
+		return manifests.Params{}, err
+	}
 	return manifests.Params{
 		Config:   r.config,
 		Client:   r.Client,
-		OtelCol:  instance,
+		OtelCol:  otelCol,
 		Log:      r.log,
 		Scheme:   r.scheme,
 		Recorder: r.recorder,
-	}
+	}, nil
 }
 
 // NewReconciler creates a new reconciler for OpenTelemetryCollector objects.
@@ -119,14 +124,20 @@ func (r *OpenTelemetryCollectorReconciler) Reconcile(ctx context.Context, req ct
 		return ctrl.Result{}, nil
 	}
 
-	params := r.getParams(instance)
+	params, err := r.getParams(instance)
+	if err != nil {
+		log.Error(err, "Failed to create manifest.Params")
+		return ctrl.Result{}, err
+	}
 
 	desiredObjects, buildErr := BuildCollector(params)
 	if buildErr != nil {
 		return ctrl.Result{}, buildErr
 	}
-	err := reconcileDesiredObjects(ctx, r.Client, log, &params.OtelCol, params.Scheme, desiredObjects...)
-	return collectorStatus.HandleReconcileStatus(ctx, log, params, err)
+	// TODO: https://github.com/open-telemetry/opentelemetry-operator/issues/2620
+	// TODO: Change &instance to use params.OtelCol
+	err = reconcileDesiredObjects(ctx, r.Client, log, &instance, params.Scheme, desiredObjects...)
+	return collectorStatus.HandleReconcileStatus(ctx, log, params, instance, err)
 }
 
 // SetupWithManager tells the manager what our controller is interested in.