CNF-18836 / CNF-20367: Reconfigure repo to allow mintmaker rpm updates to work with ubi9 images#716
Conversation
|
@fontivan: This pull request references CNF-20367 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughExternalizes builder/runtime image refs into Changes
Sequence Diagram(s)sequenceDiagram
participant Makefile
participant Config as .konflux/container_build_args.conf
participant Tmp as lock-*/tmp/
participant RpmLock as rpm-lock
participant LockYAML as rpms.lock.yaml
Note over Makefile,Config: Read image refs and params
Makefile->>Config: read BUILDER_IMAGE / RUNTIME_IMAGE
Makefile->>Tmp: copy rpms.in.yaml -> tmp/
alt substitutions required
Makefile->>Tmp: sed substitute $SSL_* placeholders
end
Makefile->>RpmLock: run rpm-lock with LOCK_SCRIPT_TARGET_DIR=tmp
RpmLock->>Tmp: produce rpms.lock.yaml (varsFromImage applied)
Tmp->>Makefile: rpms.lock.yaml
Makefile->>LockYAML: copy tmp/rpms.lock.yaml -> final location
Note right of LockYAML: .lock files updated to use varsFromImage and UBI CDN where applicable
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
fontivan
force-pushed
the
sskeard/cnf-20367-cnf-18836-images-overhaul
branch
from
1f2fb24 to
b862ed9
Compare
|
/hold will require several approvals |
|
@fontivan: This pull request references CNF-20367 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Related: openshift-kni/lifecycle-agent#3289 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.konflux/lock-runtime/rpms.in.yaml (1)
20-20: Verify hard-coded varsFromImage SHA remains current.The varsFromImage SHA is hard-coded identically across all six repository entries. If the ubi9/ubi-minimal image is updated, this SHA will need manual updates in multiple locations. Consider whether this should be managed externally (e.g., via container_build_args.conf or environment variable) to reduce maintenance burden.
Also applies to: 33-33, 46-46, 59-59, 72-72, 85-85
.konflux/lock-build/rpms.in.yaml (1)
26-26: Consider automating image reference injection to reduce duplication.The BUILDER_IMAGE reference is hardcoded identically in 6 places. If the image in
container_build_args.confis updated, all 6 instances must be manually updated, risking inconsistency.Consider one of these approaches:
Use the Makefile to inject the value - Since the Makefile already copies
rpms.in.yamlto tmp and performs sed substitutions (lines 97-103), extend it to replace a placeholder like$BUILDER_IMAGEwith the actual value fromcontainer_build_args.conf.Template-based generation - If the rpm-lock tooling supports it, use a template syntax or variable reference that gets resolved at lock time.
Example Makefile approach:
@echo "Copying rpms.build.in.yaml to lock-build/tmp/ directory..." cp $(PROJECT_DIR)/.konflux/lock-build/rpms.in.yaml $(PROJECT_DIR)/.konflux/lock-build/tmp/rpms.in.yaml @echo "Replacing SSL_CLIENT_KEY and SSL_CLIENT_CERT in lock-build/tmp/rpms.in.yaml..." + @echo "Injecting BUILDER_IMAGE from container_build_args.conf..." + BUILDER_IMAGE=$$(awk -F'=' '/^BUILDER_IMAGE=/ {print $$2}' $(PROJECT_DIR)/.konflux/container_build_args.conf); \ if [ "$$(uname)" = "Darwin" ]; then \ sed -i '' 's|sslclientkey: $$SSL_CLIENT_KEY|sslclientkey: /etc/pki/entitlement/placeholder-key.pem|g' $(PROJECT_DIR)/.konflux/lock-build/tmp/rpms.in.yaml; \ sed -i '' 's|sslclientcert: $$SSL_CLIENT_CERT|sslclientcert: /etc/pki/entitlement/placeholder.pem|g' $(PROJECT_DIR)/.konflux/lock-build/tmp/rpms.in.yaml; \ + sed -i '' "s|varsFromImage: \$$BUILDER_IMAGE_PLACEHOLDER|varsFromImage: $$BUILDER_IMAGE|g" $(PROJECT_DIR)/.konflux/lock-build/tmp/rpms.in.yaml; \ else \ sed -i 's|sslclientkey: $$SSL_CLIENT_KEY|sslclientkey: /etc/pki/entitlement/placeholder-key.pem|g' $(PROJECT_DIR)/.konflux/lock-build/tmp/rpms.in.yaml; \ sed -i 's|sslclientcert: $$SSL_CLIENT_CERT|sslclientcert: /etc/pki/entitlement/placeholder.pem|g' $(PROJECT_DIR)/.konflux/lock-build/tmp/rpms.in.yaml; \ + sed -i "s|varsFromImage: \$$BUILDER_IMAGE_PLACEHOLDER|varsFromImage: $$BUILDER_IMAGE|g" $(PROJECT_DIR)/.konflux/lock-build/tmp/rpms.in.yaml; \ fiThen in the YAML file, use:
varsFromImage: $BUILDER_IMAGE_PLACEHOLDERAlso applies to: 45-45, 64-64, 83-83, 102-102, 121-121
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.konflux/Dockerfile(2 hunks).konflux/container_build_args.conf(1 hunks).konflux/lock-build/.gitignore(1 hunks).konflux/lock-build/rpms.in.yaml(6 hunks).konflux/lock-build/rpms.lock.yaml(2 hunks).konflux/lock-runtime/.gitignore(1 hunks).konflux/lock-runtime/rpms.in.yaml(1 hunks).konflux/lock-runtime/rpms.lock.yaml(2 hunks).tekton/recert-4-21-pull-request.yaml(1 hunks).tekton/recert-4-21-push.yaml(1 hunks)Makefile(2 hunks)
⏰ 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
- GitHub Check: Red Hat Konflux / recert-4-21-on-pull-request
🔇 Additional comments (15)
.konflux/lock-runtime/.gitignore (1)
1-1: Aligns with tmp/ folder consolidation.The change from specific file ignores to a single
tmp/directory ignore is consistent with the PR's goal to move podman scripts into temporary directories. This simplifies the gitignore and centralizes artifact management..konflux/lock-build/rpms.lock.yaml (1)
35-41: Routine kernel-headers patch update across all architectures.The kernel-headers update from 5.14.0-570.60.1.el9_6 to 5.14.0-570.62.1.el9_6 is applied consistently to both aarch64 and x86_64 entries with matching checksums and file sizes reflecting the version bump.
Also applies to: 221-227
.konflux/lock-runtime/rpms.lock.yaml (2)
98-104:⚠️ Cross-architecture package version mismatch for openssl and openssl-libs.The aarch64 runtime includes openssl 1:3.5.1-3.el9 (lines 98–104), while x86_64 includes 1:3.5.1-4.el9_7 (lines 221–227) and openssl-libs 1:3.5.1-4.el9_7 (lines 228–234). This inconsistency between architectures could lead to unexpected behavior at runtime or during multi-arch builds. Verify whether this is intentional or if both should use the same version.
Also applies to: 221-234
7-127: UBI repository migration is systematic and consistent.The shift from RHEL CDN to UBI CDN with corresponding repoid updates (rhel-9-* → ubi-9-*) is applied uniformly across all packages and both architectures. Package selections for a minimal runtime environment are appropriate.
Also applies to: 130-255
.konflux/lock-runtime/rpms.in.yaml (1)
9-86: Systematic UBI repository migration with centralized image sourcing.All six repository entries (appstream, appstream-eus, baseos, baseos-eus, codeready-builder, codeready-builder-eus) have been migrated to UBI equivalents with corresponding repoid and baseurl updates. The addition of
varsFromImageacross all repos enables image-based sourcing, supported by helpful comments referencing container_build_args.conf..konflux/lock-build/.gitignore (1)
1-1: Consistent tmp/ directory consolidation.Mirrors the change to
lock-runtime/.gitignore, maintaining uniform gitignore patterns across lock directories for managing temporary build artifacts..tekton/recert-4-21-push.yaml (1)
59-64: Unified pipeline configuration across push and pull-request workflows.The push pipeline now mirrors pull-request changes with
build-args-fileparameter and trailing-slash prefetch paths, ensuring consistent build argument sourcing and path handling across both workflow triggers.Same verification as pull-request file: confirm
.konflux/container_build_args.confexists with required variables..konflux/Dockerfile (3)
2-3: Image references introduced via ARG with sensible defaults.The Dockerfile now accepts configurable
BUILDER_IMAGEandRUNTIME_IMAGEarguments with appropriate defaults (RHEL 9.6 for build toolchain, UBI 9 minimal for runtime), enabling external override via build-args-file.
6-6: Builder stage rename is consistent across FROM and COPY instructions.The rename from
build-imagetobuilder-imageis correctly applied to both the stage definition (line 6) and the COPY instruction (line 27), maintaining internal consistency.Also applies to: 27-27
2-3: The build configuration correctly addresses the concern.Verification confirms that
.konflux/container_build_args.confoverrides the Dockerfile's:latestdefault with the pinned SHA (sha256:53ea1f6d835898acda5becdb3f8b1292038a480384bbcf994fc0bcf1f7e8eaf7), which matches exactly with the varsFromImage SHA inrpms.in.yaml. The build will use the pinned reference, preventing image drift..tekton/recert-4-21-pull-request.yaml (1)
61-66: Verified: build-args-file parameter and container_build_args.conf are properly configured.The
.konflux/container_build_args.conffile exists and contains the expectedBUILDER_IMAGEandRUNTIME_IMAGEvariables. The newbuild-args-fileparameter correctly references this external configuration, enabling ARG sourcing for the Dockerfile. Prefetch paths with trailing slashes are appropriately aligned with tooling requirements. No issues identified..konflux/container_build_args.conf (1)
1-13: LGTM! Good use of SHA256 digests for reproducible builds.The configuration file is well-structured with clear comments linking to the related YAML files. The use of SHA256 digests for image pinning ensures reproducible builds.
Makefile (2)
75-88: LGTM! Temporary directory approach aids debugging.The logic correctly extracts image references from the config file and maintains the tmp directory for debugging purposes, which is helpful for troubleshooting rpm-lock issues.
92-118: LGTM! Proper handling of cross-platform sed differences.The target correctly handles macOS vs Linux sed syntax differences and follows the same pattern as the runtime lock target. The sed substitutions for SSL certificates enable local development while maintaining Konflux compatibility.
.konflux/lock-build/rpms.in.yaml (1)
17-20: LGTM! Environment-driven SSL configuration improves flexibility.The change from static paths to environment variables enables the same configuration to work in both Konflux (via environment) and local development (via Makefile sed substitution).
Also applies to: 36-39, 55-58, 74-77, 93-96, 112-115
|
/lgtm |
fontivan
force-pushed
the
sskeard/cnf-20367-cnf-18836-images-overhaul
branch
from
b862ed9 to
baa317e
Compare
|
@fontivan: This pull request references CNF-20367 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.konflux/lock-runtime/rpms.in.yaml (1)
19-20: HardcodedvarsFromImagedigest may diverge fromcontainer_build_args.conf.The comments correctly note that
varsFromImageshould matchRUNTIME_IMAGEincontainer_build_args.conf, but the values are hardcoded. If the image reference changes incontainer_build_args.conf, this file won't update automatically, creating a maintenance burden and potential inconsistency.Consider whether
varsFromImageshould be templated or derived fromcontainer_build_args.confat build time, rather than hardcoded. This would ensure the two stay synchronized.Also applies to: 32-33, 45-46, 58-59, 71-72, 84-85
.konflux/lock-build/rpms.in.yaml (1)
25-26: HardcodedvarsFromImagedigests may diverge fromcontainer_build_args.conf.All six repository entries reference a hardcoded RHEL builder image digest (
registry.redhat.io/rhel9-6-els/rhel:9.6@sha256:5f284a072ae41d6db952300c5e4aa1bb2410960075e158f81fc9c552f9d6d7e1), but comments indicate these should matchBUILDER_IMAGEincontainer_build_args.conf. Like the runtime file, hardcoding creates a maintenance burden if the image reference changes.Consider templating
varsFromImageentries to derive fromcontainer_build_args.confat build time, ensuring consistency with theBUILDER_IMAGEvalue and reducing manual synchronization burden.Also applies to: 44-45, 63-64, 82-83, 101-102, 120-121
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.konflux/Dockerfile(2 hunks).konflux/container_build_args.conf(1 hunks).konflux/lock-build/.gitignore(1 hunks).konflux/lock-build/rpms.in.yaml(6 hunks).konflux/lock-build/rpms.lock.yaml(2 hunks).konflux/lock-runtime/.gitignore(1 hunks).konflux/lock-runtime/rpms.in.yaml(1 hunks).konflux/lock-runtime/rpms.lock.yaml(2 hunks).tekton/recert-4-21-pull-request.yaml(1 hunks).tekton/recert-4-21-push.yaml(1 hunks)Makefile(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- .konflux/container_build_args.conf
🚧 Files skipped from review as they are similar to previous changes (5)
- .konflux/lock-runtime/.gitignore
- .tekton/recert-4-21-push.yaml
- .tekton/recert-4-21-pull-request.yaml
- .konflux/Dockerfile
- .konflux/lock-build/.gitignore
⏰ 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: Red Hat Konflux / recert-4-21-on-pull-request
- GitHub Check: build
🔇 Additional comments (7)
.konflux/lock-build/rpms.lock.yaml (1)
35-41: Kernel-headers version bumps look reasonable.The lock file updates reflect a minor patch version increment (5.14.0-570.60.1 → 5.14.0-570.62.1) with consistent metadata changes across both architectures. No issues detected.
Also applies to: 221-227
.konflux/lock-runtime/rpms.lock.yaml (1)
7-132: UBI migration appears systematic and consistent across architectures.The lock file reflects a migration from RHEL CDN to UBI CDN sources with corresponding package updates. Changes are symmetric across aarch64 and x86_64. Package version increments (e.g., openssh 8.7p1-45 → 8.7p1-46, openssl 3.5.1-4.el9_6 → 3.5.1-4.el9_7) appear intentional for UBI9 alignment.
Please verify that UBI9 package versions don't introduce any breaking changes or incompatibilities with recert's runtime requirements, particularly for openssh and openssl.
Also applies to: 137-262
Makefile (4)
32-32: RHEL9_IMAGE extraction now correctly uses KEY=VALUE format.The awk pattern has been updated from a Dockerfile
FROMline pattern to matchKEY=VALUEformat. This correctly extracts the BUILDER_IMAGE value.
74-87: Runtime lock generation creates tmp directory and properly manages workflow.The process correctly creates a tmp directory, copies the input YAML, runs rpm-lock with proper arguments, and copies the result back. The tmp directory is intentionally preserved for debugging. Image references are correctly derived from
container_build_args.confwith appropriate transformations (ubi-minimal→ubi, digest removal for execution image).
28-28: Awk pattern verified—extraction works correctly.The verification confirms that the awk pattern successfully extracts the RHEL version from
container_build_args.conf. The actual configuration format matches the pattern expectations, and field 3 extraction correctly yields the version number (9.6).
91-117: Verify the Darwin sed condition works correctly on macOS.The OS-specific sed handling is syntactically correct:
- macOS:
sed -i ''requires an empty suffix for in-place editing- Linux:
sed -irequires no suffix with GNU sedHowever, this can only be fully verified by testing on macOS. Ensure the Darwin branch executes successfully with the exact sed commands shown when you test on macOS hardware or a macOS CI runner.
.konflux/lock-build/rpms.in.yaml (1)
17-20: SSL certificate configuration uses placeholder substitution strategy for local builds.The configuration uses environment variables (
$SSL_CLIENT_KEY,$SSL_CLIENT_CERT) that will be replaced by Makefilesedcommands (lines 96-102 in Makefile) during localmakeinvocation, while remaining as environment variables for Konflux cluster execution. This dual-mode approach is well-documented with clear comments.
fontivan
force-pushed
the
sskeard/cnf-20367-cnf-18836-images-overhaul
branch
from
baa317e to
672325d
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fontivan, rauhersu The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
|
/unhold |
…s to work with ubi9 images
- Move container pins into container_build_args.conf file
- Update container_build_args.conf to use the new ubi9 image for runtime
- Update Makefile to parse the new images correctly and update the targets invoking the rpm-lock tooling
- Update runtime rpms.in.yaml
- Remove now unnecessary ssl configuration
- Remove context on Dockerfile
- Add varsFromImage with reference to the same image in container_build_args.conf
- Update build rpms.in.yaml
- Update ssl configuration to work for both mintmaker and local target
- Remove context on Dockerfile
- Add varsFromImage with reference to the same image in container_build_args.conf
- Update rpms.lock.yaml files with outputs from the rpm-lock script
- Move the podman scripts to run inside a tmp/ folder inside their lock folder
Assisted-by: Cursor/claude-4-sonnet
AI-attribution: AIA,Primarily human-created,Human-initiated,Reviewed,Cursor/claude-4-sonnet,v1.0
For more information on AI attribution statements, see: https://aiattribution.github.io/
fontivan
force-pushed
the
sskeard/cnf-20367-cnf-18836-images-overhaul
branch
from
672325d to
d0027a9
Compare
|
New changes are detected. LGTM label has been removed. |
|
@fontivan: This pull request references CNF-20367 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.konflux/lock-runtime/rpms.in.yaml (1)
9-92: Multiple identical varsFromImage values - consider the maintenance burden.All six repository entries have the identical
varsFromImagehash pointing to the UBI minimal image. While this is explicit and maintains clarity, maintaining this duplication across multiple entries could become error-prone if the image ever needs to be updated. The comments help flag this need for manual synchronization withcontainer_build_args.conf, but verify that Mintmaker's automated update process will correctly update all six entries simultaneously..konflux/lock-build/rpms.in.yaml (2)
17-20: Environment variable syntax in YAML is fragile; ensure sed substitution is bulletproof.Using
$SSL_CLIENT_KEYsyntax in YAML is unconventional. While the approach of using sed to substitute these values locally (before rpm-lock) works, ensure:
- The sed patterns in the Makefile (lines 93-97) correctly match the
$prefix- Escape characters are handled properly across both Linux and macOS (Darwin)
- In Konflux environment, these variables are actually provided at runtime
Consider whether using placeholder paths directly for local builds and storing actual credentials in Tekton secrets would be cleaner than relying on sed substitution.
25-27: Maintain consistency: Six identical varsFromImage entries should be kept in sync carefully.Like the runtime rpms.in.yaml, all six repository entries have identical
varsFromImagehashes. Ensure Mintmaker's automated tooling updates all six entries atomically when the BUILDER_IMAGE is updated, otherwise drift could cause subtle lock file inconsistencies.Also applies to: 45-47, 65-67, 85-87, 105-107, 125-127
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.konflux/Dockerfile(2 hunks).konflux/container_build_args.conf(1 hunks).konflux/lock-build/.gitignore(1 hunks).konflux/lock-build/rpms.in.yaml(6 hunks).konflux/lock-build/rpms.lock.yaml(2 hunks).konflux/lock-runtime/.gitignore(1 hunks).konflux/lock-runtime/rpms.in.yaml(1 hunks).konflux/lock-runtime/rpms.lock.yaml(2 hunks).tekton/recert-4-21-pull-request.yaml(1 hunks).tekton/recert-4-21-push.yaml(1 hunks)Makefile(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- .konflux/lock-runtime/.gitignore
- .tekton/recert-4-21-pull-request.yaml
- .konflux/container_build_args.conf
- .tekton/recert-4-21-push.yaml
- .konflux/lock-build/.gitignore
⏰ 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: Red Hat Konflux / recert-4-21-on-pull-request
- GitHub Check: build
🔇 Additional comments (7)
.konflux/Dockerfile (2)
2-3: ARG defaults are reasonable, but verify BUILD_ARGS_FILE integration in Tekton tasks.The ARG declarations with sensible defaults are correct. Ensure the Tekton pipeline tasks pass
BUILD_ARGS_FILEpointing to.konflux/container_build_args.confso these defaults are overridden at build time, otherwise the hardcoded defaults will be used.
6-6: Multi-stage build refactoring looks good.Renaming
build-imagetobuilder-imageand updating all references is consistent. The addition ofCOPY . .in the build stage ensures source files are available.Also applies to: 19-19, 27-27
.konflux/lock-build/rpms.lock.yaml (1)
35-41: kernel-headers package updates expected for lock file.Lock file updates with new kernel-headers versions (570.62.1.el9_6) appear to be expected regeneration as part of the rpm-lock workflow changes.
Also applies to: 221-227
.konflux/lock-runtime/rpms.lock.yaml (1)
7-262: UBI-based runtime lock file regeneration looks correct.Package migrations from RHEL CDN to UBI CDN repositories and updated package versions (particularly openssh and openssl updates) are expected outcomes of the rpm-lock regeneration workflow. The structure and format are correct.
Makefile (3)
70-82: The.konflux/container_build_args.conffile is present and properly configured.Verification confirms:
- ✓ File exists at
.konflux/container_build_args.confand is included in this PR- ✓ Contains both
BUILDER_IMAGE=andRUNTIME_IMAGE=entries inKEY=VALUEformat- ✓ Both values are complete image references with SHA256 digest hashes
The Makefile target
konflux-update-rpm-lock-runtimewill function correctly. The file prerequisites cited in the review comment are already satisfied.Likely an incorrect or invalid review comment.
86-113: Sed replacement logic verified as correct.The sed patterns have been tested and verified:
- Linux replacement works correctly, successfully replacing both
$SSL_CLIENT_KEYand$SSL_CLIENT_CERTwith their placeholder paths- macOS sed syntax (
-i ''flag) is correct per standard macOS conventions (the empty string argument tells sed to perform in-place edits without creating backup files)- The escape sequences (
\$) properly match the literal dollar signs in the YAML valuesThe Makefile conditional logic appropriately handles platform differences, and the sed replacements will function as intended on both Darwin and Linux systems.
28-28: Code is correct; no fixes required.The review comment's core concern is refuted. The awk pattern with
-F'[=:@]'correctly extracts the version number9.6even when the BUILDER_IMAGE contains a digest suffix (@sha256:...). The field separator:and@properly delimit the version string, placing it in field 3 as intended. Testing with the actual container_build_args.conf file confirms the current Makefile implementation works correctly.Likely an incorrect or invalid review comment.
|
/retest |
|
@fontivan: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/cherry-pick release-4.20 |
|
/cherry-pick release-4.19 |
|
/cherry-pick release-4.18 |
|
/cherry-pick release-4.17 |
|
/cherry-pick release-4.16 |
|
/cherry-pick release-4.14 |
|
@fontivan: new pull request created: #717 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@fontivan: #716 failed to apply on top of branch "release-4.19": DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@fontivan: #716 failed to apply on top of branch "release-4.18": DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@fontivan: #716 failed to apply on top of branch "release-4.17": DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@fontivan: #716 failed to apply on top of branch "release-4.16": DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@fontivan: #716 failed to apply on top of branch "release-4.14": DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
5 participants
Assisted-by: Cursor/claude-4-sonnet
AI-attribution: AIA,Primarily human-created,Human-initiated,Reviewed,Cursor/claude-4-sonnet,v1.0
For more information on AI attribution statements, see: https://aiattribution.github.io/
Summary by CodeRabbit
New Features
Updates
Chores