Travis CI: Verify that "Ansible --check mode" passes #220

gildegoma · 2020-08-01T15:43:37Z

Initially motivated by #218.

In order to get a build success, this PR (temporarily) includes the changes proposed in #219.

pkuczynski · 2020-08-01T16:34:09Z

Does not seem to help. The build is still failing :(

gildegoma · 2020-08-01T16:46:04Z

The CI job detected a new problem, occurring only with the root.yml playbook.

See https://travis-ci.org/github/rvm/rvm1-ansible/jobs/714023964#L296 for instance.

danochoa · 2020-08-01T17:42:31Z

.travis.yml

+  - docker-compose config --services | grep "$PLATFORM" | xargs docker-compose up -d
+  - ansible-playbook $PLAYBOOK -l "$PLATFORM:&latest" --check --diff
+  - ansible-playbook $PLAYBOOK -l "${PLATFORM:-latest}" -vv
+  - ansible-playbook $PLAYBOOK -l "$PLATFORM:&latest" --check --diff


why run check mode twice?

Running before and after the effective ansible-playbook run verifies the check mode in two completely different situations. In the current root.yml failure (See "Symlink ruby related binaries on the system path" task), the problem only occurs when running --check before the role is applied.

It is to project maintainers to decide if they want that --check mode should work on an "empty" or "incompletely configured" system.

Ok I see now. Can we add a comment to explain this?

I think we definitely want --check mode to pass, before we apply changes, as I think this is the more common workflow, at least for me and from what I understand. And I think it seems redundant to run again after normal mode, although I'm not entirely sure either way.

I should add: I see what you mean about running --check mode a second time because the system is in a different state, but I think it's up to the normal mode test run to make sure any changes are good at this point.

My second "--check mode" run is intended to guarantee that you can execute the role in check-mode without error.

But I get you're point, and actually it made me realize that there is something more important to verify: role idempotency, by executing twice the tasks (not in check mode). I'll put some more propositions... stay tuned 😉

@danochoa please, look at c999c43. Without this change, the second part (rubies.yml) of the role would fail.
@pkuczynski @thbar I defer to you to decide if the 'check mode' should pass on a "non-bootstrapped" system (i.e. RVM not available yet).

Note: Proposal for the "full idempotency" check comes...

Would you mind adding a comment in the code here too to explain running check mode twice? I think that would be helpful for someone looking at the code for the first time without reading our PR comments.

.travis.yml

thbar

Per #219 (comment), installing rubies all the time is very slow in my testing environment. I am not sure why the same code is on both PR, but the same reasoning applies!

tasks/rubies.yml

thbar · 2020-08-18T09:06:12Z

We have updated & tested #219. I believe it will be good to first merge #219, then update #220!

For sake of CI speed, the check mode is verified only against one container (the latest version of each operating system platform). Such check is performed before, and after, the Ansible role has been applied to the managed instance.

Skip all the 'rubies.yml' tasks if RVM is not installed yet.

gildegoma · 2020-08-19T04:50:25Z

Rebased and now ready for review.

danochoa

I think some comments could help explain a couple things that might seem unusual but feel free to ignore. Otherwise I think this looks good.

danochoa · 2020-08-19T14:28:38Z

tasks/main.yml

@@ -5,7 +5,16 @@
  become: yes
  become_user: '{{ rvm1_user }}'

+- name: Check that RVM is installed


I'm not too sure about this but I think its ok to let the role fail if nothing has been installed, rather than adding tasks like this to satisfy check mode. Do you mind adding a comment in the code here to explain this task to help avoid confusion (since the previous tasks are there to ensure RVM is installed)?

I agree with you. For the moment, I made this to be "always" check mode compatible, but I am not sure if it is worth it. Up to core maintainers to decide...

danochoa · 2020-08-19T14:29:18Z

tasks/rubies.yml

@@ -4,6 +4,7 @@
  command: '{{ rvm1_rvm }} list strings'
  register: rvm_list_strings
  changed_when: False
+  check_mode: no # Run in normal mode when in --check mode (http://docs.ansible.com/ansible/playbooks_checkmode.html)


danochoa · 2020-08-19T14:36:07Z

.travis.yml

+  - docker-compose config --services | grep "$PLATFORM" | xargs docker-compose up -d
+  - ansible-playbook $PLAYBOOK -l "$PLATFORM:&latest" --check --diff
+  - ansible-playbook $PLAYBOOK -l "${PLATFORM:-latest}" -vv
+  - ansible-playbook $PLAYBOOK -l "$PLATFORM:&latest" --check --diff


Would you mind adding a comment in the code here too to explain running check mode twice? I think that would be helpful for someone looking at the code for the first time without reading our PR comments.

gildegoma · 2020-08-19T16:42:20Z

Would you mind adding a comment in the code here too to explain running check mode twice? I think that would be helpful for someone looking at the code for the first time without reading our PR comments.

@danochoa thank you for all the feebacks 😃. Yes, if we stick the goal to support "check mode" in any situation, I'll add a comment to try to make things crystal clear.

pkuczynski · 2022-03-25T20:59:35Z

As we are moving away from Travis to GHA, is this still valid @danochoa ?

thbar · 2022-03-26T10:24:08Z

See #235, which maybe will have to be merged first, to see if current PR is worth adapting to GH actions.

pkuczynski · 2022-03-26T21:40:27Z

@gildegoma as we migrated to Github Actions in #235, is this still somehow valid, and would you like to migrate this to the new CI?

thbar · 2022-03-30T12:51:59Z

Related: #226

pkuczynski · 2022-05-09T21:18:45Z

Ping @gildegoma...

gildegoma requested a review from a team as a code owner August 1, 2020 15:43

mergeable bot assigned gildegoma Aug 1, 2020

gildegoma requested a review from danochoa August 1, 2020 15:44

pkuczynski requested a review from thbar August 1, 2020 16:34

pkuczynski added the enhancement label Aug 1, 2020

danochoa reviewed Aug 1, 2020

View reviewed changes

gildegoma force-pushed the ci-check-mode branch from 181c558 to bb22033 Compare August 2, 2020 17:36

thbar requested changes Aug 3, 2020

View reviewed changes

tasks/rubies.yml Show resolved Hide resolved

gildegoma marked this pull request as draft August 5, 2020 09:32

Travis CI: Verify that Ansible check mode passes

0a1aa09

For sake of CI speed, the check mode is verified only against one container (the latest version of each operating system platform). Such check is performed before, and after, the Ansible role has been applied to the managed instance.

gildegoma force-pushed the ci-check-mode branch from bb22033 to 0a1aa09 Compare August 19, 2020 04:41

Fix Check Mode

c999c43

Skip all the 'rubies.yml' tasks if RVM is not installed yet.

gildegoma marked this pull request as ready for review August 19, 2020 04:50

danochoa approved these changes Aug 19, 2020

View reviewed changes

gildegoma mentioned this pull request Aug 21, 2020

Add rvm1_delete_unmanaged_rubies option (and deprecate rvm1_delete_ruby) #222

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Travis CI: Verify that "Ansible --check mode" passes #220

Travis CI: Verify that "Ansible --check mode" passes #220

gildegoma commented Aug 1, 2020

pkuczynski commented Aug 1, 2020

gildegoma commented Aug 1, 2020 •

edited

danochoa Aug 1, 2020

gildegoma Aug 1, 2020

danochoa Aug 1, 2020

danochoa Aug 1, 2020

gildegoma Aug 2, 2020 •

edited

gildegoma Aug 19, 2020

danochoa Aug 19, 2020

thbar left a comment

thbar commented Aug 18, 2020

gildegoma commented Aug 19, 2020

danochoa left a comment

danochoa Aug 19, 2020

gildegoma Aug 19, 2020

danochoa Aug 19, 2020

danochoa Aug 19, 2020

gildegoma commented Aug 19, 2020

pkuczynski commented Mar 25, 2022 •

edited

thbar commented Mar 26, 2022

pkuczynski commented Mar 26, 2022

thbar commented Mar 30, 2022

pkuczynski commented May 9, 2022

Travis CI: Verify that "Ansible --check mode" passes #220

Are you sure you want to change the base?

Travis CI: Verify that "Ansible --check mode" passes #220

Conversation

gildegoma commented Aug 1, 2020

pkuczynski commented Aug 1, 2020

gildegoma commented Aug 1, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gildegoma Aug 2, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thbar left a comment

Choose a reason for hiding this comment

thbar commented Aug 18, 2020

gildegoma commented Aug 19, 2020

danochoa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gildegoma commented Aug 19, 2020

pkuczynski commented Mar 25, 2022 • edited

thbar commented Mar 26, 2022

pkuczynski commented Mar 26, 2022

thbar commented Mar 30, 2022

pkuczynski commented May 9, 2022

gildegoma commented Aug 1, 2020 •

edited

gildegoma Aug 2, 2020 •

edited

pkuczynski commented Mar 25, 2022 •

edited