-
Notifications
You must be signed in to change notification settings - Fork 1
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
Use MIRSG molecule-test
#5
Conversation
This workflow uses the mirsg molecule-test action (requires version 0.37+)
Requires version 0.38+ of the molecule-test action
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems a bit odd to use a reusable workflow to call a composite workflow? Perfectly valid, but is it needed?
@@ -11,7 +11,7 @@ jobs: | |||
linting: | |||
runs-on: ubuntu-latest | |||
steps: | |||
- uses: UCL-MIRSG/.github/actions/linting@v0.36.0 | |||
- uses: UCL-MIRSG/.github/actions/linting@v0.38.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One day Renovate will come back 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can only dream
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dreams do come true - I just reenabled it...
I put it on the shared calendar to reenable, but I still forgot...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was scared to re-enable it haha
.github/workflows/molecule.yml
Outdated
scenario: ${{ matrix.scenario }} | ||
checkout_path: ansible_collections/mirsg/infrastructure | ||
tests_path: ansible_collections/mirsg/infrastructure/tests | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing new line at the end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think new line was there but it wasn't empty 😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, just saw the dreaded 🚫
fail-fast: true | ||
matrix: | ||
scenario: | ||
- centos7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only centos7
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah for now, I'll add Rocky 9 in a separate pr
.github/workflows/molecule.yml
Outdated
on: | ||
workflow_call: | ||
inputs: | ||
run_tags: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally people use run-tags
rather than run_tags
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've renamed it now
Yeah it is a little convoluted. The |
Happy with whatever you decide. If it's a duplication concern, I assume one composite action can depend on another (maybe)? |
Yeah we could have another action ansible-collection-infra/.github/actions/molecule-test-collection/action.yml Lines 1 to 19 in 0ca6652
And using it in a workflow: ansible-collection-infra/.github/workflows/molecule-firewalld-with-action.yml Lines 1 to 26 in 0ca6652
Compared to the current workflow for the same role: ansible-collection-infra/.github/workflows/molecule-firewalld.yml Lines 1 to 13 in 0ca6652
So using a reusable workflow allows us to avoid some boilerplate that can't be avoided when using a composite action |
.github/workflows/molecule.yml
Outdated
required: false | ||
type: string | ||
default: all |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
required
isn't required if a default
is set
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well spotted, I've removed it
Reduce duplication in workflows by:
molecule-test
GitHub Actionprovision
andfirewalld
roles (more workflows can be added in a separate pr)Note, this depends on UCL-MIRSG/.github#80 being merged first.
And linting is broken will fix in a different pr