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

Don't exclude Dockerfile, Containerfiles from tar content #10890

Merged
merged 1 commit into from
Jul 12, 2021

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Jul 9, 2021

If the user specifies "*" in a .dockerignore or a .containerignore
then podman-remote build should not exclude the Dockerfile or
Containerfile or any content pointed to by -f in the context
directory.

We still need these files on the server side to complete the build.

Fixes: #9867

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

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

rhatdan commented Jul 9, 2021

@edsantiago PTAL

@rhatdan
Copy link
Member Author

rhatdan commented Jul 9, 2021

@containers/podman-maintainers PTAL

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

Other than leaving a breadcrumb in the test, LGTM

@@ -509,6 +509,36 @@ EOF
done
}

@test "podman build with ignore '*'" {
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment that this is a regression test for ##9867?

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 10, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan, vrothberg

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

If the user specifies "*" in a .dockerignore or a .containerignore
then podman-remote build should not exclude the Dockerfile or
Containerfile or any content pointed to by `-f` in the context
directory.

We still need these files on the server side to complete the build.

Fixes: containers#9867

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

/hold

Does not seem to be working: log. I will look into the failures soon this morning, but want to place a hold on this ASAP until I understand the failure better.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 12, 2021
@edsantiago
Copy link
Member

Here's a short reproducer for the first error:

$ mkdir zzz;cd zzz;printf "FROM quay.io/libpod/testimage:20210610\nCOPY ./ ./\nCOPY subdir ./\n" >Dockerfile
$ printf "*\nsubdir\n\!*/sub1*\n" >.dockerignore
$ mkdir subdir; touch subdir/sub1.txt

! Try podman non-remote. It should fail.
$ ../bin/podman build .
STEP 1/3: FROM quay.io/libpod/testimage:20210610
STEP 2/3: COPY ./ ./
--> f644a38d122
STEP 3/3: COPY subdir ./
Error: error building at STEP "COPY subdir ./": no items matching glob "/home/esm/src/atomic/2018-02.podman/libpod/zzz/subdir" copied (1 filtered out): no such file or directory

! podman-remote, though, succeeds (it should fail)
$ ../bin/podman-remote build .
STEP 1/3: FROM quay.io/libpod/testimage:20210610
STEP 2/3: COPY ./ ./
--> c3555113657
STEP 3/3: COPY subdir ./
COMMIT
--> 5f9f29bd454
5f9f29bd45409e016dfccdea718348e8788c6e0d4de9ed3d35920a3c70c93dc3

My gut feel is that this is a new bug, unrelated to #9867, but I'm not qualified to make that call. Other dockerignore-related tests are passing.

@edsantiago
Copy link
Member

Whew - figured out the second failure. TL;DR if both .dockerignore and .containerignore exist in the context directory, podman reads .containerignore but podman-remote reads both:

$ mkdir zzz;cd zzz;printf "FROM quay.io/libpod/testimage:20210610\nCOPY ./ ./\n" >Dockerfile
$ echo 'test1*' >.containerignore
$ echo 'test2*' >.dockerignore
$ touch test1 test2
../bin/podman build -q -t foo .;../bin/podman run --rm foo ls
700dae51df0fecf3cf2fbc59343c36f505a5ff87074494180d59dfb258faa9f9
Dockerfile
pause
test2            <---- good. .dockerfile was ignored
testimage-id

$ ../bin/podman-remote build -q -t foo .;../bin/podman run --rm foo ls
707107350bb1bcc1f7b4b29ca857f2b5649a5ab962e4da26cc2696b6100b9b36
Dockerfile
pause                     <--- neither test1 nor test2 exist. Bad.
testimage-id

As with the above comment, this may merit its own separate issue.

@rhatdan
Copy link
Member Author

rhatdan commented Jul 12, 2021

@edsantiago The both files in same context directory is strange. Open a separate issue. But do you still want to block this merge or does it LGTM?

@edsantiago
Copy link
Member

Looking at the diffs in this PR, I see no way they could cause these new failures -- so I assume they were preexisting bugs masked by #9867. I will file new issues. Leaving the hold for you or anyone else to release (with my blessing)

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 12, 2021
@mheon
Copy link
Member

mheon commented Jul 12, 2021

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 12, 2021
@openshift-merge-robot openshift-merge-robot merged commit 788c2d1 into containers:main Jul 12, 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 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 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, with '*' in .dockerignore: overzealously ignores Dockerfile
5 participants