-
Notifications
You must be signed in to change notification settings - Fork 289
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
Ci #1218
Ci #1218
Conversation
Any idea or guesses as to why the If I |
molecule==4.0.4 | ||
molecule-docker==2.1.0 | ||
molecule<5 | ||
molecule-docker @ git+https://github.com/ansible-community/molecule-docker@main |
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.
Is there a reason to change the source to straight to github?
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.
Yes, there's a fix that went in after 2.1.0, cb1c1c31a4a468fb243fc2000656504a3ca00bb3
that fixes a bug where the molecule.yml would get overwritten during image building. (I introduced the simple image-building to get python3 on centos7. But I think it's a good way for us to always stay current on all distribution images, instead of the geerlingguy images)
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.
Also, the repo is deprecated/archived, so there won't come any further changes to it, but I could pin it to the hash instead?
We should probably go over to molecule 5, but I'd rather get these refactors in before overhauling the new molecule version.
@@ -68,11 +68,19 @@ jobs: | |||
|
|||
- name: Build the collection | |||
run: | | |||
# Pin versions to speed up CI | |||
sed -i 's/ansible\.windows:\s*"\*"/ansible.windows: "2.3.0"/' galaxy.yml |
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.
Why do this? In my mind we should be testing against what we publish in the galaxy.yml file otherwise we may have an issue of experiencing/not experiencing failures that we should/shouldn't during CI/CD testing. Not to mention having to change versions in 5 places instead one 1.
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.
This part where galaxy looks up every single version, for every single dependency where we use *
takes multiple minutes, multiply that by the matrix of configurations we're testing in github workflows, you're looking at a couple hours just to resolve these dependencies. (Yes, it's really that stupid, I've provided links to reported issues in ce6b2da)
I think it's better we pin this for CI only, as a user could have installed a different collection that pins postgresql to X.Y.Z and if we're fine with that (given we set it to *
), galaxy will say requirements are already met and leave it alone anyway, right?
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.
Pinning in CI is not ideal, and especially since it's in this many places, but pinning in galaxy.yml would be the worse of two options.
In any case we should probably specify minimum requirements for all the *
dependencies though.
Nevermind, I think I see the error of my ways. It's not the full name as used above. |
The self-hosted GitHub actions-runner does not scale with a single runner on a "powerful" machine. But we can create multiple users on a single machine, and have each user run an actions-runner. Problem is they all share the same docker daemon. And when multiple runners try starting up a container with the same name as an existing one, the jobs fail. This is an attempt to make the container names unique enough for each job, and allows us to scale out actions-runners on a single machine.
Following up on unique names for containers, we need unique names for their corresponding database-container during testing.
Grab the latest packaged molecule 4 release. And the latest molecule-docker straight from git. There's a fix that went in after 2.1.0 (cb1c1c31a4a468fb243fc2000656504a3ca00bb3) that fixes a bug where the molecule.yml would get overwritten during image building.
We don't want to update galaxy.yml directly where others might need different versions because of requirements, but pinning it here to speed up the CI seems like a good compromise.
SUMMARY
This attempts to fix some shortcomings in the CI
ISSUE TYPE
COMPONENT NAME
molecule, github-actions
ADDITIONAL INFORMATION
It would be good if this could get merged, the change to
molecule/requirements.txt
results in a lot of unnecessary testing of other roles during my refactoring of the code.