Merge pull request #514 from silvin-lubecki/installation-error

Installation is no longer blocked in case of previous failure

Author: silvin-lubecki

e2e/commands_test.go

@@ -305,53 +305,35 @@ func testDockerAppLifecycle(t *testing.T, useBindMount bool) {
 	cmd, cleanup := dockerCli.createTestCmd()
 	defer cleanup()
 	appName := strings.Replace(t.Name(), "/", "_", 1)
-
 	tmpDir := fs.NewDir(t, appName)
 	defer tmpDir.Remove()
-
-	cmd.Env = append(cmd.Env, "DOCKER_TARGET_CONTEXT=swarm-target-context")
-
 	// Running a swarm using docker in docker to install the application
 	// and run the invocation image
 	swarm := NewContainer("docker:18.09-dind", 2375)
 	swarm.Start(t)
 	defer swarm.Stop(t)
+	initializeDockerAppEnvironment(t, &cmd, tmpDir, swarm, useBindMount)
 
-	// The dind doesn't have the cnab-app-base image so we save it in order to load it later
-	icmd.RunCommand(dockerCli.path, "save", fmt.Sprintf("docker/cnab-app-base:%s", internal.Version), "--output", tmpDir.Join("cnab-app-base.tar.gz")).Assert(t, icmd.Success)
-
-	// We  need two contexts:
-	// - one for `docker` so that it connects to the dind swarm created before
-	// - the target context for the invocation image to install within the swarm
-	cmd.Command = dockerCli.Command("context", "create", "swarm-context", "--docker", fmt.Sprintf(`"host=tcp://%s"`, swarm.GetAddress(t)), "--default-stack-orchestrator", "swarm")
-	icmd.RunCmd(cmd).Assert(t, icmd.Success)
-
-	// When creating a context on a Windows host we cannot use
-	// the unix socket but it's needed inside the invocation image.
-	// The workaround is to create a context with an empty host.
-	// This host will default to the unix socket inside the
-	// invocation image
-	host := "host="
-	if !useBindMount {
-		host += fmt.Sprintf("tcp://%s", swarm.GetPrivateAddress(t))
-	}
-
-	cmd.Command = dockerCli.Command("context", "create", "swarm-target-context", "--docker", host, "--default-stack-orchestrator", "swarm")
-	icmd.RunCmd(cmd).Assert(t, icmd.Success)
-
-	// Initialize the swarm
-	cmd.Env = append(cmd.Env, "DOCKER_CONTEXT=swarm-context")
-	cmd.Command = dockerCli.Command("swarm", "init")
-	icmd.RunCmd(cmd).Assert(t, icmd.Success)
+	// Install an illformed Docker Application Package
+	cmd.Command = dockerCli.Command("app", "install", "testdata/simple/simple.dockerapp", "--set", "web_port=-1", "--name", appName)
+	icmd.RunCmd(cmd).Assert(t, icmd.Expected{
+		ExitCode: 1,
+		Err:      "error decoding 'Ports': Invalid hostPort: -1",
+	})
+	// TODO: List the installation and check the failed status
 
-	// Load the needed base cnab image into the swarm docker engine
-	cmd.Command = dockerCli.Command("load", "--input", tmpDir.Join("cnab-app-base.tar.gz"))
-	icmd.RunCmd(cmd).Assert(t, icmd.Success)
+	// Upgrading a failed installation is not allowed
+	cmd.Command = dockerCli.Command("app", "upgrade", appName)
+	icmd.RunCmd(cmd).Assert(t, icmd.Expected{
+		ExitCode: 1,
+		Err:      fmt.Sprintf("Installation %q has failed and cannot be upgraded, reinstall it using 'docker app install'", appName),
+	})
 
-	// Install a Docker Application Package
+	// Install a Docker Application Package with an existing failed installation is fine
 	cmd.Command = dockerCli.Command("app", "install", "testdata/simple/simple.dockerapp", "--name", appName)
 	checkContains(t, icmd.RunCmd(cmd).Assert(t, icmd.Success).Combined(),
 		[]string{
+			fmt.Sprintf("WARNING: installing over previously failed installation %q", appName),
 			fmt.Sprintf("Creating network %s_back", appName),
 			fmt.Sprintf("Creating network %s_front", appName),
 			fmt.Sprintf("Creating service %s_db", appName),
@@ -368,6 +350,13 @@ func testDockerAppLifecycle(t *testing.T, useBindMount bool) {
 			fmt.Sprintf("[[:alnum:]]+        %s_api   replicated          [0-1]/1                 python:3.6", appName),
 		})
 
+	// Installing again the same application is forbidden
+	cmd.Command = dockerCli.Command("app", "install", "testdata/simple/simple.dockerapp", "--name", appName)
+	icmd.RunCmd(cmd).Assert(t, icmd.Expected{
+		ExitCode: 1,
+		Err:      fmt.Sprintf("Installation %q already exists, use 'docker app upgrade' instead", appName),
+	})
+
 	// Upgrade the application, changing the port
 	cmd.Command = dockerCli.Command("app", "upgrade", appName, "--set", "web_port=8081")
 	checkContains(t, icmd.RunCmd(cmd).Assert(t, icmd.Success).Combined(),
@@ -393,6 +382,41 @@ func testDockerAppLifecycle(t *testing.T, useBindMount bool) {
 		})
 }
 
+func initializeDockerAppEnvironment(t *testing.T, cmd *icmd.Cmd, tmpDir *fs.Dir, swarm *Container, useBindMount bool) {
+	cmd.Env = append(cmd.Env, "DOCKER_TARGET_CONTEXT=swarm-target-context")
+
+	// The dind doesn't have the cnab-app-base image so we save it in order to load it later
+	icmd.RunCommand(dockerCli.path, "save", fmt.Sprintf("docker/cnab-app-base:%s", internal.Version), "--output", tmpDir.Join("cnab-app-base.tar.gz")).Assert(t, icmd.Success)
+
+	// We  need two contexts:
+	// - one for `docker` so that it connects to the dind swarm created before
+	// - the target context for the invocation image to install within the swarm
+	cmd.Command = dockerCli.Command("context", "create", "swarm-context", "--docker", fmt.Sprintf(`"host=tcp://%s"`, swarm.GetAddress(t)), "--default-stack-orchestrator", "swarm")
+	icmd.RunCmd(*cmd).Assert(t, icmd.Success)
+
+	// When creating a context on a Windows host we cannot use
+	// the unix socket but it's needed inside the invocation image.
+	// The workaround is to create a context with an empty host.
+	// This host will default to the unix socket inside the
+	// invocation image
+	host := "host="
+	if !useBindMount {
+		host += fmt.Sprintf("tcp://%s", swarm.GetPrivateAddress(t))
+	}
+
+	cmd.Command = dockerCli.Command("context", "create", "swarm-target-context", "--docker", host, "--default-stack-orchestrator", "swarm")
+	icmd.RunCmd(*cmd).Assert(t, icmd.Success)
+
+	// Initialize the swarm
+	cmd.Env = append(cmd.Env, "DOCKER_CONTEXT=swarm-context")
+	cmd.Command = dockerCli.Command("swarm", "init")
+	icmd.RunCmd(*cmd).Assert(t, icmd.Success)
+
+	// Load the needed base cnab image into the swarm docker engine
+	cmd.Command = dockerCli.Command("load", "--input", tmpDir.Join("cnab-app-base.tar.gz"))
+	icmd.RunCmd(*cmd).Assert(t, icmd.Success)
+}
+
 func checkContains(t *testing.T, combined string, expectedLines []string) {
 	for _, expected := range expectedLines {
 		exp := regexp.MustCompile(expected)

internal/commands/cnab.go

@@ -335,3 +335,8 @@ func prepareCustomAction(actionName string, dockerCli command.Cli, appname strin
 	}
 	return a, c, errBuf, nil
 }
+
+func isInstallationFailed(installation *claim.Claim) bool {
+	return installation.Result.Action == claim.ActionInstall &&
+		installation.Result.Status == claim.StatusFailure
+}

internal/commands/install.go

@@ -2,6 +2,7 @@ package commands
 
 import (
 	"fmt"
+	"os"
 
 	"github.com/deislabs/duffle/pkg/action"
 	"github.com/deislabs/duffle/pkg/claim"
@@ -90,8 +91,15 @@ func runInstall(dockerCli command.Cli, appname string, opts installOptions) erro
 	if installationName == "" {
 		installationName = bndl.Name
 	}
-	if _, err = installationStore.Read(installationName); err == nil {
-		return fmt.Errorf("installation %q already exists", installationName)
+	if installation, err := installationStore.Read(installationName); err == nil {
+		// A failed installation can be overridden, but with a warning
+		if isInstallationFailed(&installation) {
+			fmt.Fprintf(os.Stderr, "WARNING: installing over previously failed installation %q\n", installationName)
+		} else {
+			// Return an error in case of successful installation, or even failed upgrade, which means
+			// their was already a successful installation.
+			return fmt.Errorf("Installation %q already exists, use 'docker app upgrade' instead", installationName)
+		}
 	}
 	c, err := claim.New(installationName)
 	if err != nil {
@@ -123,12 +131,17 @@ func runInstall(dockerCli command.Cli, appname string, opts installOptions) erro
 	inst := &action.Install{
 		Driver: driverImpl,
 	}
-	err = inst.Run(c, creds, dockerCli.Out())
+	err = inst.Run(c, creds, os.Stdout)
 	// Even if the installation failed, the installation is persisted with its failure status,
 	// so any installation needs a clean uninstallation.
 	err2 := installationStore.Store(*c)
 	if err != nil {
-		return fmt.Errorf("install failed: %s", errBuf)
+		return fmt.Errorf("Installation failed: %s", errBuf)
 	}
-	return err2
+	if err2 != nil {
+		return err2
+	}
+
+	fmt.Printf("Application %q installed on context %q\n", installationName, opts.targetContext)
+	return nil
 }

internal/commands/uninstall.go

@@ -2,6 +2,7 @@ package commands
 
 import (
 	"fmt"
+	"os"
 
 	"github.com/deislabs/duffle/pkg/action"
 	"github.com/deislabs/duffle/pkg/credentials"
@@ -58,12 +59,15 @@ func runUninstall(dockerCli command.Cli, installationName string, opts credentia
 	uninst := &action.Uninstall{
 		Driver: driverImpl,
 	}
-	err = uninst.Run(&c, creds, dockerCli.Out())
-	if err == nil {
-		return installationStore.Delete(installationName)
+	if err := uninst.Run(&c, creds, os.Stdout); err != nil {
+		if err2 := installationStore.Store(c); err2 != nil {
+			return fmt.Errorf("%s while %s", err2, errBuf)
+		}
+		return fmt.Errorf("Uninstall failed: %s", errBuf)
 	}
-	if err2 := installationStore.Store(c); err2 != nil {
-		fmt.Fprintf(dockerCli.Err(), "failed to update installation: %s\n", err2)
+	if err := installationStore.Delete(installationName); err != nil {
+		return fmt.Errorf("Failed to delete installation %q from the installation store: %s", installationName, err)
 	}
-	return fmt.Errorf("uninstall failed: %s", errBuf)
+	fmt.Printf("Application %q uninstalled on context %q\n", installationName, opts.targetContext)
+	return nil
 }

internal/commands/upgrade.go

@@ -2,6 +2,7 @@ package commands
 
 import (
 	"fmt"
+	"os"
 
 	"github.com/deislabs/duffle/pkg/action"
 	"github.com/deislabs/duffle/pkg/credentials"
@@ -46,48 +47,56 @@ func runUpgrade(dockerCli command.Cli, installationName string, opts upgradeOpti
 		return err
 	}
 
-	c, err := installationStore.Read(installationName)
+	installation, err := installationStore.Read(installationName)
 	if err != nil {
 		return err
 	}
 
+	if isInstallationFailed(&installation) {
+		return fmt.Errorf("Installation %q has failed and cannot be upgraded, reinstall it using 'docker app install'", installationName)
+	}
+
 	if opts.bundleOrDockerApp != "" {
 		b, err := resolveBundle(dockerCli, bundleStore, opts.bundleOrDockerApp, opts.pull, opts.insecureRegistries)
 		if err != nil {
 			return err
 		}
-		c.Bundle = b
+		installation.Bundle = b
 	}
-	if err := mergeBundleParameters(&c,
+	if err := mergeBundleParameters(&installation,
 		withFileParameters(opts.parametersFiles),
 		withCommandLineParameters(opts.overrides),
 		withSendRegistryAuth(opts.sendRegistryAuth),
 	); err != nil {
 		return err
 	}
 
-	bind, err := requiredClaimBindMount(c, opts.targetContext, dockerCli)
+	bind, err := requiredClaimBindMount(installation, opts.targetContext, dockerCli)
 	if err != nil {
 		return err
 	}
 	driverImpl, errBuf, err := prepareDriver(dockerCli, bind, nil)
 	if err != nil {
 		return err
 	}
-	creds, err := prepareCredentialSet(c.Bundle, opts.CredentialSetOpts(dockerCli, credentialStore)...)
+	creds, err := prepareCredentialSet(installation.Bundle, opts.CredentialSetOpts(dockerCli, credentialStore)...)
 	if err != nil {
 		return err
 	}
-	if err := credentials.Validate(creds, c.Bundle.Credentials); err != nil {
+	if err := credentials.Validate(creds, installation.Bundle.Credentials); err != nil {
 		return err
 	}
 	u := &action.Upgrade{
 		Driver: driverImpl,
 	}
-	err = u.Run(&c, creds, dockerCli.Out())
-	err2 := installationStore.Store(c)
+	err = u.Run(&installation, creds, os.Stdout)
+	err2 := installationStore.Store(installation)
 	if err != nil {
-		return fmt.Errorf("upgrade failed: %s", errBuf)
+		return fmt.Errorf("Upgrade failed: %s", errBuf)
+	}
+	if err2 != nil {
+		return err2
 	}
-	return err2
+	fmt.Printf("Application %q upgraded on context %q\n", installationName, opts.targetContext)
+	return nil
 }

internal/store/app.go

@@ -57,7 +57,8 @@ func (a ApplicationStore) InstallationStore(context string) (InstallationStore,
 	if err := os.MkdirAll(path, 0755); err != nil {
 		return nil, errors.Wrapf(err, "failed to create installation store directory for context %q", context)
 	}
-	return claim.NewClaimStore(crud.NewFileSystemStore(path, "json")), nil
+	claimStore := claim.NewClaimStore(crud.NewFileSystemStore(path, "json"))
+	return &installationStore{claimStore: claimStore}, nil
 }
 
 // CredentialStore initializes and returns a context based credential store

internal/store/installation.go

@@ -1,13 +1,41 @@
 package store
 
-import "github.com/deislabs/duffle/pkg/claim"
+import (
+	"fmt"
+
+	"github.com/deislabs/duffle/pkg/claim"
+)
 
 // InstallationStore is an interface to persist claims.
 type InstallationStore interface {
 	List() ([]string, error)
-	Store(installationName claim.Claim) error
+	Store(installation claim.Claim) error
 	Read(installationName string) (claim.Claim, error)
 	Delete(installationName string) error
 }
 
-var _ InstallationStore = &claim.Store{}
+var _ InstallationStore = &installationStore{}
+
+type installationStore struct {
+	claimStore claim.Store
+}
+
+func (i installationStore) List() ([]string, error) {
+	return i.claimStore.List()
+}
+
+func (i installationStore) Store(installation claim.Claim) error {
+	return i.claimStore.Store(installation)
+}
+
+func (i installationStore) Read(installationName string) (claim.Claim, error) {
+	c, err := i.claimStore.Read(installationName)
+	if err == claim.ErrClaimNotFound {
+		return claim.Claim{}, fmt.Errorf("Installation %q not found", installationName)
+	}
+	return c, err
+}
+
+func (i installationStore) Delete(installationName string) error {
+	return i.claimStore.Delete(installationName)
+}