SEP 22: Compartmentalize salt: Use pop-grains (grainsv2) for grain collection

[Akm0d]

Grains is currently a static component of salt. We want it to be developed in a more compartmentalize fashion.
The engineers at SaltStack have created platform-specific projects that functionally replace the grains provided by salt with POP. We propose that this project, codenamed grainsv2 becomes available in salt while maintaining backwards compatibility.

[s0undt3ch]

Which release is this?
We currently support 3.5, will Magnesium be >= 3.6?

[Akm0d]

It would have to be >=python3.6 to support pop code, this is something we should discuss with the team in greater detail. @dmurphy18 What is tying us to python 3.5?

[dmurphy18]

@Akm0d Debian 9, Ubuntu 16.04, SLES are the last supported platforms still using Py 3.5

[Akm0d]

@dmurphy18 I handled this in the code. Grainsv2 won't be included in python3.5 and lower builds. The __hub__ will also not be available on <=python3.5

[dwoz]

I'm definitely not a big fan of having another dunder added to salt. It seems like sprawl in lack of a properly architected design change.

[mchugh19]

Will the current methods of setting grains be supported in corn?

1. _grains directory
2. grains in minion config
3. grains in /etc/salt/grains & the grain modules which modify grains

[Akm0d]

Yes, custom grains will be able to be added to salt the same ways they always have.
You will also be able to add grains the new pop/idem way; such as pip installing a project like corn cowsay or idem-linux into the same python environment as salt.

[max-arnold]

A couple of additional compatibility questions:

1. What about other ways to add custom grains (other than _grains): salt-call --module-dirs DIR, grains_dirs setting, setuptools entry points?

2. Will a custom grain that accepts the optional grains argument get the core grains passed in? See this feature that was introduced in Salt Fluorine: Allow for passing in previously compiled grains to custom grain modules salt#47372

3. Is there a way to automatically compare both old and new style grains on each platform to see if their implementations produce the same result?

[max-arnold]

Also, is there a plan to sync corn grains with the recent changes in Salt core grains? For example, the FQDNs grain change and the new setting that disables it on some platforms saltstack/salt#55581

Maybe a core grain code freeze period could help?

[Akm0d]

Yes, a core grain code freeze is part of this SEP. We don't want to maintain two forks of grains and all future development of grains should go into the compartmentalized projects..

Other ways of adding grains will remain available as they are.

A custom grain that accepts the optional grains argument would get the grains generated by grainsv2.

Yes, the test in the Grainsv2 PR shows a line by line diff of grainsv2 vs salt grains. Running that test on that branch in my fork will show the diff you are asking for.

Grainsv2 hasn't had active development since February, recent changes will need to be brought in.

[Akm0d]

Opened a PR to start making the change: saltstack/salt#57681

[OrangeDog]

So could simply not having them all in a single file.

[waynew]

☝️

I would be curious to see a comparison between these approaches.

[Akm0d]

It's also about having grainsv2 be tested thoroughly on each platform, having a much faster cadence/release cycle for grains, having working group/community maintained projects that implement platform-specific grains/states/exec modules, speeding up the test suite, speeding up grains.... Breaking up core.py is just one piece of the puzzle

[OrangeDog]

Is there any code in salt core that depends on specific grains?

[OrangeDog]

This sounds like the biggest problem. Even though you say splitting them up has made them more consistent, it would be a lot easier to maintain if they were all in the same place.

How is this going to work for grains whose implementations differ not based on OS? For example based on whether particular services, package managers, or virtualisations are being used? There's a big matrix of how the same grain should get its data on different minions.

[Akm0d]

damon's solution below is perfect for this. Something like an idem-posix that contains all shared code will keep maintainability simple.

[OrangeDog]

I don't see how? This isn't about duplicating shared code, it's about making sure different code stays consistent.

And then how do you find which project is implementing your grain? Is it in idem-posix or overridden by idem-linux or by idem-debian or idem-dpkg or idem-ubuntu or idem-ec2 or idem-networkmanager etc?

It's far easier to get everything right if everything for a single grain is in a single location.

[waynew]

One of the things that we've also been seeing -- at least I have -- is something that we had in the past but definitely haven't been keeping up, and that's maturity.

Do we have any kind of formalized patterns around maturity for corn grains? I think this will become essential as we explore pop-style dependencies. Some users will be comfortable with rapidly changing and experimental grains/features, others will definitely only want to use mature/stable features that aren't rapidly changing.

[Akm0d]

Grains in grainsv2 aren't easily overwritten. We thought of this early on and each grain can only be set once. If idem-debian tries to overwrite a grain originally created by idem-linux an error will be thrown. It's certainly possible, but not easy, to overwrite a grain.

As for maturity, the new grains will be gated and disabled by default. Users that enjoy the stability of grains as they know and love them will still be able to have that. Grainsv2 will be able to gain maturity through those who choose to enable them. It's super easy to share custom grains with the whole world in grainsv2 with plugins. Maintaining platform-specific projects is easy.

I will also note that grains on platforms like BSD, solaris, and AIX, are fully tested in their idem-{platform} projects but have no running tests in salt.

[damon-atkins]

I have looked at the structure of corn.

• idem-aix
• idem-bsd
• idem-solaris
• idem-dawwin
• idem-linux

corn/grain fqdn is the same on all platforms, why write the code 5 times?

Why is there a idem-linux vs idem-rhel and idem-debian etc I assume to save having the same code in multiple places.

Why not a idem-posix This is well documented standard which Linux/Unix is based on, and then anything outside this standard then use idem-solaris, etc.

Why not have idem-py i.e. anything which can be completed in pure python e.g. fqdn. idem-py Would exclude the use of posix functions like chmod(), chown() as they are Unix based and hence not 100% multi-platform like most of python.

[OrangeDog]

@damon-atkins "pure python" describes non-core packages that are written only in python. It has nothing to do with whether it uses platform-specific functions.

[waynew]

This will be SEP 22, feel free to update your filename/SEP

[waynew]

I think the biggest question here: what scale are we looking for - how do we know if corn/pop-grains meets this required scale?

[waynew]

I definitely support this idea. I know libraries like flask support this kind of thing with the flask.ext namespace. I would like to see some type of discussion around this approach - maybe in the alternatives section? What tradeoffs are we making going with pop/corn vs the ext approach?

[dwoz]

I think we should at least explore if there are simpler, less complex solutions.

[waynew]

What is the risk of API drift? That is, can particular versions Salt and Corn become incompatible?

[Akm0d]

Salt will agressively pin a version of corn and all the implementations of corn. That said, with this new method, users could have a stable version of salt but a development version of windows grains.

[waynew]

I think we also need to determine how this will be gated. And what happens if this thing goes completely sideways? How can people roll back to the existing way?

[Akm0d]

I think that gating the use of new grains is an excellent compromise. It means OSes that still require python3.5 can get anadultered existing grains, but all new development could go into the gated code until python3.5 is deprecated

[Akm0d]

I'm going to add that to the SEP, we will keep core.py as it is; no changes and it won't be removed for a year. No PRs will be accepted into it and all development of grains will be shifted into pop-grains. new grains will be gated by a config option.

[dwoz]

My impression is that scaling issues currently revolve around the event bus and job storage, not the grains implementation. Can we elaborate on what we mean by scale here? How does the current implementation fail to scale? How does the proposed change make things more scalable?

[Akm0d]

@thatch45 ^^

[dwoz]

Would and alternate method be to simply split up core.py into multiple files?

[Akm0d]

Grains is the first focus of this SEP and the associated changes. It also opens the door for getting execution modules and state modules from idem in the future. For example, Mac specific execution modules could be put in idem-darwin, tested in idem-darwin, and PRs/issues can be handled by the Mac working group. Grainsv2 is the simplest and first step towards more compartmentalization of salt in this way.

[dwoz]

This is one of the thing I think that should be highlighted more in the SEP. We're saying this is opening the door to door to all of these changes, but we're also saying

What will further adoption of POP mean inside of salt?  The purpose of this sep and these features is not to provide a definitive answer to this question

Which is it?

[dwoz]

Are we sure that all of the changes that have taken place in recent releases are also present in the current pop version?

[Akm0d]

I have been focused on idem-cloud since March, there will need to be a code freeze of future PRs into core.py and changes in the past two months into core.py will need to be merged into the new platform.

[dwoz]

Which bugs specifically?

[Akm0d]

No specific open issues, but a lot of work has been done to standardize grains between the different idem platforms. For example, idem-windows implements the oscodename grain and the gpus grain, which are missing on windows in salt. grainsv2 is more complete and consistent across all platforms

[OrangeDog]

Have PRs been opened to fix these things in the current grains?

[Akm0d]

not to my knowledge.

[terminalmage]

This will resolve many bugs that are open against grains

conflicts directly with

No specific open issues

Are there open bugs that this solves, or not? If not, perhaps this SEP should not claim that there are. Regardless, I think this point needs to be clarified.

[dwoz]

I think we should discuss how we gate this change. Adding a config value that will be rendered un-needed in a future release doesn't seem like a great approach to me.

[terminalmage]

Perhaps a new value isn't necessary. We have use_superseded, which was only being used for the format change in module.run. Couldn't it be used here?

[dwoz]

While the purpose of this sep does not intend to provide an answer to this question it certainly seems to provide an opinion about it. I feel it would be pertinent to this discussion, especially because the current PR implementing this change is adding a lot more than just a grains specific library. The addition of idem-* means would be adding idem and POP into salt core.

[Akm0d]

You're right, this SEP and the accompanying PR do provide strong opinions on this, I mean to say that the future of idem in POP in salt has been discussed by a small group, but warrants a larger discussion, more firm plans, and possibly it's own SEP. With this PR we add the __hub__ to the loader, which means custom execution modules and states could make use of idem's hub... but we have discussed adding the option to specify the salt/idem engine in salt states (defaulting to the salt engine) is a possible future integration of idem into salt. This SEP opens the door for that discussion, but that is an implementation/discussion that should happen separately from grains.

[dwoz]

Seems like the addition of __hub__ is inviting a lot of confusion and potential complexity to and already complex code base in regards to the loader. What problem is the addition of __hub__ intended to solve? How will developers know which one to use? What happens if we mix the use of __salt__ and __hub__?

[dwoz]

Have we had a discussion with the core team and community at large around how we want to compartmentalize salt or has this decision been made (by a small group) already?

[Akm0d]

Compartmentalizing salt is a discussion that has been had in small groups, and it will happen in larger groups before decisions are made

[dwoz]

I feel the asyncio portion also warents further exploration and discussion. Especially considering the challenges we've had trying to upgrade tornado to a version which uses asyncio.

[Akm0d]

I have looked at the structure of corn.

• idem-aix
• idem-bsd
• idem-solaris
• idem-dawwin
• idem-linux

corn/grain fqdn is the same on all platforms, why write the code 5 times?
Why is there a idem-linux vs idem-rhel and idem-debian etc I assume to save having the same code in multiple places.
Why not a idem-posix This is well documented standard which Linux/Unix is based on, and then anything outside this standard then use idem-solaris, etc.
Why not have idem-py i.e. anything which can be completed in pure python e.g. fqdn. idem-py Would exclude the use of posix functions like chmod(), chown() as they are Unix based and hence not 100% multi-platform like most of python.

Is this comment going to be addressed?

Pure python grains and idem-posix are great ideas 👍. We will create an idem-universal for pure python grains/exec/state modules and idem-posix that has the shared code between posix platforms. I am especially a fan of not having to push updates to idem's implementation of cmd.run to 7 different platform specific repos. Code that is similar between platforms should be in universal repos.

[dwoz]

This seems like we are adding a fair amount management and community synchronization overhead. Are we sure we have thought through all of this and developed the processes needed to deal with it?

[Akm0d]

I think this is an excellent point and exposes the crux of what needs to be discussed in this SEP. Broadly I've speculated that the windows working group would manage idem-windows and the mac working group would manage idem-darwin and at a certain point in the release there would be a discussion about which version of idem-darwin and idem-windows would be pinned to the next release of salt. But we do need a concrete process in place.

[dwoz]

Specifically what documentation are we lacking now? What is the plan and time-lines around getting all of the needed documentation in place?

[Akm0d]

For grains there isn't much more documentation changes that will need to happen, but for the future of the compartmentalization of salt this is an unaddressed problem. Execution and state modules could be maintained in platform specific projects... How do we merge that documentation into salt?

[dwoz]

I have looked at the structure of corn.

• idem-aix
• idem-bsd
• idem-solaris
• idem-dawwin
• idem-linux

corn/grain fqdn is the same on all platforms, why write the code 5 times?

Why is there a idem-linux vs idem-rhel and idem-debian etc I assume to save having the same code in multiple places.

Why not a idem-posix This is well documented standard which Linux/Unix is based on, and then anything outside this standard then use idem-solaris, etc.

Why not have idem-py i.e. anything which can be completed in pure python e.g. fqdn. idem-py Would exclude the use of posix functions like chmod(), chown() as they are Unix based and hence not 100% multi-platform like most of python.

Is this comment going to be addressed?

[dwoz]

Are they tested against current versions of salt?

[Akm0d]

yes, this PR has a test that runs both versions of grains and does a line by line comparison of each

[taigrr]

Farming out implementations to outside of salt itself h bring up new continuity issues when there's no central authority to determine if for example Ubuntu or Void has the "official/accepted/blessed" format/implementation.

(Examples picked for systemd disagreement reasons)

[dwoz]

Good point, also, how would we ensure that a requires_reboot grain on one system isn't named requires_restart on another system?

[terminalmage]

To build on @dwoz's question, assuming that the platform-specific projects are going to be providing certain grains that used to be in core.py, what are the plans for enforcing that a given platform's idem-XXXXX project provides the grains it is supposed to?

[dwoz]

Are there other alternative approaches we could take instead of 'Plugin Oriented Programming'?

[taigrr]

For something as important as grains to be changing, I'd like to take a more in depth look at POP in salt to understand the significance before a major migration is undertaken.

[terminalmage]

So does this mean that no new core grains will be able to be added to Python 3.5 platforms (Ubuntu 16.04 LTS, etc.)?

[Akm0d]

1. We will rescind this SEP for now, it clearly does too many things
2. We want to redo it in a way that allows grains to be generated by an external source, that way we can have a pluggable interface for generating grains (that doesn't mess with the loader) -- and we need to discuss this more
3. For now, the hub will not be part of salt

[dwoz]

Note: I already typed this out be the SEP was closed. So, just adding the comment for posterity.

I agree that at least some 'compartmentalization' of salt needs to take place. However, I think the main things we should be concerned with are rarely used, un-maintained, and potentially un-needed execution modules and states. I'm not sold on the idea that we need to compartmentalize core components of the salt code base. The idea around moving execution modules to their own repositories is that the people focused on core can focus on the core and not spend time maintaining modules may not be used by a majority of users. Separating out core functionality into smaller code bases seems like it could have the un-desired side-effect of having to split our (anyone interested in contributing to the core's) focus from one place into many places. It will may make the core of salt a lot harder to reason about, and troubleshoot. It might mean that one group of people focused on a specific piece of the core (or platform) ends up making decisions that effect the everyone else without everyone being aware. I feel like we already struggle with this today.

There are upsides and downsides of monoliths. Anyone who went through the micro-services revolution knows there are also upsides and downsides to having components split up into potentially overly small units.

[cachedout]

I spent a few minutes looking at this before I noticed that it had been closed but I think that @dwoz has this exactly right.

It's very tempting to believe that a repo split would be beneficial but I am deeply skeptical. Specifically, here are some points that I struggle with:

Working group leaders and other community members focused on specific platforms can manage the release cycle for
individual idem-platform projects independently from salt.

TBH, this sounds like an absolute nightmare for users to maintain and troubleshoot.

Suddenly, Salt users will need to reason not just about a single version of Salt but (n) number of subsystems as well, all of which are on different release cycles? It's hard to imagine a coherent testing strategy for a matrix of compatibility like this and I think it's unlikely that development concerns will remain as siloed as this model assumes.

While I believe I understand the arguments in favor of this type of compartmentalization, I strongly suspect the practical reality is going to be a bit of mess. I think this is going to be decision that the Salt project regrets. (And it's worth noting that a competing project tried this exact approach in 2016 and reverted to a mono-repo after a user revolt.)

- This will resolve many bugs that are open against grains and will migrate those bugs OUT of the salt repo.

I'm struggling to see how this resolves anything. This sounds more like a rewrite of what's there and then a vague hope that bugs will be fixed along the way. Meanwhile, the user experience degrades dramatically. A contributor now has to reason about the right repo, go there to file a bug, and hope a maintainer is active there? I struggle to imagine a world in which this is better experience for a user. As I have suggested repeatedly, a better solution for this problem would be to facilitate the further participation of working groups and the Salt community inside the mono-repo.

[plinkable]

Having missed all the fun and only read all this after it was closed, I have a simple suggestion instead: create a way to build component packages out of the single repo.

Reviewing and maintaining changes would be in a single place, and individual component release cycles could be managed easily.

Major salt releases would then include/reference already-published-and-tested components, rather than building the bleeding edge that hasn't felt like a stable release for some time now.