Merge ~ballot/charm-prometheus-alertmanager/+git/charm-prometheus-alertmanager:log-level into charm-prometheus-alertmanager:master

[Benjamin Allot]

Add the log-level config item

In order to easily debug, configuring the log-level of alertmanager can be handy.
The old default value of info (if not specified) is kept so an upgrade won't be bothered by the change.

[Drew Freiberger (afreiberger)]

I'd like to see this charm go into a blocked state for an incorrectly configured log level value, and would appreciate seeing the values accepted for log-level in the config.yaml description.

I am also concerned that this change when upgrade-charm runs will restart the service (which is typically frowned upon) even if no configuration change is expected. Maybe the default should be a blank string, which equates to not setting a --log.level, which should keep the config file from being updated and the service from restarting based on service context change.

review: Needs Fixing

[Drew Freiberger (afreiberger)]

LGTM, running tests now. Thank you so much for the contribution and updates!

review: Approve

[Drew Freiberger (afreiberger)]

tests pass. no functional testing in charm.

[Xav Paice (xavpaice)]

Ran manual functest, LGTM.

Am adding a new commit for some basic functests, outside the scope of this change.

review: Approve

diff --git a/src/config.yaml b/src/config.yaml
index 5c7bf30..fe0c1b1 100644
--- a/src/config.yaml
+++ b/src/config.yaml
@@ -9,6 +9,12 @@ options:
     type: int
     default: 9093
     description: "Alertmanager listening port"
+  log-level:
+    type: string
+    default: ""
+    description: |
+        Loggging level used for alertmanager.
+        Valid values are "error", "warn", "info" and "debug".
   external_url:
     default: ""
     type: string
diff --git a/src/reactive/alertmanager.py b/src/reactive/alertmanager.py
index e67f0e0..c9c028c 100644
--- a/src/reactive/alertmanager.py
+++ b/src/reactive/alertmanager.py
@@ -18,6 +18,7 @@ ALERTMANAGER_YML = "/var/snap/prometheus-alertmanager/current/alertmanager.yml"
 ALERTMANAGER_DEF = "/var/snap/prometheus-alertmanager/current/daemon_arguments"
 ALERTMANAGER_YML_TMPL = "alertmanager.yml.j2"
 ALERTMANAGER_DEF_TMPL = "daemon_arguments.j2"
+LOG_LEVELS = ["debug", "error", "info", "warn"]
 
 
 def templates_changed(tmpl_list):
@@ -121,8 +122,22 @@ def write_alertmanager_config_yml():
 def write_alertmanager_config_def():
     config = hookenv.config()
     port = config.get("port", "9093")
+    log_level = config.get("log-level")
+    if log_level:
+        if log_level in LOG_LEVELS:
+            runtime_args("--log.level", "{}".format(log_level))
+        else:
+            hookenv.status_set(
+                "blocked",
+                "log-level {} is unknown."
+                " Pick one of the following value: {}".format(
+                    log_level, ",".join(LOG_LEVELS)
+                ),
+            )
+            return
     check_ports(port)
     runtime_args("--web.listen-address", ":{}".format(port))
+
     if config.get("external_url", False):
         vars = {
             "private_address": hookenv.unit_get("private-address"),
diff --git a/src/tests/unit/test_reactive_alertmanager.py b/src/tests/unit/test_reactive_alertmanager.py
index 98ff2f1..ed27ba5 100644
--- a/src/tests/unit/test_reactive_alertmanager.py
+++ b/src/tests/unit/test_reactive_alertmanager.py
@@ -176,6 +176,57 @@ class TestPrometheusContext(unittest.TestCase):
                 contents, "ARGS=.*-web.listen.address :{}.*".format(fake_port)
             )
 
+    def test_log_level_configs(self, mock_get_peers, mock_hookenv_config, *args):
+        config = self.def_config
+        fake_port = "9876"
+        fake_ext_url = "http://foo:1234/bar"
+        config.update(
+            {
+                "port": fake_port,
+                "external_url": fake_ext_url,
+                "log-level": "debug",
+            }
+        )
+        mock_hookenv_config.return_value = config
+        alertmanager.write_alertmanager_config_yml()
+        alertmanager.write_alertmanager_config_def()
+        # Verify etc/default/alertmanager has --log.level
+        with open(self.cfg_def) as fh:
+            contents = fh.read()
+            self.assertRegex(contents, "ARGS=.*--log.level debug.*")
+
+    def test_bad_log_level_configs(
+        self,
+        mock_get_peers,
+        mock_hookenv_config,
+        mock_hookenv_unit_get,
+        mock_valudate_config,
+        mock_data_changed,
+        mock_host_service_running,
+        mock_host_service_restart,
+        mock_hookenv_status_set,
+        *args
+    ):
+        config = self.def_config
+        fake_port = "9876"
+        fake_ext_url = "http://foo:1234/bar"
+        config.update(
+            {
+                "port": fake_port,
+                "external_url": fake_ext_url,
+                "log-level": "dummylevel",
+            }
+        )
+        mock_hookenv_config.return_value = config
+        alertmanager.write_alertmanager_config_yml()
+        alertmanager.write_alertmanager_config_def()
+        # Verify charm status is blocked
+        mock_hookenv_status_set.assert_called_with(
+            "blocked",
+            "log-level dummylevel is unknown."
+            " Pick one of the following value: debug,error,info,warn",
+        )
+
     @mock.patch("reactive.alertmanager.remove_state")
     def test_install_packages_conditionally_called(
         self,