Merge lp:~corey.bryant/charm-helpers/git-1531612 into lp:charm-helpers

[Corey Bryant]

[lp:~corey.bryant/charm-helpers/git-1531612]

515. By Corey Bryant on 2016-01-08

Add git clone depth option to OpenStack deploy from source

[Corey Bryant (corey.bryant)]

The deploy from source support in the OpenStack charms performs shallow git clones (e.g. git clone --depth 1) by default. This increases the speed of a git clone significantly. However, there is a side-effect of shallow clones when they are pip installed, in that it causes pip to list the package version as 0.0.0, when in actuality the package version may have been 11.0.0. The next time a pip installed package needs a minimum version of that package (e.g. let's say it needs >= 10.0.0), it'll see that 0.0.0 is installed and it'll install the latest package from pypi.

Non-shallow git clones enable pip to see the actual version and prevent this issue. The openstack-origin-git yaml should support an option to override the default depth of git clones on a per-repo basis.

[Corey Bryant (corey.bryant)]

Sample of yaml that overrides the default shallow clone to a normal git clone (see pbr depth):

repositories:
  - {name: requirements,
     repository: 'git://github.com/openstack/requirements',
     branch: stable/kilo}
  - {name: pbr,
     repository: 'git://github.com/coreycb/pbr',
     branch: stable/kilo,
     depth: 0}
  - {name: neutron-fwaas,
     repository: 'git://github.com/openstack/neutron-fwaas',
     branch: stable/kilo}
  - {name: neutron-lbaas,
     repository: 'git://github.com/openstack/neutron-lbaas',
     branch: stable/kilo}
  - {name: neutron-vpnaas,
     repository: 'git://github.com/openstack/neutron-vpnaas',
     branch: stable/kilo}
  - {name: neutron,
     repository: 'git://github.com/openstack/neutron',
     branch: stable/kilo}

[Corey Bryant (corey.bryant)]

Added a few inline comments.

[Liam Young (gnuoy)]

Approved

review: Approve

=== modified file 'charmhelpers/contrib/openstack/utils.py'
--- charmhelpers/contrib/openstack/utils.py	2016-01-05 09:10:53 +0000
+++ charmhelpers/contrib/openstack/utils.py	2016-01-08 13:28:07 +0000
@@ -593,7 +593,7 @@
     return yaml.load(projects_yaml)
 
 
-def git_clone_and_install(projects_yaml, core_project, depth=1):
+def git_clone_and_install(projects_yaml, core_project):
     """
     Clone/install all specified OpenStack repositories.
 
@@ -643,6 +643,9 @@
     for p in projects['repositories']:
         repo = p['repository']
         branch = p['branch']
+        depth = '1'
+        if 'depth' in p.keys():
+            depth = p['depth']
         if p['name'] == 'requirements':
             repo_dir = _git_clone_and_install_single(repo, branch, depth,
                                                      parent_dir, http_proxy,
@@ -687,19 +690,13 @@
     """
     Clone and install a single git repository.
     """
-    dest_dir = os.path.join(parent_dir, os.path.basename(repo))
-
     if not os.path.exists(parent_dir):
         juju_log('Directory already exists at {}. '
                  'No need to create directory.'.format(parent_dir))
         os.mkdir(parent_dir)
 
-    if not os.path.exists(dest_dir):
-        juju_log('Cloning git repo: {}, branch: {}'.format(repo, branch))
-        repo_dir = install_remote(repo, dest=parent_dir, branch=branch,
-                                  depth=depth)
-    else:
-        repo_dir = dest_dir
+    juju_log('Cloning git repo: {}, branch: {}'.format(repo, branch))
+    repo_dir = install_remote(repo, dest=parent_dir, branch=branch, depth=depth)
 
     venv = os.path.join(parent_dir, 'venv')
 

=== modified file 'charmhelpers/fetch/giturl.py'
--- charmhelpers/fetch/giturl.py	2015-12-11 17:02:13 +0000
+++ charmhelpers/fetch/giturl.py	2016-01-08 13:28:07 +0000
@@ -22,7 +22,6 @@
     filter_installed_packages,
     apt_install,
 )
-from charmhelpers.core.host import mkdir
 
 if filter_installed_packages(['git']) != []:
     apt_install(['git'])
@@ -62,8 +61,6 @@
         else:
             dest_dir = os.path.join(os.environ.get('CHARM_DIR'), "fetched",
                                     branch_name)
-        if not os.path.exists(dest_dir):
-            mkdir(dest_dir, perms=0o755)
         try:
             self.clone(source, dest_dir, branch, depth)
         except OSError as e:

=== modified file 'tests/contrib/openstack/test_openstack_utils.py'
--- tests/contrib/openstack/test_openstack_utils.py	2016-01-05 09:10:53 +0000
+++ tests/contrib/openstack/test_openstack_utils.py	2016-01-08 13:28:07 +0000
@@ -714,7 +714,7 @@
              - {name: requirements,
                 repository: 'git://git.openstack.org/openstack/requirements',
                 branch: stable/juno}"""
-        openstack.git_clone_and_install(git_wrong_order_1, 'keystone', depth=1)
+        openstack.git_clone_and_install(git_wrong_order_1, 'keystone')
         error_out.assert_called_with('keystone git repo must be specified last')
 
         git_wrong_order_2 = """
@@ -722,7 +722,7 @@
              - {name: keystone,
                 repository: 'git://git.openstack.org/openstack/keystone',
                 branch: stable/juno}"""
-        openstack.git_clone_and_install(git_wrong_order_2, 'keystone', depth=1)
+        openstack.git_clone_and_install(git_wrong_order_2, 'keystone')
         error_out.assert_called_with('requirements git repo must be specified first')
 
     @patch('os.path.join')
@@ -740,7 +740,7 @@
         _git_install_single.return_value = '/mnt/openstack-git/requirements'
         join.return_value = '/mnt/openstack-git/venv'
 
-        openstack.git_clone_and_install(openstack_origin_git, proj, depth=1)
+        openstack.git_clone_and_install(openstack_origin_git, proj)
         self.assertTrue(pip_venv.called)
         pip_install.assert_called_with('setuptools', upgrade=True,
                                        proxy=None,
@@ -748,10 +748,10 @@
         self.assertTrue(_git_install_single.call_count == 2)
         expected = [
             call('git://git.openstack.org/openstack/requirements',
-                 'stable/juno', 1, '/mnt/openstack-git', None,
+                 'stable/juno', '1', '/mnt/openstack-git', None,
                  update_requirements=False),
             call('git://git.openstack.org/openstack/keystone',
-                 'stable/juno', 1, '/mnt/openstack-git', None,
+                 'stable/juno', '1', '/mnt/openstack-git', None,
                  update_requirements=True)
         ]
         self.assertEquals(expected, _git_install_single.call_args_list)

=== modified file 'tests/fetch/test_giturl.py'
--- tests/fetch/test_giturl.py	2015-12-11 17:02:13 +0000
+++ tests/fetch/test_giturl.py	2016-01-08 13:28:07 +0000
@@ -89,8 +89,7 @@
             if dst:
                 shutil.rmtree(dst, ignore_errors=True)
 
-    @patch('charmhelpers.fetch.giturl.mkdir')
-    def test_installs(self, _mkdir):
+    def test_installs(self):
         self.fh.clone = MagicMock()
 
         for url in self.valid_urls:
@@ -100,10 +99,8 @@
             with patch.dict('os.environ', {'CHARM_DIR': 'foo'}):
                 where = self.fh.install(url)
             self.assertEqual(where, dest)
-            _mkdir.assert_called_with(where, perms=0o755)
 
-    @patch('charmhelpers.fetch.giturl.mkdir')
-    def test_installs_specified_dest(self, _mkdir):
+    def test_installs_specified_dest(self):
         self.fh.clone = MagicMock()
 
         for url in self.valid_urls: