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

VMware: add support for http_proxy in connection API #52936

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
7 participants
@Akasurde
Copy link
Member

Akasurde commented Feb 25, 2019

SUMMARY

This fix allows users to specify http_proxy and https_proxy
while connection to vCenter or ESXi systems

Fixes: #42221

Signed-off-by: Abhijeet Kasurde akasurde@redhat.com

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

lib/ansible/module_utils/vmware.py
lib/ansible/plugins/doc_fragments/vmware.py
test/units/module_utils/test_vmware.py

@Akasurde

This comment has been minimized.

Copy link
Member Author

Akasurde commented Feb 25, 2019

@pdellaert @dericcrago @Im0 @ckotte @jeking3 @Tomorrow9 @moshloop Could you please review this ? Thanks.

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 25, 2019

@jamescassell
Copy link
Contributor

jamescassell left a comment

Does the module currently look at the 'http_proxy', 'https_proxy', 'no_proxy' (all lower case) env vars? That is the de-facto standard way of configuring these items. (These are also the only env vars I know of that are required to be lower case, rather than required as upper case.)

VMware: add support for http_proxy in connection API
This fix allows users to specify http_proxy and https_proxy
while connection to vCenter or ESXi systems

Fixes: #42221

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

@Akasurde Akasurde force-pushed the Akasurde:i42221 branch from ab97ec9 to cefff7b Feb 26, 2019

@samdoran samdoran removed the needs_triage label Feb 26, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 26, 2019

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/cloud/kubevirt/kubevirt_template.py:0:0: E325 Argument 'resource_definition' in argument_spec defines type as <function list_dict_str at 0x7f1d3ed30c80> but documentation defines type as 'str'

click here for bot help

@dagwieers dagwieers added the cloud label Feb 28, 2019

@Im0

This comment has been minimized.

Copy link
Contributor

Im0 commented Mar 4, 2019

@Akasurde - this is great, thank you. I'll try to test in the coming days.

@Akasurde

This comment has been minimized.

Copy link
Member Author

Akasurde commented Mar 19, 2019

@Im0 Could you please confirm if this works for you ?

@pdellaert @dericcrago @ckotte @jeking3 @Tomorrow9 @alongchamps @adarobin Could you please review this ?

@jamescassell

This comment has been minimized.

Copy link
Contributor

jamescassell commented Mar 19, 2019

Does this change honor the no_proxy environment variable? If not, this will break for a lot of folks who have a corporate proxy but a VMware server on the local network. I'll try to test when I get a chance, but it might be a while.

description:
- Address of a proxy that will receive all HTTP requests and relay them.
- The format is a URL including a port number. For example, http://10.0.0.1:9090.
- If the value is not specified in the task, the value of environment variable C(https_proxy) will be used instead.

This comment has been minimized.

@alongchamps

alongchamps Mar 19, 2019

I think this line should say C(http_proxy)

@alongchamps

This comment has been minimized.

Copy link

alongchamps commented Mar 19, 2019

I unfortunately won't be able to test this due to my schedule for the next couple of weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.