Salt: Salt Targeting Terminology

[grichmond-salt]

When we specify targeting types in Salt states, we have three keywords that we can use. They are:

• match (top.sls file)
• expr_form (orchestration/reactor states)
• tgt_type (orchestration/reactor states)

This has raised questions during training classes. In discussing this with Seth, he stated that submitting an issue to eventually use a single term for matching would be valuable. Seth suggested that “expr_form” would be the most canonical.

Thanks!

[cachedout]

Is this strictly a documentation issue or are you recommending functional changes as well?

[grichmond-salt]

Following Seth's preference, my thought would be to make sure “expr_form” works in all places, and then change our online documentation and training materials.

[lorengordon]

From a semantics perspective, expr_form is just noise. Seems unlikely to me that most will be able to guess at its meaning when reading a state file, so it will only make sense once the user is already a salt expert. match is much more clear and easier to remember.

[cachedout]

I sort of disagree, to be honest. To me, match invokes a boolean operation, while expr_form expressly indicates that it's a type of matching to be done. Anyhow, it's purely subjective of course and I'd be fine with either. :]

[lorengordon]

My hang up is that expr_form, as a word, has no inherent meaning. The lack of meaning makes it hard to read the states and understand them (without a fairly decent amount of prior experience). At the very least, you need to guess what did this person mean by expr? And then you have to wonder, if that's shorthand for something, is form also shorthand? I think for developers, perhaps it makes some sense, if they twizzle around in the code for a while. But that's only good if developers are the only, or at least the primary, target user base.

I admit to having stumbled into a somewhat recently acquired pet peeve of mine. Feel free to ignore me if the discussion is not valuable. :)

[whiteinge]

@lorengordon Agreed that expr_form isn't self-describing at first glance. (I always assumed it was short for "expression form", e.g. what kind of expression should the tgt string be evaluated as. But I don't know that for sure.) My recommendation above was because that name is the one most commonly used throughout the codebase.

It sounds like the core team has decided to standardize on tgt_type and put expr_form on a deprecation path starting with the Nitrogen release. I'm not sure if top files are included in that plan, though they should be as well IMO. I don't know if there's an issue to track that work yet -- it might be this issue.

[whiteinge]

Oh, this is already a done deal: #37963.

@terminalmage (Thanks for the link. Moving discussion here.) Any discussion yet on whether top files should also use this same name instead of match?

[terminalmage]

I'm not familiar with match, to be honest. In what context would it be used?

[terminalmage]

Oh wait, you mean:

base:
  'os:Linux':
    - foo
    - match: grain

Right?

[whiteinge]

Yeah

[terminalmage]

I don't know about deprecating match, a LOT more people will be using it than will be using the LocalClient.

If we do this, we'd need to document the heck out of the change and probably extend the "grace period" where people can continue to use match for an extra release cycle (i.e. Neon rather than Fluorine).

[whiteinge]

Def agree. Per my comment in the other thread, I feel similarly about this deprecation in LocalClient. More people will be using top files than are using LocalClient, of course, but many, many people do use LocalClient and of those many have build scripts and applications on top of it which is a much harder, slower, more expensive thing to update.

I'm in favor of grabbing everything all at once, including top files, and extending the deprecation out by an extra release.

Sorry if this has been discussed already: was _not_ deprecating brought up at all? I.e., add compat aliases everywhere and then just remove expr_form from the docs but not actually remove it from the code? We might want to do that for top files regardless of what we do elsewhere.

[terminalmage]

When I spoke with @thatch45 he decided deprecation was the way to go for the LocalClient. It would take very little to cancel the deprecation and just support it as an alias, all we'd need to do is remove the deprecation notices (and update the related documentaton).

[whiteinge]

Deprecation is certainly ideal for future maintenance. I'm interested
to hear Tom's thoughts on match too.

[lorengordon]

In the context of top.sls:

base:
  'os:Linux':
    - foo
    - match: grain

match is a modifier for the grain 'os:Linux'. That seems clear. Match this grain. What semantic logic would make tgt_type similarly clear in this context?

[thatch45]

I am considering deprecation match in the top file and moving to just supporting compound targets in the top:

base:
  'G@os:Arch':
    - foo

[terminalmage]

That's a good point @lorengordon.

[lorengordon]

I can understand the appeal of deprecating match in favor of the flexibility of the compound targeting syntax. My concern would be that the compound syntax is a little wonky and finicky (things like spaces around the expression if using parentheses). People love languages like python and yaml because they are so easy to write in a way that is super easy to read. They barely feel like code sometimes. That ultimately comes down to natural language and semantics and visual structure.

[terminalmage]

Yeah, another good point. Any problem in the compound matcher becomes a single point of failure for the top file. Not ideal.

For what it's worth, I think that using match: grain is way more readable than just using G@os:Linux.

[whiteinge]

What semantic logic would make tgt_type similarly clear in this context?

I think this term is just as clear in the context of a top file as in other contexts. A top file is comprised of targets -- the docs use that same language.

IMO it's important to not think of a target as a minion but rather as an expression that is used to match minions. In other words: a target is synonymous with matching. In that context there's no semantic difference between matching minions via the salt CLI, via LocalClient, via a top file, or elsewhere.

[lorengordon]

@whiteinge, that is reasonable. 'os:Linux' is the target, grain is the target type. I think it requires a little more explanation than match, and a greater understanding of salt terminology and top file structure, but that does work. I would point out that you still fall back to the 'match' terminology in your own explanation of how it works... 😉

Perhaps it is that 'match' works as a verb in this context and 'target' is the subject. 'type' doesn't work as a verb here; it's a typically a noun. In this case, it is used to describe a property. In the top.sls structure, we're saying "match these targets of this type." With the match: grain syntax, match is the explicit instruction and type is the property being matched.

If we switch it to tgt_type: grain, we're trying to say the same thing, but "match" becomes implicit rather than explicit. We're probably more technically correct, but if we otherwise keep the same top.sls structure we are conveying less information (to humans).

Anyway, I'm just explaining how I think of it. I'll adapt no matter how it goes down. :)

[whiteinge]

I don't disagree with anything you said. Of the three, match is arguably the most self-descriptive term for how this is actually used in any context. My personal preference is match (though probably a non-starter to add to LocalClient at this point) -> expr_form (invokes much the same meaning as match but req's explanation) -> tgt_type (slightly misleading in some contexts).

All that said, my only horse in this race is using the same terminology for the same thing. 🏇

It's a useful indication to users that the behavior in the top file is identical to the behavior at the CLI, Reactor, Orchestrate, Mine, grains Runner, LocalClient, match Execution module, and everywhere else we use target matching.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.