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.

Sign up for GitHub

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

Jump to bottom

CRI: clarify purpose of annotations #36446

Merged
merged 1 commit into from
Dec 29, 2016
Merged

CRI: clarify purpose of annotations #36446

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
64 changes: 48 additions & 16 deletions pkg/kubelet/api/v1alpha1/runtime/api.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

64 changes: 48 additions & 16 deletions pkg/kubelet/api/v1alpha1/runtime/api.proto
View file
Open in desktop
Expand Up @@ -234,14 +234,29 @@ message PodSandboxConfig {
repeated PortMapping port_mappings = 5;
// Key-value pairs that may be used to scope and select individual resources.
map<string, string> labels = 6;
// Annotations is an unstructured key value map that may be set by external
// tools to store and retrieve arbitrary metadata. There are a few features are
// driven by annotations, Runtimes could support them optionally:
// Unstructured key-value map that may be set by the kubelet to store and
// retrieve arbitrary metadata. This will include any annotations set on a
// pod through the Kubernetes API.
//
// Annotations MUST NOT be altered by the runtime; the annotations stored
// here MUST be returned in the PodSandboxStatus associated with the pod
// this PodSandboxConfig creates.
//
// In general, in order to preserve a well-defined interface between the
// kubelet and the container runtime, annotations SHOULD NOT influence
// runtime behaviour. For legacy reasons, there are some annotations which
// currently explicitly break this rule, listed below; in future versions
// of the interface these will be promoted to typed features.
//
// Annotations can also be useful for runtime authors to experiment with
// new features that are opaque to the Kubernetes APIs (both user-facing
// and the CRI). Whenever possible, however, runtime authors SHOULD
// consider proposing new typed fields for any new features instead.
//
// 1. AppArmor
//
// key: container.apparmor.security.beta.kubernetes.io/<container_name>
// description: apparmor profile for the container.
// description: apparmor profile for a container in this pod.
Copy link
Member

Choose a reason for hiding this comment

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

nit: s/a/the

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nack (it is an apparmor profile for a container in this pod; if you really wanted to get definite, it is "apparmor profile for the container in this pod by the given container name" but IMO that's unnecessary)

// value:
// * runtime/default: equivalent to not specifying a profile.
// * localhost/<profile_name>: profile loaded on the node
Expand All @@ -255,8 +270,8 @@ message PodSandboxConfig {
// value: see below.
//
// key: security.alpha.kubernetes.io/seccomp/container/<container name>
// description: the seccomp profile for the container (overides pod).
// values: see below
// description: the seccomp profile for the container (overrides pod).
// value: see below
//
// The value of seccomp is runtime agnostic:
// * runtime/default: the default profile for the container runtime
Expand Down Expand Up @@ -348,10 +363,12 @@ message PodSandboxStatus {
optional PodSandboxNetworkStatus network = 5;
// Linux-specific status to a pod sandbox.
optional LinuxPodSandboxStatus linux = 6;
// Labels are key value pairs that may be used to scope and select individual resources.
// Labels are key-value pairs that may be used to scope and select individual resources.
map<string, string> labels = 7;
// Annotations is an unstructured key value map that may be set by external
// tools to store and retrieve arbitrary metadata.
// Unstructured key-value map holding arbitrary metadata.
// Annotations MUST NOT be altered by the runtime; the value of this field
// MUST be identical to that of the corresponding PodSandboxConfig used to
// instantiate the pod sandbox this status represents.
map<string, string> annotations = 8;
Copy link

Choose a reason for hiding this comment

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

Why is this field even here? What purpose does it serve?

Copy link

Choose a reason for hiding this comment

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

Same below... Why do we provide annotations anywhere other than the PodSandboxConfig?

Copy link
Contributor

Choose a reason for hiding this comment

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

Kubelet actually stores metadata in the annotations, and it needs to get the metadata back when querying the container status.

Copy link

Choose a reason for hiding this comment

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

Why pass it off to the runtime though? Why not store the annotations in the Kubelet pod representations?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not store the annotations in the Kubelet pod representations

If you mean the k8s API "Pod", kubelet can't rely on the remote apiserver to checkpoint per-container instance information.

If kubelet implements a local checkpoint, it may eliminate most of the need to use annotations.

Copy link

Choose a reason for hiding this comment

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

I see, I was confused by the distinction between api (user-set) annotations and the system-set per-container annotations. IIUC the annotations on the pod are the api annotations, and the container level annotations are system-set, right? I think it might be worth distinguishing between these explicitly, either in a comment or the field name.

If that's the case, it seems like the annotations on the PodSandbox and PodSandboxStatus messages are redundant?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, I was confused by the distinction between api (user-set) annotations and the system-set per-container annotations. IIUC the annotations on the pod are the api annotations, and the container level annotations are system-set, right? I think it might be worth distinguishing between these explicitly, either in a comment or the field name.

Hmm..kubelet is a user of the API, and whatever it chooses to store in the annotations should not be defined in the API (unless we want the runtime to take actions).
As for the k8s api pod annotations, we currently don't pass them down to CRI.

Copy link
Contributor

Choose a reason for hiding this comment

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

If that's the case, it seems like the annotations on the PodSandbox and PodSandboxStatus messages are redundant?

Spoke to @timstclair offline. The podsandbox annotations today include the annotations from the k8s pod (similar to labels). I forgot about that completely.
We can add in the comment that kubelet pass the pod annotations to the podsandbox.

I think we can change the comment to reflect that.

Copy link
Member

Choose a reason for hiding this comment

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

We can add in the comment that kubelet pass the pod annotations to the podsandbox.

+1. And this is also for some annotation-based features e.g. AppArmor/Seccomp/Sysctls.

}

Expand Down Expand Up @@ -391,8 +408,10 @@ message PodSandbox {
optional int64 created_at = 4;
// Labels of the PodSandbox.
map<string, string> labels = 5;
// Annotations is an unstructured key value map that may be set by external
// tools to store and retrieve arbitrary metadata.
// Unstructured key-value map holding arbitrary metadata.
// Annotations MUST NOT be altered by the runtime; the value of this field
// MUST be identical to that of the corresponding PodSandboxConfig used to
// instantiate this PodSandbox.
map<string, string> annotations = 6;
}

Expand Down Expand Up @@ -551,8 +570,16 @@ message ContainerConfig {
// prefix ::= DNS_SUBDOMAIN
// name ::= DNS_LABEL
map<string, string> labels = 9;
// Annotations is an unstructured key value map that may be set by external
// tools to store and retrieve arbitrary metadata.
// Unstructured key-value map that may be used by the kubelet to store and
// retrieve arbitrary metadata.
//
// Annotations MUST NOT be altered by the runtime; the annotations stored
// here MUST be returned in the ContainerStatus associated with the container
// this ContainerConfig creates.
//
// In general, in order to preserve a well-defined interface between the
// kubelet and the container runtime, annotations SHOULD NOT influence
// runtime behaviour.
map<string, string> annotations = 10;
// Path relative to PodSandboxConfig.LogDirectory for container to store
// the log (STDOUT and STDERR) on the host.
Expand Down Expand Up @@ -665,8 +692,10 @@ message Container {
optional int64 created_at = 7;
// Key-value pairs that may be used to scope and select individual resources.
map<string, string> labels = 8;
// Annotations is an unstructured key value map that may be set by external
// tools to store and retrieve arbitrary metadata.
// Unstructured key-value map holding arbitrary metadata.
// Annotations MUST NOT be altered by the runtime; the value of this field
// MUST be identical to that of the corresponding ContainerConfig used to
// instantiate this Container.
map<string, string> annotations = 9;
}

Expand Down Expand Up @@ -708,7 +737,10 @@ message ContainerStatus {
optional string message = 11;
// Key-value pairs that may be used to scope and select individual resources.
map<string,string> labels = 12;
// Annotations is an unstructured key value map.
// Unstructured key-value map holding arbitrary metadata.
// Annotations MUST NOT be altered by the runtime; the value of this field
// MUST be identical to that of the corresponding ContainerConfig used to
// instantiate the Container this status represents.
map<string,string> annotations = 13;
// Mounts for the container.
repeated Mount mounts = 14;
Expand Down