SEP 25: Add runtime registers to salt

[Timbus]

No description provided.

[waynew]

My first impression of this was that this would require a complete change to how we're processing states, because right now we render the templates and then process the states.

But I see in your PR that your proposed uses different markers so it shouldn't do this. I'm pretty sure that this will require pretty significant changes to some of the ordering code, though you might be able to piggyback on the existing requisite system, since this is definitely adding another kind of requisite.

I think that being able to get the results of one state in another would be pretty useful - though now that I'm thinking about it what about simply calling {{ __salt__['some.module']('some', 'arguments') }}, in combination with a requisite? Obviously that's executing modules twice which might be some extra overhead - but I would like to see how this approach compares to doing something like that. You already call out how some of these other pieces of Salt are maybe lackluster, so I think it's worthwhile actually doing a comparison and saying:

Here's how to workaround this current limitation using slots. But this approach is better because you can write X instead.

Basically... if this approach is better or more robust than what we currently have, why? Can we show an example where it's clearly better because it's easier to write, faster, etc.?

I'm not against this at all - but I think that it would be valuable for people to see how they can improve their Salt states by using this proposed feature.

[Timbus]

I'm pretty sure that this will require pretty significant changes to some of the ordering code, though you might be able to piggyback on the existing requisite system, since this is definitely adding another kind of requisite.

I proposed that these changes don't do that, do you think they need to? I can think of some scenarios where registers alter ordering -- decoupling states from each other maybe? But that's not really an issue I see in need of solving.
This is more just adding variables to the current system.

I think that being able to get the results of one state in another would be pretty useful - though now that I'm thinking about it what about simply calling {{ __salt__['some.module']('some', 'arguments') }}, in combination with a requisite?

Does that cause the .sls file to be reevalueted midway through a highstate? I didn't think that was the case. What if you need the result of a previous state, such as needing to fetch a filename from a zip you extracted? That's possible using your approach? If so then yeah this is just a frustration born from a lack of clear documentation.
Can your approach also work with thorium? I've been wondering how to react to an event in thorium and the only thing missing is that I can't interpolate the register containing the minion, so I can't send commands to a dynamic target. If your trick causes .sls to be rerendered then I may have a solution

Basically... if this approach is better or more robust than what we currently have, why? Can we show an example where it's clearly better because it's easier to write, faster, etc.?

I'm not against this at all - but I think that it would be valuable for people to see how they can improve their Salt states by using this proposed feature.

Fair enough, this is good feedback. I'll add more examples to the proposal.

[cassandrafaris]

Hi @Timbus! I'd like to add this SEP to the Open Hour for this Thursday or next Thursday. Are you available to attend at 11am MT/5pm UTC on either day? You could present what's in the SEP and lead a discussion about it. Let me know if you're available.

[Timbus]

I would love to, but:

:(

Sorry I haven't given this proposal another once-over yet, been keeping myself a little too busy. Will try to get to it since I don't think I can present it in-person -- which would have been nice.

[Oloremo]

Hi,

We're doing Ansible to Salt migration and the lack of any runtime variables logic was a huge problem for us.
We mitigated that by writing a lot of helper states but sometimes it feels like we have to write a 100 python module just to do a thing like "look here and use output here" as we would do with 2-3 tasks in Ansible.

We looked at the slots but slots are a bit ugly and also they don't cover the "save" part, like when you save or register output to a variable and re-use it many times after.

Runtime requisites help in for conditions but they also very naive right now although it will be improved in the next Salt release.

The bottom line is:
I think it's a great idea but it has to be implemented as a core Salt feature, not as an additional limited way to do stuff like Salt already has. For example, I think it should extend OR replace slots. Salt already very confusing and complex, not need to make it worse.

[Timbus]

Just thought I'd add another datapoint:

Yesterday I tried to disable some scheduled tasks that an installer enabled, as a post-install cleanup.
I tried to use jinja to fetch the list of installed tasks with task.list_tasks, figure out if any are still 'enabled', then run the post install step based on this. Turns out it's not possible on a fresh machine, as the jinja is evaluated before the installer runs.
Slots can't work since task.list_tasks returns a list, and I need to apply a matcher to each item.

In the end I used powershell with only_if (could also use stateful). In other words; the win_tasks module was not usable, despite having the exact functionality I need.

[whytewolf]

So, I love this concept.

One item I might consider
I think it might be beneficial to handle the formating on the output to registers instead of only from input. This would have a couple of effects. easier memory management, since you are only saving what you use, not the whole state output. Only having to translate once instead of multiple times, which means less time parsing.

The minor change to the setup would be to break register into a list of dicts something like

Get a list of binary patches from something like a database:
  my_api.call
    - name: get_patches
    - register:
      - bin_list: {| $output$ |list |}
      - first_item: {| bin_list | first |}

So in essence the code would in order

• translate the normal jinja
• import that yaml
• run the state
  • split the register functions off of the state with translation info intact.
  • run the translation info on anything that remains
  • run the state
  • update the $output$ register which should just be a register for the last run states output
  • loop through the register list to add new registers by running the translation on each formatting string and saving the results into the labeled register.

[Timbus]

I proposed using 'nop' states to achieve that same effect because I felt it was clearer, though a little less terse.
That said it seems like something people would use fairly often so, perhaps its worth adding. Maybe something more like:

    - register: output
    - posthook:
      - bin_list: {| output | list |}
      - first_item: {| bin_list | first |}

I propose this because it avoids magic variables, and can have a much more easily defined set of behaviors, especially when combined with names (ie: make 'output' a list, run posthook once after loops are done)

[sagetherage]

@Timbus still looking at this SEP and I was under the impression with it being "Chapter 1" that there might be more additions, but I may be assuming, there and do not mean to do so! We like where this going, and do not mean to rush it! I am clarifying since it is likely my blunder.

[root360-AndreasUlm]

I like this proposal as we have a use-case that requires such a 'global' information storage.
Our use case is that we deploy source code into a randomized directory and active it by switching a symlink.
The implementation requires splitting the symlink switch and randomized directory creation into two states to give control over the symlink-switching time.

The current implementation looks like this:

application.sls
{%- set curtime = None | strftime("%s") %}
{%- set path = '/srv/foo-' + curtime %}
create_dir:
  file.directory:
    - name: {{ path }}
[...]

activate.sls
{%- set path = salt['cmd.run']('ls -dt "/srv/foo-"* | head -n 1') %}
switch_symlink:
  file.symlink:
    - name: /var/www
    - target: {{ path }}

With a feature like the registers described in this proposal the cmd.run could be removed by writing the path into the register.

[sagetherage]

This was discussed in the Open Hour on 2020-SEP-17 you can see all the details here: https://github.com/saltstack/community/wiki/Open-Hour-Agendas-and-Notes:-2020-09#salt-enhancement-proposal-sep-25-add-runtime-registers-to-salt and quoted here for clarity:

Salt Enhancement Proposal (SEP) 25: Add runtime registers to Salt

Community Manager Cassandra Faris
Discussion begins at 7:15 on the video

• GitHub conversation about the SEP is being monitored.
• There is some question as to whether this is a SEP or something else. This is TBD after further conversations.
• SEP submitter is in a time zone that doesn't work with community events, driving much of the conversation to the SEP itself.

[Timbus]

Sorry for being a bit lax here, it was a mix of not knowing if any progress was to be made, and being quite busy at my work.

@Timbus still looking at this SEP and I was under the impression with it being "Chapter 1" that there might be more additions, but I may be assuming, there and do not mean to do so!

To be honest, I was going to collect feedback and update the document while looking into the technical details + possible complications of any 'whatif' scenarios that popped up, but after my first POC and draft it all just seemed.. quite easy, thanks to jinja being able to handle most of the leg work. Were you, yourself, hoping for more functionality?

Perhaps what is being lost is that I wanted this to be a global fix, hence it's a SEP. Not just state runs.. Orchestrations are quite lacking without vars imo, and thorium doesn't really know how to make use of its registers, which really kneecaps it.. But yeah I'm not changing how salt works in any fundamental way

[sagetherage]

@Timbus that is ok! I wanted to make sure I wasn't making assumptions and I will get this assigned to someone to get more comments from the greater community.

[Elufimov]

Any news about this feature? By now this is only one feature that ansible is winning in comparison.

[sagetherage]

@Elufimov I have this on the Core Team's Friday meeting agenda to discuss and continue with next steps and more details will be coming soon.

[Elufimov]

Any news? Maybe all actions take place somewhere else?

[Dr-Bone]

Some nice person in irc pointed out this topic to me. I'm currently at a point where reading output from an orchestration really would make things easier. Looking forward to seeing this topic get some movement again.

[waynew]

Apologies for the long hiatus, we're working to refocus on getting SEPs through the process.

---

To summarize the current status:

There has been a lot of discussion around the idea of runtime registers and that it could be cool. @whytewolf in particular is a fan, but nobody else on the core team has weighed in, and at least at one point there was a question if this was even proposing a significant change or something else.

We probably need to take the time to grok this proposal.

[waynew]

This SEP has reached final comment period

[waynew]

The current sentiment is that we like the idea of this SEP but we have concerns about the details. Because of the magnitude of this kind of change, we really want a more comprehensive plan and maybe some PoC implementation to go with it. That would give us a lot more concrete ideas of how it's going to behave and interact with Salt.

Unless something changes in the next ~week, we'll close this SEP as Abandoned (or Withdrawn) for now. One approach that could be a good idea is to form a working group to put together a more detailed SEP and PoC for this.

[jtraub91]

I really like this idea and think I have a relatively easy way to achieve it with salt. Proof of concept to come. Stay tuned.

[jtraub91]

So, for this proof of concept I did not concern myself with the jinja rendering, which will be essential, but it seems easy enough to add that in later so I didn't worry about it for now. The key concept I will demonstrate is that salt already allows data to persist for subsequent id blocks in a state thru the use of the __context__ variable (doc)

The following is not necessarily intended to be a useful example, nor comprehensive, but illustrates the use of the concept of "registering" a variable for use in a subsequent state id block. The code used is located at my public repo here.

See the following create_user.sls

create_a_user:
  user_demo.present:
    - name: {{ pillar['user'] }}
    - password: {{ pillar['password'] }}
    - register: new_user_uid

manage_contents_latest_salt_user_uid:
  file_demo.managed:
    - name: /root/last_user_created_by_salt_uid
    - contents: new_user_uid
    - onchanges:
      - create_a_user

For demonstration purposes, I have essentially wrapped user.present and file.managed with user_demo.present and file_demo.managed, respectively. For user_demo.present I have added the kwarg register for the name of the variable to "register" the relevant return data. Subsequently in file_demo.managed I set contents equal to the variable I "registered". In the file_demo custom state module I examine if this variable appears in __context__, and if it does, use the value, otherwise use the value of contents. See the following for the resulting usage and verification.

root@salt-exp:~/sep-25-demo# salt \* state.sls create_user pillar="{'user': 'satchmo', 'password': 'bigfoot'}"
salt-exp:
----------
          ID: create_a_user
    Function: user_demo.present
        Name: satchmo
      Result: True
     Comment: New user satchmo created
     Started: 00:34:46.676877
    Duration: 62.664 ms
     Changes:
              ----------
              fullname:
              gid:
                  1003
              groups:
                  - satchmo
              home:
                  /home/satchmo
              homephone:
              name:
                  satchmo
              other:
              passwd:
                  x
              password:
                  XXX-REDACTED-XXX
              roomnumber:
              shell:
                  /bin/sh
              uid:
                  1003
              workphone:
----------
          ID: manage_contents_latest_salt_user_uid
    Function: file_demo.managed
        Name: /root/last_user_created_by_salt_uid
      Result: True
     Comment: File /root/last_user_created_by_salt_uid updated
     Started: 00:34:46.741616
    Duration: 97.619 ms
     Changes:
              ----------
              diff:
                  New file

Summary for salt-exp
------------
Succeeded: 2 (changed=2)
Failed:    0
------------
Total states run:     2
Total run time: 160.283 ms
root@salt-exp:~/sep-25-demo# cat /root/last_user_created_by_salt_uid
1003
root@salt-exp:~/sep-25-demo# salt \* state.sls create_user pillar="{'user': 'satchmo', 'password': 'bigfoot'}"
salt-exp:
----------
          ID: create_a_user
    Function: user_demo.present
        Name: satchmo
      Result: True
     Comment: User satchmo is present and up to date
     Started: 00:35:18.911971
    Duration: 40.814 ms
     Changes:
----------
          ID: manage_contents_latest_salt_user_uid
    Function: file_demo.managed
        Name: /root/last_user_created_by_salt_uid
      Result: True
     Comment: State was not run because none of the onchanges reqs changed
     Started: 00:35:18.954085
    Duration: 0.008 ms
     Changes:

Summary for salt-exp
------------
Succeeded: 2
Failed:    0
------------
Total states run:     2
Total run time:  40.822 ms
root@salt-exp:~/sep-25-demo# cat /root/last_user_created_by_salt_uid
1003
root@salt-exp:~/sep-25-demo# salt \* state.sls create_user pillar="{'user': 'lochness', 'password': 'monster'}"
salt-exp:
----------
          ID: create_a_user
    Function: user_demo.present
        Name: lochness
      Result: True
     Comment: New user lochness created
     Started: 00:35:38.463871
    Duration: 72.908 ms
     Changes:
              ----------
              fullname:
              gid:
                  1004
              groups:
                  - lochness
              home:
                  /home/lochness
              homephone:
              name:
                  lochness
              other:
              passwd:
                  x
              password:
                  XXX-REDACTED-XXX
              roomnumber:
              shell:
                  /bin/sh
              uid:
                  1004
              workphone:
----------
          ID: manage_contents_latest_salt_user_uid
    Function: file_demo.managed
        Name: /root/last_user_created_by_salt_uid
      Result: True
     Comment: File /root/last_user_created_by_salt_uid updated
     Started: 00:35:38.539235
    Duration: 95.048 ms
     Changes:
              ----------
              diff:
                  ---
                  +++
                  @@ -1 +1 @@
                  -1003
                  +1004

Summary for salt-exp
------------
Succeeded: 2 (changed=2)
Failed:    0
------------
Total states run:     2
Total run time: 167.956 ms
root@salt-exp:~/sep-25-demo# salt \* state.sls create_user pillar="{'user': 'lochness', 'password': 'monster'}"
salt-exp:
----------
          ID: create_a_user
    Function: user_demo.present
        Name: lochness
      Result: True
     Comment: User lochness is present and up to date
     Started: 00:35:44.238437
    Duration: 25.039 ms
     Changes:
----------
          ID: manage_contents_latest_salt_user_uid
    Function: file_demo.managed
        Name: /root/last_user_created_by_salt_uid
      Result: True
     Comment: State was not run because none of the onchanges reqs changed
     Started: 00:35:44.264348
    Duration: 0.005 ms
     Changes:

Summary for salt-exp
------------
Succeeded: 2
Failed:    0
------------
Total states run:     2
Total run time:  25.044 ms
root@salt-exp:~/sep-25-demo# cat /root/last_user_created_by_salt_uid
1004

Again, this was for demo purposes as there may be other ways to achieve this with salt, however, since the precise user id a new user will be given is unknown until it occurs, I have demonstrated salt's ability to register this return to be used in a subsequent state. In practice, I imagine a new dunder added, e.g. __register__ so that it doesn't conflict with other salt internals. Furthermore, jinja-like syntax, as noted in this SEP, will be of great value so that many more possibilities are opened by this feature. You may notice that the lack of jinja forced my demo example to explicitly write the data I needed (uid) to the __context__. In reality, I would imagine the entire return, from the state id block in question, added to the register so that the user can grab whatever data it needs later, with jinja. Furthermore, my crude example was hardcoded to check the register for only the contents variable, only for expedience of this demo, but obviously this logic should be performed for all variables and perhaps even requisites, in practice.

To conclude, I have demonstrated, by purely altering the state modules themselves, we can achieve salt registers, with no fundamental changes. I do, however, realize that this task might be quite extensive, i.e. to retrofit all states, requisites, etc. Therefore, I defer to the salt core team to comment on whether this logic can be applied to the state runtime code itself. If it can, this would alleviate the task of applying this logic to all states, since altering the runtime would make it de-facto applied to all states, much like requisites.

I'm eager to hear all of your thoughts. Cheers! 🍺

[rmsc]

It's been over a year, any feedback from the core salt team?

Coming from Ansible, orchestrations feel kind of useless without the this...