Skip to content
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

podman-remote build should handle -f option properly #10550

Merged
merged 1 commit into from
Jun 9, 2021

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Jun 3, 2021

podman-remote build has to handle multiple different locations
for the Containerfile. Currently this works in local mode but not
when using podman-remote.

Fixes: #9871

Signed-off-by: Daniel J Walsh [email protected]

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 3, 2021

[APPROVALNOTIFIER] This PR is APPROVED

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 3, 2021
@rhatdan
Copy link
Member Author

rhatdan commented Jun 3, 2021

@edsantiago PTAL

@edsantiago
Copy link
Member

Almost working (in buildah-bud tests) but still some problems; I'm looking into them, will report back once I understand them.

@edsantiago
Copy link
Member

Here's one problem: multiple -f flags, all except the first are ignored:

$ mkdir aaa
$ printf "FROM scratch\nCOPY Containerfile1 /\n" >aaa/Containerfile1
$ printf "COPY Containerfile2 /\n" >aaa/Containerfile2
$ bin/podman-remote build -t foo -f aaa/Containerfile1 -f aaa/Containerfile2 aaa
STEP 1: FROM scratch
STEP 2: COPY Containerfile1 /
  !!! missing STEP 3: COPY Containerfile2 /      (works fine with bin/podman)
STEP 3: COMMIT foo
--> 7a15f1ab70b
Successfully tagged localhost/foo:latest
7a15f1ab70b3e68b8f6f25fa86a1f268636d25befe1ea026849aca613d19d4b5

This may be related to (and fixable in) your PR, or it might need a separate issue. Please LMK if I should file one.

@edsantiago
Copy link
Member

Here's another one: -f - (minus) is not handled:

$ mkdir aaa
$ echo "FROM scratch" | bin/podman-remote build -t foo -f - aaa
Error: error building: no stages to build

(works fine with non-remote podman).

@edsantiago
Copy link
Member

The other two problems are not related to this PR. One is the --device option, which probably makes no sense anyway under podman-remote, and the other seems to be a variant of #9867. Full log FYI

@edsantiago
Copy link
Member

Oh... and my nits are not meant as complaints. This PR allows me to enable forty-eight tests in buildah bud.bats that have so far been skipped! (There are 53 total, but five of those are the problems reported above). Thank you!

@edsantiago
Copy link
Member

Interesting. This new revision works much better with podman-remote + buildah-bud.bats, but fails in APIv2 and compose tests with what looks like a real error - one that is not caught by buildah-bud tests!

@rhatdan rhatdan force-pushed the Dockerfile branch 2 times, most recently from 098debe to 3f487d9 Compare June 8, 2021 18:25
@edsantiago
Copy link
Member

buildah-bud-under-podman also failing with same error as CI:

Error: failed to parse query parameter 'dockerfile': "Dockerfile": invalid character 'D' looking for beginning of value

podman-remote build has to handle multiple different locations
for the Containerfile.  Currently this works in local mode but not
when using podman-remote.

Fixes: containers#9871

Signed-off-by: Daniel J Walsh <[email protected]>
@edsantiago
Copy link
Member

LGTM. A lot more tests are passing.

@mheon
Copy link
Member

mheon commented Jun 9, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 9, 2021
@openshift-merge-robot openshift-merge-robot merged commit 2970e35 into containers:master Jun 9, 2021
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

podman-remote build: -f gets confused
4 participants