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.

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

cmd: Update each index instead of just default #588

Merged
merged 8 commits into from
Apr 13, 2020
Merged
Changes from all commits
Commits
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 cmd/krew/cmd/install.go
Original file line number Diff line number Diff line change
@@ -189,7 +189,7 @@ Remarks:
klog.V(4).Infof("--no-update-index specified, skipping updating local copy of plugin index")
return nil
}
return ensureIndexUpdated(cmd, args)
return ensureIndexesUpdated(cmd, args)
},
}

16 changes: 3 additions & 13 deletions cmd/krew/cmd/search.go
Original file line number Diff line number Diff line change
@@ -27,7 +27,6 @@ import (
"sigs.k8s.io/krew/internal/index/indexoperations"
"sigs.k8s.io/krew/internal/index/indexscanner"
"sigs.k8s.io/krew/internal/installation"
"sigs.k8s.io/krew/pkg/constants"
)

// searchCmd represents the search command
@@ -44,18 +43,9 @@ Examples:
To fuzzy search plugins with a keyword:
kubectl krew search KEYWORD`,
RunE: func(cmd *cobra.Command, args []string) error {
indexes := []indexoperations.Index{
{
Name: constants.DefaultIndexName,
URL: constants.DefaultIndexURI, // unused here but providing for completeness
},
}
if os.Getenv(constants.EnableMultiIndexSwitch) != "" {
out, err := indexoperations.ListIndexes(paths)
if err != nil {
return errors.Wrapf(err, "failed to list plugin indexes available")
}
indexes = out
indexes, err := indexoperations.ListIndexes(paths)
if err != nil {
return errors.Wrap(err, "failed to list indexes")
}

klog.V(3).Infof("found %d indexes", len(indexes))
2 changes: 1 addition & 1 deletion cmd/krew/cmd/system.go
Original file line number Diff line number Diff line change
@@ -51,7 +51,7 @@ This command will be removed without further notice from future versions of krew
RunE: func(cmd *cobra.Command, args []string) error {
return receiptsmigration.Migrate(paths)
},
PreRunE: ensureIndexUpdated,
PreRunE: ensureIndexesUpdated,
}

var indexUpgradeCmd = &cobra.Command{
29 changes: 23 additions & 6 deletions cmd/krew/cmd/update.go
Original file line number Diff line number Diff line change
@@ -19,12 +19,14 @@ import (
"fmt"
"io"
"os"
"strings"

"github.com/pkg/errors"
"github.com/spf13/cobra"
"k8s.io/klog"

"sigs.k8s.io/krew/internal/gitutil"
"sigs.k8s.io/krew/internal/index/indexoperations"
"sigs.k8s.io/krew/internal/index/indexscanner"
"sigs.k8s.io/krew/internal/installation"
"sigs.k8s.io/krew/pkg/constants"
@@ -43,7 +45,7 @@ plugin index from the internet.
Remarks:
You don't need to run this command: Running "krew update" or "krew upgrade"
will silently run this command.`,
RunE: ensureIndexUpdated,
RunE: ensureIndexesUpdated,
}

func showFormattedPluginsInfo(out io.Writer, header string, plugins []string) {
@@ -102,13 +104,28 @@ func showUpdatedPlugins(out io.Writer, preUpdate, posUpdate []index.Plugin, inst
}
}

func ensureIndexUpdated(_ *cobra.Command, _ []string) error {
func ensureIndexesUpdated(_ *cobra.Command, _ []string) error {
preUpdateIndex, _ := indexscanner.LoadPluginListFromFS(paths.IndexPluginsPath(constants.DefaultIndexName))

klog.V(1).Infof("Updating the local copy of plugin index (%s)", paths.IndexPath(constants.DefaultIndexName))
if err := gitutil.EnsureUpdated(constants.DefaultIndexURI, paths.IndexPath(constants.DefaultIndexName)); err != nil {
return errors.Wrap(err, "failed to update the local index")
indexes, err := indexoperations.ListIndexes(paths)
if err != nil {
return errors.Wrap(err, "failed to list indexes")
}

var failed []string
var returnErr error
for _, idx := range indexes {
indexPath := paths.IndexPath(idx.Name)
klog.V(1).Infof("Updating the local copy of plugin index (%s)", indexPath)
if err := gitutil.EnsureUpdated(idx.URL, indexPath); err != nil {
klog.Warningf("failed to update index %q: %v", idx.Name, err)
failed = append(failed, idx.Name)
if returnErr == nil {
returnErr = err
}
}
}

fmt.Fprintln(os.Stderr, "Updated the local copy of plugin index.")

if len(preUpdateIndex) == 0 {
@@ -132,7 +149,7 @@ func ensureIndexUpdated(_ *cobra.Command, _ []string) error {
// TODO(chriskim06) consider commenting this out when refactoring for custom indexes
showUpdatedPlugins(os.Stderr, preUpdateIndex, posUpdateIndex, installedPlugins)

Copy link
Member

Choose a reason for hiding this comment

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

I’ve realized this if (+counting) isnt necessary since errors.Wrap already handles nil err. Maybe remove?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure I can do that. Do you think it will look weird if this ends with the return errors.Wrapf(...)?

Copy link
Member

Choose a reason for hiding this comment

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

I think it’s fine. I just realized the check is redundant.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok I'll remove the check then.

return nil
return errors.Wrapf(returnErr, "failed to update the following indexes: %s\n", strings.Join(failed, ", "))
}

func init() {
2 changes: 1 addition & 1 deletion cmd/krew/cmd/upgrade.go
Original file line number Diff line number Diff line change
@@ -106,7 +106,7 @@ kubectl krew upgrade foo bar"`,
klog.V(4).Infof("--no-update-index specified, skipping updating local copy of plugin index")
return nil
}
return ensureIndexUpdated(cmd, args)
return ensureIndexesUpdated(cmd, args)
},
}

2 changes: 1 addition & 1 deletion integration_test/index_test.go
Original file line number Diff line number Diff line change
@@ -150,7 +150,7 @@ func TestKrewIndexRemove_unsafe(t *testing.T) {
func TestKrewIndexRemoveFailsWhenPluginsInstalled(t *testing.T) {
skipShort(t)
test, cleanup := NewTest(t)
test = test.WithEnv(constants.EnableMultiIndexSwitch, 1)
test = test.WithEnv(constants.EnableMultiIndexSwitch, 1).WithIndex()
defer cleanup()

test.Krew("install", validPlugin).RunOrFailOutput()
46 changes: 46 additions & 0 deletions integration_test/update_test.go
Original file line number Diff line number Diff line change
@@ -49,6 +49,52 @@ func TestKrewUpdate(t *testing.T) {
}
}

func TestKrewUpdateMultipleIndexes(t *testing.T) {
skipShort(t)
test, cleanup := NewTest(t)
defer cleanup()

test = test.WithEnv(constants.EnableMultiIndexSwitch, 1).WithIndex()
// to enable new paths in environment.NewPaths()
os.Setenv(constants.EnableMultiIndexSwitch, "1")
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a comment like

// to enable new paths in environment.NewPaths()

defer os.Unsetenv(constants.EnableMultiIndexSwitch)

paths := environment.NewPaths(test.Root())
test.Krew("index", "add", "foo", paths.IndexPath(constants.DefaultIndexName)).RunOrFail()
if err := os.RemoveAll(paths.IndexPluginsPath("foo")); err != nil {
t.Errorf("error removing plugins directory from index: %s", err)
}
test.Krew("update").RunOrFailOutput()
out := string(test.Krew("search").RunOrFailOutput())
if !strings.Contains(out, "\nctx") {
t.Error("expected plugin ctx in list of plugins")
}
if !strings.Contains(out, "foo/ctx") {
t.Error("expected plugin foo/ctx in list of plugins")
}
}

func TestKrewUpdateFailedIndex(t *testing.T) {
skipShort(t)
test, cleanup := NewTest(t)
defer cleanup()

test = test.WithEnv(constants.EnableMultiIndexSwitch, 1).WithIndex()
os.Setenv(constants.EnableMultiIndexSwitch, "1")
defer os.Unsetenv(constants.EnableMultiIndexSwitch)

paths := environment.NewPaths(test.Root())
test.TempDir().InitEmptyGitRepo(paths.IndexPath("foo"), "invalid-git")
out, err := test.Krew("update").Run()
if err == nil {
t.Error("expected update to fail")
}
msg := "failed to update the following indexes: foo"
if !strings.Contains(string(out), msg) {
t.Errorf("%q doesn't contain msg=%q", string(out), msg)
}
}

func TestKrewUpdateListsNewPlugins(t *testing.T) {
skipShort(t)
test, cleanup := NewTest(t)
10 changes: 10 additions & 0 deletions internal/index/indexoperations/index.go
Original file line number Diff line number Diff line change
@@ -23,6 +23,7 @@ import (

"sigs.k8s.io/krew/internal/environment"
"sigs.k8s.io/krew/internal/gitutil"
"sigs.k8s.io/krew/pkg/constants"
)

var validNamePattern = regexp.MustCompile(`^[A-Za-z0-9_-]+$`)
@@ -36,6 +37,15 @@ type Index struct {
// ListIndexes returns a slice of Index objects. The path argument is used as
// the base path of the index.
func ListIndexes(paths environment.Paths) ([]Index, error) {
if _, ok := os.LookupEnv(constants.EnableMultiIndexSwitch); !ok {
return []Index{
{
Name: constants.DefaultIndexName,
URL: constants.DefaultIndexURI,
},
}, nil
}

dirs, err := ioutil.ReadDir(paths.IndexBase())
if err != nil {
return nil, errors.Wrap(err, "failed to list directory")
23 changes: 4 additions & 19 deletions internal/index/indexoperations/index_test.go
Original file line number Diff line number Diff line change
@@ -22,7 +22,6 @@ import (
"github.com/google/go-cmp/cmp"

"sigs.k8s.io/krew/internal/environment"
"sigs.k8s.io/krew/internal/gitutil"
"sigs.k8s.io/krew/internal/testutil"
"sigs.k8s.io/krew/pkg/constants"
)
@@ -49,7 +48,7 @@ func TestListIndexes(t *testing.T) {
paths := environment.NewPaths(tmpDir.Root())
for _, index := range wantIndexes {
path := paths.IndexPath(index.Name)
initEmptyGitRepo(t, path, index.URL)
tmpDir.InitEmptyGitRepo(path, index.URL)
}

gotIndexes, err := ListIndexes(paths)
@@ -71,7 +70,7 @@ func TestAddIndexSuccess(t *testing.T) {

indexName := "foo"
localRepo := tmpDir.Path("local/" + indexName)
initEmptyGitRepo(t, localRepo, "")
tmpDir.InitEmptyGitRepo(localRepo, "")

paths := environment.NewPaths(tmpDir.Root())
if err := AddIndex(paths, indexName, localRepo); err != nil {
@@ -107,8 +106,8 @@ func TestAddIndexFailure(t *testing.T) {
}

localRepo := tmpDir.Path("local/" + indexName)
initEmptyGitRepo(t, tmpDir.Path("index/"+indexName), "")
initEmptyGitRepo(t, localRepo, "")
tmpDir.InitEmptyGitRepo(tmpDir.Path("index/"+indexName), "")
tmpDir.InitEmptyGitRepo(localRepo, "")

if err := AddIndex(paths, indexName, localRepo); err == nil {
t.Error("expected error when adding an index that already exists")
@@ -151,20 +150,6 @@ func TestDeleteIndex(t *testing.T) {
}
}

func initEmptyGitRepo(t *testing.T, path, url string) {
t.Helper()

if err := os.MkdirAll(path, os.ModePerm); err != nil {
t.Fatalf("cannot create directory %q: %s", filepath.Dir(path), err)
}
if _, err := gitutil.Exec(path, "init"); err != nil {
t.Fatalf("error initializing git repo: %s", err)
}
if _, err := gitutil.Exec(path, "remote", "add", "origin", url); err != nil {
t.Fatalf("error setting remote origin: %s", err)
}
}

func TestIsValidIndexName(t *testing.T) {
tests := []struct {
name string
16 changes: 16 additions & 0 deletions internal/testutil/tempdir.go
Original file line number Diff line number Diff line change
@@ -23,6 +23,8 @@ import (
"testing"

"sigs.k8s.io/yaml"

"sigs.k8s.io/krew/internal/gitutil"
)

type TempDir struct {
@@ -83,3 +85,17 @@ func (td *TempDir) WriteYAML(file string, obj interface{}) *TempDir {
}
return td.Write(file, content)
}

func (td *TempDir) InitEmptyGitRepo(path, url string) {
td.t.Helper()

if err := os.MkdirAll(path, os.ModePerm); err != nil {
td.t.Fatalf("cannot create directory %q: %s", filepath.Dir(path), err)
}
if _, err := gitutil.Exec(path, "init"); err != nil {
td.t.Fatalf("error initializing git repo: %s", err)
}
if _, err := gitutil.Exec(path, "remote", "add", "origin", url); err != nil {
td.t.Fatalf("error setting remote origin: %s", err)
}
}