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

WIP: Move proxy driver to use Provisioner::Base and not the legacy SSHBase… #1220

Closed
wants to merge 2 commits into from
Closed

WIP: Move proxy driver to use Provisioner::Base and not the legacy SSHBase… #1220

wants to merge 2 commits into from

Conversation

smurawski
Copy link
Contributor

@smurawski smurawski commented Apr 22, 2017 •
edited

🚧
…. All the defaults from SSH base should be handled elsewhere (like in transport).

This change allows the proxy driver to work with the reboot and continue behavior introduced in 1.10.0.

Signed-off-by: Steven Murawski steven.murawski@gmail.com

@smurawski smurawski mentioned this pull request Apr 22, 2017
Merged
@smurawski smurawski changed the title Move proxy driver to use Provisioner::Base and not the legacy SSHBase… WIP: Move proxy driver to use Provisioner::Base and not the legacy SSHBase… Apr 22, 2017
@smurawski smurawski force-pushed the smurawski/move_proxy_off_sshbase branch 3 times, most recently from f2e3b76 to a9a3812 Compare April 22, 2017 15:55
@smurawski smurawski force-pushed the smurawski/move_proxy_off_sshbase branch from a9a3812 to 6883a71 Compare May 23, 2017 11:09
@cheeseplus cheeseplus mentioned this pull request Sep 23, 2017
Merged
@coderanger
Copy link
Contributor

@smurawski Do you remember off-hand what was unfinished here?

@robbkidd robbkidd force-pushed the smurawski/move_proxy_off_sshbase branch from 6883a71 to 41c1ff4 Compare March 8, 2018 18:41
@robbkidd
Copy link
Contributor

robbkidd commented Mar 8, 2018

Rebased to resolve merge conflicts and re-run tests.

robbkidd
robbkidd reviewed Mar 8, 2018
View reviewed changes
lib/kitchen/driver/proxy.rb
@@ -42,29 +42,36 @@ class Proxy < Kitchen::Driver::SSHBase
def create(state)
# TODO: Once this isn't using SSHBase, it should call `super` to support pre_create_command.
Copy link
Contributor

Choose a reason for hiding this comment

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

Any thoughts on this comment? This PR moves off of SSHBase, so should a super be added?

@smurawski @robbkidd
Move proxy driver to use Provisioner::Base and not the legacy SSHBase…
1009d1a 
…. All the defaults from SSH base should be handled elsewhere (like in transport).

This change allows the proxy driver to work with the reboot and continue behavior introduced in 1.10.0.

Signed-off-by: Steven Murawski <steven.murawski@gmail.com>
@robbkidd robbkidd force-pushed the smurawski/move_proxy_off_sshbase branch from 41c1ff4 to 1009d1a Compare March 12, 2018 15:15
@robbkidd
fix proxy text environment variables
dd7bb34 
Noticed in the failing Travis run that the machine_user variable was not
getting set correctly.

> Setting environment variables from .travis.yml
> $ export global=["machine_user=travis"
> $ export machine_pass=travis
> $ export machine_port=22
> ...

The extra `- global:` when these env vars were already in a matrix
stanza seems to have dorked up the first var set, machine_user.

Signed-off-by: Robb Kidd <robb@thekidds.org>
@robbkidd
Copy link
Contributor 

Running kitchen create -c 5 ...
-----> Starting Kitchen (v1.20.0)
-----> Creating <default-ubuntu-1604>...
root@localhost password:

... replaced by ...

-----> Installing Chef Omnibus (install only if missing)
       Downloading https://omnitruck.chef.io/install.sh to file /tmp/install.sh
       Trying wget...
No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.

... so ... progress?

I think the proxy driver punting to a transport fails when in a proxied environment because there appears to be no code in the Transport plugin umbrella to handle http_proxy (and its cousins) coming from the kitchen config or from the environment.

configurable things have resolve_proxy_settings_from_config available via a wrap_shell_code method, but nothing in the core transports use this method to inject environment variables into the command to run on the "remote" host. I suspect proxy has been working heretofore because, descending from SSHBase—it had env_cmd under the hood shimming in all those proxy settings to any command run on the host-under-test.

@robbkidd
Copy link
Contributor

I reckon I'm going to pause on getting this fixed up.

I see that the SSH transport does not make use of any of http_proxy&friends from Configurable and it probably should. I assume that it has not used them previously because transports were more about moving files to the host-under-test than they were about executing commands upon them. But it a world where the proxy driver is going to delegate command execution to a transport, Transport::Base or Transport::SSH and Transport::Winrm will need to take advantage of the wrap_shell_code (or its underlying resolve_proxy_settings_from_config) available from Configurable.

@smurawski smurawski closed this Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robbkidd robbkidd robbkidd left review comments

Assignees
No one assigned
Labels
Developing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@smurawski @coderanger @robbkidd