test(pillar): use static test/salt/pillar/top.sls

baby-gnu commented 3 years ago

The kitchen-salt provisionner have the pillars_from_directories option to recusively copy directories under target pillar root.

This has 3 advantages:

simplify kitchen.yml
manage pillar assignment with standard salt targetting mechanism, this avoid the dedicated gentoo suite (it could have been done from kitchen.yml itself by the way)

ease the test outside kitchen by running salt-call directly with --pillar-root like:

salt-call --local --id test-minion.example.net --file-root=template-formula/ --pillar-root=template-formula/test/salt/pillar/ state.show_sls TEMPLATE

test/salt/pillar/top.sls: limit gentoo pillars based on os_family grain.
kitchen.yml (suites): remove the now useless gentoo suite. Define an empty required pillars to not override static top.sls Define pillars_from_directories to copy them under pillar_root.

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
[x] [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
[x] [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

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

[x] 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

myii commented 3 years ago

Thanks, @baby-gnu. This looks like a nice idea.

This will also necessitate some changes in how the ssf-formula works, so I'm going to need a little while to get around to this. Currently got a bit of a backlog due to the 3003.2 release, etc.

baby-gnu commented 3 years ago

This will also necessitate some changes in how the ssf-formula works, so I'm going to need a little while to get around to this. Currently got a bit of a backlog due to the 3003.2 release, etc.

Thanks, it was an idea inspired by a user in SaltStack matrix room, it's not urgent.

myii commented 3 years ago

@baby-gnu I thought I'd familiarise myself with these changes by adapting it for the salt-formula. This is the commit I used, which is passing in the CI:

https://github.com/myii/salt-formula/commit/29c955e906e4edf9ecbc50cd35170c17f28c6b43

A couple of things I came across so far, which are valid for this PR as well:

There's no need to use pillars: {} since that's the default provided by kitchen-salt.
(I haven't done this in the commit there but) We probably need to remove the following line as well, since it prevents the top.sls from being git added except using -f:

https://github.com/saltstack-formulas/template-formula/blob/8d506ea93eca196c60e178f8dd2eb2a7ad6594e9/.gitignore#L129

Tested it out with:

$ salt-call --local \
            --id test-minion.example.net \
            --file-root=./ \
            --pillar-root=./test/salt/pillar/ \
            --out=yaml \
            state.show_sls salt

baby-gnu commented 3 years ago

There's no need to use pillars: {} since that's the default provided by kitchen-salt.

You are right, I'll remove it on the final rework of this PR

(I haven't done this in the commit there but) We probably need to remove the following line as well, since it prevents the top.sls from being git added expect using -f:

I'm really not sure, I think we should keep it to avoid having a top.sls in the states tree, so I propose to use:

top.sls
!test/salt/pillar/top.sls

This way, all top.sls will be ignored except the test/salt/pillar/top.sls one.

myii commented 2 years ago

Thanks for the patience, @baby-gnu -- merged.

Rebased your changes on the latest version of this formula (the CI instances had changed).

The main notes regarding the additional commit:

Moved pillars_from_directories under provisioner instead of under suites, as discussed in the working group meetings.
Used test/salt/pillar/default.sls instead of test/salt/pillar/defaults.sls, to maintain consistency with the large number of formulas out there (default suite has a default.sls pillar).
Other minor consistency fixes, such as updating .travis.yml and CODEOWNERS.

saltstack-formulas-travis commented 2 years ago

:tada: This PR is included in version 5.0.4 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

saltstack-formulas / template-formula