Skip to content

Commit

Permalink
Fix otelcol cr deletion failures (open-telemetry#3076)
Browse files Browse the repository at this point in the history
* Fix otelcol cr deletion failures

Set OpenTelemetryCollector CRD `spec.config.service.pipelines.processors` to optional to align with OTel Collector usage recommendations.

* Fix e2e
  • Loading branch information
fyuan1316 authored Jul 2, 2024
1 parent 3a300b1 commit 107d2c3
Show file tree
Hide file tree
Showing 131 changed files with 61 additions and 175 deletions.
18 changes: 18 additions & 0 deletions .chloggen/fix-otelcol-crd.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# 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: collector

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "Fix deletion issue of `otelcol` CR by making `spec.config.service.pipelines.processors` optional"

# One or more tracking issues related to the change
issues: [3075]

# (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 makes `spec.config.service.pipelines.processors` in `OpenTelemetryCollector` CRD optional, aligning with OTel Collector best practices. It resolves deletion issues by providing flexibility in CRD configuration, addressing conflicts between strict validation and practical uses.
Note: Updating the `opentelemetrycollectors.opentelemetry.io` CRD resource is required.
4 changes: 0 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ spec:
pipelines:
traces:
receivers: [jaeger]
processors: []
exporters: [debug]
EOF

Expand Down Expand Up @@ -629,7 +628,6 @@ spec:
pipelines:
metrics:
receivers: [prometheus]
processors: []
exporters: [debug]
EOF
```
Expand All @@ -654,7 +652,6 @@ service:
pipelines:
metrics:
receivers: [prometheus]
processors: []
exporters: [debug]
```

Expand Down Expand Up @@ -713,7 +710,6 @@ spec:
pipelines:
metrics:
receivers: [prometheus]
processors: []
exporters: [debug]
EOF
```
Expand Down
2 changes: 1 addition & 1 deletion apis/v1beta1/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (c *AnyConfig) MarshalJSON() ([]byte, error) {
// Pipeline is a struct of component type to a list of component IDs.
type Pipeline struct {
Exporters []string `json:"exporters" yaml:"exporters"`
Processors []string `json:"processors" yaml:"processors"`
Processors []string `json:"processors,omitempty" yaml:"processors,omitempty"`
Receivers []string `json:"receivers" yaml:"receivers"`
}

Expand Down
2 changes: 0 additions & 2 deletions apis/v1beta1/testdata/otelcol-connectors.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@ service:
pipelines:
traces:
receivers: [foo]
processors: []
exporters: [count]
metrics:
receivers: [count]
processors: []
exporters: [bar]
1 change: 0 additions & 1 deletion apis/v1beta1/testdata/otelcol-extensions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,5 @@ service:
traces:
receivers:
- otlp
processors: []
exporters:
- otlp/auth
1 change: 0 additions & 1 deletion apis/v1beta1/testdata/otelcol-filelog.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -81,5 +81,4 @@ service:
pipelines:
logs:
receivers: [filelog]
processors: []
exporters: [debug]
14 changes: 14 additions & 0 deletions apis/v1beta1/testdata/otelcol-pipelines.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
receivers:
otlp:
protocols:
grpc:
http:
exporters:
debug:
service:
pipelines:
traces:
receivers:
- otlp
exporters:
- debug
Original file line number Diff line number Diff line change
Expand Up @@ -5856,7 +5856,6 @@ spec:
type: array
required:
- exporters
- processors
- receivers
type: object
type: object
Expand Down
6 changes: 0 additions & 6 deletions cmd/operator-opamp-bridge/agent/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,6 @@ func TestAgent_onMessage(t *testing.T) {
"name: " + testCollectorName,
"namespace: " + testNamespace,
"send_batch_size: 10000",
"processors: []",
"status:",
},
},
Expand Down Expand Up @@ -688,7 +687,6 @@ func TestAgent_onMessage(t *testing.T) {
"name: " + testCollectorName,
"namespace: " + testNamespace,
"send_batch_size: 10000",
"processors: []",
"status:",
},
},
Expand All @@ -702,7 +700,6 @@ func TestAgent_onMessage(t *testing.T) {
"name: " + testCollectorName,
"namespace: " + testNamespace,
"send_batch_size: 10000",
"processors: []",
"status:",
},
},
Expand Down Expand Up @@ -735,7 +732,6 @@ func TestAgent_onMessage(t *testing.T) {
"name: " + testCollectorName,
"namespace: " + testNamespace,
"send_batch_size: 10000",
"processors: []",
"status:",
},
},
Expand All @@ -749,7 +745,6 @@ func TestAgent_onMessage(t *testing.T) {
"name: " + testCollectorName,
"namespace: " + testNamespace,
"send_batch_size: 10000",
"processors: []",
"status:",
},
otherCollectorKey: {
Expand Down Expand Up @@ -786,7 +781,6 @@ func TestAgent_onMessage(t *testing.T) {
"name: " + testCollectorName,
"namespace: " + testNamespace,
"send_batch_size: 10000",
"processors: []",
"status:",
},
},
Expand Down
1 change: 0 additions & 1 deletion cmd/operator-opamp-bridge/agent/testdata/basic.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,4 @@ spec:
pipelines:
traces:
receivers: [otlp]
processors: []
exporters: [debug]
1 change: 0 additions & 1 deletion cmd/operator-opamp-bridge/agent/testdata/invalid.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,4 @@ spec:
pipelines:
traces:
receivers: [otlp]
processors: []
exporters: [debug]
6 changes: 2 additions & 4 deletions cmd/operator-opamp-bridge/operator/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,8 @@ func Test_collectorUpdate(t *testing.T) {
require.NoError(t, err, "Should apply base config")

// Get the newly created collector
instance, err := c.GetInstance(name, namespace)
_, err = c.GetInstance(name, namespace)
require.NoError(t, err, "Should be able to get the newly created instance")
assert.Contains(t, instance.Spec.Config, "processors: []")

// Try updating with an invalid one
configmap.Body = []byte("empty, invalid!")
Expand Down Expand Up @@ -224,9 +223,8 @@ func Test_collectorDelete(t *testing.T) {
require.NoError(t, err, "Should apply base config")

// Get the newly created collector
instance, err := c.GetInstance(name, namespace)
_, err = c.GetInstance(name, namespace)
require.NoError(t, err, "Should be able to get the newly created instance")
assert.Contains(t, instance.Spec.Config, "processors: []")

// Delete it
err = c.Delete(name, namespace)
Expand Down
1 change: 0 additions & 1 deletion cmd/operator-opamp-bridge/operator/testdata/collector.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,4 @@ spec:
pipelines:
traces:
receivers: [otlp]
processors: []
exporters: [debug]
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,4 @@ spec:
pipelines:
traces:
receivers: [otlp]
processors: []
exporters: [debug]
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,4 @@ spec:
pipelines:
traces:
receivers: [otlp]
processors: []
exporters: [debug]
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,4 @@ spec:
pipelines:
traces:
receivers: [otlp]
processors: []
exporters: [debug]
1 change: 0 additions & 1 deletion cmd/otel-allocator/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ spec:
pipelines:
metrics:
receivers: [prometheus]
processors: []
exporters: [logging]
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5842,7 +5842,6 @@ spec:
type: array
required:
- exporters
- processors
- receivers
type: object
type: object
Expand Down
30 changes: 15 additions & 15 deletions controllers/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ service:
"app.kubernetes.io/version": "latest",
},
Annotations: map[string]string{
"opentelemetry-operator-config/sha256": "8f1dbf79c3a8d89f9799abcb169581e2de70318e00e10cac1cd71fe234185401",
"opentelemetry-operator-config/sha256": "6f6f11da374b2c1e42fc78fbe55e2d9bcc2f5998ab63a631b49c478e8c0f6af8",
"prometheus.io/path": "/metrics",
"prometheus.io/port": "8888",
"prometheus.io/scrape": "true",
Expand All @@ -156,7 +156,7 @@ service:
"app.kubernetes.io/version": "latest",
},
Annotations: map[string]string{
"opentelemetry-operator-config/sha256": "8f1dbf79c3a8d89f9799abcb169581e2de70318e00e10cac1cd71fe234185401",
"opentelemetry-operator-config/sha256": "6f6f11da374b2c1e42fc78fbe55e2d9bcc2f5998ab63a631b49c478e8c0f6af8",
"prometheus.io/path": "/metrics",
"prometheus.io/port": "8888",
"prometheus.io/scrape": "true",
Expand Down Expand Up @@ -242,7 +242,7 @@ service:
Annotations: nil,
},
Data: map[string]string{
"collector.yaml": "receivers:\n examplereceiver:\n endpoint: 0.0.0.0:12345\nexporters:\n logging: null\nservice:\n pipelines:\n metrics:\n exporters:\n - logging\n processors: []\n receivers:\n - examplereceiver\n",
"collector.yaml": "receivers:\n examplereceiver:\n endpoint: 0.0.0.0:12345\nexporters:\n logging: null\nservice:\n pipelines:\n metrics:\n exporters:\n - logging\n receivers:\n - examplereceiver\n",
},
},
&corev1.ServiceAccount{
Expand Down Expand Up @@ -385,7 +385,7 @@ service:
"app.kubernetes.io/version": "latest",
},
Annotations: map[string]string{
"opentelemetry-operator-config/sha256": "8f1dbf79c3a8d89f9799abcb169581e2de70318e00e10cac1cd71fe234185401",
"opentelemetry-operator-config/sha256": "6f6f11da374b2c1e42fc78fbe55e2d9bcc2f5998ab63a631b49c478e8c0f6af8",
"prometheus.io/path": "/metrics",
"prometheus.io/port": "8888",
"prometheus.io/scrape": "true",
Expand All @@ -407,7 +407,7 @@ service:
"app.kubernetes.io/version": "latest",
},
Annotations: map[string]string{
"opentelemetry-operator-config/sha256": "8f1dbf79c3a8d89f9799abcb169581e2de70318e00e10cac1cd71fe234185401",
"opentelemetry-operator-config/sha256": "6f6f11da374b2c1e42fc78fbe55e2d9bcc2f5998ab63a631b49c478e8c0f6af8",
"prometheus.io/path": "/metrics",
"prometheus.io/port": "8888",
"prometheus.io/scrape": "true",
Expand Down Expand Up @@ -493,7 +493,7 @@ service:
Annotations: nil,
},
Data: map[string]string{
"collector.yaml": "receivers:\n examplereceiver:\n endpoint: 0.0.0.0:12345\nexporters:\n logging: null\nservice:\n pipelines:\n metrics:\n exporters:\n - logging\n processors: []\n receivers:\n - examplereceiver\n",
"collector.yaml": "receivers:\n examplereceiver:\n endpoint: 0.0.0.0:12345\nexporters:\n logging: null\nservice:\n pipelines:\n metrics:\n exporters:\n - logging\n receivers:\n - examplereceiver\n",
},
},
&corev1.ServiceAccount{
Expand Down Expand Up @@ -672,7 +672,7 @@ service:
"app.kubernetes.io/version": "latest",
},
Annotations: map[string]string{
"opentelemetry-operator-config/sha256": "8f1dbf79c3a8d89f9799abcb169581e2de70318e00e10cac1cd71fe234185401",
"opentelemetry-operator-config/sha256": "6f6f11da374b2c1e42fc78fbe55e2d9bcc2f5998ab63a631b49c478e8c0f6af8",
"prometheus.io/path": "/metrics",
"prometheus.io/port": "8888",
"prometheus.io/scrape": "true",
Expand All @@ -694,7 +694,7 @@ service:
"app.kubernetes.io/version": "latest",
},
Annotations: map[string]string{
"opentelemetry-operator-config/sha256": "8f1dbf79c3a8d89f9799abcb169581e2de70318e00e10cac1cd71fe234185401",
"opentelemetry-operator-config/sha256": "6f6f11da374b2c1e42fc78fbe55e2d9bcc2f5998ab63a631b49c478e8c0f6af8",
"prometheus.io/path": "/metrics",
"prometheus.io/port": "8888",
"prometheus.io/scrape": "true",
Expand Down Expand Up @@ -780,7 +780,7 @@ service:
Annotations: nil,
},
Data: map[string]string{
"collector.yaml": "receivers:\n examplereceiver:\n endpoint: 0.0.0.0:12345\nexporters:\n logging: null\nservice:\n pipelines:\n metrics:\n exporters:\n - logging\n processors: []\n receivers:\n - examplereceiver\n",
"collector.yaml": "receivers:\n examplereceiver:\n endpoint: 0.0.0.0:12345\nexporters:\n logging: null\nservice:\n pipelines:\n metrics:\n exporters:\n - logging\n receivers:\n - examplereceiver\n",
},
},
&corev1.Service{
Expand Down Expand Up @@ -1202,7 +1202,7 @@ service:
"app.kubernetes.io/version": "latest",
},
Annotations: map[string]string{
"opentelemetry-operator-config/sha256": "57434ab43283b8d3cd0b8584aad7f05226ec24225f28d93b3969d3d2ccf7d009",
"opentelemetry-operator-config/sha256": "39cae697770f9d7e183e8fa9ba56043315b62e19c7231537870acfaaabc30a43",
"prometheus.io/path": "/metrics",
"prometheus.io/port": "8888",
"prometheus.io/scrape": "true",
Expand All @@ -1225,7 +1225,7 @@ service:
"app.kubernetes.io/version": "latest",
},
Annotations: map[string]string{
"opentelemetry-operator-config/sha256": "57434ab43283b8d3cd0b8584aad7f05226ec24225f28d93b3969d3d2ccf7d009",
"opentelemetry-operator-config/sha256": "39cae697770f9d7e183e8fa9ba56043315b62e19c7231537870acfaaabc30a43",
"prometheus.io/path": "/metrics",
"prometheus.io/port": "8888",
"prometheus.io/scrape": "true",
Expand Down Expand Up @@ -1311,7 +1311,7 @@ service:
Annotations: nil,
},
Data: map[string]string{
"collector.yaml": "exporters:\n logging: null\nreceivers:\n prometheus:\n config: {}\n target_allocator:\n collector_id: ${POD_NAME}\n endpoint: http://test-targetallocator:80\n interval: 30s\nservice:\n pipelines:\n metrics:\n exporters:\n - logging\n processors: []\n receivers:\n - prometheus\n",
"collector.yaml": "exporters:\n logging: null\nreceivers:\n prometheus:\n config: {}\n target_allocator:\n collector_id: ${POD_NAME}\n endpoint: http://test-targetallocator:80\n interval: 30s\nservice:\n pipelines:\n metrics:\n exporters:\n - logging\n receivers:\n - prometheus\n",
},
},
&corev1.ServiceAccount{
Expand Down Expand Up @@ -1598,7 +1598,7 @@ prometheus_cr:
"app.kubernetes.io/version": "latest",
},
Annotations: map[string]string{
"opentelemetry-operator-config/sha256": "57434ab43283b8d3cd0b8584aad7f05226ec24225f28d93b3969d3d2ccf7d009",
"opentelemetry-operator-config/sha256": "39cae697770f9d7e183e8fa9ba56043315b62e19c7231537870acfaaabc30a43",
"prometheus.io/path": "/metrics",
"prometheus.io/port": "8888",
"prometheus.io/scrape": "true",
Expand All @@ -1621,7 +1621,7 @@ prometheus_cr:
"app.kubernetes.io/version": "latest",
},
Annotations: map[string]string{
"opentelemetry-operator-config/sha256": "57434ab43283b8d3cd0b8584aad7f05226ec24225f28d93b3969d3d2ccf7d009",
"opentelemetry-operator-config/sha256": "39cae697770f9d7e183e8fa9ba56043315b62e19c7231537870acfaaabc30a43",
"prometheus.io/path": "/metrics",
"prometheus.io/port": "8888",
"prometheus.io/scrape": "true",
Expand Down Expand Up @@ -1707,7 +1707,7 @@ prometheus_cr:
Annotations: nil,
},
Data: map[string]string{
"collector.yaml": "exporters:\n logging: null\nreceivers:\n prometheus:\n config: {}\n target_allocator:\n collector_id: ${POD_NAME}\n endpoint: http://test-targetallocator:80\n interval: 30s\nservice:\n pipelines:\n metrics:\n exporters:\n - logging\n processors: []\n receivers:\n - prometheus\n",
"collector.yaml": "exporters:\n logging: null\nreceivers:\n prometheus:\n config: {}\n target_allocator:\n collector_id: ${POD_NAME}\n endpoint: http://test-targetallocator:80\n interval: 30s\nservice:\n pipelines:\n metrics:\n exporters:\n - logging\n receivers:\n - prometheus\n",
},
},
&corev1.ServiceAccount{
Expand Down
1 change: 0 additions & 1 deletion controllers/testdata/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,4 @@ service:
pipelines:
metrics:
receivers: [prometheus, jaeger]
processors: []
exporters: [logging]
1 change: 0 additions & 1 deletion controllers/testdata/test_ta_update.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,4 @@ service:
pipelines:
metrics:
receivers: [prometheus, jaeger]
processors: []
exporters: [logging]
Loading

0 comments on commit 107d2c3

Please sign in to comment.