From cf3af27b34b934b77e6c702270467af9de9e7c9f Mon Sep 17 00:00:00 2001 From: Arif Ali Date: Mon, 30 Oct 2023 23:07:08 +0000 Subject: [PATCH] [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 --- sos/collector/clusters/juju.py | 9 ++++ tests/unittests/juju/juju_cluster_tests.py | 56 +++++++++++++++++++--- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/sos/collector/clusters/juju.py b/sos/collector/clusters/juju.py index a44d445115..be69759a27 100644 --- a/sos/collector/clusters/juju.py +++ b/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: diff --git a/tests/unittests/juju/juju_cluster_tests.py b/tests/unittests/juju/juju_cluster_tests.py index eae50b71ee..1c8054f69d 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"] @@ -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(