Support Dockerfile.dockerignore

[victornoel]

This fixes #776

Description

• Use Dockerfile.dockerignore in priority over the context's .dockerignore file.
• Actual name checked is based on the docker file passed to the command line
• Introduces unit tests and integration test to validate the behaviour.

I haven't added an unit test because it wasn't clear what was to correct way to test this because excludes seems to be managed globally via a static field. I would be interested by guidance from knowledgeable people on this.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

• Includes unit tests
• Adds integration tests if needed.

See the contribution guide for more details.

Reviewer Notes

• The code flow looks good.
• Unit tests and or integration tests added.

Release Notes

• Support Dockerfile.dockerignore file in the same way as docker build

[tejal29]

LGTM except additional tests.

[tejal29]

i think we should test ignore/bar is not excluded along with ignore_relative/foo here to make sure context dockerignore is not used.? What do you think?

[victornoel]

very good point, yes!

[tejal29]

We should test ignore_relative/bar is not excluded here to make sure Dockerfile_test_dockerignore_relative.dockerignore is not used?

[tejal29]

The newly added dockerfile failed

Error building images: Failed to build image gcr.io/kaniko-test/docker-dockerfile_test_dockerignore_relative with docker command "[docker build -t gcr.io/kaniko-test/docker-dockerfile_test_dockerignore_relative -f dockerfiles/Dockerfile_test_dockerignore_relative .]": exit status 1 Sending build context to Docker daemon  122.4kB

Step 1/2 : FROM scratch
 ---> 
Step 2/2 : COPY ignore_relative/* /foo
When using COPY with more than one source file, the destination must be a directory and end with a /
exit status 1
FAIL	github.com/GoogleContainerTools/kaniko/integration	265.988s

The way integration tests are setup,

1. it first builds with docker build
2. builds an image with kaniko
3. Compared them

So, in your case step 1 fails.

[victornoel]

@tejal29 apparently, docker only reads the Dockerfile.dockerignore when DOCKER_BUILDKIT=1 is used for docker build: moby/moby#12886 (comment)

I'm not sure what is the best course of action then now. Is there some way to alter the integration test for this one or should we just drop it because it's not supported by vanilla docker build?

[victornoel]

@tejal29 I added another commit with more unit tests (and refactored it to separate them in multiple t.Run) per your comments

[tejal29]

Can we remove this from integration test and rename this to "Dockerfile_dockeringore_relative"

[victornoel]

@tejal29 I'm not 100% sure how things work: if the file doesn't have the word test in it, then it won't be included in integration tests or is there more to do?

[victornoel]

ah you answered below, thx :)

[tejal29]

Thanks @victornoel, your integration test is still going to fail.

@tejal29 I added another commit with more unit tests (and refactored it to separate them in multiple t.Run) per your comments

Awesome!

The integration test will be fixed if you rename Dockerfile_test_dockerignore_relative to not match pattern

kaniko/integration/images.go

Line 99 in 2218859

allDockerfiles, err := filepath.Glob(path.Join(dockerfilesPath, "Dockerfile_test*"))

[victornoel]

@tejal29 it's pushed, do you want me to squash everything into one commit or should I leave it like that?

[victornoel]

@tejal29 @kokoro-team seams to have remove the kokoro:run tag just after you added it :)

[tejal29]

@victornoel, whenever kokoro (internal jenkins) receives the notification via label, it kicks off a job to run the tests on the PR and removes the label.

[victornoel]

@tejal29 ah, ok, excellent, thank you for your help, my first PR here and in Go :)

[guillaume86]

Could this be upstreamed to moby/moby ?
I would love to have this option available for my windows builds.
As noted here it would probably get approved: moby/moby#12886 (comment)

[victornoel]

@guillaume86 isn't what moby/buildkit#901 was for? (cited in GoogleContainerTools/skaffold#3837)

[guillaume86]

@victornoel Yes but buildkit is not supported on windows.

[victornoel]

@guillaume86 ah, sorry about that, well here it's about kaniko, you would need to ask in the moby repository for implementing this feature, not sure you will get help here :)