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

baby-gnu · 2021-08-18T09:49:57Z

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

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

myii · 2021-08-19T06:59:44Z

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 · 2021-08-19T07:30:25Z

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

@baby-gnu I like the idea of this PR and I reckon we could benefit from using pillars_from_directories in various formulas. However, we've got a few situations to consider, which I've mentioned inline.

NOTE: I haven't had a chance to actually test any of the ideas suggested, yet.

kitchen.yml

myii · 2021-08-23T10:25:20Z

kitchen.yml

-      pillars:
-        top.sls:
-          base:
-            '*':
-              - TEMPLATE
-              - gentoo
-              - define_roles


We probably can avoid a dedicated test/salt/pillar/top.sls and just adapt the same method used in other formulas under state_top.

Some examples:

logrotate-formula

ntp-formula

rng-tools-formula

Suggested change

pillars:

top.sls:

base:

'*':

- TEMPLATE

- gentoo

- define_roles

pillars:

top.sls:

base:

'*':

- TEMPLATE

- define_roles

'G@os_family:Gentoo':

- gentoo

Yes, It could be done from kitchen.yml directly but the main point of having a dedicated top.sls is to be able to run it outside kitchen.

@baby-gnu I haven't come across this idea before -- could you give me an example of this being run outside of Kitchen? Does this also apply to the using the state_top's top.sls as a separate file as well?

The idea came from user colttt on SaltStack matrix room to better validate formula since salt-lint does not cover all possible errors.

colttt uses the following command to compile the states:

salt-call --id=test-minion.example.de --local --file-root=/srv/salt/states --pillar-root=/srv/salt/pillars --retcode-passthrough state.show_sls

But it can't be done easily when the formula requires pillars.

Then, my idea is to provide all the necessary to try out a formula in two steps:

clone the formula:
git clone https://github.com/saltstack-formulas/template-formula.git && cd template-formula

execute the formula locally with our test pillar values:
salt-call --local --id test-minion.example.net --file-root=./ --pillar-root=./test/salt/pillar/ state.show_sls TEMPLATE

There is no point in having state_top's top.sls in a dedicated file too since there is no salt-call option to select that file and I think we should avoid having one in the formula directory referenced by the --file-root option above.

@baby-gnu The example you've shared doesn't appear to work, because the TEMPLATE pillar (i.e. pillar.example) is not in that directory:

[ERROR ] Specified SLS 'TEMPLATE' in environment 'base' is not available on the salt master [ERROR ] Template was specified incorrectly: False [CRITICAL] Pillar render error: Specified SLS 'TEMPLATE' in environment 'base' is not available on the salt master Error running 'state.show_sls': Pillar failed to render. Additional info follows: - Specified SLS 'TEMPLATE' in environment 'base' is not available on the salt master

If I comment out the TEMPLATE line in the newly provided top.sls, then the state runs -- but obviously missing everything from pillar.example (the TEMPLATE pillar).

Yes, that's an issue.

To be completely functional, the test/salt/pillar/ should contains all the required files, in which case:

the pillars_from_files is no more used

the pillar.example is just an example and not a seed for tests.

I push a second commit to move testing values from pillar.example to test/salt/pillar/TEMPLATE.sls and remove pillars_from_files from kitchen.yml.

@baby-gnu If we're going to attempt to use these ideas across existing formulas then that's not going to be fun to refactor. There are more formulas using pillar.example in the CI than any other file. Due to that, it's the default value used from the ssf-formula as well.

50+ formulas across the whole org.

Around 37 formulas that are managed by the ssf-formula.

We can discuss the relative merits of this idea in the upcoming meeting. I'm still not quite sure what the use cases are so it would great if you could demonstrate it.

test/salt/pillar/top.sls

myii · 2021-08-25T09:05:00Z

@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:

myii/salt-formula@29c955e

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:

template-formula/.gitignore

Line 129 in 8d506ea

top.sls

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 · 2021-08-25T13:16:57Z

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.

test/salt/pillar/top.sls

* saltstack-formulas/template-formula#238

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 ``` * pillar.example: remove settings for testing purpose. * test/salt/pillar/top.sls: limit `gentoo` pillars based on `os_family` grain. * test/salt/pillar/defaults.sls: base pillar dedicated to tests. * kitchen.yml (suites): remove the now useless `gentoo` suite. Define `pillars_from_directories` to copy them under `pillar_root`. Remove useless `pillars_from_files`. * .gitlab-ci.yml: fix gentoo suite name * .gitignore: do not ignore test pillar `top.sls`

* saltstack-formulas/template-formula#238

* Automated using myii/ssf-formula#383

* saltstack-formulas/template-formula#238

myii · 2021-11-16T22:09:30Z

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 · 2021-11-16T22:11:50Z

🎉 This PR is included in version 5.0.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

baby-gnu requested review from a team as code owners August 18, 2021 09:49

baby-gnu force-pushed the test/pillar-top-from-file branch 3 times, most recently from f1b657b to 7fc9982 Compare August 18, 2021 09:59

myii reviewed Aug 23, 2021

View reviewed changes

baby-gnu force-pushed the test/pillar-top-from-file branch from 7fc9982 to baa6674 Compare August 24, 2021 12:45

baby-gnu force-pushed the test/pillar-top-from-file branch 3 times, most recently from eba74d7 to c63f4d5 Compare August 30, 2021 13:01

baby-gnu force-pushed the test/pillar-top-from-file branch from c63f4d5 to 759ed7a Compare September 14, 2021 15:45

myii suggested changes Sep 14, 2021

View reviewed changes

test/salt/pillar/top.sls Outdated Show resolved Hide resolved

baby-gnu force-pushed the test/pillar-top-from-file branch from 759ed7a to 2ef5c5e Compare September 15, 2021 09:06

myii added a commit to myii/ssf-formula that referenced this pull request Oct 5, 2021

feat(gitignore): allow test pillar top.sls (template-formula PR)

dbe0823

* saltstack-formulas/template-formula#238

myii mentioned this pull request Oct 5, 2021

feat(saltimages): update with latest changes from salt-image-builder (3003.3, 3002.7 & 3001.8) myii/ssf-formula#374

Merged

myii added a commit to myii/ssf-formula that referenced this pull request Nov 16, 2021

feat(template): review PR 238 (use pillars_from_directories, etc.)

c5cc52b

* saltstack-formulas/template-formula#238

myii mentioned this pull request Nov 16, 2021

feat: use pillars_from_directories & test/salt/pillar/top.sls myii/ssf-formula#383

Merged

myii force-pushed the test/pillar-top-from-file branch from 2ef5c5e to 9c8b67d Compare November 16, 2021 21:50

ci(kitchen): use pillars_from_directories under provisioner

5f80cf2

* Automated using myii/ssf-formula#383

myii added a commit to myii/ssf-formula that referenced this pull request Nov 16, 2021

feat(template): review PR 238 (use pillars_from_directories, etc.)

e3808e6

* saltstack-formulas/template-formula#238

myii force-pushed the test/pillar-top-from-file branch from 9c8b67d to 5f80cf2 Compare November 16, 2021 21:57

myii approved these changes Nov 16, 2021

View reviewed changes

myii merged commit 939e422 into saltstack-formulas:master Nov 16, 2021

saltstack-formulas-travis added the released label Nov 16, 2021

This was referenced Nov 17, 2021

test: merge all test suites into a single (default) suite saltstack-formulas/salt-formula#523

Merged

ci: use pillars_from_directories & test/salt/pillar/top.sls saltstack-formulas/proftpd-formula#6

Merged

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

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

Uh oh!

Conversation

baby-gnu commented Aug 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR progress checklist (to be filled in by reviewers)

What type of PR is this?

Primary type

Secondary type

Does this PR introduce a BREAKING CHANGE?

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

Testing checklist

Additional context

Uh oh!

myii commented Aug 19, 2021

Uh oh!

baby-gnu commented Aug 19, 2021

Uh oh!

myii left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

myii Aug 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

baby-gnu Aug 23, 2021

Choose a reason for hiding this comment

Uh oh!

myii Aug 23, 2021

Choose a reason for hiding this comment

Uh oh!

baby-gnu Aug 23, 2021

Choose a reason for hiding this comment

Uh oh!

myii Aug 23, 2021

Choose a reason for hiding this comment

Uh oh!

baby-gnu Aug 24, 2021

Choose a reason for hiding this comment

Uh oh!

baby-gnu Aug 24, 2021

Choose a reason for hiding this comment

Uh oh!

myii Aug 24, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

myii commented Aug 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

baby-gnu commented Aug 25, 2021

Uh oh!

Uh oh!

myii commented Nov 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

saltstack-formulas-travis commented Nov 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

baby-gnu commented Aug 18, 2021 •

edited

Loading

Does this PR introduce a `BREAKING CHANGE`?

myii left a comment •

edited

Loading

myii Aug 23, 2021 •

edited

Loading

myii commented Aug 25, 2021 •

edited

Loading

myii commented Nov 16, 2021 •

edited

Loading