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

Feature idea: Add a new molecule command to perform check (dry run) #128

Closed
conorsch opened this issue Feb 24, 2016 · 6 comments
Closed

Comments

@conorsch
Copy link
Contributor

Supporting --check mode for Ansible roles isn't trivial; you have to make sure any tasks that register variables are read-only and marked with always_run: true. Personally I'm a big fan of leveraging check mode, but I think the difficulty of testing it makes many folks writing Ansible roles leave it by the wayside. Molecule can help with that.

As low-hanging fruit during edits for #110, adding an additional (optional) stage for the test functionality would be great:

  test:
    # sequence of commands to run when performing `molecule test`
    sequence:
      - destroy
      - syntax-check
      - create
      - converge
      - idempotence
      - check
      - verify
      - destroy

Comments welcome.

@retr0h
Copy link
Contributor

retr0h commented Jul 8, 2016

Implemented in #201.

@retr0h retr0h closed this as completed Jul 8, 2016
@conorsch
Copy link
Contributor Author

conorsch commented Jul 8, 2016

@retr0h The --check and --syntax-check arguments to ansible-playbook are distinct—looks like #201 adds the --syntax-check functionality as molecule check. That's fine, but it's not the same thing as --check, which performs a dry-run of the playbook.

@retr0h
Copy link
Contributor

retr0h commented Jul 9, 2016

Opening

@retr0h retr0h reopened this Jul 9, 2016
retr0h added a commit to retr0h/molecule that referenced this issue Jul 9, 2016
This is in preparation for implementing dry-run ansible#128.
retr0h added a commit to retr0h/molecule that referenced this issue Jul 9, 2016
This is in preparation for implementing dry-run ansible#128.
retr0h added a commit to retr0h/molecule that referenced this issue Jul 9, 2016
* Renamed check to syntax-check
* Implemented a dry-run as check

Fixes: ansible#128
retr0h added a commit to retr0h/molecule that referenced this issue Jul 16, 2016
This is in preparation for implementing dry-run ansible#128.
retr0h added a commit to retr0h/molecule that referenced this issue Jul 16, 2016
This is in preparation for implementing dry-run ansible#128.
retr0h added a commit to retr0h/molecule that referenced this issue Jul 16, 2016
This is in preparation for implementing dry-run ansible#128.
retr0h added a commit that referenced this issue Jul 18, 2016
This is in preparation for implementing dry-run #128.
@retr0h retr0h self-assigned this Sep 3, 2016
@retr0h
Copy link
Contributor

retr0h commented Sep 3, 2016

@conorsch I think I'm going to need more details on this. I'm not entirely sure how it would be helpful.

If we are to add check into the test sequence. Where would it go? It doesn't make sense to run it after converge, since nothing would then change.

Since change always returns success, I don't see how it would be useful in the test sequence. I can implement the changed subcommand, but not add it to the test sequence. People can then do what they wish with it?

retr0h added a commit to retr0h/molecule that referenced this issue Sep 3, 2016
The ability to call `molecule check` to perform a "Dry Run".  This is
currently not incorporated into the default test sequence.

Fixes: ansible#128
retr0h added a commit to retr0h/molecule that referenced this issue Sep 3, 2016
The ability to call `molecule check` to perform a "Dry Run".  This is
currently not incorporated into the default test sequence.

Fixes: ansible#128
retr0h added a commit to retr0h/molecule that referenced this issue Sep 3, 2016
The ability to call `molecule check` to perform a "Dry Run".  This is
currently not incorporated into the default test sequence.

Fixes: ansible#128
retr0h added a commit to retr0h/molecule that referenced this issue Sep 3, 2016
The ability to call `molecule check` to perform a "Dry Run".  This is
currently not incorporated into the default test sequence.

Fixes: ansible#128
@conorsch
Copy link
Contributor Author

conorsch commented Sep 6, 2016

If we are to add check into the test sequence. Where would it go? It doesn't make sense to run it after converge, since nothing would then change.

Adding after converge actually does make sense, although it's not intuitive. Many roles use a command/register pragma to store a dynamic variable, then inspect that var in a subsequent task as part of conditional logic. By default, Ansible skips command tasks, because it can't know the severity of side-effects, so the subsequent tasks inspecting the var will fail in check mode. Having Molecule run a check mode after converge would be very useful to catch errors like this.

It's technically possible to support dry-runs of a first-time role run with Ansible, but I don't see many role authors striving for it. For example, get_url tasks will be skipped by default, and then any tasks referencing a file downloaded by get_url will fail, since the file doesn't exist. A reasonable order for running --check mode seems to be:

      - destroy
      - syntax
      - create
      - converge
      - idempotence
      - check
      - verify

If you run check before converge, I'd wager that most Ansible roles would fail, since folks simply don't plan for that use case.

retr0h added a commit that referenced this issue Sep 6, 2016
* Implemented `check`

The ability to call `molecule check` to perform a "Dry Run".  This is
currently not incorporated into the default test sequence.

Fixes: #128

* Added molecule check functional scenario

* Implemented feedback on check
@geerlingguy
Copy link
Contributor

Thanks for this, and to anyone finding this in the future, you can add it to your sequence in molecule.yml with:

scenario:
  name: default
  test_sequence:
    - lint
    - destroy
    - dependency
    - syntax
    - create
    - prepare
    - converge
    - idempotence
    # Added --check run test.
    - check
    - side_effect
    - verify
    - destroy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants