Skip to content
This repository has been archived by the owner on Jan 5, 2023. It is now read-only.

Allow for setting context of Docker build #37

Closed
wants to merge 3 commits into from

Conversation

chrisjsewell
Copy link

@chrisjsewell chrisjsewell commented Feb 8, 2021

re-open of #30 (which I totally forgot about 馃槵 )
I've removed the additional "bug fixes", which I'm not sure where correct, to focus on the main issue

Fixes: #18

@chrisjsewell
Copy link
Author

the test failures appear to be occurring for all PRs and not related to this one

@ssbarnea ssbarnea added the bug This issue/PR relates to a bug. label May 12, 2021
@ssbarnea ssbarnea closed this May 12, 2021
@ssbarnea ssbarnea reopened this May 12, 2021
@@ -67,7 +67,7 @@
- not item.item.pre_build_image | default(false)
docker_image:
build:
path: "{{ molecule_ephemeral_directory }}"
path: "{{ item.item.context | default(molecule_ephemeral_directory) }}"
Copy link
Member

Choose a reason for hiding this comment

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

Why would this be called context and not path? I also do not find this documented anywhere, which worries me a little bit.

Choose a reason for hiding this comment

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

This is how docker is calling it. Because the image is built by the docker engine (that might not even run on the same host as the CLI command), this is what happens when we execute the build command (a bit simplified, but close enough):

  1. CLI tool packages the current directory into a build context and sends it to the engine.
  2. Engine executes instructions from the Dockerfile, taking any files required from the context that has been sent to it.

I hope this explains things a bit.

@ssbarnea ssbarnea requested a review from tadeboro May 14, 2021 15:13
@tadeboro
Copy link

We should probably close this PR or #51 since they are both trying to achieve the same thing. This way, we can have a discussion about the "right" implementation in one place.

@ssbarnea
Copy link
Member

Closed per @tadeboro comment.

@ssbarnea ssbarnea closed this May 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue/PR relates to a bug.
Projects
None yet
3 participants