Skip to content

Commit

Permalink
Merge pull request #18279 from siyuanfoundation/fg-1
Browse files Browse the repository at this point in the history
Add "server-feature-gates" flag.
  • Loading branch information
serathius authored Jul 23, 2024
2 parents e84ba1c + 7b35514 commit 3b4e2f4
Show file tree
Hide file tree
Showing 7 changed files with 146 additions and 9 deletions.
13 changes: 10 additions & 3 deletions pkg/featuregate/feature_gate.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package featuregate

import (
"flag"
"fmt"
"sort"
"strconv"
Expand All @@ -30,7 +31,7 @@ import (
type Feature string

const (
flagName = "feature-gates"
defaultFlagName = "feature-gates"

// allAlphaGate is a global toggle for alpha features. Per-feature key
// values override the default set by allAlphaGate. Examples:
Expand Down Expand Up @@ -98,7 +99,7 @@ type MutableFeatureGate interface {
FeatureGate

// AddFlag adds a flag for setting global feature gates to the specified FlagSet.
AddFlag(fs *pflag.FlagSet)
AddFlag(fs *flag.FlagSet, flagName string)
// Set parses and stores flag gates for known features
// from a string like feature1=true,feature2=false,...
Set(value string) error
Expand Down Expand Up @@ -165,6 +166,9 @@ func setUnsetBetaGates(known map[Feature]FeatureSpec, enabled map[Feature]bool,
var _ pflag.Value = &featureGate{}

func New(name string, lg *zap.Logger) *featureGate {
if lg == nil {
lg = zap.NewNop()
}
known := map[Feature]FeatureSpec{}
for k, v := range defaultFeatures {
known[k] = v
Expand Down Expand Up @@ -349,7 +353,10 @@ func (f *featureGate) Enabled(key Feature) bool {
}

// AddFlag adds a flag for setting global feature gates to the specified FlagSet.
func (f *featureGate) AddFlag(fs *pflag.FlagSet) {
func (f *featureGate) AddFlag(fs *flag.FlagSet, flagName string) {
if flagName == "" {
flagName = defaultFlagName
}
f.lock.Lock()
// TODO(mtaufen): Shouldn't we just close it on the first Set/SetFromMap instead?
// Not all components expose a feature gates flag using this AddFlag method, and
Expand Down
12 changes: 6 additions & 6 deletions pkg/featuregate/feature_gate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@
package featuregate

import (
"flag"
"fmt"
"strings"
"testing"

"github.com/spf13/pflag"
"github.com/stretchr/testify/assert"
"go.uber.org/zap/zaptest"
)
Expand Down Expand Up @@ -203,15 +203,15 @@ func TestFeatureGateFlag(t *testing.T) {
}
for i, test := range tests {
t.Run(test.arg, func(t *testing.T) {
fs := pflag.NewFlagSet("testfeaturegateflag", pflag.ContinueOnError)
fs := flag.NewFlagSet("testfeaturegateflag", flag.ContinueOnError)
f := New("test", zaptest.NewLogger(t))
f.Add(map[Feature]FeatureSpec{
testAlphaGate: {Default: false, PreRelease: Alpha},
testBetaGate: {Default: false, PreRelease: Beta},
})
f.AddFlag(fs)
f.AddFlag(fs, defaultFlagName)

err := fs.Parse([]string{fmt.Sprintf("--%s=%s", flagName, test.arg)})
err := fs.Parse([]string{fmt.Sprintf("--%s=%s", defaultFlagName, test.arg)})
if test.parseError != "" {
if !strings.Contains(err.Error(), test.parseError) {
t.Errorf("%d: Parse() Expected %v, Got %v", i, test.parseError, err)
Expand Down Expand Up @@ -603,8 +603,8 @@ func TestFeatureGateOverrideDefault(t *testing.T) {

t.Run("returns error if already added to flag set", func(t *testing.T) {
f := New("test", zaptest.NewLogger(t))
fs := pflag.NewFlagSet("test", pflag.ContinueOnError)
f.AddFlag(fs)
fs := flag.NewFlagSet("test", flag.ContinueOnError)
f.AddFlag(fs, defaultFlagName)

if err := f.OverrideDefault("TestFeature", true); err == nil {
t.Error("expected a non-nil error to be returned")
Expand Down
11 changes: 11 additions & 0 deletions server/embed/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
"go.etcd.io/etcd/client/pkg/v3/transport"
"go.etcd.io/etcd/client/pkg/v3/types"
clientv3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/pkg/v3/featuregate"
"go.etcd.io/etcd/pkg/v3/flags"
"go.etcd.io/etcd/pkg/v3/netutil"
"go.etcd.io/etcd/server/v3/config"
Expand All @@ -50,6 +51,7 @@ import (
"go.etcd.io/etcd/server/v3/etcdserver/api/rafthttp"
"go.etcd.io/etcd/server/v3/etcdserver/api/v3compactor"
"go.etcd.io/etcd/server/v3/etcdserver/api/v3discovery"
"go.etcd.io/etcd/server/v3/features"
)

const (
Expand Down Expand Up @@ -108,6 +110,8 @@ const (
maxElectionMs = 50000
// backend freelist map type
freelistArrayType = "array"

ServerFeatureGateFlagName = "feature-gates"
)

var (
Expand Down Expand Up @@ -455,6 +459,9 @@ type Config struct {

// V2Deprecation describes phase of API & Storage V2 support
V2Deprecation config.V2DeprecationEnum `json:"v2-deprecation"`

// ServerFeatureGate is a server level feature gate
ServerFeatureGate featuregate.FeatureGate
}

// configYAML holds the config suitable for yaml parsing
Expand Down Expand Up @@ -576,6 +583,7 @@ func NewConfig() *Config {
},

AutoCompactionMode: DefaultAutoCompactionMode,
ServerFeatureGate: features.NewDefaultServerFeatureGate(DefaultName, nil),
}
cfg.InitialCluster = cfg.InitialClusterFromName(cfg.Name)
return cfg
Expand Down Expand Up @@ -762,6 +770,9 @@ func (cfg *Config) AddFlags(fs *flag.FlagSet) {
// unsafe
fs.BoolVar(&cfg.UnsafeNoFsync, "unsafe-no-fsync", false, "Disables fsync, unsafe, will cause data loss.")
fs.BoolVar(&cfg.ForceNewCluster, "force-new-cluster", false, "Force to create a new one member cluster.")

// featuregate
cfg.ServerFeatureGate.(featuregate.MutableFeatureGate).AddFlag(fs, ServerFeatureGateFlagName)
}

func ConfigFromFile(path string) (*Config, error) {
Expand Down
1 change: 1 addition & 0 deletions server/embed/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) {
ExperimentalMaxLearners: cfg.ExperimentalMaxLearners,
V2Deprecation: cfg.V2DeprecationEffective(),
ExperimentalLocalAddress: cfg.InferLocalAddr(),
ServerFeatureGate: cfg.ServerFeatureGate,
}

if srvcfg.ExperimentalEnableDistributedTracing {
Expand Down
51 changes: 51 additions & 0 deletions server/etcdmain/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ import (

"sigs.k8s.io/yaml"

"go.etcd.io/etcd/pkg/v3/featuregate"
"go.etcd.io/etcd/pkg/v3/flags"
"go.etcd.io/etcd/server/v3/embed"
"go.etcd.io/etcd/server/v3/features"
)

func TestConfigParsingMemberFlags(t *testing.T) {
Expand Down Expand Up @@ -395,6 +397,55 @@ func TestFlagsPresentInHelp(t *testing.T) {
})
}

func TestParseFeatureGateFlags(t *testing.T) {
testCases := []struct {
name string
args []string
expectErr bool
expectedFeatures map[featuregate.Feature]bool
}{
{
name: "default",
expectedFeatures: map[featuregate.Feature]bool{
features.DistributedTracing: false,
features.StopGRPCServiceOnDefrag: false,
},
},
{
name: "can set feature gate flag",
args: []string{
"--experimental-stop-grpc-service-on-defrag=false",
fmt.Sprintf("--%s=DistributedTracing=true,StopGRPCServiceOnDefrag=true", embed.ServerFeatureGateFlagName),
},
expectedFeatures: map[featuregate.Feature]bool{
features.DistributedTracing: true,
features.StopGRPCServiceOnDefrag: true,
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
cfg := newConfig()
err := cfg.parse(tc.args)
if tc.expectErr {
if err == nil {
t.Fatal("expect parse error")
}
return
}
if err != nil {
t.Fatal(err)
}
for k, v := range tc.expectedFeatures {
if cfg.ec.ServerFeatureGate.Enabled(k) != v {
t.Errorf("expected feature gate %s=%v, got %v", k, v, cfg.ec.ServerFeatureGate.Enabled(k))
}
}
})
}
}

func mustCreateCfgFile(t *testing.T, b []byte) *os.File {
tmpfile, err := os.CreateTemp("", "servercfg")
if err != nil {
Expand Down
4 changes: 4 additions & 0 deletions server/etcdmain/help.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ package etcdmain
import (
"fmt"
"strconv"
"strings"

"golang.org/x/crypto/bcrypt"

cconfig "go.etcd.io/etcd/server/v3/config"
"go.etcd.io/etcd/server/v3/embed"
"go.etcd.io/etcd/server/v3/etcdserver/api/rafthttp"
"go.etcd.io/etcd/server/v3/features"
)

var (
Expand Down Expand Up @@ -103,6 +105,8 @@ Member:
Read timeout set on each rafthttp connection
--raft-write-timeout '` + rafthttp.DefaultConnWriteTimeout.String() + `'
Write timeout set on each rafthttp connection
--feature-gates ''
A set of key=value pairs that describe server level feature gates for alpha/experimental features. Options are:` + "\n " + strings.Join(features.NewDefaultServerFeatureGate("", nil).KnownFeatures(), "\n ") + `
Clustering:
--initial-advertise-peer-urls 'http://localhost:2380'
Expand Down
63 changes: 63 additions & 0 deletions server/features/etcd_features.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright 2024 The etcd 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 features

import (
"fmt"

"go.uber.org/zap"

"go.etcd.io/etcd/pkg/v3/featuregate"
)

const (
// Every feature gate should add method here following this template:
//
// // owner: @username
// // kep: https://kep.k8s.io/NNN (or issue: https://github.com/etcd-io/etcd/issues/NNN, or main PR: https://github.com/etcd-io/etcd/pull/NNN)
// // alpha: v3.X
// MyFeature featuregate.Feature = "MyFeature"
//
// Feature gates should be listed in alphabetical, case-sensitive
// (upper before any lower case character) order. This reduces the risk
// of code conflicts because changes are more likely to be scattered
// across the file.

// DistributedTracing enables experimental distributed tracing using OpenTelemetry Tracing.
// owner: @dashpole
// alpha: v3.5
// issue: https://github.com/etcd-io/etcd/issues/12460
DistributedTracing featuregate.Feature = "DistributedTracing"
// StopGRPCServiceOnDefrag enables etcd gRPC service to stop serving client requests on defragmentation.
// owner: @chaochn47
// alpha: v3.6
// main PR: https://github.com/etcd-io/etcd/pull/18279
StopGRPCServiceOnDefrag featuregate.Feature = "StopGRPCServiceOnDefrag"
)

var (
DefaultEtcdServerFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
DistributedTracing: {Default: false, PreRelease: featuregate.Alpha},
StopGRPCServiceOnDefrag: {Default: false, PreRelease: featuregate.Alpha},
}
)

func NewDefaultServerFeatureGate(name string, lg *zap.Logger) featuregate.FeatureGate {
fg := featuregate.New(fmt.Sprintf("%sServerFeatureGate", name), lg)
if err := fg.Add(DefaultEtcdServerFeatureGates); err != nil {
panic(err)
}
return fg
}

0 comments on commit 3b4e2f4

Please sign in to comment.