podman-remote build use .containerignore over .dockerignore #10913

rhatdan · 2021-07-12T20:10:51Z

$ 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
$ ../bin/podman-remote build .

Should fail, but succeeds because we are not sending over the
.dockerignore file to the server side. This PR will send the
.dockerignore so the server side and use it.

Fixes: #10907

Also if both .containerignore and .dockerignore in the context
directory, podman-remote should prefer .containerignore and not use
.dockerignore.

Fixes: #10908

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

rhatdan · 2021-07-12T20:11:05Z

@edsantiago PTAL

rhatdan · 2021-07-12T20:37:50Z

Fixes: #10908

edsantiago · 2021-07-13T12:32:51Z

I don't think this solves the problem; it actually breaks in a different way. I'll update later, after f2f.

edsantiago · 2021-07-13T18:31:42Z

Yes, this doesn't seem to fix anything. The fault is mine: I got lazy in writing the reproducer, and wrote something that probably didn't create the correct dockerignore file because of shell expansions. Try this for your dockerignore:

$ cat >zzz/.dockerignore <<EOF
*
subdir
!*/sub1*
EOF

With that file, podman build fails (as it should) but podman-remote build succeeds (bad, #10907)

edsantiago · 2021-07-13T19:56:13Z

That seems to have done it, thanks. Passes on my laptop. New CI test run in progress

edsantiago · 2021-07-13T20:29:15Z

LGTM! All dockerignore tests are now passing. (Failures are the usual stream-dropped bug, #10154)

edsantiago · 2021-07-13T20:57:58Z

test/system/070-build.bats

@@ -887,4 +887,61 @@ function teardown() {
    basic_teardown
 }

+@test "podman build .dockerignore failure test " {


Could you remove the trailing whitespace from this and all the other new tests?

Not sure what you mean?

Could you remove the space between failure test and the closing double-quote?

Also, still two unresolved bugs from my previous feedback; I'll repeat them

edsantiago · 2021-07-13T21:02:02Z

test/system/070-build.bats

+
+@test "podman build .containerignore and .dockerignore test " {
+    tmpdir=$PODMAN_TMPDIR/build-test
+    mkdir -p $subdir


I think you forgot:

subdir=$tmpdir/subdir

Again, this one seems to have slipped

edsantiago · 2021-07-13T21:06:41Z

test/system/070-build.bats

+    is "$output" ".*test2" "test2 should exists in the DockerfileExpected to fail"
+}
+
+@test "podman build .containerignore and .dockerignore test " {


Could you give this test a unique name? (It's the same as line 909)

Removed since it was a duplicate of prior.

edsantiago · 2021-07-13T21:09:29Z

test/system/070-build.bats

+RUN ls /tmp/test/
+EOF
+    run_podman build -t build_test $tmpdir
+    is "$output" ".*test2" "test2 should exists in the DockerfileExpected to fail"


Oops, hit Submit too soon. Also, should be 'exist' (no ess). And, I think the "Expected to fail" is a copy-paste error?

edsantiago · 2021-07-13T21:09:43Z

test/system/070-build.bats

+RUN ls /tmp/test/
+EOF
+    run_podman build -t build_test $tmpdir
+    is "$output" ".*test1" "test1 should exists in the DockerfileExpected to fail"


same thing (exist, Expected to fail)

edsantiago · 2021-07-14T13:41:11Z

test/system/070-build.bats

+RUN ls /tmp/test/
+EOF
+    run_podman build -t build_test $tmpdir
+    is "$output" ".*test1" "test1   should exists in the final image" "Confirm only .containerignore is used"


Too many arguments to is(): the last one ("Confirm only...") is ignored. Could you pick one or the other between that and "test1 should exist"?

TomSweeneyRedHat · 2021-07-14T15:13:22Z

All kinds of red test unhappiness @rhatdan

edsantiago · 2021-07-14T19:14:14Z

oh, you fixed them

openshift-ci · 2021-07-14T19:15:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago, 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:

~~OWNERS~~ [edsantiago,rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

edsantiago · 2021-07-14T20:38:45Z

Looks like the podman-remote error is different:

#| expected: '.*Error: error building at STEP "COPY subdir ./": no items matching glob'
#|   actual:   'Error: error building at STEP "COPY subdir ./": checking on sources under "/var/tmp/libpod_builder446168272/build": copier: stat: "/subdir": no such file or directory'

My simpleminded workaround would be something like:

    expect_msg="no items matching glob"
    if is_remote; then expect_msg="checking on sources under .*/subdir.*no such file or directory";fi

$ 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 $ ../bin/podman-remote build . Should fail, but succeeds because we are not sending over the .dockerignore file to the server side. This PR will send the .dockerignore so the server side and use it. Fixes: containers#10907 Also if both .containerignore and .dockerignore in the context directory, podman-remote should prefer .containerignore and not use .dockerignore. Fixes: containers#10908 Signed-off-by: Daniel J Walsh <[email protected]>

edsantiago · 2021-07-15T12:14:12Z

/lgtm

Thanks @rhatdan !

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 12, 2021

rhatdan force-pushed the build branch 2 times, most recently from d9f63eb to 3971357 Compare July 12, 2021 20:37

rhatdan force-pushed the build branch from 3971357 to f71f344 Compare July 12, 2021 20:38

rhatdan changed the title ~~podman-remote build shouldn't exclude .dockerignore/.containerignore~~ podman-remote build use .containerignore over .dockerignore Jul 12, 2021

rhatdan force-pushed the build branch from f71f344 to 665828d Compare July 12, 2021 20:40

rhatdan force-pushed the build branch from 665828d to 259fbd1 Compare July 13, 2021 19:45

edsantiago suggested changes Jul 13, 2021

View reviewed changes

edsantiago reviewed Jul 13, 2021

View reviewed changes

rhatdan force-pushed the build branch from 259fbd1 to 6555a82 Compare July 14, 2021 12:10

edsantiago reviewed Jul 14, 2021

View reviewed changes

rhatdan force-pushed the build branch from 6555a82 to be1fbae Compare July 14, 2021 19:08

edsantiago approved these changes Jul 14, 2021

View reviewed changes

rhatdan force-pushed the build branch from be1fbae to 9a103dd Compare July 14, 2021 19:22

rhatdan force-pushed the build branch from 9a103dd to c997064 Compare July 15, 2021 00:22

openshift-ci bot assigned edsantiago Jul 15, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 15, 2021

openshift-merge-robot merged commit 47f3517 into containers:main Jul 15, 2021

edsantiago mentioned this pull request Aug 19, 2021

Error: readlink: Permission denied: OCI permission denied #10912

Closed

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 bot locked as resolved and limited conversation to collaborators Sep 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

podman-remote build use .containerignore over .dockerignore #10913

podman-remote build use .containerignore over .dockerignore #10913

rhatdan commented Jul 12, 2021 •

edited

Loading

rhatdan commented Jul 12, 2021

rhatdan commented Jul 12, 2021

edsantiago commented Jul 13, 2021

edsantiago commented Jul 13, 2021

edsantiago commented Jul 13, 2021

edsantiago commented Jul 13, 2021

edsantiago Jul 13, 2021

rhatdan Jul 14, 2021

edsantiago Jul 14, 2021

edsantiago Jul 13, 2021

edsantiago Jul 14, 2021

edsantiago Jul 14, 2021

edsantiago Jul 13, 2021

rhatdan Jul 14, 2021

edsantiago Jul 13, 2021

rhatdan Jul 14, 2021

edsantiago Jul 13, 2021

rhatdan Jul 14, 2021

edsantiago Jul 14, 2021

TomSweeneyRedHat commented Jul 14, 2021

edsantiago commented Jul 14, 2021

openshift-ci bot commented Jul 14, 2021

edsantiago commented Jul 14, 2021

edsantiago commented Jul 15, 2021

podman-remote build use .containerignore over .dockerignore #10913

podman-remote build use .containerignore over .dockerignore #10913

Conversation

rhatdan commented Jul 12, 2021 • edited Loading

rhatdan commented Jul 12, 2021

rhatdan commented Jul 12, 2021

edsantiago commented Jul 13, 2021

edsantiago commented Jul 13, 2021

edsantiago commented Jul 13, 2021

edsantiago commented Jul 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomSweeneyRedHat commented Jul 14, 2021

edsantiago commented Jul 14, 2021

openshift-ci bot commented Jul 14, 2021

edsantiago commented Jul 14, 2021

edsantiago commented Jul 15, 2021

rhatdan commented Jul 12, 2021 •

edited

Loading