-
Notifications
You must be signed in to change notification settings - Fork 400
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: also run integration test on Ubuntu #2492
Conversation
1./ The obvious concern with this PR is that it adds to the resource cost to the CI - specifically it runs 15 more tests for each CI run. My opinion as one of the maintainers is that a project has a lot to gain by having an active contributor from Debian/Ubuntu and if that active contributor is willing to invest in maintaining an Ubuntu CI then we should positively consider it. 2./ The somewhat less obvious concern with the extra test container is the potential churn and review/commit cost it takes to maintain it. The Ubuntu container is almost an exact copy of the Debian container. Adding a new test container is a good time to discuss how we can we better maintain all of these test containers. There is a PR under review that is trying to address this concern - #2423 |
@bdrung Can you please also add this new container to manualtest |
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.
The Ubuntu container is almost an exact copy of the Debian container.
+1
Adding a new test container is a good time to discuss how we can we better maintain all of these test containers.
I'd love to see Ubuntu running on the CI, but adding a new container implies having to support it, we already have 5, and upstream resources (talking about people) are veeery limited...
Add a Dockerfile for Ubuntu (very similar to Debian) and run the integration tests also on Ubuntu. Signed-off-by: Benjamin Drung <[email protected]>
Added ubuntu to manialtest.
Ubuntu will use dracut-install in initramfs-tools (see manual_add_modules is slow / consumes the most time). So at least dracut-install will become part of Ubuntu main (see [MIR] dracut) and part of the 1000 packages our Ubuntu Foundations team will maintain. |
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 support adding Ubuntu to the CI.
upstream resources (talking about people) are veeery limited
The way I see it, landing this PR is actually the best way to solve the resource issue. We already got many fixes for all distributions (not just for Ubuntu) from @bdrung (and this is only week 2).
I think it is good to remember that each PR is reviewed independently and this PR does not necessary imply any Ubuntu specific changes (as that would be another PR, if any).
It is also good to remember that it is very easy to reverse this decision with a follow-up PR to remove the test container - just like we did in the past - 0b339d7 .
Does this PR need to be merged before any of the Ubuntu tests go green? |
Yes, the Ubuntu tests need the Ubuntu container, which is only built from the master branch. |
..sadly.. investigating, but not very hopeful.. |
Update: this is now resolved.. here is a manual test run - https://github.com/dracutdevs/dracut/actions/runs/5927762437 |
Changes
Add a Dockerfile for Ubuntu (very similar to Debian) and run the integration tests also on Ubuntu.
I built the container locally and tested that the test cases from the integration test succeed there.
Checklist
Fixes #