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

runtime: Document state annotations as a copy of config annotations #946

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
runtime: Document state annotations as a copy of config annotations
The spec was not very clear on how state annotations are related to
config annotations.  In the pull-request that landed state
annotations, it sounds like these were supposed to be copied opaquely
from the config [1].  It's still not clear to me why we'd copy
annotations but not the rest of the config [2], but I'm leaving that
alone for now.

There was previous interest in runtime-specified annotations [3,4]
(e.g. a RunV socket path [5]), but this commit does not allow runtimes
to inject additional entries because I don't like:

* Relying on config authors to avoid squatting on the namespace used
  by the runtime (if ties are broken in favor of the config) or
* Silently clobbering configured annotations (if ties are broken in
  favor of the runtime).

My preference would be to follow [3] and:

* Only include runtime-specified information in the state annotations.
* Require state readers to follow 'bundle' to the config.json if they
  wanted configured annotations (or embed the whole config.json in the
  state).

But with 1.0 released and spec-maintainer comments like [1], I think
it's too late to return to that approach.  If we want to expose
runtime-specified annotations, I think we'll need a new state
property.  There has been previous discussion of using "labels" and
"annotations" to carry both types of information in the state [6], and
while it's not as elegant as a full config copy, the
labels/annotations approach is still viable.

[1]: #484 (comment)
[2]: #484 (comment)
[3]: #188
[4]: #331 (comment)
[5]: #188 (comment)
[6]: #331 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
  • Loading branch information
wking committed Jan 11, 2018
commit 76217dc7010b67a7e293b8c49546d23f1caa4f26
3 changes: 2 additions & 1 deletion runtime.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ The state of a container includes the following properties:
* **`bundle`** (string, REQUIRED) is the absolute path to the container's bundle directory.
This is provided so that consumers can find the container's configuration and root filesystem on the host.
* **`annotations`** (map, OPTIONAL) contains the list of annotations associated with the container.
If no annotations were provided then this property MAY either be absent or an empty map.
If the configuration set [`annotations`](config.md#annotations), this value MUST exactly match the configured `annotations`.
If no annotations were configured then this property MAY either be absent or an empty map.

The state MAY include additional properties.

Expand Down