Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Saltcheck merge #49195

Merged
merged 20 commits into from
Aug 28, 2018
Merged

Saltcheck merge #49195

merged 20 commits into from
Aug 28, 2018

Conversation

mchugh19
Copy link
Contributor

@mchugh19 mchugh19 commented Aug 19, 2018

What does this PR do?

  • Sync with upstream
    • minor cleanups
    • support test timing (changes output format)
    • add skip syntax
    • add pillar-data
  • Adds empty and not empty assertions to saltcheck
  • Support saltenv environments
  • Removes the requirement to specify the expected-return key (for use by assertEmpty or assertTrue)
  • Attempt to cleanup web docs https://docs.saltstack.com/en/develop/ref/modules/all/salt.modules.saltcheck.html by marking class functions as private (I'm not sure what is proper here)
  • Complete redo of test file naming logic. Now tests are called by the saltcheck.run_state_tests command by looking for the passed name in the saltcheck-tests directory. As documented in the module, this means tests are now associated to states by having the same name. The check_all option can be used to run all tests in the state's saltcheck-tests directory.
/srv/salt/apache/
    init.sls
    config.sls
    saltcheck-tests/
        init.tst
        config.tst

Tests can be run for each state, or all apache tests

salt '*' saltcheck.run_state_tests apache,apache.config
salt '*' saltcheck.run_state_tests apache check_all=True
salt '*' saltcheck.run_highstate_tests

For the state

install_common_package:
  pkg.installed:
    - pkgs:
      - vim
      - avahi-daemon
      - libnss-mdns

This allows for the following saltchecks:

{%- set package_list = ["vim", "avahi-daemon", "libnss-mdns", "lua-unit"] %}

{% for package in package_list %}
verify_{{ package }}:
  module_and_function: pkg.version
  args:
    - {{ package }}
  assertion: assertNotEmpty
{% endfor %}

providing the output

local:
    |_
      ----------
      common:
          ----------
          verfity_avahi-daemon:
              ----------
              duration:
                  0.8547
              status:
                  Pass
          verfity_libnss-mdns:
              ----------
              duration:
                  0.0013
              status:
                  Pass
          verfity_lua-unit:
              ----------
              duration:
                  0.0003
              status:
                  Fail: value is empty
          verfity_vim:
              ----------
              duration:
                  0.0004
              status:
                  Pass
    |_
      ----------
      TEST RESULTS:
          ----------
          Execution Time:
              0.8567
          Failed:
              1
          Missing Tests:
              0
          Passed:
              3
          Skipped:
              0

Skip example

package_latest:
  module_and_function: pkg.upgrade_available
  args:
    - apache2
  assertion: assertFalse
  skip: True

Pillar example

pillar_test:
  module_and_function: pillar.items
  assertion: assertNotEqual
  expected-return: ''
  pillar-data:
    thing: yep

Tests written?

No

Commits signed with GPG?

No

@cachedout
Copy link
Contributor

Hi @mchugh19. There are some test failures here: https://jenkinsci.saltstack.com/job/pr-kitchen-centos7-py3/job/PR-49195/4/

Christian McHugh added 2 commits August 20, 2018 14:44
@mchugh19
Copy link
Contributor Author

Tests corrected and also took care of the issue from #49207

@mchugh19
Copy link
Contributor Author

It's probably worth mentioning the change in to the release notes of the saltcheck output format. Parsing the new format requires checking the status key.

Previous:

local:
    |_
      ----------
      ntp:
          ----------
          ntp-client-installed:
              Pass
          ntp-service-status:
              Pass
    |_
      ----------
      TEST RESULTS:
          ----------
          Failed:
              0
          Missing Tests:
              0
          Passed:
              2

New

local:
    |_
      ----------
      ntp:
          ----------
          ntp-client-installed:
              ----------
              duration:
                  1.0408
              status:
                  Pass
          ntp-service-status:
              ----------
              duration:
                  1.464
              status:
                  Pass
    |_
      ----------
      TEST RESULTS:
          ----------
          Execution Time:
              2.5048
          Failed:
              0
          Missing Tests:
              0
          Passed:
              2
          Skipped:
              0

@rallytime
Copy link
Contributor

Nicole Thomas and others added 5 commits August 24, 2018 16:06
…mes to states. If previous behavior is desired, then check_all=True can be passed to run all tests in the specified saltcheck-tests directory.
@cachedout
Copy link
Contributor

@mchugh19 The static analyzer has a few suggestions here. Could you take a look and see if any of them make sense to you to fix? https://codeclimate.com/github/saltstack/salt/pull/49195

@mchugh19
Copy link
Contributor Author

Yep, I looked over the codeclimate report, and it doesn't seem useful or applicable.

@rallytime rallytime requested a review from gtmanfred August 28, 2018 14:32
@gtmanfred gtmanfred merged commit 12d77a5 into saltstack:develop Aug 28, 2018
garethgreenaway added a commit to garethgreenaway/salt that referenced this pull request Sep 18, 2019
@waynew waynew added the has master-port port to master has been created label Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has master-port port to master has been created
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants