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

Update plugin listings to use install receipts #276

Merged
merged 2 commits into from
Jul 18, 2019
Merged
Show file tree
Hide file tree
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/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ Remarks:
the names of the plugins installed. This output can be piped back to the
"install" command.`,
RunE: func(cmd *cobra.Command, args []string) error {
plugins, err := installation.ListInstalledPlugins(paths.InstallPath(), paths.BinPath())
plugins, err := installation.ListInstalledPlugins(paths.InstallReceiptsPath())
if err != nil {
return errors.Wrap(err, "failed to find all installed versions")
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/krew/cmd/search.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ Examples:
pluginMap[p.Name] = p
}

installed, err := installation.ListInstalledPlugins(paths.InstallPath(), paths.BinPath())
installed, err := installation.ListInstalledPlugins(paths.InstallReceiptsPath())
if err != nil {
return errors.Wrap(err, "failed to load installed plugins")
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/krew/cmd/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ kubectl krew upgrade foo bar"`,
var pluginNames []string
// Upgrade all plugins.
if len(args) == 0 {
installed, err := installation.ListInstalledPlugins(paths.InstallPath(), paths.BinPath())
installed, err := installation.ListInstalledPlugins(paths.InstallReceiptsPath())
if err != nil {
return errors.Wrap(err, "failed to find all installed versions")
}
Expand Down
36 changes: 18 additions & 18 deletions pkg/installation/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,17 @@
package installation

import (
"io/ioutil"
"os"
"path/filepath"
"strings"

"github.com/golang/glog"
"github.com/pkg/errors"

"sigs.k8s.io/krew/pkg/constants"
"sigs.k8s.io/krew/pkg/index"
"sigs.k8s.io/krew/pkg/pathutil"
"sigs.k8s.io/krew/pkg/receipt"
)

func findInstalledPluginVersion(installPath, binDir, pluginName string) (name string, installed bool, err error) {
Expand Down Expand Up @@ -79,27 +80,26 @@ func getDownloadTarget(index index.Plugin) (version, uri string, fos []index.Fil
return version, uri, p.Files, p.Bin, nil
}

// ListInstalledPlugins returns a list of all name:version for all plugins.
func ListInstalledPlugins(installDir, binDir string) (map[string]string, error) {
installed := make(map[string]string)
plugins, err := ioutil.ReadDir(installDir)
// ListInstalledPlugins returns a list of all install plugins in a
// name:version format based on the install receipts at the specified dir.
func ListInstalledPlugins(receiptsDir string) (map[string]string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, that just got a lot simpler 👍
Would be a good time to also add some tests for this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Since adding tests to this requires me to Store() some manifests by serializing the in-memory Plugin objects (#270) to a tempdir, I will need #270 to happen first.

To unblock here, I'm adding a TODO to the code explaining this situation.

// TODO(ahmetb): Write unit tests for this method. Currently blocked by
// lack of an in-memory recipt object (issue#270) that we can use to save
// receipts to a tempdir that can be read from unit tests.

matches, err := filepath.Glob(filepath.Join(receiptsDir, "*"+constants.ManifestExtension))
if err != nil {
return installed, errors.Wrap(err, "failed to read install dir")
return nil, errors.Wrapf(err, "failed to grab receipts directory (%s) for manifests", receiptsDir)
}
glog.V(4).Infof("Read installation directory: %s (%d items)", installDir, len(plugins))
for _, plugin := range plugins {
if !plugin.IsDir() {
glog.V(4).Infof("Skip non-directory item: %s", plugin.Name())
continue
}
version, ok, err := findInstalledPluginVersion(installDir, binDir, plugin.Name())
glog.V(4).Infof("Found %d install receipts in %s", len(matches), receiptsDir)
installed := make(map[string]string)
for _, m := range matches {
r, err := receipt.Load(m)
if err != nil {
return installed, errors.Wrap(err, "failed to get plugin version")
}
if ok {
installed[plugin.Name()] = version
glog.V(4).Infof("Found %q, with version %s", plugin.Name(), version)
return nil, errors.Wrapf(err, "failed to parse plugin install receipt %s", m)
}
glog.V(4).Infof("parsed receipt for %s: version=%s", r.GetObjectMeta().GetName(), r.Spec.Version)
installed[r.GetObjectMeta().GetName()] = r.Spec.Version
}
return installed, nil
}
7 changes: 7 additions & 0 deletions pkg/receipt/receipt.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"sigs.k8s.io/yaml"

"sigs.k8s.io/krew/pkg/index"
"sigs.k8s.io/krew/pkg/index/indexscanner"
)

// Store saves the given plugin at the destination.
Expand All @@ -34,3 +35,9 @@ func Store(plugin index.Plugin, dest string) error {
err = ioutil.WriteFile(dest, yamlBytes, 0644)
return errors.Wrapf(err, "write plugin receipt %q", dest)
}

// Load reads the plugin receipt at the specified destination.
// If not found, it returns os.IsNotExist error.
func Load(path string) (index.Plugin, error) {
return indexscanner.ReadPluginFile(path)
}
19 changes: 19 additions & 0 deletions pkg/receipt/receipt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package receipt

import (
"os"
"path/filepath"
"testing"

"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -68,3 +70,20 @@ func TestStore(t *testing.T) {
t.Fatal(diff)
}
}

func TestLoad(t *testing.T) {
// TODO(ahmetb): Avoid reading test data from other packages. It would be
// good to have an in-memory Plugin object (issue#270) that we can Store()
// first then load here.
_, err := Load(filepath.Join("..", "..", "integration_test", "testdata", "foo.yaml"))
if err != nil {
t.Fatal(err)
}
}

func TestLoad_preservesNonExistsError(t *testing.T) {
_, err := Load(filepath.Join("foo", "non-existing.yaml"))
if !os.IsNotExist(err) {
t.Fatalf("returned error is not ENOENT: %+v", err)
}
}