diff --git a/changelog/64505.fixed.md b/changelog/64505.fixed.md new file mode 100644 index 000000000000..d0b0d773b7d3 --- /dev/null +++ b/changelog/64505.fixed.md @@ -0,0 +1 @@ +Fixed incorrect minion presence events being sent out on hourly `Maintenance` process restarts \ No newline at end of file diff --git a/salt/master.py b/salt/master.py index 5a317bf93b72..211f03c178d4 100644 --- a/salt/master.py +++ b/salt/master.py @@ -258,7 +258,16 @@ def run(self): now = int(time.time()) git_pillar_update_interval = self.opts.get("git_pillar_update_interval", 0) - old_present = set() + # load the presence data from the cache on disk + presence_cache = salt.utils.cache.CacheFactory.factory( + "disk", + 3600, + minion_cache_path=os.path.join(self.opts["cachedir"], "presence-data"), + ) + try: + last_present = set(presence_cache["present"]) + except KeyError: + last_present = set() while time.time() - start < self.restart_interval: log.trace("Running maintenance routines") if not last or (now - last) >= self.loop_interval: @@ -270,7 +279,7 @@ def run(self): self.handle_git_pillar() self.handle_schedule() self.handle_key_cache() - self.handle_presence(old_present) + self.handle_presence(last_present) self.handle_key_rotate(now) salt.utils.verify.check_max_open_files(self.opts) last = now @@ -375,6 +384,14 @@ def handle_presence(self, old_present): old_present.clear() old_present.update(present) + # update the cache on disk + presence_cache = salt.utils.cache.CacheFactory.factory( + "disk", + 3600, + minion_cache_path=os.path.join(self.opts["cachedir"], "presence-data"), + ) + presence_cache["present"] = list(set(present)) + class FileserverUpdate(salt.utils.process.SignalHandlingProcess): """ @@ -698,7 +715,6 @@ def start(self): # manager. We don't want the processes being started to inherit those # signal handlers with salt.utils.process.default_signals(signal.SIGINT, signal.SIGTERM): - # Setup the secrets here because the PubServerChannel may need # them as well. SMaster.secrets["aes"] = { diff --git a/tests/pytests/unit/test_master.py b/tests/pytests/unit/test_master.py index 502767d3e343..f86a63247773 100644 --- a/tests/pytests/unit/test_master.py +++ b/tests/pytests/unit/test_master.py @@ -1,10 +1,29 @@ +import os import time import pytest import salt.master import salt.utils.platform -from tests.support.mock import patch +from tests.support.mock import MagicMock, call, patch + + +@pytest.fixture +def maintenance_opts(master_opts): + """ + Options needed for master's Maintenence class + """ + opts = master_opts.copy() + opts.update(git_pillar_update_interval=180, maintenance_interval=181) + yield opts + + +@pytest.fixture +def maintenance(maintenance_opts): + """ + The master's Maintenence class + """ + return salt.master.Maintenance(maintenance_opts) @pytest.fixture @@ -160,3 +179,90 @@ def test_when_syndic_return_processes_load_then_correct_values_should_be_returne with patch.object(encrypted_requests, "_return", autospec=True) as fake_return: encrypted_requests._syndic_return(payload) fake_return.assert_called_with(expected_return) + + +@pytest.mark.parametrize( + "presence_cache_data,connected_ids,change_expected", + [ + ( + ["minion1", "minion2"], + {"minion1", "minion2"}, + False, + ), + ( + ["minion1"], + {"minion1", "minion2"}, + True, + ), + ( + ["minion1", "minion2"], + {"minion1"}, + True, + ), + ], +) +def test_handle_presence( + maintenance, presence_cache_data, connected_ids, change_expected +): + """ + Test the handle_presence function inside Maintenance class. + """ + fire_event = MagicMock() + + # populate the presence cache with old data + presence_cache = salt.utils.cache.CacheFactory.factory( + "disk", + 3600, + minion_cache_path=os.path.join(maintenance.opts["cachedir"], "presence-data"), + ) + presence_cache.clear() + presence_cache["present"] = presence_cache_data + + with patch("salt.master.Maintenance.run", MagicMock()), patch( + "salt.master.Maintenance.presence_events", True, create=True + ), patch( + "salt.master.Maintenance.event", + MagicMock( + connect_pull=MagicMock(return_value=True), + fire_event=fire_event, + ), + create=True, + ), patch( + "salt.master.Maintenance.ckminions", + MagicMock(connected_ids=MagicMock(return_value=connected_ids)), + create=True, + ): + maintenance.handle_presence(set(presence_cache["present"])) + + assert fire_event.called + + if change_expected: + assert fire_event.call_count == 2 + change_events = [ + call[0][0] + for call in fire_event.call_args_list + if "/change" in call[0][1] + ] + assert change_events + else: + assert fire_event.call_count == 1 + + present_event = [ + call[0][0] for call in fire_event.call_args_list if "/present" in call[0][1] + ][0] + assert ( + set(present_event["present"]) == connected_ids + ), "The presence event sent does not contain the expected minions" + + # reload the presence data from disk + new_presence_cache = salt.utils.cache.CacheFactory.factory( + "disk", + 3600, + minion_cache_path=os.path.join( + maintenance.opts["cachedir"], "presence-data" + ), + ) + new_present = set(new_presence_cache["present"]) + assert ( + new_present == connected_ids + ), "The presence cache on disk does not contain the expected minions"