Proxy support for cloning ansible repo and add provider #15762

ailisp · 2017-08-08T13:32:28Z

ISSUE: currently we get proxy setting from Configuration -> Advanced but didn't use it for cloning ansible repo or add provider. In pure ipv6 network, there is no Internet access without proxy so proxy must be used.
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1475954

\cc @bdunne @yrudman @gtanzillo

@miq-bot add-label wip, bug

ailisp · 2017-08-08T13:52:07Z

Current status:
Our way of using git command is wrapped in GitBasedDomainImportService and GitRepositoryService (both in manageiq-classic-ui/app/services/). They are calling methods from GitRepository class in manageiq/app/models/ to do actual work. And GitRepository then call git methods provided by GitWorkTree. GitWorkTree directly wraps rugged gem, which is a wrapper of libgit2 in C. There is no explicit way to use proxy in rugged gem or libgit2, but if libgit2 version that rugged used is compiled with libcurl flag set, the whole network related stuff will be controlled over http_proxy. Unfortunately, I have tested the version shipped with rugged doesn't support proxy, and no libgit2 that supports proxy can be found in RHEL/CentOS software repo. We have to compile our own version and shipped with it. The good news is curl support both Squid proxy and socks5 proxy, so Pavol's concern about socks5 proxy is not a problem.
See: libgit2/rugged#440

In summary:
Support proxy for git part: need discussion to continue (how to incorporate libgit2 with our compilation), and vm for testing was deleted by accident, need tested on vm)
Support proxy for adding provider part [TODO]

bdunne

@ailisp The bug referenced is about embedded ansible with IPv6, however the code changes are related to the git worktree (automate model code).

If you want to solve the embedded ansible bug, you need to look at how the embedded ansible application is configured. You'll may have to give it the proxy information when running the setup playbook.

ailisp · 2017-08-12T02:22:47Z

@bdunne
I'm sorry, but after several days research, I suspect if proxy support can actually done in our ManageIQ side. I think what we really need is a new Ansible Tower Rest API that allow us to change environment part of a Ansible play or the a better choice, to implement global proxy in Ansible. I'm start to learn Ansible, Ansible Tower and Ansible Tower REST api this week and not very familiar with them by now, but have done a walk through towards their documentation and a close look at embeded ansible worker code. In ManageIQ side, I think what we do is setting up an Ansible and Ansible Tower installation, as well as running an Ansible Tower REST api server at localhost. Then run a ansible tower api client in a EmbeddedAnsibleWorker process, which talk to that ansible api server. Their are only two pages of documentation about using proxy in Ansible and Ansible Tower:
http://docs.ansible.com/ansible/latest/playbooks_environment.html
http://docs.ansible.com/ansible-tower/latest/html/administration/proxy-support.html
The first one, need to patch in every play. Although it's better to have a global proxy for all playbooks, this at least give us a way. But after after seeing all of Ansible Tower API Documentation, I still can't find a way to change the environment field of a play through Ansible Tower REST api. The second one is not we want. ( I'm probably wrong )
Since what we can done in EmbeddedAnsibleWorker is the Ansible&Ansible Tower installation time setup and call Ansible Tower REST API, and Ansible only have per play level proxy:

  tasks:

    - apt: name=cobbler state=installed
      environment:
        http_proxy: http://proxy.example.com:8080

I think it's better to have a "global proxy" configuration for Ansible or Ansible Tower. Then we reconfigure it when save proxy settings in ManageIQ. Or have an Ansible Tower API to add http_proxy for a play's enviroment, then call that in ManageIQ. What do you think? Thanks.

carbonin

@ailisp We do not use git directly when dealing with playbook repos, the code changed in this PR is not related to EmbeddedAnsible and will not change it's behavior.

Additionally, the BZ referenced seems to be talking about not being able to add a cloud provider after setting up the proxy settings. To me, it doesn't seem like they even got to setting up EmbeddedAnsible.

I'll need some more clarity in the BZ before we can come up with a way of addressing whatever it is they are saying is broken.

ailisp · 2017-08-15T01:12:55Z

@carbonin right. Brandon has explained similar idea to me and thanks for having some clarification for this BZ.

ailisp · 2017-09-13T17:15:30Z

@carbonin @bdunne
The proxy and proxy setting part should now works with http and socks5 proxy for any process that launched by evm server using AwesomeSpawn. However, the AnsibleTower server is not launched by evm server in docker_start case, so I need a way to pass env vars to it. docker run -e seems a solution.
In appliance_start case, the ansible-tower-script is launched by evm server and I need to check if it successfully pass related env vars to AnsibleTower server process. I need an AnsibleTower installation to check that, but it's not free and I don't have one. Do you have vm images with AnsibleTower installed? After verified that, I can pass proxy settings at AnsibleTower start time in lib/vmdb/util.rb. I also need access to AnsibleTower repo to make sure it really use libgit2 python binding or run git command, which is now an assumption. Thanks

carbonin · 2017-09-14T18:57:25Z

@ailisp I think this approach likely won't work for sending the proxy environment to embedded ansible.

The services that make up the embedded ansible installation are started by systemd which cleans the environment when it starts services. I think we would need to either set this stuff up in the systemd environment files for the ansible services or need to specifically configure the ansible setup to configure it iteslf (which I'm not sure how to do).

ailisp · 2017-09-15T14:50:23Z

@carbonin
Thanks for confirmation. We need to have a proxy option for ansible tower. And make it accessible to us either in ansible tower's inventory file, extra vars or api. Can we leave ansible tower team to do this?

carbonin · 2017-09-20T13:50:16Z

lib/embedded_ansible.rb

+EOF
+      File.open(SETTING_FILE, 'a') do |file|
+        file.write(proxy_settings)
+      end


I think this would be cleaner if it were just two calls to File#write:

File.open(SETTING_FILE, 'a') do |file| file.write("AWX_TASK_ENV['HTTP_PROXY'] = #{proxy}\n") file.write("AWX_TASK_ENV['HTTPS_PROXY'] = #{proxy}\n") end

Also can we check that these ENV vars are not present in settings.py before writing them? It feels like this would write the config multiple times if the role were removed and re-added.

Thanks. Do we need an extra file.write('\n') before the first file.write. In case settings.py doesn't contain a fresh empty line. And what is preferred way to check that ENV vars? By search AWX_TASK_ENV['HTTP_PROXY'] string in py file?

Do we need an extra file.write('\n') before the first file.write. In case settings.py doesn't contain a fresh empty line.

Yeah, that's probably a good idea.

And what is preferred way to check that ENV vars? By search AWX_TASK_ENV['HTTP_PROXY'] string in py file?

Sounds good to me.

carbonin · 2017-09-20T13:51:22Z

lib/embedded_ansible.rb

+    if VMDB::Util.http_proxy_uri(:embedded_ansible)
+      _log.error("Can't set proxy for Embedded Ansible Tower in container environment")
+    elsif VMDB::Util.http_proxy_uri
+      _log.warn("Global proxy settings will not work for Embedded Ansible Tower in container environment")


I'm not sure that this is the case, but I don't think this would really be needed for the container case either way.

For container case, we can't modify it's settings.py and set the proxy settings since the ansible tgwer already set up in container. You mean we don't need to consider this case?

we can't modify it's settings.py and set the proxy settings since the ansible tgwer already set up in container.

Of course, you're right here.

You mean we don't need to consider this case?

I mean that in OpenShift at least, it's likely that we will have more permissive network that will allow us to access the resources we need. I'm just thinking that a proxy will be a less common use case in containers.

carbonin · 2017-09-20T13:52:43Z

config/settings.yml

@@ -825,11 +825,13 @@
    :password:
    :port:
    :user:
+    :scheme:


Why is this change being made?

It's for socks5 proxy. If leave empty or "http", VMDB::Util.http_uri_proxy will return a http proxy uri. If scheme is "socks5", it will construct a socks5://... proxy. Both http proxy and socks5 proxy works when putting in http_proxy env var.
VMDB::Util.http_uri_proxy: https://github.com/ManageIQ/manageiq/blob/master/lib/vmdb/util.rb#L3

carbonin · 2017-09-20T13:53:06Z

lib/embedded_ansible.rb

@@ -8,6 +8,7 @@ class EmbeddedAnsible
  ANSIBLE_ROLE           = "embedded_ansible".freeze
  SETUP_SCRIPT           = "ansible-tower-setup".freeze
  SECRET_KEY_FILE        = "/etc/tower/SECRET_KEY".freeze
+  SETTING_FILE           = "/etc/tower/settings.py".freeze


I think SETTINGS_FILE would be a better constant name.

Dismissing Brandon's review as the patch is no longer touching the referenced code.

carbonin · 2017-09-20T13:55:48Z

@ailisp Can you squash out some of the commits that are making changes which are no longer present in this patch?

ailisp · 2017-09-20T14:23:40Z

@carbonin OK, will squash them after these changes and works for test

ailisp · 2017-09-22T17:45:28Z

@miq-bot remove-label wip

ailisp · 2017-09-22T17:46:38Z

@carbonin Finally it works on Pavol's VM. Thank you a lot for your help.

carbonin

Can you write some specs for the process of setting the proxy info in the file.

Feel free to just test the private method on its own.

carbonin · 2017-09-22T18:12:40Z

lib/embedded_ansible.rb

+                     else
+                       exist_settings
+                     end
+    end


I feel like this is all a bit over-engineered.

I think it would be easier to read if we just deleted the lines with these keys from the file, then appended the new proxy lines to the end if the proxy is set in our settings.

I don't think this process of finding the start of the previous proxy settings is really necessary.

Pseudo code should reduce to:

current_contents = File.read(SETTINGS_FILE) new_contents = current.gsub(/^.*AWX_TASK_ENV\['(HTTPS?_PROXY|NO_PROXY)'\].*$/, "") if proxy_is_set new_contents << proxy_info end File.write(SETTINGS_FILE, new_contents)

Yeah, this looks better

https://bugzilla.redhat.com/show_bug.cgi?id=1475954

…s5 proxy

miq-bot · 2017-09-23T02:28:29Z

Checked commits ailisp/manageiq@c9ca903~...7672e03 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🏆

ailisp · 2017-09-23T02:41:10Z

@carbonin rewrite as you suggest and with a spec now. Thanks
@miq-bot remove-label wip

Proxy support for cloning ansible repo and add provider (cherry picked from commit 2526eb7) https://bugzilla.redhat.com/show_bug.cgi?id=1496912

simaishi · 2017-09-29T13:22:47Z

Fine backport details:

$ git log -1
commit 0bd48e2171b2a3037232e6416f2e428a07041964
Author: Nick Carboni <[email protected]>
Date:   Mon Sep 25 09:49:53 2017 -0400

    Merge pull request #15762 from ailisp/proxy-not-work
    
    Proxy support for cloning ansible repo and add provider
    (cherry picked from commit 2526eb7300ee8e0ac0c44e4cb6950f095241fe53)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1496912

Proxy support for cloning ansible repo and add provider (cherry picked from commit 2526eb7) https://bugzilla.redhat.com/show_bug.cgi?id=1496912

miq-bot added bug wip labels Aug 8, 2017

bdunne previously requested changes Aug 8, 2017

View reviewed changes

carbonin suggested changes Aug 14, 2017

View reviewed changes

chessbyte assigned gtanzillo Sep 19, 2017

carbonin suggested changes Sep 20, 2017

View reviewed changes

ailisp closed this Sep 22, 2017

ailisp force-pushed the proxy-not-work branch from cfd8ff0 to 3b3f43f Compare September 22, 2017 17:36

ailisp reopened this Sep 22, 2017

ailisp changed the title ~~[WIP] Proxy support for cloning ansible repo and add provider~~ Proxy support for cloning ansible repo and add provider Sep 22, 2017

miq-bot removed the wip label Sep 22, 2017

miq-bot changed the title ~~Proxy support for cloning ansible repo and add provider~~ [WIP] Proxy support for cloning ansible repo and add provider Sep 22, 2017

miq-bot added the wip label Sep 22, 2017

carbonin reviewed Sep 22, 2017

View reviewed changes

ailisp force-pushed the proxy-not-work branch from 415b338 to 1ffa5e9 Compare September 22, 2017 23:38

ailisp added 2 commits September 22, 2017 21:34

add http proxy support for embedded ansible tower

c9ca903

https://bugzilla.redhat.com/show_bug.cgi?id=1475954

add new embedded_ansible specific proxy settings and :scheme for sock…

0594836

…s5 proxy

ailisp force-pushed the proxy-not-work branch from 5f082f1 to b7166b2 Compare September 23, 2017 01:34

spec for update_proxy_settings

7672e03

ailisp force-pushed the proxy-not-work branch from 105b862 to 7672e03 Compare September 23, 2017 02:25

miq-bot changed the title ~~[WIP] Proxy support for cloning ansible repo and add provider~~ Proxy support for cloning ansible repo and add provider Sep 23, 2017

miq-bot removed the wip label Sep 23, 2017

carbonin assigned carbonin and unassigned gtanzillo Sep 25, 2017

carbonin approved these changes Sep 25, 2017

View reviewed changes

carbonin added blocker fine/yes labels Sep 25, 2017

carbonin added this to the Sprint 70 Ending Oct 2, 2017 milestone Sep 25, 2017

carbonin merged commit 2526eb7 into ManageIQ:master Sep 25, 2017

simaishi pushed a commit that referenced this pull request Sep 29, 2017

Merge pull request #15762 from ailisp/proxy-not-work

0bd48e2

Proxy support for cloning ansible repo and add provider (cherry picked from commit 2526eb7) https://bugzilla.redhat.com/show_bug.cgi?id=1496912

simaishi added fine/backported and removed fine/yes labels Sep 29, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Proxy support for cloning ansible repo and add provider #15762

Proxy support for cloning ansible repo and add provider #15762

ailisp commented Aug 8, 2017

ailisp commented Aug 8, 2017 •

edited

Loading

bdunne left a comment

ailisp commented Aug 12, 2017

carbonin left a comment

ailisp commented Aug 15, 2017

ailisp commented Sep 13, 2017 •

edited

Loading

carbonin commented Sep 14, 2017

ailisp commented Sep 15, 2017

carbonin Sep 20, 2017

ailisp Sep 20, 2017

carbonin Sep 20, 2017

carbonin Sep 20, 2017

ailisp Sep 20, 2017

carbonin Sep 20, 2017

carbonin Sep 20, 2017

ailisp Sep 20, 2017

carbonin Sep 20, 2017

ailisp Sep 20, 2017

carbonin commented Sep 20, 2017

ailisp commented Sep 20, 2017

ailisp commented Sep 22, 2017

ailisp commented Sep 22, 2017

carbonin left a comment

carbonin Sep 22, 2017

ailisp Sep 22, 2017

miq-bot commented Sep 23, 2017

ailisp commented Sep 23, 2017

simaishi commented Sep 29, 2017

Proxy support for cloning ansible repo and add provider #15762

Proxy support for cloning ansible repo and add provider #15762

Conversation

ailisp commented Aug 8, 2017

ailisp commented Aug 8, 2017 • edited Loading

bdunne left a comment

Choose a reason for hiding this comment

ailisp commented Aug 12, 2017

carbonin left a comment

Choose a reason for hiding this comment

ailisp commented Aug 15, 2017

ailisp commented Sep 13, 2017 • edited Loading

carbonin commented Sep 14, 2017

ailisp commented Sep 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carbonin commented Sep 20, 2017

ailisp commented Sep 20, 2017

ailisp commented Sep 22, 2017

ailisp commented Sep 22, 2017

carbonin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miq-bot commented Sep 23, 2017

ailisp commented Sep 23, 2017

simaishi commented Sep 29, 2017

ailisp commented Aug 8, 2017 •

edited

Loading

ailisp commented Sep 13, 2017 •

edited

Loading