Read state file in cli to pass the info to the checks and summaries

check_patroni/cli.py

@@ -333,11 +333,18 @@ def cluster_config_has_changed(
             "Either --hash or --state-file should be provided for this service", ctx
         )
 
+    old_config_hash = config_hash
+    if state_file is not None:
+        cookie = nagiosplugin.Cookie(state_file)
+        cookie.open()
+        old_config_hash = cookie.get("hash")
+        cookie.close()
+
     check = nagiosplugin.Check()
     check.add(
-        ClusterConfigHasChanged(ctx.obj.connection_info, config_hash, state_file),
+        ClusterConfigHasChanged(ctx.obj.connection_info, old_config_hash, state_file),
         nagiosplugin.ScalarContext("is_configuration_changed", None, "@1:1"),
-        ClusterConfigHasChangedSummary(),
+        ClusterConfigHasChangedSummary(old_config_hash),
     )
     check.main(verbose=ctx.obj.verbose, timeout=ctx.obj.timeout)
 
@@ -471,12 +478,19 @@ def node_tl_has_changed(ctx: click.Context, timeline: str, state_file: str) -> N
             "Either --timeline or --state-file should be provided for this service", ctx
         )
 
+    old_timeline = timeline
+    if state_file is not None:
+        cookie = nagiosplugin.Cookie(state_file)
+        cookie.open()
+        old_timeline = cookie.get("timeline")
+        cookie.close()
+
     check = nagiosplugin.Check()
     check.add(
-        NodeTLHasChanged(ctx.obj.connection_info, timeline, state_file),
+        NodeTLHasChanged(ctx.obj.connection_info, old_timeline, state_file),
         nagiosplugin.ScalarContext("is_timeline_changed", None, "@1:1"),
         nagiosplugin.ScalarContext("timeline"),
-        NodeTLHasChangedSummary(timeline),
+        NodeTLHasChangedSummary(old_timeline),
     )
     check.main(verbose=ctx.obj.verbose, timeout=ctx.obj.timeout)
 

check_patroni/cluster.py

@@ -129,8 +129,8 @@ class ClusterConfigHasChanged(PatroniResource):
     def __init__(
         self: "ClusterConfigHasChanged",
         connection_info: ConnectionInfo,
-        config_hash: str,
-        state_file: str,
+        config_hash: str,  # Always contains the old hash
+        state_file: str,   # Only used to update the hash in the state_file (when needed)
     ):
         super().__init__(connection_info)
         self.state_file = state_file
@@ -143,16 +143,14 @@ class ClusterConfigHasChanged(PatroniResource):
 
         new_hash = hashlib.md5(r.data).hexdigest()
 
+        old_hash = self.config_hash
         if self.state_file is not None:
-            _log.debug(f"Using state file / cookie {self.state_file}")
+            _log.debug(f"Saving new hash to state file / cookie {self.state_file}")
             cookie = nagiosplugin.Cookie(self.state_file)
             cookie.open()
-            old_hash = cookie.get("hash")
             cookie["hash"] = new_hash
             cookie.commit()
-        else:
-            _log.debug(f"Using input value {self.config_hash}")
-            old_hash = self.config_hash
+            cookie.close()
 
         _log.debug(f"hash info: old hash {old_hash}, new hash {new_hash}")
 
@@ -165,15 +163,19 @@ class ClusterConfigHasChanged(PatroniResource):
 
 
 class ClusterConfigHasChangedSummary(nagiosplugin.Summary):
-    # TODO: It would be helpful to display the old / new hash here, but it's not a metric.
+    def __init__(self: "ClusterConfigHasChangedSummary", config_hash: str) -> None:
+        self.old_config_hash = config_hash
+
+    # Note: It would be helpful to display the old / new hash here. Unfortunately, it's not a metric.
+    # So we only have the old / expected one.
     def ok(self: "ClusterConfigHasChangedSummary", results: nagiosplugin.Result) -> str:
-        return "The hash of patroni's dynamic configuration has not changed."
+        return f"The hash of patroni's dynamic configuration has not changed ({self.old_config_hash})."
 
     @handle_unknown
     def problem(
         self: "ClusterConfigHasChangedSummary", results: nagiosplugin.Result
     ) -> str:
-        return "The hash of patroni's dynamic configuration has changed."
+        return "The hash of patroni's dynamic configuration has changed. The old hash was {self.old_config_hash}."
 
 
 class ClusterIsInMaintenance(PatroniResource):

check_patroni/node.py

@@ -92,8 +92,8 @@ class NodeTLHasChanged(PatroniResource):
     def __init__(
         self: "NodeTLHasChanged",
         connection_info: ConnectionInfo,
-        timeline: str,
-        state_file: str,
+        timeline: str,    # Always contains the old timeline
+        state_file: str,  # Only used to update the timeline in the state_file (when needed)
     ) -> None:
         super().__init__(connection_info)
         self.state_file = state_file
@@ -107,16 +107,14 @@ class NodeTLHasChanged(PatroniResource):
         item_dict = json.loads(r.data)
         new_tl = item_dict["timeline"]
 
+        old_tl = self.timeline
         if self.state_file is not None:
-            _log.debug(f"Using state file / cookie {self.state_file}")
+            _log.debug(f"Saving new timeline to state file / cookie {self.state_file}")
             cookie = nagiosplugin.Cookie(self.state_file)
             cookie.open()
-            old_tl = cookie.get("timeline")
             cookie["timeline"] = new_tl
             cookie.commit()
-        else:
-            _log.debug(f"Using input value {self.timeline}")
-            old_tl = self.timeline
+            cookie.close()
 
         _log.debug(f"Tl data: old tl {old_tl}, new tl {new_tl}")
 

test/test_cluster_config_has_changed.py

@@ -4,6 +4,8 @@ from pytest_mock import MockerFixture
 from check_patroni.cli import main
 from tools import my_mock, here
 
+import nagiosplugin
+
 
 def test_cluster_config_has_changed_params(mocker: MockerFixture) -> None:
     runner = CliRunner()
@@ -101,3 +103,11 @@ def test_cluster_config_has_changed_ko_with_state_file(mocker: MockerFixture) ->
         ],
     )
     assert result.exit_code == 2
+
+    # the new hash was saved
+    cookie = nagiosplugin.Cookie(here / "cluster_config_has_changed.state_file")
+    cookie.open()
+    new_config_hash = cookie.get("hash")
+    cookie.close()
+
+    assert new_config_hash == "640df9f0211c791723f18fc3ed9dbb95"

test/test_node_tl_has_changed.py

@@ -3,6 +3,8 @@ from pytest_mock import MockerFixture
 
 from check_patroni.cli import main
 
+import nagiosplugin
+
 from tools import my_mock, here
 
 
@@ -102,3 +104,11 @@ def test_node_tl_has_changed_ko_with_state_file(mocker: MockerFixture) -> None:
         ],
     )
     assert result.exit_code == 2
+
+    # the new timeline was saved
+    cookie = nagiosplugin.Cookie(here / "node_tl_has_changed.state_file")
+    cookie.open()
+    new_tl = cookie.get("timeline")
+    cookie.close()
+
+    assert new_tl == 58