fix: erroneous "new" minion presence events on process startup#64508
fix: erroneous "new" minion presence events on process startup#64508raddessi wants to merge 10 commits intosaltstack:3006.xfrom
Conversation
salt-project-bot-prod-environment
bot
changed the title
raddessi
changed the title
|
Just as background info, I noticed this because I use the slack engine and presence reactor. |
raddessi
changed the title
|
so, this should not be labeled a feature, or it should be going into master. if this is a fix for a bug it should be labeled as such. if it is a new feature it needs to go into master. Next, it is going to need a set of tests and changelog. as well as fixing the pre-commit which just looks like an isort issue. |
|
| so, this should not be labeled a feature, or it should be going into master Good point. I started writing this as a feature but it does appear to be more of a bug fix at this point. I'll swap the title thank you. I'll add tests etc once I get approval on the concept. I don't want to spend time on that if this gets denied |
|
Apologies for the bump, can I get another review when you have time @dwoz - Thanks! If this looks good I will add tests and docs |
|
bumping this @dwoz @cmcmarrow, if the implementation looks good I will add tests |
|
test added |
|
Changelog entry added, I don't think we need docs on this one as I said above. Ready to go if you think this is the right approach 🤞 Not that I would like to add more work for myself but does this need to be able to wipe the cache file if it becomes corrupted for some reason? |
| import salt.utils.platform | ||
| from tests.support.mock import patch | ||
| from tests.support.mock import MagicMock, 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) | ||
|
|
There was a problem hiding this comment.
I copied this in from master
|
wait this needs to check for the change event also dammit. Will fix |
|
Are these conflicts able to be resolved? |
5 participants
What does this PR do?
This fixes the issue of the master processes sending incorrect presence events on either the restart of the salt-master process or any of the
Maintenanceprocess restart events (default is every hour). This issue began in 3006.0.Although this was intended to fix the hourly incorrect presence update this also fixes the spammy presence updates between salt-master process restarts.
What issues does this PR fix or reference?
Fixes: #64505
Previous Behavior
The salt-master process sends incorrect presence events at every
restart_intervalof theMaintenanceprocess, signalling that every node connected to the master is "new".New Behavior
Only actual presence events are sent.
Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Maintenanceprocess in the current documentation so I think this does not need a specific mention since we are fixing a bugI will update all of these once this is commented on and direction is approved.
Commits signed with GPG?
Yes
Please review Salt's Contributing Guide for best practices.
See GitHub's page on GPG signing for more information about signing commits with GPG.