Skip to content

Commit e9bd73d

Browse files
gdfastjaronoff97
andauthored
OpAMP Bridge: Applier support for OTel CRD v1beta1 API Version (open-telemetry#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)**: open-telemetry#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 <[email protected]>
1 parent b806c27 commit e9bd73d

15 files changed

+290
-147
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
2+
change_type: breaking
3+
4+
# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
5+
component: opamp
6+
7+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
8+
note: Adds support for v1beta1 OpenTelemetry Collector API in the OpAMP Bridge
9+
10+
# One or more tracking issues related to the change
11+
issues: [2985]
12+
13+
# (Optional) One or more lines of additional information to render under the primary note.
14+
# These lines will be padded with 2 spaces and then inserted directly into the document.
15+
# Use pipe (|) for multiline entries.
16+
subtext: |
17+
This change adds support for the OpAMP Bridge to manage and apply OpenTelemetry Collectors using the v1beta1 API in
18+
the OpAMP Bridge. This change removes support for applying OpenTelemetry Collectors using the v1alpha1 API version.
19+
The v1beta1 API is the latest version of the OpenTelemetry Collector API and is the recommended version for new
20+
deployments.

cmd/operator-opamp-bridge/agent/agent.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import (
3030
"k8s.io/utils/clock"
3131
"sigs.k8s.io/yaml"
3232

33-
"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
33+
"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
3434
"github.com/open-telemetry/opentelemetry-operator/cmd/operator-opamp-bridge/config"
3535
"github.com/open-telemetry/opentelemetry-operator/cmd/operator-opamp-bridge/metrics"
3636
"github.com/open-telemetry/opentelemetry-operator/cmd/operator-opamp-bridge/operator"
@@ -136,7 +136,7 @@ func (agent *Agent) generateCollectorPoolHealth() (map[string]*protobufs.Compone
136136
}
137137

138138
// getCollectorSelector destructures the collectors scale selector if present, if uses the labelmap from the operator.
139-
func (agent *Agent) getCollectorSelector(col v1alpha1.OpenTelemetryCollector) map[string]string {
139+
func (agent *Agent) getCollectorSelector(col v1beta1.OpenTelemetryCollector) map[string]string {
140140
if len(col.Status.Scale.Selector) > 0 {
141141
selMap := map[string]string{}
142142
for _, kvPair := range strings.Split(col.Status.Scale.Selector, ",") {

cmd/operator-opamp-bridge/agent/agent_test.go

+36-17
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import (
3939
"sigs.k8s.io/controller-runtime/pkg/client/fake"
4040

4141
"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
42+
"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
4243
"github.com/open-telemetry/opentelemetry-operator/cmd/operator-opamp-bridge/config"
4344
"github.com/open-telemetry/opentelemetry-operator/cmd/operator-opamp-bridge/operator"
4445
)
@@ -147,15 +148,15 @@ type mockOpampClient struct {
147148
settings types.StartSettings
148149
}
149150

150-
func (m *mockOpampClient) SetCustomCapabilities(customCapabilities *protobufs.CustomCapabilities) error {
151+
func (m *mockOpampClient) SetCustomCapabilities(_ *protobufs.CustomCapabilities) error {
151152
return nil
152153
}
153154

154-
func (m *mockOpampClient) SendCustomMessage(message *protobufs.CustomMessage) (messageSendingChannel chan struct{}, err error) {
155+
func (m *mockOpampClient) SendCustomMessage(_ *protobufs.CustomMessage) (messageSendingChannel chan struct{}, err error) {
155156
return nil, nil
156157
}
157158

158-
func (m *mockOpampClient) RequestConnectionSettings(request *protobufs.ConnectionSettingsRequest) error {
159+
func (m *mockOpampClient) RequestConnectionSettings(_ *protobufs.ConnectionSettingsRequest) error {
159160
return nil
160161
}
161162

@@ -201,6 +202,7 @@ func (m *mockOpampClient) SetPackageStatuses(_ *protobufs.PackageStatuses) error
201202
func getFakeApplier(t *testing.T, conf *config.Config, lists ...runtimeClient.ObjectList) *operator.Client {
202203
schemeBuilder := runtime.NewSchemeBuilder(func(s *runtime.Scheme) error {
203204
s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.OpenTelemetryCollector{}, &v1alpha1.OpenTelemetryCollectorList{})
205+
s.AddKnownTypes(v1beta1.GroupVersion, &v1beta1.OpenTelemetryCollector{}, &v1beta1.OpenTelemetryCollectorList{})
204206
s.AddKnownTypes(v1.SchemeGroupVersion, &v1.Pod{}, &v1.PodList{})
205207
metav1.AddToGroupVersion(s, v1alpha1.GroupVersion)
206208
return nil
@@ -414,14 +416,17 @@ func TestAgent_getHealth(t *testing.T) {
414416
agent.clock = fakeClock
415417
err := agent.Start()
416418
defer agent.Shutdown()
419+
417420
require.NoError(t, err, "should be able to start agent")
418421
if len(tt.args.configs) > 0 {
419-
require.True(t, len(tt.args.configs) == len(tt.want), "must have an equal amount of configs and checks.")
422+
require.Len(t, tt.args.configs, len(tt.want), "must have an equal amount of configs and checks.")
420423
} else {
421424
require.Len(t, tt.want, 1, "must have exactly one want if no config is supplied.")
422425
require.Equal(t, tt.want[0], agent.getHealth())
423426
}
427+
424428
for i, configMap := range tt.args.configs {
429+
var data *types.MessageData
425430
data, err := getMessageDataFromConfigFile(configMap)
426431
require.NoError(t, err, "should be able to load data")
427432
agent.onMessage(tt.args.ctx, data)
@@ -495,7 +500,8 @@ func TestAgent_onMessage(t *testing.T) {
495500
"name: " + testCollectorName,
496501
"namespace: " + testNamespace,
497502
"send_batch_size: 10000",
498-
"receivers: [otlp]",
503+
"receivers:",
504+
"- otlp",
499505
"status:",
500506
},
501507
},
@@ -523,7 +529,8 @@ func TestAgent_onMessage(t *testing.T) {
523529
"name: " + testCollectorName,
524530
"namespace: " + testNamespace,
525531
"send_batch_size: 10000",
526-
"receivers: [otlp]",
532+
"receivers:",
533+
"- otlp",
527534
"status:",
528535
},
529536
},
@@ -549,7 +556,7 @@ func TestAgent_onMessage(t *testing.T) {
549556
status: &protobufs.RemoteConfigStatus{
550557
LastRemoteConfigHash: []byte(invalidYamlConfigHash),
551558
Status: protobufs.RemoteConfigStatuses_RemoteConfigStatuses_FAILED,
552-
ErrorMessage: "error converting YAML to JSON: yaml: line 23: could not find expected ':'",
559+
ErrorMessage: "failed to unmarshal config into v1beta1 API Version: error converting YAML to JSON: yaml: line 23: could not find expected ':'",
553560
},
554561
},
555562
},
@@ -571,7 +578,8 @@ func TestAgent_onMessage(t *testing.T) {
571578
"name: " + testCollectorName,
572579
"namespace: " + testNamespace,
573580
"send_batch_size: 10000",
574-
"receivers: [otlp]",
581+
"receivers:",
582+
"- otlp",
575583
"status:",
576584
},
577585
},
@@ -655,7 +663,9 @@ func TestAgent_onMessage(t *testing.T) {
655663
"name: " + testCollectorName,
656664
"namespace: " + testNamespace,
657665
"send_batch_size: 10000",
658-
"processors: [memory_limiter, batch]",
666+
"processors:",
667+
"- memory_limiter",
668+
"- batch",
659669
"replicas: 3",
660670
"status:",
661671
},
@@ -706,7 +716,7 @@ func TestAgent_onMessage(t *testing.T) {
706716
nextStatus: &protobufs.RemoteConfigStatus{
707717
LastRemoteConfigHash: []byte(invalidYamlConfigHash), // The new hash should be of the bad config
708718
Status: protobufs.RemoteConfigStatuses_RemoteConfigStatuses_FAILED,
709-
ErrorMessage: "error converting YAML to JSON: yaml: line 23: could not find expected ':'",
719+
ErrorMessage: "failed to unmarshal config into v1beta1 API Version: error converting YAML to JSON: yaml: line 23: could not find expected ':'",
710720
},
711721
},
712722
},
@@ -752,7 +762,9 @@ func TestAgent_onMessage(t *testing.T) {
752762
"name: " + otherCollectorName,
753763
"namespace: " + testNamespace,
754764
"send_batch_size: 10000",
755-
"processors: [memory_limiter, batch]",
765+
"processors:",
766+
"- memory_limiter",
767+
"- batch",
756768
"status:",
757769
},
758770
},
@@ -799,14 +811,17 @@ func TestAgent_onMessage(t *testing.T) {
799811
for _, tt := range tests {
800812
t.Run(tt.name, func(t *testing.T) {
801813
mockClient := &mockOpampClient{}
814+
802815
conf := config.NewConfig(logr.Discard())
803816
loadErr := config.LoadFromFile(conf, tt.fields.configFile)
804817
require.NoError(t, loadErr, "should be able to load config")
818+
805819
applier := getFakeApplier(t, conf)
806820
agent := NewAgent(l, applier, conf, mockClient)
807821
err := agent.Start()
808822
defer agent.Shutdown()
809823
require.NoError(t, err, "should be able to start agent")
824+
810825
data, err := getMessageDataFromConfigFile(tt.args.configFile)
811826
require.NoError(t, err, "should be able to load data")
812827
agent.onMessage(tt.args.ctx, data)
@@ -818,17 +833,20 @@ func TestAgent_onMessage(t *testing.T) {
818833
}
819834
assert.NotNilf(t, effectiveConfig.ConfigMap.GetConfigMap(), "configmap should have data")
820835
for colNameNamespace, expectedContents := range tt.want.contents {
821-
assert.Contains(t, effectiveConfig.ConfigMap.GetConfigMap(), colNameNamespace)
836+
configFileMap := effectiveConfig.ConfigMap.GetConfigMap()
837+
require.Contains(t, configFileMap, colNameNamespace)
838+
configFileString := string(configFileMap[colNameNamespace].GetBody())
822839
for _, content := range expectedContents {
823-
asString := string(effectiveConfig.ConfigMap.GetConfigMap()[colNameNamespace].GetBody())
824-
assert.Contains(t, asString, content)
840+
assert.Contains(t, configFileString, content, "config should contain %s", content)
825841
}
826842
}
827843
assert.Equal(t, tt.want.status, mockClient.lastStatus)
844+
828845
if tt.args.nextConfigFile == nil {
829846
// Nothing left to do!
830847
return
831848
}
849+
832850
nextData, err := getMessageDataFromConfigFile(tt.args.nextConfigFile)
833851
require.NoError(t, err, "should be able to load updated data")
834852
agent.onMessage(tt.args.ctx, nextData)
@@ -837,10 +855,11 @@ func TestAgent_onMessage(t *testing.T) {
837855
assert.Equal(t, nextEffectiveConfig, mockClient.lastEffectiveConfig, "client's config should be updated")
838856
assert.NotNilf(t, nextEffectiveConfig.ConfigMap.GetConfigMap(), "configmap should have updated data")
839857
for colNameNamespace, expectedContents := range tt.want.nextContents {
840-
assert.Contains(t, nextEffectiveConfig.ConfigMap.GetConfigMap(), colNameNamespace)
858+
configFileMap := nextEffectiveConfig.ConfigMap.GetConfigMap()
859+
require.Contains(t, configFileMap, colNameNamespace)
860+
configFileString := string(configFileMap[colNameNamespace].GetBody())
841861
for _, content := range expectedContents {
842-
asString := string(nextEffectiveConfig.ConfigMap.GetConfigMap()[colNameNamespace].GetBody())
843-
assert.Contains(t, asString, content)
862+
assert.Contains(t, configFileString, content)
844863
}
845864
}
846865
assert.Equal(t, tt.want.nextStatus, mockClient.lastStatus)

cmd/operator-opamp-bridge/agent/testdata/basic.yaml

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1+
apiVersion: opentelemetry.io/v1beta1
12
kind: OpenTelemetryCollector
23
metadata:
34
name: simplest
45
labels:
56
"opentelemetry.io/opamp-managed": "true"
67
spec:
7-
config: |
8+
config:
89
receivers:
910
otlp:
1011
protocols:

cmd/operator-opamp-bridge/agent/testdata/invalid.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ metadata:
44
labels:
55
"opentelemetry.io/opamp-managed": "true"
66
spec:
7-
config: |
7+
config:
88
receivers:
99
otlp:
1010
protocols:

cmd/operator-opamp-bridge/agent/testdata/updated.yaml

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1+
apiVersion: opentelemetry.io/v1beta1
12
kind: OpenTelemetryCollector
23
metadata:
34
name: simplest
45
labels:
56
"opentelemetry.io/opamp-managed": "test-bridge"
67
spec:
7-
config: |
8+
config:
89
receivers:
910
otlp:
1011
protocols:

cmd/operator-opamp-bridge/config/config.go

+3
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import (
4141
"sigs.k8s.io/controller-runtime/pkg/log/zap"
4242

4343
"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
44+
"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
4445
"github.com/open-telemetry/opentelemetry-operator/cmd/operator-opamp-bridge/logger"
4546
)
4647

@@ -56,7 +57,9 @@ var (
5657

5758
func registerKnownTypes(s *k8sruntime.Scheme) error {
5859
s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.OpenTelemetryCollector{}, &v1alpha1.OpenTelemetryCollectorList{})
60+
s.AddKnownTypes(v1beta1.GroupVersion, &v1beta1.OpenTelemetryCollector{}, &v1beta1.OpenTelemetryCollectorList{})
5961
metav1.AddToGroupVersion(s, v1alpha1.GroupVersion)
62+
metav1.AddToGroupVersion(s, v1beta1.GroupVersion)
6063
return nil
6164
}
6265

0 commit comments

Comments
 (0)