Support lxc attach options with RunCommand* about go-lxc HOT 11 CLOSED

I'm going to dig into this now and will submit a PR with proposed changes.

from go-lxc.

What are your thoughts on something like the following for the API:

c.RunCommand([]string{"/bin/sh"}, &AttachOptions{ClearEnv: true})

So the primary change would be the signature is always args + options. We could also add something like NewAttachOptions to set any defaults that might be needed.

from go-lxc.

I'm assuming you are proposing this instead your old approach (see below), right?

cmd := &Command{Args: []string{"/bin/sh"}, ClearEnv: true,}
cmd.Run(container)

If so I think I like this args + options (+NewAttachOptions for defaults) signature more than previous.

from go-lxc.

@caglar10ur yeah I am. I think it's a better approach, and if we're gonna break API compatibility it seems reasonable to me.

from go-lxc.

Oh by the way, would it be possible for you to base your commits on top of v2 branch?

from go-lxc.

@caglar10ur regarding the API, do you prefer RunCommand(options, args...) or RunCommand([]args, options) ?

from go-lxc.

I believe RunCommand([]args, options) makes more sense cause first one allows one to skip args parameter and I don't think that's a valid use.

from go-lxc.

Oh wait I think I misunderstood something. Are you putting the command name into the options struct?

from go-lxc.

@caglar10ur I was planning to put the entirety of the command (same as it is now) in the first param

So:

c.RunCommand([]string{"/bin/sh", "ls"}, &AttachOptions{
        Stdinfd: os.Stdin.Fd(),
        Stdoutfd: os.Stdout.Fd(),
        Stderrfd: os.Stderr.Fd(),
        Cwd: "/tmp",
        Env: []string{"FOO=BAR"},
})

from go-lxc.

@dcramer OK we are in the same page :)

from go-lxc.

I think we now have everything in, closing this...

from go-lxc.