Merge lp:~cprov/adt-continuous-deployer/secgroup-leak into lp:adt-continuous-deployer

[Celso Providelo]

Extending juju.destroy_environment() to ensure related security-groups are removed too.

Extending juju.destroy_environment() to ensure related security-groups are removed too.

See more information about the impacts on https://trello.com/c/N7l9ydBz/101-1-infra-neutron-quota-ports-bump-for-stg-ue-ci-engineering-on-ps4

[Para Siva (psivaa)]

+1, LGTM.
I'm not sure though, if running 'destroy-environment' a couple of more times, with a delay will be a preferred approach. That used to work for me with bootstack.

review: Approve

[Celso Providelo (cprov)]

Psivaa,

Thanks for the review, I see your point about simply re-running `juju destroy-environment` a couple of times, but isn't more precise if we address the problems we know specifically, which is the secgroups left behind ?

[Para Siva (psivaa)]

cprov,

Re-running destroy-environment is a bigger hammer, I think.

iiuc, the reason why the secgroups were leaked was because of the race inside destroy-environment. i.e. The secgroups were attempted to be deleted before the services weren't fully destroyed. They were in-use and hence were left alone. Re-running destroy-environment would usually clean up the leftover secgroups, and any other resources that leaked in the first run.

I agree that deleting the secgroups is a good solution for the problem in hand and more specific. We would not, however, know if there is any other resource leaked by destroy-environment but we could find out that later and delete them. Not a biggie to block this MP.

[Francis Ginther (fginther)]

Discussed one comment with Celso on irc. 'juju destroy-environment' is synchronous with regard to the behavior of cleaning up secgroups. This is, if the command exists and there is still a secgroup defined, it's going to remain until it is manually cleaned up.

review: Approve

=== modified file 'ci_automation/juju.py'
--- ci_automation/juju.py	2015-04-02 19:29:27 +0000
+++ ci_automation/juju.py	2015-04-30 04:57:44 +0000
@@ -19,6 +19,7 @@
 import os
 import shutil
 import subprocess
+import time
 import yaml
 
 from . import utils
@@ -47,7 +48,35 @@
         args = ['juju', 'destroy-environment', '-y', '--force', name]
         subprocess.check_call(args, env=env)
 
-    # XXX cprov 20150402: also destroy related bits secgroups,
+    # Ensure all related security-groups left behind by `juju` are removed,
+    # otherwise they will remain consuming resources (and quota).
+    nova_output = subprocess.check_output(['nova', 'secgroup-list'])
+    secgroups = [
+        line.split('|')[2].strip()
+        for line in nova_output.decode('utf-8').splitlines()
+        if name in line
+    ]
+
+    for secgroup in secgroups:
+        secgroup = secgroup.strip()
+        # Try harder because sometimes the instance retaining the secgroup
+        # takes sometime to be purged.
+        for i in range(3):
+            try:
+                subprocess.check_call(
+                    ['nova', 'secgroup-delete', secgroup],
+                    stdout=subprocess.DEVNULL,
+                    stderr=subprocess.DEVNULL
+                )
+            except subprocess.CalledProcessError:
+                time.sleep(1)
+            else:
+                break
+        else:
+            print(
+                'The security-group "{}" could not be removed. '
+                'Please remove it manually.'.format(secgroup)
+            )
 
     shutil.rmtree(env_dir)