Skip to content

Commit 7afbf5c

Browse files
AliDatadogAliDatadog
committed
Fix bug with false
1 parent 698105b commit 7afbf5c

File tree

2 files changed

+98
-71
lines changed

2 files changed

+98
-71
lines changed

‎internal/controller/datadogagent/feature/otelcollector/feature.go‎

Lines changed: 37 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,13 @@ func buildOtelCollectorFeature(options *feature.Options) feature.Feature {
4242
}
4343

4444
type otelCollectorFeature struct {
45-
customConfig *v2alpha1.CustomConfig
46-
owner metav1.Object
47-
configMapName string
48-
ports []*corev1.ContainerPort
49-
coreAgentConfig coreAgentConfig
50-
useStandaloneImage *bool
45+
customConfig *v2alpha1.CustomConfig
46+
owner metav1.Object
47+
configMapName string
48+
ports []*corev1.ContainerPort
49+
coreAgentConfig coreAgentConfig
50+
useStandaloneImage *bool
51+
nodeAgentImageOverride *v2alpha1.AgentImageConfig
5152

5253
customConfigAnnotationKey string
5354
customConfigAnnotationValue string
@@ -85,6 +86,7 @@ func (o *otelCollectorFeature) Configure(dda metav1.Object, ddaSpec *v2alpha1.Da
8586
if nodeAgent, ok := ddaSpec.Override[v2alpha1.NodeAgentComponentName]; ok {
8687
if nodeAgent.Image != nil {
8788
agentVersion = common.GetAgentVersionFromImage(*nodeAgent.Image)
89+
o.nodeAgentImageOverride = nodeAgent.Image
8890
}
8991
}
9092

@@ -99,14 +101,6 @@ func (o *otelCollectorFeature) Configure(dda metav1.Object, ddaSpec *v2alpha1.Da
99101
o.useStandaloneImage = apiutils.NewBoolPointer(false)
100102
}
101103

102-
// Log the final decision for debugging
103-
if apiutils.BoolValue(ddaSpec.Features.OtelCollector.Enabled) {
104-
o.logger.Info("OTel Collector configuration",
105-
"agent_version", agentVersion,
106-
"version_supported", supportedVersion,
107-
"use_standalone_image", apiutils.BoolValue(o.useStandaloneImage))
108-
}
109-
110104
if ddaSpec.Features.OtelCollector.CoreConfig != nil {
111105
o.coreAgentConfig.enabled = ddaSpec.Features.OtelCollector.CoreConfig.Enabled
112106
o.coreAgentConfig.extension_timeout = ddaSpec.Features.OtelCollector.CoreConfig.ExtensionTimeout
@@ -210,8 +204,8 @@ func (o *otelCollectorFeature) ManageClusterAgent(managers feature.PodTemplateMa
210204

211205
func (o *otelCollectorFeature) ManageNodeAgent(managers feature.PodTemplateManagers, provider string) error {
212206
if apiutils.BoolValue(o.useStandaloneImage) {
213-
// When UseStandaloneImage is true (default), use the ddot-collector image for the otel-agent container
214-
// and ensure other containers don't use -full image
207+
// When UseStandaloneImage is true, use the ddot-collector image for the otel-agent container
208+
// and ensure other containers don't use -full image. Ignore any image overrides.
215209
for i, container := range managers.PodTemplateSpec().Spec.Containers {
216210
if container.Name == string(apicommon.OtelAgent) {
217211
image := images.FromString(container.Image).
@@ -225,19 +219,34 @@ func (o *otelCollectorFeature) ManageNodeAgent(managers feature.PodTemplateManag
225219
}
226220
}
227221
} else {
228-
// Use -full image for all containers when UseStandaloneImage is false
229-
// Note: if an image tag override is configured, this image tag will be overwritten
230-
image := &images.Image{}
231-
for i, container := range managers.PodTemplateSpec().Spec.Containers {
232-
image = images.FromString(container.Image).
233-
WithFull(true)
234-
managers.PodTemplateSpec().Spec.Containers[i].Image = image.ToString()
235-
}
222+
// When UseStandaloneImage is false, all containers (including OTel agent) should use the regular agent image with -full suffix
223+
// However, if there's an explicit image override, respect it and don't modify it
224+
if o.nodeAgentImageOverride != nil {
225+
// User has provided an explicit image override, respect it as-is
226+
o.logger.Info("Respecting explicit image override, skipping -full suffix modification",
227+
"override_image", o.nodeAgentImageOverride.Name+":"+o.nodeAgentImageOverride.Tag)
228+
} else {
229+
// No explicit override, apply the -full suffix logic
230+
image := &images.Image{}
231+
for i, container := range managers.PodTemplateSpec().Spec.Containers {
232+
if container.Name == string(apicommon.OtelAgent) {
233+
// For OTel agent container, keep the custom registry and tag but use the ddot-collector image name
234+
image = images.FromString(container.Image).
235+
WithName(images.DefaultAgentImageName).
236+
WithFull(true)
237+
} else {
238+
// For other containers, just add -full suffix
239+
image = images.FromString(container.Image).
240+
WithFull(true)
241+
}
242+
managers.PodTemplateSpec().Spec.Containers[i].Image = image.ToString()
243+
}
236244

237-
for i, container := range managers.PodTemplateSpec().Spec.InitContainers {
238-
image = images.FromString(container.Image).
239-
WithFull(true)
240-
managers.PodTemplateSpec().Spec.InitContainers[i].Image = image.ToString()
245+
for i, container := range managers.PodTemplateSpec().Spec.InitContainers {
246+
image = images.FromString(container.Image).
247+
WithFull(true)
248+
managers.PodTemplateSpec().Spec.InitContainers[i].Image = image.ToString()
249+
}
241250
}
242251
}
243252

‎internal/controller/datadogagent/feature/otelcollector/feature_test.go‎

Lines changed: 61 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -481,8 +481,7 @@ func Test_otelCollectorFeature_ManageNodeAgent(t *testing.T) {
481481
expectedOtelImage := images.GetLatestDdotCollectorImage()
482482
assert.Equal(t, expectedOtelImage, container.Image, "OTel Agent container should use ddot-collector image when UseStandaloneImage is true")
483483
} else {
484-
// Other containers should not use -full image when UseStandaloneImage is true
485-
assert.NotContains(t, container.Image, "-full", "Non-OTel containers should not use -full image when UseStandaloneImage is true")
484+
assert.Equal(t, images.GetLatestAgentImage(), container.Image, "Other containers should use agent image when UseStandaloneImage is true")
486485
}
487486
}
488487
assert.True(t, otelContainerFound, "OTel Agent container should be present")
@@ -496,16 +495,37 @@ func Test_otelCollectorFeature_ManageNodeAgent(t *testing.T) {
496495
WithOTelCollectorUseStandaloneImage(false).
497496
Build(),
498497
WantConfigure: true,
499-
Agent: test.NewDefaultComponentTest().WithWantFunc(
500-
func(t testing.TB, mgrInterface feature.PodTemplateManagers) {
498+
Agent: &test.ComponentTest{
499+
CreateFunc: func(t testing.TB) (feature.PodTemplateManagers, string) {
500+
// Create pod template with both CoreAgent and OtelAgent containers
501+
newPTS := corev1.PodTemplateSpec{
502+
Spec: corev1.PodSpec{
503+
Containers: []corev1.Container{
504+
{
505+
Name: string(apicommon.CoreAgentContainerName),
506+
Image: images.GetLatestAgentImageWithSuffix(false, false, true),
507+
Command: []string{"agent", "run"},
508+
},
509+
{
510+
Name: string(apicommon.OtelAgent),
511+
Image: images.GetLatestAgentImageWithSuffix(false, false, true),
512+
Command: []string{"otel-agent"},
513+
},
514+
},
515+
},
516+
}
517+
return fake.NewPodTemplateManagers(t, newPTS), kubernetes.DefaultProvider
518+
},
519+
WantFunc: func(t testing.TB, mgrInterface feature.PodTemplateManagers) {
501520
mgr := mgrInterface.(*fake.PodTemplateManagers)
502521

503-
// When UseStandaloneImage is false, all containers should use -full image
522+
// When UseStandaloneImage is false, all containers should use agent image with -full suffix
504523
for _, container := range mgr.PodTemplateSpec().Spec.Containers {
505-
assert.Contains(t, container.Image, "-full", "All containers should use -full image when UseStandaloneImage is false")
524+
expectedImage := images.GetLatestAgentImageWithSuffix(false, false, true)
525+
assert.Equal(t, expectedImage, container.Image, "All containers should use agent image with -full suffix when UseStandaloneImage is false")
506526
}
507527
},
508-
),
528+
},
509529
},
510530
}
511531

@@ -552,9 +572,10 @@ func Test_otelCollectorFeature_VersionCheck(t *testing.T) {
552572
func(t testing.TB, mgrInterface feature.PodTemplateManagers) {
553573
mgr := mgrInterface.(*fake.PodTemplateManagers)
554574
// Version 7.66.0 is below 7.67.0, so UseStandaloneImage should be disabled
555-
// All containers should use -full image
575+
// However, with explicit image override, the override should be respected as-is
576+
// No containers should have -full suffix when there's an explicit override
556577
for _, container := range mgr.PodTemplateSpec().Spec.Containers {
557-
assert.Contains(t, container.Image, "-full", "All containers should use -full image when agent version < 7.67.0")
578+
assert.NotContains(t, container.Image, "-full", "Containers should not use -full image when there's an explicit image override")
558579
}
559580
},
560581
),
@@ -572,19 +593,42 @@ func Test_otelCollectorFeature_VersionCheck(t *testing.T) {
572593
}).
573594
Build(),
574595
WantConfigure: true,
575-
Agent: test.NewDefaultComponentTest().WithWantFunc(
576-
func(t testing.TB, mgrInterface feature.PodTemplateManagers) {
596+
Agent: &test.ComponentTest{
597+
CreateFunc: func(t testing.TB) (feature.PodTemplateManagers, string) {
598+
newPTS := corev1.PodTemplateSpec{
599+
Spec: corev1.PodSpec{
600+
Containers: []corev1.Container{
601+
{
602+
Name: string(apicommon.CoreAgentContainerName),
603+
Image: "datadog/agent:7.68.0",
604+
Command: []string{"agent", "run"},
605+
},
606+
{
607+
Name: string(apicommon.OtelAgent),
608+
Image: "datadog/agent:7.68.0", // This will be changed to ddot-collector:7.68.0
609+
Command: []string{"otel-agent"},
610+
},
611+
},
612+
},
613+
}
614+
return fake.NewPodTemplateManagers(t, newPTS), kubernetes.DefaultProvider
615+
},
616+
WantFunc: func(t testing.TB, mgrInterface feature.PodTemplateManagers) {
577617
mgr := mgrInterface.(*fake.PodTemplateManagers)
578618
// Version 7.68.0 is >= 7.67.0, so UseStandaloneImage should be enabled
619+
// With image override, should use the override tag but with appropriate image names
579620
for _, container := range mgr.PodTemplateSpec().Spec.Containers {
580621
if container.Name == string(apicommon.OtelAgent) {
581-
assert.Equal(t, images.GetLatestDdotCollectorImage(), container.Image)
622+
// OTel Agent should use ddot-collector image with override tag
623+
// Since the override is datadog/agent:7.68.0, it should become datadog/ddot-collector:7.68.0
624+
assert.Equal(t, "datadog/ddot-collector:7.68.0", container.Image)
582625
} else {
583-
assert.Equal(t, images.GetLatestAgentImage(), container.Image)
626+
// Other containers should use the override image as-is
627+
assert.Equal(t, "datadog/agent:7.68.0", container.Image)
584628
}
585629
}
586630
},
587-
),
631+
},
588632
},
589633
{
590634
Name: "UseStandaloneImage disabled for 7.60.0 with image override",
@@ -603,8 +647,9 @@ func Test_otelCollectorFeature_VersionCheck(t *testing.T) {
603647
func(t testing.TB, mgrInterface feature.PodTemplateManagers) {
604648
mgr := mgrInterface.(*fake.PodTemplateManagers)
605649
// Version 7.60.0 is below 7.67.0, so UseStandaloneImage should be disabled
650+
// However, with explicit image override, the override should be respected as-is
606651
for _, container := range mgr.PodTemplateSpec().Spec.Containers {
607-
assert.Contains(t, container.Image, "-full", "All containers should use -full image when agent version < 7.67.0")
652+
assert.NotContains(t, container.Image, "-full", "Containers should not use -full image when there's an explicit image override")
608653
}
609654
},
610655
),
@@ -647,7 +692,7 @@ func Test_otelCollectorFeature_VersionCheck(t *testing.T) {
647692
// Version 7.67.0 is >= 7.67.0, so UseStandaloneImage should be enabled
648693
for _, container := range mgr.PodTemplateSpec().Spec.Containers {
649694
if container.Name == string(apicommon.OtelAgent) {
650-
// With custom image override, OTel Agent should use custom ddot-collector image
695+
// With custom image override, OTel Agent should use custom ddot-collector image and the custom registry + tag
651696
assert.Equal(t, "custom-registry/ddot-collector:7.67.0-custom", container.Image)
652697
} else {
653698
// With custom image override, non-OTel containers should use the custom image without -full suffix
@@ -657,33 +702,6 @@ func Test_otelCollectorFeature_VersionCheck(t *testing.T) {
657702
},
658703
},
659704
},
660-
{
661-
Name: "UseStandaloneImage disabled with invalid version string",
662-
DDA: testutils.NewDatadogAgentBuilder().
663-
WithOTelCollectorEnabled(true).
664-
WithOTelCollectorUseStandaloneImage(true).
665-
WithComponentOverride(v2alpha1.NodeAgentComponentName, v2alpha1.DatadogAgentComponentOverride{
666-
Image: &v2alpha1.AgentImageConfig{
667-
Name: "datadog/agent",
668-
Tag: "invalid-version",
669-
},
670-
}).
671-
Build(),
672-
WantConfigure: true,
673-
Agent: test.NewDefaultComponentTest().WithWantFunc(
674-
func(t testing.TB, mgrInterface feature.PodTemplateManagers) {
675-
mgr := mgrInterface.(*fake.PodTemplateManagers)
676-
// Invalid version strings should be treated as >= 7.67.0 per utils.IsAboveMinVersion behavior
677-
for _, container := range mgr.PodTemplateSpec().Spec.Containers {
678-
if container.Name == string(apicommon.OtelAgent) {
679-
assert.Equal(t, images.GetLatestDdotCollectorImage(), container.Image)
680-
} else {
681-
assert.Equal(t, images.GetLatestAgentImage(), container.Image)
682-
}
683-
}
684-
},
685-
),
686-
},
687705
}
688706

689707
tests.Run(t, buildOtelCollectorFeature)

0 commit comments

Comments
 (0)