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

Add "salt" beacon #56575

Merged
merged 4 commits into from
Mar 2, 2021
Merged

Add "salt" beacon #56575

merged 4 commits into from
Mar 2, 2021

Conversation

jtraub91
Copy link
Contributor

@jtraub91 jtraub91 commented Apr 7, 2020

What does this PR do?

Adds new beacon: salt_monitor

What issues does this PR fix or reference?

Closes #56461

New Behavior

Allows user to specify one or more salt function (optionally with args / kwargs) to execute every beacon interval. The beacon will fire an event only if the return from its function is "truthy" and the event will include the function call (with args / kwargs, if applicable), as well as the return data.

Tests written?

Yes

Commits signed with GPG?

No

@jtraub91 jtraub91 requested a review from a team as a code owner April 7, 2020 21:20
@ghost ghost requested a review from twangboy April 7, 2020 21:20
@jtraub91 jtraub91 force-pushed the add_monitor_beacon branch from 40d3466 to 15e8cea Compare April 7, 2020 22:15
@Ch3LL Ch3LL removed the request for review from a team April 15, 2020 14:13
@jtraub91
Copy link
Contributor Author

Did anyone get a chance to look at this PR?

@jtraub91 jtraub91 force-pushed the add_monitor_beacon branch 3 times, most recently from fba7479 to 3562690 Compare September 22, 2020 03:29
@jtraub91
Copy link
Contributor Author

@twangboy I fixed the failing tests. Can this PR be reviewed, please ?

@jtraub91
Copy link
Contributor Author

@twangboy bump

pr suggestions
xeacott
xeacott previously approved these changes Jan 26, 2021
@jtraub91
Copy link
Contributor Author

@xeacott Does your approval mean this will go into Aluminum? Sorry to bug ya, but I'm just curious and wanted to make sure this gets included before it becomes too out of date with master.

@xeacott
Copy link
Contributor

xeacott commented Jan 28, 2021

@jtraub91 Unfortunately no; we have a traditional ruling in the Core team that most PR's require 3 approvals to be considered "done done" by us. I will kindly ping @sagetherage as she can provide a much better answer about this getting Aluminum.

@sagetherage sagetherage requested a review from s0undt3ch February 1, 2021 22:50
@sagetherage sagetherage added Aluminium Release Post Mg and Pre Si Feature new functionality including changes to functionality and code refactors, etc. labels Feb 1, 2021
@sagetherage
Copy link
Contributor

Since @s0undt3ch requested changed I will ask for his re-review and I have updated this with master so it is running tests again, then likely it can be merged, these labels will also help :)

@sagetherage
Copy link
Contributor

how many reviewers? well, if tests are passing and tests written, docs, and changelog then only 1 review is needed, but we do like review from those that have requested changes, and 3 approvals is mandatory for anything that doesn't have all of those - clearer? Maybe, I have labeled so Megan can review for merge.

@Ch3LL Ch3LL requested a review from garethgreenaway February 4, 2021 18:52
twangboy
twangboy previously approved these changes Feb 10, 2021
@Ch3LL Ch3LL dismissed stale reviews from twangboy, garethgreenaway, and xeacott via 13fa412 March 1, 2021 13:37
@Ch3LL Ch3LL merged commit 7bff39e into saltstack:master Mar 2, 2021
@max-arnold
Copy link
Contributor

Apparently, the indentation after the salt_fun keyword should be increased:

beacons:
  salt_monitor:
    - salt_fun:
      - slsutil.renderer:
          args:
            - salt://states/apache.sls
          kwargs:
            - default_renderer: jinja
      - test.ping
    - interval: 3600  # seconds

@jtraub91
Copy link
Contributor Author

@max-arnold yes good catch that was a typo in the docstring! 😳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Aluminium Release Post Mg and Pre Si Feature new functionality including changes to functionality and code refactors, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: New beacon "monitor"
9 participants