stage_executor: check platform of cache candidates#6269
stage_executor: check platform of cache candidates#6269openshift-merge-bot[bot] merged 1 commit intocontainers:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: flouthoc The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
flouthoc
force-pushed
the
cache-canidates
branch
2 times, most recently
from
10082ac to
857c931
Compare
mtrmac
left a comment
mtrmac
left a comment
There was a problem hiding this comment.
This looks plausible from a 1-minute skim but I’m not very familiar with this subpackage.
imagebuildah/stage_executor.go
Outdated
| // Check if the cached image's platform matches the current build's target platform. | ||
| // This is critical for multi-platform builds to ensure each platform gets its own | ||
| // properly built image instead of incorrectly reusing cached images from other platforms. | ||
| imageRef, err := is.Transport.ParseStoreReference(s.executor.store, "@"+image.ID) |
There was a problem hiding this comment.
Non-blocking: NewStoreReference(store, nil, image.ID)
TomSweeneyRedHat
changed the title
|
LGTM |
nalind
left a comment
nalind
left a comment
There was a problem hiding this comment.
This should probably have getImageTypeAndHistoryAndDiffIDs() cache the config fields along with the things it already caches, and return them instead of doing a fresh lookup and parse here.
flouthoc
force-pushed
the
cache-canidates
branch
from
857c931 to
d2c92ba
Compare
|
Is this meant to catch cases like buildah build --layers --platform linux/ppc64le -f ./tests/bud/from-scratch/Containerfile2 ./tests/bud/from-scratch/
buildah build --layers -f ./tests/bud/from-scratch/Containerfile2 ./tests/bud/from-scratch/ |
|
LGTM |
If `arch` and `os` are not set, parse and use default values inorder to prevent matching against incorrect cache candidates. Addresses: containers#6269 (comment) Signed-off-by: flouthoc <[email protected]>
@nalind Added a new commit to address this. |
If `arch` and `os` are not set, parse and use default values inorder to prevent matching against incorrect cache candidates. Addresses: containers#6269 (comment) Signed-off-by: flouthoc <[email protected]>
flouthoc
force-pushed
the
cache-canidates
branch
from
2342150 to
4b10cc9
Compare
If `arch` and `os` are not set, parse and use default values inorder to prevent matching against incorrect cache candidates. Addresses: containers#6269 (comment) Signed-off-by: flouthoc <[email protected]>
flouthoc
force-pushed
the
cache-canidates
branch
from
4b10cc9 to
3c7c562
Compare
|
Ephemeral COPR build failed. @containers/packit-build please check. |
|
Squash the commits together, fix the typo in the commit title, then it'll LGTM. |
flouthoc
force-pushed
the
cache-canidates
branch
from
3c7c562 to
b1b4b42
Compare
|
@nalind Done. |
|
LGTM |
|
/lgtm |
When building images for `manifest` list using `--platform` same image is used for multiple platform if base is `scratch` , following PR adds a check to always verify `platform` of `cache` with `target`. Closes: containers/podman#18723 Signed-off-by: flouthoc <[email protected]>
flouthoc
force-pushed
the
cache-canidates
branch
from
b1b4b42 to
3502889
Compare
|
New changes are detected. LGTM label has been removed. |
|
Rebased to remove conflicts. |
openshift-merge-bot
bot
merged commit 8403fd6
into
containers:main
5 participants
When building images for
manifestlist using--platformsame image is used for multiple platform if base isscratch, following PR adds a check to always verifyplatformofcachewithtarget.Closes: containers/podman#18723
What type of PR is this?
What this PR does / why we need it:
How to verify it
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?