Skip to content
This repository has been archived by the owner on Aug 14, 2020. It is now read-only.

Add the ability to create empty dir when the host path doesn't exist in the volume when kind=host #648

Closed
yifan-gu opened this issue Aug 18, 2016 · 8 comments

Comments

@yifan-gu
Copy link
Contributor

yifan-gu commented Aug 18, 2016

Ref kubernetes/kubernetes#26816

Docker has the ability to create empty dir for if the hostpath in the volume doesn't exist on the host (kind=host), and some kubernetes e2e tests / examples reply on this functionality, so it would be preferable if rkt provides such ability as an option.

One way to achieve this is to add an optional boolean in the volume type saying "Create empty dir if the path doesn't exist". e.g.

type Volume struct {
    Name ACName `json:"name"`
    Kind string `json:"kind"`

    // currently used only by "host"
    // TODO(jonboulle): factor out?
    Source    string `json:"source,omitempty"`
    ReadOnly  *bool  `json:"readOnly,omitempty"`
    Recursive *bool  `json:"recursive,omitempty"`
    // Automatically create an empty dir if the path doesn't exist.
    CreateIfNotExisted *bool `json:create,omitempty"`

    // currently used only by "empty"
    Mode *string `json:"mode,omitempty"`
    UID  *int    `json:"uid,omitempty"`
    GID  *int    `json:"gid,omitempty"`
}

cc @jonboulle @sgotti @iaguis @alban @lucab @s-urbaniak

@yifan-gu yifan-gu changed the title Add the ability to create empty file for dir when the host path doesn't exist in the volume when kind=host Add the ability to create empty dir when the host path doesn't exist in the volume when kind=host Aug 18, 2016
@lucab
Copy link
Contributor

lucab commented Aug 19, 2016

Is this the same as rkt/rkt#2329 (comment)? In that case, it may be a rkt bug, not a spec one. Something that was not clear to me from there: how should this interact with RO containers?

EDIT: Nevermind, I missed the "host" part.

@jonboulle
Copy link
Contributor

This seems like an antipattern to me, so I'd rather push back on it in kubernetes. But even if we decide to do it, I think it should be rkt specific, not a general spec thing.

@yifan-gu
Copy link
Contributor Author

@jonboulle I agree, at first I am not comfortable about creating arbitrary dirs on host. If we all agree we don't want this, we can just implement in rktnetes.

@sgotti
Copy link
Member

sgotti commented Aug 19, 2016

I'm also for delegating the host path creation (in any way like directory creation or a tmpfs like rkt does when a volume for a mountpoint isn't specified) to who is specifying the volume definition (the orchestrator, the user calling rkt etc...).

@euank
Copy link
Contributor

euank commented Aug 19, 2016

The only possible use for a kind=host volume is an antipattern. It expresses an external system dependency that means a less portable, less self-contained, container. I think once you're already speaking in terms of an antipattern, adding more antipattern knobs is less egregious.


There are multiple reasons a host volume might be used, which I've tried to list below:

  1. Communicating with an external system that produced data, is producing data, or is listening on a unix socket, such as a user who produced a configuration directory for the application, or X and the X11 socket.
  2. Communicating with external systems that will consume data created by the application; e.g. a container that produces logs at /var/log/fooApp and wishes to bindmount it to the host at /var/log/fooApp as well because the operator knows an external logging process will scan through that directory and collect them.
  3. Communicating between instances of yourself: e.g. a container that bindmounts in /var/cache/appName/thumbnails for each run and populates it with any thumbnails not present, but prefers to re-use existing ones between invocations simply to avoid unneeded processing.

In the case of 1, creating the nonexistent thing is not a desirable action and can end up breaking things (aside, docker once had a bug where it created the docker.sock after starting old containers, so any restart=always containers bindmounting the docker.sock would create a directory there and crashloop the daemon).

In case 2 and 3, you actually gain a slight bit of portability by creating the directory, and the person running the application can decide whether their application/use-case matches 2 or 3.

There's actually a case of 3 on CoreOS itself: the etcd-wrapper file explicitly created the data directory prior to running the aci. There's also an example of type 1 where the kubelet-wrapper bindmounts over the manifests directory which a user should have created as part of configuration.

In reality, the difference between the user and the rkt process making a directory is very minor. I think the only value it provides is that it indicates the intent of the volume slightly more clearly (as 1 or 2/3; is it expected the user created and populated it, or a different process is running which provides it... or is it fine that it's created fresh for this app to consume), and makes it slightly simpler to copy-paste around

@euank
Copy link
Contributor

euank commented Aug 22, 2016

@jonboulle wanna add a few more details to that close? Did this issue end up getting moved somewhere else, or do we have sufficient consensus of "orchestration systems should do this, not rkt"?

@jonboulle
Copy link
Contributor

I surmised that we had consensus that this is better handled by the orchestration system or rkt (not the spec), but if I'm off the mark let me know.

@euank
Copy link
Contributor

euank commented Aug 23, 2016

I'm fine with that assertion. Thanks for clarifying... I'll open an issue against Kubernetes as to whether it should specify something like this to its volume concept.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants