Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

*: disable v2 API by default #10935

Merged
merged 4 commits into from
Jul 29, 2019
Merged

*: disable v2 API by default #10935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ecb9156
embed: disable v2 by default
gyuho Jul 25, 2019
3812842
Documentation/op-guide: disable v2 by default
gyuho Jul 25, 2019
ae87b21
tests/e2e: enable-v2 for v2 e2e tests
gyuho Jul 25, 2019
2f30e9a
etcdserver: document v2 usage in "publish" method
gyuho Jul 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Documentation/op-guide/configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ To start etcd automatically using custom settings at startup in Linux, using a [

### --enable-v2
+ Accept etcd V2 client requests
+ default: true
+ default: false
+ env variable: ETCD_ENABLE_V2

## Proxy flags
Expand Down
5 changes: 2 additions & 3 deletions embed/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,8 @@ const (
// It's enabled by default.
DefaultStrictReconfigCheck = true
// DefaultEnableV2 is the default value for "--enable-v2" flag.
// v2 is enabled by default.
// TODO: disable v2 when deprecated.
DefaultEnableV2 = true
// v2 API is disabled by default.
DefaultEnableV2 = false

// maxElectionMs specifies the maximum value of election timeout.
// More details are listed in ../Documentation/tuning.md#time-parameters.
Expand Down
6 changes: 6 additions & 0 deletions etcdserver/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1980,6 +1980,12 @@ func (s *EtcdServer) sync(timeout time.Duration) {
// static clientURLs of the server.
// The function keeps attempting to register until it succeeds,
// or its server is stopped.
//
// Use v2 store to encode member attributes, and apply through Raft
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

// but does not go through v2 API endpoint, which means even with v2
// client handler disabled (e.g. --enable-v2=false), cluster can still
// process publish requests through rafthttp
// TODO: Deprecate v2 store
func (s *EtcdServer) publish(timeout time.Duration) {
b, err := json.Marshal(s.attributes)
if err != nil {
Expand Down
4 changes: 4 additions & 0 deletions tests/e2e/cluster_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ type etcdProcessClusterConfig struct {
initialToken string
quotaBackendBytes int64
noStrictReconfig bool
enableV2 bool
initialCorruptCheck bool
authTokenOpts string
}
Expand Down Expand Up @@ -241,6 +242,9 @@ func (cfg *etcdProcessClusterConfig) etcdServerProcessConfigs() []*etcdServerPro
if cfg.noStrictReconfig {
args = append(args, "--strict-reconfig-check=false")
}
if cfg.enableV2 {
args = append(args, "--enable-v2")
}
if cfg.initialCorruptCheck {
args = append(args, "--experimental-initial-corrupt-check")
}
Expand Down
26 changes: 21 additions & 5 deletions tests/e2e/ctl_v2_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ func testCtlV2Set(t *testing.T, cfg *etcdProcessClusterConfig, quorum bool) {
defer os.Unsetenv("ETCDCTL_API")
defer testutil.AfterTest(t)

cfg.enableV2 = true
epc := setupEtcdctlTest(t, cfg, quorum)
defer func() {
if errC := epc.Close(); errC != nil {
Expand All @@ -61,6 +62,7 @@ func testCtlV2Mk(t *testing.T, cfg *etcdProcessClusterConfig, quorum bool) {
defer os.Unsetenv("ETCDCTL_API")
defer testutil.AfterTest(t)

cfg.enableV2 = true
epc := setupEtcdctlTest(t, cfg, quorum)
defer func() {
if errC := epc.Close(); errC != nil {
Expand Down Expand Up @@ -89,6 +91,7 @@ func testCtlV2Rm(t *testing.T, cfg *etcdProcessClusterConfig) {
defer os.Unsetenv("ETCDCTL_API")
defer testutil.AfterTest(t)

cfg.enableV2 = true
epc := setupEtcdctlTest(t, cfg, true)
defer func() {
if errC := epc.Close(); errC != nil {
Expand Down Expand Up @@ -118,6 +121,7 @@ func testCtlV2Ls(t *testing.T, cfg *etcdProcessClusterConfig, quorum bool) {
defer os.Unsetenv("ETCDCTL_API")
defer testutil.AfterTest(t)

cfg.enableV2 = true
epc := setupEtcdctlTest(t, cfg, quorum)
defer func() {
if errC := epc.Close(); errC != nil {
Expand All @@ -144,6 +148,7 @@ func testCtlV2Watch(t *testing.T, cfg *etcdProcessClusterConfig, noSync bool) {
defer os.Unsetenv("ETCDCTL_API")
defer testutil.AfterTest(t)

cfg.enableV2 = true
epc := setupEtcdctlTest(t, cfg, true)
defer func() {
if errC := epc.Close(); errC != nil {
Expand Down Expand Up @@ -172,7 +177,9 @@ func TestCtlV2GetRoleUser(t *testing.T) {
defer os.Unsetenv("ETCDCTL_API")
defer testutil.AfterTest(t)

epc := setupEtcdctlTest(t, &configNoTLS, false)
copied := configNoTLS
copied.enableV2 = true
epc := setupEtcdctlTest(t, &copied, false)
defer func() {
if err := epc.Close(); err != nil {
t.Fatalf("error closing etcd processes (%v)", err)
Expand Down Expand Up @@ -207,7 +214,9 @@ func testCtlV2UserList(t *testing.T, username string) {
defer os.Unsetenv("ETCDCTL_API")
defer testutil.AfterTest(t)

epc := setupEtcdctlTest(t, &configNoTLS, false)
copied := configNoTLS
copied.enableV2 = true
epc := setupEtcdctlTest(t, &copied, false)
defer func() {
if err := epc.Close(); err != nil {
t.Fatalf("error closing etcd processes (%v)", err)
Expand All @@ -227,7 +236,9 @@ func TestCtlV2RoleList(t *testing.T) {
defer os.Unsetenv("ETCDCTL_API")
defer testutil.AfterTest(t)

epc := setupEtcdctlTest(t, &configNoTLS, false)
copied := configNoTLS
copied.enableV2 = true
epc := setupEtcdctlTest(t, &copied, false)
defer func() {
if err := epc.Close(); err != nil {
t.Fatalf("error closing etcd processes (%v)", err)
Expand Down Expand Up @@ -261,6 +272,7 @@ func testCtlV2Backup(t *testing.T, snapCount int, v3 bool) {

etcdCfg := configNoTLS
etcdCfg.snapshotCount = snapCount
etcdCfg.enableV2 = true
epc1 := setupEtcdctlTest(t, &etcdCfg, false)

// v3 put before v2 set so snapshot happens after v3 operations to confirm
Expand Down Expand Up @@ -293,6 +305,7 @@ func testCtlV2Backup(t *testing.T, snapCount int, v3 bool) {
cfg2.dataDirPath = backupDir
cfg2.keepDataDir = true
cfg2.forceNewCluster = true
cfg2.enableV2 = true
epc2 := setupEtcdctlTest(t, &cfg2, false)

// check if backup went through correctly
Expand Down Expand Up @@ -333,7 +346,7 @@ func TestCtlV2AuthWithCommonName(t *testing.T) {

copiedCfg := configClientTLS
copiedCfg.clientCertAuthEnabled = true

copiedCfg.enableV2 = true
epc := setupEtcdctlTest(t, &copiedCfg, false)
defer func() {
if err := epc.Close(); err != nil {
Expand Down Expand Up @@ -368,7 +381,10 @@ func TestCtlV2ClusterHealth(t *testing.T) {
os.Setenv("ETCDCTL_API", "2")
defer os.Unsetenv("ETCDCTL_API")
defer testutil.AfterTest(t)
epc := setupEtcdctlTest(t, &configNoTLS, true)

copied := configNoTLS
copied.enableV2 = true
epc := setupEtcdctlTest(t, &copied, true)
defer func() {
if err := epc.Close(); err != nil {
t.Fatalf("error closing etcd processes (%v)", err)
Expand Down
5 changes: 4 additions & 1 deletion tests/e2e/v2_curl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func testCurlPutGet(t *testing.T, cfg *etcdProcessClusterConfig) {
// stale reads that will break the test
cfg = configStandalone(*cfg)

cfg.enableV2 = true
epc, err := newEtcdProcessCluster(cfg)
if err != nil {
t.Fatalf("could not start etcd process cluster (%v)", err)
Expand Down Expand Up @@ -69,7 +70,9 @@ func TestV2CurlIssue5182(t *testing.T) {
defer os.Unsetenv("ETCDCTL_API")
defer testutil.AfterTest(t)

epc := setupEtcdctlTest(t, &configNoTLS, false)
copied := configNoTLS
copied.enableV2 = true
epc := setupEtcdctlTest(t, &copied, false)
defer func() {
if err := epc.Close(); err != nil {
t.Fatalf("error closing etcd processes (%v)", err)
Expand Down