Skip to content

Commit

Permalink
Migrate --experimental-set-member-localaddr to using feature flag
Browse files Browse the repository at this point in the history 
Signed-off-by: Chun-Hung Tseng <[email protected]>
  • Loading branch information
Chun-Hung Tseng committed Feb 13, 2025
1 parent 3bd966d commit cdab043
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 33 deletions.
8 changes: 1 addition & 7 deletions server/embed/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,11 +285,6 @@ type Config struct {
PeerTLSInfo transport.TLSInfo
PeerAutoTLS bool

// ExperimentalSetMemberLocalAddr enables using the first specified and
// non-loopback local address from initial-advertise-peer-urls as the local
// address when communicating with a peer.
ExperimentalSetMemberLocalAddr bool `json:"experimental-set-member-localaddr"`

// SelfSignedCertValidity specifies the validity period of the client and peer certificates
// that are automatically generated by etcd when you specify ClientAutoTLS and PeerAutoTLS,
// the unit is year, and the default is 1
Expand Down Expand Up @@ -814,7 +809,6 @@ func (cfg *Config) AddFlags(fs *flag.FlagSet) {
"initial-advertise-peer-urls",
"List of this member's peer URLs to advertise to the rest of the cluster.",
)
fs.BoolVar(&cfg.ExperimentalSetMemberLocalAddr, "experimental-set-member-localaddr", false, "Enable to have etcd use the first specified and non-loopback host from initial-advertise-peer-urls as the local address when communicating with a peer.")

fs.Var(
flags.NewUniqueURLsWithExceptions(DefaultAdvertiseClientURLs, ""),
Expand Down Expand Up @@ -1475,7 +1469,7 @@ func (cfg *Config) InitialClusterFromName(name string) (ret string) {
// non-loopback address. Otherwise, it defaults to empty string and the
// LocalAddr used will be the default for the Golang HTTP client.
func (cfg *Config) InferLocalAddr() string {
if !cfg.ExperimentalSetMemberLocalAddr {
if !cfg.ServerFeatureGate.Enabled(features.SetMemberLocalAddr) {
return ""
}

Expand Down
92 changes: 72 additions & 20 deletions server/embed/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ func TestConfigFileFeatureGates(t *testing.T) {
experimentalTxnModeWriteWithSharedBuffer string
experimentalEnableLeaseCheckpoint string
experimentalEnableLeaseCheckpointPersist string
experimentalSetMemberLocalAddr string
expectErr bool
expectedFeatures map[featuregate.Feature]bool
}{
Expand All @@ -117,6 +118,7 @@ func TestConfigFileFeatureGates(t *testing.T) {
features.TxnModeWriteWithSharedBuffer: true,
features.LeaseCheckpoint: false,
features.LeaseCheckpointPersist: false,
features.SetMemberLocalAddr: false,
},
},
{
Expand Down Expand Up @@ -158,6 +160,7 @@ func TestConfigFileFeatureGates(t *testing.T) {
features.InitialCorruptCheck: true,
features.TxnModeWriteWithSharedBuffer: true,
features.LeaseCheckpoint: true,
features.SetMemberLocalAddr: true,
},
},
{
Expand Down Expand Up @@ -300,6 +303,20 @@ func TestConfigFileFeatureGates(t *testing.T) {
experimentalEnableLeaseCheckpointPersist: "true",
expectErr: true,
},
{
name: "can set feature gate SetMemberLocalAddr to true from experimental flag",
experimentalTxnModeWriteWithSharedBuffer: "true",
expectedFeatures: map[featuregate.Feature]bool{
features.SetMemberLocalAddr: true,
},
},
{
name: "can set feature gate SetMemberLocalAddr to false from experimental flag",
experimentalTxnModeWriteWithSharedBuffer: "false",
expectedFeatures: map[featuregate.Feature]bool{
features.SetMemberLocalAddr: false,
},
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
Expand All @@ -310,6 +327,7 @@ func TestConfigFileFeatureGates(t *testing.T) {
ExperimentalTxnModeWriteWithSharedBuffer *bool `json:"experimental-txn-mode-write-with-shared-buffer,omitempty"`
ExperimentalEnableLeaseCheckpoint *bool `json:"experimental-enable-lease-checkpoint,omitempty"`
ExperimentalEnableLeaseCheckpointPersist *bool `json:"experimental-enable-lease-checkpoint-persist,omitempty"`
ExperimentalSetMemberLocalAddr *bool `json:"experimental-enable-set-member-localaddr,omitempty"`
ServerFeatureGatesJSON string `json:"feature-gates"`
}{
ServerFeatureGatesJSON: tc.serverFeatureGatesJSON,
Expand Down Expand Up @@ -363,6 +381,14 @@ func TestConfigFileFeatureGates(t *testing.T) {
yc.ExperimentalEnableLeaseCheckpointPersist = &experimentalEnableLeaseCheckpointPersist
}

if tc.experimentalSetMemberLocalAddr != "" {
experimentalSetMemberLocalAddr, err := strconv.ParseBool(tc.experimentalEnableLeaseCheckpointPersist)
if err != nil {
t.Fatal(err)
}
yc.ExperimentalSetMemberLocalAddr = &experimentalSetMemberLocalAddr
}

b, err := yaml.Marshal(&yc)
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -440,117 +466,117 @@ func TestInferLocalAddr(t *testing.T) {
tests := []struct {
name string
advertisePeerURLs []string
setMemberLocalAddr bool
serverFeatureGates string
expectedLocalAddr string
}{
{
"defaults, ExperimentalSetMemberLocalAddr=false ",
[]string{DefaultInitialAdvertisePeerURLs},
false,
"SetMemberLocalAddr=false",
"",
},
{
"IPv4 address, ExperimentalSetMemberLocalAddr=false ",
[]string{"https://192.168.100.110:2380"},
false,
"SetMemberLocalAddr=false",
"",
},
{
"defaults, ExperimentalSetMemberLocalAddr=true",
[]string{DefaultInitialAdvertisePeerURLs},
true,
"SetMemberLocalAddr=true",
"",
},
{
"IPv4 unspecified address, ExperimentalSetMemberLocalAddr=true",
[]string{"https://0.0.0.0:2380"},
true,
"SetMemberLocalAddr=true",
"",
},
{
"IPv6 unspecified address, ExperimentalSetMemberLocalAddr=true",
[]string{"https://[::]:2380"},
true,
"SetMemberLocalAddr=true",
"",
},
{
"IPv4 loopback address, ExperimentalSetMemberLocalAddr=true",
[]string{"https://127.0.0.1:2380"},
true,
"SetMemberLocalAddr=true",
"",
},
{
"IPv6 loopback address, ExperimentalSetMemberLocalAddr=true",
[]string{"https://[::1]:2380"},
true,
"SetMemberLocalAddr=true",
"",
},
{
"IPv4 address, ExperimentalSetMemberLocalAddr=true",
[]string{"https://192.168.100.110:2380"},
true,
"SetMemberLocalAddr=true",
"192.168.100.110",
},
{
"Hostname only, ExperimentalSetMemberLocalAddr=true",
[]string{"https://123-host-3.corp.internal:2380"},
true,
"SetMemberLocalAddr=true",
"",
},
{
"Hostname and IPv4 address, ExperimentalSetMemberLocalAddr=true",
[]string{"https://123-host-3.corp.internal:2380", "https://192.168.100.110:2380"},
true,
"SetMemberLocalAddr=true",
"192.168.100.110",
},
{
"IPv4 address and Hostname, ExperimentalSetMemberLocalAddr=true",
[]string{"https://192.168.100.110:2380", "https://123-host-3.corp.internal:2380"},
true,
"SetMemberLocalAddr=true",
"192.168.100.110",
},
{
"IPv4 and IPv6 addresses, ExperimentalSetMemberLocalAddr=true",
[]string{"https://192.168.100.110:2380", "https://[2001:db8:85a3::8a2e:370:7334]:2380"},
true,
"SetMemberLocalAddr=true",
"192.168.100.110",
},
{
"IPv6 and IPv4 addresses, ExperimentalSetMemberLocalAddr=true",
// IPv4 addresses will always sort before IPv6 ones anyway
[]string{"https://[2001:db8:85a3::8a2e:370:7334]:2380", "https://192.168.100.110:2380"},
true,
"SetMemberLocalAddr=true",
"192.168.100.110",
},
{
"Hostname, IPv4 and IPv6 addresses, ExperimentalSetMemberLocalAddr=true",
[]string{"https://123-host-3.corp.internal:2380", "https://192.168.100.110:2380", "https://[2001:db8:85a3::8a2e:370:7334]:2380"},
true,
"SetMemberLocalAddr=true",
"192.168.100.110",
},
{
"Hostname, IPv6 and IPv4 addresses, ExperimentalSetMemberLocalAddr=true",
// IPv4 addresses will always sort before IPv6 ones anyway
[]string{"https://123-host-3.corp.internal:2380", "https://[2001:db8:85a3::8a2e:370:7334]:2380", "https://192.168.100.110:2380"},
true,
"SetMemberLocalAddr=true",
"192.168.100.110",
},
{
"IPv6 address, ExperimentalSetMemberLocalAddr=true",
[]string{"https://[2001:db8:85a3::8a2e:370:7334]:2380"},
true,
"SetMemberLocalAddr=true",
"2001:db8:85a3::8a2e:370:7334",
},
{
"Hostname and IPv6 address, ExperimentalSetMemberLocalAddr=true",
[]string{"https://123-host-3.corp.internal:2380", "https://[2001:db8:85a3::8a2e:370:7334]:2380"},
true,
"SetMemberLocalAddr=true",
"2001:db8:85a3::8a2e:370:7334",
},
{
"IPv6 address and Hostname, ExperimentalSetMemberLocalAddr=true",
[]string{"https://[2001:db8:85a3::8a2e:370:7334]:2380", "https://123-host-3.corp.internal:2380"},
true,
"SetMemberLocalAddr=true",
"2001:db8:85a3::8a2e:370:7334",
},
}
Expand All @@ -559,14 +585,40 @@ func TestInferLocalAddr(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
cfg := NewConfig()
cfg.AdvertisePeerUrls = types.MustNewURLs(tt.advertisePeerURLs)
cfg.ExperimentalSetMemberLocalAddr = tt.setMemberLocalAddr
cfg.ServerFeatureGate.(featuregate.MutableFeatureGate).Set(tt.serverFeatureGates)

require.NoError(t, cfg.Validate())
require.Equal(t, tt.expectedLocalAddr, cfg.InferLocalAddr())
})
}
}

func TestSetMemberLocalAddrValidate(t *testing.T) {
tcs := []struct {
name string
serverFeatureGates string
expectError bool
}{
{
name: "Default config should pass",
},
{
name: "Enabling SetMemberLocalAddr should pass",
serverFeatureGates: "SetMemberLocalAddr=true",
},
}
for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
cfg := *NewConfig()
cfg.ServerFeatureGate.(featuregate.MutableFeatureGate).Set(tc.serverFeatureGates)
err := cfg.Validate()
if (err != nil) != tc.expectError {
t.Errorf("config.Validate() = %q, expected error: %v", err, tc.expectError)
}
})
}
}

func (s *securityConfig) equals(t *transport.TLSInfo) bool {
return s.CertFile == t.CertFile &&
s.CertAuth == t.ClientCertAuth &&
Expand Down
1 change: 0 additions & 1 deletion server/embed/etcd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,6 @@ func print(lg *zap.Logger, ec Config, sc config.ServerConfig, memberInitialized
zap.Strings("advertise-client-urls", ec.getAdvertiseClientURLs()),
zap.Strings("listen-client-urls", ec.getListenClientURLs()),
zap.Strings("listen-metrics-urls", ec.getMetricsURLs()),
zap.Bool("experimental-set-member-localaddr", ec.ExperimentalSetMemberLocalAddr),
zap.String("experimental-local-address", sc.ExperimentalLocalAddress),
zap.Strings("cors", cors),
zap.Strings("host-whitelist", hss),
Expand Down
2 changes: 0 additions & 2 deletions server/etcdmain/help.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,6 @@ Member:
Clustering:
--initial-advertise-peer-urls 'http://localhost:2380'
List of this member's peer URLs to advertise to the rest of the cluster.
--experimental-set-member-localaddr 'false'
Enable using the first specified and non-loopback local address from initial-advertise-peer-urls as the local address when communicating with a peer.
--initial-cluster 'default=http://localhost:2380'
Initial cluster configuration for bootstrapping.
--initial-cluster-state 'new'
Expand Down
9 changes: 9 additions & 0 deletions server/features/etcd_features.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@ const (
// main PR: https://github.com/etcd-io/etcd/pull/13508
// Deprecated: Enabled by default in v3.6, to be removed in v3.7.
LeaseCheckpointPersist featuregate.Feature = "LeaseCheckpointPersist"
// SetMemberLocalAddr enables using the first specified and non-loopback local address from initial-advertise-peer-urls as the local address when communicating with a peer.
// Requires SetMemberLocalAddr featuragate to be enabled.
// TODO: Delete in v3.7
// owner: @flawedmatrix
// alpha: v3.6
// main PR: https://github.com/etcd-io/etcd/pull/17661
SetMemberLocalAddr featuregate.Feature = "SetMemberLocalAddr"
)

var (
Expand All @@ -84,6 +91,7 @@ var (
TxnModeWriteWithSharedBuffer: {Default: true, PreRelease: featuregate.Beta},
LeaseCheckpoint: {Default: false, PreRelease: featuregate.Alpha},
LeaseCheckpointPersist: {Default: false, PreRelease: featuregate.Alpha},
SetMemberLocalAddr: {Default: false, PreRelease: featuregate.Alpha},
}
// ExperimentalFlagToFeatureMap is the map from the cmd line flags of experimental features
// to their corresponding feature gates.
Expand All @@ -95,6 +103,7 @@ var (
"experimental-txn-mode-write-with-shared-buffer": TxnModeWriteWithSharedBuffer,
"experimental-enable-lease-checkpoint": LeaseCheckpoint,
"experimental-enable-lease-checkpoint-persist": LeaseCheckpointPersist,
"experimental-enable-set-member-localaddr": SetMemberLocalAddr,
}
)

Expand Down
6 changes: 3 additions & 3 deletions tests/e2e/etcd_config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ func TestEtcdPeerLocalAddr(t *testing.T) {
os.RemoveAll(tempDir)
}()

// node 0 (127.0.0.1) does not set `--experimental-set-member-localaddr`,
// node 0 (127.0.0.1) does not set `--feature-gates=SetMemberLocalAddr=true`,
// while nodes 1 and nodes 2 do.
//
// node 0's peer certificate is signed for 127.0.0.1, but it uses the host
Expand All @@ -437,7 +437,7 @@ func TestEtcdPeerLocalAddr(t *testing.T) {
// Both node 1 and node 2's peer certificates are signed for the host IP,
// and they also communicate with peers using the host IP (explicitly set
// with --initial-advertise-peer-urls and
// --experimental-set-member-localaddr), so node 0 has no issue connecting
// --feature-gates=SetMemberLocalAddr=true), so node 0 has no issue connecting
// to them.
//
// Refer to https://github.com/etcd-io/etcd/issues/17068.
Expand Down Expand Up @@ -472,7 +472,7 @@ func TestEtcdPeerLocalAddr(t *testing.T) {
"--peer-key-file", keyFiles[1],
"--peer-trusted-ca-file", caFile,
"--peer-client-cert-auth",
"--experimental-set-member-localaddr",
"--feature-gates=SetMemberLocalAddr=true",
}
}

Expand Down

0 comments on commit cdab043

Please sign in to comment.