[juju][collect] Fix new juju collection

New version of juju uses colorisation, and therefore juju status
and json.loads doesn't load the juju status correctly. By using
--no-color based on the version of juju this should fix this
particular use-case

Resolves: #3399
Resolves: SET-339

Signed-off-by: Arif Ali <arif.ali@canonical.com>

2 files changed, 58 insertions, 7 deletions

diff --git a/sos/collector/clusters/juju.py b/sos/collector/clusters/juju.py
index a44d4451..be69759a 100644
--- a/sos/collector/clusters/juju.py
+++ b/sos/collector/clusters/juju.py
@@ -13,6 +13,8 @@ import json
 import re
 
 from sos.collector.clusters import Cluster
+from sos.utilities import parse_version
+from sos.utilities import sos_get_command_output
 
 
 def _parse_option_string(strings=None):
@@ -156,9 +158,16 @@ class juju(Cluster):
 
         return index
 
+    def _get_juju_version(self):
+        """Grab the version of juju"""
+        res = sos_get_command_output("juju version")
+        return res['output'].split("-")[0]
+
     def _execute_juju_status(self, model_name):
         model_option = f"-m {model_name}" if model_name else ""
         format_option = "--format json"
+        if parse_version(self._get_juju_version()) > parse_version("3"):
+            format_option += " --no-color"
         status_cmd = f"{self.cmd} status {model_option} {format_option}"
         res = self.exec_primary_cmd(status_cmd)
         if not res["status"] == 0:
diff --git a/tests/unittests/juju/juju_cluster_tests.py b/tests/unittests/juju/juju_cluster_tests.py
index eae50b71..1c8054f6 100644
--- a/tests/unittests/juju/juju_cluster_tests.py
+++ b/tests/unittests/juju/juju_cluster_tests.py
@@ -40,6 +40,10 @@ def get_juju_status(cmd):
     }
 
 
+def get_juju_version():
+    return "2.9.45"
+
+
 def test_parse_option_string():
     result = _parse_option_string("    a,b,c")
     assert result == ["a", "b", "c"]
@@ -68,10 +72,16 @@ class JujuTest(unittest.TestCase):
         assert nodes == []
 
     @patch(
+        "sos.collector.clusters.juju.juju._get_juju_version",
+        side_effect=get_juju_version,
+    )
+    @patch(
         "sos.collector.clusters.juju.juju.exec_primary_cmd",
         side_effect=get_juju_status,
     )
-    def test_get_nodes_app_filter(self, mock_exec_primary_cmd):
+    def test_get_nodes_app_filter(
+        self, mock_exec_primary_cmd, mock_get_juju_version
+    ):
         """Application filter."""
         mock_opts = MockOptions()
         mock_opts.cluster_options.append(
@@ -96,10 +106,16 @@ class JujuTest(unittest.TestCase):
         )
 
     @patch(
+        "sos.collector.clusters.juju.juju._get_juju_version",
+        side_effect=get_juju_version,
+    )
+    @patch(
         "sos.collector.clusters.juju.juju.exec_primary_cmd",
         side_effect=get_juju_status,
     )
-    def test_get_nodes_app_regex_filter(self, mock_exec_primary_cmd):
+    def test_get_nodes_app_regex_filter(
+        self, mock_exec_primary_cmd, mock_get_juju_version
+    ):
         """Application filter."""
         mock_opts = MockOptions()
         mock_opts.cluster_options.append(
@@ -124,11 +140,15 @@ class JujuTest(unittest.TestCase):
         )
 
     @patch(
+        "sos.collector.clusters.juju.juju._get_juju_version",
+        side_effect=get_juju_version,
+    )
+    @patch(
         "sos.collector.clusters.juju.juju.exec_primary_cmd",
         side_effect=get_juju_status,
     )
     def test_get_nodes_model_filter_multiple_models(
-        self, mock_exec_primary_cmd
+        self, mock_exec_primary_cmd, mock_get_juju_version
     ):
         """Multiple model filter."""
         mock_opts = MockOptions()
@@ -171,10 +191,16 @@ class JujuTest(unittest.TestCase):
         )
 
     @patch(
+        "sos.collector.clusters.juju.juju._get_juju_version",
+        side_effect=get_juju_version,
+    )
+    @patch(
         "sos.collector.clusters.juju.juju.exec_primary_cmd",
         side_effect=get_juju_status,
     )
-    def test_get_nodes_model_filter(self, mock_exec_primary_cmd):
+    def test_get_nodes_model_filter(
+        self, mock_exec_primary_cmd, mock_get_juju_version
+    ):
         """Model filter."""
         mock_opts = MockOptions()
         mock_opts.cluster_options.append(
@@ -213,10 +239,16 @@ class JujuTest(unittest.TestCase):
         )
 
     @patch(
+        "sos.collector.clusters.juju.juju._get_juju_version",
+        side_effect=get_juju_version,
+    )
+    @patch(
         "sos.collector.clusters.juju.juju.exec_primary_cmd",
         side_effect=get_juju_status,
     )
-    def test_get_nodes_unit_filter(self, mock_exec_primary_cmd):
+    def test_get_nodes_unit_filter(
+        self, mock_exec_primary_cmd, mock_get_juju_version
+    ):
         """Node filter."""
         mock_opts = MockOptions()
         mock_opts.cluster_options.append(
@@ -238,10 +270,16 @@ class JujuTest(unittest.TestCase):
         assert nodes == [":0", ":2"]
 
     @patch(
+        "sos.collector.clusters.juju.juju._get_juju_version",
+        side_effect=get_juju_version,
+    )
+    @patch(
         "sos.collector.clusters.juju.juju.exec_primary_cmd",
         side_effect=get_juju_status,
     )
-    def test_get_nodes_machine_filter(self, mock_exec_primary_cmd):
+    def test_get_nodes_machine_filter(
+        self, mock_exec_primary_cmd, mock_get_juju_version
+    ):
         """Machine filter."""
         mock_opts = MockOptions()
         mock_opts.cluster_options.append(
@@ -264,10 +302,14 @@ class JujuTest(unittest.TestCase):
         assert nodes == [":0", ":2"]
 
     @patch(
+        "sos.collector.clusters.juju.juju._get_juju_version",
+        side_effect=get_juju_version,
+    )
+    @patch(
         "sos.collector.clusters.juju.juju.exec_primary_cmd",
         side_effect=get_juju_status,
     )
-    def test_subordinates(self, mock_exec_primary_cmd):
+    def test_subordinates(self, mock_exec_primary_cmd, mock_get_juju_version):
         """Subordinate filter."""
         mock_opts = MockOptions()
         mock_opts.cluster_options.append(