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

server: Make --v2-deprecation=write-only the default and remove not-y… #13612

Merged
merged 1 commit into from
Feb 14, 2022
Merged

server: Make --v2-deprecation=write-only the default and remove not-y… #13612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions server/config/v2_deprecation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ package config
type V2DeprecationEnum string

const (
// Default in v3.5. Issues a warning if v2store have meaningful content.
// No longer supported in v3.6
V2_DEPR_0_NOT_YET = V2DeprecationEnum("not-yet")
Copy link
Member

Choose a reason for hiding this comment

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

If it is not supported in 3.6, can we just remove it and remove the switch case at line 40?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm also usually for removing the dead code, however in this case I think it's worth to leave for historical context. Of course when V2 is fully removed this whole file should be deleted, however I think it's worth to keep this so that someone reading this file understands all previous enum stages.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, it isn't a big deal. Feel free to keep it unchanged.

It's OK because this file will eventually be removed.

// Default in v3.6. Meaningful v2 state is not allowed.
// The V2 files are maintained for v3.5 rollback.
Expand All @@ -28,7 +28,7 @@ const (
// ability to rollback to etcd v3.5.
V2_DEPR_2_GONE = V2DeprecationEnum("gone")

V2_DEPR_DEFAULT = V2_DEPR_0_NOT_YET
V2_DEPR_DEFAULT = V2_DEPR_1_WRITE_ONLY
)

func (e V2DeprecationEnum) IsAtLeast(v2d V2DeprecationEnum) bool {
Expand Down
1 change: 0 additions & 1 deletion server/etcdmain/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ func newConfig() *config {
proxyFlagOn,
),
v2deprecation: flags.NewSelectiveStringsValue(
string(cconfig.V2_DEPR_0_NOT_YET),
string(cconfig.V2_DEPR_1_WRITE_ONLY),
string(cconfig.V2_DEPR_1_WRITE_ONLY_DROP),
string(cconfig.V2_DEPR_2_GONE)),
Expand Down
29 changes: 13 additions & 16 deletions tests/e2e/v2store_deprecation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,6 @@ func createV2store(t testing.TB, lastReleaseBinary string, dataDirPath string) {
}
}

func assertVerifyCanStartV2deprecationNotYet(t testing.TB, dataDirPath string) {
t.Log("verify: possible to start etcd with --v2-deprecation=not-yet mode")

cfg := e2e.ConfigStandalone(e2e.EtcdProcessClusterConfig{DataDirPath: dataDirPath, V2deprecation: "not-yet", KeepDataDir: true})
epc, err := e2e.NewEtcdProcessCluster(t, cfg)
assert.NoError(t, err)

defer func() {
assert.NoError(t, epc.Stop())
}()
}

func assertVerifyCannotStartV2deprecationWriteOnly(t testing.TB, dataDirPath string) {
t.Log("Verify its infeasible to start etcd with --v2-deprecation=write-only mode")
proc, err := e2e.SpawnCmd([]string{e2e.BinDir + "/etcd", "--v2-deprecation=write-only", "--data-dir=" + dataDirPath}, nil)
Expand All @@ -65,6 +53,15 @@ func assertVerifyCannotStartV2deprecationWriteOnly(t testing.TB, dataDirPath str
assert.NoError(t, err)
}

func assertVerifyCannotStartV2deprecationNotYet(t testing.TB, dataDirPath string) {
t.Log("Verify its infeasible to start etcd with --v2-deprecation=not-yet mode")
proc, err := e2e.SpawnCmd([]string{e2e.BinDir + "/etcd", "--v2-deprecation=not-yet", "--data-dir=" + dataDirPath}, nil)
assert.NoError(t, err)

_, err = proc.Expect(`invalid value "not-yet" for flag -v2-deprecation: invalid value "not-yet"`)
assert.NoError(t, err)
}

func TestV2Deprecation(t *testing.T) {
e2e.BeforeTest(t)
dataDirPath := t.TempDir()
Expand All @@ -78,12 +75,12 @@ func TestV2Deprecation(t *testing.T) {
createV2store(t, lastReleaseBinary, dataDirPath)
})

t.Run("--v2-deprecation=write-only fails", func(t *testing.T) {
assertVerifyCannotStartV2deprecationWriteOnly(t, dataDirPath)
t.Run("--v2-deprecation=not-yet fails", func(t *testing.T) {
assertVerifyCannotStartV2deprecationNotYet(t, dataDirPath)
})

t.Run("--v2-deprecation=not-yet succeeds", func(t *testing.T) {
assertVerifyCanStartV2deprecationNotYet(t, dataDirPath)
t.Run("--v2-deprecation=write-only fails", func(t *testing.T) {
assertVerifyCannotStartV2deprecationWriteOnly(t, dataDirPath)
})

}