Unverified Commit c02b784b authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub

Merge pull request #58172 from NVIDIA/annotations

Automatic merge from submit-queue (batch tested with PRs 58184, 59307, 58172). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Add annotations to the device plugin API **What this PR does / why we need it**: **Which issue(s) this PR fixes** : Related to #56649 but does not fix it This adds the ability for the device plugins to annotate containers. Product wise, this allows the NVIDIA device plugin to support CRI-O (which allows hooks through container annotations). **Special notes for your reviewer**: /area hw-accelerators /cc @vishh @jiayingz @vikaschoudhary16 I'm wondering if it would make sense to fire a blank call to `newContainerAnnotations` at the start of the deviceplugin to get Annotations that are forbidden. Current behavior is that any Annotations that conflicts with Kubelet will be overwritten by Kubelet. **Release note**: ```release-note NONE ```
parents 792e637f db537e59
...@@ -96,6 +96,8 @@ message AllocateResponse { ...@@ -96,6 +96,8 @@ message AllocateResponse {
repeated Mount mounts = 2; repeated Mount mounts = 2;
// Devices for the container. // Devices for the container.
repeated DeviceSpec devices = 3; repeated DeviceSpec devices = 3;
// Container annotations to pass to the container runtime
map<string, string> annotations = 4;
} }
// Mount specifies a host volume to mount into a container. // Mount specifies a host volume to mount into a container.
......
...@@ -620,6 +620,7 @@ func (cm *containerManagerImpl) GetResources(pod *v1.Pod, container *v1.Containe ...@@ -620,6 +620,7 @@ func (cm *containerManagerImpl) GetResources(pod *v1.Pod, container *v1.Containe
opts.Devices = append(opts.Devices, devOpts.Devices...) opts.Devices = append(opts.Devices, devOpts.Devices...)
opts.Mounts = append(opts.Mounts, devOpts.Mounts...) opts.Mounts = append(opts.Mounts, devOpts.Mounts...)
opts.Envs = append(opts.Envs, devOpts.Envs...) opts.Envs = append(opts.Envs, devOpts.Envs...)
opts.Annotations = append(opts.Annotations, devOpts.Annotations...)
return opts, nil return opts, nil
} }
......
...@@ -191,6 +191,7 @@ func (pdev podDevices) deviceRunContainerOptions(podUID, contName string) *Devic ...@@ -191,6 +191,7 @@ func (pdev podDevices) deviceRunContainerOptions(podUID, contName string) *Devic
devsMap := make(map[string]string) devsMap := make(map[string]string)
mountsMap := make(map[string]string) mountsMap := make(map[string]string)
envsMap := make(map[string]string) envsMap := make(map[string]string)
annotationsMap := make(map[string]string)
// Loops through AllocationResponses of all cached device resources. // Loops through AllocationResponses of all cached device resources.
for _, devices := range resources { for _, devices := range resources {
resp := devices.allocResp resp := devices.allocResp
...@@ -198,17 +199,18 @@ func (pdev podDevices) deviceRunContainerOptions(podUID, contName string) *Devic ...@@ -198,17 +199,18 @@ func (pdev podDevices) deviceRunContainerOptions(podUID, contName string) *Devic
// Environment variables // Environment variables
// Mount points // Mount points
// Device files // Device files
// Container annotations
// These artifacts are per resource per container. // These artifacts are per resource per container.
// Updates RunContainerOptions.Envs. // Updates RunContainerOptions.Envs.
for k, v := range resp.Envs { for k, v := range resp.Envs {
if e, ok := envsMap[k]; ok { if e, ok := envsMap[k]; ok {
glog.V(3).Infof("skip existing env %s %s", k, v) glog.V(4).Infof("Skip existing env %s %s", k, v)
if e != v { if e != v {
glog.Errorf("Environment variable %s has conflicting setting: %s and %s", k, e, v) glog.Errorf("Environment variable %s has conflicting setting: %s and %s", k, e, v)
} }
continue continue
} }
glog.V(4).Infof("add env %s %s", k, v) glog.V(4).Infof("Add env %s %s", k, v)
envsMap[k] = v envsMap[k] = v
opts.Envs = append(opts.Envs, kubecontainer.EnvVar{Name: k, Value: v}) opts.Envs = append(opts.Envs, kubecontainer.EnvVar{Name: k, Value: v})
} }
...@@ -216,14 +218,14 @@ func (pdev podDevices) deviceRunContainerOptions(podUID, contName string) *Devic ...@@ -216,14 +218,14 @@ func (pdev podDevices) deviceRunContainerOptions(podUID, contName string) *Devic
// Updates RunContainerOptions.Devices. // Updates RunContainerOptions.Devices.
for _, dev := range resp.Devices { for _, dev := range resp.Devices {
if d, ok := devsMap[dev.ContainerPath]; ok { if d, ok := devsMap[dev.ContainerPath]; ok {
glog.V(3).Infof("skip existing device %s %s", dev.ContainerPath, dev.HostPath) glog.V(4).Infof("Skip existing device %s %s", dev.ContainerPath, dev.HostPath)
if d != dev.HostPath { if d != dev.HostPath {
glog.Errorf("Container device %s has conflicting mapping host devices: %s and %s", glog.Errorf("Container device %s has conflicting mapping host devices: %s and %s",
dev.ContainerPath, d, dev.HostPath) dev.ContainerPath, d, dev.HostPath)
} }
continue continue
} }
glog.V(4).Infof("add device %s %s", dev.ContainerPath, dev.HostPath) glog.V(4).Infof("Add device %s %s", dev.ContainerPath, dev.HostPath)
devsMap[dev.ContainerPath] = dev.HostPath devsMap[dev.ContainerPath] = dev.HostPath
opts.Devices = append(opts.Devices, kubecontainer.DeviceInfo{ opts.Devices = append(opts.Devices, kubecontainer.DeviceInfo{
PathOnHost: dev.HostPath, PathOnHost: dev.HostPath,
...@@ -231,17 +233,18 @@ func (pdev podDevices) deviceRunContainerOptions(podUID, contName string) *Devic ...@@ -231,17 +233,18 @@ func (pdev podDevices) deviceRunContainerOptions(podUID, contName string) *Devic
Permissions: dev.Permissions, Permissions: dev.Permissions,
}) })
} }
// Updates RunContainerOptions.Mounts. // Updates RunContainerOptions.Mounts.
for _, mount := range resp.Mounts { for _, mount := range resp.Mounts {
if m, ok := mountsMap[mount.ContainerPath]; ok { if m, ok := mountsMap[mount.ContainerPath]; ok {
glog.V(3).Infof("skip existing mount %s %s", mount.ContainerPath, mount.HostPath) glog.V(4).Infof("Skip existing mount %s %s", mount.ContainerPath, mount.HostPath)
if m != mount.HostPath { if m != mount.HostPath {
glog.Errorf("Container mount %s has conflicting mapping host mounts: %s and %s", glog.Errorf("Container mount %s has conflicting mapping host mounts: %s and %s",
mount.ContainerPath, m, mount.HostPath) mount.ContainerPath, m, mount.HostPath)
} }
continue continue
} }
glog.V(4).Infof("add mount %s %s", mount.ContainerPath, mount.HostPath) glog.V(4).Infof("Add mount %s %s", mount.ContainerPath, mount.HostPath)
mountsMap[mount.ContainerPath] = mount.HostPath mountsMap[mount.ContainerPath] = mount.HostPath
opts.Mounts = append(opts.Mounts, kubecontainer.Mount{ opts.Mounts = append(opts.Mounts, kubecontainer.Mount{
Name: mount.ContainerPath, Name: mount.ContainerPath,
...@@ -252,6 +255,20 @@ func (pdev podDevices) deviceRunContainerOptions(podUID, contName string) *Devic ...@@ -252,6 +255,20 @@ func (pdev podDevices) deviceRunContainerOptions(podUID, contName string) *Devic
SELinuxRelabel: false, SELinuxRelabel: false,
}) })
} }
// Updates for Annotations
for k, v := range resp.Annotations {
if e, ok := annotationsMap[k]; ok {
glog.V(4).Infof("Skip existing annotation %s %s", k, v)
if e != v {
glog.Errorf("Annotation %s has conflicting setting: %s and %s", k, e, v)
}
continue
}
glog.V(4).Infof("Add annotation %s %s", k, v)
annotationsMap[k] = v
opts.Annotations = append(opts.Annotations, kubecontainer.Annotation{Name: k, Value: v})
}
} }
return opts return opts
} }
...@@ -66,6 +66,8 @@ type DeviceRunContainerOptions struct { ...@@ -66,6 +66,8 @@ type DeviceRunContainerOptions struct {
Mounts []kubecontainer.Mount Mounts []kubecontainer.Mount
// The host devices mapped into the container. // The host devices mapped into the container.
Devices []kubecontainer.DeviceInfo Devices []kubecontainer.DeviceInfo
// The Annotations for the container
Annotations []kubecontainer.Annotation
} }
// TODO: evaluate whether we need these error definitions. // TODO: evaluate whether we need these error definitions.
......
...@@ -382,6 +382,11 @@ type EnvVar struct { ...@@ -382,6 +382,11 @@ type EnvVar struct {
Value string Value string
} }
type Annotation struct {
Name string
Value string
}
type Mount struct { type Mount struct {
// Name of the volume mount. // Name of the volume mount.
// TODO(yifan): Remove this field, as this is not representing the unique name of the mount, // TODO(yifan): Remove this field, as this is not representing the unique name of the mount,
...@@ -431,6 +436,10 @@ type RunContainerOptions struct { ...@@ -431,6 +436,10 @@ type RunContainerOptions struct {
Devices []DeviceInfo Devices []DeviceInfo
// The port mappings for the containers. // The port mappings for the containers.
PortMappings []PortMapping PortMappings []PortMapping
// The annotations for the container
// These annotations are generated by other components (i.e.,
// not users). Currently, only device plugins populate the annotations.
Annotations []Annotation
// If the container has specified the TerminationMessagePath, then // If the container has specified the TerminationMessagePath, then
// this directory will be used to create and mount the log file to // this directory will be used to create and mount the log file to
// container.TerminationMessagePath // container.TerminationMessagePath
......
...@@ -202,7 +202,7 @@ func (m *kubeGenericRuntimeManager) generateContainerConfig(container *v1.Contai ...@@ -202,7 +202,7 @@ func (m *kubeGenericRuntimeManager) generateContainerConfig(container *v1.Contai
Args: args, Args: args,
WorkingDir: container.WorkingDir, WorkingDir: container.WorkingDir,
Labels: newContainerLabels(container, pod, containerType), Labels: newContainerLabels(container, pod, containerType),
Annotations: newContainerAnnotations(container, pod, restartCount), Annotations: newContainerAnnotations(container, pod, restartCount, opts),
Devices: makeDevices(opts), Devices: makeDevices(opts),
Mounts: m.makeMounts(opts, container), Mounts: m.makeMounts(opts, container),
LogPath: containerLogsPath, LogPath: containerLogsPath,
......
...@@ -222,7 +222,7 @@ func makeExpectedConfig(m *kubeGenericRuntimeManager, pod *v1.Pod, containerInde ...@@ -222,7 +222,7 @@ func makeExpectedConfig(m *kubeGenericRuntimeManager, pod *v1.Pod, containerInde
Args: []string(nil), Args: []string(nil),
WorkingDir: container.WorkingDir, WorkingDir: container.WorkingDir,
Labels: newContainerLabels(container, pod, kubecontainer.ContainerTypeRegular), Labels: newContainerLabels(container, pod, kubecontainer.ContainerTypeRegular),
Annotations: newContainerAnnotations(container, pod, restartCount), Annotations: newContainerAnnotations(container, pod, restartCount, opts),
Devices: makeDevices(opts), Devices: makeDevices(opts),
Mounts: m.makeMounts(opts, container), Mounts: m.makeMounts(opts, container),
LogPath: containerLogsPath, LogPath: containerLogsPath,
......
...@@ -111,8 +111,14 @@ func newContainerLabels(container *v1.Container, pod *v1.Pod, containerType kube ...@@ -111,8 +111,14 @@ func newContainerLabels(container *v1.Container, pod *v1.Pod, containerType kube
} }
// newContainerAnnotations creates container annotations from v1.Container and v1.Pod. // newContainerAnnotations creates container annotations from v1.Container and v1.Pod.
func newContainerAnnotations(container *v1.Container, pod *v1.Pod, restartCount int) map[string]string { func newContainerAnnotations(container *v1.Container, pod *v1.Pod, restartCount int, opts *kubecontainer.RunContainerOptions) map[string]string {
annotations := map[string]string{} annotations := map[string]string{}
// Kubelet always overrides device plugin annotations if they are conflicting
for _, a := range opts.Annotations {
annotations[a.Name] = a.Value
}
annotations[containerHashLabel] = strconv.FormatUint(kubecontainer.HashContainer(container), 16) annotations[containerHashLabel] = strconv.FormatUint(kubecontainer.HashContainer(container), 16)
annotations[containerRestartCountLabel] = strconv.Itoa(restartCount) annotations[containerRestartCountLabel] = strconv.Itoa(restartCount)
annotations[containerTerminationMessagePathLabel] = container.TerminationMessagePath annotations[containerTerminationMessagePathLabel] = container.TerminationMessagePath
......
...@@ -156,6 +156,11 @@ func TestContainerAnnotations(t *testing.T) { ...@@ -156,6 +156,11 @@ func TestContainerAnnotations(t *testing.T) {
restartCount := 5 restartCount := 5
deletionGracePeriod := int64(10) deletionGracePeriod := int64(10)
terminationGracePeriod := int64(10) terminationGracePeriod := int64(10)
opts := &kubecontainer.RunContainerOptions{
Annotations: []kubecontainer.Annotation{
{Name: "Foo", Value: "bar"},
},
}
lifecycle := &v1.Lifecycle{ lifecycle := &v1.Lifecycle{
// Left PostStart as nil // Left PostStart as nil
PreStop: &v1.Handler{ PreStop: &v1.Handler{
...@@ -216,11 +221,14 @@ func TestContainerAnnotations(t *testing.T) { ...@@ -216,11 +221,14 @@ func TestContainerAnnotations(t *testing.T) {
} }
// Test whether we can get right information from label // Test whether we can get right information from label
annotations := newContainerAnnotations(container, pod, restartCount) annotations := newContainerAnnotations(container, pod, restartCount, opts)
containerInfo := getContainerInfoFromAnnotations(annotations) containerInfo := getContainerInfoFromAnnotations(annotations)
if !reflect.DeepEqual(containerInfo, expected) { if !reflect.DeepEqual(containerInfo, expected) {
t.Errorf("expected %v, got %v", expected, containerInfo) t.Errorf("expected %v, got %v", expected, containerInfo)
} }
if v, ok := annotations[opts.Annotations[0].Name]; !ok || v != opts.Annotations[0].Value {
t.Errorf("expected annotation %s to exist got %v, %v", opts.Annotations[0].Name, ok, v)
}
// Test when DeletionGracePeriodSeconds, TerminationGracePeriodSeconds and Lifecycle are nil, // Test when DeletionGracePeriodSeconds, TerminationGracePeriodSeconds and Lifecycle are nil,
// the information got from annotations should also be nil // the information got from annotations should also be nil
...@@ -232,11 +240,14 @@ func TestContainerAnnotations(t *testing.T) { ...@@ -232,11 +240,14 @@ func TestContainerAnnotations(t *testing.T) {
expected.PreStopHandler = nil expected.PreStopHandler = nil
// Because container is changed, the Hash should be updated // Because container is changed, the Hash should be updated
expected.Hash = kubecontainer.HashContainer(container) expected.Hash = kubecontainer.HashContainer(container)
annotations = newContainerAnnotations(container, pod, restartCount) annotations = newContainerAnnotations(container, pod, restartCount, opts)
containerInfo = getContainerInfoFromAnnotations(annotations) containerInfo = getContainerInfoFromAnnotations(annotations)
if !reflect.DeepEqual(containerInfo, expected) { if !reflect.DeepEqual(containerInfo, expected) {
t.Errorf("expected %v, got %v", expected, containerInfo) t.Errorf("expected %v, got %v", expected, containerInfo)
} }
if v, ok := annotations[opts.Annotations[0].Name]; !ok || v != opts.Annotations[0].Value {
t.Errorf("expected annotation %s to exist got %v, %v", opts.Annotations[0].Name, ok, v)
}
} }
func TestPodLabels(t *testing.T) { func TestPodLabels(t *testing.T) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment