Automatic index migration (#607)

* Make migration automatic and remove system cmd

Add integration test for auto migration

* Fix typo

* Add integration test for no migration scenario

* Check for existence of default dir in test

* Fix linting error

* Code review changes

Just call migrate in root and add logging around migration. Also make
migration check function private

* Code review changes

* Code review changes

cmd/krew/cmd/root.go

@@ -150,13 +150,14 @@ func preRun(cmd *cobra.Command, _ []string) error {
 	}
 
 	if _, ok := os.LookupEnv(constants.EnableMultiIndexSwitch); ok {
-		isMigrated, err = indexmigration.Done(paths)
+		isMigrated, err := indexmigration.Done(paths)
 		if err != nil {
-			return errors.Wrap(err, "error getting file info")
+			return errors.Wrap(err, "failed to check if index migration is complete")
 		}
-		if !isMigrated && cmd.Use != "index-upgrade" {
-			fmt.Fprintln(os.Stderr, "You need to perform a migration to continue using krew.\nPlease run `kubectl krew system index-upgrade`")
-			return errors.New("krew index outdated")
+		if !isMigrated {
+			if err := indexmigration.Migrate(paths); err != nil {
+				return errors.Wrap(err, "index migration failed")
+			}
 		}
 	}
 

integration_test/system_test.go → integration_test/migration_test.go

@@ -19,8 +19,26 @@ import (
 	"os"
 	"strings"
 	"testing"
+
+	"sigs.k8s.io/krew/pkg/constants"
 )
 
+func TestKrewIndexAutoMigration(t *testing.T) {
+	skipShort(t)
+
+	test, cleanup := NewTest(t)
+	defer cleanup()
+
+	test.WithEnv(constants.EnableMultiIndexSwitch, 1).WithDefaultIndex()
+	prepareOldIndexLayout(test)
+
+	// any command here should cause the index migration to occur
+	test.Krew("index", "list").RunOrFail()
+	if !isIndexMigrated(test) {
+		t.Error("index should have been auto-migrated")
+	}
+}
+
 func TestKrewUnsupportedVersion(t *testing.T) {
 	skipShort(t)
 
@@ -42,6 +60,28 @@ func TestKrewUnsupportedVersion(t *testing.T) {
 	}
 }
 
+func isIndexMigrated(it *ITest) bool {
+	indexPath := it.TempDir().Path("index/default")
+	_, err := os.Stat(indexPath)
+	return err == nil
+}
+
+// TODO remove when testing indexmigration is no longer necessary
+func prepareOldIndexLayout(it *ITest) {
+	indexPath := it.TempDir().Path("index/default")
+	tmpPath := it.TempDir().Path("tmp_index")
+	newPath := it.TempDir().Path("index")
+	if err := os.Rename(indexPath, tmpPath); err != nil {
+		it.t.Fatal(err)
+	}
+	if err := os.Remove(newPath); err != nil {
+		it.t.Fatal(err)
+	}
+	if err := os.Rename(tmpPath, newPath); err != nil {
+		it.t.Fatal(err)
+	}
+}
+
 func prepareOldKrewRoot(test *ITest) {
 	// receipts are not present in old krew home
 	if err := os.RemoveAll(test.tempDir.Path("receipts")); err != nil {

internal/indexmigration/migration.go

@@ -27,23 +27,18 @@ import (
 // Done checks if the krew installation requires a migration to support multiple indexes.
 // A migration is necessary when the index directory contains a ".git" directory.
 func Done(paths environment.Paths) (bool, error) {
+	klog.V(2).Info("Checking if index migration is needed.")
 	_, err := os.Stat(filepath.Join(paths.IndexBase(), ".git"))
 	if err != nil && os.IsNotExist(err) {
+		klog.V(2).Infoln("Index already migrated.")
 		return true, nil
 	}
 	return false, err
 }
 
-// Migrate removes the index directory and then clones krew-index to the new default index path.
+// Migrate moves the index directory to the new default index path.
 func Migrate(paths environment.Paths) error {
-	isMigrated, err := Done(paths)
-	if err != nil {
-		return errors.Wrap(err, "failed to check if index migration is complete")
-	}
-	if isMigrated {
-		klog.V(2).Infoln("Already migrated")
-		return nil
-	}
+	klog.Info("Migrating krew index layout.")
 	indexPath := paths.IndexBase()
 	tmpPath := filepath.Join(paths.BasePath(), "tmp_index_migration")
 	newPath := filepath.Join(paths.IndexBase(), "default")
@@ -53,13 +48,13 @@ func Migrate(paths environment.Paths) error {
 	}
 
 	if err := os.Mkdir(indexPath, os.ModePerm); err != nil {
-		return errors.Wrapf(err, "could create index directory %q", indexPath)
+		return errors.Wrapf(err, "could not create index directory %q", indexPath)
 	}
 
 	if err := os.Rename(tmpPath, newPath); err != nil {
 		return errors.Wrapf(err, "could not move temporary index directory %q to new location %q", tmpPath, newPath)
 	}
 
-	klog.Infof("Migration completed successfully.")
+	klog.Info("Migration completed successfully.")
 	return nil
 }

internal/indexmigration/migration_test.go

@@ -63,30 +63,18 @@ func TestIsMigrated(t *testing.T) {
 }
 
 func TestMigrate(t *testing.T) {
-	var tests = []struct {
-		name   string
-		gitDir string
-	}{
-		{name: "migration is necessary", gitDir: "index/.git"},
-		{name: "no migration is necessary", gitDir: "index/default/.git"},
-	}
-	for _, tt := range tests {
-		tt := tt
-		t.Run(tt.name, func(t *testing.T) {
-			tmpDir, cleanup := testutil.NewTempDir(t)
-			defer cleanup()
+	tmpDir, cleanup := testutil.NewTempDir(t)
+	defer cleanup()
 
-			tmpDir.Write(tt.gitDir, nil)
+	tmpDir.Write("index/.git", nil)
 
-			newPaths := environment.NewPaths(tmpDir.Root())
-			err := Migrate(newPaths)
-			if err != nil {
-				t.Fatal(err)
-			}
-			done, err := Done(newPaths)
-			if err != nil || !done {
-				t.Errorf("expected migration to be done: %s", err)
-			}
-		})
+	newPaths := environment.NewPaths(tmpDir.Root())
+	err := Migrate(newPaths)
+	if err != nil {
+		t.Fatal(err)
+	}
+	done, err := Done(newPaths)
+	if err != nil || !done {
+		t.Errorf("expected migration to be done: %s", err)
 	}
 }