Skip to content

Commit

Permalink
apis/v1beta1: remove ComponentUseLocalHostAsDefaultHost feature gate
Browse files Browse the repository at this point in the history
Signed-off-by: Benedikt Bongartz <[email protected]>
  • Loading branch information
frzifus committed Oct 7, 2024
1 parent 63f5d1c commit 00750c3
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 109 deletions.
44 changes: 26 additions & 18 deletions apis/v1beta1/collector_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package v1beta1
import (
"context"
"fmt"
"slices"
"strings"

"github.com/go-logr/logr"
Expand Down Expand Up @@ -78,7 +79,9 @@ func (c CollectorWebhook) Default(_ context.Context, obj runtime.Object) error {
otelcol.Spec.TargetAllocator.Replicas = &one
}

ComponentUseLocalHostAsDefaultHost(otelcol)
// NOTE: we may want to remove that part in the near future.
// https://github.com/open-telemetry/opentelemetry-operator/issues/3306
otelcol.Spec.Args = RemoveFeatureGate(otelcol.Spec.Args, "-component.UseLocalHostAsDefaultHost")

if otelcol.Spec.Autoscaler != nil && otelcol.Spec.Autoscaler.MaxReplicas != nil {
if otelcol.Spec.Autoscaler.MinReplicas == nil {
Expand Down Expand Up @@ -454,22 +457,27 @@ func SetupCollectorWebhook(mgr ctrl.Manager, cfg config.Config, reviewer *rbac.R
Complete()
}

// ComponentUseLocalHostAsDefaultHost enables component.UseLocalHostAsDefaultHost
// featuregate on the given collector instance.
// NOTE: For more details, visit:
// https://github.com/open-telemetry/opentelemetry-collector/issues/8510
func ComponentUseLocalHostAsDefaultHost(otelcol *OpenTelemetryCollector) {
const (
baseFlag = "feature-gates"
fgFlag = "component.UseLocalHostAsDefaultHost"
)
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
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
}
68 changes: 56 additions & 12 deletions apis/v1beta1/collector_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
name: "update config defaults",
otelcol: v1beta1.OpenTelemetryCollector{
Spec: v1beta1.OpenTelemetryCollectorSpec{
OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{
Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
},
Config: func() v1beta1.Config {
const input = `{"receivers":{"otlp":{"protocols":{"grpc":null,"http":null}}},"exporters":{"debug":null},"service":{"pipelines":{"traces":{"receivers":["otlp"],"exporters":["debug"]}}}}`
var cfg v1beta1.Config
Expand All @@ -162,8 +165,8 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
},
Spec: v1beta1.OpenTelemetryCollectorSpec{
OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{
Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
ManagementState: v1beta1.ManagementStateManaged,
Args: map[string]string{},
Replicas: &one,
},
Mode: v1beta1.ModeDeployment,
Expand Down Expand Up @@ -195,7 +198,6 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
},
Spec: v1beta1.OpenTelemetryCollectorSpec{
OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{
Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
ManagementState: v1beta1.ManagementStateManaged,
Replicas: &one,
},
Expand All @@ -219,7 +221,6 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
},
Spec: v1beta1.OpenTelemetryCollectorSpec{
OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{
Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
ManagementState: v1beta1.ManagementStateManaged,
Replicas: &one,
},
Expand Down Expand Up @@ -247,7 +248,6 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
Mode: v1beta1.ModeSidecar,
UpgradeStrategy: "adhoc",
OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{
Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
Replicas: &five,
ManagementState: v1beta1.ManagementStateManaged,
},
Expand All @@ -274,7 +274,6 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
Mode: v1beta1.ModeSidecar,
UpgradeStrategy: "adhoc",
OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{
Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
Replicas: &five,
ManagementState: v1beta1.ManagementStateUnmanaged,
},
Expand All @@ -299,7 +298,6 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
Mode: v1beta1.ModeDeployment,
UpgradeStrategy: v1beta1.UpgradeStrategyAutomatic,
OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{
Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
Replicas: &one,
ManagementState: v1beta1.ManagementStateManaged,
},
Expand Down Expand Up @@ -328,7 +326,6 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
Spec: v1beta1.OpenTelemetryCollectorSpec{
Mode: v1beta1.ModeDeployment,
OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{
Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
ManagementState: v1beta1.ManagementStateManaged,
Replicas: &one,
},
Expand Down Expand Up @@ -364,7 +361,6 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
Spec: v1beta1.OpenTelemetryCollectorSpec{
Mode: v1beta1.ModeDeployment,
OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{
Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
Replicas: &one,
ManagementState: v1beta1.ManagementStateManaged,
PodDisruptionBudget: &v1beta1.PodDisruptionBudgetSpec{
Expand Down Expand Up @@ -402,7 +398,6 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
Spec: v1beta1.OpenTelemetryCollectorSpec{
Mode: v1beta1.ModeDeployment,
OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{
Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
Replicas: &one,
ManagementState: v1beta1.ManagementStateManaged,
},
Expand Down Expand Up @@ -445,7 +440,6 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
Spec: v1beta1.OpenTelemetryCollectorSpec{
Mode: v1beta1.ModeDeployment,
OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{
Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
Replicas: &one,
ManagementState: v1beta1.ManagementStateManaged,
},
Expand Down Expand Up @@ -483,7 +477,6 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
Spec: v1beta1.OpenTelemetryCollectorSpec{
Mode: v1beta1.ModeDeployment,
OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{
Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
Replicas: &one,
ManagementState: v1beta1.ManagementStateManaged,
},
Expand Down Expand Up @@ -514,7 +507,6 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
Spec: v1beta1.OpenTelemetryCollectorSpec{
Mode: v1beta1.ModeDeployment,
OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{
Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
Replicas: &one,
ManagementState: v1beta1.ManagementStateManaged,
},
Expand Down Expand Up @@ -1455,3 +1447,55 @@ func getReviewer(shouldFailSAR bool) *rbac.Reviewer {
})
return rbac.NewReviewer(c)
}

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 := v1beta1.RemoveFeatureGate(test.args, test.feature)
assert.Equal(t, test.expected, args)
})
}
}
28 changes: 1 addition & 27 deletions pkg/collector/upgrade/v0_105_0.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package upgrade

import (
"slices"
"strings"

"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
Expand All @@ -34,32 +33,7 @@ func upgrade0_105_0(_ VersionUpgrade, otelcol *v1beta1.OpenTelemetryCollector) (
}

envVarExpansionFeatureFlag := "-confmap.unifyEnvVarExpansion"
otelcol.Spec.OpenTelemetryCommonFields.Args = RemoveFeatureGate(otelcol.Spec.OpenTelemetryCommonFields.Args, envVarExpansionFeatureFlag)
otelcol.Spec.OpenTelemetryCommonFields.Args = v1beta1.RemoveFeatureGate(otelcol.Spec.OpenTelemetryCommonFields.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
}
52 changes: 0 additions & 52 deletions pkg/collector/upgrade/v0_105_0_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,55 +71,3 @@ func Test0_105_0Upgrade(t *testing.T) {
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)
})
}
}

0 comments on commit 00750c3

Please sign in to comment.