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

Set container_uuid environment #7685

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
@rhatdan
Contributor

rhatdan commented Aug 22, 2014

If you are running with systemd as init 1 and specify the container_uuid environment
variable, systemd will create /etc/machine-id with the correct data.

Then on the host you can setup journald to watch the container.

systemd only allows 32 chars in the UUID stored in /etc/machine-id

Then we can later apply a different patch to setup journald to watch
containers from the host and log all syslog/stdout/stderr data together
int the hosts journal.

The following link explains what systemd expects to be setup.

http://www.freedesktop.org/wiki/Software/systemd/ContainerInterface/

This should replace

#3506

Docker-DCO-1.1-Signed-off-by: Dan Walsh dwalsh@redhat.com (github: rhatdan)

@LK4D4

This comment has been minimized.

Show comment
Hide comment
@LK4D4

LK4D4 Aug 22, 2014

Contributor

@rhatdan Could you provide image and run command with which I can test this?

Contributor

LK4D4 commented Aug 22, 2014

@rhatdan Could you provide image and run command with which I can test this?

@crosbymichael

This comment has been minimized.

Show comment
Hide comment
@crosbymichael

crosbymichael Aug 22, 2014

Contributor

nice, I like this approach

Contributor

crosbymichael commented Aug 22, 2014

nice, I like this approach

@LK4D4

This comment has been minimized.

Show comment
Hide comment
@LK4D4

LK4D4 Aug 22, 2014

Contributor

@rhatdan You should write integration-cli test.
And I didn't get expected ID from /etc/machine-id with codekoala/arch:latest and fedora/systemd-systemd:latest. I think it is some hardcoded /etc/machine-id in these images.

Contributor

LK4D4 commented Aug 22, 2014

@rhatdan You should write integration-cli test.
And I didn't get expected ID from /etc/machine-id with codekoala/arch:latest and fedora/systemd-systemd:latest. I think it is some hardcoded /etc/machine-id in these images.

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Aug 22, 2014

Contributor

It will not override an existing /etc/machine-id. So I guess you need to make sure there is no /etc/machine-id in the image, before running. I have not tested this with a systemd container yet. I am just following the example given on the document page. I will look into testing or having someone test this next week to make sure it works properly. (I am on Vacation next week).

Contributor

rhatdan commented Aug 22, 2014

It will not override an existing /etc/machine-id. So I guess you need to make sure there is no /etc/machine-id in the image, before running. I have not tested this with a systemd container yet. I am just following the example given on the document page. I will look into testing or having someone test this next week to make sure it works properly. (I am on Vacation next week).

@SvenDowideit

This comment has been minimized.

Show comment
Hide comment
@SvenDowideit

SvenDowideit Aug 25, 2014

Contributor

is this really so insignificant to not need documentation?

Contributor

SvenDowideit commented Aug 25, 2014

is this really so insignificant to not need documentation?

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Aug 25, 2014

Contributor

I added some mention of container_uuid to the examples and a line to docker run man page. Do you want this mentioned elsewhere. Not sure we document any other environment variables being set.

Contributor

rhatdan commented Aug 25, 2014

I added some mention of container_uuid to the examples and a line to docker run man page. Do you want this mentioned elsewhere. Not sure we document any other environment variables being set.

@crosbymichael

This comment has been minimized.

Show comment
Hide comment
@crosbymichael

crosbymichael Aug 25, 2014

Contributor

ping @shykes

What do you think about this proposed change? Are we ready to support letting containers know the ID?

Contributor

crosbymichael commented Aug 25, 2014

ping @shykes

What do you think about this proposed change? Are we ready to support letting containers know the ID?

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Sep 2, 2014

Contributor

Back from Vacation, updating Pull Requests and seeing if there is anything I can do to move these along.

Contributor

rhatdan commented Sep 2, 2014

Back from Vacation, updating Pull Requests and seeing if there is anything I can do to move these along.

@SvenDowideit

This comment has been minimized.

Show comment
Hide comment
@SvenDowideit
Contributor

SvenDowideit commented Sep 4, 2014

Docs LGTM - @jamtur01 @fredlf @ostezer

Show outdated Hide outdated docs/man/docker-run.1.md Outdated
@fredlf

This comment has been minimized.

Show comment
Hide comment
@fredlf

fredlf Sep 4, 2014

Contributor

LGTM. Ping @jamtur01

Contributor

fredlf commented Sep 4, 2014

LGTM. Ping @jamtur01

@vbatts

This comment has been minimized.

Show comment
Hide comment
@vbatts

vbatts Sep 5, 2014

Contributor

@crosbymichael @shykes this would also close #6481

Contributor

vbatts commented Sep 5, 2014

@crosbymichael @shykes this would also close #6481

@jamtur01

This comment has been minimized.

Show comment
Hide comment
@jamtur01

jamtur01 Sep 19, 2014

Contributor

Docs LGTM

Contributor

jamtur01 commented Sep 19, 2014

Docs LGTM

@gdm85

This comment has been minimized.

Show comment
Hide comment
@gdm85

gdm85 Sep 21, 2014

Contributor

Will there be a way to disable this? I can perfectly imagine that I wouldn't like my processes to know they're running within Docker by checking for the existance of this environment variable; sorry for the pedant minimalism, but I would also not clutter (further) the environment variables with this feature.
I understand systemd expects this environment variable and it would be nice if it were already set, but on the other hand not everybody is running systemd in containers...

Contributor

gdm85 commented Sep 21, 2014

Will there be a way to disable this? I can perfectly imagine that I wouldn't like my processes to know they're running within Docker by checking for the existance of this environment variable; sorry for the pedant minimalism, but I would also not clutter (further) the environment variables with this feature.
I understand systemd expects this environment variable and it would be nice if it were already set, but on the other hand not everybody is running systemd in containers...

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Sep 21, 2014

Contributor

Well you could unset container_uuid within the containers init. I think adding an option to stop this would be a waste.

Contributor

rhatdan commented Sep 21, 2014

Well you could unset container_uuid within the containers init. I think adding an option to stop this would be a waste.

@LK4D4

This comment has been minimized.

Show comment
Hide comment
@LK4D4

LK4D4 Sep 21, 2014

Contributor

At first, there is still no tests. And I still want to see how this works.

Contributor

LK4D4 commented Sep 21, 2014

At first, there is still no tests. And I still want to see how this works.

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Sep 22, 2014

Contributor

@LK4D4 Added integration test.

Contributor

rhatdan commented Sep 22, 2014

@LK4D4 Added integration test.

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Sep 22, 2014

Contributor

What is the magic command to allow me to test only my test? Not the entire test suite?

Contributor

rhatdan commented Sep 22, 2014

What is the magic command to allow me to test only my test? Not the entire test suite?

@vbatts

This comment has been minimized.

Show comment
Hide comment
@vbatts

vbatts Sep 22, 2014

Contributor

TESTFLAGS='-run TestCLIImageTagRemove' make test-integration-cli

Contributor

vbatts commented Sep 22, 2014

TESTFLAGS='-run TestCLIImageTagRemove' make test-integration-cli

Show outdated Hide outdated daemon/utils.go Outdated
Show outdated Hide outdated docs/man/docker-run.1.md Outdated
@gdm85

This comment has been minimized.

Show comment
Hide comment
@gdm85

gdm85 Oct 20, 2014

Contributor

From the perspective of minimalism (do not add a file for all users, if not all users use systemd), wouldn't be better to have a way to specify an ENV variable that contains the container id (something like DOCKERENV container_id $CONTAINER_ID) and then use that to write the file when needed, as a normal Dockerfile RUN echo $container_id > /etc/machine-id?

Contributor

gdm85 commented Oct 20, 2014

From the perspective of minimalism (do not add a file for all users, if not all users use systemd), wouldn't be better to have a way to specify an ENV variable that contains the container id (something like DOCKERENV container_id $CONTAINER_ID) and then use that to write the file when needed, as a normal Dockerfile RUN echo $container_id > /etc/machine-id?

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Oct 20, 2014

Contributor

We are not adding a file, this patch only adds an environment variable. If you start systemd within the container and it sees the environment variable it will setup /etc/machine-id. I don't see this as hurting the other model of running containers.

Contributor

rhatdan commented Oct 20, 2014

We are not adding a file, this patch only adds an environment variable. If you start systemd within the container and it sees the environment variable it will setup /etc/machine-id. I don't see this as hurting the other model of running containers.

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Oct 20, 2014

Contributor

How about if I change this patch to add a parameter to the docker daemon to run in systemd mode? Then the environment variable and the eventual patch to connect journald to the container would only be done on platforms that will run systemd containers?

Contributor

rhatdan commented Oct 20, 2014

How about if I change this patch to add a parameter to the docker daemon to run in systemd mode? Then the environment variable and the eventual patch to connect journald to the container would only be done on platforms that will run systemd containers?

@jessfraz

This comment has been minimized.

Show comment
Hide comment
@jessfraz

jessfraz Oct 20, 2014

Contributor

I think a systemd mode seems a bit too specific, right now we aren't
catering to init modes, what would be next an upstart mode? Just saying...
having the container id readily available is one thing, but what you are
proposing is something else entirely.

On Mon, Oct 20, 2014 at 5:59 AM, rhatdan notifications@github.com wrote:

How about if I change this patch to add a parameter to the docker daemon
to run in systemd mode? Then the environment variable and the eventual
patch to connect journald to the container would only be done on platforms
that will run systemd containers?


Reply to this email directly or view it on GitHub
#7685 (comment).

Contributor

jessfraz commented Oct 20, 2014

I think a systemd mode seems a bit too specific, right now we aren't
catering to init modes, what would be next an upstart mode? Just saying...
having the container id readily available is one thing, but what you are
proposing is something else entirely.

On Mon, Oct 20, 2014 at 5:59 AM, rhatdan notifications@github.com wrote:

How about if I change this patch to add a parameter to the docker daemon
to run in systemd mode? Then the environment variable and the eventual
patch to connect journald to the container would only be done on platforms
that will run systemd containers?


Reply to this email directly or view it on GitHub
#7685 (comment).

@jessfraz

This comment has been minimized.

Show comment
Hide comment
@jessfraz

jessfraz Oct 20, 2014

Contributor

I think we should close this in favor of container introspection re #8427

Contributor

jessfraz commented Oct 20, 2014

I think we should close this in favor of container introspection re #8427

@jessfraz

This comment has been minimized.

Show comment
Hide comment
@jessfraz

jessfraz Nov 12, 2014

Contributor

@rhatdan doesn't the creator of systemd work for redhat? is there a reason why the container_uuid must be formatted that way instead of say a container_id env variable?

Contributor

jessfraz commented Nov 12, 2014

@rhatdan doesn't the creator of systemd work for redhat? is there a reason why the container_uuid must be formatted that way instead of say a container_id env variable?

@crosbymichael

This comment has been minimized.

Show comment
Hide comment
@crosbymichael

crosbymichael Nov 12, 2014

Contributor

This is not about init systems, it's about providing information to a container's process that it will then start depending on. We are +1 on providing the container id and even the env var name.

The format, however, is unfortunate because it is systemd specific and nothing else can use it. This is causing problems because we would be adding this env var that could have been used generically by applications but this is not so, it can only be consumed by one application which makes it useless for docker users not running systemd as the entrypoint for the container.

Contributor

crosbymichael commented Nov 12, 2014

This is not about init systems, it's about providing information to a container's process that it will then start depending on. We are +1 on providing the container id and even the env var name.

The format, however, is unfortunate because it is systemd specific and nothing else can use it. This is causing problems because we would be adding this env var that could have been used generically by applications but this is not so, it can only be consumed by one application which makes it useless for docker users not running systemd as the entrypoint for the container.

@sghosh151

This comment has been minimized.

Show comment
Hide comment
@sghosh151

sghosh151 Nov 12, 2014

@crosbymichael What is the alternate format? any proposal? Any other consumers of the container id?

sghosh151 commented Nov 12, 2014

@crosbymichael What is the alternate format? any proposal? Any other consumers of the container id?

@LK4D4

This comment has been minimized.

Show comment
Hide comment
@LK4D4

LK4D4 Nov 12, 2014

Contributor

@sgosh151 Just container_id? I believe that nearly ~50% of docker users wants to know container_id inside container.

Contributor

LK4D4 commented Nov 12, 2014

@sgosh151 Just container_id? I believe that nearly ~50% of docker users wants to know container_id inside container.

@sghosh151

This comment has been minimized.

Show comment
Hide comment
@sghosh151

sghosh151 Nov 13, 2014

@LK4D4 trying to understand if the ~50% of docker user want the explicit "container_id" env or they just want an id for the container? If it is the explicit "container_id" - what tools are looking at it? or is it just individual user scripts?

If the other 50% where to look for /etc/machine_id that would be populated for container_uuid, would that be enough of a user base to get both env vars?

sghosh151 commented Nov 13, 2014

@LK4D4 trying to understand if the ~50% of docker user want the explicit "container_id" env or they just want an id for the container? If it is the explicit "container_id" - what tools are looking at it? or is it just individual user scripts?

If the other 50% where to look for /etc/machine_id that would be populated for container_uuid, would that be enough of a user base to get both env vars?

@LK4D4

This comment has been minimized.

Show comment
Hide comment
@LK4D4

LK4D4 Nov 13, 2014

Contributor

They want just id in any way. In discussed patch container_uuid != container_id.

Contributor

LK4D4 commented Nov 13, 2014

They want just id in any way. In discussed patch container_uuid != container_id.

@sghosh151

This comment has been minimized.

Show comment
Hide comment
@sghosh151

sghosh151 Nov 13, 2014

@LK4D4 if the var name and it's content are different between container_id and container_uuid - why block one for the other? It would seem that both can coexist.

sghosh151 commented Nov 13, 2014

@LK4D4 if the var name and it's content are different between container_id and container_uuid - why block one for the other? It would seem that both can coexist.

@fabiokung

This comment has been minimized.

Show comment
Hide comment
@fabiokung

fabiokung Nov 13, 2014

Contributor

$(hostname) already exposes the container uuid from inside the container. Wouldn't it be better to just allow systemd containers read from it and set their own env, without Docker having to do anything about it?

gethostname(2) is a very simple and general way for Docker to expose the container uuid inside containers. I'm not a big fan of magic env vars being injected into the process env, when there is already another mechanism on Linux that was designed to do just that (gethostname(2) and UTS namespaces).

Contributor

fabiokung commented Nov 13, 2014

$(hostname) already exposes the container uuid from inside the container. Wouldn't it be better to just allow systemd containers read from it and set their own env, without Docker having to do anything about it?

gethostname(2) is a very simple and general way for Docker to expose the container uuid inside containers. I'm not a big fan of magic env vars being injected into the process env, when there is already another mechanism on Linux that was designed to do just that (gethostname(2) and UTS namespaces).

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Nov 13, 2014

Contributor

hostname does not show the UUID it shows a truncated UUID and can be overridden in the command line.

Contributor

rhatdan commented Nov 13, 2014

hostname does not show the UUID it shows a truncated UUID and can be overridden in the command line.

@fabiokung

This comment has been minimized.

Show comment
Hide comment
@fabiokung

fabiokung Nov 13, 2014

Contributor

so why not expose the full container_uuid on gethostname(2) instead of injecting an env var?

An env var can also be overridden inside the container.

Contributor

fabiokung commented Nov 13, 2014

so why not expose the full container_uuid on gethostname(2) instead of injecting an env var?

An env var can also be overridden inside the container.

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Nov 13, 2014

Contributor

hostname makes sense to override, Env container_uuid not so much.

Contributor

rhatdan commented Nov 13, 2014

hostname makes sense to override, Env container_uuid not so much.

@LK4D4

This comment has been minimized.

Show comment
Hide comment
@LK4D4

LK4D4 Dec 16, 2014

Contributor

@rhatdan Hello, I just realized that we talking about inside container env, so will not this be the same like:

docker run -it --rm -e container_uuid=$(uuidgen) busybox printenv container_uuid
Contributor

LK4D4 commented Dec 16, 2014

@rhatdan Hello, I just realized that we talking about inside container env, so will not this be the same like:

docker run -it --rm -e container_uuid=$(uuidgen) busybox printenv container_uuid
@tiborvass

This comment has been minimized.

Show comment
Hide comment
@tiborvass

tiborvass Dec 16, 2014

Collaborator

How about we just set the Docker container ID in a CID envvar ?

Collaborator

tiborvass commented Dec 16, 2014

How about we just set the Docker container ID in a CID envvar ?

@jessfraz

This comment has been minimized.

Show comment
Hide comment
@jessfraz

jessfraz Dec 16, 2014

Contributor

I would be okay with adding container_uuid if we also add container_id or something like that so systemd and users can benefit :) but this is just my opinion
maybe an okay compromise?

Contributor

jessfraz commented Dec 16, 2014

I would be okay with adding container_uuid if we also add container_id or something like that so systemd and users can benefit :) but this is just my opinion
maybe an okay compromise?

@shykes

This comment has been minimized.

Show comment
Hide comment
@shykes

shykes Dec 16, 2014

Collaborator

I'm fine with making the container's ID available to the application in a well-know and well-supported env variable. It would be strictly better than applications trying to infer it from something that is not fully supported, like the hostname.

I think we should avoid the term "uuid" since, technically, the current ID is not a uuid, and even if it were, we reserve the right to change the allocation method later. For example with the new trust primitives, it is likely that each container will have its own encryption keypair, and we could use the public key as an identifier. As far as the application is concerned, all that matters is that there is a single, globally unique string which identifies the container in which it is running.

So, I suggest we store the container ID in $cid, document that fact in the specification so that applications can rely on it, and move on.

Collaborator

shykes commented Dec 16, 2014

I'm fine with making the container's ID available to the application in a well-know and well-supported env variable. It would be strictly better than applications trying to infer it from something that is not fully supported, like the hostname.

I think we should avoid the term "uuid" since, technically, the current ID is not a uuid, and even if it were, we reserve the right to change the allocation method later. For example with the new trust primitives, it is likely that each container will have its own encryption keypair, and we could use the public key as an identifier. As far as the application is concerned, all that matters is that there is a single, globally unique string which identifies the container in which it is running.

So, I suggest we store the container ID in $cid, document that fact in the specification so that applications can rely on it, and move on.

@shykes

This comment has been minimized.

Show comment
Hide comment
@shykes

shykes Dec 16, 2014

Collaborator

I realize that systemd expects the precise variable name $container_uuid. I don't think that's an issue, in this context systemd is the application, and if you're packaging an image running systemd, there are various ways to "translate" $cid to whatever the application expects. It doesn't seem viable for us to try to honor natively all the requirements of all the applications we might call. What if a systemd competitor expects another env variable, or if a future version of systemd changes the variable name? Will we be expected to change?

Hopefully this makes sense to everyone. It seems like a no-brainer to me.

Collaborator

shykes commented Dec 16, 2014

I realize that systemd expects the precise variable name $container_uuid. I don't think that's an issue, in this context systemd is the application, and if you're packaging an image running systemd, there are various ways to "translate" $cid to whatever the application expects. It doesn't seem viable for us to try to honor natively all the requirements of all the applications we might call. What if a systemd competitor expects another env variable, or if a future version of systemd changes the variable name? Will we be expected to change?

Hopefully this makes sense to everyone. It seems like a no-brainer to me.

@vbatts

This comment has been minimized.

Show comment
Hide comment
@vbatts

vbatts Dec 16, 2014

Contributor

@shykes true, but what if there were an abstraction that would allow manipulation for a $container_id. So if a UUID env is expected, then that could be accomplished?
Something like the container create/start/stop hooks we talked about?

Contributor

vbatts commented Dec 16, 2014

@shykes true, but what if there were an abstraction that would allow manipulation for a $container_id. So if a UUID env is expected, then that could be accomplished?
Something like the container create/start/stop hooks we talked about?

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Dec 16, 2014

Contributor

Turns out there is more to getting systemd wired up correctly then just passing in the container_uuid. I have extended this patch to handle setting up journalling to allow journal inside the container to write messages that the journal outside of the container can see. I have to work on one more patch to setup the machine-ctl to recognise that a there is a container running. docker needs to send the UUID to docker via dbus to make this happen. Once it is fully instrumented then there would be more container information available in the HOST OS, including using machinectl, ps -o machine, journal -M machine etc.

Contributor

rhatdan commented Dec 16, 2014

Turns out there is more to getting systemd wired up correctly then just passing in the container_uuid. I have extended this patch to handle setting up journalling to allow journal inside the container to write messages that the journal outside of the container can see. I have to work on one more patch to setup the machine-ctl to recognise that a there is a container running. docker needs to send the UUID to docker via dbus to make this happen. Once it is fully instrumented then there would be more container information available in the HOST OS, including using machinectl, ps -o machine, journal -M machine etc.

rhatdan added some commits Dec 8, 2014

Set container_uuid environment
If you are running with systemd as init 1 and specify the container_uuid environment
variable, systemd will create /etc/machine-id with the correct data.

Then on the host you can setup journald to watch the container.

systemd only allows 32 chars in the UUID stored in /etc/machine-id

Then we can later apply a different patch to setup journald to watch
containers from the host and log all syslog/stdout/stderr data together
int the hosts journal.

The following link explains what systemd expects to be setup.

http://www.freedesktop.org/wiki/Software/systemd/ContainerInterface/

This should replace

#3506

Docker-DCO-1.1-Signed-off-by: Dan Walsh <dwalsh@redhat.com> (github: rhatdan)
Create Journal directory so that journal on the host can see journal …
…content in container

Docker-DCO-1.1-Signed-off-by: Dan Walsh <dwalsh@redhat.com> (github: rhatdan)
Have docker register its machine with systemd
Docker-DCO-1.1-Signed-off-by: Dan Walsh <dwalsh@redhat.com> (github: rhatdan)
Merge branch 'master' of github.com:docker/docker into machine-id
Docker-DCO-1.1-Signed-off-by: Dan Walsh <dwalsh@redhat.com> (github: rhatdan)
@icecrime

This comment has been minimized.

Show comment
Hide comment
@icecrime

icecrime Jan 6, 2015

Contributor

Review session with @tiborvass @unclejack @crosbymichael

Although the PR initially started about "exposing" container ID, the requirements have changed along the way to supporting the systemd journal. We believe with this new information we should take a step back and think about the requirements to support the systemd journal with Docker containers.

We're closing the PR as it won't be merged as it is, but discussions can continue here.

Contributor

icecrime commented Jan 6, 2015

Review session with @tiborvass @unclejack @crosbymichael

Although the PR initially started about "exposing" container ID, the requirements have changed along the way to supporting the systemd journal. We believe with this new information we should take a step back and think about the requirements to support the systemd journal with Docker containers.

We're closing the PR as it won't be merged as it is, but discussions can continue here.

@icecrime icecrime closed this Jan 6, 2015

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Jan 6, 2015

Contributor

This is not unexpected, since if I could not convince you to support setting an Environment Variable within an image, I doubt you would consider mounting /var/lib/journal. Sadly it leaves those of us looking to run systemd correctly within a container little options.

Contributor

rhatdan commented Jan 6, 2015

This is not unexpected, since if I could not convince you to support setting an Environment Variable within an image, I doubt you would consider mounting /var/lib/journal. Sadly it leaves those of us looking to run systemd correctly within a container little options.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment