opencontainers / runtime-spec

OCI Runtime Specification
http://www.opencontainers.org
Apache License 2.0
3.19k stars 541 forks source link

Guidelines for config.json annotations vs. an external file? #492

Open wking opened 8 years ago

wking commented 8 years ago

In opencontainers/image-spec#87, @philips is wondering whether the runtime-spec maintainers think network configuration and other metadata should go in config.json's annotations or in an external file. That sounds like something that deserves a SHOULD suggestion in this spec, so folks who don't already have an opinion can follow whatever the best practice is deemed to be without having to think to hard.

Besides the labels/annotation work in this repository with #188, #331, and #480, there was also some IRC discussion recorded here, with me arguing for external files and @vbatts arguing for embedding in config.json. More recently, @julz floated an external hooks.json (and @mrunalp came right back and suggested annotations). So possible OCI positions:

We really don't care

Language (in the annotations docs?) like:

Opaque metadata MAY also be stored in external files, and this specification makes no recommendation on whether to use external files or annotations.

Recommend external files for everything

Language like:

Information that does not target the OCI runtime implementation SHOULD be stored in separate files alongside the runtime's config.json.

This is my current preference, since it means the runtime is obviously not responsible for anything related to the extra data. But it means that having annotations in the config.json spec at all will be a bit confusing (I'd prefer removing it).

Recommend annotations for everything

Language like:

Information that does not target the OCI runtime implementation SHOULD be stored in annotations. Information that is structured should be flattened to a string, since the annotations map requires string values.

If we lift the string requirement and allow opaque data in annotations, we could drop the second sentence.

Recommend annotations for string data, and external files for structured data

Language like:

Information that does not target the OCI runtime implementation SHOULD be stored in annotation if it can be represented as a string without difficulty. Information that is more structured (e.g. JSON objects) SHOULD be stored in external files alongside the runtime's config.json.

Other ideas?

Are there breakdowns I'm missing? It's hard to say much about opaque data. And while @philips clearly has expectations about [downstream consumption of the image-spec annotations][6], I'm not sure how generic those expectations are (and maybe he doesn't think they apply to the runtime-spec's annotations anyway).

JakeWarner commented 8 years ago

I highly favor the 'Recommend external files for everything' approach.

To take it a step further, I'm still in favor of splitting some of the host-specific stuff out of the config.json file. I know at one point the OCI bundle contained a runtime.json and then it was decided against. I wasn't as involved within the community back at the time so I'm not fully sure what the criteria for what went into runtime.json vs config.json. That being said, having an immutable drop-in config that was specific to the container and didn't require any customization regardless of the host is very appealing as an implementor of the spec. It'd be great to be able to pull in an OCI-bundle, drop in my own custom "host-config" along side the container config and just immediately start the container. The fact that I currently have to unmarshal, modify, re-marshal and save the configurations for users importing containers to our platform just feels meh.

I fear that by adding more stuff to the config.json, we're going to end up with more confusion of "where does the spec start/end?"

To me, the ideal OCI-bundle structure would be something along the lines of: /bundle/rootfs/ /bundle/config.json /bundle/host-config.json /bundle/plugins/NAME.json <-- not sure if 'plugins' is the best word here

'Plugins' would contain optional json configurations created by different tools/platforms that implemented the OCI-spec which could be distributed within the bundle itself.

(sorry, got a little off topic-ish)

cyphar commented 8 years ago

@JakeWarner As another aside, I don't like JSON as a config format. Personally I think TOML would've been a better choice -- JSON has way too many quirks to make it a good choice of config format.

wking commented 8 years ago

On Tue, Jun 07, 2016 at 11:16:00PM -0700, Jake Warner wrote:

I wasn't as involved within the community back at the time so I'm not fully sure what the criteria for what went into runtime.json vs config.json.

284 has the big picture, and the list thread 1 has detailed

rebuttals of all the reasons put forward in favor of splitting the config file.

/bundle/plugins/.json <-- not sure if 'plugins' is the best word here

'Plugins' would contain optional json configurations created by different tools/platforms that implemented the OCI-spec.

The use-case that spawned this issue was “where do we put information about exposed ports?” (opencontainers/image-spec#87). That's targetting network managers, which are currently out-of-scope for the OCI. So I'd caution against phrasing the full recommendation in terms of OCI-spec tools; the extra configuration can be for anything, OCI-related or not. Of course, folks may want to use OCI-related-ness in the recommendation (e.g. suggesting OCI-related stuff go in annotations, while OCI-independent stuff go in external files).

 Subject: Single, unified config file (i.e. rolling back specs#88)
 Date: Wed, 4 Nov 2015 09:53:20 -0800
 Message-ID: <20151104175320.GC24652@odin.tremily.us>
wking commented 8 years ago

On Wed, Jun 08, 2016 at 12:38:45AM -0700, Aleksa Sarai wrote:

As another aside, I don't like JSON as a config format. Personally I think TOML would've been a better choice…

Orthogonal? Can you open a new issue?

JakeWarner commented 8 years ago

284 has the big picture, and the list thread [1] has detailed

rebuttals of all the reasons put forward in favor of splitting the config file.

Will review.

The use-case that spawned this issue was “where do we put information about exposed ports?” (opencontainers/image-spec#87). That's targetting network managers, which are currently out-of-scope for the OCI. So I'd caution against phrasing the full recommendation in terms of OCI-spec tools; the extra configuration can be for anything, OCI-related or not. Of course, folks may want to use OCI-related-ness in the recommendation (e.g. suggesting OCI-related stuff go in annotations, while OCI-independent stuff go in external files).

I'm all for ExposedPorts going into the config.json since almost everything requires it (probably should be part of the spec) but since networking can get so complex and it's not defined by the spec, why include it in the config.json, even as an annotation?

wking commented 8 years ago

On Wed, Jun 08, 2016 at 10:41:16AM -0700, Jake Warner wrote:

I'm all for ExposedPorts going into the config.json since almost everything requires it but since networking can get so complex and it's not defined by the spec, why include it in the config.json, even as an annotation?

After the discussion in today's meeting, I'm not sure there's a consensus on the broader issue that I was targetting here. For networking in particular, my impression of consensus was:

For the broader issue, @brendandburns also wants to allow structured data in annotations 5, although that remains contentions. And I didn't see anyone speaking up for separate files (I've already explained my position on that in this issue).

vbatts commented 8 years ago

As a nit, I did not recommend embedding the document, but rather brainstorming possibilities.

On Wed, Jun 8, 2016, 13:32 W. Trevor King notifications@github.com wrote:

On Wed, Jun 08, 2016 at 10:41:16AM -0700, Jake Warner wrote:

I'm all for ExposedPorts going into the config.json since almost everything requires it but since networking can get so complex and it's not defined by the spec, why include it in the config.json, even as an annotation?

After the discussion in today's meeting, I'm not sure there's a consensus on the broader issue that I was targetting here. For networking in particular, my impression of consensus was:

  • ExposedPorts is pretty simple, and the image-spec structure [1] can get packed down to a reasonable string [2], so this should get put in ‘annotations’ [3].
  • The rest of networking is complicated, and will likely be handled by the orchestration layer (reading from whatever it's particular image-format is) without writing any configuration to the bundle directory [4].

For the broader issue, @brendandburns also wants to allow structured data in annotations [5], although that remains contentions. And I didn't see anyone speaking up for separate files (I've already explained my position on that in this issue).

[1]: https://github.com/opencontainers/image-spec/blob/v0.2.0/serialization.md#container-runconfig-field-descriptions [2]: https://github.com/opencontainers/image-spec/issues/87#issuecomment-224185157 [3]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-06-08-17.01.log.html#l-65 [4]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-06-08-17.01.log.html#l-60 [5]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-06-08-17.01.log.html#l-47

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/opencontainers/runtime-spec/issues/492#issuecomment-224685445, or mute the thread https://github.com/notifications/unsubscribe/AAEF6ayW2mq1oXZEpF_rDEmcUcNEy-lOks5qJwqigaJpZM4IwlII .

philips commented 8 years ago

I am personally +1 for an annotation like:

{
    "ociVersion": "0.2.0",
...
    "annotations": {
        "opencontainers.org/image-spec/exposedPorts": "8080,53/udp"
    }
}

I think in general we should avoid forcing someone to parse and deal with additional files between OCI specs. And this is first case where the OCI Image Spec has data that it doesn't know how to give to the OCI Runtime Spec.

Original comment here: https://github.com/opencontainers/image-spec/issues/87#issuecomment-224185157

wking commented 8 years ago

On Thu, Jun 09, 2016 at 07:39:20PM -0700, Brandon Philips wrote:

I think in general we should avoid forcing someone to parse and deal with additional files between OCI specs. And this is first case where the OCI Image Spec has data that it doesn't know how to give to the OCI Runtime Spec.

So are you comfortable requiring external lookup tables (e.g. bundle path → source image name / CAS ID) to efficiently build new images 1? If you don't copy that information (or embed the image config/manifest) into the bundle somewhere, it's going to be hard to build an efficient new image using just the bundle tree.

wking commented 7 years ago

I wrote

So are you comfortable requiring external lookup tables…

To ground that in something concrete, umoci uses a parallel umoci.json which stores a reference to the original manifest. Then on repacking, umoci loads the original manifest and appends to the history, etc., preserving all the other complicated values that are too tedious to serialize into annotations. So combining umoci's parallel-file approach and opencontainers/image-spec#492's recommendation to populate a org.opencontainers.imageSpec.exposedPorts annotation, etc., I think the consensus position here seems to be either “We really don't care” or “Recommend annotations for string data, and external files for structured data” from my original set of possible positions.