Skip to content

Commit

Permalink
Remove unifyEnvVarExpansion workaround for Prometheus receiver (#3174)
Browse files Browse the repository at this point in the history
* Remove unifyEnvVarExpansion workaround for Prometheus receiver

Signed-off-by: Pavol Loffay <[email protected]>

* Fix

Signed-off-by: Pavol Loffay <[email protected]>

* Remove flag

Signed-off-by: Pavol Loffay <[email protected]>

* Remove flag

Signed-off-by: Pavol Loffay <[email protected]>

* Fix

Signed-off-by: Pavol Loffay <[email protected]>

---------

Signed-off-by: Pavol Loffay <[email protected]>
  • Loading branch information
pavolloffay authored Jul 30, 2024
1 parent c0c0c79 commit 2f322f0
Show file tree
Hide file tree
Showing 9 changed files with 281 additions and 71 deletions.
16 changes: 16 additions & 0 deletions .chloggen/0150-workaround.yaml
Original file line number Diff line number Diff line change
@@ -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:
32 changes: 0 additions & 32 deletions apis/v1beta1/collector_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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:
Expand Down
37 changes: 0 additions & 37 deletions apis/v1beta1/collector_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
34 changes: 33 additions & 1 deletion pkg/collector/upgrade/v0_104_0.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
}
}
37 changes: 37 additions & 0 deletions pkg/collector/upgrade/v0_104_0_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
65 changes: 65 additions & 0 deletions pkg/collector/upgrade/v0_105_0.go
Original file line number Diff line number Diff line change
@@ -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
}
125 changes: 125 additions & 0 deletions pkg/collector/upgrade/v0_105_0_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
4 changes: 4 additions & 0 deletions pkg/collector/upgrade/versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading

0 comments on commit 2f322f0

Please sign in to comment.