ci: only install podman and crun for podman5 install task - #2453
ci: only install podman and crun for podman5 install task#2453olliewalsh wants to merge 2 commits into
Conversation
apt-get upgrade was upgrading a number of unncessary packages. Switch to apt-get install instead. Signed-off-by: Oliver Walsh <owalsh@redhat.com>
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdate CI workflow to avoid unnecessary package upgrades by installing only required dependencies (including podman/crun) and consistently purging Firefox to free space before installs. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
@mikebonnet PTAL |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider adding
-yto theapt-get purge firefoxcommands (as you already do for someapt-get installcalls) to avoid any chance of interactive prompts blocking CI jobs. - Since
firefoxis now purged in multiple jobs, you might factor this into a shared reusable step or composite action to keep the workflow DRY and make future changes to that behavior easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider adding `-y` to the `apt-get purge firefox` commands (as you already do for some `apt-get install` calls) to avoid any chance of interactive prompts blocking CI jobs.
- Since `firefox` is now purged in multiple jobs, you might factor this into a shared reusable step or composite action to keep the workflow DRY and make future changes to that behavior easier.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
e2e CI is failing when trying to run a local registry. One of the other package upgrades appears to be significant but not obvious which/why. Setting the PR to draft for now... |
|
@olliewalsh It's not an issue with this PR. That test is failing everywhere. I was working on this yesterday but can't reproduce it locally. It seems like there's an issue with podman not finding the built imageid in the CI job. |
It's passing on main, something about this change seems to be triggering the issue. |
|
shrug I hit this exact test failure, with the exact same traceback (modulo the imageid) on two different PRs related to the inference spec ctx size yesterday. I'm skeptical three different PRs covering unrelated functional capabilities all triggered the exact same error in the exact same way but, who knows, maybe so. |
|
A friendly reminder that this PR had no activity for 30 days. |
|
A friendly reminder that this PR had no activity for 30 days. |
apt-get upgrade was upgrading a number of unncessary packages. Switch to apt-get install instead.
Reduces the podman5 install step to ~40s. Previously this would take at least 3-4mins but in some cases could take up to 30mins.
Summary by Sourcery
Adjust CI workflows to avoid unnecessary package upgrades and explicitly manage Firefox removal and Podman installation.
CI: