PR feedback

Author: benjaminjb

cmd/postgres-operator/main.go

@@ -134,8 +134,10 @@ func main() {
 
 	features := feature.NewGate()
 	assertNoError(features.Set(os.Getenv("PGO_FEATURE_GATES")))
+
+	ctx = feature.NewContext(ctx, features)
 	// This logs just the feature gates as set by the user
-	log.Info("feature gates enabled during deployment", "PGO_FEATURE_GATES", features.String())
+	log.Info("feature gates enabled during deployment", "PGO_FEATURE_GATES", feature.ShowAssigned(ctx))
 
 	cfg, err := runtime.GetConfig()
 	assertNoError(err)

internal/feature/features.go

@@ -43,7 +43,7 @@ package feature
 import (
 	"context"
 	"fmt"
-	"sort"
+	"slices"
 	"strings"
 
 	"k8s.io/component-base/featuregate"
@@ -54,8 +54,6 @@ type Feature = featuregate.Feature
 // Gate indicates what features exist and which are enabled.
 type Gate interface {
 	Enabled(Feature) bool
-	String() string
-	GetAll() map[Feature]featuregate.FeatureSpec
 }
 
 // MutableGate contains features that can be enabled or disabled.
@@ -126,13 +124,13 @@ func NewContext(ctx context.Context, gate Gate) context.Context {
 	return context.WithValue(ctx, contextKey{}, gate)
 }
 
-// ShowGates returns all the gates that are on by default
-// (if not turned off by the user) or were explicitly set
-// on by the user.
-func ShowGates(ctx context.Context) string {
+// ShowEnabled returns all the features enabled in the Gate contained in ctx.
+func ShowEnabled(ctx context.Context) string {
 	featuresEnabled := []string{}
-	gate, ok := ctx.Value(contextKey{}).(Gate)
-	if ok {
+	if gate, ok := ctx.Value(contextKey{}).(interface {
+		Gate
+		GetAll() map[Feature]featuregate.FeatureSpec
+	}); ok {
 		specs := gate.GetAll()
 		for feature := range specs {
 			// `gate.Enabled` first checks if the feature is enabled;
@@ -143,6 +141,19 @@ func ShowGates(ctx context.Context) string {
 			}
 		}
 	}
-	sort.Strings(featuresEnabled)
+	slices.Sort(featuresEnabled)
 	return strings.Join(featuresEnabled, ",")
 }
+
+// ShowAssigned returns the features enabled or disabled by Set and SetFromMap
+// in the Gate contained in ctx.
+func ShowAssigned(ctx context.Context) string {
+	featuresAssigned := ""
+	if gate, ok := ctx.Value(contextKey{}).(interface {
+		Gate
+		String() string
+	}); ok {
+		featuresAssigned = gate.String()
+	}
+	return featuresAssigned
+}

internal/feature/features_test.go

@@ -6,6 +6,7 @@ package feature
 
 import (
 	"context"
+	"strings"
 	"testing"
 
 	"gotest.tools/v3/assert"
@@ -14,6 +15,7 @@ import (
 func TestDefaults(t *testing.T) {
 	t.Parallel()
 	gate := NewGate()
+	ctx := NewContext(context.Background(), gate)
 
 	assert.Assert(t, false == gate.Enabled(AppendCustomQueries))
 	assert.Assert(t, true == gate.Enabled(AutoCreateUserSchema))
@@ -24,16 +26,17 @@ func TestDefaults(t *testing.T) {
 	assert.Assert(t, false == gate.Enabled(TablespaceVolumes))
 	assert.Assert(t, false == gate.Enabled(VolumeSnapshots))
 
-	assert.Equal(t, gate.String(), "")
+	assert.Equal(t, ShowAssigned(ctx), "")
 }
 
 func TestStringFormat(t *testing.T) {
 	t.Parallel()
 	gate := NewGate()
+	ctx := NewContext(context.Background(), gate)
 
 	assert.NilError(t, gate.Set(""))
 	assert.NilError(t, gate.Set("TablespaceVolumes=true"))
-	assert.Equal(t, gate.String(), "TablespaceVolumes=true")
+	assert.Equal(t, ShowAssigned(ctx), "TablespaceVolumes=true")
 	assert.Assert(t, true == gate.Enabled(TablespaceVolumes))
 
 	err := gate.Set("NotAGate=true")
@@ -54,20 +57,20 @@ func TestContext(t *testing.T) {
 	gate := NewGate()
 	ctx := NewContext(context.Background(), gate)
 
-	// ShowGates returns all fields that are turned on, whether explicitly
-	// by the user or implicitly due to feature default.
-	// Currently, the only feature defaulting to true is `AutoCreateUserSchema`.
-	assert.Equal(t, ShowGates(ctx), "AutoCreateUserSchema=true")
+	assert.Equal(t, ShowAssigned(ctx), "")
+	assert.Assert(t, ShowEnabled(ctx) != "") // This assumes some feature is enabled by default.
 
 	assert.NilError(t, gate.Set("TablespaceVolumes=true"))
-	assert.Assert(t, true == Enabled(ctx, TablespaceVolumes))
-	assert.Equal(t, ShowGates(ctx), "AutoCreateUserSchema=true,TablespaceVolumes=true")
+	assert.Assert(t, Enabled(ctx, TablespaceVolumes))
+	assert.Equal(t, ShowAssigned(ctx), "TablespaceVolumes=true")
+	assert.Assert(t,
+		strings.Contains(ShowEnabled(ctx), "TablespaceVolumes=true"),
+		"got: %v", ShowEnabled(ctx))
 
-	assert.NilError(t, gate.SetFromMap(map[string]bool{
-		TablespaceVolumes:    false,
-		AutoCreateUserSchema: false,
-	}))
-	assert.Assert(t, false == Enabled(ctx, TablespaceVolumes))
-	assert.Assert(t, false == Enabled(ctx, AutoCreateUserSchema))
-	assert.Equal(t, ShowGates(ctx), "")
+	assert.NilError(t, gate.SetFromMap(map[string]bool{TablespaceVolumes: false}))
+	assert.Assert(t, !Enabled(ctx, TablespaceVolumes))
+	assert.Equal(t, ShowAssigned(ctx), "TablespaceVolumes=false")
+	assert.Assert(t,
+		!strings.Contains(ShowEnabled(ctx), "TablespaceVolumes"),
+		"got: %v", ShowEnabled(ctx))
 }

internal/upgradecheck/header.go

@@ -57,7 +57,7 @@ func generateHeader(ctx context.Context, cfg *rest.Config, crClient crclient.Cli
 		BridgeClustersTotal: getBridgeClusters(ctx, crClient),
 		BuildSource:         os.Getenv("BUILD_SOURCE"),
 		DeploymentID:        ensureDeploymentID(ctx, crClient),
-		FeatureGatesEnabled: feature.ShowGates(ctx),
+		FeatureGatesEnabled: feature.ShowEnabled(ctx),
 		IsOpenShift:         isOpenShift,
 		KubernetesEnv:       getServerVersion(ctx, cfg),
 		PGOClustersTotal:    getManagedClusters(ctx, crClient),

internal/upgradecheck/header_test.go

@@ -136,7 +136,10 @@ func TestGenerateHeader(t *testing.T) {
 		assert.Equal(t, len(pgoList.Items), res.PGOClustersTotal)
 		assert.Equal(t, "1.2.3", res.PGOVersion)
 		assert.Equal(t, server.String(), res.KubernetesEnv)
-		assert.Equal(t, "TablespaceVolumes=true", res.FeatureGatesEnabled)
+		assert.Check(t, strings.Contains(
+			res.FeatureGatesEnabled,
+			"TablespaceVolumes=true",
+		))
 		assert.Equal(t, "test", res.PGOInstaller)
 		assert.Equal(t, "test-origin", res.PGOInstallerOrigin)
 		assert.Equal(t, "developer", res.BuildSource)