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

spec: get rid of fulfills for the volumes #182

Closed
philips opened this issue Dec 3, 2014 · 15 comments
Closed

spec: get rid of fulfills for the volumes #182

philips opened this issue Dec 3, 2014 · 15 comments

Comments

@philips
Copy link
Contributor

philips commented Dec 3, 2014

Right now volumes in the container can fulfill particular labels. This is a bit troublesome because there might be colliding names for the same volume.

https://github.com/coreos/rocket/blob/master/app-container/SPEC.md#container-runtime-manifest-schema

Instead of doing this give each volume a name and each app will get volumes filled in by name to its mountpoints.

    "apps": [
        {
            "app": "example.com/reduce-worker-1.0.0",
            "imageID": "sha256-277205b3ae3eb3a8e042a62ae46934b470e431ac",
            "mountPoints": [{"src": "work", "dest": "data"}]
        },
        {
            "app": "example.com/worker-backup-1.0.0",
            "imageID": "sha256-3e86b59982e49066c5d813af1c2e2579cbf573de",
             "mountPoints": [{"src": "buildoutput", "dest": "backup"}]
        },
    ],
    "volumes": [
        {"name": "work", "kind": "host", "source": "/opt/tenant1/work", "readOnly": true},
        {"name": "buildoutput", "kind": "empty"}
    ],
@jonboulle
Copy link
Contributor

@philips this isn't strictly solving your issue because it's still possible to have collisions:

        {"name": "work", "kind": "host", "source": "/opt/tenant1/work", "readOnly": true},
        {"name": "work", "kind": "empty"}

i.e. we then need to validate name uniqueness. Maybe we want to make this change anyway (?) but it seems we could just as easily fix the issue by asserting that fulfils must be non-overlapping.

@philips
Copy link
Contributor Author

philips commented Dec 3, 2014

We need to validate name uniqueness in the volumes and the app. The problem I am trying to solve is that two apps might have two mount points with identical names but different meanings.

@philips
Copy link
Contributor Author

philips commented Dec 4, 2014

@jonboulle does this make sense? I want to move forward with the work on this.

@jonboulle
Copy link
Contributor

@philips with the patient help of @vcaputo I think I understand what you're getting at now, but the UX is still unclear; would rkt run volume specifiers become scoped by app?

@philips
Copy link
Contributor Author

philips commented Dec 7, 2014

@jonboulle Does this example help:

rkt run 
 example.com/webapp
 --volume ssl-share,type=empty
 example.com/ssl-key-downloader
 --mount ssl-share=example.com/ssl-key-downloader:ssl-out
 example.com/nginx-proxy
 --mount ssl-share=example.com/nginx-proxy:private-certs

@jonboulle
Copy link
Contributor

Yeah that clarifies (although holy complicated command line, batman)

@jonboulle
Copy link
Contributor

It also brings up the point again that we need to decide about label uniqueness (eg what if someone runs two example.com/webapp with different versions in a single container?)

@philips
Copy link
Contributor Author

philips commented Dec 8, 2014

@jonboulle Yea the command line is ugly. On your second point we shouldn't attempt to make everything a CLI flag. We can support the 80% use cases. But, if as you point out, people want multiple copies of the same app they will need to write a JSON object instead.

@philips
Copy link
Contributor Author

philips commented Dec 9, 2014

This issue was moved to appc/spec#13

@vaibhavkhanduja
Copy link

I am struggling to get the volumes seen inside rocket container ....

the manifest file looks like:
{
"acVersion": "1.0.0",
"acKind": "AppManifest",
"name": "coreos.com/hello-1.0.0",
"os": "linux",
"arch": "amd64",
"version": "1.0.0",
"exec": [
"/apps/hello.sh"
],
"ports": [
{
"name": "www",
"protocol": "tcp",
"port": 5000
}
],
"mountPoints": [{"src": "work", "dest": "data"}]
"volumes": [
{"name": "work", "kind": "host", "source": "/data/"}
],
}

My application does see the volumes inside the container .. I cannot see systemd-nspawn not having any bind mount in its command line

stage1/usr/lib/ld-linux-x86-64.so.2 stage1/usr/bin/systemd-nspawn --boot --register false --uuid=4499f289-952f-4f86-9bf9-08b6cd21d769 --directory=stage1 -- --default-standard-output=tty]

what's wrong here?

@jonboulle
Copy link
Contributor

What is your rkt command line?

On Wed, Dec 10, 2014 at 4:22 PM, Vaibhav Khanduja notifications@github.com
wrote:

I am struggling to get the volumes seen inside rocket container ....

the manifest file looks like:
{
"acVersion": "1.0.0",
"acKind": "AppManifest",
"name": "coreos.com/hello-1.0.0",
"os": "linux",
"arch": "amd64",
"version": "1.0.0",
"exec": [
"/apps/hello.sh"
],
"ports": [
{
"name": "www",
"protocol": "tcp",
"port": 5000
}
],
"mountPoints": [{"src": "work", "dest": "data"}]
"volumes": [
{"name": "work", "kind": "host", "source": "/data/"}
],
}

My application does see the volumes inside the container .. I cannot see
systemd-nspawn not having any bind mount in its command line

stage1/usr/lib/ld-linux-x86-64.so.2 stage1/usr/bin/systemd-nspawn --boot
--register false --uuid=4499f289-952f-4f86-9bf9-08b6cd21d769
--directory=stage1 -- --default-standard-output=tty]

what's wrong here?


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

@vaibhavkhanduja
Copy link

My rocket command line did not include any special volume parameter ...
rkt run hello.aci ... 
Am I supposed to do it? Can this be documented well?

 On Tuesday, December 9, 2014 11:38 AM, Brandon Philips <notifications@github.com> wrote:

Closed #182.—
Reply to this email directly or view it on GitHub.

@jonboulle
Copy link
Contributor

Volumes are not part of the image manifest spec, only mountPoints are: https://github.com/appc/spec/blob/master/SPEC.md#manifest-schemas

Keep the mountPoint in your image manifest, and then try rkt run -volume work:/data ..., that should do it.

The way this is structured will likely change as we resolve appc/spec#13

@vaibhavkhanduja
Copy link

I guess then it is a bug on actool as it validated the manifest file with volume  ..
Anyways thanks ..

 On Friday, December 12, 2014 11:04 AM, Jonathan Boulle <notifications@github.com> wrote:

Volumes are not part of the image manifest spec, only mountPoints are: https://github.com/appc/spec/blob/master/SPEC.md#manifest-schemasKeep the mountPoint in your image manifest, and then try rkt run -volume work:/data, that should do it.The way this is structured will likely change as we resolve appc/spec#13
Reply to this email directly or view it on GitHub.

@jonboulle
Copy link
Contributor

I guess then it is a bug on actool as it validated the manifest file with volume ..

We do not validate any superfluous fields in the manifests as we have not really strictly mandated this. We could consider being stricter /cc @philips

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

3 participants