Skip to content
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

Change default state directory to /run/oci #159

Merged
merged 1 commit into from
Jul 29, 2015

Conversation

LK4D4
Copy link
Contributor

@LK4D4 LK4D4 commented Jul 28, 2015

No description provided.

@marcosnils
Copy link
Contributor

👍

@wking
Copy link
Contributor

wking commented Jul 28, 2015

On Tue, Jul 28, 2015 at 10:47:49AM -0700, Alexander Morozov wrote:

  • Change default state directory to /var/run/oci

Why not /run/runc? There could be alternative implementations of
opencontainers/specs besides runC, and (since I don't think we need to
standardize on the state storage 1), I don't see a need to squat
on the common namespace with runC-specific storage.

And /var/run → /run is 2.

@crosbymichael
Copy link
Member

@wking because we want all runtimes to store state in the same place so all tools can consume this.

@crosbymichael
Copy link
Member

LGTM

@wking
Copy link
Contributor

wking commented Jul 28, 2015

On Tue, Jul 28, 2015 at 04:13:35PM -0700, Michael Crosby wrote:

@wking because we want all runtimes to store state in the same place
so all tools can consume this.

Can't tools consume it from something more flexible? Like a “give me
the state for $CONTAINER” command that runtimes must support 1?
Then runtimes are free to store it anywhere they feel is convenient
(or to just build the state on the fly by querying the kernel, etc.)

@crosbymichael
Copy link
Member

@wking but then you have to know what runtime you are using and call it. Maybe I want to spawn a container with one runtime then use something else to get stats and other information. Maybe I don't know that runc created the container so I cannot use it to get the state.

If there is one location on disk that has the state information that I can iterate and open then I know everything I need to know about that containers are running on a host without having to know anything about a specific runtime. I don't think on disk state is a bad idea because we are already very bound to the disk with the image rootfs, cgroups, etc....

@crosbymichael
Copy link
Member

@LK4D4 isn't /var/run going away and we should use /run now also?

@LK4D4
Copy link
Contributor Author

LK4D4 commented Jul 28, 2015

@crosbymichael I dunno, isn't this only on systemd systems?

@wking
Copy link
Contributor

wking commented Jul 28, 2015

On Tue, Jul 28, 2015 at 04:33:26PM -0700, Michael Crosby wrote:

@wking but then you have to know what runtime you are using and call
it. Maybe I want to spawn a container with one runtime then use
something else to get stats and other information. Maybe I don't
know that runc created the container so I cannot use it to get the
state.

Having parallel runtimes collaborating at the same layer seems a bit
odd. Why not have the stats layer running above runC and just call
‘runc state $CONTAINER’ to get the state? Then I'd add that ‘$RUNTIME
state $CONTAINER’ interface in the runtime-consumer spec 1, and you
could happily swap runtimes that stored the state wherever they wanted
and collected as much of that state dynamically as they wanted.

@wking
Copy link
Contributor

wking commented Jul 28, 2015

On Tue, Jul 28, 2015 at 04:47:19PM -0700, Alexander Morozov wrote:

@crosbymichael I dunno, isn't this only on systemd systems?

It's in the FHS 3.0 [1,2].

Signed-off-by: Alexander Morozov <lk4d4@docker.com>
@LK4D4 LK4D4 changed the title Change default state directory to /var/run/oci Change default state directory to /run/oci Jul 29, 2015
@LK4D4
Copy link
Contributor Author

LK4D4 commented Jul 29, 2015

Changed for /run. Thanks!

@LK4D4
Copy link
Contributor Author

LK4D4 commented Jul 29, 2015

@wking Anyway for now it's runc implementation detail. It's better to continue discussion about state format in spec repo.

@mrunalp
Copy link
Contributor

mrunalp commented Jul 29, 2015

LGTM

mrunalp pushed a commit that referenced this pull request Jul 29, 2015
Change default state directory to /run/oci
@mrunalp mrunalp merged commit b40c790 into opencontainers:master Jul 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants