Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE REQUEST] Allow reload_modules to reload beacons #60541

Closed
garethgreenaway opened this issue Jul 14, 2021 · 3 comments · Fixed by #60542
Closed

[FEATURE REQUEST] Allow reload_modules to reload beacons #60541

garethgreenaway opened this issue Jul 14, 2021 · 3 comments · Fixed by #60542
Assignees
Labels
Feature new functionality including changes to functionality and code refactors, etc. Silicon v3004.0 Release code name
Milestone

Comments

@garethgreenaway
Copy link
Contributor

Is your feature request related to a problem? Please describe.
When reload_modules is included in a state, any modules will be reloaded used by the functions for Salt as well as any returners. This is useful when installing Python libraries via a Salt state and not wanting to have to restart the Salt minion for those new functions to be become available. Currently reload_modules does not refresh available beacons when set to True. This would be helpful for similar reasons.

Describe the solution you'd like
When reload_modules is included in a state in addition to available modules being refreshed, the available beacons will be refreshed.

Please Note
If this feature request would be considered a substantial change or addition, this should go through a SEP process here https://github.com/saltstack/salt-enhancement-proposals, instead of a feature request.

@garethgreenaway garethgreenaway added Feature new functionality including changes to functionality and code refactors, etc. needs-triage labels Jul 14, 2021
@garethgreenaway garethgreenaway self-assigned this Jul 14, 2021
@garethgreenaway garethgreenaway added Silicon v3004.0 Release code name and removed needs-triage labels Jul 14, 2021
@garethgreenaway garethgreenaway added this to the Approved milestone Jul 14, 2021
@OrangeDog
Copy link
Contributor

Surely this should be a new arg, reload_beacons? The other systems all have their own separate function.

have to restart the Salt minion

Last time I tested it, they're available on the next run. No service restart is necessary.

@cbosdo
Copy link
Contributor

cbosdo commented Jul 22, 2021

There is the other way around as well: imagine a pillar defining the beacons to be running. The minion pillar_refresh() function first reload the modules, then fetches the pillar and then reload beacons and matchers... but the LazyLoader doesn't get the new pillar and the modules would need to be reloaded once more.

Looks like we'll need to watch out for reloading loops at some point ➿

@garethgreenaway
Copy link
Contributor Author

@OrangeDog I debated having a new argument but the existing code handles refreshing of functions in execution modules, state modules, and returner modules so having it also refresh the beacon modules made sense.

You are correct that the beacon will be available on the next run but for the particular use case that inspired this addition that doesn't work. For example, If you attempt to setup the inotify beacon via a Salt state before the necessary Python libraries are installed, the beacon will be configured and started but will produce an error.

With this change, ensuring that reload_modules is set to True on the state that is installing the library, the library can be installed and the available beacon modules will be reloaded before the beacon is configured.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature new functionality including changes to functionality and code refactors, etc. Silicon v3004.0 Release code name
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants