Skip to content
This repository has been archived by the owner on Jun 13, 2021. It is now read-only.

Align docker app with docker run UX #681

Merged
merged 3 commits into from
Oct 14, 2019
Merged

Align docker app with docker run UX #681

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
38753b7
Align docker app with `docker run` UX
ndeloof Oct 10, 2019
072287b
Fix help to match current behaviour
ndeloof Oct 14, 2019
f0cd91c
app_image is a required parameter
ndeloof Oct 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions e2e/cnab_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func TestCallCustomStatusAction(t *testing.T) {
icmd.RunCmd(cmd).Assert(t, icmd.Success)

// docker app install
cmd.Command = dockerCli.Command("app", "install", path.Join(testDir, "bundle.json"), "--name", testCase.name)
cmd.Command = dockerCli.Command("app", "run", path.Join(testDir, "bundle.json"), "--name", testCase.name)
silvin-lubecki marked this conversation as resolved.
Show resolved Hide resolved
icmd.RunCmd(cmd).Assert(t, icmd.Success)

// docker app uninstall
Expand Down Expand Up @@ -78,7 +78,7 @@ func TestCnabParameters(t *testing.T) {
}()

// docker app install
cmd.Command = dockerCli.Command("app", "install", path.Join(testDir, "bundle.json"), "--name", "cnab-parameters",
cmd.Command = dockerCli.Command("app", "run", path.Join(testDir, "bundle.json"), "--name", "cnab-parameters",
"--set", "boolParam=true",
"--set", "stringParam=value",
"--set", "intParam=42",
Expand Down
16 changes: 8 additions & 8 deletions e2e/commands_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func testDockerAppLifecycle(t *testing.T, useBindMount bool) {
initializeDockerAppEnvironment(t, &cmd, tmpDir, swarm, useBindMount)

// Install an illformed Docker Application Package
cmd.Command = dockerCli.Command("app", "install", "testdata/simple/simple.dockerapp", "--set", "web_port=-1", "--name", appName)
cmd.Command = dockerCli.Command("app", "run", "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",
Expand All @@ -220,7 +220,7 @@ func testDockerAppLifecycle(t *testing.T, useBindMount bool) {
})

// Install a Docker Application Package with an existing failed installation is fine
cmd.Command = dockerCli.Command("app", "install", "testdata/simple/simple.dockerapp", "--name", appName)
cmd.Command = dockerCli.Command("app", "run", "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),
Expand All @@ -240,7 +240,7 @@ func testDockerAppLifecycle(t *testing.T, useBindMount bool) {
})

// Installing again the same application is forbidden
cmd.Command = dockerCli.Command("app", "install", "testdata/simple/simple.dockerapp", "--name", appName)
cmd.Command = dockerCli.Command("app", "run", "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),
Expand Down Expand Up @@ -307,7 +307,7 @@ func TestCredentials(t *testing.T) {

t.Run("missing", func(t *testing.T) {
cmd.Command = dockerCli.Command(
"app", "install",
"app", "run",
"--credential", "secret1=foo",
// secret2 deliberately omitted.
"--credential", "secret3=baz",
Expand All @@ -322,7 +322,7 @@ func TestCredentials(t *testing.T) {

t.Run("full", func(t *testing.T) {
cmd.Command = dockerCli.Command(
"app", "install",
"app", "run",
"--credential", "secret1=foo",
"--credential", "secret2=bar",
"--credential", "secret3=baz",
Expand All @@ -334,7 +334,7 @@ func TestCredentials(t *testing.T) {

t.Run("mixed-credstore", func(t *testing.T) {
cmd.Command = dockerCli.Command(
"app", "install",
"app", "run",
"--credential-set", "test-creds",
"--credential", "secret3=xyzzy",
"--name", "mixed-credstore", bundle,
Expand All @@ -345,7 +345,7 @@ func TestCredentials(t *testing.T) {

t.Run("mixed-local-cred", func(t *testing.T) {
cmd.Command = dockerCli.Command(
"app", "install",
"app", "run",
"--credential-set", tmpDir.Join("local", "test-creds.yaml"),
"--credential", "secret3=xyzzy",
"--name", "mixed-local-cred", bundle,
Expand All @@ -356,7 +356,7 @@ func TestCredentials(t *testing.T) {

t.Run("overload", func(t *testing.T) {
cmd.Command = dockerCli.Command(
"app", "install",
"app", "run",
"--credential-set", "test-creds",
"--credential", "secret1=overload",
"--credential", "secret3=xyzzy",
Expand Down
12 changes: 6 additions & 6 deletions e2e/pushpull_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func TestPushInstall(t *testing.T) {
cmd.Command = dockerCli.Command("app", "push", "--tag", ref, filepath.Join("testdata", "push-pull", "push-pull.dockerapp"))
icmd.RunCmd(cmd).Assert(t, icmd.Success)

cmd.Command = dockerCli.Command("app", "install", ref, "--name", t.Name())
cmd.Command = dockerCli.Command("app", "run", ref, "--name", t.Name())
icmd.RunCmd(cmd).Assert(t, icmd.Success)
cmd.Command = dockerCli.Command("service", "ls")
assert.Check(t, cmp.Contains(icmd.RunCmd(cmd).Assert(t, icmd.Success).Combined(), ref))
Expand All @@ -165,7 +165,7 @@ func TestPushPullInstall(t *testing.T) {
info.stopRegistry()

// install from local store
cmd.Command = dockerCli.Command("app", "install", ref+tag, "--name", t.Name())
cmd.Command = dockerCli.Command("app", "run", ref+tag, "--name", t.Name())
icmd.RunCmd(cmd).Assert(t, icmd.Success)
cmd.Command = dockerCli.Command("service", "ls")
assert.Check(t, cmp.Contains(icmd.RunCmd(cmd).Assert(t, icmd.Success).Combined(), ref))
Expand All @@ -178,7 +178,7 @@ func TestPushPullInstall(t *testing.T) {
})

// install should fail (registry is stopped)
cmd.Command = dockerCli.Command("app", "install", "unknown")
cmd.Command = dockerCli.Command("app", "run", "unknown")
//nolint: lll
expected := `Unable to find application image "unknown:latest" locally
Unable to find application "unknown": failed to resolve bundle manifest "docker.io/library/unknown:latest": pull access denied, repository does not exist or may require authorization: server message: insufficient_scope: authorization failed`
Expand All @@ -204,7 +204,7 @@ func TestPushInstallBundle(t *testing.T) {
cmd.Command = dockerCli.Command("app", "push", "--tag", ref, "a-simple-app:1.0.0")
icmd.RunCmd(cmd).Assert(t, icmd.Success)

cmd.Command = dockerCli.Command("app", "install", ref, "--name", name)
cmd.Command = dockerCli.Command("app", "run", ref, "--name", name)
icmd.RunCmd(cmd).Assert(t, icmd.Success)
cmd.Command = dockerCli.Command("service", "ls")
assert.Check(t, cmp.Contains(icmd.RunCmd(cmd).Assert(t, icmd.Success).Combined(), ref))
Expand All @@ -224,7 +224,7 @@ func TestPushInstallBundle(t *testing.T) {
cmd.Command = dockerCli.Command("app", "push", "--tag", ref2, ref+":latest")
icmd.RunCmd(cmd).Assert(t, icmd.Success)

cmd.Command = dockerCli.Command("app", "install", ref2, "--name", name)
cmd.Command = dockerCli.Command("app", "run", ref2, "--name", name)
icmd.RunCmd(cmd).Assert(t, icmd.Success)
cmd.Command = dockerCli.Command("service", "ls")
assert.Check(t, cmp.Contains(icmd.RunCmd(cmd).Assert(t, icmd.Success).Combined(), ref2))
Expand All @@ -251,7 +251,7 @@ func TestPushInstallBundle(t *testing.T) {
// remove the bundle from the bundle store to be sure it won't be used instead of registry
cleanupIsolatedStore()
// install from the registry
cmd.Command = dockerCli.Command("app", "install", ref2, "--name", name)
cmd.Command = dockerCli.Command("app", "run", ref2, "--name", name)
icmd.RunCmd(cmd).Assert(t, icmd.Success)
cmd.Command = dockerCli.Command("service", "ls")
assert.Check(t, cmp.Contains(icmd.RunCmd(cmd).Assert(t, icmd.Success).Combined(), ref))
Expand Down
2 changes: 1 addition & 1 deletion e2e/testdata/plugin-usage-experimental.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ Commands:
build Build service images for the application
init Initialize Docker Application definition
inspect Shows metadata, parameters and a summary of the Compose file for a given application
install Install an application
ls List the installations and their last known installation result
pull Pull an application package from a registry
push Push an application package to a registry
rm Remove an application
run Run an application
upgrade Upgrade an installed application
validate Checks the rendered application is syntactically correct

Expand Down
2 changes: 1 addition & 1 deletion e2e/testdata/plugin-usage.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ Commands:
build Build service images for the application
init Initialize Docker Application definition
inspect Shows metadata, parameters and a summary of the Compose file for a given application
install Install an application
ls List the installations and their last known installation result
pull Pull an application package from a registry
push Push an application package to a registry
rm Remove an application
run Run an application
upgrade Upgrade an installed application
validate Checks the rendered application is syntactically correct

Expand Down
2 changes: 1 addition & 1 deletion internal/commands/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func NewRootCmd(use string, dockerCli command.Cli) *cobra.Command {

func addCommands(cmd *cobra.Command, dockerCli command.Cli) {
cmd.AddCommand(
installCmd(dockerCli),
runCmd(dockerCli),
upgradeCmd(dockerCli),
removeCmd(dockerCli),
listCmd(dockerCli),
Expand Down
24 changes: 9 additions & 15 deletions internal/commands/install.go → internal/commands/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
"github.com/spf13/cobra"
)

type installOptions struct {
type runOptions struct {
parametersOptions
credentialOptions
orchestrator string
Expand All @@ -33,28 +33,22 @@ const (
nameKindReference
)

const longDescription = `Install an application.
By default, the application definition in the current directory will be
installed. The APP_NAME can also be:
- a path to a Docker Application definition (.dockerapp) or a CNAB bundle.json
- a registry Application Package reference`
const longDescription = `Run an application based on a docker app image.`

const example = `$ docker app install myapp.dockerapp --name myinstallation --target-context=mycontext
$ docker app install myrepo/myapp:mytag --name myinstallation --target-context=mycontext
$ docker app install bundle.json --name myinstallation --credential-set=mycredentials.yml`
const example = `$ docker app run myrepo/myapp:mytag --name myinstallation --target-context=mycontext`

func installCmd(dockerCli command.Cli) *cobra.Command {
var opts installOptions
func runCmd(dockerCli command.Cli) *cobra.Command {
var opts runOptions

cmd := &cobra.Command{
Use: "install [APP_NAME] [--name INSTALLATION_NAME] [--target-context TARGET_CONTEXT] [OPTIONS]",
Use: "run [APP_NAME] [--name INSTALLATION_NAME] [--target-context TARGET_CONTEXT] [OPTIONS]",
Copy link
Contributor

Choose a reason for hiding this comment

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

Use: "run [OPTIONS] APP_IMAGE" ? To better fit docker run synopsis ?

Aliases: []string{"deploy"},
Short: "Install an application",
Short: "Run an application",
Long: longDescription,
Example: example,
Args: cli.RequiresMaxArgs(1),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be ExactArgs(1) ?

RunE: func(cmd *cobra.Command, args []string) error {
return runInstall(dockerCli, firstOrEmpty(args), opts)
return runRun(dockerCli, firstOrEmpty(args), opts)
Copy link
Contributor

Choose a reason for hiding this comment

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

runRun! 🏃

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't be firstOrEmpty(args) I think right? Now the command needs the app image reference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mayeb I should rename RunForestRun ?

Copy link
Contributor

Choose a reason for hiding this comment

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

or RunLolaRun?

},
}
opts.parametersOptions.addFlags(cmd.Flags())
Expand All @@ -66,7 +60,7 @@ func installCmd(dockerCli command.Cli) *cobra.Command {
return cmd
}

func runInstall(dockerCli command.Cli, appname string, opts installOptions) error {
func runRun(dockerCli command.Cli, appname string, opts runOptions) error {
opts.SetDefaultTargetContext(dockerCli)

bind, err := requiredBindMount(opts.targetContext, opts.orchestrator, dockerCli.ContextStore())
Expand Down