SEP 16: Delayed rendering

[nixjdm]

Link to rendered markdown: https://github.com/terminal-labs/salt-enhancement-proposals/blob/delayed-rendering/0016-delayed-state-rendering.md

This SEP hopes to outline how we can add a new feature to Salt to allow for more powerful state renders, by allowing delayed renders of Jinja after execution of previous states. It has broad uses, but is perhaps a feature that would be most frequently looked for when doing cloud related things. In general, you’d want this when a state has an unpredictable effect that needs to inform another state.

I’ve been working on this and talking about it internally with Terminal Labs for about a year. Over the summer I started to flesh out the details in earnest. @verhulstm informed me that @thatch45 expressed a similar / related desire to the Salt-Cloud working group recently. I don’t know his thoughts in detail about this. I hope they align with this SEP, or at least that this SEP acts as a starting point to create a well fleshed out implementation.

We at Terminal Labs have wanted something like this since I thought of it. We think this will be of great utility!

Thanks for reading!

[dstensnes]

Could an idea be to implement handling of an init.sls.d directory inside the state? Each file inside is rendered and executed in alphabetical order, acting together as a single init.sls file?

This would work similarly to how e.g. /etc/grub.d or /etc/profile.d or pretty much any other folder ending in .d inside /etc

[thatch45]

While I think that this problem needs to be solved I am worried about the complexity of this proposal, and I am worried about how to actually implement it without a rewrite of the state system.

My comments to the cloud group last Friday were in reference to the Idem project, that project makes it much easier to re-render state files during runtime because the runtime is fundamentally different. In this case I think it would be a lot simpler to just re-render state files right before execution so that the on-the-fly rendering would be transparent to the user.

In a nutshell, requisites seem too hard for a lot of people to grasp as is, adding render interactions into requisites strikes me as just too complicated. It also means that we would need to push this to other renderers beyond Jinja.

I also don't know if this solves the issue of taking data from one state run and applying it to another. Granted I do think these are difficult issues overall that do need to be figured out.

[waynew]

@nixjdm this will be SEP 16 🚀

[nixjdm]

@thatch45 I see. Thanks for the clarification about the meeting. Can you elaborate on how Idem would solve the same problem? I had heard of Idem, but I did not understand it to be related to this problem. I’ve read the couple documentation pages in that repo, but I haven’t managed to piece that together in my head. Rendering state files just before execution is one of the two main options in this SEP (the other being blocks). I don’t see what you have in mind with Idem yet. I’d like to understand :)

Currently, a user might use orchestration every time something like this comes up, possibly having a few different manually run, sequential orchestrations, before getting to be able to run a useful highstate. Adding a requisite to get rid of an orchestration run seems like a fair trade to me. Users can also ignore this feature if they don’t want to use it or have no need of it.

This feature should help a lot in passing data between states by virtue of Jinja having access to all of the Salt context vars like salt, and the direct return value of the last state. Even if one has to resort to something like {% set delayed_var = salt[‘cmd.run’](...) %}, the data is there.

[nixjdm]

@dstensnes, I think you could do something like that to reference a whole dir. I’m not sure how that would look best at the moment.

Referencing whole state files is something you can do in other requisites. The blocks in this proposal isn’t relevant to other requisites. Your idea seems like it could generally work with other requisites too, if you wanted. In practice though I think you don’t need many of these delayed_renders. My intuition is that very few uses would be better off with a .d/ dir than just specifying the few things you’d need directly.

[thatch45]

I have been chewing on this a lot in the last few days since you posted this. The support for blocks in here really worries me, primarily from an implementation basis, but also from a complication perspective. Also questions about how to make this work beyond just Jinja. I think that a good start would be to tackle late loading of the whole file first. That can be implemented outside of the render system, I was thinking to add it to the shebang line, maybe a line that looks like this:

#!jinja|yaml:require:ID_DEC

The more I look at it I think that this could be doable inside of Salt.

As for Idem, this project is crazy young, I would like to imagine that it to be able to eventually superseded the Salt state system, but it is FAR to early to see.

Idem is built entirely on POP and makes the system fully pluggable, so you can, for instance, add a simple plugin to add a new requisite type like require or on changes. Idem also evaluates the runtime differently. The Salt State system is run via a multi-branching recursive algorithm. Idem evaluates the runtime with each stage and makes how the executions get run, pluggable. This means that it is MUCH easier to augment the runtime dataset during runtime, while the salt state system potentially needs to be restarted mid-run if we want to add new states to it.

[thatch45]

on the .d dir I am hesitant, I think that it makes sense to keep to the existing constructs. I would also like to avoid adding syntax to make things work better. The new features should be naturally intuitive to people who already use Salt.

[nixjdm]

Thank you for elaborating on Idem, that seems a bit clearer to me. I’m certainly curious to see how it develops.

I agree that of the two ways outlined in this SEP, the delayed rendering of a file I think would be simpler to implement than delayed render of a block. I think the block has a bit more utility though, with the ability to pass the template context in with scoped. The block method also lets you keep closely related sls code next to each other, which has a soft value.

I also agree that implementing the delayed whole file would be the best place to start, and could even be released well ahead of the block method, on its own.

Adding a shebang to the top of a delayed file I think would do the trick. An alternative, which I suppose I was tacitly envisioning, would be to not list the delayed file in the top.sls. That way the file is never told by the state system to render it before it’s ready, but only on the callback, after the callback says “look for and use this file.” This would be similar to how orchestration files can run other sls files with

myorch:
  salt.state:
    - sls: mystate

but without the need for a tgt, or rather, implicitly targeting the specific minion that made the callback.

This does make me want to reconsider something similar to scoped for a delayed file. It wasn’t obvious to me how best to do that, but it should be doable. That’s the only functional difference to the user between the delayed file and delayed block, as written. However that looks though, it could also be a later feature. A lot of this could be piecemeal implemented.

[thatch45]

This also makes me wonder if we could syntactically handle the code blocks in a way that would be outside of the templating language. Again, just a thought, but have a code block delimited that would take the raw sls data and cut it up before sending it to the renderer so that it could be delayed. Maybe like this:

#!jinja|yaml:require:ID_DEC
something:
  file.managed:
    - source: salt://foobar
#!require:OTHER_ID_DEC
{{salt.cmd.run(ls /foo)}}:
  service.running
#!CLOSE

The idea here just being that the chunks of files would be carved up.

But from an implementation perspective we would:

1. Make the state system able to extend the lowdata during an execution
2. Create a file-level delayed render with some kind of file level syntax
3. Using the same/similar syntax allow for more file chunks to be cut up

As for downloading the sls, it would need to be included at some point in another sls file so that the resolver could get it. We would also need to account for include statements inside of delayed render files and blocks...

[thatch45]

Woah, that last thing I wrote also makes me wonder, I might be off base here though. Should this be part of the include statement?

include:
  foo: 
    require: bar

(I would need to double check all of the current include capabilities)
So that the file is included but render is delayed until the requisite is satisfied?

[nixjdm]

I think I like the idea of making the blocks template agnostic. That seems like a pretty good improvement on the SEP. My inspiration was in part traditional Jinja blocks - but that doesn't have to stay, I think. This makes implementing blocks quite a bit simpler. Using something like

#!require:blockname
...
#!CLOSE

is a fine replacement for a custom Jinja template tag. The start tag would still need to accept certain arguments though - but that's fine. It's a new shebang, so we could parse it however we like. It could be

#!require:blockname|repeat_limit=3

or

#!require:blockname repeat_limit=3 scoped

for instance.

I don't think a file-level header is required, right? The callback I would think would be sufficient to indicate that the file should be looked for and rendered, and when to render it. So then a file that's entirely delayed wouldn't have to have any new shebangs / template tags in it.

# baz.sls
include:
  foo: 
    require: bar

The purpose of this - to check that I'm following - is to tell Salt where to look for and attach the bar.sls, and mark it to be delay-rendered, yes? And you would still need the state-specific callback to tell Salt when to render? If I have that right, this might make implementation easier, but it isn't strictly required since both of those instructions (attach file & when to render) could be gathered from having just the delayed_render requisite. Both the requisite and the include are potentially understandable when the yaml is parsed, which is to say, at basically the same time. I'm fine keeping that if it helps though.

Thanks for the great feedback!

[nixjdm]

Small tweak. I think I like #!delayed:blockname repeat_limit=3 scoped better than #!require:blockname repeat_limit=3 scoped, because the blocks would only be used if there is a callback for them. If there isn't, they'd be cut out and effectively ignored.

[thatch45]

Ok, did you change up the SEP to reflect these changes? Also that there are implementation phases

[nixjdm]

Not yet, but I will. I'll update it within a few days, as soon as I can. I'll also write about a carving up implementation into separately chunks.

I'll comment here again when I have that finished.

[nixjdm]

I have updated the SEP. It now uses hashbangs instead of template tags, so that no new template tags are needed. It also makes a note about implementation being able to come in pieces, changes the scoping rules of delayed_sls to be the same as delayed_block (previously delayed_sls was not identical in that it couldn't be passed scoped), and some minor language tweaks.

The diff showing all of the changes in detail can be read here.

[thatch45]

I like this setup a lot more, but I am still worried about how to make it a reality. I also worry about the block assignments and requisites. In a nutshell, I like this, but it is a massive lift, and I still worry that it is a complicated interface to learn. It might make more sense to break this SEP into multiple smaller, progressive SEPs.

[nixjdm]

I can break this SEP up into smaller ones if you like. I see how that could make understanding them easier, which might help implementation and docs writing, and thus learning for users. I’m ok with that. It might take me a bit of time to rewrite them though.

If I maximally break this up, it would be into the following series:

1. The fundamental pieces: the requisite, together with either the delayed sls file or block. The file is simpler, so let’s pick that.
2. The delayed_block: Requires adding a pre-render parsing to look for the hashbang block tags and storing the contents for later evaluation. The later evaluation should be the same as the delayed_sls file.
3. Adding the prev_ret context var into delayed renders
4. The delayed_render_limit:
   4.a. Tracking the number of times a file/block is delay rendered and add a default delayed_render_limit of 1. Note that if this was previously un-tracked, and then changed to tracked and enforced, that would be a breaking change. If that’s done, a deprecation cycle should be used. Avoiding that might make it worth coupling the implementation of this with the implementation of the first SEP - though they can still be separate SEPs.
   4.b. Allow setting the delayed_render_limit via hashbangs (block and file)
   4.c. Allow setting the default delayed_render_limit via a master config
5. Allow scoped delayed renders

I see 3-5 as handy smaller options for the user, built on the assumption of the primary feature. How about I make three SEPs, for 1, 2, and 3-5? On their own I’m not sure 3-5 are really SEP worthy, but together they represent fair amount of value and change. Or, I could make each number in the list a separate SEP. 4 can also be broken up, but it should really stay as one I think.

@thatch45 or @waynew Would you prefer I leave this SEP to be merged "Rejected", and start a new one, or re-draft this one to be the first in the series?

[thatch45]

I like these steps, it likely lines up well with getting it done. @waynew do we have a way to merge this for historic purposes as "Great stuff but we opted to break it up"?

[nixjdm]

@thatch45 Sounds good to me. Are you fine with the 3 SEPs breakdown, or prefer the full 5?

[thatch45]

I think 3 should be enough

[waynew]

I suppose the most accurate state would be "Withdrawn", since the OP is pulling this in favor of splitting into 3 different SEPs. I'll go ahead and close this PR, add the label, and update the list.