OpAMP Bridge: Applier support for OTel CRD v1beta1 API Version (#3080)

* OpAMP Bridge: Applier support for OTel CRD v1beta1 API Version **Description**: Updates the opamp bridge's config applier interface and client to support opentelemetry.io/v1beta1, explicitly removing support for applying OpenTelemetryCollector configurations of the opentelemetry.io/v1alph1 version **Link to tracking Issue(s)**: #2985 **Testing**: Update Documentation: n/a * Add chloggen entry * Simplify list instances code * Register v1beta1 types * Register v1beta1 group version --------- Co-authored-by: Jacob Aronoff <jaronoff97@users.noreply.github.com>
open-telemetry · Jul 9, 2024 · e9bd73d · e9bd73d
1 parent b806c27
commit e9bd73d
Show file tree

Hide file tree

Showing 15 changed files with 290 additions and 147 deletions.
diff --git a/.chloggen/opamp-bridge-v1beta1-support.yaml b/.chloggen/opamp-bridge-v1beta1-support.yaml
@@ -0,0 +1,20 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: breaking
+
+# 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: opamp
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Adds support for v1beta1 OpenTelemetry Collector API in the OpAMP Bridge
+
+# One or more tracking issues related to the change
+issues: [2985]
+
+# (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: |
+  This change adds support for the OpAMP Bridge to manage and apply OpenTelemetry Collectors using the v1beta1 API in 
+  the OpAMP Bridge. This change removes support for applying OpenTelemetry Collectors using the v1alpha1 API version.
+  The v1beta1 API is the latest version of the OpenTelemetry Collector API and is the recommended version for new 
+  deployments.
diff --git a/cmd/operator-opamp-bridge/agent/agent.go b/cmd/operator-opamp-bridge/agent/agent.go
@@ -30,7 +30,7 @@ import (
 	"k8s.io/utils/clock"
 	"sigs.k8s.io/yaml"
 
-	"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
+	"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
 	"github.com/open-telemetry/opentelemetry-operator/cmd/operator-opamp-bridge/config"
 	"github.com/open-telemetry/opentelemetry-operator/cmd/operator-opamp-bridge/metrics"
 	"github.com/open-telemetry/opentelemetry-operator/cmd/operator-opamp-bridge/operator"
@@ -136,7 +136,7 @@ func (agent *Agent) generateCollectorPoolHealth() (map[string]*protobufs.Compone
 }
 
 // getCollectorSelector destructures the collectors scale selector if present, if uses the labelmap from the operator.
-func (agent *Agent) getCollectorSelector(col v1alpha1.OpenTelemetryCollector) map[string]string {
+func (agent *Agent) getCollectorSelector(col v1beta1.OpenTelemetryCollector) map[string]string {
 	if len(col.Status.Scale.Selector) > 0 {
 		selMap := map[string]string{}
 		for _, kvPair := range strings.Split(col.Status.Scale.Selector, ",") {

diff --git a/cmd/operator-opamp-bridge/agent/agent_test.go b/cmd/operator-opamp-bridge/agent/agent_test.go
@@ -39,6 +39,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
+	"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
 	"github.com/open-telemetry/opentelemetry-operator/cmd/operator-opamp-bridge/config"
 	"github.com/open-telemetry/opentelemetry-operator/cmd/operator-opamp-bridge/operator"
 )
@@ -147,15 +148,15 @@ type mockOpampClient struct {
 	settings            types.StartSettings
 }
 
-func (m *mockOpampClient) SetCustomCapabilities(customCapabilities *protobufs.CustomCapabilities) error {
+func (m *mockOpampClient) SetCustomCapabilities(_ *protobufs.CustomCapabilities) error {
 	return nil
 }
 
-func (m *mockOpampClient) SendCustomMessage(message *protobufs.CustomMessage) (messageSendingChannel chan struct{}, err error) {
+func (m *mockOpampClient) SendCustomMessage(_ *protobufs.CustomMessage) (messageSendingChannel chan struct{}, err error) {
 	return nil, nil
 }
 
-func (m *mockOpampClient) RequestConnectionSettings(request *protobufs.ConnectionSettingsRequest) error {
+func (m *mockOpampClient) RequestConnectionSettings(_ *protobufs.ConnectionSettingsRequest) error {
 	return nil
 }
 
@@ -201,6 +202,7 @@ func (m *mockOpampClient) SetPackageStatuses(_ *protobufs.PackageStatuses) error
 func getFakeApplier(t *testing.T, conf *config.Config, lists ...runtimeClient.ObjectList) *operator.Client {
 	schemeBuilder := runtime.NewSchemeBuilder(func(s *runtime.Scheme) error {
 		s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.OpenTelemetryCollector{}, &v1alpha1.OpenTelemetryCollectorList{})
+		s.AddKnownTypes(v1beta1.GroupVersion, &v1beta1.OpenTelemetryCollector{}, &v1beta1.OpenTelemetryCollectorList{})
 		s.AddKnownTypes(v1.SchemeGroupVersion, &v1.Pod{}, &v1.PodList{})
 		metav1.AddToGroupVersion(s, v1alpha1.GroupVersion)
 		return nil
@@ -414,14 +416,17 @@ func TestAgent_getHealth(t *testing.T) {
 			agent.clock = fakeClock
 			err := agent.Start()
 			defer agent.Shutdown()
+
 			require.NoError(t, err, "should be able to start agent")
 			if len(tt.args.configs) > 0 {
-				require.True(t, len(tt.args.configs) == len(tt.want), "must have an equal amount of configs and checks.")
+				require.Len(t, tt.args.configs, len(tt.want), "must have an equal amount of configs and checks.")
 			} else {
 				require.Len(t, tt.want, 1, "must have exactly one want if no config is supplied.")
 				require.Equal(t, tt.want[0], agent.getHealth())
 			}
+
 			for i, configMap := range tt.args.configs {
+				var data *types.MessageData
 				data, err := getMessageDataFromConfigFile(configMap)
 				require.NoError(t, err, "should be able to load data")
 				agent.onMessage(tt.args.ctx, data)
@@ -495,7 +500,8 @@ func TestAgent_onMessage(t *testing.T) {
 						"name: " + testCollectorName,
 						"namespace: " + testNamespace,
 						"send_batch_size: 10000",
-						"receivers: [otlp]",
+						"receivers:",
+						"- otlp",
 						"status:",
 					},
 				},
@@ -523,7 +529,8 @@ func TestAgent_onMessage(t *testing.T) {
 						"name: " + testCollectorName,
 						"namespace: " + testNamespace,
 						"send_batch_size: 10000",
-						"receivers: [otlp]",
+						"receivers:",
+						"- otlp",
 						"status:",
 					},
 				},
@@ -549,7 +556,7 @@ func TestAgent_onMessage(t *testing.T) {
 				status: &protobufs.RemoteConfigStatus{
 					LastRemoteConfigHash: []byte(invalidYamlConfigHash),
 					Status:               protobufs.RemoteConfigStatuses_RemoteConfigStatuses_FAILED,
-					ErrorMessage:         "error converting YAML to JSON: yaml: line 23: could not find expected ':'",
+					ErrorMessage:         "failed to unmarshal config into v1beta1 API Version: error converting YAML to JSON: yaml: line 23: could not find expected ':'",
 				},
 			},
 		},
@@ -571,7 +578,8 @@ func TestAgent_onMessage(t *testing.T) {
 						"name: " + testCollectorName,
 						"namespace: " + testNamespace,
 						"send_batch_size: 10000",
-						"receivers: [otlp]",
+						"receivers:",
+						"- otlp",
 						"status:",
 					},
 				},
@@ -655,7 +663,9 @@ func TestAgent_onMessage(t *testing.T) {
 						"name: " + testCollectorName,
 						"namespace: " + testNamespace,
 						"send_batch_size: 10000",
-						"processors: [memory_limiter, batch]",
+						"processors:",
+						"- memory_limiter",
+						"- batch",
 						"replicas: 3",
 						"status:",
 					},
@@ -706,7 +716,7 @@ func TestAgent_onMessage(t *testing.T) {
 				nextStatus: &protobufs.RemoteConfigStatus{
 					LastRemoteConfigHash: []byte(invalidYamlConfigHash), // The new hash should be of the bad config
 					Status:               protobufs.RemoteConfigStatuses_RemoteConfigStatuses_FAILED,
-					ErrorMessage:         "error converting YAML to JSON: yaml: line 23: could not find expected ':'",
+					ErrorMessage:         "failed to unmarshal config into v1beta1 API Version: error converting YAML to JSON: yaml: line 23: could not find expected ':'",
 				},
 			},
 		},
@@ -752,7 +762,9 @@ func TestAgent_onMessage(t *testing.T) {
 						"name: " + otherCollectorName,
 						"namespace: " + testNamespace,
 						"send_batch_size: 10000",
-						"processors: [memory_limiter, batch]",
+						"processors:",
+						"- memory_limiter",
+						"- batch",
 						"status:",
 					},
 				},
@@ -799,14 +811,17 @@ func TestAgent_onMessage(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			mockClient := &mockOpampClient{}
+
 			conf := config.NewConfig(logr.Discard())
 			loadErr := config.LoadFromFile(conf, tt.fields.configFile)
 			require.NoError(t, loadErr, "should be able to load config")
+
 			applier := getFakeApplier(t, conf)
 			agent := NewAgent(l, applier, conf, mockClient)
 			err := agent.Start()
 			defer agent.Shutdown()
 			require.NoError(t, err, "should be able to start agent")
+
 			data, err := getMessageDataFromConfigFile(tt.args.configFile)
 			require.NoError(t, err, "should be able to load data")
 			agent.onMessage(tt.args.ctx, data)
@@ -818,17 +833,20 @@ func TestAgent_onMessage(t *testing.T) {
 			}
 			assert.NotNilf(t, effectiveConfig.ConfigMap.GetConfigMap(), "configmap should have data")
 			for colNameNamespace, expectedContents := range tt.want.contents {
-				assert.Contains(t, effectiveConfig.ConfigMap.GetConfigMap(), colNameNamespace)
+				configFileMap := effectiveConfig.ConfigMap.GetConfigMap()
+				require.Contains(t, configFileMap, colNameNamespace)
+				configFileString := string(configFileMap[colNameNamespace].GetBody())
 				for _, content := range expectedContents {
-					asString := string(effectiveConfig.ConfigMap.GetConfigMap()[colNameNamespace].GetBody())
-					assert.Contains(t, asString, content)
+					assert.Contains(t, configFileString, content, "config should contain %s", content)
 				}
 			}
 			assert.Equal(t, tt.want.status, mockClient.lastStatus)
+
 			if tt.args.nextConfigFile == nil {
 				// Nothing left to do!
 				return
 			}
+
 			nextData, err := getMessageDataFromConfigFile(tt.args.nextConfigFile)
 			require.NoError(t, err, "should be able to load updated data")
 			agent.onMessage(tt.args.ctx, nextData)
@@ -837,10 +855,11 @@ func TestAgent_onMessage(t *testing.T) {
 			assert.Equal(t, nextEffectiveConfig, mockClient.lastEffectiveConfig, "client's config should be updated")
 			assert.NotNilf(t, nextEffectiveConfig.ConfigMap.GetConfigMap(), "configmap should have updated data")
 			for colNameNamespace, expectedContents := range tt.want.nextContents {
-				assert.Contains(t, nextEffectiveConfig.ConfigMap.GetConfigMap(), colNameNamespace)
+				configFileMap := nextEffectiveConfig.ConfigMap.GetConfigMap()
+				require.Contains(t, configFileMap, colNameNamespace)
+				configFileString := string(configFileMap[colNameNamespace].GetBody())
 				for _, content := range expectedContents {
-					asString := string(nextEffectiveConfig.ConfigMap.GetConfigMap()[colNameNamespace].GetBody())
-					assert.Contains(t, asString, content)
+					assert.Contains(t, configFileString, content)
 				}
 			}
 			assert.Equal(t, tt.want.nextStatus, mockClient.lastStatus)

diff --git a/cmd/operator-opamp-bridge/agent/testdata/basic.yaml b/cmd/operator-opamp-bridge/agent/testdata/basic.yaml
@@ -1,10 +1,11 @@
+apiVersion: opentelemetry.io/v1beta1
 kind: OpenTelemetryCollector
 metadata:
   name: simplest
   labels:
     "opentelemetry.io/opamp-managed": "true"
 spec:
-  config: |
+  config:
     receivers:
       otlp:
         protocols:

diff --git a/cmd/operator-opamp-bridge/agent/testdata/invalid.yaml b/cmd/operator-opamp-bridge/agent/testdata/invalid.yaml
@@ -4,7 +4,7 @@ metadata:
   labels:
     "opentelemetry.io/opamp-managed": "true"
 spec:
-  config: |
+  config:
     receivers:
       otlp:
         protocols:

diff --git a/cmd/operator-opamp-bridge/agent/testdata/updated.yaml b/cmd/operator-opamp-bridge/agent/testdata/updated.yaml
@@ -1,10 +1,11 @@
+apiVersion: opentelemetry.io/v1beta1
 kind: OpenTelemetryCollector
 metadata:
   name: simplest
   labels:
     "opentelemetry.io/opamp-managed": "test-bridge"
 spec:
-  config: |
+  config:
     receivers:
       otlp:
         protocols:

diff --git a/cmd/operator-opamp-bridge/config/config.go b/cmd/operator-opamp-bridge/config/config.go
@@ -41,6 +41,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 
 	"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
+	"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
 	"github.com/open-telemetry/opentelemetry-operator/cmd/operator-opamp-bridge/logger"
 )
 
@@ -56,7 +57,9 @@ var (
 
 func registerKnownTypes(s *k8sruntime.Scheme) error {
 	s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.OpenTelemetryCollector{}, &v1alpha1.OpenTelemetryCollectorList{})
+	s.AddKnownTypes(v1beta1.GroupVersion, &v1beta1.OpenTelemetryCollector{}, &v1beta1.OpenTelemetryCollectorList{})
 	metav1.AddToGroupVersion(s, v1alpha1.GroupVersion)
+	metav1.AddToGroupVersion(s, v1beta1.GroupVersion)
 	return nil
 }