refactor(sls): make states more readable by spliting into sub-components

[baby-gnu]

PR progress checklist (to be filled in by reviewers)

• Changes to documentation are appropriate (or tick if not required)
• Changes to tests are appropriate (or tick if not required)
• Reviews completed

---

What type of PR is this?

Primary type

• [build] Changes related to the build system
• [chore] Changes to the build process or auxiliary tools and libraries such as documentation generation
• [ci] Changes to the continuous integration configuration
• [feat] A new feature
• [fix] A bug fix
• [perf] A code change that improves performance
• [refactor] A code change that neither fixes a bug nor adds a feature
• [revert] A change used to revert a previous commit
• [style] Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)

Secondary type

• [docs] Documentation changes
• [test] Adding missing or correcting existing tests

Does this PR introduce a BREAKING CHANGE?

No.

Related issues and/or pull requests

Describe the changes you're proposing

The current salt-formula is quite hard to read with lots of jinja conditionnals in the middle of states which is against the current standards.

This PR introduces a refactoring of the states into subcomponents:

• master
• TODO

and provides backward compatible state IDs which only notify user that their references must be changed.

Pillar / config required to test the proposed changes

Debug log showing how the proposed changes work

Documentation checklist

• Updated the README (e.g. Available states).
• Updated pillar.example.

Testing checklist

• Included in Kitchen (i.e. under state_top).
• Covered by new/existing tests (e.g. InSpec, Serverspec, etc.).
• Updated the relevant test pillar.

Additional context

[baby-gnu]

One point, the windows tests are failing because the salt.master was called for that platform:

• before, a conditionnal made the rendered SLS empty
• now, I used conditionnal to check for platform and using salt.master on windows platform results in False return codes for the states

logs of salt.master on default-windows-2022-latest-py3

          ID: salt-windows-excluded-test.fail_without_changes
           Function: test.fail_without_changes
        Name: Verify that current platform is not Windows
             Result: False
            Comment: Platform Windows is not supported
            Started: 07:44:17.269399
           Duration: 0.999 ms
            Changes:   
       ----------
          ID: salt-master-package-install-pkg.installed
           Function: pkg.installed
        Name: salt-master
             Result: False
            Comment: One or more requisite failed: salt.windows-excluded.salt-windows-excluded-test.fail_without_changes
            Started: 07:44:40.025083
           Duration: 0.0 ms
            Changes:   
       ----------
          ID: salt-master-config-files-file.recurse
           Function: file.recurse
        Name: C:\salt\conf/master.d
             Result: False
            Comment: One or more requisite failed: salt.master.package.install.salt-master-package-install-pkg.installed, salt.windows-excluded.salt-windows-excluded-test.fail_without_changes
            Started: 07:44:40.025083
           Duration: 0.0 ms
            Changes:   
         Name: C:\salt\conf/master.d/_defaults.conf - Function: file.absent - Result: Clean Started: - 07:44:40.025083 Duration: 1.0 ms
       ----------
          ID: salt-master-service-running
           Function: service.running
        Name: salt-master
             Result: False
            Comment: One or more requisite failed: salt.master.config.files.salt-master-config-files-file.recurse
            Started: 07:44:40.034083
           Duration: 0.0 ms
            Changes:   
         Name: salt-minion-py3 - Function: pkg.installed - Result: Clean Started: - 07:44:40.034083 Duration: 923.292 ms

I prefer to have the failing SLS instead of believing that everything was fine to assign an sls to the wrong platform, but if the community prefer the previous behaviour I'm ok to revert that part.

Regards.

[daks]

One point, the windows tests are failing because the salt.master was called for that platform:

* before, a conditionnal made the rendered SLS empty

* now, I used conditionnal to check for platform and using `salt.master` on windows platform results in `False` return codes for the states

logs of salt.master on default-windows-2022-latest-py3

I prefer to have the failing SLS instead of believing that everything was fine to assign an sls to the wrong platform, but if the community prefer the previous behaviour I'm ok to revert that part.

Regards.

If salt.master is not compatible with Windows, I think it should be clearly documented in the README for the meta-state and therefore make the state return nothing instead of returning an error.

(I didn't read the PR itself for now)

[baby-gnu]

If salt.master is not compatible with Windows, I think it should be clearly documented in the README for the meta-state and therefore make the state return nothing instead of returning an error.

Thanks @daks, that's exactly the kind of feedback I want.

So, as an example, you prefer the following execution output:

# salt '*' state.apply salt.master
PC-760282.example.net:
----------
          ID: salt-master-install-skip
    Function: test.show_notification
      Result: True
     Comment: No salt-master state for Windows
     Started: 11:11:14.046875
    Duration: 0.0 ms
     Changes:   

Summary for PC-760282.example.net
------------
Succeeded: 1
Failed:    0
------------
Total states run:     1
Total run time:   0.000 ms

over this one:

# salt '*' state.apply salt.master
PC-760282.example.net:
----------
          ID: salt-windows-excluded-test.fail_without_changes
    Function: test.fail_without_changes
        Name: Verify that current platform is not Windows
      Result: False
     Comment: Platform Windows is not supported
     Started: 11:15:17.453125
    Duration: 0.0 ms
     Changes:   

Summary for PC-760282.example.net
------------
Succeeded: 0
Failed:    1
------------
Total states run:     1
Total run time:   0.000 ms
ERROR: Minions returned with non-zero exit code

[daks]

Yes, it seems a better solution. I don't use Windows but if I did I wouldn't want to have an error when everything is fine. And I consider that everything is fine because salt-master is just not supported on Windows.
I think this can be a problem if you run automated highstates and just check for errors: this will pop up everytime as an error when it's not.

Of course, another solution would be to just run the compatible states.

[baby-gnu]

Ok, I consider an error to attribute an sls to an incompatible minion.

But having errors that states can't be run could be an issue, I'll look at our it render with state warning instead of simple notification.

[daks]

Ok, I consider an error to attribute an sls to an incompatible minion.

You're not wrong. As I said, for my use case, I would simply not run the meta-state on Windows.

Let see what other people think about that. I have no strong position on this.

[vveliev-tc]

I think the error is better than the success message. not everyone knows windows is not supported for master.

As a user, if I write salt-call state.sls salt.master - if it's successful I consider that everything is installed.