diff --git a/.chloggen/0150-workaround.yaml b/.chloggen/0150-workaround.yaml new file mode 100755 index 0000000000..ca3cb31c7a --- /dev/null +++ b/.chloggen/0150-workaround.yaml @@ -0,0 +1,16 @@ +# 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: Remove workaround for 0.104.0 that enabled feature-gate `confmap.unifyEnvVarExpansion` when Prometheus receiver was enabled. + +# One or more tracking issues related to the change +issues: [3142] + +# (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: diff --git a/apis/v1beta1/collector_webhook.go b/apis/v1beta1/collector_webhook.go index 7ba5c823f2..8374615a88 100644 --- a/apis/v1beta1/collector_webhook.go +++ b/apis/v1beta1/collector_webhook.go @@ -79,7 +79,6 @@ func (c CollectorWebhook) Default(_ context.Context, obj runtime.Object) error { otelcol.Spec.TargetAllocator.Replicas = &one } - TAUnifyEnvVarExpansion(otelcol) ComponentUseLocalHostAsDefaultHost(otelcol) if otelcol.Spec.Autoscaler != nil && otelcol.Spec.Autoscaler.MaxReplicas != nil { @@ -453,37 +452,6 @@ func SetupCollectorWebhook(mgr ctrl.Manager, cfg config.Config, reviewer *rbac.R Complete() } -// TAUnifyEnvVarExpansion disables confmap.unifyEnvVarExpansion featuregate on -// collector instances if a prometheus receiver is configured. -// NOTE: We need this for now until 0.105.0 is out with this fix: -// https://github.com/open-telemetry/opentelemetry-collector/commit/637b1f42fcb7cbb7ef8a50dcf41d0a089623a8b7 -func TAUnifyEnvVarExpansion(otelcol *OpenTelemetryCollector) { - var enable bool - for receiver := range otelcol.Spec.Config.Receivers.Object { - if strings.Contains(receiver, "prometheus") { - enable = true - break - } - } - if !enable { - return - } - - const ( - baseFlag = "feature-gates" - fgFlag = "confmap.unifyEnvVarExpansion" - ) - if otelcol.Spec.Args == nil { - otelcol.Spec.Args = make(map[string]string) - } - args, ok := otelcol.Spec.Args[baseFlag] - if !ok || len(args) == 0 { - otelcol.Spec.Args[baseFlag] = "-" + fgFlag - } else if !strings.Contains(otelcol.Spec.Args[baseFlag], fgFlag) { - otelcol.Spec.Args[baseFlag] += ",-" + fgFlag - } -} - // ComponentUseLocalHostAsDefaultHost enables component.UseLocalHostAsDefaultHost // featuregate on the given collector instance. // NOTE: For more details, visit: diff --git a/apis/v1beta1/collector_webhook_test.go b/apis/v1beta1/collector_webhook_test.go index cf28e9cbb6..bfc5c6e095 100644 --- a/apis/v1beta1/collector_webhook_test.go +++ b/apis/v1beta1/collector_webhook_test.go @@ -1335,40 +1335,3 @@ func getReviewer(shouldFailSAR bool) *rbac.Reviewer { }) return rbac.NewReviewer(c) } - -func TestTAUnifyEnvVarExpansion(t *testing.T) { - otelcol := &OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ - Args: nil, - }, - }, - } - TAUnifyEnvVarExpansion(otelcol) - assert.Nil(t, otelcol.Spec.OpenTelemetryCommonFields.Args, "expect nil") - otelcol.Spec.Config.Receivers.Object = map[string]interface{}{ - "prometheus": nil, - } - TAUnifyEnvVarExpansion(otelcol) - assert.NotNil(t, otelcol.Spec.OpenTelemetryCommonFields.Args, "expect not nil") - expect := map[string]string{ - "feature-gates": "-confmap.unifyEnvVarExpansion", - } - assert.EqualValues(t, otelcol.Spec.OpenTelemetryCommonFields.Args, expect) - TAUnifyEnvVarExpansion(otelcol) - assert.EqualValues(t, otelcol.Spec.OpenTelemetryCommonFields.Args, expect) - expect = map[string]string{ - "feature-gates": "-confmap.unifyEnvVarExpansion,+abc", - } - otelcol.Spec.OpenTelemetryCommonFields.Args = expect - TAUnifyEnvVarExpansion(otelcol) - assert.EqualValues(t, otelcol.Spec.OpenTelemetryCommonFields.Args, expect) - otelcol.Spec.OpenTelemetryCommonFields.Args = map[string]string{ - "feature-gates": "+abc", - } - TAUnifyEnvVarExpansion(otelcol) - expect = map[string]string{ - "feature-gates": "+abc,-confmap.unifyEnvVarExpansion", - } - assert.EqualValues(t, otelcol.Spec.OpenTelemetryCommonFields.Args, expect) -} diff --git a/pkg/collector/upgrade/v0_104_0.go b/pkg/collector/upgrade/v0_104_0.go index eafa9036aa..45fe8a7461 100644 --- a/pkg/collector/upgrade/v0_104_0.go +++ b/pkg/collector/upgrade/v0_104_0.go @@ -16,12 +16,13 @@ package upgrade import ( "fmt" + "strings" "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" ) func upgrade0_104_0_TA(_ VersionUpgrade, otelcol *v1beta1.OpenTelemetryCollector) (*v1beta1.OpenTelemetryCollector, error) { - v1beta1.TAUnifyEnvVarExpansion(otelcol) + TAUnifyEnvVarExpansion(otelcol) return otelcol, nil } @@ -37,3 +38,34 @@ func upgrade0_104_0(u VersionUpgrade, otelcol *v1beta1.OpenTelemetryCollector) ( u.Recorder.Event(otelcol, "Warning", "Upgrade", warnStr) return otelcol, nil } + +// TAUnifyEnvVarExpansion disables confmap.unifyEnvVarExpansion featuregate on +// collector instances if a prometheus receiver is configured. +// NOTE: We need this for now until 0.105.0 is out with this fix: +// https://github.com/open-telemetry/opentelemetry-collector/commit/637b1f42fcb7cbb7ef8a50dcf41d0a089623a8b7 +func TAUnifyEnvVarExpansion(otelcol *v1beta1.OpenTelemetryCollector) { + var enable bool + for receiver := range otelcol.Spec.Config.Receivers.Object { + if strings.Contains(receiver, "prometheus") { + enable = true + break + } + } + if !enable { + return + } + + const ( + baseFlag = "feature-gates" + fgFlag = "confmap.unifyEnvVarExpansion" + ) + if otelcol.Spec.Args == nil { + otelcol.Spec.Args = make(map[string]string) + } + args, ok := otelcol.Spec.Args[baseFlag] + if !ok || len(args) == 0 { + otelcol.Spec.Args[baseFlag] = "-" + fgFlag + } else if !strings.Contains(otelcol.Spec.Args[baseFlag], fgFlag) { + otelcol.Spec.Args[baseFlag] += ",-" + fgFlag + } +} diff --git a/pkg/collector/upgrade/v0_104_0_test.go b/pkg/collector/upgrade/v0_104_0_test.go index 22fbd03000..9f9faf9a74 100644 --- a/pkg/collector/upgrade/v0_104_0_test.go +++ b/pkg/collector/upgrade/v0_104_0_test.go @@ -59,3 +59,40 @@ func Test0_104_0Upgrade(t *testing.T) { map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"}, col.Spec.Args, "missing featuregate") } + +func TestTAUnifyEnvVarExpansion(t *testing.T) { + otelcol := &v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ + Args: nil, + }, + }, + } + upgrade.TAUnifyEnvVarExpansion(otelcol) + assert.Nil(t, otelcol.Spec.OpenTelemetryCommonFields.Args, "expect nil") + otelcol.Spec.Config.Receivers.Object = map[string]interface{}{ + "prometheus": nil, + } + upgrade.TAUnifyEnvVarExpansion(otelcol) + assert.NotNil(t, otelcol.Spec.OpenTelemetryCommonFields.Args, "expect not nil") + expect := map[string]string{ + "feature-gates": "-confmap.unifyEnvVarExpansion", + } + assert.EqualValues(t, otelcol.Spec.OpenTelemetryCommonFields.Args, expect) + upgrade.TAUnifyEnvVarExpansion(otelcol) + assert.EqualValues(t, otelcol.Spec.OpenTelemetryCommonFields.Args, expect) + expect = map[string]string{ + "feature-gates": "-confmap.unifyEnvVarExpansion,+abc", + } + otelcol.Spec.OpenTelemetryCommonFields.Args = expect + upgrade.TAUnifyEnvVarExpansion(otelcol) + assert.EqualValues(t, otelcol.Spec.OpenTelemetryCommonFields.Args, expect) + otelcol.Spec.OpenTelemetryCommonFields.Args = map[string]string{ + "feature-gates": "+abc", + } + upgrade.TAUnifyEnvVarExpansion(otelcol) + expect = map[string]string{ + "feature-gates": "+abc,-confmap.unifyEnvVarExpansion", + } + assert.EqualValues(t, otelcol.Spec.OpenTelemetryCommonFields.Args, expect) +} diff --git a/pkg/collector/upgrade/v0_105_0.go b/pkg/collector/upgrade/v0_105_0.go new file mode 100644 index 0000000000..3f497ce6a7 --- /dev/null +++ b/pkg/collector/upgrade/v0_105_0.go @@ -0,0 +1,65 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package upgrade + +import ( + "slices" + "strings" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" +) + +func upgrade0_105_0(_ VersionUpgrade, otelcol *v1beta1.OpenTelemetryCollector) (*v1beta1.OpenTelemetryCollector, error) { + var enable bool + for receiver := range otelcol.Spec.Config.Receivers.Object { + if strings.Contains(receiver, "prometheus") { + enable = true + break + } + } + if !enable { + return otelcol, nil + } + + envVarExpansionFeatureFlag := "-confmap.unifyEnvVarExpansion" + otelcol.Spec.Args = RemoveFeatureGate(otelcol.Spec.Args, envVarExpansionFeatureFlag) + + return otelcol, nil +} + +const featureGateFlag = "feature-gates" + +// RemoveFeatureGate removes a feature gate from args. +func RemoveFeatureGate(args map[string]string, feature string) map[string]string { + featureGates, ok := args[featureGateFlag] + if !ok { + return args + } + if !strings.Contains(featureGates, feature) { + return args + } + + features := strings.Split(featureGates, ",") + features = slices.DeleteFunc(features, func(s string) bool { + return s == feature + }) + if len(features) == 0 { + delete(args, featureGateFlag) + } else { + featureGates = strings.Join(features, ",") + args[featureGateFlag] = featureGates + } + return args +} diff --git a/pkg/collector/upgrade/v0_105_0_test.go b/pkg/collector/upgrade/v0_105_0_test.go new file mode 100644 index 0000000000..c92880790d --- /dev/null +++ b/pkg/collector/upgrade/v0_105_0_test.go @@ -0,0 +1,125 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package upgrade_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" + "github.com/open-telemetry/opentelemetry-operator/internal/version" + "github.com/open-telemetry/opentelemetry-operator/pkg/collector/upgrade" +) + +func Test0_105_0Upgrade(t *testing.T) { + collectorInstance := v1beta1.OpenTelemetryCollector{ + TypeMeta: metav1.TypeMeta{ + Kind: "OpenTelemetryCollector", + APIVersion: "v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "otel-my-instance", + Namespace: "somewhere", + }, + Status: v1beta1.OpenTelemetryCollectorStatus{ + Version: "0.104.0", + }, + Spec: v1beta1.OpenTelemetryCollectorSpec{ + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ + Args: map[string]string{ + "foo": "bar", + "feature-gates": "+baz,-confmap.unifyEnvVarExpansion", + }, + }, + Config: v1beta1.Config{ + Receivers: v1beta1.AnyConfig{ + Object: map[string]interface{}{ + "prometheus": []interface{}{}, + }, + }, + }, + }, + } + + versionUpgrade := &upgrade.VersionUpgrade{ + Log: logger, + Version: version.Get(), + Client: k8sClient, + Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize), + } + + col, err := versionUpgrade.ManagedInstance(context.Background(), collectorInstance) + if err != nil { + t.Errorf("expect err: nil but got: %v", err) + } + assert.EqualValues(t, + map[string]string{"foo": "bar", "feature-gates": "+baz"}, col.Spec.Args) +} + +func TestRemoveFeatureGate(t *testing.T) { + tests := []struct { + test string + args map[string]string + feature string + expected map[string]string + }{ + { + test: "empty", + args: map[string]string{}, + expected: map[string]string{}, + }, + { + test: "no feature gates", + args: map[string]string{"foo": "bar"}, + feature: "foo", + expected: map[string]string{"foo": "bar"}, + }, + { + test: "remove enabled feature gate", + args: map[string]string{"foo": "bar", "feature-gates": "+foo"}, + feature: "-foo", + expected: map[string]string{"foo": "bar", "feature-gates": "+foo"}, + }, + { + test: "remove disabled feature gate", + args: map[string]string{"foo": "bar", "feature-gates": "-foo"}, + feature: "-foo", + expected: map[string]string{"foo": "bar"}, + }, + { + test: "remove disabled feature gate, start", + args: map[string]string{"foo": "bar", "feature-gates": "-foo,bar"}, + feature: "-foo", + expected: map[string]string{"foo": "bar", "feature-gates": "bar"}, + }, + { + test: "remove disabled feature gate, end", + args: map[string]string{"foo": "bar", "feature-gates": "bar,-foo"}, + feature: "-foo", + expected: map[string]string{"foo": "bar", "feature-gates": "bar"}, + }, + } + + for _, test := range tests { + t.Run(test.test, func(t *testing.T) { + args := upgrade.RemoveFeatureGate(test.args, test.feature) + assert.Equal(t, test.expected, args) + }) + } +} diff --git a/pkg/collector/upgrade/versions.go b/pkg/collector/upgrade/versions.go index 7f40a40ea6..a0a06898a0 100644 --- a/pkg/collector/upgrade/versions.go +++ b/pkg/collector/upgrade/versions.go @@ -98,6 +98,10 @@ var ( Version: *semver.MustParse("0.104.0"), upgradeV1beta1: upgrade0_104_0, }, + { + Version: *semver.MustParse("0.105.0"), + upgradeV1beta1: upgrade0_105_0, + }, } // Latest represents the latest version that we need to upgrade. This is not necessarily the latest known version. diff --git a/tests/e2e-targetallocator/targetallocator-features/00-assert.yaml b/tests/e2e-targetallocator/targetallocator-features/00-assert.yaml index 2516c022ed..69dea24df6 100644 --- a/tests/e2e-targetallocator/targetallocator-features/00-assert.yaml +++ b/tests/e2e-targetallocator/targetallocator-features/00-assert.yaml @@ -9,7 +9,7 @@ spec: containers: - args: - --config=/conf/collector.yaml - - --feature-gates=-confmap.unifyEnvVarExpansion,-component.UseLocalHostAsDefaultHost + - --feature-gates=-component.UseLocalHostAsDefaultHost name: otc-container volumeMounts: - mountPath: /conf