diff --git a/.chloggen/update-opamp-bridge-spec.yaml b/.chloggen/update-opamp-bridge-spec.yaml new file mode 100755 index 0000000000..d5420243bf --- /dev/null +++ b/.chloggen/update-opamp-bridge-spec.yaml @@ -0,0 +1,18 @@ +# 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. operator, target allocator, github action) +component: OpAMP Bridge + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Currently, the bridge doesn't adhere to the spec for the naming structure. This changes the bridge to use the / structure as described. + +# One or more tracking issues related to the change +issues: [2131] + +# (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: | + * Updates the bridge to get collectors using the reporting annotation + * Fixes a bug where we were using the incorrect structure for the collectors diff --git a/cmd/operator-opamp-bridge/agent/agent.go b/cmd/operator-opamp-bridge/agent/agent.go index 1923810a35..90bbf13f25 100644 --- a/cmd/operator-opamp-bridge/agent/agent.go +++ b/cmd/operator-opamp-bridge/agent/agent.go @@ -166,7 +166,7 @@ func (agent *Agent) getEffectiveConfig(ctx context.Context) (*protobufs.Effectiv agent.logger.Error(err, "failed to marhsal config") return nil, err } - mapKey := newCollectorKey(instance.GetName(), instance.GetNamespace()) + mapKey := newCollectorKey(instance.GetNamespace(), instance.GetName()) instanceMap[mapKey.String()] = &protobufs.AgentConfigFile{ Body: marshaled, ContentType: "yaml", diff --git a/cmd/operator-opamp-bridge/agent/agent_test.go b/cmd/operator-opamp-bridge/agent/agent_test.go index 9c8ca4a702..42d0d1cb6d 100644 --- a/cmd/operator-opamp-bridge/agent/agent_test.go +++ b/cmd/operator-opamp-bridge/agent/agent_test.go @@ -40,27 +40,61 @@ import ( "github.com/open-telemetry/opentelemetry-operator/cmd/operator-opamp-bridge/operator" ) +const ( + collectorBasicFile = "testdata/basic.yaml" + collectorUpdatedFile = "testdata/updated.yaml" + collectorInvalidFile = "testdata/invalid.yaml" + + testNamespace = "testnamespace" + testCollectorName = "collector" + otherCollectorName = "other" + emptyConfigHash = "" + testCollectorKey = testNamespace + "/" + testCollectorName + otherCollectorKey = testNamespace + "/" + otherCollectorName + + agentTestFileName = "testdata/agent.yaml" + agentTestFileHttpName = "testdata/agenthttpbasic.yaml" + agentTestFileBasicComponentsAllowedName = "testdata/agentbasiccomponentsallowed.yaml" + agentTestFileBatchNotAllowedName = "testdata/agentbatchnotallowed.yaml" + agentTestFileNoProcessorsAllowedName = "testdata/agentnoprocessorsallowed.yaml" +) + var ( l = logr.Discard() _ client.OpAMPClient = &mockOpampClient{} + + basicYamlConfigHash = getConfigHash(testCollectorKey, collectorBasicFile) + invalidYamlConfigHash = getConfigHash(testCollectorKey, collectorInvalidFile) + updatedYamlConfigHash = getConfigHash(testCollectorKey, collectorUpdatedFile) + otherUpdatedYamlConfigHash = getConfigHash(otherCollectorKey, collectorUpdatedFile) ) +func getConfigHash(key, file string) string { + fi, err := os.Stat(file) + if err != nil { + return "" + } + // get the size + size := fi.Size() + return fmt.Sprintf("%s%d", key, size) +} + type mockOpampClient struct { lastStatus *protobufs.RemoteConfigStatus lastEffectiveConfig *protobufs.EffectiveConfig settings types.StartSettings } -func (m *mockOpampClient) Start(ctx context.Context, settings types.StartSettings) error { +func (m *mockOpampClient) Start(_ context.Context, settings types.StartSettings) error { m.settings = settings return nil } -func (m *mockOpampClient) Stop(ctx context.Context) error { +func (m *mockOpampClient) Stop(_ context.Context) error { return nil } -func (m *mockOpampClient) SetAgentDescription(descr *protobufs.AgentDescription) error { +func (m *mockOpampClient) SetAgentDescription(_ *protobufs.AgentDescription) error { return nil } @@ -68,7 +102,7 @@ func (m *mockOpampClient) AgentDescription() *protobufs.AgentDescription { return nil } -func (m *mockOpampClient) SetHealth(health *protobufs.AgentHealth) error { +func (m *mockOpampClient) SetHealth(_ *protobufs.AgentHealth) error { return nil } @@ -86,7 +120,7 @@ func (m *mockOpampClient) SetRemoteConfigStatus(status *protobufs.RemoteConfigSt return nil } -func (m *mockOpampClient) SetPackageStatuses(statuses *protobufs.PackageStatuses) error { +func (m *mockOpampClient) SetPackageStatuses(_ *protobufs.PackageStatuses) error { return nil } @@ -109,15 +143,15 @@ func TestAgent_onMessage(t *testing.T) { } type args struct { ctx context.Context - // Mapping from name/namespace to a config in testdata + // Mapping from namespace/name to a config in testdata configFile map[string]string - // Mapping from name/namespace to a config in testdata (for testing updates) + // Mapping from namespace/name to a config in testdata (for testing updates) nextConfigFile map[string]string } type want struct { - // Mapping from name/namespace to a list of expected contents + // Mapping from namespace/name to a list of expected contents contents map[string][]string - // Mapping from name/namespace to a list of updated expected contents + // Mapping from namespace/name to a list of updated expected contents nextContents map[string][]string // The status after the initial config loading status *protobufs.RemoteConfigStatus @@ -133,7 +167,7 @@ func TestAgent_onMessage(t *testing.T) { { name: "no data", fields: fields{ - configFile: "testdata/agent.yaml", + configFile: agentTestFileName, }, args: args{ ctx: context.Background(), @@ -147,27 +181,27 @@ func TestAgent_onMessage(t *testing.T) { { name: "base case", fields: fields{ - configFile: "testdata/agent.yaml", + configFile: agentTestFileName, }, args: args{ ctx: context.Background(), configFile: map[string]string{ - "good/testnamespace": "basic.yaml", + testCollectorKey: collectorBasicFile, }, }, want: want{ contents: map[string][]string{ - "good/testnamespace": { + testCollectorKey: { "kind: OpenTelemetryCollector", - "name: good", - "namespace: testnamespace", + "name: " + testCollectorName, + "namespace: " + testNamespace, "send_batch_size: 10000", "receivers: [otlp]", "status:", }, }, status: &protobufs.RemoteConfigStatus{ - LastRemoteConfigHash: []byte("good/testnamespace401"), + LastRemoteConfigHash: []byte(basicYamlConfigHash), Status: protobufs.RemoteConfigStatuses_RemoteConfigStatuses_APPLIED, }, }, @@ -175,27 +209,27 @@ func TestAgent_onMessage(t *testing.T) { { name: "base case http", fields: fields{ - configFile: "testdata/agenthttpbasic.yaml", + configFile: agentTestFileHttpName, }, args: args{ ctx: context.Background(), configFile: map[string]string{ - "good/testnamespace": "basic.yaml", + testCollectorKey: collectorBasicFile, }, }, want: want{ contents: map[string][]string{ - "good/testnamespace": { + testCollectorKey: { "kind: OpenTelemetryCollector", - "name: good", - "namespace: testnamespace", + "name: " + testCollectorName, + "namespace: " + testNamespace, "send_batch_size: 10000", "receivers: [otlp]", "status:", }, }, status: &protobufs.RemoteConfigStatus{ - LastRemoteConfigHash: []byte("good/testnamespace401"), + LastRemoteConfigHash: []byte(basicYamlConfigHash), Status: protobufs.RemoteConfigStatuses_RemoteConfigStatuses_APPLIED, }, }, @@ -203,47 +237,47 @@ func TestAgent_onMessage(t *testing.T) { { name: "failure", fields: fields{ - configFile: "testdata/agent.yaml", + configFile: agentTestFileName, }, args: args{ ctx: context.Background(), configFile: map[string]string{ - "bad/testnamespace": "invalid.yaml", + testCollectorKey: collectorInvalidFile, }, }, want: want{ contents: nil, status: &protobufs.RemoteConfigStatus{ - LastRemoteConfigHash: []byte("bad/testnamespace404"), + LastRemoteConfigHash: []byte(invalidYamlConfigHash), Status: protobufs.RemoteConfigStatuses_RemoteConfigStatuses_FAILED, - ErrorMessage: "yaml: line 16: could not find expected ':'", + ErrorMessage: "error converting YAML to JSON: yaml: line 21: could not find expected ':'", }, }, }, { name: "all components are allowed", fields: fields{ - configFile: "testdata/agentbasiccomponentsallowed.yaml", + configFile: agentTestFileBasicComponentsAllowedName, }, args: args{ ctx: context.Background(), configFile: map[string]string{ - "good/testnamespace": "basic.yaml", + testCollectorKey: collectorBasicFile, }, }, want: want{ contents: map[string][]string{ - "good/testnamespace": { + testCollectorKey: { "kind: OpenTelemetryCollector", - "name: good", - "namespace: testnamespace", + "name: " + testCollectorName, + "namespace: " + testNamespace, "send_batch_size: 10000", "receivers: [otlp]", "status:", }, }, status: &protobufs.RemoteConfigStatus{ - LastRemoteConfigHash: []byte("good/testnamespace401"), + LastRemoteConfigHash: []byte(basicYamlConfigHash), Status: protobufs.RemoteConfigStatuses_RemoteConfigStatuses_APPLIED, }, }, @@ -251,18 +285,18 @@ func TestAgent_onMessage(t *testing.T) { { name: "batch not allowed", fields: fields{ - configFile: "testdata/agentbatchnotallowed.yaml", + configFile: agentTestFileBatchNotAllowedName, }, args: args{ ctx: context.Background(), configFile: map[string]string{ - "good/testnamespace": "basic.yaml", + testCollectorKey: collectorBasicFile, }, }, want: want{ contents: nil, status: &protobufs.RemoteConfigStatus{ - LastRemoteConfigHash: []byte("good/testnamespace401"), + LastRemoteConfigHash: []byte(basicYamlConfigHash), Status: protobufs.RemoteConfigStatuses_RemoteConfigStatuses_FAILED, ErrorMessage: "Items in config are not allowed: [processors.batch]", }, @@ -271,18 +305,18 @@ func TestAgent_onMessage(t *testing.T) { { name: "processors not allowed", fields: fields{ - configFile: "testdata/agentnoprocessorsallowed.yaml", + configFile: agentTestFileNoProcessorsAllowedName, }, args: args{ ctx: context.Background(), configFile: map[string]string{ - "good/testnamespace": "basic.yaml", + testCollectorKey: collectorBasicFile, }, }, want: want{ contents: nil, status: &protobufs.RemoteConfigStatus{ - LastRemoteConfigHash: []byte("good/testnamespace401"), + LastRemoteConfigHash: []byte(basicYamlConfigHash), Status: protobufs.RemoteConfigStatuses_RemoteConfigStatuses_FAILED, ErrorMessage: "Items in config are not allowed: [processors]", }, @@ -291,23 +325,23 @@ func TestAgent_onMessage(t *testing.T) { { name: "can update config and replicas", fields: fields{ - configFile: "testdata/agent.yaml", + configFile: agentTestFileName, }, args: args{ ctx: context.Background(), configFile: map[string]string{ - "good/testnamespace": "basic.yaml", + testCollectorKey: collectorBasicFile, }, nextConfigFile: map[string]string{ - "good/testnamespace": "updated.yaml", + testCollectorKey: collectorUpdatedFile, }, }, want: want{ contents: map[string][]string{ - "good/testnamespace": { + testCollectorKey: { "kind: OpenTelemetryCollector", - "name: good", - "namespace: testnamespace", + "name: " + testCollectorName, + "namespace: " + testNamespace, "send_batch_size: 10000", "processors: []", "replicas: 1", @@ -315,14 +349,14 @@ func TestAgent_onMessage(t *testing.T) { }, }, status: &protobufs.RemoteConfigStatus{ - LastRemoteConfigHash: []byte("good/testnamespace401"), + LastRemoteConfigHash: []byte(basicYamlConfigHash), Status: protobufs.RemoteConfigStatuses_RemoteConfigStatuses_APPLIED, }, nextContents: map[string][]string{ - "good/testnamespace": { + testCollectorKey: { "kind: OpenTelemetryCollector", - "name: good", - "namespace: testnamespace", + "name: " + testCollectorName, + "namespace: " + testNamespace, "send_batch_size: 10000", "processors: [memory_limiter, batch]", "replicas: 3", @@ -330,7 +364,7 @@ func TestAgent_onMessage(t *testing.T) { }, }, nextStatus: &protobufs.RemoteConfigStatus{ - LastRemoteConfigHash: []byte("good/testnamespace435"), + LastRemoteConfigHash: []byte(updatedYamlConfigHash), Status: protobufs.RemoteConfigStatuses_RemoteConfigStatuses_APPLIED, }, }, @@ -338,23 +372,23 @@ func TestAgent_onMessage(t *testing.T) { { name: "cannot update with bad config", fields: fields{ - configFile: "testdata/agent.yaml", + configFile: agentTestFileName, }, args: args{ ctx: context.Background(), configFile: map[string]string{ - "good/testnamespace": "basic.yaml", + testCollectorKey: collectorBasicFile, }, nextConfigFile: map[string]string{ - "good/testnamespace": "invalid.yaml", + testCollectorKey: collectorInvalidFile, }, }, want: want{ contents: map[string][]string{ - "good/testnamespace": { + testCollectorKey: { "kind: OpenTelemetryCollector", - "name: good", - "namespace: testnamespace", + "name: " + testCollectorName, + "namespace: " + testNamespace, "send_batch_size: 10000", "processors: []", "replicas: 1", @@ -362,14 +396,14 @@ func TestAgent_onMessage(t *testing.T) { }, }, status: &protobufs.RemoteConfigStatus{ - LastRemoteConfigHash: []byte("good/testnamespace401"), + LastRemoteConfigHash: []byte(basicYamlConfigHash), Status: protobufs.RemoteConfigStatuses_RemoteConfigStatuses_APPLIED, }, nextContents: map[string][]string{ - "good/testnamespace": { + testCollectorKey: { "kind: OpenTelemetryCollector", - "name: good", - "namespace: testnamespace", + "name: " + testCollectorName, + "namespace: " + testNamespace, "send_batch_size: 10000", "processors: []", "replicas: 1", @@ -377,62 +411,62 @@ func TestAgent_onMessage(t *testing.T) { }, }, nextStatus: &protobufs.RemoteConfigStatus{ - LastRemoteConfigHash: []byte("good/testnamespace404"), // The new hash should be of the bad config + LastRemoteConfigHash: []byte(invalidYamlConfigHash), // The new hash should be of the bad config Status: protobufs.RemoteConfigStatuses_RemoteConfigStatuses_FAILED, - ErrorMessage: "yaml: line 16: could not find expected ':'", + ErrorMessage: "error converting YAML to JSON: yaml: line 21: could not find expected ':'", }, }, }, { name: "update with new collector", fields: fields{ - configFile: "testdata/agent.yaml", + configFile: agentTestFileName, }, args: args{ ctx: context.Background(), configFile: map[string]string{ - "good/testnamespace": "basic.yaml", + testCollectorKey: collectorBasicFile, }, nextConfigFile: map[string]string{ - "good/testnamespace": "basic.yaml", - "other/testnamespace": "updated.yaml", + testCollectorKey: collectorBasicFile, + otherCollectorKey: collectorUpdatedFile, }, }, want: want{ contents: map[string][]string{ - "good/testnamespace": { + testCollectorKey: { "kind: OpenTelemetryCollector", - "name: good", - "namespace: testnamespace", + "name: " + testCollectorName, + "namespace: " + testNamespace, "send_batch_size: 10000", "processors: []", "status:", }, }, status: &protobufs.RemoteConfigStatus{ - LastRemoteConfigHash: []byte("good/testnamespace401"), + LastRemoteConfigHash: []byte(basicYamlConfigHash), Status: protobufs.RemoteConfigStatuses_RemoteConfigStatuses_APPLIED, }, nextContents: map[string][]string{ - "good/testnamespace": { + testCollectorKey: { "kind: OpenTelemetryCollector", - "name: good", - "namespace: testnamespace", + "name: " + testCollectorName, + "namespace: " + testNamespace, "send_batch_size: 10000", "processors: []", "status:", }, - "other/testnamespace": { + otherCollectorKey: { "kind: OpenTelemetryCollector", - "name: other", - "namespace: testnamespace", + "name: " + otherCollectorName, + "namespace: " + testNamespace, "send_batch_size: 10000", "processors: [memory_limiter, batch]", "status:", }, }, nextStatus: &protobufs.RemoteConfigStatus{ - LastRemoteConfigHash: []byte("good/testnamespace401other/testnamespace435"), + LastRemoteConfigHash: []byte(basicYamlConfigHash + otherUpdatedYamlConfigHash), Status: protobufs.RemoteConfigStatuses_RemoteConfigStatuses_APPLIED, }, }, @@ -440,33 +474,33 @@ func TestAgent_onMessage(t *testing.T) { { name: "can delete existing collector", fields: fields{ - configFile: "testdata/agent.yaml", + configFile: agentTestFileName, }, args: args{ ctx: context.Background(), configFile: map[string]string{ - "good/testnamespace": "basic.yaml", + testCollectorKey: collectorBasicFile, }, nextConfigFile: map[string]string{}, }, want: want{ contents: map[string][]string{ - "good/testnamespace": { + testCollectorKey: { "kind: OpenTelemetryCollector", - "name: good", - "namespace: testnamespace", + "name: " + testCollectorName, + "namespace: " + testNamespace, "send_batch_size: 10000", "processors: []", "status:", }, }, status: &protobufs.RemoteConfigStatus{ - LastRemoteConfigHash: []byte("good/testnamespace401"), + LastRemoteConfigHash: []byte(basicYamlConfigHash), Status: protobufs.RemoteConfigStatuses_RemoteConfigStatuses_APPLIED, }, nextContents: map[string][]string{}, nextStatus: &protobufs.RemoteConfigStatus{ - LastRemoteConfigHash: []byte(""), + LastRemoteConfigHash: []byte(emptyConfigHash), Status: protobufs.RemoteConfigStatuses_RemoteConfigStatuses_APPLIED, }, }, @@ -528,11 +562,11 @@ func Test_CanUpdateIdentity(t *testing.T) { mockClient := &mockOpampClient{} fs := config.GetFlagSet(pflag.ContinueOnError) - configFlag := []string{"--config-file", "testdata/agent.yaml"} + configFlag := []string{"--config-file", agentTestFileName} err := fs.Parse(configFlag) assert.NoError(t, err) conf := config.NewConfig(logr.Discard()) - loadErr := config.LoadFromFile(conf, "testdata/agent.yaml") + loadErr := config.LoadFromFile(conf, agentTestFileName) require.NoError(t, loadErr, "should be able to load config") applier := getFakeApplier(t, conf) agent := NewAgent(l, applier, conf, mockClient) @@ -568,7 +602,7 @@ func getMessageDataFromConfigFile(filemap map[string]string) (*types.MessageData sort.Strings(fileNames) for _, key := range fileNames { - yamlFile, err := os.ReadFile(fmt.Sprintf("testdata/%s", filemap[key])) + yamlFile, err := os.ReadFile(filemap[key]) if err != nil { return toReturn, err } diff --git a/cmd/operator-opamp-bridge/agent/collector_key.go b/cmd/operator-opamp-bridge/agent/collector_key.go index f1353d3472..d2e6c8d4f8 100644 --- a/cmd/operator-opamp-bridge/agent/collector_key.go +++ b/cmd/operator-opamp-bridge/agent/collector_key.go @@ -25,7 +25,7 @@ type collectorKey struct { namespace string } -func newCollectorKey(name string, namespace string) collectorKey { +func newCollectorKey(namespace string, name string) collectorKey { return collectorKey{name: name, namespace: namespace} } @@ -39,5 +39,5 @@ func collectorKeyFromKey(key string) (collectorKey, error) { } func (k collectorKey) String() string { - return fmt.Sprintf("%s/%s", k.name, k.namespace) + return fmt.Sprintf("%s/%s", k.namespace, k.name) } diff --git a/cmd/operator-opamp-bridge/agent/collector_key_test.go b/cmd/operator-opamp-bridge/agent/collector_key_test.go index b0b2546485..7a27180f69 100644 --- a/cmd/operator-opamp-bridge/agent/collector_key_test.go +++ b/cmd/operator-opamp-bridge/agent/collector_key_test.go @@ -34,7 +34,7 @@ func Test_collectorKeyFromKey(t *testing.T) { { name: "base case", args: args{ - key: "good/namespace", + key: "namespace/good", }, want: collectorKey{ name: "good", @@ -86,12 +86,12 @@ func Test_collectorKey_String(t *testing.T) { name: "good", namespace: "namespace", }, - want: "good/namespace", + want: "namespace/good", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - k := newCollectorKey(tt.fields.name, tt.fields.namespace) + k := newCollectorKey(tt.fields.namespace, tt.fields.name) assert.Equalf(t, tt.want, k.String(), "String()") }) } diff --git a/cmd/operator-opamp-bridge/agent/testdata/basic.yaml b/cmd/operator-opamp-bridge/agent/testdata/basic.yaml index 264bd4af99..acefddb7ea 100644 --- a/cmd/operator-opamp-bridge/agent/testdata/basic.yaml +++ b/cmd/operator-opamp-bridge/agent/testdata/basic.yaml @@ -1,24 +1,28 @@ -config: | - receivers: - otlp: - protocols: - grpc: - http: - processors: - memory_limiter: - check_interval: 1s - limit_percentage: 75 - spike_limit_percentage: 15 - batch: - send_batch_size: 10000 - timeout: 10s - - exporters: - debug: - - service: - pipelines: - traces: - receivers: [otlp] - processors: [] - exporters: [debug] \ No newline at end of file +kind: OpenTelemetryCollector +metadata: + name: simplest +spec: + config: | + receivers: + otlp: + protocols: + grpc: + http: + processors: + memory_limiter: + check_interval: 1s + limit_percentage: 75 + spike_limit_percentage: 15 + batch: + send_batch_size: 10000 + timeout: 10s + + exporters: + debug: + + service: + pipelines: + traces: + receivers: [otlp] + processors: [] + exporters: [debug] diff --git a/cmd/operator-opamp-bridge/agent/testdata/invalid.yaml b/cmd/operator-opamp-bridge/agent/testdata/invalid.yaml index bfd977915e..a54813263a 100644 --- a/cmd/operator-opamp-bridge/agent/testdata/invalid.yaml +++ b/cmd/operator-opamp-bridge/agent/testdata/invalid.yaml @@ -1,24 +1,28 @@ -config: | - receivers: - otlp: - protocols: - grpc: - http: - processors: - memory_limiter: - check_interval: 1s - limit_percentage: 75 - spike_limit_percentage: 15 - batch: - send_batch_size: 10000 - timeout: 10s +kind: OpenTelemetryCollector +metadata: + name: simplest +spec: + config: | + receivers: + otlp: + protocols: + grpc: + http: + processors: + memory_limiter: + check_interval: 1s + limit_percentage: 75 + spike_limit_percentage: 15 + batch: + send_batch_size: 10000 + timeout: 10s -GARBAGE - exporters: - debug: - service: - pipelines: - traces: - receivers: [otlp] - processors: [] - exporters: [debug] \ No newline at end of file + GARBAGE + exporters: + debug: + service: + pipelines: + traces: + receivers: [otlp] + processors: [] + exporters: [debug] diff --git a/cmd/operator-opamp-bridge/agent/testdata/updated.yaml b/cmd/operator-opamp-bridge/agent/testdata/updated.yaml index 8ac7f16cbc..1fdfd6aa67 100644 --- a/cmd/operator-opamp-bridge/agent/testdata/updated.yaml +++ b/cmd/operator-opamp-bridge/agent/testdata/updated.yaml @@ -1,25 +1,29 @@ -config: | - receivers: - otlp: - protocols: - grpc: - http: - processors: - memory_limiter: - check_interval: 1s - limit_percentage: 75 - spike_limit_percentage: 15 - batch: - send_batch_size: 10000 - timeout: 10s - - exporters: - debug: - - service: - pipelines: - traces: - receivers: [otlp] - processors: [memory_limiter, batch] - exporters: [debug] -replicas: 3 +kind: OpenTelemetryCollector +metadata: + name: simplest +spec: + config: | + receivers: + otlp: + protocols: + grpc: + http: + processors: + memory_limiter: + check_interval: 1s + limit_percentage: 75 + spike_limit_percentage: 15 + batch: + send_batch_size: 10000 + timeout: 10s + + exporters: + debug: + + service: + pipelines: + traces: + receivers: [otlp] + processors: [memory_limiter, batch] + exporters: [debug] + replicas: 3 diff --git a/cmd/operator-opamp-bridge/operator/client.go b/cmd/operator-opamp-bridge/operator/client.go index dd5bec74b2..bf58a618fc 100644 --- a/cmd/operator-opamp-bridge/operator/client.go +++ b/cmd/operator-opamp-bridge/operator/client.go @@ -17,12 +17,13 @@ package operator import ( "context" "fmt" + "strings" "github.com/go-logr/logr" "github.com/open-telemetry/opamp-go/protobufs" - "gopkg.in/yaml.v3" "k8s.io/apimachinery/pkg/api/errors" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/yaml" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" ) @@ -31,6 +32,7 @@ const ( CollectorResource = "OpenTelemetryCollector" ResourceIdentifierKey = "created-by" ResourceIdentifierValue = "operator-opamp-bridge" + ReportingAnnotationKey = "opentelemetry.io/opamp-reporting" ) type ConfigApplier interface { @@ -104,31 +106,34 @@ func (c Client) update(ctx context.Context, old *v1alpha1.OpenTelemetryCollector func (c Client) Apply(name string, namespace string, configmap *protobufs.AgentConfigFile) error { c.log.Info("Received new config", "name", name, "namespace", namespace) - var collectorSpec v1alpha1.OpenTelemetryCollectorSpec - err := yaml.Unmarshal(configmap.Body, &collectorSpec) + var collector v1alpha1.OpenTelemetryCollector + err := yaml.Unmarshal(configmap.Body, &collector) if err != nil { return err } - if len(collectorSpec.Config) == 0 { + if len(collector.Spec.Config) == 0 { return errors.NewBadRequest("Must supply valid configuration") } - reasons, validateErr := c.validate(collectorSpec) + reasons, validateErr := c.validate(collector.Spec) if validateErr != nil { return validateErr } if len(reasons) > 0 { return errors.NewBadRequest(fmt.Sprintf("Items in config are not allowed: %v", reasons)) } - collector := &v1alpha1.OpenTelemetryCollector{Spec: collectorSpec} + updatedCollector := collector.DeepCopy() ctx := context.Background() instance, err := c.GetInstance(name, namespace) if err != nil { return err } - if instance != nil { - return c.update(ctx, instance, collector) + if instance == nil { + return c.create(ctx, name, namespace, updatedCollector) } - return c.create(ctx, name, namespace, collector) + if labels := instance.GetLabels(); labels != nil && strings.EqualFold(labels[ReportingAnnotationKey], "true") { + return errors.NewBadRequest("cannot modify a collector with `opentelemetry.io/opamp-reporting: true`") + } + return c.update(ctx, instance, updatedCollector) } func (c Client) Delete(name string, namespace string) error { @@ -156,7 +161,19 @@ func (c Client) ListInstances() ([]v1alpha1.OpenTelemetryCollector, error) { if err != nil { return nil, err } - return result.Items, nil + reportingCollectors := v1alpha1.OpenTelemetryCollectorList{} + err = c.k8sClient.List(ctx, &reportingCollectors, client.MatchingLabels{ + ReportingAnnotationKey: "true", + }) + if err != nil { + return nil, err + } + items := append(result.Items, reportingCollectors.Items...) + for i := range items { + items[i].SetManagedFields(nil) + } + + return items, nil } func (c Client) GetInstance(name string, namespace string) (*v1alpha1.OpenTelemetryCollector, error) { diff --git a/cmd/operator-opamp-bridge/operator/client_test.go b/cmd/operator-opamp-bridge/operator/client_test.go index 9d4284b90c..13cccca761 100644 --- a/cmd/operator-opamp-bridge/operator/client_test.go +++ b/cmd/operator-opamp-bridge/operator/client_test.go @@ -15,18 +15,19 @@ package operator import ( + "context" "os" "testing" - "github.com/stretchr/testify/require" - "github.com/open-telemetry/opamp-go/protobufs" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/yaml" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" ) @@ -87,6 +88,15 @@ func TestClient_Apply(t *testing.T) { }, wantErr: true, }, + { + name: "create reporting-only", + args: args{ + name: "test", + namespace: "opentelemetry", + file: "testdata/reporting-collector.yaml", + }, + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -104,8 +114,9 @@ func TestClient_Apply(t *testing.T) { Body: colConfig, ContentType: "yaml", } - if err := c.Apply(tt.args.name, tt.args.namespace, configmap); (err != nil) != tt.wantErr { - t.Errorf("Apply() error = %v, wantErr %v", err, tt.wantErr) + applyErr := c.Apply(tt.args.name, tt.args.namespace, configmap) + if (applyErr != nil) != tt.wantErr { + t.Errorf("Apply() error = %v, wantErr %v", applyErr, tt.wantErr) } }) } @@ -116,6 +127,24 @@ func Test_collectorUpdate(t *testing.T) { namespace := "testing" fakeClient := getFakeClient(t) c := NewClient(clientLogger, fakeClient, nil) + + // Load reporting-only collector + reportingColConfig, err := loadConfig("testdata/reporting-collector.yaml") + require.NoError(t, err, "Should be no error on loading test configuration") + var reportingCol v1alpha1.OpenTelemetryCollector + err = yaml.Unmarshal(reportingColConfig, &reportingCol) + require.NoError(t, err, "Should be no error on unmarshal") + reportingCol.Default() + reportingCol.TypeMeta.Kind = CollectorResource + reportingCol.TypeMeta.APIVersion = v1alpha1.GroupVersion.String() + reportingCol.ObjectMeta.Name = "simplest" + reportingCol.ObjectMeta.Namespace = namespace + err = fakeClient.Create(context.Background(), &reportingCol) + require.NoError(t, err, "Should be able to make reporting col") + allInstances, err := c.ListInstances() + require.NoError(t, err, "Should be able to list all collectors") + require.Len(t, allInstances, 1) + colConfig, err := loadConfig("testdata/collector.yaml") require.NoError(t, err, "Should be no error on loading test configuration") configmap := &protobufs.AgentConfigFile{ @@ -151,10 +180,11 @@ func Test_collectorUpdate(t *testing.T) { require.NoError(t, err, "Should be able to get the updated instance") assert.Contains(t, updatedInstance.Spec.Config, "processors: [memory_limiter, batch]") - allInstances, err := c.ListInstances() + allInstances, err = c.ListInstances() require.NoError(t, err, "Should be able to list all collectors") - assert.Len(t, allInstances, 1) - assert.Equal(t, allInstances[0], *updatedInstance) + assert.Len(t, allInstances, 2) + assert.Contains(t, allInstances, reportingCol) + assert.Contains(t, allInstances, *updatedInstance) } func Test_collectorDelete(t *testing.T) { diff --git a/cmd/operator-opamp-bridge/operator/testdata/collector.yaml b/cmd/operator-opamp-bridge/operator/testdata/collector.yaml index 264bd4af99..acefddb7ea 100644 --- a/cmd/operator-opamp-bridge/operator/testdata/collector.yaml +++ b/cmd/operator-opamp-bridge/operator/testdata/collector.yaml @@ -1,24 +1,28 @@ -config: | - receivers: - otlp: - protocols: - grpc: - http: - processors: - memory_limiter: - check_interval: 1s - limit_percentage: 75 - spike_limit_percentage: 15 - batch: - send_batch_size: 10000 - timeout: 10s - - exporters: - debug: - - service: - pipelines: - traces: - receivers: [otlp] - processors: [] - exporters: [debug] \ No newline at end of file +kind: OpenTelemetryCollector +metadata: + name: simplest +spec: + config: | + receivers: + otlp: + protocols: + grpc: + http: + processors: + memory_limiter: + check_interval: 1s + limit_percentage: 75 + spike_limit_percentage: 15 + batch: + send_batch_size: 10000 + timeout: 10s + + exporters: + debug: + + service: + pipelines: + traces: + receivers: [otlp] + processors: [] + exporters: [debug] diff --git a/cmd/operator-opamp-bridge/operator/testdata/invalid-collector.yaml b/cmd/operator-opamp-bridge/operator/testdata/invalid-collector.yaml index 54015f8149..f64ebcedf4 100644 --- a/cmd/operator-opamp-bridge/operator/testdata/invalid-collector.yaml +++ b/cmd/operator-opamp-bridge/operator/testdata/invalid-collector.yaml @@ -1,24 +1,28 @@ -config: | - receivers: - otlp: - protocols: - grpc: - http: - processors: - memory_limiter: - check_interval: 1s - limit_percentage: 75 - spike_limit_percentage: 15 - batch: - send_batch_size: 10000 - timeout: 10s -GARBAGE - exporters: - debug: - - service: - pipelines: - traces: - receivers: [otlp] - processors: [] - exporters: [debug] \ No newline at end of file +kind: OpenTelemetryCollector +metadata: + name: simplest +spec: + config: | + receivers: + otlp: + protocols: + grpc: + http: + processors: + memory_limiter: + check_interval: 1s + limit_percentage: 75 + spike_limit_percentage: 15 + batch: + send_batch_size: 10000 + timeout: 10s + GARBAGE + exporters: + debug: + + service: + pipelines: + traces: + receivers: [otlp] + processors: [] + exporters: [debug] diff --git a/cmd/operator-opamp-bridge/operator/testdata/reporting-collector.yaml b/cmd/operator-opamp-bridge/operator/testdata/reporting-collector.yaml new file mode 100644 index 0000000000..3b0a925464 --- /dev/null +++ b/cmd/operator-opamp-bridge/operator/testdata/reporting-collector.yaml @@ -0,0 +1,31 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: simplest + labels: + "opentelemetry.io/opamp-reporting": "true" +spec: + config: | + receivers: + otlp: + protocols: + grpc: + http: + processors: + memory_limiter: + check_interval: 1s + limit_percentage: 75 + spike_limit_percentage: 15 + batch: + send_batch_size: 10000 + timeout: 10s + + exporters: + debug: + + service: + pipelines: + traces: + receivers: [otlp] + processors: [] + exporters: [debug] diff --git a/cmd/operator-opamp-bridge/operator/testdata/updated-collector.yaml b/cmd/operator-opamp-bridge/operator/testdata/updated-collector.yaml index 993dcbb77d..0cf824b5e3 100644 --- a/cmd/operator-opamp-bridge/operator/testdata/updated-collector.yaml +++ b/cmd/operator-opamp-bridge/operator/testdata/updated-collector.yaml @@ -1,24 +1,28 @@ -config: | - receivers: - otlp: - protocols: - grpc: - http: - processors: - memory_limiter: - check_interval: 1s - limit_percentage: 75 - spike_limit_percentage: 15 - batch: - send_batch_size: 10000 - timeout: 10s - - exporters: - debug: - - service: - pipelines: - traces: - receivers: [otlp] - processors: [memory_limiter, batch] - exporters: [debug] \ No newline at end of file +kind: OpenTelemetryCollector +metadata: + name: simplest +spec: + config: | + receivers: + otlp: + protocols: + grpc: + http: + processors: + memory_limiter: + check_interval: 1s + limit_percentage: 75 + spike_limit_percentage: 15 + batch: + send_batch_size: 10000 + timeout: 10s + + exporters: + debug: + + service: + pipelines: + traces: + receivers: [otlp] + processors: [memory_limiter, batch] + exporters: [debug]