p0deje / vagrant-exec

Execute commands in Vagrant synced folder
MIT License
140 stars 6 forks source link

Multi-VM environment #12

Open ealprr opened 9 years ago

ealprr commented 9 years ago

In case of multi-vm environnement, it seems impossible to use exec.

$ vagrant exec machine-name command
This command requires a specific VM name to target in a multi-VM environment.

Do I miss something?

p0deje commented 9 years ago

Currently, vagrant-exec only support single-VM. The reason behind that is because I was trying to make CLI as simple and predictable as possible.

With multi-VM, you have to pass machine name as argument. Here are some examples of how it could work:

$ vagrant exec machine-name command
$ vagrant exec -m machine-name command
$ VAGRANT_EXEC_MACHINE="machine-name" vagrant exec command

First example is bad because:

  1. CLI is vagrant exec command and if command can be machine-name - it's really confusing.
  2. What happens if machine-name is the same as command (e.g. you have machine postgres and execute vagrant exec postgres?

Second example is much better, but having separate switch for machine-name is inconsistent with Vagrant CLI (which accepts machine-name without any switch).

Third example requires exporting/passing additional environment variable which IMO is not very convenient.

With that said, vagrant-exec was designed to be run for single-VM because I cannot figure out how CLI should work for multi-VM. If you have any ideas about that, I'd be happy to hear them.

ealprr commented 9 years ago

Thank you for your answer!

According to me, first, you should warn user that the plugin is not (at least for now) compatible with multi-machine environnement in 'know issues' section of README. ;)

Then, in order to add multi-machine capabilities, I think this is important to be consistent with vanilla vagrant CLI.

I thought about it and, taking Controlling Multiple Machines section in doc and ssh command usage as references, here is my proposition:

$ vagrant exec -c command             # run 'command' in all VM
$ vagrant exec name -c command        # run 'command' in VM named 'name'
$ vagrant exec /name[0-9]/ -c command # run 'command' in all VM matching regular expression

And to go further, this syntax could be extended to support those usages:

$ vagrant exec name                # read stdin for command to run on VM

Which will then allow:

$ cat cmd_list | vagrant exec name # run each line from 'cmd_list' file into VM
$ vagrant exec name < ./cmd_list   # same as above
$ vagrant exec << \
command1
command2
[...]
EOF
p0deje commented 9 years ago

I disagree with the approach you suggest here because you add -c switch and this make it similar to vagrant ssh. What's the point of vagrant exec -c foo if one can write vagrant ssh -c foo to achieve the same result (it's even shorter).

I also don't see much point in reading stdin for commands because:

  1. It will make it complicated to apply all those command-based options and might make things confusing.
  2. Most important, I think it's better to just have a script and run it rather than pass a number of separated commands.
  3. Passing stdin should probably work at some degree (though I can't really test now).
ealprr commented 9 years ago

Precisely: the -c switch ensure the consistency with vagrant cli.

I think that the interest of vagrant-exec is not the alias around ssh: vagrant exec commandvagrant ssh -c command but in the options (prepend, directory, env) provided as well as the binstub capabilities which would still be interesting with a -c switch.

To resume my opinion:

p0deje commented 9 years ago

The CLI I was trying to simulate is bundle exec command, so I'm still against -c switch.

On the other hand, I'm getting to think that vagrant exec name command might actually work out even though it might be a bit confusing for user (see notes above). Regarding regex and stdin, I think separate issues should be opened for them.

@ealprr If you are willing to implement vagrant exec machine command, that'd be great because I'm not sure when I find time to do that.

ealprr commented 9 years ago

Right now, I will not be able to do this, but I will check later. In the meanwhile, we should think about the behavior expected with this syntax for the particular case you raised in your previous comment.

Maybe we should also wait/call for the opinion of other users.

You are right, I will open seperate issue for regex and stdin later. ;)

pauloschilling commented 8 years ago

Well, I'm a new vagrant exec user, but I totally agree with @p0deje, the plugin's cli is supposed to be as simple as possible, so anything you may try to change to make it compatible with a multi-VM configuration will probably result in a bad usability.

Something you can do is to provide a configuration like config.exec.multi_vm_support = true (default false), and then always expect that the first value after exec will be the VM name, and the rest will be the command, like:

$ vagrant exec vm1 pwd
$ vagrant exec vm2 pwd
$ vagrant exec * pwd # this can be used to run in all VM
$ vagrant exec pwd   # will return an error: 'pwd' VM does not exist

When multi_vm_support = false, the vagrant exec behavior will be the same as today.

Just a suggestion, you decide if this can be useful. :)

And thank you for your plugin!

Best, Paulo

p0deje commented 8 years ago

Thanks for all your comments and ideas, I think the desired behavior should be something like this:

$ vagrant exec vm1 pwd 
$ vagrant exec vm2 pwd 
$ vagrant exec * pwd       # special syntax to run in all VMs
$ vagrant exec /vm\d+/ pwd # execute in VMs matching /vm\d+/ Ruby regexp
$ vagrant exec pwd         # raises error asking for VM

I plan to implement this, but will definitely take some time and unfortunately I don't have much time right now, so patches are all welcome!

TjWallas commented 6 years ago

Bump! This would be a useful feature.

TjWallas commented 6 years ago

For those who need this feature, you can try shell commander: https://github.com/fgimenez/vagrant-shell-commander

p0deje commented 6 years ago

I'm not actively using/developing the plugin, but I'll happy to merge PRs.