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

Dedup tests #2693

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

Dedup tests #2693

wants to merge 8 commits into from

Conversation

acozine
Copy link
Contributor

@acozine acozine commented Jan 27, 2022

First attempt / first steps toward streamlining Molecule tests as discussed in #2685.

This branch uses the openjdk role as a test case, because it's small, has only one dependency, and is used in several playbooks. This branch:

  • removes the dependency on the common role from the openjdk/meta/main.yml file
  • explicitly adds the common role to every playbook that calls the openjdk role
  • explicitly adds the common role to the tests for the openjdk role

@acozine
Copy link
Contributor Author

acozine commented Jan 28, 2022

Note to Future Me:
The openjdk role listed a dependency on common.
The saxon role listed a dependency on openjdk, with no other depedencies.
When I deleted the dependency in the openjdk role, the saxon role tests failed with a missing package failure.

I'm betting this means that the saxon tests need to run the common role, and did not. So I've added a dependency on common to the saxon role (and also to the zookeeper role, which had the same single dependency on openjdk). If both pass CI, I will understand more about how all this fits together.

roles/openjdk/meta/main.yml Outdated Show resolved Hide resolved
@acozine
Copy link
Contributor Author

acozine commented Feb 1, 2022

I updated the ADR to reflect the agreement we would be making if/when we merge this PR. The change is still under discussion, though!

@acozine
Copy link
Contributor Author

acozine commented Feb 4, 2022

@carolyncole is concerned about the maintenance burden in playbooks, and specifically about the possibility of drift between staging and production playbooks. This led to a conversation about the way the repo currently handles groups in the hosts file, group_vars, role variables, and playbooks. Can we have a single playbook per project, with both the target hosts and the variables determined at runtime (for example, something like ansible-playbook playbooks/pulmap.yml -e env=prod and ansible-playbook playbooks/pulmap.yml -e env=staging instead of separate playbooks pulmap_production.yml and pulmap_staging.yml? This would reduce both maintenance and drift.

@hackartisan
Copy link
Member

providing the stage at playbook runtime seems fine to me.

@acozine
Copy link
Contributor Author

acozine commented Apr 19, 2022

Recent changes to our CI have made improvements to this problem - see https://github.com/pulibrary/princeton_ansible/runs/6040136600?check_suite_focus=true for a run in which the common role was NOT run multiple times!!!

@hackartisan
Copy link
Member

Recent changes to our CI have made improvements to this problem

What were the changes?

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

Successfully merging this pull request may close these issues.

5 participants