Merge lp:~corey.bryant/charm-helpers/fix-global-reqs into lp:charm-helpers

Proposed by Corey Bryant
Status: Merged
Merged at revision: 391
Proposed branch: lp:~corey.bryant/charm-helpers/fix-global-reqs
Merge into: lp:charm-helpers
Diff against target: 145 lines (+35/-15)
2 files modified
charmhelpers/contrib/openstack/utils.py (+13/-7)
tests/contrib/openstack/test_openstack_utils.py (+22/-8)
To merge this branch: bzr merge lp:~corey.bryant/charm-helpers/fix-global-reqs
Reviewer Review Type Date Requested Status
Jorge Niedbalski (community) Approve
Billy Olsen Approve
Review via email: mp+262469@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Corey Bryant (corey.bryant) wrote :

This fixes the following error: http://paste.ubuntu.com/11737211/

The problem is that on trusty a virtualenv is created with an old setuptools and therefore and old pkg_resource. When the update.py script from the master branch of global requirements is executed, it fails because of the old version of pkg_resources.

This fix provides two things:
1) upgrade setuptools to latest version after creating the virtualenv
2) when executing 'python update.py <package_dir>' use the python from the virtualenv

Revision history for this message
Billy Olsen (billy-olsen) wrote :

Looks good to me.

review: Approve
Revision history for this message
Jorge Niedbalski (niedbalski) wrote :

LGTM

review: Approve
Revision history for this message
Corey Bryant (corey.bryant) wrote :

Thanks Jorge!

On Mon, Jun 22, 2015 at 6:49 PM, <email address hidden> wrote:

> The proposal to merge lp:~corey.bryant/charm-helpers/fix-global-reqs into
> lp:charm-helpers has been updated.
>
> Status: Needs review => Merged
>
> For more details, see:
>
> https://code.launchpad.net/~corey.bryant/charm-helpers/fix-global-reqs/+merge/262469
> --
> You are the owner of lp:~corey.bryant/charm-helpers/fix-global-reqs.
>

--
Regards,
Corey

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'charmhelpers/contrib/openstack/utils.py'
2--- charmhelpers/contrib/openstack/utils.py 2015-06-11 14:41:04 +0000
3+++ charmhelpers/contrib/openstack/utils.py 2015-06-19 16:09:56 +0000
4@@ -555,6 +555,11 @@
5
6 pip_create_virtualenv(os.path.join(parent_dir, 'venv'))
7
8+ # Upgrade setuptools from default virtualenv version. The default version
9+ # in trusty breaks update.py in global requirements master branch.
10+ pip_install('setuptools', upgrade=True, proxy=http_proxy,
11+ venv=os.path.join(parent_dir, 'venv'))
12+
13 for p in projects['repositories']:
14 repo = p['repository']
15 branch = p['branch']
16@@ -616,24 +621,24 @@
17 else:
18 repo_dir = dest_dir
19
20+ venv = os.path.join(parent_dir, 'venv')
21+
22 if update_requirements:
23 if not requirements_dir:
24 error_out('requirements repo must be cloned before '
25 'updating from global requirements.')
26- _git_update_requirements(repo_dir, requirements_dir)
27+ _git_update_requirements(venv, repo_dir, requirements_dir)
28
29 juju_log('Installing git repo from dir: {}'.format(repo_dir))
30 if http_proxy:
31- pip_install(repo_dir, proxy=http_proxy,
32- venv=os.path.join(parent_dir, 'venv'))
33+ pip_install(repo_dir, proxy=http_proxy, venv=venv)
34 else:
35- pip_install(repo_dir,
36- venv=os.path.join(parent_dir, 'venv'))
37+ pip_install(repo_dir, venv=venv)
38
39 return repo_dir
40
41
42-def _git_update_requirements(package_dir, reqs_dir):
43+def _git_update_requirements(venv, package_dir, reqs_dir):
44 """
45 Update from global requirements.
46
47@@ -642,7 +647,8 @@
48 """
49 orig_dir = os.getcwd()
50 os.chdir(reqs_dir)
51- cmd = ['python', 'update.py', package_dir]
52+ python = os.path.join(venv, 'bin/python')
53+ cmd = [python, 'update.py', package_dir]
54 try:
55 subprocess.check_call(cmd)
56 except subprocess.CalledProcessError:
57
58=== modified file 'tests/contrib/openstack/test_openstack_utils.py'
59--- tests/contrib/openstack/test_openstack_utils.py 2015-05-11 18:53:44 +0000
60+++ tests/contrib/openstack/test_openstack_utils.py 2015-06-19 16:09:56 +0000
61@@ -642,11 +642,13 @@
62 error_out.assert_called_with(
63 'openstack-origin-git key \'%s\' is missing' % key)
64
65+ @patch('os.path.join')
66 @patch.object(openstack, 'error_out')
67 @patch.object(openstack, '_git_clone_and_install_single')
68+ @patch.object(openstack, 'pip_install')
69 @patch.object(openstack, 'pip_create_virtualenv')
70- def test_git_clone_and_install_errors(self, pip_venv, git_install_single,
71- error_out):
72+ def test_git_clone_and_install_errors(self, pip_venv, pip_install,
73+ git_install_single, error_out, join):
74 git_missing_repos = """
75 repostories:
76 - {name: requirements,
77@@ -704,19 +706,26 @@
78 openstack.git_clone_and_install(git_wrong_order_2, 'keystone', depth=1)
79 error_out.assert_called_with('requirements git repo must be specified first')
80
81+ @patch('os.path.join')
82 @patch.object(openstack, 'charm_dir')
83 @patch.object(openstack, 'error_out')
84 @patch.object(openstack, '_git_clone_and_install_single')
85+ @patch.object(openstack, 'pip_install')
86 @patch.object(openstack, 'pip_create_virtualenv')
87- def test_git_clone_and_install_success(self, pip_venv, _git_install_single,
88- error_out, charm_dir):
89+ def test_git_clone_and_install_success(self, pip_venv, pip_install,
90+ _git_install_single, error_out,
91+ charm_dir, join):
92 proj = 'keystone'
93 charm_dir.return_value = '/var/lib/juju/units/testing-foo-0/charm'
94 # the following sets the global requirements_dir
95 _git_install_single.return_value = '/mnt/openstack-git/requirements'
96+ join.return_value = '/mnt/openstack-git/venv'
97
98 openstack.git_clone_and_install(openstack_origin_git, proj, depth=1)
99 self.assertTrue(pip_venv.called)
100+ pip_install.assert_called_with('setuptools', upgrade=True,
101+ proxy=None,
102+ venv='/mnt/openstack-git/venv')
103 self.assertTrue(_git_install_single.call_count == 2)
104 expected = [
105 call('git://git.openstack.org/openstack/requirements',
106@@ -775,6 +784,7 @@
107 parent_dir = '/mnt/openstack-git/'
108 http_proxy = 'http://squid-proxy-url'
109 dest_dir = '/mnt/openstack-git'
110+ venv_dir = '/mnt/openstack-git'
111 reqs_dir = '/mnt/openstack-git/requirements-dir'
112 join.return_value = dest_dir
113 openstack.requirements_dir = reqs_dir
114@@ -786,23 +796,27 @@
115 mkdir.assert_called_with(parent_dir)
116 install_remote.assert_called_with(repo, dest=parent_dir, depth=1,
117 branch=branch)
118- _git_update_reqs.assert_called_with(dest_dir, reqs_dir)
119+ _git_update_reqs.assert_called_with(venv_dir, dest_dir, reqs_dir)
120 pip_install.assert_called_with(dest_dir, venv='/mnt/openstack-git',
121 proxy='http://squid-proxy-url')
122
123+ @patch('os.path.join')
124 @patch('os.getcwd')
125 @patch('os.chdir')
126 @patch('subprocess.check_call')
127- def test_git_update_requirements(self, check_call, chdir, getcwd):
128+ def test_git_update_requirements(self, check_call, chdir, getcwd, join):
129 pkg_dir = '/mnt/openstack-git/repo-dir'
130 reqs_dir = '/mnt/openstack-git/reqs-dir'
131 orig_dir = '/var/lib/juju/units/testing-foo-0/charm'
132+ venv_dir = '/mnt/openstack-git/venv'
133 getcwd.return_value = orig_dir
134+ join.return_value = '/mnt/openstack-git/venv/python'
135
136- openstack._git_update_requirements(pkg_dir, reqs_dir)
137+ openstack._git_update_requirements(venv_dir, pkg_dir, reqs_dir)
138 expected = [call(reqs_dir), call(orig_dir)]
139 self.assertEquals(expected, chdir.call_args_list)
140- check_call.assert_called_with(['python', 'update.py', pkg_dir])
141+ check_call.assert_called_with(['/mnt/openstack-git/venv/python',
142+ 'update.py', pkg_dir])
143
144 @patch('os.path.join')
145 @patch('subprocess.check_call')

Subscribers

People subscribed via source and target branches