Install python modules required for embedded ansible credentials #341

carbonin · 2019-08-08T16:06:25Z

We install these into a new venv so that we don't conflict with
appliance python packages.

This list is taken directly from
https://raw.githubusercontent.com/ansible/awx/c7bb0f10e10476e10df8a7e7b26732eb60958ca1/requirements/requirements_ansible.txt
which is the result of resolving the dependencies from
https://github.com/ansible/awx/blob/c7bb0f10e10476e10df8a7e7b26732eb60958ca1/requirements/requirements_ansible.in

I feel like taking the resolved file first is safer since pip doesn't
do true dependency resolution (pypa/pip#988)

Replacement for #340

Looking for input on places to put the list of packages. I felt like people wouldn't go looking for this list in manageiq-appliance, but it needs to live on disk somewhere ...

I could put it in manageiq core, but this seemed like a better place to start since it's really a build-time concern.

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

NickLaMuro

I am fine with this route for things.

I think where we get the source of truth for the requirements.txt is the only thing of concern here. That said, I think we can determine that out of band, and that doesn't really change the core functionality of this PR, just what we are version locking things to.

carbonin · 2019-08-08T20:46:00Z

So I made all the changes needed to get pip install to succeed without warnings or errors on the appliance while keeping as many of the modules as I could.

Unfortunately, now, a significant number are different from what is listed in awx and also from what is shipped in the tower rpm. Plus we added a few additional rpms.

Not sure what option we want to go with here. I can always revert this PR to its original state though if we think that's better. @simaishi @NickLaMuro @Fryguy ?

NickLaMuro · 2019-08-08T21:06:16Z

@carbonin Yeah, I don't know. Personally, I would rather we be as close to downstream as possible, or there eventually will be someone who tries fixing/replicating a bug on upstream and then turns out it is an issue with these packages and fixing it in one spot doesn't solve it in the other.

I am going to have to come back to this, but this how I started evaluating it:

Took a look at what changed from the first pass (using awx versus the Tower rpm):

carbonin/manageiq-appliance-build@9bb4516...f367e74

Which looks pretty bad, but when looking at
The changes that got the "straight from tower" list to our new our current list (starting one commit up):

carbonin/manageiq-appliance-build@cab28a7...f367e74

There is mostly removals from the list.

So I want to take a finer look at the about, but have to work on a few of my own TODO items first.

carbonin · 2019-08-08T21:08:43Z

Yeah, I came to the same conclusion @NickLaMuro I think we're closer to the downstream set with what we have here even though it required a bunch of workarounds vs the upstream set.

I'm going to un-wip this and if we need to follow up on it we can do that after. Better to just get cloud modules working more quickly.

NickLaMuro

Something I just caught that I think should be addressed before merged, if nothing else just to say it isn't a big deal.

kickstarts/partials/post/python_modules.ks.erb

simaishi · 2019-08-09T13:36:13Z

I'll leave it up to @carbonin and @NickLaMuro for what should be included in requirements.txt, the rest looks good to me.

Fryguy · 2019-08-09T14:43:34Z

kickstarts/partials/post/python_modules.ks.erb

+ virtualenv venv
+ source venv/bin/activate
+
+ cat > requirements.txt << EOF


Minor, but would this be better as a straight file in COPY?

I considered that, but thought that it might be more natural to keep what is essentially a packages list in this repo rather than in manageiq-appliance.

carbonin · 2019-08-09T15:21:25Z

@simaishi brought up that ansible and ansible-runner are not actually present in the tower rpm. I was using the list from pip freeze to create this, but that also seems to be listing modules installed at the system level (not just the ones in the venv). I can re-work this to try to just install the modules included in the venv in the rpm (not using pip freeze) ... thoughts?

carbonin · 2019-08-09T15:55:10Z

pip freeze -l is a thing. So now I have a new an updated list of modules that are actually installed using the tower venv rpm. Updating this PR to use that list.

We install these into a new venv so that we don't conflict with appliance python packages. This list is taken from `pip freeze -l` on an appliance with the ansible-tower-venv-ansible rpm installed

This is needed for building python modules as we install via pip https://bugzilla.redhat.com/show_bug.cgi?id=1734129

miq-bot · 2019-08-09T16:01:26Z

Checked commits carbonin/manageiq-appliance-build@4e6ae89~...23df7d9 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. ⭐

Fryguy · 2019-08-09T16:20:35Z

well, this got a lot simpler 😂

NickLaMuro

So looked through the differences between what is in requirements_ansible.txt and what you have here:

applicationinsights==0.11.9             => applicationinsights==0.11.1
argcomplete==1.9.5                      => argcomplete==1.9.4
bcrypt==3.1.6                           => bcrypt==3.1.4
botocore==1.9.23                        => botocore==1.9.3
cachetools==3.1.0                       => cachetools==3.0.0
certifi==2019.3.9                       => certifi==2018.1.18
cffi==1.12.3                            => cffi==1.11.5
colorama==0.4.1                         => colorama==0.3.9
decorator==4.4.0                        => decorator==4.2.1
dogpile.cache==0.7.1                    => dogpile.cache==0.6.5
enum34==1.1.6; python_version < "3"     => <missing>
futures==3.2.0; python_version < "3"    => <missing>
humanfriendly==4.18                     => humanfriendly==4.8
idna==2.8                               => idna==2.6
ipaddress==1.0.22; python_version < "3" => ipaddress==1.0.19
jmespath==0.9.4                         => jmespath==0.9.3
jsonpatch==1.23                         => jsonpatch==1.21
keystoneauth1==3.14.0                   => keystoneauth1==3.11.2
lxml==4.3.3                             => lxml==4.1.1
monotonic==1.5; python_version < "3"    => monotonic==1.4
munch==2.3.2                            => munch==2.2.0
netifaces==0.10.9                       => netifaces==0.10.6
ntlm-auth==1.3.0                        => ntlm-auth==1.0.6
oauthlib==3.0.1                         => oauthlib==2.0.6
openstacksdk==0.31.1                    => openstacksdk==0.23.0
os-service-types==1.7.0                 => os-service-types==1.2.0
ovirt-engine-sdk-python==4.3.0          => ovirt-engine-sdk-python==4.2.4
packaging==19.0                         => packaging==17.1
pbr==5.2.0                              => pbr==3.1.1
pyasn1==0.4.5                           => pyasn1==0.4.2
pyasn1-modules==0.2.5                   => pyasn1-modules==0.2.3
pycparser==2.19                         => pycparser==2.18
pycurl==7.43.0.1                        => <missing>
pygments==2.3.1                         => pygments==2.2.0
pyjwt==1.7.1                            => pyjwt==1.6.0
pynacl==1.3.0                           => pynacl==1.2.1
pyopenssl==19.0.0                       => pyopenssl==17.5.0
pyparsing==2.4.0                        => pyparsing==2.2.0
pywinrm[kerberos]==0.3.0                => pywinrm==0.3.0
requests==2.21.0                        => requests==2.20.0
requests-credssp==1.0.2                 => requests-credssp==0.1.0
requests-oauthlib==1.2.0                => requests-oauthlib==0.8.0
rsa==4.0                                => <missing>
six==1.12.0                             => six==1.11.0
stevedore==1.30.1                       => stevedore==1.28.0
tabulate==0.8.2                         => tabulate==0.7.7
typing==3.6.6; python_version < "3"     => <missing>
wheel==0.30.0                           => <missing>
xmltodict==0.12.0                       => xmltodict==0.11.0
pip==9.0.1                              => <missing and not needed>
setuptools==36.0.1                      => <missing and not needed>

And everything should just be a downgrade (which is expected), or is missing, though probably not needed. I think this is good, and we are definitely making less changes than what was done previously. 👍

NickLaMuro · 2019-08-09T16:29:12Z

kickstarts/partials/post/python_modules.ks.erb

+colorama==0.3.9
+cryptography==2.6.1
+decorator==4.2.1
+deprecation==2.0


This is the only new package that isn't in the source from awx:

https://github.com/ansible/awx/blob/c7bb0f10/requirements/requirements_ansible.txt#L50-L51

NickLaMuro · 2019-08-09T16:40:09Z

kickstarts/partials/post/python_modules.ks.erb

+pyparsing==2.2.0
+python-dateutil==2.6.1
+pyvmomi==6.5
+pywinrm==0.3.0


This one is the only one that has a different notation (pywinrm[kerberos]==0.3.0) in the ansible/awx file:

https://github.com/ansible/awx/blob/c7bb0f10/requirements/requirements_ansible.txt#L99

Install python modules required for embedded ansible credentials (cherry picked from commit 441d102) https://bugzilla.redhat.com/show_bug.cgi?id=1734129

simaishi · 2019-08-09T21:19:25Z

Ivanchuk backport details:

$ git log -1
commit df94d2e90f0942d9458a831ecd3f4199ca121603
Author: Jason Frey <jfrey@redhat.com>
Date:   Fri Aug 9 14:49:45 2019 -0400

    Merge pull request #341 from carbonin/install_python_modules
    
    Install python modules required for embedded ansible credentials
    
    (cherry picked from commit 441d102a558b8a29df35532cf5ecec21f77083f6)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1734129

carbonin added bug wip changelog/yes embedded ansible ivanchuk/yes labels Aug 8, 2019

carbonin requested review from Fryguy and NickLaMuro August 8, 2019 16:06

carbonin assigned simaishi Aug 8, 2019

NickLaMuro approved these changes Aug 8, 2019

View reviewed changes

carbonin mentioned this pull request Aug 8, 2019

Run ansible-runner with PYTHONPATH set to access additional modules ManageIQ/manageiq#19118

Merged

carbonin removed the wip label Aug 8, 2019

carbonin changed the title ~~[WIP] Install python modules required for embedded ansible credentials~~ Install python modules required for embedded ansible credentials Aug 8, 2019

NickLaMuro suggested changes Aug 8, 2019

View reviewed changes

kickstarts/partials/post/python_modules.ks.erb Outdated Show resolved Hide resolved

Fryguy reviewed Aug 9, 2019

View reviewed changes

Fryguy approved these changes Aug 9, 2019

View reviewed changes

carbonin added the wip label Aug 9, 2019

carbonin changed the title ~~Install python modules required for embedded ansible credentials~~ [WIP] Install python modules required for embedded ansible credentials Aug 9, 2019

carbonin added 2 commits August 9, 2019 11:59

Install python modules required for embedded ansible credentials

4e6ae89

We install these into a new venv so that we don't conflict with appliance python packages. This list is taken from `pip freeze -l` on an appliance with the ansible-tower-venv-ansible rpm installed

Add python-devel

23df7d9

This is needed for building python modules as we install via pip https://bugzilla.redhat.com/show_bug.cgi?id=1734129

carbonin force-pushed the install_python_modules branch from f367e74 to 23df7d9 Compare August 9, 2019 16:00

carbonin removed the wip label Aug 9, 2019

carbonin changed the title ~~[WIP] Install python modules required for embedded ansible credentials~~ Install python modules required for embedded ansible credentials Aug 9, 2019

Fryguy requested a review from NickLaMuro August 9, 2019 16:21

NickLaMuro approved these changes Aug 9, 2019

View reviewed changes

Fryguy merged commit 441d102 into ManageIQ:master Aug 9, 2019

Fryguy added this to the Sprint 118 Ending Aug 19, 2019 milestone Aug 9, 2019

Fryguy assigned Fryguy and unassigned simaishi Aug 9, 2019

simaishi added ivanchuk/backported and removed ivanchuk/yes labels Aug 9, 2019

simaishi mentioned this pull request Mar 5, 2020

Add ansible/ansible-runner and python dependency modules ManageIQ/manageiq-pods#381

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Install python modules required for embedded ansible credentials #341

Install python modules required for embedded ansible credentials #341

carbonin commented Aug 8, 2019

NickLaMuro left a comment

carbonin commented Aug 8, 2019

NickLaMuro commented Aug 8, 2019

carbonin commented Aug 8, 2019

NickLaMuro left a comment

simaishi commented Aug 9, 2019

Fryguy Aug 9, 2019

carbonin Aug 9, 2019

carbonin commented Aug 9, 2019

carbonin commented Aug 9, 2019

miq-bot commented Aug 9, 2019

Fryguy commented Aug 9, 2019

NickLaMuro left a comment

NickLaMuro Aug 9, 2019

NickLaMuro Aug 9, 2019

simaishi commented Aug 9, 2019

Install python modules required for embedded ansible credentials #341

Install python modules required for embedded ansible credentials #341

Conversation

carbonin commented Aug 8, 2019

NickLaMuro left a comment

Choose a reason for hiding this comment

carbonin commented Aug 8, 2019

NickLaMuro commented Aug 8, 2019

carbonin commented Aug 8, 2019

NickLaMuro left a comment

Choose a reason for hiding this comment

simaishi commented Aug 9, 2019

Fryguy Aug 9, 2019

Choose a reason for hiding this comment

carbonin Aug 9, 2019

Choose a reason for hiding this comment

carbonin commented Aug 9, 2019

carbonin commented Aug 9, 2019

miq-bot commented Aug 9, 2019

Fryguy commented Aug 9, 2019

NickLaMuro left a comment

Choose a reason for hiding this comment

NickLaMuro Aug 9, 2019

Choose a reason for hiding this comment

NickLaMuro Aug 9, 2019

Choose a reason for hiding this comment

simaishi commented Aug 9, 2019