From 1062aaf1bf59c8d23cfd16becf3d67a84be3b1a7 Mon Sep 17 00:00:00 2001
From: Marshall Margenau <mmargenau@gmail.com>
Date: Mon, 25 Jun 2018 19:03:31 -0500
Subject: [PATCH] Manage temp dirs from git clone.

- In some cases armada was not cleaning up temp dirs properly

Change-Id: I567989d9ac4363fff49c95802b61a3c459074d36
---
 armada/handlers/armada.py              | 15 ++++++---------
 armada/tests/unit/utils/test_source.py |  2 +-
 armada/utils/source.py                 |  3 ++-
 3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/armada/handlers/armada.py b/armada/handlers/armada.py
index 030c38e4..29bd8f12 100644
--- a/armada/handlers/armada.py
+++ b/armada/handlers/armada.py
@@ -106,6 +106,7 @@ class Armada(object):
         self.k8s_wait_attempt_sleep = k8s_wait_attempt_sleep
         self.manifest = Manifest(
             self.documents, target_manifest=target_manifest).get_manifest()
+        self.cloned_dirs = set()
 
     def find_release_chart(self, known_releases, release_name):
         '''
@@ -193,8 +194,9 @@ class Armada(object):
                     *repo_branch,
                     proxy_server=proxy_server,
                     auth_method=auth_method)
-                repos[repo_branch] = repo_dir
+                self.cloned_dirs.add(repo_dir)
 
+                repos[repo_branch] = repo_dir
                 chart['source_dir'] = (repo_dir, subpath)
             else:
                 chart['source_dir'] = (repos.get(repo_branch), subpath)
@@ -506,14 +508,9 @@ class Armada(object):
         LOG.info("Performing post-flight operations.")
 
         # Delete temp dirs used for deployment
-        for group in self.manifest.get(const.KEYWORD_ARMADA, {}).get(
-                const.KEYWORD_GROUPS, []):
-            for ch in group.get(const.KEYWORD_CHARTS, []):
-                chart = ch.get('chart', {})
-                if chart.get('source', {}).get('type') == 'git':
-                    source_dir = chart.get('source_dir')
-                    if isinstance(source_dir, tuple) and source_dir:
-                        source.source_cleanup(source_dir[0])
+        for cloned_dir in self.cloned_dirs:
+            LOG.debug('Removing cloned temp directory: %s', cloned_dir)
+            source.source_cleanup(cloned_dir)
 
     def _wait_until_ready(self, release_name, wait_labels, namespace, timeout):
         if self.dry_run:
diff --git a/armada/tests/unit/utils/test_source.py b/armada/tests/unit/utils/test_source.py
index 9885fd65..21fd6308 100644
--- a/armada/tests/unit/utils/test_source.py
+++ b/armada/tests/unit/utils/test_source.py
@@ -52,7 +52,7 @@ class GitTestCase(base.ArmadaTestCase):
         url = 'https://github.com/openstack/airship-armada'
         commit = 'cba78d1d03e4910f6ab1691bae633c5bddce893d'
         git_dir = source.git_clone(url, commit)
-        self._validate_git_clone(git_dir)
+        self._validate_git_clone(git_dir, commit)
 
     @testtools.skipUnless(base.is_connected(),
                           'git clone requires network connectivity.')
diff --git a/armada/utils/source.py b/armada/utils/source.py
index 513502de..16737f0b 100644
--- a/armada/utils/source.py
+++ b/armada/utils/source.py
@@ -57,7 +57,6 @@ def git_clone(repo_url, ref='master', proxy_server=None, auth_method=None):
         raise source_exceptions.GitException(repo_url)
 
     env_vars = {'GIT_TERMINAL_PROMPT': '0'}
-    temp_dir = tempfile.mkdtemp(prefix='armada')
     ssh_cmd = None
 
     if auth_method and auth_method.lower() == 'ssh':
@@ -79,6 +78,8 @@ def git_clone(repo_url, ref='master', proxy_server=None, auth_method=None):
             'with no authentication.', repo_url, ref)
 
     try:
+        temp_dir = tempfile.mkdtemp(prefix='armada')
+
         if proxy_server:
             LOG.debug('Cloning [%s] with proxy [%s]', repo_url, proxy_server)
             repo = Repo.clone_from(