Skip to content

Commit

Permalink
Update feature gate logging to include default on (#4029)
Browse files Browse the repository at this point in the history
* Update feature gate logging to include default on

After the 5.7 release, when AutoUserSchemaCreate was graduated to
default on/true, we discovered that our current system (and the
underlying featuregate implementation) treats features explicitly
turned on by the user differently than features turned on by default.

This PR updates that logging to make clear what features are
specifically requested by the user and what features are actually enabled
(a union of defaults and end user settings).

Issues: [PGO-1824]

Co-authored-by: Chris Bandy <chris.bandy@crunchydata.com>
  • Loading branch information
benjaminjb and cbandy authored Dec 3, 2024
1 parent 58351d3 commit 12c8207
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 19 deletions.
8 changes: 7 additions & 1 deletion cmd/postgres-operator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,13 @@ func main() {

features := feature.NewGate()
assertNoError(features.Set(os.Getenv("PGO_FEATURE_GATES")))
log.Info("feature gates enabled", "PGO_FEATURE_GATES", features.String())

ctx = feature.NewContext(ctx, features)
log.Info("feature gates",
// These are set by the user
"PGO_FEATURE_GATES", feature.ShowAssigned(ctx),
// These are enabled, including features that are on by default
"enabled", feature.ShowEnabled(ctx))

cfg, err := runtime.GetConfig()
assertNoError(err)
Expand Down
41 changes: 34 additions & 7 deletions internal/feature/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ package feature

import (
"context"
"fmt"
"slices"
"strings"

"k8s.io/component-base/featuregate"
)
Expand All @@ -51,7 +54,6 @@ type Feature = featuregate.Feature
// Gate indicates what features exist and which are enabled.
type Gate interface {
Enabled(Feature) bool
String() string
}

// MutableGate contains features that can be enabled or disabled.
Expand Down Expand Up @@ -122,11 +124,36 @@ func NewContext(ctx context.Context, gate Gate) context.Context {
return context.WithValue(ctx, contextKey{}, gate)
}

func ShowGates(ctx context.Context) string {
featuresEnabled := ""
gate, ok := ctx.Value(contextKey{}).(Gate)
if ok {
featuresEnabled = gate.String()
// ShowEnabled returns all the features enabled in the Gate contained in ctx.
func ShowEnabled(ctx context.Context) string {
featuresEnabled := []string{}
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;
// then (if not explicitly set by the user),
// it checks if the feature is on/true by default
if gate.Enabled(feature) {
featuresEnabled = append(featuresEnabled, fmt.Sprintf("%s=true", feature))
}
}
}
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 featuresEnabled
return featuresAssigned
}
22 changes: 14 additions & 8 deletions internal/feature/features_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package feature

import (
"context"
"strings"
"testing"

"gotest.tools/v3/assert"
Expand All @@ -23,8 +24,6 @@ func TestDefaults(t *testing.T) {
assert.Assert(t, false == gate.Enabled(PGBouncerSidecars))
assert.Assert(t, false == gate.Enabled(TablespaceVolumes))
assert.Assert(t, false == gate.Enabled(VolumeSnapshots))

assert.Equal(t, gate.String(), "")
}

func TestStringFormat(t *testing.T) {
Expand All @@ -33,7 +32,6 @@ func TestStringFormat(t *testing.T) {

assert.NilError(t, gate.Set(""))
assert.NilError(t, gate.Set("TablespaceVolumes=true"))
assert.Equal(t, gate.String(), "TablespaceVolumes=true")
assert.Assert(t, true == gate.Enabled(TablespaceVolumes))

err := gate.Set("NotAGate=true")
Expand All @@ -53,13 +51,21 @@ func TestContext(t *testing.T) {
t.Parallel()
gate := NewGate()
ctx := NewContext(context.Background(), gate)
assert.Equal(t, ShowGates(ctx), "")

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), "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}))
assert.Assert(t, false == Enabled(ctx, TablespaceVolumes))
assert.Equal(t, ShowGates(ctx), "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))
}
2 changes: 1 addition & 1 deletion internal/upgradecheck/header.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func generateHeader(ctx context.Context, crClient crclient.Client,
BridgeClustersTotal: getBridgeClusters(ctx, crClient),
BuildSource: os.Getenv("BUILD_SOURCE"),
DeploymentID: ensureDeploymentID(ctx, crClient),
FeatureGatesEnabled: feature.ShowGates(ctx),
FeatureGatesEnabled: feature.ShowEnabled(ctx),
IsOpenShift: kubernetes.IsOpenShift(ctx),
KubernetesEnv: kubernetes.VersionString(ctx),
PGOClustersTotal: getManagedClusters(ctx, crClient),
Expand Down
5 changes: 4 additions & 1 deletion internal/upgradecheck/header_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,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, discovery.Version().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)
Expand Down
2 changes: 1 addition & 1 deletion internal/upgradecheck/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func TestCheckForUpgrades(t *testing.T) {
assert.Equal(t, data.RegistrationToken, "speakFriend")
assert.Equal(t, data.BridgeClustersTotal, 2)
assert.Equal(t, data.PGOClustersTotal, 2)
assert.Equal(t, data.FeatureGatesEnabled, "TablespaceVolumes=true")
assert.Equal(t, data.FeatureGatesEnabled, "AutoCreateUserSchema=true,TablespaceVolumes=true")
}

t.Run("success", func(t *testing.T) {
Expand Down

0 comments on commit 12c8207

Please sign in to comment.