From 6d20c9031156907c1570f5310adb87f750817548 Mon Sep 17 00:00:00 2001 From: Ashwani Kumar Date: Wed, 3 Jul 2019 16:26:02 +0530 Subject: [PATCH 1/4] Enhanced the test coverage for restorer --- .ci/unit_test | 16 +- pkg/miscellaneous/miscellaneous.go | 1 + pkg/snapshot/restorer/restorer_suite_test.go | 31 +- pkg/snapshot/restorer/restorer_test.go | 641 ++++++++++++++++--- 4 files changed, 584 insertions(+), 105 deletions(-) diff --git a/.ci/unit_test b/.ci/unit_test index 3afe97c4c..02c53591e 100755 --- a/.ci/unit_test +++ b/.ci/unit_test @@ -39,8 +39,8 @@ function test_with_coverage() { local output_dir=test/output local coverprofile_file=coverprofile.out mkdir -p test/output - ginkgo $GINKGO_COMMON_FLAGS --coverprofile ${coverprofile_file} -covermode=set -outputdir ${output_dir} ${TEST_PACKAGES} - sed -i '/mode: set/d' ${output_dir}/${coverprofile_file} + ginkgo $GINKGO_COMMON_FLAGS --coverprofile ${coverprofile_file} -covermode=set -outputdir ${output_dir} ${TEST_PACKAGES} + sed -i='' '/mode: set/d' ${output_dir}/${coverprofile_file} {( echo "mode: set"; cat ${output_dir}/${coverprofile_file} )} > ${output_dir}/${coverprofile_file}.temp mv ${output_dir}/${coverprofile_file}.temp ${output_dir}/${coverprofile_file} go tool cover -func ${output_dir}/${coverprofile_file} @@ -49,10 +49,18 @@ function test_with_coverage() { ################################################################################ TEST_PACKAGES="cmd pkg" -GINKGO_COMMON_FLAGS="-r -timeout=1h0m0s --randomizeAllSpecs --randomizeSuites --failOnPending --progress" +GINKGO_COMMON_FLAGS="-r -timeout=1h0m0s --progress" + if [ -z $COVER ] || [ "$COVER" = false ] ; then echo "[INFO] Test coverage is disabled." - ginkgo -race -trace $GINKGO_COMMON_FLAGS ${TEST_PACKAGES} + + # run everything which is not part of negative scenarios with randomizeAllSpecs parameters. + ginkgo -race -trace $GINKGO_COMMON_FLAGS --randomizeAllSpecs --randomizeSuites --failOnPending --skip="NEGATIVE\:.*" ${TEST_PACKAGES} + + + #run negative scenario in a sequenced manner (removed failOnPending as one spec in restore test is marked as 'X' for excluding) + ginkgo -race -trace $GINKGO_COMMON_FLAGS --focus="NEGATIVE\:.*" ${TEST_PACKAGES} + else test_with_coverage fi diff --git a/pkg/miscellaneous/miscellaneous.go b/pkg/miscellaneous/miscellaneous.go index 35465af04..72a7288ff 100644 --- a/pkg/miscellaneous/miscellaneous.go +++ b/pkg/miscellaneous/miscellaneous.go @@ -38,5 +38,6 @@ func GetLatestFullSnapshotAndDeltaSnapList(store snapstore.SnapStore) (*snapstor } deltaSnapList = append(deltaSnapList, snapList[index-1]) } + sort.Sort(deltaSnapList) //added to ensure the list is well formed for only deltasnapshots scenarios as well return nil, deltaSnapList, nil } diff --git a/pkg/snapshot/restorer/restorer_suite_test.go b/pkg/snapshot/restorer/restorer_suite_test.go index 53dbd3995..0c503f733 100644 --- a/pkg/snapshot/restorer/restorer_suite_test.go +++ b/pkg/snapshot/restorer/restorer_suite_test.go @@ -17,6 +17,7 @@ package restorer_test import ( "context" "fmt" + "math" "os" "strconv" "sync" @@ -38,7 +39,7 @@ const ( etcdDir = outputDir + "/default.etcd" snapstoreDir = outputDir + "/snapshotter.bkp" etcdEndpoint = "http://localhost:2379" - snapshotterDurationSeconds = 100 + snapshotterDurationSeconds = 20 keyPrefix = "key-" valuePrefix = "val-" keyFrom = 1 @@ -72,7 +73,7 @@ var _ = SynchronizedBeforeSuite(func() []byte { etcd, err = startEmbeddedEtcd(etcdDir, logger) Expect(err).ShouldNot(HaveOccurred()) wg := &sync.WaitGroup{} - deltaSnapshotPeriod := 5 + deltaSnapshotPeriod := 1 wg.Add(1) go populateEtcd(wg, logger, endpoints, errCh, populatorStopCh) go func() { @@ -83,21 +84,24 @@ var _ = SynchronizedBeforeSuite(func() []byte { close(ssrStopCh) }() - err = runSnapshotter(logger, deltaSnapshotPeriod, endpoints, ssrStopCh) + err = runSnapshotter(logger, deltaSnapshotPeriod, endpoints, ssrStopCh, true) Expect(err).ShouldNot(HaveOccurred()) etcd.Server.Stop() etcd.Close() return data + }, func(data []byte) {}) -var _ = SynchronizedAfterSuite(func() {}, func() { +var _ = SynchronizedAfterSuite(func() {}, cleanUp) + +func cleanUp() { err = os.RemoveAll(etcdDir) Expect(err).ShouldNot(HaveOccurred()) err = os.RemoveAll(snapstoreDir) Expect(err).ShouldNot(HaveOccurred()) -}) +} // startEmbeddedEtcd starts an embedded etcd server func startEmbeddedEtcd(dir string, logger *logrus.Logger) (*embed.Etcd, error) { @@ -124,7 +128,7 @@ func startEmbeddedEtcd(dir string, logger *logrus.Logger) (*embed.Etcd, error) { } // runSnapshotter creates a snapshotter object and runs it for a duration specified by 'snapshotterDurationSeconds' -func runSnapshotter(logger *logrus.Logger, deltaSnapshotPeriod int, endpoints []string, stopCh chan struct{}) error { +func runSnapshotter(logger *logrus.Logger, deltaSnapshotPeriod int, endpoints []string, stopCh chan struct{}, startWithFullSnapshot bool) error { var ( store snapstore.SnapStore certFile string @@ -177,7 +181,7 @@ func runSnapshotter(logger *logrus.Logger, deltaSnapshotPeriod int, endpoints [] snapshotterConfig, ) - return ssr.Run(stopCh, true) + return ssr.Run(stopCh, startWithFullSnapshot) } // populateEtcd sequentially puts key-value pairs into the embedded etcd, until stopped @@ -218,6 +222,19 @@ func populateEtcd(wg *sync.WaitGroup, logger *logrus.Logger, endpoints []string, return } time.Sleep(time.Second * 1) + //call a delete for every 10th Key after putting it in the store to check deletes in consistency check + // handles deleted keys as every 10th key is deleted during populate etcd call + // this handling is also done in the checkDataConsistency() in restorer_test.go file + // also it assumes that the deltaSnapshotDuration is more than 10 -- + // if you change the constant please change the factor accordingly to have coverage of delete scenarios. + if math.Mod(float64(currKey), 10) == 0 { + _, err = cli.Delete(context.TODO(), key) + if err != nil { + errCh <- fmt.Errorf("unable to delete key (%s) from embedded etcd: %v", key, err) + return + } + } + } } } diff --git a/pkg/snapshot/restorer/restorer_test.go b/pkg/snapshot/restorer/restorer_test.go index 9a2e1fbdb..6a015971b 100644 --- a/pkg/snapshot/restorer/restorer_test.go +++ b/pkg/snapshot/restorer/restorer_test.go @@ -17,8 +17,11 @@ package restorer_test import ( "context" "fmt" + "math" "os" + "path" "strconv" + "sync" "time" "github.com/coreos/etcd/clientv3" @@ -52,10 +55,14 @@ var _ = Describe("Running Restorer", func() { peerUrls types.URLs baseSnapshot *snapstore.Snapshot deltaSnapList snapstore.SnapList + wg *sync.WaitGroup + + errCh chan error + populatorStopCh chan bool + ssrStopCh chan struct{} ) BeforeEach(func() { - fmt.Println("Initializing snapstore and restorer") logger = logrus.New() restoreDataDir = etcdDir @@ -71,116 +78,554 @@ var _ = Describe("Running Restorer", func() { maxFetchers = 6 embeddedEtcdQuotaBytes = 8 * 1024 * 1024 * 1024 - err = corruptEtcdDir() - Expect(err).ShouldNot(HaveOccurred()) + }) - store, err = snapstore.GetSnapstore(&snapstore.Config{Container: snapstoreDir, Provider: "Local"}) - Expect(err).ShouldNot(HaveOccurred()) + Describe("For pre-loaded Snapstore", func() { + BeforeEach(func() { + fmt.Println("Initializing snapstore and restorer") + errCh = make(chan error) + populatorStopCh = make(chan bool) + ssrStopCh = make(chan struct{}) - baseSnapshot, deltaSnapList, err = miscellaneous.GetLatestFullSnapshotAndDeltaSnapList(store) - Expect(err).ShouldNot(HaveOccurred()) + err = corruptEtcdDir() + Expect(err).ShouldNot(HaveOccurred()) - rstr = NewRestorer(store, logger) - }) + store, err = snapstore.GetSnapstore(&snapstore.Config{Container: snapstoreDir, Provider: "Local"}) + Expect(err).ShouldNot(HaveOccurred()) - Context("with zero fetchers", func() { - fmt.Println("Testing for max-fetchers=0") - It("should return error", func() { - maxFetchers = 0 - - restoreOptions := RestoreOptions{ - ClusterURLs: clusterUrlsMap, - ClusterToken: restoreClusterToken, - RestoreDataDir: restoreDataDir, - PeerURLs: peerUrls, - SkipHashCheck: skipHashCheck, - Name: restoreName, - MaxFetchers: maxFetchers, - EmbeddedEtcdQuotaBytes: embeddedEtcdQuotaBytes, - BaseSnapshot: *baseSnapshot, - DeltaSnapList: deltaSnapList, - } - err = rstr.Restore(restoreOptions) - Expect(err).Should(HaveOccurred()) + baseSnapshot, deltaSnapList, err = miscellaneous.GetLatestFullSnapshotAndDeltaSnapList(store) + Expect(err).ShouldNot(HaveOccurred()) + + rstr = NewRestorer(store, logger) }) - }) - Context("with maximum of one fetcher allowed", func() { - fmt.Println("Testing for max-fetchers=1") - It("should restore etcd data directory", func() { - maxFetchers = 1 - - restoreOptions := RestoreOptions{ - ClusterURLs: clusterUrlsMap, - ClusterToken: restoreClusterToken, - RestoreDataDir: restoreDataDir, - PeerURLs: peerUrls, - SkipHashCheck: skipHashCheck, - Name: restoreName, - MaxFetchers: maxFetchers, - EmbeddedEtcdQuotaBytes: embeddedEtcdQuotaBytes, - BaseSnapshot: *baseSnapshot, - DeltaSnapList: deltaSnapList, - } - err = rstr.Restore(restoreOptions) - Expect(err).ShouldNot(HaveOccurred()) + Context("with zero fetchers", func() { + fmt.Println("Testing for max-fetchers=0") + It("should return error", func() { + maxFetchers = 0 + + restoreOptions := RestoreOptions{ + ClusterURLs: clusterUrlsMap, + ClusterToken: restoreClusterToken, + RestoreDataDir: restoreDataDir, + PeerURLs: peerUrls, + SkipHashCheck: skipHashCheck, + Name: restoreName, + MaxFetchers: maxFetchers, + EmbeddedEtcdQuotaBytes: embeddedEtcdQuotaBytes, + BaseSnapshot: *baseSnapshot, + DeltaSnapList: deltaSnapList, + } + err = rstr.Restore(restoreOptions) + Expect(err).Should(HaveOccurred()) + }) + }) - err = checkDataConsistency(restoreDataDir, logger) - Expect(err).ShouldNot(HaveOccurred()) + Context("with embedded etcd quota not set", func() { + fmt.Println("Testing for default embedded etcd quota") + It("should be set to defalut value of 8 GB and restore", func() { + embeddedEtcdQuotaBytes = 0 + + RestoreOptions := RestoreOptions{ + ClusterURLs: clusterUrlsMap, + ClusterToken: restoreClusterToken, + RestoreDataDir: restoreDataDir, + PeerURLs: peerUrls, + SkipHashCheck: skipHashCheck, + Name: restoreName, + MaxFetchers: maxFetchers, + EmbeddedEtcdQuotaBytes: embeddedEtcdQuotaBytes, + BaseSnapshot: *baseSnapshot, + DeltaSnapList: deltaSnapList, + } + err = rstr.Restore(RestoreOptions) + Expect(err).ShouldNot(HaveOccurred()) + + }) }) - }) - Context("with maximum of four fetchers allowed", func() { - fmt.Println("Testing for max-fetchers=4") - It("should restore etcd data directory", func() { - maxFetchers = 4 - - restoreOptions := RestoreOptions{ - ClusterURLs: clusterUrlsMap, - ClusterToken: restoreClusterToken, - RestoreDataDir: restoreDataDir, - PeerURLs: peerUrls, - SkipHashCheck: skipHashCheck, - Name: restoreName, - MaxFetchers: maxFetchers, - EmbeddedEtcdQuotaBytes: embeddedEtcdQuotaBytes, - BaseSnapshot: *baseSnapshot, - DeltaSnapList: deltaSnapList, - } - err = rstr.Restore(restoreOptions) - Expect(err).ShouldNot(HaveOccurred()) + Context("with invalid cluster URLS", func() { + fmt.Println("Testing for invalid cluster URLS") + It("should fail with an error ", func() { + restoreCluster = restoreName + "=http://localhost:2390" + restorePeerURLs = []string{"http://localhost:2390"} + clusterUrlsMap, err = types.NewURLsMap(restoreCluster) + + RestoreOptions := RestoreOptions{ + ClusterURLs: clusterUrlsMap, + ClusterToken: restoreClusterToken, + RestoreDataDir: restoreDataDir, + PeerURLs: peerUrls, + SkipHashCheck: skipHashCheck, + Name: restoreName, + MaxFetchers: maxFetchers, + EmbeddedEtcdQuotaBytes: embeddedEtcdQuotaBytes, + BaseSnapshot: *baseSnapshot, + DeltaSnapList: deltaSnapList, + } + err = rstr.Restore(RestoreOptions) + Expect(err).Should(HaveOccurred()) + + }) + }) - err = checkDataConsistency(restoreDataDir, logger) - Expect(err).ShouldNot(HaveOccurred()) + Context("with invalid restore directory", func() { + fmt.Println("Testing for invalid restore directory") + It("should fail to restore", func() { + + restoreOptions := RestoreOptions{ + ClusterURLs: clusterUrlsMap, + ClusterToken: restoreClusterToken, + RestoreDataDir: "", //restoreDataDir, + PeerURLs: peerUrls, + SkipHashCheck: skipHashCheck, + Name: restoreName, + MaxFetchers: maxFetchers, + EmbeddedEtcdQuotaBytes: embeddedEtcdQuotaBytes, + BaseSnapshot: *baseSnapshot, + DeltaSnapList: deltaSnapList, + } + err = rstr.Restore(restoreOptions) + Expect(err).Should(HaveOccurred()) + + }) + }) + + Context("with invalid snapdir and snapname", func() { + fmt.Println("Testing for invalid snapdir and snapname") + It("should fail to restore", func() { + + restoreOptions := RestoreOptions{ + ClusterURLs: clusterUrlsMap, + ClusterToken: restoreClusterToken, + RestoreDataDir: restoreDataDir, + PeerURLs: peerUrls, + SkipHashCheck: skipHashCheck, + Name: restoreName, + MaxFetchers: maxFetchers, + EmbeddedEtcdQuotaBytes: embeddedEtcdQuotaBytes, + BaseSnapshot: *baseSnapshot, + DeltaSnapList: deltaSnapList, + } + restoreOptions.BaseSnapshot.SnapDir = "test" + restoreOptions.BaseSnapshot.SnapName = "test" + err := rstr.Restore(restoreOptions) + Expect(err).Should(HaveOccurred()) + + }) + }) + + Context("with maximum of one fetcher allowed", func() { + fmt.Println("Testing for max-fetchers=1") + It("should restore etcd data directory", func() { + maxFetchers = 1 + + restoreOptions := RestoreOptions{ + ClusterURLs: clusterUrlsMap, + ClusterToken: restoreClusterToken, + RestoreDataDir: restoreDataDir, + PeerURLs: peerUrls, + SkipHashCheck: skipHashCheck, + Name: restoreName, + MaxFetchers: maxFetchers, + EmbeddedEtcdQuotaBytes: embeddedEtcdQuotaBytes, + BaseSnapshot: *baseSnapshot, + DeltaSnapList: deltaSnapList, + } + err = rstr.Restore(restoreOptions) + Expect(err).ShouldNot(HaveOccurred()) + + err = checkDataConsistency(restoreDataDir, logger) + Expect(err).ShouldNot(HaveOccurred()) + }) }) + + Context("with maximum of four fetchers allowed", func() { + fmt.Println("Testing for max-fetchers=4") + It("should restore etcd data directory", func() { + maxFetchers = 4 + + restoreOptions := RestoreOptions{ + ClusterURLs: clusterUrlsMap, + ClusterToken: restoreClusterToken, + RestoreDataDir: restoreDataDir, + PeerURLs: peerUrls, + SkipHashCheck: skipHashCheck, + Name: restoreName, + MaxFetchers: maxFetchers, + EmbeddedEtcdQuotaBytes: embeddedEtcdQuotaBytes, + BaseSnapshot: *baseSnapshot, + DeltaSnapList: deltaSnapList, + } + err = rstr.Restore(restoreOptions) + Expect(err).ShouldNot(HaveOccurred()) + + err = checkDataConsistency(restoreDataDir, logger) + Expect(err).ShouldNot(HaveOccurred()) + }) + }) + + Context("with maximum of hundred fetchers allowed", func() { + fmt.Println("Testing for max-fetchers=100") + It("should restore etcd data directory", func() { + maxFetchers = 100 + + restoreOptions := RestoreOptions{ + ClusterURLs: clusterUrlsMap, + ClusterToken: restoreClusterToken, + RestoreDataDir: restoreDataDir, + PeerURLs: peerUrls, + SkipHashCheck: skipHashCheck, + Name: restoreName, + MaxFetchers: maxFetchers, + EmbeddedEtcdQuotaBytes: embeddedEtcdQuotaBytes, + BaseSnapshot: *baseSnapshot, + DeltaSnapList: deltaSnapList, + } + err = rstr.Restore(restoreOptions) + Expect(err).ShouldNot(HaveOccurred()) + + err = checkDataConsistency(restoreDataDir, logger) + Expect(err).ShouldNot(HaveOccurred()) + }) + }) + }) - Context("with maximum of hundred fetchers allowed", func() { - fmt.Println("Testing for max-fetchers=100") - It("should restore etcd data directory", func() { - maxFetchers = 100 - - restoreOptions := RestoreOptions{ - ClusterURLs: clusterUrlsMap, - ClusterToken: restoreClusterToken, - RestoreDataDir: restoreDataDir, - PeerURLs: peerUrls, - SkipHashCheck: skipHashCheck, - Name: restoreName, - MaxFetchers: maxFetchers, - EmbeddedEtcdQuotaBytes: embeddedEtcdQuotaBytes, - BaseSnapshot: *baseSnapshot, - DeltaSnapList: deltaSnapList, - } - err = rstr.Restore(restoreOptions) - Expect(err).ShouldNot(HaveOccurred()) + Describe("NEGATIVE:For Dynamic Loads and Negative Scenarios", func() { + BeforeEach(func() { + errCh = make(chan error) + populatorStopCh = make(chan bool) + ssrStopCh = make(chan struct{}) - err = checkDataConsistency(restoreDataDir, logger) + etcd, err = startEmbeddedEtcd(etcdDir, logger) Expect(err).ShouldNot(HaveOccurred()) + wg = &sync.WaitGroup{} + + }) + + AfterEach(cleanUp) + + Context("with only delta snapshots and no full snapshots", func() { + fmt.Println("Testing for no base snapshot and only delta snapshots ") + It("should restore from the delta snapshots ", func() { + cleanUp() + deltaSnapshotPeriod := 1 + wg.Add(1) + go populateEtcd(wg, logger, endpoints, errCh, populatorStopCh) + go stopLoaderAndSnapshotter(wg, 2, 2, populatorStopCh, ssrStopCh) + + //<-time.After(time.Duration(5 * time.Second)) + logger.Infoln("Starting snapshotter with basesnapshot set to false") + err = runSnapshotter(logger, deltaSnapshotPeriod, endpoints, ssrStopCh, false) + Expect(err).ShouldNot(HaveOccurred()) + + etcd.Server.Stop() + etcd.Close() + + err = corruptEtcdDir() + Expect(err).ShouldNot(HaveOccurred()) + store, err = snapstore.GetSnapstore(&snapstore.Config{Container: snapstoreDir, Provider: "Local"}) + Expect(err).ShouldNot(HaveOccurred()) + + baseSnapshot, deltaSnapList, err = miscellaneous.GetLatestFullSnapshotAndDeltaSnapList(store) + Expect(err).ShouldNot(HaveOccurred()) + logger.Infoln(deltaSnapList.Len()) + logger.Infof("base snapshot is %v", baseSnapshot) + + rstr = NewRestorer(store, logger) + restoreOptions := RestoreOptions{ + ClusterURLs: clusterUrlsMap, + ClusterToken: restoreClusterToken, + RestoreDataDir: restoreDataDir, + PeerURLs: peerUrls, + SkipHashCheck: skipHashCheck, + Name: restoreName, + MaxFetchers: maxFetchers, + EmbeddedEtcdQuotaBytes: embeddedEtcdQuotaBytes, + //BaseSnapshot: *baseSnapshot, + DeltaSnapList: deltaSnapList, + } + + restoreOptions.BaseSnapshot.SnapDir = "" + restoreOptions.BaseSnapshot.SnapName = "" + //logger.Info(restoreOptions.BaseSnapshot) + err := rstr.Restore(restoreOptions) + Expect(err).Should(BeNil()) + err = checkDataConsistency(restoreDataDir, logger) + Expect(err).ShouldNot(HaveOccurred()) + + }) + }) + + Context("with no delta snapshots", func() { + fmt.Println("Testing with no delta events") + It("Should restore only full snapshot", func() { + + deltaSnapshotPeriod := 3 + wg.Add(1) + go populateEtcd(wg, logger, endpoints, errCh, populatorStopCh) + go stopLoaderAndSnapshotter(wg, 1, 1, populatorStopCh, ssrStopCh) + + //<-time.After(time.Duration(15 * time.Second)) + logger.Infoln("Starting snapshotter for no delta snapshots") + err = runSnapshotter(logger, deltaSnapshotPeriod, endpoints, ssrStopCh, true) + Expect(err).ShouldNot(HaveOccurred()) + + etcd.Server.Stop() + etcd.Close() + + err = corruptEtcdDir() + Expect(err).ShouldNot(HaveOccurred()) + store, err = snapstore.GetSnapstore(&snapstore.Config{Container: snapstoreDir, Provider: "Local"}) + Expect(err).ShouldNot(HaveOccurred()) + + baseSnapshot, deltaSnapList, err = miscellaneous.GetLatestFullSnapshotAndDeltaSnapList(store) + Expect(err).ShouldNot(HaveOccurred()) + Expect(deltaSnapList.Len()).Should(BeZero()) + + rstr = NewRestorer(store, logger) + + RestoreOptions := RestoreOptions{ + ClusterURLs: clusterUrlsMap, + ClusterToken: restoreClusterToken, + RestoreDataDir: restoreDataDir, + PeerURLs: peerUrls, + SkipHashCheck: skipHashCheck, + Name: restoreName, + MaxFetchers: maxFetchers, + EmbeddedEtcdQuotaBytes: embeddedEtcdQuotaBytes, + BaseSnapshot: *baseSnapshot, + DeltaSnapList: deltaSnapList, + } + err = rstr.Restore(RestoreOptions) + Expect(err).ShouldNot(HaveOccurred()) + + }) + }) + + Context("with corrupted snapstore", func() { + fmt.Println("Testing with missing snapshots in the store") + It("Should not restore and return error", func() { + + deltaSnapshotPeriod := 1 + wg.Add(1) + go populateEtcd(wg, logger, endpoints, errCh, populatorStopCh) + go stopLoaderAndSnapshotter(wg, 2, 2, populatorStopCh, ssrStopCh) + + logger.Infoln("Starting snapshotter for corrupted snapstore") + err = runSnapshotter(logger, deltaSnapshotPeriod, endpoints, ssrStopCh, true) + Expect(err).ShouldNot(HaveOccurred()) + + etcd.Server.Stop() + etcd.Close() + + err = corruptEtcdDir() + Expect(err).ShouldNot(HaveOccurred()) + store, err = snapstore.GetSnapstore(&snapstore.Config{Container: snapstoreDir, Provider: "Local"}) + Expect(err).ShouldNot(HaveOccurred()) + + baseSnapshot, deltaSnapList, err = miscellaneous.GetLatestFullSnapshotAndDeltaSnapList(store) + Expect(err).ShouldNot(HaveOccurred()) + logger.Infoln(deltaSnapList.Len()) + + snapshotToRemove := path.Join(snapstoreDir, deltaSnapList[deltaSnapList.Len()-1].SnapDir, deltaSnapList[deltaSnapList.Len()-1].SnapName) + logger.Infoln(snapshotToRemove) + err = os.Remove(snapshotToRemove) + logger.Infof("Removed snapshot to cause corruption %s", snapshotToRemove) + Expect(err).ShouldNot(HaveOccurred()) + + rstr = NewRestorer(store, logger) + + RestoreOptions := RestoreOptions{ + ClusterURLs: clusterUrlsMap, + ClusterToken: restoreClusterToken, + RestoreDataDir: restoreDataDir, + PeerURLs: peerUrls, + SkipHashCheck: skipHashCheck, + Name: restoreName, + MaxFetchers: maxFetchers, + EmbeddedEtcdQuotaBytes: embeddedEtcdQuotaBytes, + BaseSnapshot: *baseSnapshot, + DeltaSnapList: deltaSnapList, + } + + err = rstr.Restore(RestoreOptions) + Expect(err).Should(HaveOccurred()) + // the below consistency fails with index out of range error hence commented, + // but the etcd directory is filled partially as part of the restore which should be relooked. + // err = checkDataConsistency(restoreDataDir, logger) + // Expect(err).Should(HaveOccurred()) + + }) + }) + + Context("with etcd client is unavailable ", func() { + fmt.Println("Testing restore while etcd client is still in use") + It("Should fail to restore", func() { + + deltaSnapshotPeriod := 1 + wg.Add(1) + go populateEtcd(wg, logger, endpoints, errCh, populatorStopCh) + go stopLoaderAndSnapshotter(wg, 2, 2, populatorStopCh, ssrStopCh) + + logger.Infoln("Starting snapshotter for etcd client deferred closing") + err = runSnapshotter(logger, deltaSnapshotPeriod, endpoints, ssrStopCh, true) + Expect(err).ShouldNot(HaveOccurred()) + //this will ensure that etcd client is unavailable for the restore + defer func() { + etcd.Server.Stop() + etcd.Close() + }() + //time.Sleep(time.Duration(5 * time.Second)) + err = corruptEtcdDir() + Expect(err).ShouldNot(HaveOccurred()) + logger.Infoln("corrupted the etcd dir") + + store, err = snapstore.GetSnapstore(&snapstore.Config{Container: snapstoreDir, Provider: "Local"}) + Expect(err).ShouldNot(HaveOccurred()) + baseSnapshot, deltaSnapList, err = miscellaneous.GetLatestFullSnapshotAndDeltaSnapList(store) + Expect(err).ShouldNot(HaveOccurred()) + + rstr = NewRestorer(store, logger) + + RestoreOptions := RestoreOptions{ + ClusterURLs: clusterUrlsMap, + ClusterToken: restoreClusterToken, + RestoreDataDir: restoreDataDir, + PeerURLs: peerUrls, + SkipHashCheck: skipHashCheck, + Name: restoreName, + MaxFetchers: maxFetchers, + EmbeddedEtcdQuotaBytes: embeddedEtcdQuotaBytes, + BaseSnapshot: *baseSnapshot, + DeltaSnapList: deltaSnapList, + } + logger.Infoln("starting restore while snapshotter is running") + err = rstr.Restore(RestoreOptions) + logger.Infof("Failed because : %s", err) + Expect(err).Should(HaveOccurred()) + + }) + }) + + Context("with etcd data dir not cleaned up before restore", func() { + fmt.Println("Testing restore on an existing etcd data directory") + It("Should fail to restore", func() { + + deltaSnapshotPeriod := 1 + wg.Add(1) + go populateEtcd(wg, logger, endpoints, errCh, populatorStopCh) + + go stopLoaderAndSnapshotter(wg, 2, 2, populatorStopCh, ssrStopCh) + + logger.Infoln("Starting snapshotter for not cleaned etcd dir scenario") + err = runSnapshotter(logger, deltaSnapshotPeriod, endpoints, ssrStopCh, true) + Expect(err).ShouldNot(HaveOccurred()) + + etcd.Server.Stop() + etcd.Close() + + //time.Sleep(time.Duration(50 * time.Second)) + // err = corruptEtcdDir() + // Expect(err).ShouldNot(HaveOccurred()) + // logger.Infoln("corrupted the etcd dir") + + store, err = snapstore.GetSnapstore(&snapstore.Config{Container: snapstoreDir, Provider: "Local"}) + Expect(err).ShouldNot(HaveOccurred()) + baseSnapshot, deltaSnapList, err = miscellaneous.GetLatestFullSnapshotAndDeltaSnapList(store) + Expect(err).ShouldNot(HaveOccurred()) + + rstr = NewRestorer(store, logger) + + RestoreOptions := RestoreOptions{ + ClusterURLs: clusterUrlsMap, + ClusterToken: restoreClusterToken, + RestoreDataDir: restoreDataDir, + PeerURLs: peerUrls, + SkipHashCheck: skipHashCheck, + Name: restoreName, + MaxFetchers: maxFetchers, + EmbeddedEtcdQuotaBytes: embeddedEtcdQuotaBytes, + BaseSnapshot: *baseSnapshot, + DeltaSnapList: deltaSnapList, + } + logger.Infoln("starting restore restore directory exists already") + err = rstr.Restore(RestoreOptions) + logger.Infof("Failed to restore becasue :: %s", err) + Expect(err).Should(HaveOccurred()) + + }) + }) + //this test is excluded for now and is kept for reference purpose only + // there needs to be some relook done to validate the senarios when a restore can happen on a running snapshot and accordingly include the test + // as per current understanding the flow ensures it cannot happen but external intervention can not be ruled out as the command allows calling restore while snapshotting. + XContext("while snapshotter is running ", func() { + fmt.Println("Testing restore while snapshotter is happening") + It("Should stop snapshotter while restore is happening", func() { + + deltaSnapshotPeriod := 1 + wg.Add(1) + go populateEtcd(wg, logger, endpoints, errCh, populatorStopCh) + go stopLoaderAndSnapshotter(wg, 5, 15, populatorStopCh, ssrStopCh) + + logger.Infoln("Starting snapshotter while loading is happening") + go runSnapshotter(logger, deltaSnapshotPeriod, endpoints, ssrStopCh, true) + Expect(err).ShouldNot(HaveOccurred()) + + time.Sleep(time.Duration(5 * time.Second)) + etcd.Server.Stop() + etcd.Close() + + err = corruptEtcdDir() + Expect(err).ShouldNot(HaveOccurred()) + logger.Infoln("corrupted the etcd dir") + + store, err = snapstore.GetSnapstore(&snapstore.Config{Container: snapstoreDir, Provider: "Local"}) + Expect(err).ShouldNot(HaveOccurred()) + baseSnapshot, deltaSnapList, err = miscellaneous.GetLatestFullSnapshotAndDeltaSnapList(store) + Expect(err).ShouldNot(HaveOccurred()) + + rstr = NewRestorer(store, logger) + + RestoreOptions := RestoreOptions{ + ClusterURLs: clusterUrlsMap, + ClusterToken: restoreClusterToken, + RestoreDataDir: restoreDataDir, + PeerURLs: peerUrls, + SkipHashCheck: skipHashCheck, + Name: restoreName, + MaxFetchers: maxFetchers, + EmbeddedEtcdQuotaBytes: embeddedEtcdQuotaBytes, + BaseSnapshot: *baseSnapshot, + DeltaSnapList: deltaSnapList, + } + logger.Infoln("starting restore while snapshotter is running") + err = rstr.Restore(RestoreOptions) + Expect(err).ShouldNot(HaveOccurred()) + err = checkDataConsistency(restoreDataDir, logger) + Expect(err).ShouldNot(HaveOccurred()) + + // Although the test has passed but the logic currently doesn't stop snapshotter explicitly but assumes that restore + // shall be triggered only on restart of the etcd pod, so in the current case the snapshotter and restore were both running + // together. However data corruption was not simulated as the embeded etcd used to populate need to be stopped for restore to begin. + // In a productive scenarios as the command is exposed so it's possible to run this without knowledge of the tightly coupled + // behavior of etcd restart. + + }) }) }) + }) +func stopLoaderAndSnapshotter(wg *sync.WaitGroup, populatorStopDuration int, snapshotterStopDuration int, populatorStopCh chan bool, ssrStopCh chan struct{}) { + <-time.After(time.Duration(int64(populatorStopDuration) * int64(time.Second))) + close(populatorStopCh) + wg.Wait() + time.Sleep(time.Duration(int64(snapshotterStopDuration) * int64(time.Second))) + close(ssrStopCh) +} + // checkDataConsistency starts an embedded etcd and checks for correctness of the values stored in etcd against the keys 'keyFrom' through 'keyTo' func checkDataConsistency(dir string, logger *logrus.Logger) error { etcd, err := startEmbeddedEtcd(dir, logger) @@ -215,17 +660,25 @@ func checkDataConsistency(dir string, logger *logrus.Logger) error { for currKey := keyFrom; currKey <= keyTo; currKey++ { key = keyPrefix + strconv.Itoa(currKey) value = valuePrefix + strconv.Itoa(currKey) - resp, err := cli.Get(context.TODO(), key, opts...) if err != nil { return fmt.Errorf("unable to get value from etcd: %v", err) } if len(resp.Kvs) == 0 { - return fmt.Errorf("entry not found for key %s", resKey) + // handles deleted keys as every 10th key is deleted during populate etcd call + // this handling is also done in the populateEtcd() in restorer_suite_test.go file + // also it assumes that the deltaSnapshotDuration is more than 10 -- + // if you change the constant please change the factor accordingly to have coverage of delete scenarios. + if math.Mod(float64(currKey), 10) == 0 { + continue //it should continue as key was put for action delete + } else { + return fmt.Errorf("entry not found for key %s", key) + } } res := resp.Kvs[0] resKey = string(res.Key) resValue = string(res.Value) + if resKey != key { return fmt.Errorf("key mismatch for %s and %s", resKey, key) } From caf9c6b172d55bb59b795c79b9c40a14220a60ba Mon Sep 17 00:00:00 2001 From: Ashwani Kumar Date: Wed, 4 Sep 2019 11:10:38 +0530 Subject: [PATCH 2/4] enhanced the test with handling for no restore directory provided --- pkg/snapshot/restorer/restorer.go | 1 + pkg/snapshot/restorer/restorer_suite_test.go | 16 ++++++++++++++++ pkg/snapshot/restorer/restorer_test.go | 2 +- 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/pkg/snapshot/restorer/restorer.go b/pkg/snapshot/restorer/restorer.go index 58349ed40..593047ed2 100644 --- a/pkg/snapshot/restorer/restorer.go +++ b/pkg/snapshot/restorer/restorer.go @@ -67,6 +67,7 @@ func (r *Restorer) Restore(ro RestoreOptions) error { r.logger.Infof("Quota size for etcd must be greater than 0. Input EmbeddedEtcdQuotaBytes: %d. Defaulting to 8GB.", ro.EmbeddedEtcdQuotaBytes) ro.EmbeddedEtcdQuotaBytes = int64(8 * 1024 * 1024 * 1024) } + ro.RestoreDataDir = path.Clean(ro.RestoreDataDir) if err := r.restoreFromBaseSnapshot(ro); err != nil { return fmt.Errorf("failed to restore from the base snapshot :%v", err) } diff --git a/pkg/snapshot/restorer/restorer_suite_test.go b/pkg/snapshot/restorer/restorer_suite_test.go index 0c503f733..bf5c2feed 100644 --- a/pkg/snapshot/restorer/restorer_suite_test.go +++ b/pkg/snapshot/restorer/restorer_suite_test.go @@ -19,6 +19,7 @@ import ( "fmt" "math" "os" + "path" "strconv" "sync" "testing" @@ -101,6 +102,21 @@ func cleanUp() { err = os.RemoveAll(snapstoreDir) Expect(err).ShouldNot(HaveOccurred()) + + //for the negative scenario for invalid restoredir set to "" we need to cleanup the member folder in the working directory + restoreDir := path.Clean("") + info, err := os.Stat(fmt.Sprintf("%s/%s", restoreDir, "member")) + if err != nil { + if os.IsNotExist(err) { + return + } else { + Expect(err).ShouldNot(HaveOccurred()) + } + } + if info.IsDir() { + err = os.RemoveAll(info.Name()) + Expect(err).ShouldNot(HaveOccurred()) + } } // startEmbeddedEtcd starts an embedded etcd server diff --git a/pkg/snapshot/restorer/restorer_test.go b/pkg/snapshot/restorer/restorer_test.go index 6a015971b..55bb06fe5 100644 --- a/pkg/snapshot/restorer/restorer_test.go +++ b/pkg/snapshot/restorer/restorer_test.go @@ -186,7 +186,7 @@ var _ = Describe("Running Restorer", func() { DeltaSnapList: deltaSnapList, } err = rstr.Restore(restoreOptions) - Expect(err).Should(HaveOccurred()) + Expect(err).ShouldNot(HaveOccurred()) }) }) From bc80568b7f07f359a274a5ce7823d5386940f356 Mon Sep 17 00:00:00 2001 From: Ashwani Kumar Date: Wed, 4 Sep 2019 13:49:49 +0530 Subject: [PATCH 3/4] enchanced the cleanup in restorer_suite_test.go with review comments --- pkg/snapshot/restorer/restorer_suite_test.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/snapshot/restorer/restorer_suite_test.go b/pkg/snapshot/restorer/restorer_suite_test.go index bf5c2feed..22b6d7bb8 100644 --- a/pkg/snapshot/restorer/restorer_suite_test.go +++ b/pkg/snapshot/restorer/restorer_suite_test.go @@ -105,7 +105,8 @@ func cleanUp() { //for the negative scenario for invalid restoredir set to "" we need to cleanup the member folder in the working directory restoreDir := path.Clean("") - info, err := os.Stat(fmt.Sprintf("%s/%s", restoreDir, "member")) + // info, err := os.Stat(fmt.Sprintf("%s/%s", restoreDir, "member")) + info, err := os.Stat(path.Join(restoreDir, "member")) if err != nil { if os.IsNotExist(err) { return @@ -113,10 +114,10 @@ func cleanUp() { Expect(err).ShouldNot(HaveOccurred()) } } - if info.IsDir() { - err = os.RemoveAll(info.Name()) - Expect(err).ShouldNot(HaveOccurred()) - } + + err = os.RemoveAll(info.Name()) + Expect(err).ShouldNot(HaveOccurred()) + } // startEmbeddedEtcd starts an embedded etcd server From ace0f090ba55cfc1b5bf5236fab12f55b22448da Mon Sep 17 00:00:00 2001 From: Ashwani Kumar Date: Wed, 4 Sep 2019 14:18:46 +0530 Subject: [PATCH 4/4] review comments incorporated on cleanup logic of handling restore directory properly --- pkg/snapshot/restorer/restorer_suite_test.go | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/pkg/snapshot/restorer/restorer_suite_test.go b/pkg/snapshot/restorer/restorer_suite_test.go index 22b6d7bb8..78e104d2e 100644 --- a/pkg/snapshot/restorer/restorer_suite_test.go +++ b/pkg/snapshot/restorer/restorer_suite_test.go @@ -105,17 +105,7 @@ func cleanUp() { //for the negative scenario for invalid restoredir set to "" we need to cleanup the member folder in the working directory restoreDir := path.Clean("") - // info, err := os.Stat(fmt.Sprintf("%s/%s", restoreDir, "member")) - info, err := os.Stat(path.Join(restoreDir, "member")) - if err != nil { - if os.IsNotExist(err) { - return - } else { - Expect(err).ShouldNot(HaveOccurred()) - } - } - - err = os.RemoveAll(info.Name()) + err = os.RemoveAll(path.Join(restoreDir, "member")) Expect(err).ShouldNot(HaveOccurred()) }