Merge ~gabrielcocenza/juju-lint:models-no-apps into juju-lint:master

[Gabriel Cocenza]

update get_juju_bundle method

- models without apps don't crash juju-lint.
- added saas into cloud_state for future cmr support.
- added unit tests and fixtures for cloud module.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Eric Chen (eric-chen)]

+1

review: Approve

[Martin Kalcok (martin-kalcok)]

+1

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision a04794c69be9e3c34979fc4a1f1a89f0ce175f1e

diff --git a/jujulint/cloud.py b/jujulint/cloud.py
index 3382e3b..6d0ec44 100644
--- a/jujulint/cloud.py
+++ b/jujulint/cloud.py
@@ -34,7 +34,7 @@ Todo:
 
 """
 import socket
-from subprocess import check_output
+from subprocess import CalledProcessError, check_output
 
 import yaml
 from fabric2 import Config, Connection
@@ -94,6 +94,7 @@ class Cloud:
             self.logger.debug("Running local command: {}".format(command))
             args = command.split(" ")
             return check_output(args)
+
         elif self.access_method == "ssh":
             if self.sudo_user:
                 self.logger.debug(
@@ -272,9 +273,22 @@ class Cloud:
 
     def get_juju_bundle(self, controller, model):
         """Get an export of the juju bundle for the provided model."""
-        bundle_data = self.run_command(
-            "juju export-bundle -m {}:{}".format(controller, model)
-        )
+        try:
+            bundle_data = self.run_command(
+                "juju export-bundle -m {}:{}".format(controller, model)
+            )
+        except CalledProcessError as e:
+            self.logger.error(e)
+            self.logger.warn(
+                (
+                    "An error happened to get the bundle on {}:{}. "
+                    "If the model doesn't have apps, disconsider this message.".format(
+                        controller, model
+                    )
+                )
+            )
+            return
+
         bundles = self.parse_yaml(bundle_data)
         self.logger.info(
             "[{}] Processing Juju bundle export for model {} on controller {}".format(
@@ -286,37 +300,33 @@ class Cloud:
                 model, controller, bundles
             )
         )
-        if len(bundles) > 0:
-            combined = {}
-            for bundle in bundles:
-                combined.update(bundle)
-            if "applications" in combined:
-                for application in combined["applications"].keys():
+        # NOTE(gabrielcocenza) export-bundle can have an overlay when there is crm.
+        for bundle in bundles:
+            if "applications" in bundle:
+                for application in bundle["applications"].keys():
                     self.logger.debug(
                         "Parsing configuration for application {} in model {}: {}".format(
-                            application, model, combined
+                            application, model, bundle
                         )
                     )
-                    application_config = combined["applications"][application]
-                    if (
+                    application_config = bundle["applications"][application]
+                    self.cloud_state[controller]["models"][model].setdefault(
+                        "applications", {}
+                    )
+
+                    self.cloud_state[controller]["models"][model][
                         "applications"
-                        not in self.cloud_state[controller]["models"][model]
-                    ):
+                    ].setdefault(application, {}).update(application_config)
+            if "saas" in bundle:
+                for app in bundle.get("saas"):
+                    # offer side doesn't show the url of the app
+                    if bundle["saas"][app].get("url"):
+                        self.cloud_state[controller]["models"][model].setdefault(
+                            "saas", {}
+                        ).update(bundle["saas"])
                         self.cloud_state[controller]["models"][model][
-                            "applications"
-                        ] = {}
-                    if (
-                        application
-                        not in self.cloud_state[controller]["models"][model][
-                            "applications"
-                        ].keys()
-                    ):
-                        self.cloud_state[controller]["models"][model]["applications"][
-                            application
-                        ] = {}
-                    self.cloud_state[controller]["models"][model]["applications"][
-                        application
-                    ].update(application_config)
+                            "saas"
+                        ].setdefault(app, {}).update(bundle["saas"][app])
 
     def get_juju_state(self):
         """Update our view of Juju-managed application state."""
diff --git a/tests/conftest.py b/tests/conftest.py
index 55ac1ac..b43a273 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -10,6 +10,7 @@
 
 import os
 import sys
+from unittest.mock import MagicMock
 
 import mock
 import pytest
@@ -76,6 +77,24 @@ def linter(parser):
 
 
 @pytest.fixture
+def cloud():
+    """Provide a Cloud instance to test."""
+    from jujulint.cloud import Cloud
+
+    rules = {
+        "known charms": ["nrpe", "ubuntu", "nagios"],
+        "operations mandatory": ["nagios"],
+    }
+    cloud = Cloud(name="test_cloud", lint_rules=rules)
+    # set initial cloud state
+    cloud.cloud_state = {
+        "my_controller": {"models": {"my_model_1": {}, "my_model_2": {}}}
+    }
+    cloud.logger = MagicMock()
+    return cloud
+
+
[e-mail]
 def juju_status():
     """Provide a base juju status for testing."""
     return {
@@ -125,3 +144,69 @@ def juju_status():
             },
         },
     }
+
+
[e-mail]
+def juju_export_bundle():
+    """Simulate a cloud with one controller and two bundles.
+
+    my_model_1 nrpe offers the monitors endpoint
+    my_model_2 nagios consumes the monitors endpoint from my_model_1
+    """
+    return {
+        "my_model_1": [
+            {
+                "series": "focal",
+                "saas": {"remote-2290e64ea1ac41858eb06a69b6a9d8cc": {}},
+                "applications": {
+                    "nrpe": {"charm": "nrpe", "channel": "stable", "revision": 86},
+                    "ubuntu": {
+                        "charm": "ubuntu",
+                        "channel": "stable",
+                        "revision": 19,
+                        "num_units": 1,
+                        "to": ["0"],
+                        "constraints": "arch=amd64",
+                    },
+                },
+                "machines": {"0": {"constraints": "arch=amd64"}},
+                "relations": [
+                    ["nrpe:general-info", "ubuntu:juju-info"],
+                    [
+                        "nrpe:monitors",
+                        "remote-2290e64ea1ac41858eb06a69b6a9d8cc:monitors",
+                    ],
+                ],
+            },
+            {
+                "applications": {
+                    "nrpe": {
+                        "offers": {
+                            "nrpe": {
+                                "endpoints": ["monitors"],
+                                "acl": {"admin": "admin"},
+                            }
+                        }
+                    }
+                }
+            },
+        ],
+        "my_model_2": [
+            {
+                "series": "bionic",
+                "saas": {"nrpe": {"url": "my_controller:admin/my_model_1.nrpe"}},
+                "applications": {
+                    "nagios": {
+                        "charm": "nagios",
+                        "channel": "stable",
+                        "revision": 49,
+                        "num_units": 1,
+                        "to": ["0"],
+                        "constraints": "arch=amd64",
+                    }
+                },
+                "machines": {"0": {"constraints": "arch=amd64"}},
+                "relations": [["nagios:monitors", "nrpe:monitors"]],
+            }
+        ],
+    }
diff --git a/tests/test_cloud.py b/tests/test_cloud.py
new file mode 100644
index 0000000..885f319
--- /dev/null
+++ b/tests/test_cloud.py
@@ -0,0 +1,129 @@
+#!/usr/bin/python3
+"""Tests for Cloud."""
+from subprocess import CalledProcessError
+from unittest.mock import call, patch
+
+
+class TestCloud:
+    """Test the main Cloud class."""
+
+    @patch("jujulint.cloud.check_output")
+    def test_get_bundle_no_apps(self, mock_check_out, cloud):
+        """Models with no apps raises CalledProcessError to export bundle."""
+        cmd = ["juju", "export-bundle", "-m", "my_controller:controller"]
+        e = CalledProcessError(1, cmd)
+        mock_check_out.side_effect = e
+        cloud.get_juju_bundle("my_controller", "controller")
+        expected_error_msg = call.error(e)
+
+        expected_warn_msg = call.warn(
+            (
+                "An error happened to get the bundle on my_controller:controller. "
+                "If the model doesn't have apps, disconsider this message."
+            )
+        )
+        assert expected_error_msg in cloud.logger.method_calls
+        assert expected_warn_msg in cloud.logger.method_calls
+
+    @patch("jujulint.cloud.Cloud.parse_yaml")
+    @patch("jujulint.cloud.Cloud.run_command")
+    def test_get_bundle_offer_side(
+        self, mock_run, mock_parse, cloud, juju_export_bundle
+    ):
+        """Test the bundle generated in the offer side."""
+        # simulate cloud_state with info that came from "get_juju_status"
+        cloud.cloud_state = {
+            "my_controller": {
+                "models": {
+                    "my_model_1": {
+                        "applications": {
+                            "nrpe": {
+                                "charm-origin": "charmhub",
+                                "os": "ubuntu",
+                                "endpoint-bindings": {"": "alpha", "monitors": "alpha"},
+                            }
+                        }
+                    },
+                    "my_model_2": {},
+                }
+            }
+        }
+        mock_parse.return_value = juju_export_bundle["my_model_1"]
+        # "offers" field exists inside nrpe because of the overlay bundle.
+        # saas field doesn't exist in the offer side because there is no url.
+        # don't overwrite information that came from "get_juju_status".
+        expected_cloud_state = {
+            "my_controller": {
+                "models": {
+                    "my_model_1": {
+                        "applications": {
+                            "nrpe": {
+                                "charm": "nrpe",
+                                "charm-origin": "charmhub",
+                                "os": "ubuntu",
+                                "endpoint-bindings": {"": "alpha", "monitors": "alpha"},
+                                "channel": "stable",
+                                "revision": 86,
+                                "offers": {
+                                    "nrpe": {
+                                        "endpoints": ["monitors"],
+                                        "acl": {"admin": "admin"},
+                                    }
+                                },
+                            },
+                            "ubuntu": {
+                                "charm": "ubuntu",
+                                "channel": "stable",
+                                "revision": 19,
+                                "num_units": 1,
+                                "to": ["0"],
+                                "constraints": "arch=amd64",
+                            },
+                        }
+                    },
+                    "my_model_2": {},
+                }
+            }
+        }
+        cloud.get_juju_bundle("my_controller", "my_model_1")
+        assert mock_run.called_once_with(
+            "juju export-bundle -m my_controller:my_model_1"
+        )
+        assert cloud.cloud_state == expected_cloud_state
+
+    @patch("jujulint.cloud.Cloud.parse_yaml")
+    @patch("jujulint.cloud.Cloud.run_command")
+    def test_get_bundle_consumer_side(
+        self, mock_run, mock_parse, cloud, juju_export_bundle
+    ):
+        """Test the bundle generated in the consumer side."""
+        mock_parse.return_value = juju_export_bundle["my_model_2"]
+        # "offers" field won't exist in the consumer side
+        # saas field exists because the consumer side shows url
+        expected_cloud_state = {
+            "my_controller": {
+                "models": {
+                    "my_model_1": {},
+                    "my_model_2": {
+                        "applications": {
+                            "nagios": {
+                                "charm": "nagios",
+                                "channel": "stable",
+                                "revision": 49,
+                                "num_units": 1,
+                                "to": ["0"],
+                                "constraints": "arch=amd64",
+                            }
+                        },
+                        "saas": {
+                            "nrpe": {"url": "my_controller:admin/my_model_1.nrpe"}
+                        },
+                    },
+                }
+            }
+        }
+        cloud.get_juju_bundle("my_controller", "my_model_2")
+        assert mock_run.called_once_with(
+            "juju export-bundle -m my_controller:my_model_2"
+        )
+        assert cloud.cloud_state == expected_cloud_state