From d99d0df5a54e4840ed0472cbefbb85741bb2904d Mon Sep 17 00:00:00 2001 From: Piotr Tabor Date: Sun, 16 May 2021 16:01:07 +0200 Subject: [PATCH] Adding etcdutl test coverage. --- etcdutl/etcdutl/common.go | 2 +- server/mvcc/backend/backend.go | 2 +- tests/e2e/ctl_v2_test.go | 39 +++++++++++++++++++------- tests/e2e/ctl_v3_alarm_test.go | 2 +- tests/e2e/ctl_v3_auth_test.go | 4 +-- tests/e2e/ctl_v3_defrag_test.go | 27 +++++++++++++++--- tests/e2e/ctl_v3_snapshot_test.go | 28 +++++++++++++------ tests/e2e/ctl_v3_test.go | 46 +++++++++++++++++++++++++++++-- tests/e2e/etcd_process.go | 8 ++++-- tests/e2e/etcd_spawn_cov.go | 2 ++ tests/e2e/etcd_spawn_nocov.go | 3 +- tests/e2e/main_test.go | 1 + 12 files changed, 131 insertions(+), 33 deletions(-) diff --git a/etcdutl/etcdutl/common.go b/etcdutl/etcdutl/common.go index f3ebe3523..4b4a198aa 100644 --- a/etcdutl/etcdutl/common.go +++ b/etcdutl/etcdutl/common.go @@ -23,7 +23,7 @@ import ( func GetLogger() *zap.Logger { config := zap.NewProductionConfig() config.Encoding = "console" - config.EncoderConfig.EncodeTime=zapcore.RFC3339TimeEncoder + config.EncoderConfig.EncodeTime = zapcore.RFC3339TimeEncoder lg, err := config.Build() if err != nil { cobrautl.ExitWithError(cobrautl.ExitBadArgs, err) diff --git a/server/mvcc/backend/backend.go b/server/mvcc/backend/backend.go index ecf941f1f..055aedaff 100644 --- a/server/mvcc/backend/backend.go +++ b/server/mvcc/backend/backend.go @@ -465,7 +465,7 @@ func (b *backend) defrag() error { size2, sizeInUse2 := b.Size(), b.SizeInUse() if b.lg != nil { b.lg.Info( - "defragmented", + "finished defragmenting directory", zap.String("path", dbp), zap.Int64("current-db-size-bytes-diff", size2-size1), zap.Int64("current-db-size-bytes", size2), diff --git a/tests/e2e/ctl_v2_test.go b/tests/e2e/ctl_v2_test.go index 075c6f880..107f1c2f7 100644 --- a/tests/e2e/ctl_v2_test.go +++ b/tests/e2e/ctl_v2_test.go @@ -15,6 +15,7 @@ package e2e import ( + "fmt" "io/ioutil" "os" "strings" @@ -210,13 +211,20 @@ func TestCtlV2RoleList(t *testing.T) { } } -func TestCtlV2Backup(t *testing.T) { testCtlV2Backup(t, 0, false) } -func TestCtlV2BackupSnapshot(t *testing.T) { testCtlV2Backup(t, 1, false) } +func TestUtlCtlV2Backup(t *testing.T) { + for snap := range []int{0, 1} { + for _, v3 := range []bool{true, false} { + for _, utl := range []bool{true, false} { + t.Run(fmt.Sprintf("etcdutl:%v;snap:%v;v3:%v", utl, snap, v3), + func(t *testing.T) { + testUtlCtlV2Backup(t, snap, v3, utl) + }) + } + } + } +} -func TestCtlV2BackupV3(t *testing.T) { testCtlV2Backup(t, 0, true) } -func TestCtlV2BackupV3Snapshot(t *testing.T) { testCtlV2Backup(t, 1, true) } - -func testCtlV2Backup(t *testing.T, snapCount int, v3 bool) { +func testUtlCtlV2Backup(t *testing.T, snapCount int, v3 bool, utl bool) { BeforeTestV2(t) backupDir, err := ioutil.TempDir(t.TempDir(), "testbackup0.etcd") @@ -251,7 +259,7 @@ func testCtlV2Backup(t *testing.T, snapCount int, v3 bool) { } } t.Log("Triggering etcd backup") - if err := etcdctlBackup(t, epc1, epc1.procs[0].Config().dataDirPath, backupDir, v3); err != nil { + if err := etcdctlBackup(t, epc1, epc1.procs[0].Config().dataDirPath, backupDir, v3, utl); err != nil { t.Fatal(err) } t.Log("Closing etcd-1 backup") @@ -377,13 +385,22 @@ func TestCtlV2ClusterHealth(t *testing.T) { func etcdctlPrefixArgs(clus *etcdProcessCluster) []string { endpoints := strings.Join(clus.EndpointsV2(), ",") - cmdArgs := []string{ctlBinPath, "--endpoints", endpoints} + cmdArgs := []string{ctlBinPath} + + cmdArgs = append(cmdArgs, "--endpoints", endpoints) if clus.cfg.clientTLS == clientTLS { cmdArgs = append(cmdArgs, "--ca-file", caPath, "--cert-file", certPath, "--key-file", privateKeyPath) } return cmdArgs } +func etcductlPrefixArgs(utl bool) []string { + if utl { + return []string{utlBinPath} + } + return []string{ctlBinPath} +} + func etcdctlClusterHealth(clus *etcdProcessCluster, val string) error { cmdArgs := append(etcdctlPrefixArgs(clus), "cluster-health") return spawnWithExpect(cmdArgs, val) @@ -480,10 +497,12 @@ func etcdctlAuthEnable(clus *etcdProcessCluster) error { return spawnWithExpect(cmdArgs, "Authentication Enabled") } -func etcdctlBackup(t testing.TB, clus *etcdProcessCluster, dataDir, backupDir string, v3 bool) error { - cmdArgs := append(etcdctlPrefixArgs(clus), "backup", "--data-dir", dataDir, "--backup-dir", backupDir) +func etcdctlBackup(t testing.TB, clus *etcdProcessCluster, dataDir, backupDir string, v3 bool, utl bool) error { + cmdArgs := append(etcductlPrefixArgs(utl), "backup", "--data-dir", dataDir, "--backup-dir", backupDir) if v3 { cmdArgs = append(cmdArgs, "--with-v3") + } else if utl { + cmdArgs = append(cmdArgs, "--with-v3=false") } t.Logf("Running: %v", cmdArgs) proc, err := spawnCmd(cmdArgs) diff --git a/tests/e2e/ctl_v3_alarm_test.go b/tests/e2e/ctl_v3_alarm_test.go index e4f500604..7b9b445b0 100644 --- a/tests/e2e/ctl_v3_alarm_test.go +++ b/tests/e2e/ctl_v3_alarm_test.go @@ -84,7 +84,7 @@ func alarmTest(cx ctlCtx) { if err := ctlV3Compact(cx, sresp.Header.Revision, true); err != nil { cx.t.Fatal(err) } - if err := ctlV3Defrag(cx); err != nil { + if err := ctlV3OnlineDefrag(cx); err != nil { cx.t.Fatal(err) } diff --git a/tests/e2e/ctl_v3_auth_test.go b/tests/e2e/ctl_v3_auth_test.go index bee66e6d9..58a3b61e0 100644 --- a/tests/e2e/ctl_v3_auth_test.go +++ b/tests/e2e/ctl_v3_auth_test.go @@ -994,13 +994,13 @@ func authTestDefrag(cx ctlCtx) { // ordinary user cannot defrag cx.user, cx.pass = "test-user", "pass" - if err := ctlV3Defrag(cx); err == nil { + if err := ctlV3OnlineDefrag(cx); err == nil { cx.t.Fatal("ordinary user should not be able to issue a defrag request") } // root can defrag cx.user, cx.pass = "root", "root" - if err := ctlV3Defrag(cx); err != nil { + if err := ctlV3OnlineDefrag(cx); err != nil { cx.t.Fatal(err) } } diff --git a/tests/e2e/ctl_v3_defrag_test.go b/tests/e2e/ctl_v3_defrag_test.go index 64c3bb9f0..8fbe476f0 100644 --- a/tests/e2e/ctl_v3_defrag_test.go +++ b/tests/e2e/ctl_v3_defrag_test.go @@ -16,7 +16,14 @@ package e2e import "testing" -func TestCtlV3Defrag(t *testing.T) { testCtl(t, defragTest) } +func TestCtlV3DefragOnline(t *testing.T) { testCtl(t, defragOnlineTest) } + +func TestCtlV3DefragOffline(t *testing.T) { + testCtlWithOffline(t, maintenanceInitKeys, defragOfflineTest) +} +func TestCtlV3DefragOfflineEtcdutl(t *testing.T) { + testCtlWithOffline(t, maintenanceInitKeys, defragOfflineTest, withEtcdutl()) +} func maintenanceInitKeys(cx ctlCtx) { var kvs = []kv{{"key", "val1"}, {"key", "val2"}, {"key", "val3"}} @@ -27,19 +34,19 @@ func maintenanceInitKeys(cx ctlCtx) { } } -func defragTest(cx ctlCtx) { +func defragOnlineTest(cx ctlCtx) { maintenanceInitKeys(cx) if err := ctlV3Compact(cx, 4, cx.compactPhysical); err != nil { cx.t.Fatal(err) } - if err := ctlV3Defrag(cx); err != nil { + if err := ctlV3OnlineDefrag(cx); err != nil { cx.t.Fatalf("defragTest ctlV3Defrag error (%v)", err) } } -func ctlV3Defrag(cx ctlCtx) error { +func ctlV3OnlineDefrag(cx ctlCtx) error { cmdArgs := append(cx.PrefixArgs(), "defrag") lines := make([]string, cx.epc.cfg.clusterSize) for i := range lines { @@ -47,3 +54,15 @@ func ctlV3Defrag(cx ctlCtx) error { } return spawnWithExpects(cmdArgs, lines...) } + +func ctlV3OfflineDefrag(cx ctlCtx) error { + cmdArgs := append(cx.PrefixArgsUtl(), "defrag", "--data-dir", cx.dataDir) + lines := []string{"finished defragmenting directory"} + return spawnWithExpects(cmdArgs, lines...) +} + +func defragOfflineTest(cx ctlCtx) { + if err := ctlV3OfflineDefrag(cx); err != nil { + cx.t.Fatalf("defragTest ctlV3Defrag error (%v)", err) + } +} diff --git a/tests/e2e/ctl_v3_snapshot_test.go b/tests/e2e/ctl_v3_snapshot_test.go index c2a4d0018..ce172f4c1 100644 --- a/tests/e2e/ctl_v3_snapshot_test.go +++ b/tests/e2e/ctl_v3_snapshot_test.go @@ -28,7 +28,8 @@ import ( "go.etcd.io/etcd/pkg/v3/expect" ) -func TestCtlV3Snapshot(t *testing.T) { testCtl(t, snapshotTest) } +func TestCtlV3Snapshot(t *testing.T) { testCtl(t, snapshotTest) } +func TestCtlV3SnapshotEtcdutl(t *testing.T) { testCtl(t, snapshotTest, withEtcdutl()) } func snapshotTest(cx ctlCtx) { maintenanceInitKeys(cx) @@ -60,7 +61,8 @@ func snapshotTest(cx ctlCtx) { } } -func TestCtlV3SnapshotCorrupt(t *testing.T) { testCtl(t, snapshotCorruptTest) } +func TestCtlV3SnapshotCorrupt(t *testing.T) { testCtl(t, snapshotCorruptTest) } +func TestCtlV3SnapshotCorruptEtcdutl(t *testing.T) { testCtl(t, snapshotCorruptTest, withEtcdutl()) } func snapshotCorruptTest(cx ctlCtx) { fpath := filepath.Join(cx.t.TempDir(), "snapshot") @@ -81,10 +83,9 @@ func snapshotCorruptTest(cx ctlCtx) { f.Close() datadir := cx.t.TempDir() - defer os.RemoveAll(datadir) serr := spawnWithExpect( - append(cx.PrefixArgs(), "snapshot", "restore", + append(cx.PrefixArgsUtl(), "snapshot", "restore", "--data-dir", datadir, fpath), "expected sha256") @@ -96,6 +97,9 @@ func snapshotCorruptTest(cx ctlCtx) { // This test ensures that the snapshot status does not modify the snapshot file func TestCtlV3SnapshotStatusBeforeRestore(t *testing.T) { testCtl(t, snapshotStatusBeforeRestoreTest) } +func TestCtlV3SnapshotStatusBeforeRestoreEtcdutl(t *testing.T) { + testCtl(t, snapshotStatusBeforeRestoreTest, withEtcdutl()) +} func snapshotStatusBeforeRestoreTest(cx ctlCtx) { fpath := filepath.Join(cx.t.TempDir(), "snapshot") @@ -114,7 +118,7 @@ func snapshotStatusBeforeRestoreTest(cx ctlCtx) { dataDir := cx.t.TempDir() defer os.RemoveAll(dataDir) serr := spawnWithExpect( - append(cx.PrefixArgs(), "snapshot", "restore", + append(cx.PrefixArgsUtl(), "snapshot", "restore", "--data-dir", dataDir, fpath), "added member") @@ -129,7 +133,7 @@ func ctlV3SnapshotSave(cx ctlCtx, fpath string) error { } func getSnapshotStatus(cx ctlCtx, fpath string) (snapshot.Status, error) { - cmdArgs := append(cx.PrefixArgs(), "--write-out", "json", "snapshot", "status", fpath) + cmdArgs := append(cx.PrefixArgsUtl(), "--write-out", "json", "snapshot", "status", fpath) proc, err := spawnCmd(cmdArgs) if err != nil { @@ -152,9 +156,12 @@ func getSnapshotStatus(cx ctlCtx, fpath string) (snapshot.Status, error) { return resp, nil } +func TestIssue6361(t *testing.T) { testIssue6361(t, false) } +func TestIssue6361etcdutl(t *testing.T) { testIssue6361(t, true) } + // TestIssue6361 ensures new member that starts with snapshot correctly // syncs up with other members and serve correct data. -func TestIssue6361(t *testing.T) { +func testIssue6361(t *testing.T, etcdutl bool) { { // This tests is pretty flaky on semaphoreci as of 2021-01-10. // TODO: Remove when the flakiness source is identified. @@ -206,8 +213,13 @@ func TestIssue6361(t *testing.T) { newDataDir := filepath.Join(t.TempDir(), "test.data") + uctlBinPath := ctlBinPath + if etcdutl { + uctlBinPath = utlBinPath + } + t.Log("etcdctl restoring the snapshot...") - err = spawnWithExpect([]string{ctlBinPath, "snapshot", "restore", fpath, "--name", epc.procs[0].Config().name, "--initial-cluster", epc.procs[0].Config().initialCluster, "--initial-cluster-token", epc.procs[0].Config().initialToken, "--initial-advertise-peer-urls", epc.procs[0].Config().purl.String(), "--data-dir", newDataDir}, "added member") + err = spawnWithExpect([]string{uctlBinPath, "snapshot", "restore", fpath, "--name", epc.procs[0].Config().name, "--initial-cluster", epc.procs[0].Config().initialCluster, "--initial-cluster-token", epc.procs[0].Config().initialToken, "--initial-advertise-peer-urls", epc.procs[0].Config().purl.String(), "--data-dir", newDataDir}, "added member") if err != nil { t.Fatal(err) } diff --git a/tests/e2e/ctl_v3_test.go b/tests/e2e/ctl_v3_test.go index f6eeeecbf..5c8bb2fe9 100644 --- a/tests/e2e/ctl_v3_test.go +++ b/tests/e2e/ctl_v3_test.go @@ -21,6 +21,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" "go.etcd.io/etcd/api/v3/version" "go.etcd.io/etcd/client/pkg/v3/fileutil" "go.etcd.io/etcd/client/pkg/v3/testutil" @@ -142,6 +143,12 @@ type ctlCtx struct { // for compaction compactPhysical bool + + // to run etcdutl instead of etcdctl for suitable commands. + etcdutl bool + + // dir that was used during the test + dataDir string } type ctlOption func(*ctlCtx) @@ -197,7 +204,15 @@ func withFlagByEnv() ctlOption { return func(cx *ctlCtx) { cx.envMap = make(map[string]struct{}) } } +func withEtcdutl() ctlOption { + return func(cx *ctlCtx) { cx.etcdutl = true } +} + func testCtl(t *testing.T, testFunc func(ctlCtx), opts ...ctlOption) { + testCtlWithOffline(t, testFunc, nil, opts...) +} + +func testCtlWithOffline(t *testing.T, testFunc func(ctlCtx), testOfflineFunc func(ctlCtx), opts ...ctlOption) { BeforeTest(t) ret := ctlCtx{ @@ -217,12 +232,16 @@ func testCtl(t *testing.T, testFunc func(ctlCtx), opts ...ctlOption) { if ret.initialCorruptCheck { ret.cfg.initialCorruptCheck = ret.initialCorruptCheck } + if testOfflineFunc != nil { + ret.cfg.keepDataDir = true + } epc, err := newEtcdProcessCluster(t, &ret.cfg) if err != nil { t.Fatalf("could not start etcd process cluster (%v)", err) } ret.epc = epc + ret.dataDir = epc.procs[0].Config().dataDirPath defer func() { if ret.envMap != nil { @@ -230,8 +249,10 @@ func testCtl(t *testing.T, testFunc func(ctlCtx), opts ...ctlOption) { os.Unsetenv(k) } } - if errC := ret.epc.Close(); errC != nil { - t.Fatalf("error closing etcd processes (%v)", errC) + if ret.epc != nil { + if errC := ret.epc.Close(); errC != nil { + t.Fatalf("error closing etcd processes (%v)", errC) + } } }() @@ -239,6 +260,7 @@ func testCtl(t *testing.T, testFunc func(ctlCtx), opts ...ctlOption) { go func() { defer close(donec) testFunc(ret) + t.Log("---testFunc logic DONE") }() timeout := 2*ret.dialTimeout + time.Second @@ -250,7 +272,15 @@ func testCtl(t *testing.T, testFunc func(ctlCtx), opts ...ctlOption) { testutil.FatalStack(t, fmt.Sprintf("test timed out after %v", timeout)) case <-donec: } - t.Log("---Test logic DONE") + + t.Log("closing test cluster...") + assert.NoError(t, epc.Close()) + epc = nil + t.Log("closed test cluster...") + + if testOfflineFunc != nil { + testOfflineFunc(ret) + } } func (cx *ctlCtx) prefixArgs(eps []string) []string { @@ -296,6 +326,16 @@ func (cx *ctlCtx) PrefixArgs() []string { return cx.prefixArgs(cx.epc.EndpointsV3()) } +// PrefixArgsUtl returns prefix of the command that is either etcdctl or etcdutl +// depending on cx configuration. +// Please not thet 'utl' compatible commands does not consume --endpoints flag. +func (cx *ctlCtx) PrefixArgsUtl() []string { + if cx.etcdutl { + return []string{utlBinPath} + } + return []string{ctlBinPath} +} + func isGRPCTimedout(err error) bool { return strings.Contains(err.Error(), "grpc: timed out trying to connect") } diff --git a/tests/e2e/etcd_process.go b/tests/e2e/etcd_process.go index 67c293fe5..f744fa81c 100644 --- a/tests/e2e/etcd_process.go +++ b/tests/e2e/etcd_process.go @@ -28,6 +28,7 @@ var ( etcdServerReadyLines = []string{"ready to serve client requests"} binPath string ctlBinPath string + utlBinPath string ) // etcdProcess is a process that serves etcd requests. @@ -143,8 +144,11 @@ func (ep *etcdServerProcess) Close() error { if err := ep.Stop(); err != nil { return err } - ep.cfg.lg.Info("removing directory", zap.String("data-dir", ep.cfg.dataDirPath)) - return os.RemoveAll(ep.cfg.dataDirPath) + if !ep.cfg.keepDataDir { + ep.cfg.lg.Info("removing directory", zap.String("data-dir", ep.cfg.dataDirPath)) + return os.RemoveAll(ep.cfg.dataDirPath) + } + return nil } func (ep *etcdServerProcess) WithStopSignal(sig os.Signal) os.Signal { diff --git a/tests/e2e/etcd_spawn_cov.go b/tests/e2e/etcd_spawn_cov.go index 3bfd8433d..9b24ac9d0 100644 --- a/tests/e2e/etcd_spawn_cov.go +++ b/tests/e2e/etcd_spawn_cov.go @@ -48,6 +48,8 @@ func spawnCmdWithLogger(lg *zap.Logger, args []string) (*expect.ExpectProcess, e cmd = cmd + "_test" case strings.HasSuffix(cmd, "/etcdctl"): cmd = cmd + "_test" + case strings.HasSuffix(cmd, "/etcdutl"): + cmd = cmd + "_test" case strings.HasSuffix(cmd, "/etcdctl3"): cmd = ctlBinPath + "_test" env = append(env, "ETCDCTL_API=3") diff --git a/tests/e2e/etcd_spawn_nocov.go b/tests/e2e/etcd_spawn_nocov.go index e753a967f..b0e872fb2 100644 --- a/tests/e2e/etcd_spawn_nocov.go +++ b/tests/e2e/etcd_spawn_nocov.go @@ -19,6 +19,7 @@ package e2e import ( "os" + "strings" "go.etcd.io/etcd/pkg/v3/expect" "go.uber.org/zap" @@ -35,7 +36,7 @@ func spawnCmdWithLogger(lg *zap.Logger, args []string) (*expect.ExpectProcess, e if err != nil { return nil, err } - if args[0] == ctlBinPath+"3" { + if strings.HasSuffix(args[0], "/etcdctl3") { env := append(os.Environ(), "ETCDCTL_API=3") lg.Info("spawning process with ETCDCTL_API=3", zap.Strings("args", args), zap.String("working-dir", wd)) return expect.NewExpectWithEnv(ctlBinPath, args[1:], env) diff --git a/tests/e2e/main_test.go b/tests/e2e/main_test.go index ddb7ae4ba..41561b550 100644 --- a/tests/e2e/main_test.go +++ b/tests/e2e/main_test.go @@ -46,6 +46,7 @@ func TestMain(m *testing.M) { binPath = binDir + "/etcd" ctlBinPath = binDir + "/etcdctl" + utlBinPath = binDir + "/etcdutl" certPath = certDir + "/server.crt" privateKeyPath = certDir + "/server.key.insecure" caPath = certDir + "/ca.crt"