Commit 3e5642a4 authored by Taylor Thomas's avatar Taylor Thomas Committed by GitHub

Merge pull request #2400 from sushilkm/issues/2384

Fixes various issues related to plugin command
parents 5c227bff 084bbfa2
...@@ -16,9 +16,11 @@ limitations under the License. ...@@ -16,9 +16,11 @@ limitations under the License.
package main package main
import ( import (
"errors"
"fmt" "fmt"
"io" "io"
"os" "os"
"strings"
"k8s.io/helm/pkg/helm/helmpath" "k8s.io/helm/pkg/helm/helmpath"
"k8s.io/helm/pkg/plugin" "k8s.io/helm/pkg/plugin"
...@@ -48,8 +50,8 @@ func newPluginRemoveCmd(out io.Writer) *cobra.Command { ...@@ -48,8 +50,8 @@ func newPluginRemoveCmd(out io.Writer) *cobra.Command {
} }
func (pcmd *pluginRemoveCmd) complete(args []string) error { func (pcmd *pluginRemoveCmd) complete(args []string) error {
if err := checkArgsLength(len(args), "plugin"); err != nil { if len(args) == 0 {
return err return errors.New("please provide plugin name to remove")
} }
pcmd.names = args pcmd.names = args
pcmd.home = settings.Home pcmd.home = settings.Home
...@@ -63,15 +65,21 @@ func (pcmd *pluginRemoveCmd) run() error { ...@@ -63,15 +65,21 @@ func (pcmd *pluginRemoveCmd) run() error {
if err != nil { if err != nil {
return err return err
} }
var errorPlugins []string
for _, name := range pcmd.names { for _, name := range pcmd.names {
if found := findPlugin(plugins, name); found != nil { if found := findPlugin(plugins, name); found != nil {
if err := removePlugin(found, pcmd.home); err != nil { if err := removePlugin(found, pcmd.home); err != nil {
return err errorPlugins = append(errorPlugins, fmt.Sprintf("Failed to remove plugin %s, got error (%v)", name, err))
} else {
fmt.Fprintf(pcmd.out, "Removed plugin: %s\n", name)
} }
fmt.Fprintf(pcmd.out, "Removed plugin: %s\n", name) } else {
errorPlugins = append(errorPlugins, fmt.Sprintf("Plugin: %s not found", name))
} }
} }
if len(errorPlugins) > 0 {
return fmt.Errorf(strings.Join(errorPlugins, "\n"))
}
return nil return nil
} }
......
...@@ -40,6 +40,10 @@ type Installer interface { ...@@ -40,6 +40,10 @@ type Installer interface {
// Install installs a plugin to $HELM_HOME. // Install installs a plugin to $HELM_HOME.
func Install(i Installer) error { func Install(i Installer) error {
if _, pathErr := os.Stat(i.Path()); !os.IsNotExist(pathErr) {
return errors.New("plugin already exists")
}
return i.Install() return i.Install()
} }
......
...@@ -31,7 +31,7 @@ func TestLocalInstaller(t *testing.T) { ...@@ -31,7 +31,7 @@ func TestLocalInstaller(t *testing.T) {
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
defer os.Remove(hh) defer os.RemoveAll(hh)
home := helmpath.Home(hh) home := helmpath.Home(hh)
if err := os.MkdirAll(home.Plugins(), 0755); err != nil { if err := os.MkdirAll(home.Plugins(), 0755); err != nil {
...@@ -43,7 +43,7 @@ func TestLocalInstaller(t *testing.T) { ...@@ -43,7 +43,7 @@ func TestLocalInstaller(t *testing.T) {
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
defer os.Remove(tdir) defer os.RemoveAll(tdir)
if err := ioutil.WriteFile(filepath.Join(tdir, "plugin.yaml"), []byte{}, 0644); err != nil { if err := ioutil.WriteFile(filepath.Join(tdir, "plugin.yaml"), []byte{}, 0644); err != nil {
t.Fatal(err) t.Fatal(err)
} }
......
...@@ -16,6 +16,7 @@ limitations under the License. ...@@ -16,6 +16,7 @@ limitations under the License.
package installer // import "k8s.io/helm/pkg/plugin/installer" package installer // import "k8s.io/helm/pkg/plugin/installer"
import ( import (
"fmt"
"os" "os"
"sort" "sort"
...@@ -112,7 +113,8 @@ func (i *VCSInstaller) solveVersion(repo vcs.Repo) (string, error) { ...@@ -112,7 +113,8 @@ func (i *VCSInstaller) solveVersion(repo vcs.Repo) (string, error) {
return ver, nil return ver, nil
} }
} }
return "", nil
return "", fmt.Errorf("requested version %q does not exist for plugin %q", i.Version, i.Repo.Remote())
} }
// setVersion attempts to checkout the version // setVersion attempts to checkout the version
......
...@@ -16,8 +16,10 @@ limitations under the License. ...@@ -16,8 +16,10 @@ limitations under the License.
package installer // import "k8s.io/helm/pkg/plugin/installer" package installer // import "k8s.io/helm/pkg/plugin/installer"
import ( import (
"fmt"
"io/ioutil" "io/ioutil"
"os" "os"
"path/filepath"
"testing" "testing"
"k8s.io/helm/pkg/helm/helmpath" "k8s.io/helm/pkg/helm/helmpath"
...@@ -51,7 +53,7 @@ func TestVCSInstaller(t *testing.T) { ...@@ -51,7 +53,7 @@ func TestVCSInstaller(t *testing.T) {
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
defer os.Remove(hh) defer os.RemoveAll(hh)
home := helmpath.Home(hh) home := helmpath.Home(hh)
if err := os.MkdirAll(home.Plugins(), 0755); err != nil { if err := os.MkdirAll(home.Plugins(), 0755); err != nil {
...@@ -59,8 +61,9 @@ func TestVCSInstaller(t *testing.T) { ...@@ -59,8 +61,9 @@ func TestVCSInstaller(t *testing.T) {
} }
source := "https://github.com/adamreese/helm-env" source := "https://github.com/adamreese/helm-env"
testRepoPath, _ := filepath.Abs("../testdata/plugdir/echo")
repo := &testRepo{ repo := &testRepo{
local: "../testdata/plugdir/echo", local: testRepoPath,
tags: []string{"0.1.0", "0.1.1"}, tags: []string{"0.1.0", "0.1.1"},
} }
...@@ -87,4 +90,44 @@ func TestVCSInstaller(t *testing.T) { ...@@ -87,4 +90,44 @@ func TestVCSInstaller(t *testing.T) {
if i.Path() != home.Path("plugins", "helm-env") { if i.Path() != home.Path("plugins", "helm-env") {
t.Errorf("expected path '$HELM_HOME/plugins/helm-env', got %q", i.Path()) t.Errorf("expected path '$HELM_HOME/plugins/helm-env', got %q", i.Path())
} }
// Install again to test plugin exists error
if err := Install(i); err == nil {
t.Error("expected error for plugin exists, got none")
} else if err.Error() != "plugin already exists" {
t.Errorf("expected error for plugin exists, got (%v)", err)
}
}
func TestVCSInstallerNonExistentVersion(t *testing.T) {
hh, err := ioutil.TempDir("", "helm-home-")
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(hh)
home := helmpath.Home(hh)
if err := os.MkdirAll(home.Plugins(), 0755); err != nil {
t.Fatalf("Could not create %s: %s", home.Plugins(), err)
}
source := "https://github.com/adamreese/helm-env"
version := "0.2.0"
i, err := NewForSource(source, version, home)
if err != nil {
t.Errorf("unexpected error: %s", err)
}
// ensure a VCSInstaller was returned
_, ok := i.(*VCSInstaller)
if !ok {
t.Error("expected a VCSInstaller")
}
if err := Install(i); err == nil {
t.Error("expected error for version does not exists, got none")
} else if err.Error() != fmt.Sprintf("requested version %q does not exist for plugin %q", version, source) {
t.Errorf("expected error for version does not exists, got (%v)", err)
}
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment