-
Notifications
You must be signed in to change notification settings - Fork 144
Provider capabilities basic framework and eks.ec2.useHostnameFromFile support #3009
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
Open
levan-m
wants to merge
2
commits into
levan-m/drop-feature-provider
Choose a base branch
from
levan-m/providers-ddai
base: levan-m/drop-feature-provider
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| // Unless explicitly stated otherwise all files in this repository are licensed | ||
| // under the Apache License Version 2.0. | ||
| // This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| // Copyright 2016-present Datadog, Inc. | ||
|
|
||
| // This file is the declarative manifest of provider-conditional global | ||
| // mutations applied to the node agent pod template. It contains constants, | ||
| // type definitions, and the spec map. Behaviour (applier, helpers) lives in | ||
| // provider_apply.go. | ||
|
|
||
| package global | ||
|
|
||
| import ( | ||
| corev1 "k8s.io/api/core/v1" | ||
| "k8s.io/utils/ptr" | ||
|
|
||
| apicommon "github.com/DataDog/datadog-operator/api/datadoghq/common" | ||
| "github.com/DataDog/datadog-operator/internal/controller/datadogagent/providercaps" | ||
| "github.com/DataDog/datadog-operator/pkg/kubernetes" | ||
| ) | ||
|
|
||
| // cloudInitInstanceIDPath is the host path EKS-EC2 nodes expose containing the | ||
| // EC2 instance-id, used by the agent to derive a stable hostname. | ||
| const cloudInitInstanceIDPath = "/var/lib/cloud/data/instance-id" | ||
|
|
||
| // cloudInitInstanceIDVolumeName is the pod-level volume name for the cloud-init | ||
| // instance-id file. | ||
| const cloudInitInstanceIDVolumeName = "cloudinit-instance-id-file" | ||
|
|
||
| // NodeAgentProviderSpec is the provider-keyed capabilities map for the node | ||
| // agent pod template. The "" baseline applies to all providers; provider-keyed | ||
| // entries are applied on top (removals first, then additions). | ||
| // | ||
| // Mirrors the Helm chart's _provider-specific_ pod-template additions (see | ||
| // charts/datadog/templates/_containers-common-env.yaml and | ||
| // _container-cloudinit-volumemounts.yaml). | ||
| var NodeAgentProviderSpec = providercaps.NodeAgentProviderCapabilities{ | ||
| kubernetes.EKSEC2UseHostnameFromFileProvider: { | ||
| // DD_HOSTNAME_FILE points the agent at the EC2 instance-id so it | ||
| // derives a stable hostname even when the kubelet hostname differs. | ||
| // Helm includes this in containers-common-env, which renders into | ||
| // every main container AND every init container; mirror that. | ||
| EnvVars: []providercaps.EnvVarSet{ | ||
| { | ||
| EnvVar: corev1.EnvVar{ | ||
| Name: "DD_HOSTNAME_FILE", | ||
| Value: cloudInitInstanceIDPath, | ||
| }, | ||
| InitContainers: []apicommon.AgentContainerName{ | ||
| apicommon.InitConfigContainerName, | ||
| }, | ||
| }, | ||
| }, | ||
| // HostPath mount of the instance-id file. Helm adds this on every | ||
| // agent-side main container; enumerate the same set. | ||
| Volumes: []providercaps.VolumeAndMount{ | ||
| { | ||
| Volume: corev1.Volume{ | ||
| Name: cloudInitInstanceIDVolumeName, | ||
| VolumeSource: corev1.VolumeSource{ | ||
| HostPath: &corev1.HostPathVolumeSource{ | ||
| Path: cloudInitInstanceIDPath, | ||
| Type: ptr.To(corev1.HostPathFile), | ||
| }, | ||
| }, | ||
| }, | ||
| Mount: corev1.VolumeMount{ | ||
| Name: cloudInitInstanceIDVolumeName, | ||
| MountPath: cloudInitInstanceIDPath, | ||
| ReadOnly: true, | ||
| }, | ||
| Containers: []apicommon.AgentContainerName{ | ||
| apicommon.CoreAgentContainerName, | ||
| apicommon.TraceAgentContainerName, | ||
| apicommon.ProcessAgentContainerName, | ||
| apicommon.SystemProbeContainerName, | ||
| apicommon.SecurityAgentContainerName, | ||
| apicommon.AgentDataPlaneContainerName, | ||
| apicommon.OtelAgent, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
95 changes: 95 additions & 0 deletions
95
internal/controller/datadogagent/global/provider_spec_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| // Unless explicitly stated otherwise all files in this repository are licensed | ||
| // under the Apache License Version 2.0. | ||
| // This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| // Copyright 2016-present Datadog, Inc. | ||
|
|
||
| package global | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| corev1 "k8s.io/api/core/v1" | ||
| "k8s.io/utils/ptr" | ||
|
|
||
| apicommon "github.com/DataDog/datadog-operator/api/datadoghq/common" | ||
| "github.com/DataDog/datadog-operator/internal/controller/datadogagent/feature" | ||
| "github.com/DataDog/datadog-operator/internal/controller/datadogagent/providercaps" | ||
| "github.com/DataDog/datadog-operator/pkg/kubernetes" | ||
| ) | ||
|
|
||
| // TestNodeAgentProviderSpec_EKS asserts the EKS-EC2 provider spec injects: | ||
| // - DD_HOSTNAME_FILE env var on every main container and on init-config | ||
| // - cloudinit-instance-id-file HostPath volume + read-only mount on every | ||
| // enumerated agent container | ||
| func TestNodeAgentProviderSpec_EKS(t *testing.T) { | ||
| tmpl := &corev1.PodTemplateSpec{ | ||
| Spec: corev1.PodSpec{ | ||
| Containers: []corev1.Container{ | ||
| {Name: string(apicommon.CoreAgentContainerName)}, | ||
| {Name: string(apicommon.TraceAgentContainerName)}, | ||
| }, | ||
| InitContainers: []corev1.Container{ | ||
| {Name: string(apicommon.InitVolumeContainerName)}, | ||
| {Name: string(apicommon.InitConfigContainerName)}, | ||
| }, | ||
| }, | ||
| } | ||
| mgr := feature.NewPodTemplateManagers(tmpl) | ||
|
|
||
| providercaps.ApplyNodeAgentProviderCapabilities(mgr, kubernetes.EKSEC2UseHostnameFromFileProvider, NodeAgentProviderSpec) | ||
|
|
||
| wantEnv := corev1.EnvVar{Name: "DD_HOSTNAME_FILE", Value: "/var/lib/cloud/data/instance-id"} | ||
| for _, c := range tmpl.Spec.Containers { | ||
| assert.Contains(t, c.Env, wantEnv, "main container %q should get DD_HOSTNAME_FILE", c.Name) | ||
| } | ||
| // init-config receives the env var; init-volume does not (matches helm). | ||
| var initConfig, initVolume corev1.Container | ||
| for _, c := range tmpl.Spec.InitContainers { | ||
| switch c.Name { | ||
| case string(apicommon.InitConfigContainerName): | ||
| initConfig = c | ||
| case string(apicommon.InitVolumeContainerName): | ||
| initVolume = c | ||
| } | ||
| } | ||
| assert.Contains(t, initConfig.Env, wantEnv, "init-config should get DD_HOSTNAME_FILE") | ||
| assert.NotContains(t, initVolume.Env, wantEnv, "init-volume should NOT get DD_HOSTNAME_FILE") | ||
|
|
||
| // HostPath volume is added at pod level. | ||
| var vol corev1.Volume | ||
| for _, v := range tmpl.Spec.Volumes { | ||
| if v.Name == "cloudinit-instance-id-file" { | ||
| vol = v | ||
| } | ||
| } | ||
| assert.NotNil(t, vol.HostPath, "cloudinit-instance-id-file volume should be a HostPath") | ||
| assert.Equal(t, "/var/lib/cloud/data/instance-id", vol.HostPath.Path) | ||
| assert.Equal(t, ptr.To(corev1.HostPathFile), vol.HostPath.Type) | ||
|
|
||
| // Read-only mount is added on each main container. | ||
| wantMount := corev1.VolumeMount{ | ||
| Name: "cloudinit-instance-id-file", | ||
| MountPath: "/var/lib/cloud/data/instance-id", | ||
| ReadOnly: true, | ||
| } | ||
| for _, c := range tmpl.Spec.Containers { | ||
| assert.Contains(t, c.VolumeMounts, wantMount, "main container %q should mount cloudinit-instance-id-file", c.Name) | ||
| } | ||
| } | ||
|
|
||
| // TestNodeAgentProviderSpec_NoProvider asserts no provider == no mutations. | ||
| func TestNodeAgentProviderSpec_NoProvider(t *testing.T) { | ||
| tmpl := &corev1.PodTemplateSpec{ | ||
| Spec: corev1.PodSpec{ | ||
| Containers: []corev1.Container{{Name: string(apicommon.CoreAgentContainerName)}}, | ||
| }, | ||
| } | ||
| mgr := feature.NewPodTemplateManagers(tmpl) | ||
|
|
||
| providercaps.ApplyNodeAgentProviderCapabilities(mgr, "", NodeAgentProviderSpec) | ||
|
|
||
| assert.Empty(t, tmpl.Spec.Containers[0].Env) | ||
| assert.Empty(t, tmpl.Spec.Containers[0].VolumeMounts) | ||
| assert.Empty(t, tmpl.Spec.Volumes) | ||
| } |
190 changes: 190 additions & 0 deletions
190
internal/controller/datadogagent/providercaps/provider_capabilities.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,190 @@ | ||
| // Unless explicitly stated otherwise all files in this repository are licensed | ||
| // under the Apache License Version 2.0. | ||
| // This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| // Copyright 2016-present Datadog, Inc. | ||
|
|
||
| // Package providercaps holds the provider-conditional pod-template mutation | ||
| // framework. Both per-feature (feature.ProviderAwareFeature) and global | ||
| // (global.ApplyGlobalNodeAgentSpec) consumers declare their mutations as a | ||
| // NodeAgentProviderCapabilities map and apply them via | ||
| // ApplyNodeAgentProviderCapabilities. | ||
| package providercaps | ||
|
|
||
| import ( | ||
| corev1 "k8s.io/api/core/v1" | ||
|
|
||
| apicommon "github.com/DataDog/datadog-operator/api/datadoghq/common" | ||
| "github.com/DataDog/datadog-operator/internal/controller/datadogagent/merger" | ||
| ) | ||
|
|
||
| // PodTemplateManager is the minimal interface ApplyNodeAgentProviderCapabilities | ||
| // needs from a pod-template manager. feature.PodTemplateManagers satisfies it | ||
| // structurally, so callers pass their existing manager unchanged. | ||
| type PodTemplateManager interface { | ||
| PodTemplateSpec() *corev1.PodTemplateSpec | ||
| EnvVar() merger.EnvVarManager | ||
| Volume() merger.VolumeManager | ||
| VolumeMount() merger.VolumeMountManager | ||
| } | ||
|
|
||
| // VolumeAndMount groups a pod-level volume with a container volume mount. | ||
| // Volume is added to the pod spec; Mount is added to each listed container. | ||
| type VolumeAndMount struct { | ||
| Volume corev1.Volume | ||
| Mount corev1.VolumeMount | ||
| Containers []apicommon.AgentContainerName | ||
| } | ||
|
|
||
| // EnvVarSet groups an env var with its target containers. | ||
| // Empty Containers means the env var is added to all agent containers. | ||
| // InitContainers lists init containers that should also receive the env var | ||
| // (init containers are not iterated by the all-containers AddEnvVar path). | ||
| type EnvVarSet struct { | ||
| EnvVar corev1.EnvVar | ||
| Containers []apicommon.AgentContainerName | ||
| InitContainers []apicommon.AgentContainerName | ||
| } | ||
|
|
||
| // ContainerMountRef identifies a volume mount by volume name and the containers | ||
| // it should be stripped from. Empty Containers means strip from all containers. | ||
| type ContainerMountRef struct { | ||
| VolumeName string | ||
| Containers []apicommon.AgentContainerName | ||
| } | ||
|
|
||
| // ProviderCapabilities holds the volumes, env vars, and removals for a | ||
| // specific provider entry in a NodeAgentProviderCapabilities map. | ||
| type ProviderCapabilities struct { | ||
| Volumes []VolumeAndMount | ||
| EnvVars []EnvVarSet | ||
| // RemoveVolumes strips named volumes (vol + all mounts) before provider additions run. | ||
| RemoveVolumes []string | ||
| // RemoveMounts strips specific container-volume mount pairs before provider additions run. | ||
| RemoveMounts []ContainerMountRef | ||
| // RemoveEnvVars strips env vars by name before provider additions run. | ||
| RemoveEnvVars []string | ||
| } | ||
|
|
||
| // NodeAgentProviderCapabilities maps a provider string to its capabilities. | ||
| // The empty string key "" is the baseline applied to all providers first. | ||
| // Provider-specific entries are then applied on top: removals first, additions second. | ||
| type NodeAgentProviderCapabilities = map[string]ProviderCapabilities | ||
|
|
||
| // ApplyNodeAgentProviderCapabilities applies all provider-conditional mutations. | ||
| // The baseline ("") entry is applied first. The provider-specific entry is then | ||
| // applied: removals run before additions so a provider can replace a baseline item | ||
| // by removing it and re-adding a modified version. | ||
| func ApplyNodeAgentProviderCapabilities(mgr PodTemplateManager, provider string, caps NodeAgentProviderCapabilities) { | ||
| if len(caps) == 0 { | ||
| return | ||
| } | ||
|
|
||
| applyAdditions := func(c ProviderCapabilities) { | ||
| addedVolumes := make(map[string]bool) | ||
| for _, vm := range c.Volumes { | ||
| if !addedVolumes[vm.Volume.Name] { | ||
| mgr.Volume().AddVolume(&vm.Volume) | ||
| addedVolumes[vm.Volume.Name] = true | ||
| } | ||
| mgr.VolumeMount().AddVolumeMountToContainers(&vm.Mount, vm.Containers) | ||
| } | ||
| for _, ev := range c.EnvVars { | ||
| if len(ev.Containers) == 0 { | ||
| mgr.EnvVar().AddEnvVar(&ev.EnvVar) | ||
| } else { | ||
| mgr.EnvVar().AddEnvVarToContainers(ev.Containers, &ev.EnvVar) | ||
| } | ||
| for _, ic := range ev.InitContainers { | ||
| mgr.EnvVar().AddEnvVarToInitContainer(ic, &ev.EnvVar) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| applyRemovals := func(c ProviderCapabilities) { | ||
| tmpl := mgr.PodTemplateSpec() | ||
| for _, name := range c.RemoveVolumes { | ||
| stripVolume(tmpl, name) | ||
| } | ||
| for _, ref := range c.RemoveMounts { | ||
| stripMounts(tmpl, ref.VolumeName, ref.Containers) | ||
| } | ||
| for _, name := range c.RemoveEnvVars { | ||
| stripEnvVar(tmpl, name) | ||
| } | ||
| } | ||
|
|
||
| if baseline, ok := caps[""]; ok { | ||
| applyAdditions(baseline) | ||
| } | ||
| if provider != "" { | ||
| if providerCaps, ok := caps[provider]; ok { | ||
| applyRemovals(providerCaps) | ||
| applyAdditions(providerCaps) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // stripVolume removes a named volume from the pod spec and all its mounts | ||
| // from every container and init container. | ||
| func stripVolume(tmpl *corev1.PodTemplateSpec, volumeName string) { | ||
| filtered := tmpl.Spec.Volumes[:0] | ||
| for _, v := range tmpl.Spec.Volumes { | ||
| if v.Name != volumeName { | ||
| filtered = append(filtered, v) | ||
| } | ||
| } | ||
| tmpl.Spec.Volumes = filtered | ||
| stripMounts(tmpl, volumeName, nil) | ||
| } | ||
|
|
||
| // stripMounts removes the mount for volumeName from the specified containers. | ||
| // If containers is nil or empty, the mount is stripped from every container | ||
| // and init container. | ||
| func stripMounts(tmpl *corev1.PodTemplateSpec, volumeName string, containers []apicommon.AgentContainerName) { | ||
| targetAll := len(containers) == 0 | ||
| targetSet := make(map[string]bool, len(containers)) | ||
| for _, c := range containers { | ||
| targetSet[string(c)] = true | ||
| } | ||
|
|
||
| for i := range tmpl.Spec.Containers { | ||
| if targetAll || targetSet[tmpl.Spec.Containers[i].Name] { | ||
| tmpl.Spec.Containers[i].VolumeMounts = removeMountByName(tmpl.Spec.Containers[i].VolumeMounts, volumeName) | ||
| } | ||
| } | ||
| for i := range tmpl.Spec.InitContainers { | ||
| if targetAll || targetSet[tmpl.Spec.InitContainers[i].Name] { | ||
| tmpl.Spec.InitContainers[i].VolumeMounts = removeMountByName(tmpl.Spec.InitContainers[i].VolumeMounts, volumeName) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // stripEnvVar removes an env var by name from every container and init container. | ||
| func stripEnvVar(tmpl *corev1.PodTemplateSpec, name string) { | ||
| for i := range tmpl.Spec.Containers { | ||
| tmpl.Spec.Containers[i].Env = removeEnvVarByName(tmpl.Spec.Containers[i].Env, name) | ||
| } | ||
| for i := range tmpl.Spec.InitContainers { | ||
| tmpl.Spec.InitContainers[i].Env = removeEnvVarByName(tmpl.Spec.InitContainers[i].Env, name) | ||
| } | ||
| } | ||
|
|
||
| func removeMountByName(mounts []corev1.VolumeMount, name string) []corev1.VolumeMount { | ||
| out := mounts[:0] | ||
| for _, m := range mounts { | ||
| if m.Name != name { | ||
| out = append(out, m) | ||
| } | ||
| } | ||
| return out | ||
| } | ||
|
|
||
| func removeEnvVarByName(envs []corev1.EnvVar, name string) []corev1.EnvVar { | ||
| out := envs[:0] | ||
| for _, e := range envs { | ||
| if e.Name != name { | ||
| out = append(out, e) | ||
| } | ||
| } | ||
| return out | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The EKS
useHostnameFromFilecapability addsDD_HOSTNAME_FILEto all agent containers, but this mount target list excludesapicommon.UnprivilegedSingleAgentContainerName. In single-container strategy, onlyunprivileged-single-agentruns, so the env var points to/var/lib/cloud/data/instance-idwithout mounting that host file, which breaks the advertised provider behavior for this mode. Add the single-agent container name to this mount list (or apply the mount to all agent containers) so the file exists where the env var points.Useful? React with 👍 / 👎.