Skip to content

Commit dcdead3

Browse files
zhuminyiclaude
andcommitted
Address PR review: tighten version gate and include mode in checksum
- Bump podCollectionOnNodeMinVersion from 7.58 to 7.60 to match the documented supported floor (the start-up fix landed in 7.60). - Gate on BOTH sides: skip the feature if the node-agent override image is parseable AND < 7.60 in addition to the existing cluster-side check (cluster-agent or CCR). A cluster-agent-only upgrade can no longer mount a pod_collection_mode: node_kubelet check into older node-agents. - Move PodCollectionMode resolution above the default-config checksum and include pod_collection_on_node in the checksum input, so toggling the field changes the operator-managed cluster-agent pod-template annotation and forces a rollout instead of leaving the in-memory ConfigMap stale. - Tests: add WithNodeAgentImage builder, cover the node-agent-image fallback row, and extend ksmClusterAgentWantFunc with a withPodCollectionOnNode() option so expected-hash dicts include the new key for the active rows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5f8ac31 commit dcdead3

4 files changed

Lines changed: 121 additions & 59 deletions

File tree

internal/controller/datadogagent/feature/kubernetesstatecore/const.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,11 @@ const (
2929
// one of these alongside the cluster-side ConfigMap).
3030
defaultKSMPodsOnNodeConf string = "kube-state-metrics-core-pods-on-node-config"
3131

32-
// Minimum agent / cluster-agent version supporting the
32+
// Minimum agent / cluster-agent / node-agent version supporting the
3333
// pod_collection_mode field used by PodCollectionMode=node_kubelet.
34-
// node_kubelet shipped in 7.58; the startup fix landed in 7.60.
35-
podCollectionOnNodeMinVersion = "7.58.0-0"
34+
// node_kubelet shipped in 7.58; the startup fix landed in 7.60, so 7.60
35+
// is the supported floor across all components that load the check.
36+
podCollectionOnNodeMinVersion = "7.60.0-0"
3637
)
3738

3839
// GetKubeStateMetricsRBACResourceName return the RBAC resources name

internal/controller/datadogagent/feature/kubernetesstatecore/feature.go

Lines changed: 68 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -157,69 +157,54 @@ func (f *ksmFeature) Configure(dda metav1.Object, ddaSpec *v2alpha1.DatadogAgent
157157
}
158158
}
159159

160+
// Capture the user-supplied custom config (if any) so PodCollectionMode
161+
// resolution below can decide whether to mutate the cluster-side YAML.
160162
if ddaSpec.Features.KubeStateMetricsCore.Conf != nil {
161163
f.customConfig = ddaSpec.Features.KubeStateMetricsCore.Conf
162-
hash, err := comparison.GenerateMD5ForSpec(f.customConfig)
163-
if err != nil {
164-
f.logger.Error(err, "couldn't generate hash for ksm core custom config")
165-
} else {
166-
f.logger.V(2).Info("built ksm core from custom config", "hash", hash)
167-
}
168-
f.customConfigAnnotationValue = hash
169-
f.customConfigAnnotationKey = object.GetChecksumAnnotationKey(feature.KubernetesStateCoreIDType)
170-
} else {
171-
// Generate dynamic checksum for default configuration (based on user provided collectCrMetrics field and whether or not APIServices/CRD metrics are collected)
172-
defaultConfigData := map[string]any{
173-
"collect_crds": f.collectCRDMetrics,
174-
"collect_apiservices": f.collectAPIServiceMetrics,
175-
"collect_cr_metrics": f.collectCrMetrics,
176-
}
177-
178-
hash, err := comparison.GenerateMD5ForSpec(defaultConfigData)
179-
if err != nil {
180-
f.logger.Error(err, "couldn't generate hash for default ksm core config")
181-
} else {
182-
f.logger.V(2).Info("generated default ksm core config hash", "hash", hash, "config", defaultConfigData)
183-
}
184-
f.customConfigAnnotationValue = hash
185-
f.customConfigAnnotationKey = object.GetChecksumAnnotationKey(feature.KubernetesStateCoreIDType)
186164
}
187165

188-
f.configConfigMapName = constants.GetConfName(dda, f.customConfig, defaultKubeStateMetricsCoreConf)
189-
190-
// Resolve PodCollectionMode. When node_kubelet is requested AND the
191-
// agent version supports it, switch the cluster-side instance to
192-
// pod_collection_mode: cluster_unassigned (only when the operator owns
193-
// the cluster-side config) and deploy a pods-only check to every node
194-
// agent. When the user supplies their own .Conf, the operator still
195-
// deploys the node-side check but does not mutate the user's YAML;
196-
// they are responsible for setting cluster_unassigned themselves.
166+
// Resolve PodCollectionMode. When node_kubelet is requested AND every
167+
// component that loads the check is version-compatible, switch the
168+
// cluster-side instance to pod_collection_mode: cluster_unassigned
169+
// (only when the operator owns the cluster-side config) and deploy a
170+
// pods-only check to every node agent. When the user supplies their
171+
// own .Conf, the operator still deploys the node-side check but does
172+
// not mutate the user's YAML; they are responsible for setting
173+
// cluster_unassigned themselves.
197174
if mode := ddaSpec.Features.KubeStateMetricsCore.PodCollectionMode; mode != nil &&
198175
*mode == v2alpha1.KSMPodCollectionModeNodeKubelet {
199176
f.podCollectionOnNode = true
200-
// Version compatibility check on the component that runs the
201-
// cluster-side check (CCR if cluster-checks-runners are enabled,
202-
// otherwise the cluster-agent). Unparseable tags (`:dev`, custom
203-
// registries, etc.) are assumed compatible — matches the existing
204-
// pattern for the apiservices/CRD checks above.
205-
var override *v2alpha1.DatadogAgentComponentOverride
206-
if ovr, ok := ddaSpec.Override[v2alpha1.ClusterAgentComponentName]; ok {
207-
override = ovr
208-
}
177+
// Version compatibility check on BOTH sides: the cluster-side
178+
// component that runs the cluster_unassigned check (CCR if cluster-
179+
// checks-runners are enabled, otherwise the cluster-agent) AND the
180+
// node-agent that runs the node_kubelet check. If either image tag
181+
// is parseable AND below the supported floor, skip the feature so
182+
// the operator doesn't mount an unsupported file into a node-agent
183+
// that would silently fall back to default mode and double-collect.
184+
// Unparseable tags (`:dev`, custom registries, etc.) are assumed
185+
// compatible — matches the existing pattern for the apiservices/CRD
186+
// checks above.
187+
componentsToCheck := []v2alpha1.ComponentName{v2alpha1.NodeAgentComponentName}
209188
if f.runInClusterChecksRunner {
210-
if ovr, ok := ddaSpec.Override[v2alpha1.ClusterChecksRunnerComponentName]; ok {
211-
override = ovr
212-
}
189+
componentsToCheck = append(componentsToCheck, v2alpha1.ClusterChecksRunnerComponentName)
190+
} else {
191+
componentsToCheck = append(componentsToCheck, v2alpha1.ClusterAgentComponentName)
213192
}
214-
if override != nil && override.Image != nil {
215-
agentVersion := common.GetAgentVersionFromImage(*override.Image)
193+
for _, comp := range componentsToCheck {
194+
ovr, ok := ddaSpec.Override[comp]
195+
if !ok || ovr == nil || ovr.Image == nil {
196+
continue
197+
}
198+
agentVersion := common.GetAgentVersionFromImage(*ovr.Image)
216199
fallback := true // assume compatible when unparseable
217200
if !utils.IsAboveMinVersion(agentVersion, podCollectionOnNodeMinVersion, &fallback) {
218201
f.logger.Info(
219-
"PodCollectionMode=node_kubelet requires agent >= 7.58; falling back to default",
202+
"PodCollectionMode=node_kubelet requires agent >= 7.60; falling back to default",
203+
"component", string(comp),
220204
"version", agentVersion,
221205
)
222206
f.podCollectionOnNode = false
207+
break
223208
}
224209
}
225210
if f.podCollectionOnNode {
@@ -237,6 +222,41 @@ func (f *ksmFeature) Configure(dda metav1.Object, ddaSpec *v2alpha1.DatadogAgent
237222
}
238223
}
239224
}
225+
226+
// Compute the checksum annotation. With f.podCollectionOnNode resolved
227+
// above, toggling the field changes the input here, which propagates
228+
// to the cluster-agent pod-template annotation and forces a rollout.
229+
if f.customConfig != nil {
230+
hash, err := comparison.GenerateMD5ForSpec(f.customConfig)
231+
if err != nil {
232+
f.logger.Error(err, "couldn't generate hash for ksm core custom config")
233+
} else {
234+
f.logger.V(2).Info("built ksm core from custom config", "hash", hash)
235+
}
236+
f.customConfigAnnotationValue = hash
237+
f.customConfigAnnotationKey = object.GetChecksumAnnotationKey(feature.KubernetesStateCoreIDType)
238+
} else {
239+
// Dynamic checksum for the default configuration. Includes every
240+
// input that affects the rendered cluster-side ConfigMap so that
241+
// toggling any of them forces a rollout of the consumer.
242+
defaultConfigData := map[string]any{
243+
"collect_crds": f.collectCRDMetrics,
244+
"collect_apiservices": f.collectAPIServiceMetrics,
245+
"collect_cr_metrics": f.collectCrMetrics,
246+
"pod_collection_on_node": f.podCollectionOnNode,
247+
}
248+
249+
hash, err := comparison.GenerateMD5ForSpec(defaultConfigData)
250+
if err != nil {
251+
f.logger.Error(err, "couldn't generate hash for default ksm core config")
252+
} else {
253+
f.logger.V(2).Info("generated default ksm core config hash", "hash", hash, "config", defaultConfigData)
254+
}
255+
f.customConfigAnnotationValue = hash
256+
f.customConfigAnnotationKey = object.GetChecksumAnnotationKey(feature.KubernetesStateCoreIDType)
257+
}
258+
259+
f.configConfigMapName = constants.GetConfName(dda, f.customConfig, defaultKubeStateMetricsCoreConf)
240260
}
241261

242262
return output

internal/controller/datadogagent/feature/kubernetesstatecore/feature_test.go

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ func Test_ksmFeature_Configure(t *testing.T) {
153153
WithKSMPodCollectionMode(v2alpha1.KSMPodCollectionModeNodeKubelet).
154154
Build(),
155155
WantConfigure: true,
156-
ClusterAgent: ksmClusterAgentWantFunc(false),
156+
ClusterAgent: ksmClusterAgentWantFunc(false, withPodCollectionOnNode()),
157157
Agent: test.NewDefaultComponentTest().WithWantFunc(ksmAgentNodeWantFuncWithPodsOnNode),
158158
},
159159
{
@@ -164,7 +164,7 @@ func Test_ksmFeature_Configure(t *testing.T) {
164164
WithSingleContainerStrategy(true).
165165
Build(),
166166
WantConfigure: true,
167-
ClusterAgent: ksmClusterAgentWantFunc(false),
167+
ClusterAgent: ksmClusterAgentWantFunc(false, withPodCollectionOnNode()),
168168
Agent: test.NewDefaultComponentTest().WithWantFunc(ksmAgentSingleAgentWantFuncWithPodsOnNode),
169169
},
170170
{
@@ -179,11 +179,22 @@ func Test_ksmFeature_Configure(t *testing.T) {
179179
Agent: test.NewDefaultComponentTest().WithWantFunc(ksmAgentNodeWantFuncWithPodsOnNode),
180180
},
181181
{
182-
Name: "ksm-core enabled, podCollectionMode=node_kubelet but cluster-agent image < 7.58 -> fall back",
182+
Name: "ksm-core enabled, podCollectionMode=node_kubelet but cluster-agent image < 7.60 -> fall back",
183183
DDA: testutils.NewDatadogAgentBuilder().
184184
WithKSMEnabled(true).
185185
WithKSMPodCollectionMode(v2alpha1.KSMPodCollectionModeNodeKubelet).
186-
WithClusterAgentImage("gcr.io/datadoghq/cluster-agent:7.57.0").
186+
WithClusterAgentImage("gcr.io/datadoghq/cluster-agent:7.59.0").
187+
Build(),
188+
WantConfigure: true,
189+
ClusterAgent: ksmClusterAgentWantFunc(false),
190+
Agent: test.NewDefaultComponentTest().WithWantFunc(ksmAgentNodeWantFunc),
191+
},
192+
{
193+
Name: "ksm-core enabled, podCollectionMode=node_kubelet but node-agent image < 7.60 -> fall back",
194+
DDA: testutils.NewDatadogAgentBuilder().
195+
WithKSMEnabled(true).
196+
WithKSMPodCollectionMode(v2alpha1.KSMPodCollectionModeNodeKubelet).
197+
WithNodeAgentImage("gcr.io/datadoghq/agent:7.59.0").
187198
Build(),
188199
WantConfigure: true,
189200
ClusterAgent: ksmClusterAgentWantFunc(false),
@@ -194,7 +205,21 @@ func Test_ksmFeature_Configure(t *testing.T) {
194205
tests.Run(t, buildKSMFeature)
195206
}
196207

197-
func ksmClusterAgentWantFunc(hasCustomConfig bool) *test.ComponentTest {
208+
type ksmClusterAgentWantConfig struct {
209+
podCollectionOnNode bool
210+
}
211+
212+
type ksmClusterAgentOption func(*ksmClusterAgentWantConfig)
213+
214+
func withPodCollectionOnNode() ksmClusterAgentOption {
215+
return func(c *ksmClusterAgentWantConfig) { c.podCollectionOnNode = true }
216+
}
217+
218+
func ksmClusterAgentWantFunc(hasCustomConfig bool, opts ...ksmClusterAgentOption) *test.ComponentTest {
219+
cfg := ksmClusterAgentWantConfig{}
220+
for _, o := range opts {
221+
o(&cfg)
222+
}
198223
return test.NewDefaultComponentTest().WithWantFunc(
199224
func(t testing.TB, mgrInterface feature.PodTemplateManagers) {
200225
mgr := mgrInterface.(*fake.PodTemplateManagers)
@@ -226,9 +251,10 @@ func ksmClusterAgentWantFunc(hasCustomConfig bool) *test.ComponentTest {
226251
} else {
227252
// Verify default config annotation - CRDs and APIServices collected, no custom resource metrics
228253
defaultConfigData := map[string]any{
229-
"collect_crds": true,
230-
"collect_apiservices": true,
231-
"collect_cr_metrics": nil,
254+
"collect_crds": true,
255+
"collect_apiservices": true,
256+
"collect_cr_metrics": nil,
257+
"pod_collection_on_node": cfg.podCollectionOnNode,
232258
}
233259
hash, err := comparison.GenerateMD5ForSpec(defaultConfigData)
234260
assert.NoError(t, err)

pkg/testutils/builder.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1184,6 +1184,21 @@ func (builder *DatadogAgentBuilder) WithClusterAgentImage(image string) *Datadog
11841184
return builder
11851185
}
11861186

1187+
func (builder *DatadogAgentBuilder) WithNodeAgentImage(image string) *DatadogAgentBuilder {
1188+
if builder.datadogAgent.Spec.Override == nil {
1189+
builder.datadogAgent.Spec.Override = map[v2alpha1.ComponentName]*v2alpha1.DatadogAgentComponentOverride{}
1190+
}
1191+
1192+
if builder.datadogAgent.Spec.Override[v2alpha1.NodeAgentComponentName] == nil {
1193+
builder.datadogAgent.Spec.Override[v2alpha1.NodeAgentComponentName] = &v2alpha1.DatadogAgentComponentOverride{}
1194+
}
1195+
1196+
builder.datadogAgent.Spec.Override[v2alpha1.NodeAgentComponentName].Image = &v2alpha1.AgentImageConfig{
1197+
Name: image,
1198+
}
1199+
return builder
1200+
}
1201+
11871202
func (builder *DatadogAgentBuilder) WithClusterAgentDisabled(disabled bool) *DatadogAgentBuilder {
11881203
builder.WithComponentOverride(v2alpha1.ClusterAgentComponentName, v2alpha1.DatadogAgentComponentOverride{
11891204
Disabled: ptr.To(disabled),

0 commit comments

Comments
 (0)