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

config: Clarify ociVersion covering the configuration <-> runtime API #523

Merged
merged 1 commit into from
Sep 15, 2016

Conversation

wking
Copy link
Contributor

@wking wking commented Jul 26, 2016

There are other APIs described in this specification (e.g. the state JSON format, and the in-flight command-line API), but this string covers the configuration file and referenced objects (e.g. the filesystem at root.path). As additional, backwards compatible features are added to the spec (leading to 1.1, 1.2, etc. releases) and supported by runtimes, those runtimes will still stupport 1.0 configs. Once a 2.0 spec is cut, runtimes that only support 2.0 (and nothing in the 1.0 line) will no longer support the 1.0 config.

My preferred approach here would be to use JSON-LD to explicitly document the intended semantics for each field, which would allow us to drop the config-wide version and version each field independently. That would mean a breaking change on a particular field would only break compatibility for folks who were using that field. Unfortunately, I haven't had much luck pushing the consensus in that direction.

This commit does not add wording about how the runtime and other consumers should handle an incompatible version. We can address that once the command-line API lands.

@duglin was slated to file a PR for this, but it's been a while since that meeting so I thought I'd work one up myself ;). There is also some discussion on this issue in opencontainers/tob#16.

@wking wking force-pushed the clarify-version branch from b9b32a1 to dd79e12 Compare July 26, 2016 21:55
@@ -12,7 +12,7 @@ Below is a detailed description of each field defined in the configuration forma

* **`ociVersion`** (string, required) MUST be in [SemVer v2.0.0](http://semver.org/spec/v2.0.0.html) format and specifies the version of the OpenContainer specification with which the bundle complies.
The OpenContainer spec follows semantic versioning and retains forward and backward compatibility within major versions.
For example, if an implementation is compliant with version 1.0.1 of the spec, it is compatible with the complete 1.x series.
For example, if a configuration is compliant with version 1.1 of this specification, it is compatible with all runtimes that support any 1.1 or later release of this specification, but it not compatible with a runtime that supports 1.0 and not 1.1.
Copy link
Member

@cyphar cyphar Jul 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this break our ability to have breaking changes on major releases? Especially the wording "or any later relase of this specification".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Tue, Jul 26, 2016 at 03:00:34PM -0700, Aleksa Sarai wrote:

-For example, if an implementation is compliant with version 1.0.1 of the spec, it is compatible with the complete 1.x series.
+For example, if a configuration is compliant with version 1.1 of this specification, it is compatible with all runtimes that support any 1.1 or later release of this specification, but it not compatible with a runtime that supports 1.0 and not 1.1.

Doesn't this break our ability to have breaking changes on major
releases? I prefer the 1.x series wording for this.

Neither line mentions major bumps at all. If we want a major-bump
line, I suggest something like:

Once a 2.0 spec is cut, a 1.x configuration will not be compatible
runtimes that only support 2.x configurations (and nothing in the
1.x line).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"but it not" -> "but it is not"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Thu, Sep 08, 2016 at 10:31:52AM -0700, Vincent Batts wrote:

-For example, if an implementation is compliant with version 1.0.1 of the spec, it is compatible with the complete 1.x series.
+For example, if a configuration is compliant with version 1.1 of this specification, it is compatible with all runtimes that support any 1.1 or later release of this specification, but it not compatible with a runtime that supports 1.0 and not 1.1.

"but it not" -> "but it is not"

Oops, thanks. Fixed to “but is not” in dd79e12bd51886.

@wking
Copy link
Contributor Author

wking commented Sep 8, 2016

This one has been quiet for a while, so I thought I'd give it a bump.
Can we get this (or some similar PR) added to the 1.0.0 milestone so
we don't forget to define “breaking change” before we cut 1.0?

There seem to be other versioning interpretations floating around as
well in other contexts. For example @philips seems to consider a
schema bump a breaking change [1](in the context of image-spec, but
the same idea applies to runtime spec). Adding a property like
whiteout-file references from a manifest or image config is:

  • Backwards compatible for the configuration ↔ runtime API, because
    old-version configs (which don't use the setting) still have the
    same semantics when interpreted by an implementation built for the
    new version.
  • A breaking change for the configuration ↔ runtime, because an
    old-version runtime will not correctly process a config which uses a
    non-default value for the new property.

So breaking the configuration ↔ runtime API interface makes life
difficult for runtime callers and image authors, who now have to
update all of their images and/or invocation scripts.

And breaking the configuration ↔ runtime interface makes life
difficult for runtime maintainers, who now have to update their
implementation to handle the new property.

I'm personally more concerned about the configuration ↔ runtime API
interface, because images are much more likely to be signed and
abandoned than runtimes. By preserving compatibility with old images,
you avoid problems with verified translating 2 and such that don't
exist for runtimes (if someone ports/forks a runtime, the port/fork is
unlikely to be so automatic that you can transfer some of your trust
for the original into trust for the port/fork).

As @philips points out 1, there's a lot of mass behind runtime
implementations, so we certainly don't want to ignore configuration ↔
runtime compat issues. I just think they should determine what
constitutes a major bump.

 Subject: Any chance of changing the whiteout file approach?

 Subject: Signature verification after image-format translation
 Date: Fri, 15 Apr 2016 14:50:18 -0700
 Message-ID: <20160415215018.GR23066@odin.tremily.us>

@tianon
Copy link
Member

tianon commented Sep 8, 2016

LGTM

Approved with PullApprove

@vbatts
Copy link
Member

vbatts commented Sep 13, 2016

needs a rebase

There are other APIs described in this specification (e.g. the state
JSON format, and the in-flight command-line API [1]), but this string
covers the configuration file and referenced objects (e.g. the
filesystem at root.path).  As additional, backwards compatible
features are added to the spec (leading to 1.1, 1.2, etc. releases)
and supported by runtimes, those runtimes will *still* stupport 1.0
configs.  Once a 2.0 spec is cut, runtimes that only support 2.0 (and
nothing in the 1.0 line) will no longer support the 1.0 config.

My preferred approach here would be to use JSON-LD [2,3,4] to
explicitly document the intended semantics for each field, which would
allow us to drop the config-wide version and version each field
independently.  That would mean a breaking change on a particular
field would only break compatibility for folks who were using that
field.  Unfortunately, I haven't had much luck pushing the consensus
in that direction.

This commit does not add wording about how the runtime and other
consumers should handle an incompatible version.  We can address that
once the command-line API lands.

[1]: opencontainers#513
[2]: opencontainers#371 (comment)
[3]: opencontainers/image-spec#111 (comment)
[4]: opencontainers#510 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
@wking
Copy link
Contributor Author

wking commented Sep 14, 2016

On Tue, Sep 13, 2016 at 11:47:37AM -0700, Vincent Batts wrote:

needs a rebase

Rebased around #525's f2cc9fd with bd51886c94e7c0. The only
conflicts were in the diff context.

@vbatts
Copy link
Member

vbatts commented Sep 15, 2016

LGTM

Approved with PullApprove

@tianon
Copy link
Member

tianon commented Sep 15, 2016

LGTM redux

Approved with PullApprove

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

Successfully merging this pull request may close these issues.

4 participants