buildpacks / pack

CLI for building apps using Cloud Native Buildpacks
https://buildpacks.io
Apache License 2.0
2.58k stars 288 forks source link

Better defaults for `--docker-host`. #1338

Open matejvasek opened 2 years ago

matejvasek commented 2 years ago

Description

When running pack against docker socket at non-standard location using the DOCKER_HOST environment variable, the user also has to set the --docker-host flag so export container knows how to access the daemon.

Right now it defaults to unix://var/run/docker.sock which is not always right.

The value of the flag can be determined if docker host is either unix socket or ssh. For unix socket the mapping is identity. For ssh we can acutally ask remote what is the value of the DOCKER_HOST there.

For TCP there is no way that I know of; user simply must know what to enter. The mapping could be possibly identity but there are possible problems when when accessing localhost: the export container sees itself as localhost not the host machine running the daemon (unless --network=host).

jylin commented 2 years ago

+1

With docker rootless, I currently use "--docker-host inherit", which works, but it would be good if this were not needed.

Related to this request: can "pack" detect docker host from context if DOCKER_HOST is not set? See: https://docs.docker.com/engine/security/rootless/#client. Edit: I'll create another issue for that.

jylin commented 2 years ago

Another problem: pack seems to escape "@" when set in DOCKER_HOST, so something like "DOCKER_HOST=ssh://user@host" doesn't work, even using "--docker-host inherit".

jijeesh commented 2 years ago

Document Support for podman v2 buildpacks/docs#341

ssh-add -k "$HOME/.ssh/podman-machine-default" podman system connection default podman-machine-default-root

login into the root user

ssh root@localhost -p 58780

create link to docker.sock

ln -s /run/podman/podman.sock /var/run/docker.sock

then export

export DOCKER_HOST=ssh://root@localhost:58780

jjbustamante commented 1 year ago

Relates to #1830

till commented 1 year ago

I stumbled across this today as I used another container for dockerd and after googling a lot and reading code I think this is a (currently) bug, as the documentation states that:

You just need to set the DOCKER_HOST environment and most applications will pick it up (pack is one of them).

It seems like, parts of pack (as in downloading images), seem to recognise DOCKER_HOST already, while specifically in pack build, it's a bit mangled. I think this code should be amended in order to make the podman example work.

The other option would be to amend the documentation to tell people to use --docker-host=inherit.

Is there a downside to using DOCKER_HOST straight up?

jjbustamante commented 1 year ago

Hi @till

It seems like, parts of pack (as in downloading images), seem to recognise DOCKER_HOST already, while specifically in pack build, it's a bit mangled. I think this code should be amended in order to make the podman example work.

The other option would be to amend the documentation to tell people to use --docker-host=inherit.

I think we put it in the docs:

Screenshot 2023-09-12 at 10 47 19 AM

And this ticket was created and linked in In the future, this may be automatically detected.

till commented 1 year ago

Couldn't find it, apologies.

Do you have any pointers what you'd like to see in a PR? I can try to whip something up.

till commented 1 year ago

I may have overlooked this also because the wording is somewhat wrong in the docs. This is not (just) related to a socket connection, but overall using DOCKER_HOST with environment variables..

For reference, I set DOCKER_HOST=tcp://some.service.svc.cluster.local:2375 so I can run docker/podman in another pod, etc..

Anyhow, if anyone has any pointers, I would make a PR that uses DOCKER_HOST when it's set. Doesn't change/correct/adjust the value, etc.. Just uses it transparently. Would that be acceptable?

natalieparellano commented 1 year ago

@till this should be fine, when passed inherit we just read the value from DOCKER_HOST anyway:

        if dockerHost == "inherit" {
            dockerHost = os.Getenv("DOCKER_HOST")
        }

It probably makes sense for DOCKER_HOST to be the default value when no --docker-host is provided, but the end user should be able to override it in case the value is not correct.

matejvasek commented 1 year ago

The inherit was deliberately chosen not to be default just to be sure backward compatibility is not broken.

matejvasek commented 1 year ago

@till The thing is: The DOCKER_HOST envvar is the address of docker socket from local machine (where you run pack/docker) point of view. The --docker-host flag indicates socket path from point of view of the machine that is actually running daemon and which is mounted into build containers.

There are cases when this is one single machine, for instance your are running podman on Linux. However there are also cases when this is not true, for example when using podman-machine (VM) on macOS.

matejvasek commented 1 year ago

@till Another example: imagine you have one Linux machine with docker running there. From point of view of that machine the docker socket is unix:///var/run/docker.sock. Then imaging you have a Windows machine that is connecting to that Linux machine via TCP. From point of view of that machine the docker socket is tcp://example.local:2375. You most definitely do not do not want --docker-host=inherit which would be equivalent to --docker-host=tcp://example.local:2375. What is best here is to leave it alone and default to unix:///var/run/docker.sock.

You most definitely do not do not want --docker-host=inherit which would be equivalent to --docker-host=tcp://example.local:2375

To elaborate:

matejvasek commented 1 year ago

tl;dr we cannot universally assume --docker-host=inherit will just work.

matejvasek commented 1 year ago

Another example: DOCKER_HOST=ssh://user@server -- again we do not want --docker-host=inherit the builder container does not have password or ssh keys to access it.

till commented 1 year ago

Another example: DOCKER_HOST=ssh://user@server -- again we do not want --docker-host=inherit the builder container does not have password or ssh keys to access it.

I am not sure what you are guarding against. And why wouldn't the builder have access? Wouldn't that be up for the user to sort out? Or unset DOCKER_HOST in case it's not what they want? Or what would you do here?

I think if it's set, it should be used as is as that is behaviour most people expect and that is also mentioned in blog posts (e.g. regarding podman).

I made a PR as a pass, let me know if you have thoughts. It also falls back to the original behaviour, at least so I hope.

matejvasek commented 1 year ago

Another example: DOCKER_HOST=ssh://user@server -- again we do not want --docker-host=inherit the builder container does not have password or ssh keys to access it.

I am not sure what you are guarding against. And why wouldn't the builder have access? Wouldn't that be up for the user to sort out? Or unset DOCKER_HOST in case it's not what they want? Or what would you do here?

I think if it's set, it should be used as is as that is behaviour most people expect and that is also mentioned in blog posts (e.g. regarding podman).

I made a PR as a pass, let me know if you have thoughts. It also falls back to the original behaviour, at least so I hope.

@till I am not strongly against --docker-host=inherit being default. I am merely pointing out the complexities of setting --docker-host.

At least with ssh protocol I am quite convinced you do not want inherit, the builder container most definitely doesn't have credentials (unless somehow complicatedly mounted, if somebody is skilled enough to do this they may as well know how to set up --docker-host) .

Also with tcp you might have problem because of a) client cert authentication, b) hostname resolution. For instance if docker host is tcp://localhost:1234 then again build container won't be able to access it because localhost from build container PoV is the container itself not the machine running daemon (the machine where you run pack).

With unix socket it is more likely you want inherit, still there are scenarios where this is an issue. For instance on macOS with podman desktop the podman will somehow expose the docker socket out of the VM but the path is different than path it the VM, so again inherit won't work.

So by using defaulting --docker-host=inherit we fix issues for some users but possibly break it for some other. Question is what user group is bigger.

till commented 1 year ago

@matejvasek thanks for elaborating and clarifying, I think I understand what you mean.

I am trying to make it behave how it is generally expected for a new user, vs. what someone needs who runs pack on a k8s cluster (though I am in that bucket).

My TCP (non tls) example already works with the proper DOCKER_HOST.

So I realize changing this may be a BC break, but pack is at 0.y.z. So I hope that's acceptable?

I think this could be further improved by adding more variables to the build (such as the location of the TLS certs), or potentially providing an ssh config: https://github.com/moby/moby/blob/1a7969545d73537545645f5cd2c79b7a77e7d39f/client/options.go#L33

I have to admit, I haven't looked into the internals how docker/cli connects to an ssh host, as in, where it's getting configuration from. I have only used the socket and http(s) so far.

matejvasek commented 1 year ago

@till I think we could conditionally default to DOCKER_HOST:

matejvasek commented 1 year ago

@till even in your specific case DOCKER_HOST=tcp://some.service.svc.cluster.local:2375 --docker-host=inherit is not necessary what you want. It does work just fine but it is sub-optimal -- you drag all traffic via k8s service.

What you could do instead:

pack --network=host --docker-host=tcp://localhost:2375 <rest of args here>
till commented 1 year ago

No, the dind is running per user in a tenant/namespace with resource quotas applied. Also to ensure the cache doesn't work across boundaries etc.. I definitely do not want host network.

I used a service to ensure it's predictable between updates and longer running than the pod with pack. This also allows for simultaneous builds, etc..

matejvasek commented 1 year ago

There is one more way to avoid setting --docker-host. It is possible to use --publish flag then the image is directly pushed to registry (but it is not then present in podman storage).