Merge lp:~adam-collard/landscape-charm/collect-logs-clean-juju-2 into lp:~landscape/landscape-charm/tools

[Adam Collard]

Fix name of Juju 2's model-config in collect-logs

Fix name of Juju 2's model-config in collect-logs

Testing instructions:
* Run collect-logs on a Juju 2.1 model, verify that proxy-ssh is set

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Success
Revno: 35
Branch: lp:~adam-collard/landscape-charm/collect-logs-clean-juju-2
Jenkins: https://ci.lscape.net/job/latch-test-xenial/1092/

review: Approve (test results)

[David Britton (dpb)]

Merge conflict exists

[lp:~adam-collard/landscape-charm/collect-logs-clean-juju-2]

36. By Adam Collard on 2017-01-05

Fix merge conflicts

[Adam Collard (adam-collard)]

> Merge conflict exists

Thanks, fixed.

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Success
Revno: 36
Branch: lp:~adam-collard/landscape-charm/collect-logs-clean-juju-2
Jenkins: https://ci.lscape.net/job/latch-test-xenial/1094/

review: Approve (test results)

[David Britton (dpb)]

Ran into an issue where ls -rt didn't work:

drwx------ 3 landscape landscape 4096 Jan 4 22:46 1
drwx------ 3 landscape landscape 4096 Jan 5 04:28 2
drwx------ 3 landscape landscape 4096 Jan 5 04:19 3

Suggest we modify the "ls -rt | tail -1" to instead do 'ls | sort -n | tail -1'

This is a bit of an edge condition, but seems like a sane fix.

Approved with above caveats.

review: Approve

=== modified file 'collect-logs'
--- collect-logs	2017-01-04 02:09:01 +0000
+++ collect-logs	2017-01-05 17:33:26 +0000
@@ -131,7 +131,7 @@
         if self.binary_path == JUJU1:
             args = self._resolve("set-env", item)
         else:
-            args = self._resolve("set-model-config", item)
+            args = self._resolve("model-config", item)
         return args
 
     def format_set_model_config(self, key, value):
@@ -392,7 +392,9 @@
     try:
         check_output(args2, stderr=STDOUT, env=juju.env)
     except CalledProcessError as e:
-        pass
+        log.warning("Couldn't disable proxy-ssh in the inner environment "
+                    "using Juju 2, attempting Juju 1.")
+        log.warning("Error was:\n{}".format(e.output))
     else:
         return
 
@@ -404,13 +406,10 @@
     try:
         check_output(args1, stderr=STDOUT, env=juju.env)
     except CalledProcessError as e:
-        pass
-    else:
-        return
+        log.warning("Couldn't disable proxy-ssh in the inner environment "
+                    "using Juju 1, collecting inner logs might fail.")
+        log.warning("Error was:\n{}".format(e.output))
 
-    log.warning("Couldn't disable proxy-ssh in the inner environment,"
-                " collecting inner logs might fail.")
-    log.warning("Error was:\n{}".format(e.output))
 
 
 def find_inner_juju(juju, landscape_unit, inner_model=DEFAULT_MODEL):

=== modified file 'test_collect-logs.py'
--- test_collect-logs.py	2017-01-04 02:09:01 +0000
+++ test_collect-logs.py	2017-01-05 17:33:26 +0000
@@ -547,7 +547,7 @@
         expected = []
         cmd = ("sudo JUJU_DATA=/var/lib/landscape/juju-homes/"
                "`sudo ls -rt /var/lib/landscape/juju-homes/ | tail -1`"
-               " juju-2.1 set-model-config -m controller proxy-ssh=false")
+               " juju-2.1 model-config -m controller proxy-ssh=false")
         expected.append(mock.call(["juju", "ssh", "landscape-server/0", cmd],
                                   stderr=subprocess.STDOUT,
                                   env=self.juju.env))
@@ -625,7 +625,7 @@
         expected = []
         cmd = ("sudo JUJU_DATA=/var/lib/landscape/juju-homes/"
                "`sudo ls -rt /var/lib/landscape/juju-homes/ | tail -1`"
-               " juju-2.1 set-model-config -m controller proxy-ssh=false")
+               " juju-2.1 model-config -m controller proxy-ssh=false")
         expected.append(mock.call(["juju", "ssh", "landscape-server/0", cmd],
                                   stderr=subprocess.STDOUT,
                                   env=None))
@@ -698,7 +698,7 @@
         expected = []
         cmd = ("sudo JUJU_DATA=/var/lib/landscape/juju-homes/"
                "`sudo ls -rt /var/lib/landscape/juju-homes/ | tail -1`"
-               " juju-2.1 set-model-config -m controller proxy-ssh=false")
+               " juju-2.1 model-config -m controller proxy-ssh=false")
         expected.append(mock.call(["juju", "ssh", "landscape/0", cmd],
                                   stderr=subprocess.STDOUT,
                                   env=None))