Salt: [FEATURE REQUEST] Support checking/modifying the return of state.module.run

Created on 3 Aug 2020  路  15Comments  路  Source: saltstack/salt

Problem
Today I was struggling to use the win_service.modify command from state.module.run in a way that would allow for some result-based followup actions. I ended up writing a custom state but I feel like it should have been possible without needing to use python.

Solution
If state.module.run offered an interface to allow for checking return values and using them to signal success/change/failure, I could have called win_service.info and checked the values, then chained the state using onchange.
Strangely enough, loop.until_no_eval seems to offer something like this with the compare operator? But it's hardly fit for purpose.

Alternatives + SEP thoughts
Guess I'll also mention that exploring through other options like slots (yuck), state.cmd+stateful, and most of the requisites, seems to show there is a need for runtime variable-passing and logical operators (yes like registers from ansible), and having such a system would probably simplify a lot of states? Even being able to use thorium + custom events in my state mostly does the job. Dunno, just musing at this point.

Feature Pending Discussion

Most helpful comment

Awesome, I have made a start on this, will need to sleep on it before I finish it tomorrow. It's been a while since I've done any creative writing. Thanks for your guidance so far

All 15 comments

@Timbus Thanks for the report. Can you elaborate a bit more on what exactly you're looking to do? Do you want to capture the results from state.module.run and run something based on that? Is this within the same state run or multiple state runs?

Well, this is a bit of an 'XY' problem I guess.
For my very specific use case, problem 'X', I wanted the ability to manipulate/check the return value from module.run and use that value to possibly trigger an onchange for the next state. In short, it would allow building custom states without needing to write python.
I'm not a big fan of the design of the new module.run as it's not very extensible, but I used it as a guide to come up with something along the lines of:

      some_id_of_state:
        module.run_with_check:
          - service.info:
            - name: salt-minion
          - tests:
            # Return whatever matches first. If none match return Changed, like module.run would
            - unchanged:
                compare_operator: data.subdict_match  # stolen from the 'loop' module
                expected: 'ServiceAccount:SaltyAdmin'
            - changed:
                compare_operator: data.subdict_match
                expected: 'ServiceAccount:LocalAdmin'
            - error:
                compare_operator: data.subdict_match
                expected: 'Startup:None'

Though it might be easier to just make a state.function module for this lower-level stuff.

But the general purpose summary of the bigger problem 'Y' is: States have no runtime variables! If you could just capture, manipulate, and test values, it would probably make a lot of states easier to build. Even existing, working concepts like onlyif, and slots, become easier to understand if you model it as passing a result variable from a required state to the requirer.
If we had vars/registers and simple operators then the above enhancement to state.module probably wouldn't be needed, solving the issue more completely.

This looks like a duplicate of #38533

This looks like a duplicate of #38533

Thanks for the link, and yes this is basically another take on that.
And though your proposal in that thread is nice (and better than my initial idea), I would argue that simply being able to capture module/state results in runtime registers would allow you to use ordinary state modules to perform the evaluation, eventing, etc that you want based on the content of a register, which seems quite a bit more flexible and extensible.

I'm not sure how difficult such a feature would be to implement though, compared to the admittedly far more limited idea of implementing a better module.run

@Timbus Thanks for the explanation. What you're describing was the goal behind adding slots, is something about this feature that doesn't accomplish your goals?

I read https://docs.saltstack.com/en/master/topics/slots/index.html 5 times and I still have no idea what is being attempted there.

slots appear to be something to do with running code before a state. This issue is about running some code depending on the output of a state.

@garethgreenaway if you are involved in writing the slot code, please consider adding a real world example of how it might be used, without using "somefile". I can't understand how the slot idea could be useful.

Oh! After reading it again, it looks like slots do work with dictionaries, it's just a little hard to see from the example (.dictionary doesn't look like it's selecting a key..)
Well, with some minor re-jigging I can make that work I think.

But it's still lacking in many ways.. Like how do I pass that value to multiple states without rerunning the function, how can I chain the output to another function, and how do I use it (intuitively) to signal changes for other dependents? It just seems a bit anemic (and a little.. strange-looking?) compared to a simple register system. Just an opinion.

Considering this for Magnesium since there is a PR mentioning it (not closing it, but related) and needed to reassign due to workload. Thanks!

Well, concerning workload.. I'm more than willing to provide a PR that covers my original proposal, or I could attempt a bigger change that works similar to the thorium PR I linked using jinja.
It's just mostly the lack of consensus or direction that's holding me back; I don't want to bolt yet another over-specialized feature into salt.. I'd prefer something more fundamental/general purpose that fits the existing toolkit, is all.

@Timbus sure that makes sense - if needed we can also have a wider discussion on an implementation. For now I have assigned @DmitryKuzmenko to at least review the PR you have opened and linked here.

@Timbus I added the label Pending Discussion to see if I can get an architectural review. Still working what that looks like or when, but attempting to make progress. I will follow up by the end of the week on this ticket.

@Timbus apologize I didn't get to this on Friday. The team reviewed and would like to open this issue up as a Salt Enhancement Proposal. You can find the instructions in the readme.md here: https://github.com/saltstack/salt-enhancement-proposals. This is a great way to get more communication from the community as well as the Core Team and we do not need to wait for a meeting for this to occur! Let us know if you have questions @cassandrafaris and @waynew have helped shepherd SEPs along and will gladly help here, as well. I look forward to seeing your proposal! :)

Awesome, I have made a start on this, will need to sleep on it before I finish it tomorrow. It's been a while since I've done any creative writing. Thanks for your guidance so far

@Timbus Hi! I'm adding this topic to the agenda for Thursday's Salt Community Open Hour. I'll tell everyone to watch for the upcoming SEP. Perhaps it'll spark some realtime conversations. If you're free, join us!

Was this page helpful?
0 / 5 - 0 ratings