[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 <[email protected]>

sos/collector/clusters/juju.py

@@ -13,6 +13,8 @@
 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 @@ def _get_model_info(self, model_name):
 
         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:

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"]
@@ -67,11 +71,17 @@ def test_get_nodes_no_filter(self, mock_exec_primary_cmd):
         nodes = cluster.get_nodes()
         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(
@@ -95,11 +105,17 @@ def test_get_nodes_app_filter(self, mock_exec_primary_cmd):
             "juju status  --format json"
         )
 
+    @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(
@@ -123,12 +139,16 @@ def test_get_nodes_app_regex_filter(self, mock_exec_primary_cmd):
             "juju status  --format json"
         )
 
+    @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()
@@ -170,11 +190,17 @@ def test_get_nodes_model_filter_multiple_models(
             ]
         )
 
+    @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(
@@ -212,11 +238,17 @@ def test_get_nodes_model_filter(self, mock_exec_primary_cmd):
             ]
         )
 
+    @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(
@@ -237,11 +269,17 @@ def test_get_nodes_unit_filter(self, mock_exec_primary_cmd):
         nodes.sort()
         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(
@@ -263,11 +301,15 @@ def test_get_nodes_machine_filter(self, mock_exec_primary_cmd):
         print(nodes)
         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(