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.

Sign up for GitHub

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

Jump to bottom

Add --dry-run and --verbose flags #23

Merged
merged 1 commit into from
Jun 24, 2021
+68 −9
Merged

Add --dry-run and --verbose flags #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
43 changes: 37 additions & 6 deletions src/cmd/run/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,17 @@ func New() *cobra.Command {

cmd.Flags().BoolVar(options.Remove, "rm", false, "remove container upon exit")
cmd.Flags().BoolVar(&options.Reset, "reset", false, "remove previous container before starting a fresh one")
cmd.Flags().BoolVarP(&options.Verbose, "verbose", "v", false, "output detailed execution information to stderr")
cmd.Flags().BoolVar(&options.DryRun, "dry-run", false, "output docker command to stdout instead of executing it")

return cmd
}

type Options struct {
Remove *bool
Reset bool
Remove *bool
Reset bool
Verbose bool
DryRun bool
}

func run(ctx context.Context, names []string, options Options) error {
Expand Down Expand Up @@ -70,26 +74,53 @@ func run(ctx context.Context, names []string, options Options) error {
logging.Log("using image: %s", yeyContext.Image)
}

if options.Verbose {
fmt.Fprintf(os.Stderr, "context:\n--\n%v--\n", yeyContext)
}

// Container name
containerName := yey.ContainerName(contexts.Path, yeyContext)
if options.Verbose {
fmt.Fprintf(os.Stderr, "container: %s\n", containerName)
}

// Reset
if options.Reset {
if options.Verbose {
fmt.Fprintf(os.Stderr, "removing container first")
}
if err := docker.Remove(ctx, containerName); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

i just realized there is a bug here. What if we run with reset but the container doesn't exist? We don't want to fail. Not to do with the PR but just thought I would flag it. I probably wrote the bug.

return fmt.Errorf("failed to remove container %q: %w", containerName, err)
}
}

var runOptions []docker.RunOption
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can probably do away with creating an options slice since all options accept their values. we can pass the empty string to WithWorkDir because even if we dont't pass the option it probably initialises the backing option to empty string anyways.


// Working directory
workDir, err := getContainerWorkDir(yeyContext)
if err != nil {
return err
}

var runOptions []docker.RunOption
if workDir != "" {
runOptions = append(runOptions, docker.WithWorkDir(workDir))
}
if options.Verbose {
fmt.Fprintf(os.Stderr, "working directory: %s", workDir)
}

if err := ShowBanner(yeyContext.Name); err != nil {
return err
// Dry-run/verbose
runOptions = append(runOptions,
docker.WithDryRun(options.DryRun),
docker.WithVerbose(options.Verbose))

// Banner
if options.Verbose {
fmt.Fprintln(os.Stderr)
}
if !options.DryRun {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Dry-run does not print the banner but still starts the docker process? Wouldn't the flag be --no-banner?

Copy link
Collaborator

Choose a reason for hiding this comment

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

nvm

if err := ShowBanner(yeyContext.Name); err != nil {
return err
}
}

return docker.Start(ctx, yeyContext, containerName, runOptions...)
Expand Down
34 changes: 31 additions & 3 deletions src/internal/docker/cli.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import (

type runOptions struct {
workDir string
verbose bool
dryRun bool
}

type RunOption func(*runOptions)
Expand All @@ -26,6 +28,18 @@ func WithWorkDir(wd string) RunOption {
}
}

func WithVerbose(value bool) RunOption {
return func(ro *runOptions) {
ro.verbose = value
}
}

func WithDryRun(value bool) RunOption {
return func(ro *runOptions) {
ro.dryRun = value
}
}

func Start(ctx context.Context, yeyCtx yey.Context, containerName string, opts ...RunOption) error {
var options runOptions
for _, opt := range opts {
Expand All @@ -42,7 +56,7 @@ func Start(ctx context.Context, yeyCtx yey.Context, containerName string, opts .
case "":
return runContainer(ctx, yeyCtx, containerName, options)
case "exited":
return startContainer(ctx, containerName)
return startContainer(ctx, containerName, options)
case "running":
return execContainer(ctx, containerName, yeyCtx.Cmd, options)
default:
Expand Down Expand Up @@ -205,11 +219,21 @@ func runContainer(ctx context.Context, yeyCtx yey.Context, containerName string,
args = append(args, yeyCtx.Image)
args = append(args, yeyCtx.Cmd...)

if options.dryRun {
fmt.Printf("docker %s\n", strings.Join(args, " "))
return nil
}

return attachStdPipes(exec.CommandContext(ctx, "docker", args...)).Run()
}

func startContainer(ctx context.Context, containerName string) error {
return attachStdPipes(exec.CommandContext(ctx, "docker", "start", "-i", containerName)).Run()
func startContainer(ctx context.Context, containerName string, options runOptions) error {
args := []string{"start", "-i", containerName}
if options.dryRun {
fmt.Printf("docker %s\n", strings.Join(args, " "))
return nil
}
return attachStdPipes(exec.CommandContext(ctx, "docker", args...)).Run()
}

func execContainer(ctx context.Context, containerName string, cmd []string, options runOptions) error {
Expand All @@ -219,6 +243,10 @@ func execContainer(ctx context.Context, containerName string, cmd []string, opti
}
args = append(args, containerName)
args = append(args, cmd...)
if options.dryRun {
fmt.Printf("docker %s\n", strings.Join(args, " "))
return nil
}
return attachStdPipes(exec.CommandContext(ctx, "docker", args...)).Run()
}

Expand Down