Fix the PROXY variables, once and for all #4762

tbielawa · 2017-07-14T13:20:38Z

This will fix a myriad of proxy setting related bugs. At the very least it will impact these:

https://bugzilla.redhat.com/show_bug.cgi?id=1470165
* https://bugzilla.redhat.com/show_bug.cgi?id=1466783 (just kidding, this needs some separate work)
https://bugzilla.redhat.com/show_bug.cgi?id=1468424

tbielawa · 2017-07-17T13:39:51Z

Moving the hacky test script into this gist https://gist.github.com/tbielawa/7276903c31ad3dfb0f2759d72f2c5547

tbielawa · 2017-07-17T13:45:03Z

Squashed.
Rebased onto latest master
Removed the test script (it's in the linked gist, above)

tbielawa · 2017-07-17T13:46:01Z

[test]

tbielawa · 2017-07-17T13:46:04Z

aos-ci-test

sdodson · 2017-07-17T13:51:02Z

inventory/byo/hosts.origin.example

+#
+# Hosts in the openshift_no_proxy list will NOT use any globally
+# configured HTTP(S)_PROXYs. openshift_no_proxy accepts domains
+# (.example.com), hosts (example.com), and CIDR ranges (10.1.0.0/16)


But certain implementation's don't support CIDR ranges. Docker doesn't. OpenShift does.

sdodson · 2017-07-17T13:51:50Z

inventory/byo/hosts.origin.example

+#
+# For example, having hosts with FQDNs: m1.ex.com, n1.ex.com, and
+# n2.ex.com, one would simply add '.ex.com' to the openshift_no_proxy
+# variable (above) and set this value to False


ashcrow · 2017-07-17T13:54:28Z

roles/openshift_facts/library/openshift_facts.py

+        raw_no_proxy_list = []
+
+        # Automatic 3.6 NO_PROXY additions if a proxy is in use
+        svc_cluster_name = ['.svc', '.' + common['dns_domain'], common['hostname']]


Just verifying, but dns_domain can never fall back to be an IP address, correct? By it's name I'd assume not but just in case ...

I don't think we do anything to ensure that someone doesn't set it to an ip address but I'd never expect that to work. It's always used as a dns suffix so I'd expect everything to blow up violently if it weren't a suffix.

ashcrow · 2017-07-17T13:56:32Z

roles/openshift_facts/library/openshift_facts.py

+        # Automatic 3.6 NO_PROXY additions if a proxy is in use
+        svc_cluster_name = ['.svc', '.' + common['dns_domain'], common['hostname']]
+
+        # auto_hosts: Added to NO_PROXY list if any proxy params are


nit: Since this logic only is used if generate_no_proxy_hosts is True it may be worth moving it to line 1692 so it only is generated and stored if it's used.

Discussed on IRC. Implications of leaving the definition in place are negligible. Going to leave as is.

ashcrow

Nits but nothing to block over.

tbielawa · 2017-07-17T14:31:27Z

[merge][severity:blocker]

tbielawa · 2017-07-17T14:39:23Z

@openshift/team-documentation , @adellape this PR introduces and solidifies behavior which is new in 3.6 and may differ slightly from the existing documentation

https://docs.openshift.org/latest/install_config/install/advanced_install.html#advanced-install-configuring-global-proxy

If your hosts require use of a HTTP or HTTPS proxy in order to connect to external hosts, there are many components that must be configured to use the proxy, including masters, Docker, and builds. Node services only connect to the master API requiring no external access and therefore do not need to be configured to use a proxy.

May want to reflect explicitly in the online documentation that if any of

openshift_no_proxy
openshift_https_proxy
openshift_http_proxy

are set, then all cluster hosts will have an automatically generated NO_PROXY environment variable injected into several service configuration scripts. I'm not sure how you want to describe that, but the point I'm trying to make it, it should be clear that if you set any proxy variables then you will have a NO_PROXY environment variable set up for you.

Setting the http/s_proxy variables by themselves will cause all cluster members to be added to the NO_PROXY environment variable. The same goes for any definition of openshift_no_proxy in your inventory. Also added are the default .svc domain and your clusters dns_domain (typically .cluster.local).

Setting openshift_generate_no_proxy_hosts to False in inventory will NOT disable the automatic addition of the .svc domain and the cluster domain. These are required and added automatically if any of the proxy parameters (list, above) are set.

sdodson

LGTM

sdodson · 2017-07-17T15:08:29Z

aos-ci-test

ashcrow · 2017-07-17T18:13:34Z

flake openshift/origin#10162

ashcrow · 2017-07-17T18:13:45Z

[merge][severity:blocker]

openshift-bot · 2017-07-17T18:21:16Z

error: aos-ci-jenkins/OS_3.6_NOT_containerized for bdbc7d9 (logs)

openshift-bot · 2017-07-17T18:31:41Z

success: "aos-ci-jenkins/OS_3.6_containerized, aos-ci-jenkins/OS_3.6_containerized_e2e_tests" for bdbc7d9 (logs)

openshift-bot · 2017-07-17T18:33:36Z

success: "aos-ci-jenkins/OS_3.5_NOT_containerized, aos-ci-jenkins/OS_3.5_NOT_containerized_e2e_tests" for bdbc7d9 (logs)

openshift-bot · 2017-07-17T18:36:30Z

success: "aos-ci-jenkins/OS_3.5_containerized, aos-ci-jenkins/OS_3.5_containerized_e2e_tests" for bdbc7d9 (logs)

tbielawa · 2017-07-17T19:29:07Z

flake openshift/origin#8571

tbielawa · 2017-07-17T19:29:12Z

flake openshift/origin#10162

…I think they default it to an empty list if its not found.

tbielawa · 2017-07-17T19:52:02Z

aos-ci-test

openshift-bot · 2017-07-17T19:55:30Z

Evaluated for openshift ansible test up to b99b554

tbielawa · 2017-07-17T20:12:33Z

I think other parts of the openshift-ansible API were expecting openshift.common.no_proxy to either be defined, or if undefined then they set it to an empty list. I am hoping that by setting common['no_proxy'] to an empty string in the early-return I can avoid those default([])ing branches in the playbooks later on.

openshift-bot · 2017-07-17T21:11:06Z

success: "aos-ci-jenkins/OS_3.5_NOT_containerized, aos-ci-jenkins/OS_3.5_NOT_containerized_e2e_tests" for b99b554 (logs)

openshift-bot · 2017-07-17T21:11:18Z

success: "aos-ci-jenkins/OS_3.6_containerized, aos-ci-jenkins/OS_3.6_containerized_e2e_tests" for b99b554 (logs)

openshift-bot · 2017-07-17T21:13:18Z

success: "aos-ci-jenkins/OS_3.6_NOT_containerized, aos-ci-jenkins/OS_3.6_NOT_containerized_e2e_tests" for b99b554 (logs)

openshift-bot · 2017-07-17T21:14:17Z

success: "aos-ci-jenkins/OS_3.5_containerized, aos-ci-jenkins/OS_3.5_containerized_e2e_tests" for b99b554 (logs)

openshift-bot · 2017-07-17T22:07:53Z

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_openshift_ansible/343/) (Base Commit: da7551b) (PR Branch Commit: b99b554)

vikram-redhat · 2017-07-17T22:38:37Z

@tbielawa - Thanks Tim. Added a card to our Trello board to make these updates in the docs and tagged @adellape to it. https://trello.com/c/7IsRsDEo/596-updates-for-proxy-variables

sdodson · 2017-07-18T00:17:04Z

[merge][severity:blocker]

openshift-bot · 2017-07-18T00:23:33Z

Evaluated for openshift ansible merge up to b99b554

openshift-bot · 2017-07-18T02:07:29Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_openshift_ansible/706/) (Base Commit: 69d3800) (PR Branch Commit: b99b554) (Extended Tests: blocker)

sdodson · 2017-07-20T13:45:23Z

"once and for all" lol

tbielawa force-pushed the bz1470165_bz1468424 branch 2 times, most recently from 27c46f1 to b142cea Compare July 17, 2017 13:44

tbielawa requested review from sdodson, ashcrow and ewolinetz July 17, 2017 13:48

sdodson reviewed Jul 17, 2017

View reviewed changes

ashcrow reviewed Jul 17, 2017

View reviewed changes

ashcrow approved these changes Jul 17, 2017

View reviewed changes

tbielawa changed the title ~~[WIP] Fix the PROXY variables, once and for all~~ Fix the PROXY variables, once and for all Jul 17, 2017

tbielawa added the docs label Jul 17, 2017

sdodson approved these changes Jul 17, 2017

View reviewed changes

Fix NO_PROXY environment variable setting

bdbc7d9

tbielawa force-pushed the bz1470165_bz1468424 branch from 1ad834a to bdbc7d9 Compare July 17, 2017 15:08

tbielawa mentioned this pull request Jul 17, 2017

[WIP] Fix BZ1466783 - NO_PROXY of etcd IPs need to be configured in master-controllers #4776

Merged

Other playbooks maybe expecting this to be at least an empty string. …

b99b554

…I think they default it to an empty list if its not found.

openshift-bot merged commit 99f37b8 into openshift:master Jul 18, 2017

adellape mentioned this pull request Aug 31, 2017

Update 3.6+ proxy var behavior openshift/openshift-docs#5137

Merged

Fix the PROXY variables, once and for all #4762

Fix the PROXY variables, once and for all #4762

Conversation

tbielawa commented Jul 14, 2017 • edited Loading

tbielawa commented Jul 17, 2017

tbielawa commented Jul 17, 2017

tbielawa commented Jul 17, 2017

tbielawa commented Jul 17, 2017

sdodson Jul 17, 2017

Choose a reason for hiding this comment

sdodson Jul 17, 2017

Choose a reason for hiding this comment

ashcrow Jul 17, 2017

Choose a reason for hiding this comment

sdodson Jul 17, 2017

Choose a reason for hiding this comment

ashcrow Jul 17, 2017

Choose a reason for hiding this comment

tbielawa Jul 17, 2017

Choose a reason for hiding this comment

ashcrow left a comment

Choose a reason for hiding this comment

tbielawa commented Jul 17, 2017

tbielawa commented Jul 17, 2017 • edited Loading

sdodson left a comment

Choose a reason for hiding this comment

sdodson commented Jul 17, 2017

ashcrow commented Jul 17, 2017

ashcrow commented Jul 17, 2017

openshift-bot commented Jul 17, 2017

openshift-bot commented Jul 17, 2017

openshift-bot commented Jul 17, 2017

openshift-bot commented Jul 17, 2017

tbielawa commented Jul 17, 2017

tbielawa commented Jul 17, 2017

tbielawa commented Jul 17, 2017

openshift-bot commented Jul 17, 2017

tbielawa commented Jul 17, 2017

openshift-bot commented Jul 17, 2017

openshift-bot commented Jul 17, 2017

openshift-bot commented Jul 17, 2017

openshift-bot commented Jul 17, 2017

openshift-bot commented Jul 17, 2017

vikram-redhat commented Jul 17, 2017

sdodson commented Jul 18, 2017

openshift-bot commented Jul 18, 2017

openshift-bot commented Jul 18, 2017 • edited Loading

sdodson commented Jul 20, 2017

tbielawa commented Jul 14, 2017 •

edited

Loading

tbielawa commented Jul 17, 2017 •

edited

Loading

openshift-bot commented Jul 18, 2017 •

edited

Loading