Skip to content

Conversation

@matysek
Copy link
Contributor

@matysek matysek commented Sep 4, 2025

Description

LCORE-637: Bundle jq and patch with the image

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement

Related Tickets & Documents

  • Related Issue #
  • Closes #

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Summary by CodeRabbit

  • Chores
    • Updated container image to use uv 0.8.15 in both build and runtime stages (was 0.8.11).
    • Added jq and patch to the runtime image so those command-line tools are available.
    • Result: Consistent package manager version across stages and additional CLI utilities present in the image.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 4, 2025

Walkthrough

Bumps uv from 0.8.11 to 0.8.15 in both builder and final stages of the Containerfile; adds a root-user step in the final stage and a microdnf install of jq and patch.

Changes

Cohort / File(s) Summary of changes
Container build / Containerfile
Containerfile
- Update uv version from 0.8.110.8.15 in builder stage
- Update uv version from 0.8.110.8.15 in final stage
- Add USER root in final stage before installing tooling
- Add RUN microdnf install -y jq patch in final stage

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “LCORE-637: Bundle jq and patch with the image” succinctly highlights the core change of adding jq and patch to the container image, directly reflecting both the raw summary and stated objectives; it is clear, specific, and avoids extraneous detail.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Poem

A rabbit hops through layered stacks,
Bumps uv up and tightens the packs.
Root wakes up to fetch a patch,
JQ in pocket, ready to scratch.
Image built — a joyful hop and snap! 🥕🐇

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0dc9522 and c7bde4e.

📒 Files selected for processing (1)
  • Containerfile (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • Containerfile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build-pr
  • GitHub Check: e2e_tests
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
Containerfile (2)

20-20: Pin uv once and avoid pip cache

Use an ARG to deduplicate the uv version across stages and prevent caching to keep layers small.

-RUN pip3.12 install "uv==0.8.15"
+RUN pip3.12 install --no-cache-dir "uv==0.8.15" && pip3.12 cache purge

Outside this hunk (suggested):

# near the top of each stage
ARG UV_VERSION=0.8.15
# ...
RUN pip3.12 install --no-cache-dir "uv==${UV_VERSION}" && pip3.12 cache purge

Please confirm 0.8.15 is still the intended uv version for this release cadence.


60-61: Final stage uv install: same no-cache + fix stale comment

  • Apply the same no-cache/purge pattern here.
  • The comment at Line 37 says “Final image without uv package manager” but uv is installed below—update it.
-RUN pip3.12 install "uv==0.8.15"
+RUN pip3.12 install --no-cache-dir "uv==0.8.15" && pip3.12 cache purge
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3c0c98b and 0dc9522.

📒 Files selected for processing (1)
  • Containerfile (2 hunks)
🧰 Additional context used
🪛 GitHub Actions: E2E Tests
Containerfile

[error] 63-63: Command 'RUN microdnf install -y --nodocs --setopt=keepcache=0 --setopt=tsflags=nodocs jq patch' failed. Exit code 1. Error: 'error: Failed to create: /var/cache/yum/metadata'.


[warning] 1-1: librhsm-WARNING: Found 0 entitlement certificates.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build-pr
🔇 Additional comments (1)
Containerfile (1)

62-63: Robustify microdnf install step
Create the missing cache directories, enable UBI 9 BaseOS/AppStream repos, disable weak deps, drop the redundant tsflags (since --nodocs is already used), and clean all caches to avoid the “Failed to create: /var/cache/yum/metadata” error.
Please verify the actual repo IDs by running microdnf repolist inside a local ubi9/python-312-minimal container and adjust the --enablerepo flags as needed.

Copy link
Contributor

@manstis manstis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Copy link
Contributor

@tisnik tisnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tisnik tisnik merged commit 68a6dd1 into lightspeed-core:main Sep 10, 2025
19 checks passed
@matysek matysek deleted the lcore-637 branch September 10, 2025 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants