add first saltcheck tests #4
Conversation
|
By defining saltcheck tests in upstream formulas, it can be used as part of the forumula's CI pipeline, but also gives users of the formula the ability to validate deployed environments as well. So rather than needing to run a possibly heavyweight |
|
@mchugh19 Following on from the updated discussions in saltstack-formulas/template-formula#88, I've tested the various modifications required to get this all working as well. It would be even nicer if we could get relative imports working. |
If I'm following correctly, I think the issue with relative imports is needing map files or other includes copied over to the minion in order to fully render. This should be properly fixed in saltstack/salt#53068 which should be in neon. @myii are you able to test with this version to see if it fixes your issues? https://raw.githubusercontent.com/saltstack/salt/2088199f0714fc0bff416ba11497371cdb993a3b/salt/modules/saltcheck.py |
|
@mchugh19 Since we're running I'll test this version and let you know how it gets on. |
|
Unfortunately, I think develop does not yet include the PR mentioned which reworks how files are copied and render dependencies pulled in. Worst case, you should be able to wait a couple of weeks (fingers crossed!) for the backlog of PRs to be merged in. |
|
@mchugh19 Not a problem, worst case I can pull in the file during the Travis run. I've already been testing that when making modifications to it, such as dealing with the missing error code. I'm testing it now, so I should have a response for you fairly soon. |
|
@mchugh19 OK, making progress but I'm at the stage of figuring out how to run the tests. This was working fine up to now: $ sudo salt-call --config-dir=/tmp/kitchen/etc/salt saltcheck.run_state_tests cron check_all=TrueThat still works with the old file but not the new. It reports as |
|
Right, so it's looking in different locations to what I expected: looking in %s to cache tests salt://cron/package/install/saltcheck-tests
looking in %s to cache tests salt://cron/package/saltcheck-tests
looking in %s to cache tests salt://cron/service/running/saltcheck-tests
looking in %s to cache tests salt://cron/service/saltcheck-tests
The original location (with the file from searching path: %s /tmp/kitchen/var/cache/salt/minion/files/base
Found test: %s /tmp/kitchen/var/cache/salt/minion/files/base/cron/saltcheck-tests/config.tst
Found test: %s /tmp/kitchen/var/cache/salt/minion/files/base/cron/saltcheck-tests/service.tst
Found test: %s /tmp/kitchen/var/cache/salt/minion/files/base/cron/saltcheck-tests/install.tst |
|
Interesting, that should work! I'll take a look this week.
…On Mon, Oct 14, 2019 at 9:49 PM Imran Iqbal ***@***.***> wrote:
@mchugh19 <https://github.com/mchugh19> OK, making progress but I'm at
the stage of figuring out how to run the tests. This was working fine up to
now:
$ sudo salt-call --config-dir=/tmp/kitchen/etc/salt saltcheck.run_state_tests cron check_all=True
That still works with the old file but not the new. It reports as Missing
Tests.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4?email_source=notifications&email_token=AAKMDZLLFDSMTBBZNXWNRL3QOTLL7A5CNFSM4HEKFOJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBGO63Y#issuecomment-541912943>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAKMDZPZWFC7T2NOZA6EMT3QOTLL7ANCNFSM4HEKFOJA>
.
|
|
@mchugh19 So I've adjusted the location of the tests for the time being, noting that:
The important thing is that it appears to be working with relative imports in I'm going to try to put this all together now to form a final solution to be implemented here. Thanks for linking me to the updated file! That'll make a significant difference. |
|
OK, there's a $ sudo salt-call --config-dir=/tmp/kitchen/etc/salt config.get saltcheck_test_location
local:
tests/integration/saltcheck
$ sudo salt-call --config-dir=/tmp/kitchen/etc/salt saltcheck.run_state_tests cron check_all=True
looking in %s to cache tests salt://cron/package/install/tests/integration/saltcheck
looking in %s to cache tests salt://cron/package/tests/integration/saltcheck
looking in %s to cache tests salt://cron/service/running/tests/integration/saltcheck
looking in %s to cache tests salt://cron/service/tests/integration/saltcheckMy expectation for that configuration would be: |
@myii Can you try this untested version? https://gist.github.com/mchugh19/340874466f0799348b1c0be1ee022490 It should better handle your salt://state_root/saltcheck_test_location use-case. |
|
@mchugh19 OK, I will do. I've got it working with the current version but it's only running the tests for Called using: sudo salt-call --config-dir=/tmp/kitchen/etc/salt saltcheck.run_state_tests cron check_all=True |
|
@mchugh19 Following on from the previous comment, it looks like it stops searching for paths when it finds the first one, that's why the rest of the tests aren't found. https://travis-ci.org/myii/cron-formula/jobs/597872502#L2426-L2427 looking in %s to cache tests salt://cron/package/install/saltcheck-tests
looking in %s to cache tests salt://cron/package/saltcheck-tests |
That was the intent that you would have a saltcheck-tests directory at the root of your state. What file structure are you using? |
|
@mchugh19 That comment was referring to the version from salt://cron/config/saltcheck-tests
salt://cron/package/saltcheck-tests
salt://cron/service/saltcheck-testsI'm now in the process of checking the one from the GitHub gist. |
|
@mchugh19 Looking promising. https://travis-ci.org/myii/cron-formula/jobs/597878982#L2230-L2232 looking in %s to cache tests salt://cron/package/install/saltcheck-tests
looking in %s to cache tests salt://cron/package/saltcheck-tests
looking in %s to cache tests salt://cron/saltcheck-testsStill not running the |
|
@mchugh19 OK, finally figured it out. These new versions are still not working with the looking in %s to cache tests salt://cron/saltcheck-tests
[ERROR ] tplroot
.
[ERROR ] tplfile
[ERROR ] tpldir
.
[ERROR ] tpldot
[ERROR ] slspath
This means the pillar doesn't get merged, which is why the {%- if 'tasks' in cron_settings %}
So I'll stick with a local |
* Make consistent with the actual states
|
@mchugh19 I've got a final solution for this: https://travis-ci.org/myii/cron-formula/builds/597973852. Are you OK with me appending the commits to this PR so that we can get on with merging this? It can stimulate the necessary discussions about using |
|
@mchugh19 OK, I've pushed a few more commits here, after a discussion with @javierbertoli about some concerns. It follows on from the same points made by @daks previously, starting in saltstack-formulas/template-formula#88. @aboe76 You may find this interesting, also. One of the main concerns is that One thing I've been able to do (and appreciate) is the near 1:1 relationship between the code and the tests. You can literally work through the [1] Getting the data-{%- from tplroot ~ "/map.jinja" import cron with context %}
+{%- set cron = salt['pillar.get']('cron', {}) %}
{%- set cron = {
'enabled': true,
'tasks': {
'task1': {
'type': 'present',
'name': 'echo test > /tmp/test',
'user': 'root',
'minute': 0,
'hour': 0,
'daymonth': 7,
'month': 1,
'dayweek': 6,
'comment': 'comment1',
'commented': false,
},
'task2': {
'type': 'absent',
'name': 'echo task2 > /tmp/test2',
'user': 'root',
'minute': 'random',
'hour': 1,
},
'task3': {
'type': 'present',
'name': 'echo task3 > /tmp/test3',
'user': 'root',
'special': '@hourly',
'comment': 'comment3',
},
'task4': {
'type': 'present',
'name': 'echo task4 > /tmp/test4',
'user': 'root',
'minute': '*/5',
'hour': '*',
'comment': 'comment4',
},
'task5': {
'type': 'present',
'name': 'echo task5 > /tmp/test5',
'user': 'root',
'minute': 'random',
'hour': 1,
'commented': true,
},
},
} %}[2] Loop/variables {%- for task, task_options in cron.get('tasks', {}).items() %}
{%- set cron_type = task_options.type|d('present') %}
[3] Cron entry
|
|
Agreed @myii, I also appreciate how being able to leverage salt modules for testing gives deep introspection for validation and test coverage (I originally patched saltcheck to suit our needs when deploying large hadoop clusters in multi cloud environments and needing to ensure that data stores, xml hadoop configs, versioned jar deployments, databases, and seeded data were all in the expected state). As you say, it's also powerful in by leveraging jinja and the salt renderer, you keep a very similar syntax, and can access map.jinja or pillars as desired. I think there are a few different ways you could treat saltcheck testing. For formula CI processes, you could test
The #3 option is how I've witnessed most inspect tests, but why not do both? If you want to be paranoid and double check the map.jinja process or pillars, you could have the #1 or #2 tests, and a static dictionary of validation data feed into your CI tests. Additionally, these are formulas intended for use by the community. As such, including saltcheck tests which can be run by the end-users (like #1 or #2 above) give further confidence in their robustness and that the pillar docs all actually functioned. I think that even if you didn't use saltcheck as the primary CI validation tool, that it still makes sense to ship saltcheck tests so that functionality and validation can be testing can be leveraged by users. I do understand the reluctance to use a tool to test itself, but you also want to draw a line around what is being tested. Should |
|
And another, required (final) commit. It also contains a couple of examples of how Package: *InSpec* | *Saltcheck*
------------------------+-------------------------------------------------
package_name = | {%- set package_name = 'cronie' %}
case os[:family] |
when 'debian' | {%- if grains.os_family in ['Debian'] %}
'cron' | {%- set package_name = 'cron' %}
else |
'cronie' |
end | {%- endif %}Service: *InSpec* | *Saltcheck*
------------------------+-------------------------------------------------
service_name = | {%- set service_name = 'crond' %}
case os[:family] |
when 'debian', 'suse' | {%- if grains.os_family in ['Debian', 'Suse'] %}
'cron' | {%- set service_name = 'cron' %}
when 'linux' |
case os[:name] |
when 'arch' | {%- elif grains.os_family in ['Arch'] %}
'cronie' | {%- set service_name = 'cronie' %}
else |
'crond' |
end |
else |
'crond' |
end | {%- endif %}
|
|
@mchugh19 Referring back to an earlier comment here: #4 (comment).
Due the 1:1 relationship between state file and test file, it would be preferable to have the tests within each sub-directory instead. Had a look at the code and got to the bottom of this. Note: All of the below was when testing with: Looking at paths being evaluated with no
|
|
For clarity,
|
|
Good updates @myii! I'm traveling for the next week, but I'll incorporate your solution into the saltcheck patches when I'm back home. Thanks! |
|
After suggestions from @daks and @aboe76, rearranged all of the test files under a single root-level
With this change, I've readjusted again to base the |
|
@myii wow, that's quick. |
* Automated using myii/ssf-formula#83
|
Our first |
## [0.2.4](v0.2.3...v0.2.4) (2019-10-23) ### Bug Fixes * **saltcheck:** fix broken import and standardise across test files ([](7911b71)) * **saltcheck:** fix invalid `service` test ([](677c956)) * **saltcheck:** remove trailing spaces ([](aada0ae)) * **saltcheck:** replace `map.jinja` references with InSpec conditionals ([](7e9e619)) * **saltcheck:** update for `cron` instead of `cron_settings` ([](26cfa4f)) ### Code Refactoring * **config:** minimise and standardise between `.sls` & `.tst` ([](18585bd)) * **config:** remove duplication in using a loop ([](652ebff)) * **jinja:** used shortened form of `|default` filter ([](a0f891e)) * **saltcheck:** relocate `.tst` files according to 1:1 `.sls` files ([](ee65236)), closes [/github.com//pull/4#issuecomment-544140377](https://github.com//github.com/saltstack-formulas/cron-formula/pull/4/issues/issuecomment-544140377) * **saltcheck:** use `package.tst` instead of `install.tst` ([](d2c9544)) * **saltcheck:** use root-level `saltcheck-tests` directory ([](6e54c3f)) ### Continuous Integration * **travis:** obtain `saltcheck.py` and run the tests (only on `develop`) ([](8ae46e5)) * **travis:** run `salt-lint` for `.tst` files as well ([](baab964)) * **travis:** standardise `saltcheck` comments ([](e23276b)) * **travis:** update `salt-lint` config for `v0.0.10` [skip ci] ([](b701d79)) ### Styles * **config.tst:** rearrange Jinja statements for clarity ([](8abec54)) * **saltcheck:** merge `absent` & `present` into one `if` block ([](33f344c)) * **saltcheck:** use consistent order of assertions ([](88229f0)) ### Tests * **pillar:** add test for `commented` and clarify each test ([](3d0dcb2)) * **pillar:** ensure `special` is being tested as well ([](951a959)) * **saltcheck:** add first tests ([](9847aff)) * **saltcheck:** add support for `random` values ([](007970f)) * **saltcheck:** add test for `service.available` ([](226eb88)) * **saltcheck:** add test for `service.running` ([](5cdc50f)) * **saltcheck:** avoid `map.jinja`, use the test pillar instead ([](cce5e67)) * **saltcheck:** fix `config` tests ([](9225b18)) * **saltcheck:** remove duplication in `config.tst` using a loop ([](72281c7)) * **saltcheck:** test for `commented` and not `commented` ([](5070611)) * **saltcheck:** test for `special` in `config.tst` as well ([](6f2b323)) * **saltcheck:** use local `map.jinja` to workaround missing `tpldata` ([](8845b3c))
|
🎉 This PR is included in version 0.2.4 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
As mentioned in
saltstack-formulas/template-formula#87
First draft at adding saltcheck tests. Uses functionality from saltcheck module from Neon as well as saltstack/salt#52441 patch to cron module. Without the cron module patch functionality, the service and install tests function, but the return from
cron.lsisn't very parsible.Given a pillar:
With an existing crontab of
NOTE: I've manually edited the cron entry's "daymonth" from 7 to 8
As shown above, all tst files in the saltcheck-tests directory can be run with:
salt-call saltcheck.run_state_tests cron check_all=TrueOr individual test files can be run:
# salt-call saltcheck.run_state_tests cron.installIf this is run on a minion which has the cron module defined in top.sls as part of highstate, then tests can be run for all highstate associated states with:
# salt-call saltcheck.run_highstate_testsAlso, like all salt commands, output can be displayed as json for better parsing: