buildah manifest add localimage should work #2955
Conversation
|
Does this reintroduce the problem that #2824 was attempting to fix? |
| return err | ||
| var storeErr error | ||
| // check if the local image exists | ||
| if ref, _, storeErr = util.FindImage(store, "", systemContext, imageSpec); storeErr != nil { |
There was a problem hiding this comment.
The local image has been checked above in line297?
https://github.com/containers/buildah/pull/2955/files#diff-f87fb1de87ea57699bf6ddd7408c420b8bd7daa0f4ef41639120453a94c52b14R297
There was a problem hiding this comment.
That check never happens since the imagespec is parsed correctly as a remote image, so we always fail if we use a local image that does not exists at a registry.
There was a problem hiding this comment.
The remote image is never checked until this call.
There was a problem hiding this comment.
That check never happens since the imagespec is parsed correctly as a remote image, so we always fail if we use a local image that does not exists at a registry.
LGTM. We could also delete check L296~L297 since they can't be reached.
There was a problem hiding this comment.
No I can reach there by attempting
./bin/buildah manifest add myimage sha256:c1876ff0a66a49ae90dbe6ac1d8c413107fec317987c4e2059cf72b430f5a023
Which is also valid.
|
@nalind I don't see how, since it will first check at the remote site. It will only check local storage if it does not exists on the remote site. |
rhatdan
force-pushed
the
list
branch
2 times, most recently
from
3868921 to
a70d2f4
Compare
Currently if you attempt to build create a manifest and add a local image, the command blows up. The current code always looks for a remote image. This PR fixes the code to use the local image if it exists. Signed-off-by: Daniel J Walsh <[email protected]>
Buildah bud --manifest XYZ was not working. The manifest was never created. This PR Finishes the plumbing and allows users to create a manifest while building an image in one single command. Signed-off-by: Daniel J Walsh <[email protected]>
| run echo hello | ||
| _EOF | ||
|
|
||
| run_buildah bud -q --manifest=testlist -t arch-test --signature-policy ${TESTSDIR}/policy.json ${mytmpdir} <<< input |
There was a problem hiding this comment.
is the <<< input at the end here correct? I'm not seeing a --stdin option.
There was a problem hiding this comment.
Yes this came from other tests. Not sure what it does.
There was a problem hiding this comment.
It must just be passing input in as input into the build command and then ignoring it.
Weird, but OK and the tests are happy.
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
LGTM |
Currently if you attempt to build create a manifest
and add a local image, the command blows up.
The current code always looks for a remote image.
This PR fixes the code to use the local image if it
exists.
Signed-off-by: Daniel J Walsh [email protected]
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?