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

exec command #27

Merged
merged 1 commit into from
Nov 20, 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
46 changes: 46 additions & 0 deletions cmd/exec.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package cmd

import (
"context"
"os"

"github.com/fromanirh/pack8s/internal/pkg/podman"
"github.com/spf13/cobra"
)

type execOptions struct {
commands []string
}

var execOpt execOptions

// NewExecCommand runs given command inside container
func NewExecCommand() *cobra.Command {
exec := &cobra.Command{
Use: "exec",
Short: "exec runs given command in container",
RunE: execCommand,
Copy link
Owner

Choose a reason for hiding this comment

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

I'd use

Args:  cobra.MinimumNArgs(1),

Args: cobra.MinimumNArgs(2),
}

return exec
}

func execCommand(cmd *cobra.Command, args []string) error {
containerID := args[0]
command := args[1:]

podmanSocket, err := cmd.Flags().GetString("podman-socket")
if err != nil {
return err
}

ctx := context.Background()

hnd, err := podman.NewHandle(ctx, podmanSocket)
if err != nil {
return err
}

return hnd.Exec(containerID, command, os.Stdout)
}
3 changes: 3 additions & 0 deletions cmd/okd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type okdRunOptions struct {
sshWorkerPort uint
background bool
randomPorts bool
volume string
}

var okdRunOpts okdRunOptions
Expand Down Expand Up @@ -64,6 +65,7 @@ func NewRunCommand() *cobra.Command {
run.Flags().UintVar(&okdRunOpts.sshWorkerPort, "ssh-worker-port", 0, "port on localhost to ssh to worker node")
run.Flags().BoolVar(&okdRunOpts.background, "background", false, "go to background after nodes are up")
run.Flags().BoolVar(&okdRunOpts.randomPorts, "random-ports", true, "expose all ports on random localhost ports")
run.Flags().StringVar(&okdRunOpts.volume, "volume", "", "Bind mount a volume into the container")
Copy link
Owner

Choose a reason for hiding this comment

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

Will be the volume automatically unbound? Do we need some cleanup?

Copy link
Owner

Choose a reason for hiding this comment

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

This is still relevant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Owner

Choose a reason for hiding this comment

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

This is fine. But does this also mean that the more exec we do on a container, the more volumes we pile in? what does it happen if we do two pack8s exec one after the other? will it work? will it pile up temporary volumes in the container?
Each call of pack8s exec must be idenpotent and do proper cleanup after itself; is not OK to rely on external tools (e.g. kubevirtci scripts) to do cleaning, because it's fragile and leads to surprising (and most likely unwanted) results.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The volume is bind during start of container and persist till stop of container. It will not add or remove any volume during exec. I agree pack8s should handle cleanup and as i mentioned in previous comment, it removes all binded volumes with rm command

Copy link
Owner

Choose a reason for hiding this comment

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

I'm not 100% sure it's a good idea to have this volume bound between host and container, but since this is strictly for development and we are in the early stage of development, I'm incline to try it out and see how it looks.

return run
}

Expand Down Expand Up @@ -157,6 +159,7 @@ func run(cmd *cobra.Command, args []string) (err error) {
Privileged: &okdRunOpts.privileged,
Publish: &clusterPorts,
PublishAll: &okdRunOpts.randomPorts,
Volume: &[]string{okdRunOpts.volume},
})
if err != nil {
return err
Expand Down
1 change: 1 addition & 0 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ func NewRootCommand() *cobra.Command {
NewSSHCommand(),
NewShowCommand(),
NewPruneVolumesCommand(),
NewExecCommand(),
)

return root
Expand Down