Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
2 changes: 2 additions & 0 deletions api/datadoghq/common/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ const (
InitConfigContainerName AgentContainerName = "init-config"
// SeccompSetupContainerName is the name of the Seccomp Setup init container
SeccompSetupContainerName AgentContainerName = "seccomp-setup"
// HostProfilerSeccompSetupContainerName is the name of the host-profiler seccomp setup init container
HostProfilerSeccompSetupContainerName AgentContainerName = "host-profiler-seccomp-setup"

// UnprivilegedSingleAgentContainerName is the name of a container which may run
// any combination of Core, Trace and Process Agent processes in a single container.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,11 @@ func (o *hostProfilerFeature) ManageOtelAgentGateway(managers feature.PodTemplat
}

// resolveHostProfilerImage returns the host-profiler image to use for the seccomp init container
// and profile name. Both override sources are applied after ManageNodeAgent runs, so we read them
// during Configure instead. Priority: experimental annotation > spec.override.nodeAgent.image > "".
func resolveHostProfilerImage(dda metav1.Object, ddaSpec *v2alpha1.DatadogAgentSpec) string {
// 1. Experimental per-container image override annotation.
// and profile name when a host-profiler-specific image override is set via the experimental
// per-container annotation. Returns "" otherwise, causing ManageNodeAgent to fall back to the
// host-profiler container's own image (which is the correct until the host profiler's seccomp gets bundled in the
// Agent image).
func resolveHostProfilerImage(dda metav1.Object, _ *v2alpha1.DatadogAgentSpec) string {
if annotations := dda.GetAnnotations(); annotations != nil {
if raw := annotations["experimental.agent.datadoghq.com/image-override-config"]; raw != "" {
var overrides map[string]struct {
Expand All @@ -213,16 +214,5 @@ func resolveHostProfilerImage(dda metav1.Object, ddaSpec *v2alpha1.DatadogAgentS
}
}

// 2. spec.override.nodeAgent.image — applied to all agent containers including host-profiler.
if ddaSpec != nil {
if componentOverride, ok := ddaSpec.Override[v2alpha1.NodeAgentComponentName]; ok && componentOverride.Image != nil {
img := componentOverride.Image
if img.Name != "" && img.Tag != "" && !strings.Contains(img.Name, ":") {
return img.Name + ":" + img.Tag
}
return img.Name
}
}

return ""
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"testing"

apicommon "github.com/DataDog/datadog-operator/api/datadoghq/common"
"github.com/DataDog/datadog-operator/api/datadoghq/v2alpha1"
apiutils "github.com/DataDog/datadog-operator/api/utils"
"github.com/DataDog/datadog-operator/internal/controller/datadogagent/common"
"github.com/DataDog/datadog-operator/internal/controller/datadogagent/feature"
Expand Down Expand Up @@ -159,7 +158,6 @@ func TestResolveHostProfilerImage(t *testing.T) {
tests := []struct {
name string
annotations map[string]string
ddaSpec *v2alpha1.DatadogAgentSpec
want string
}{
{
Expand Down Expand Up @@ -206,32 +204,11 @@ func TestResolveHostProfilerImage(t *testing.T) {
},
want: "",
},
{
name: "spec.override.nodeAgent.image",
ddaSpec: &v2alpha1.DatadogAgentSpec{
Override: map[v2alpha1.ComponentName]*v2alpha1.DatadogAgentComponentOverride{
v2alpha1.NodeAgentComponentName: {Image: &v2alpha1.AgentImageConfig{Name: "gcr.io/x/agent", Tag: "7.99.0"}},
},
},
want: "gcr.io/x/agent:7.99.0",
},
{
name: "experimental annotation takes precedence over spec.override.nodeAgent.image",
annotations: map[string]string{
"experimental.agent.datadoghq.com/image-override-config": `{"host-profiler":{"name":"gcr.io/x/host-profiler:v2"}}`,
},
ddaSpec: &v2alpha1.DatadogAgentSpec{
Override: map[v2alpha1.ComponentName]*v2alpha1.DatadogAgentComponentOverride{
v2alpha1.NodeAgentComponentName: {Image: &v2alpha1.AgentImageConfig{Tag: "7.99.0"}},
},
},
want: "gcr.io/x/host-profiler:v2",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
dda := &metav1.ObjectMeta{Annotations: tt.annotations}
assert.Equal(t, tt.want, resolveHostProfilerImage(dda, tt.ddaSpec))
assert.Equal(t, tt.want, resolveHostProfilerImage(dda, nil))
})
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

corev1 "k8s.io/api/core/v1"

apicommon "github.com/DataDog/datadog-operator/api/datadoghq/common"
"github.com/DataDog/datadog-operator/internal/controller/datadogagent/common"
)

Expand Down Expand Up @@ -41,7 +42,7 @@ func defaultCapabilities() []corev1.Capability {

func buildSeccompSetupInitContainer(image string) corev1.Container {
return corev1.Container{
Name: "host-profiler-seccomp-setup",
Name: string(apicommon.HostProfilerSeccompSetupContainerName),
Image: image,
Command: []string{
"cp",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ func PodTemplateSpec(logger logr.Logger, manager feature.PodTemplateManagers, ov
}

for i, initContainer := range manager.PodTemplateSpec().Spec.InitContainers {
// host-profiler-seccomp-setup copies a seccomp profile JSON baked into the profiler image, not the agent image
// until the profiler is bundled in the agent.
if apicommon.AgentContainerName(initContainer.Name) == apicommon.HostProfilerSeccompSetupContainerName {
continue
Comment on lines +90 to +91

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Apply pull policy to the profiler seccomp init

When spec.override.nodeAgent.image.pullPolicy is set, this continue skips both the image rewrite and the pull-policy assignment below, so host-profiler-seccomp-setup keeps its default pull policy while the host-profiler container is updated. In clusters that reuse a mutable profiler tag and set Always/Never via the override, the init container can copy a stale or disallowed seccomp JSON before the main profiler image is pulled with the requested policy. The skip should avoid only changing the image, while still honoring override.Image.PullPolicy for this init container.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The Host Profiler is only supported through annotations with the image-override-config. spec.override.nodeAgent.image.pullPolicy doesn't apply to the annotation.

Comment on lines +90 to +91

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep the profiler seccomp image aligned with nodeAgent overrides

When Host Profiler is enabled and spec.override.nodeAgent.image points at a full replacement image, the regular container loop still rewrites the host-profiler container via OverrideAgentImage (line 77), but this new continue leaves host-profiler-seccomp-setup on the pre-override image and ManageNodeAgent has already hashed that pre-override image for LocalhostProfile. The pod then runs the profiler from the overridden image while copying/registering a seccomp profile from a different image/version, which can fail if the profile path or syscalls differ. This is not the earlier pull-policy comment; the fresh evidence is that the same patch's test expects the host-profiler container image to be overridden while this init container is not.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

spec.override.nodeAgent.image doesn't point at a full replacement image since the Host Profiler is not yet bundled in the agent image; the Host Profiler IS ONLY enabled by using its override annotation which has a higher precedence than spec.override.nodeAgent.image.
If the annotation wasn't set, we would be using the profiler's default image, which is the agent's image so it should also be correct for when the profiler is bundled in the agent's image.
This would need to be checked again when time comes, Codex is hard to follow here so I did my best to cover our bases.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@theomagellan I wonder if this implies that host-profiler image override should be skipped similar to host profiler init container. This will resolve image based on the annotation but if host-profiler image override is present resolved image name will be wrong.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

fyi, global settings may modify image registry and suffix

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

👋 Thank you for this! I added skip checks there as well. See 74695b6

}
manager.PodTemplateSpec().Spec.InitContainers[i].Image = images.OverrideAgentImage(initContainer.Image, override.Image)
if override.Image.PullPolicy != nil {
manager.PodTemplateSpec().Spec.InitContainers[i].ImagePullPolicy = *override.Image.PullPolicy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,39 @@ func TestPodTemplateSpec(t *testing.T) {
}, t)
},
},
{
name: "image override does not rewrite host-profiler-seccomp-setup init container",
existingManager: func() *fake.PodTemplateManagers {
manager := fake.NewPodTemplateManagers(t, v1.PodTemplateSpec{
Spec: v1.PodSpec{
Containers: []v1.Container{
{Name: string(apicommon.CoreAgentContainerName), Image: "agent:old"},
{Name: string(apicommon.HostProfiler), Image: "profiler:old"},
},
InitContainers: []v1.Container{
{Name: string(apicommon.InitVolumeContainerName), Image: "agent:old"},
{Name: string(apicommon.HostProfilerSeccompSetupContainerName), Image: "profiler:old"},
},
},
})
return manager
},
override: v2alpha1.DatadogAgentComponentOverride{
Image: &v2alpha1.AgentImageConfig{Name: "agent", Tag: "new"},
},
validateManager: func(t *testing.T, manager *fake.PodTemplateManagers) {
for _, c := range manager.PodTemplateSpec().Spec.Containers {
assert.NotEqual(t, "profiler:old", c.Image, "container %s image should have been overridden", c.Name)
}
for _, c := range manager.PodTemplateSpec().Spec.InitContainers {
if c.Name == string(apicommon.HostProfilerSeccompSetupContainerName) {
assert.Equal(t, "profiler:old", c.Image, "host-profiler-seccomp-setup image must not be overridden")
} else {
assert.NotEqual(t, "profiler:old", c.Image, "init container %s image should have been overridden", c.Name)
}
}
},
},
{
name: "add envs",
existingManager: func() *fake.PodTemplateManagers {
Expand Down
Loading