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

Add support for running commands over vagrant ssh #4348

Merged
merged 3 commits into from Sep 14, 2017

Conversation

4 participants
@shadyvb
Contributor

shadyvb commented Sep 13, 2017

This adds support for a vagrant scheme, which uses vagrant to define ssh connection parameters via vagrant ssh.

This can either be used via a cli arg: wp --ssh=vagrant:default shell or via wp-cli.yml as ssh: vagrant:default. default here being the vagrant machine ID.

I'm not sure where best to document that, let me know if I can help with that.

@miya0001

This comment has been minimized.

Show comment
Hide comment
@miya0001

miya0001 Sep 13, 2017

Member

Oh, nice! I like it. :)

Member

miya0001 commented Sep 13, 2017

Oh, nice! I like it. :)

@shadyvb

This comment has been minimized.

Show comment
Hide comment
@shadyvb

shadyvb Sep 13, 2017

Contributor

Thanks! Main aim here is to ship default wp-cli.yml files with tools like Chassis and VVV so it works out of the box.

Contributor

shadyvb commented Sep 13, 2017

Thanks! Main aim here is to ship default wp-cli.yml files with tools like Chassis and VVV so it works out of the box.

@miya0001

This comment has been minimized.

Show comment
Hide comment
@miya0001

miya0001 Sep 13, 2017

Member

I am thinking that can we get ssh configuration from vagrant ssh-config automatically?
Do you have any idea?

Member

miya0001 commented Sep 13, 2017

I am thinking that can we get ssh configuration from vagrant ssh-config automatically?
Do you have any idea?

@shadyvb

This comment has been minimized.

Show comment
Hide comment
@shadyvb

shadyvb Sep 13, 2017

Contributor

I did try that, but piping the config to ssh command via process substitution was the only clean way I could find, but it wasn't supported by sh shell, and frankly I didn't spend that much time trying to make it work since vagrant ssh was good enough.

Contributor

shadyvb commented Sep 13, 2017

I did try that, but piping the config to ssh command via process substitution was the only clean way I could find, but it wasn't supported by sh shell, and frankly I didn't spend that much time trying to make it work since vagrant ssh was good enough.

@miya0001 miya0001 requested a review from wp-cli/committers Sep 13, 2017

@miya0001

I approved it. 👍
But I want to wait suggestions from @wp-cli/committers.

@gitlost

This comment has been minimized.

Show comment
Hide comment
@gitlost

gitlost Sep 14, 2017

Contributor

Don't know anything about vagrant but just going on the doc https://www.vagrantup.com/docs/cli/ssh.html the format for the command looks wrong - does this work for you @shadyvb ? It looks from the doc that the format should be something like:

$escaped_command = sprintf(
	'vagrant ssh %s -c %s%s',
	escapeshellarg( $bits['host'] ),
	escapeshellarg( $wp_command ),
	$is_tty ? ' -- -t' : ''
);

ie the "host" (vagrant machine ID) should come first.

Contributor

gitlost commented Sep 14, 2017

Don't know anything about vagrant but just going on the doc https://www.vagrantup.com/docs/cli/ssh.html the format for the command looks wrong - does this work for you @shadyvb ? It looks from the doc that the format should be something like:

$escaped_command = sprintf(
	'vagrant ssh %s -c %s%s',
	escapeshellarg( $bits['host'] ),
	escapeshellarg( $wp_command ),
	$is_tty ? ' -- -t' : ''
);

ie the "host" (vagrant machine ID) should come first.

@shadyvb

This comment has been minimized.

Show comment
Hide comment
@shadyvb

shadyvb Sep 14, 2017

Contributor

Thanks for investigating!

-- separator is for arguments sent to the ssh command itself, not the vagrant ssh which is what -t is for. Unfortunately the docs are lacking couple of arguments to the command, but using cli help should clear this up.

And yes it is working for me, been using it since.

Contributor

shadyvb commented Sep 14, 2017

Thanks for investigating!

-- separator is for arguments sent to the ssh command itself, not the vagrant ssh which is what -t is for. Unfortunately the docs are lacking couple of arguments to the command, but using cli help should clear this up.

And yes it is working for me, been using it since.

@gitlost

This comment has been minimized.

Show comment
Hide comment
@gitlost

gitlost Sep 14, 2017

Contributor

Hmm, installed vagrant (1.9.1 on Ubuntu 17.04) and if I do vagrant ssh -t I get

An invalid option was specified. The help for this command
is available below.

Usage: vagrant ssh [options] [name|id] [-- extra ssh args]

Options:

    -c, --command COMMAND            Execute an SSH command directly
    -p, --plain                      Plain mode, leaves authentication up to user
    -h, --help                       Print this help
Contributor

gitlost commented Sep 14, 2017

Hmm, installed vagrant (1.9.1 on Ubuntu 17.04) and if I do vagrant ssh -t I get

An invalid option was specified. The help for this command
is available below.

Usage: vagrant ssh [options] [name|id] [-- extra ssh args]

Options:

    -c, --command COMMAND            Execute an SSH command directly
    -p, --plain                      Plain mode, leaves authentication up to user
    -h, --help                       Print this help
@shadyvb

This comment has been minimized.

Show comment
Hide comment
@shadyvb

shadyvb Sep 14, 2017

Contributor

Ah, so it has been removed, okay. Then only the -t needs to go. The name|id is the last argument as you can see ( which is usually default and can even be omitted ) . And we don't have extra SSH args to add AFAIK.

Nice catch!

Contributor

shadyvb commented Sep 14, 2017

Ah, so it has been removed, okay. Then only the -t needs to go. The name|id is the last argument as you can see ( which is usually default and can even be omitted ) . And we don't have extra SSH args to add AFAIK.

Nice catch!

@schlessera schlessera added this to the 1.4.0 milestone Sep 14, 2017

@schlessera schlessera merged commit e6ca705 into wp-cli:master Sep 14, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@schlessera schlessera changed the title from Add support for running commands over inside vagrant ssh to Add support for running commands over vagrant ssh Sep 14, 2017

@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera Sep 14, 2017

Member

Thanks for the pull request, @shadyvb !

If you're interested in providing documentation, we have started gathering instructions for running commands remotely here: https://github.com/wp-cli/handbook/blob/master/running-commands-remotely.md

This does not contain the docker-specific scheme either, though.

Member

schlessera commented Sep 14, 2017

Thanks for the pull request, @shadyvb !

If you're interested in providing documentation, we have started gathering instructions for running commands remotely here: https://github.com/wp-cli/handbook/blob/master/running-commands-remotely.md

This does not contain the docker-specific scheme either, though.

@shadyvb

This comment has been minimized.

Show comment
Hide comment
@shadyvb

shadyvb Sep 15, 2017

Contributor

@schlessera Cool, just issued a PR for updating the docs to reference the new vagrant scheme.

wp-cli/handbook#146

Contributor

shadyvb commented Sep 15, 2017

@schlessera Cool, just issued a PR for updating the docs to reference the new vagrant scheme.

wp-cli/handbook#146

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment