-
Notifications
You must be signed in to change notification settings - Fork 452
fix(ci): perform update of ubuntu packages earlier #13177
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
Conversation
Warning Rate limit exceeded@eqqe has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 53 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe changes reorder the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Hey @eqqe - I've reviewed your changes - here's some feedback:
- The repeated apt-get update and install blocks across workflows could be extracted into a reusable workflow or composite action to reduce duplication and ease maintenance.
- Consider adding the -y flag to the criu dependency apt-get install calls to ensure non-interactive installs and avoid potential prompts in CI.
- Since apt-key is deprecated, migrating to placing the repository GPG key in /etc/apt/trusted.gpg.d could improve compatibility with future Ubuntu releases.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The repeated apt-get update and install blocks across workflows could be extracted into a reusable workflow or composite action to reduce duplication and ease maintenance.
- Consider adding the -y flag to the criu dependency apt-get install calls to ensure non-interactive installs and avoid potential prompts in CI.
- Since apt-key is deprecated, migrating to placing the repository GPG key in /etc/apt/trusted.gpg.d could improve compatibility with future Ubuntu releases.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.github/workflows/e2e-main.yaml (1)
111-112
: Trailing whitespace – YAML-lint errorThe two blank lines after
podman version
(lines 111-112) contain stray spaces, triggering the YAML-linttrailing-spaces
rule.
Simply delete the spaces or the blank lines entirely..github/workflows/e2e-kubernetes-main.yaml (1)
97-98
: Trailing whitespace – YAML-lint errorLine 97 (the blank line after
podman version
) carries extra spaces and is flagged by YAML-lint. Please remove the trailing spaces..github/workflows/pr-check.yaml (1)
329-332
: Minor: redundantecho
/blank line with spacesLine 341 (just before the Podman version echo) has trailing spaces reported by YAML-lint. Clean-up keeps the workflow files tidy and lint-clean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/e2e-kubernetes-main.yaml
(1 hunks).github/workflows/e2e-main.yaml
(1 hunks).github/workflows/pr-check.yaml
(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/e2e-kubernetes-main.yaml
[error] 97-97: trailing spaces
(trailing-spaces)
.github/workflows/e2e-main.yaml
[error] 111-111: trailing spaces
(trailing-spaces)
.github/workflows/pr-check.yaml
[error] 341-341: trailing spaces
(trailing-spaces)
[error] 344-344: trailing spaces
(trailing-spaces)
.github/workflows/e2e-main.yaml
Outdated
PODMAN_VERSION=$(podman version | grep 'Version:' | awk '{print $2}') | ||
if [[ "$PODMAN_VERSION" == 5.* ]]; then | ||
echo "Podman major version is 5. Proceeding with the workflow." | ||
else | ||
echo "Error: Podman major version is not 5. Found: $PODMAN_VERSION" | ||
exit 1 # Fail the step if the condition is not met |
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 fi
terminator breaks the shell block
The if [[ "$PODMAN_VERSION" == 5.* ]]; then … else …
construct is not closed with a corresponding fi
.
As-is, the step will abort with a “syntax error: unexpected end of file”, failing the whole job.
echo "Error: Podman major version is not 5. Found: $PODMAN_VERSION"
exit 1 # Fail the step if the condition is not met
+ fi
🤖 Prompt for AI Agents
In .github/workflows/e2e-main.yaml around lines 113 to 118, the shell if-else
block checking the Podman version is missing the closing 'fi' statement. To fix
this, add a 'fi' line after the 'exit 1' command to properly close the if-else
construct and prevent the syntax error.
PODMAN_VERSION=$(podman version | grep 'Version:' | awk '{print $2}') | ||
if [[ "$PODMAN_VERSION" == 5.* ]]; then | ||
echo "Podman major version is 5. Proceeding with the workflow." | ||
else | ||
echo "Error: Podman major version is not 5. Found: $PODMAN_VERSION" | ||
exit 1 # Fail the step if the condition is not met |
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.
Same fi
omission here
Identical to the e2e-main workflow, the if
block performing the Podman version check is missing its terminating fi
, which will cause the step to fail.
echo "Error: Podman major version is not 5. Found: $PODMAN_VERSION"
exit 1 # Fail the step if the condition is not met
+ fi
🤖 Prompt for AI Agents
In .github/workflows/e2e-kubernetes-main.yaml around lines 99 to 104, the if
statement checking the Podman version is missing its closing fi. Add the missing
fi at the end of the if block to properly terminate the conditional and prevent
the step from failing.
Signed-off-by: Simon Rey <sfbrey+eqqe@gmail.com>
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.
LGTM
@Mergifyio backport 1.20.x |
✅ Backports have been created
|
Signed-off-by: Simon Rey <sfbrey+eqqe@gmail.com> (cherry picked from commit 5b57b04)
What does this PR do?
Fixed problem with packages installation on ubuntu.
Screenshot / video of UI
What issues does this PR fix or reference?
[#13174 ](#13174)
How to test this PR?