Skip to content

Commit 62e66bf

Browse files
committed
Report only the latest remote config update in the status
An update carries the full current config set for a product, not a delta, so the version and error recorded for it describe that update alone. Both were accumulating instead: the version was a running maximum, so a config that was removed or replaced by a lower-version one left a version reported as current that no longer applied, and an apply error was never cleared, so a single transient failure stayed in the status for the life of the process even after the product recovered. Reset both at the start of each update. The cumulative update counter still accumulates, since that is what it is for. Also stop importing pkg/config/remote/service from the status provider. It was imported only to share the default instance name, and it drags uptane and bbolt into every binary that links the provider, which showed up as six new packages in process-agent. Repeating one string is much cheaper. Assisted-by: Claude:claude-opus-5
1 parent 5660a87 commit 62e66bf

4 files changed

Lines changed: 52 additions & 3 deletions

File tree

comp/remote-config/rcstatus/impl/BUILD.bazel

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ go_library(
1414
"//comp/core/config",
1515
"//comp/core/status",
1616
"//comp/def",
17-
"//pkg/config/remote/service",
1817
"//pkg/config/utils",
1918
],
2019
)

comp/remote-config/rcstatus/impl/status.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,17 @@ import (
1515
"github.com/DataDog/datadog-agent/comp/core/config"
1616
"github.com/DataDog/datadog-agent/comp/core/status"
1717
compdef "github.com/DataDog/datadog-agent/comp/def"
18-
remoteconfig "github.com/DataDog/datadog-agent/pkg/config/remote/service"
1918
configutils "github.com/DataDog/datadog-agent/pkg/config/utils"
2019
)
2120

2221
//go:embed status_templates
2322
var templatesFS embed.FS
2423

24+
// defaultStatusInstance duplicates remoteconfig.DefaultStatusInstance rather
25+
// than importing it: that package drags uptane and bbolt into every binary
26+
// linking this one, which is far more expensive than repeating one string.
27+
const defaultStatusInstance = "Remote Config"
28+
2529
// Requires holds the dependencies for the rcstatus component.
2630
type Requires struct {
2731
compdef.In
@@ -104,7 +108,7 @@ func (rc statusProvider) populateStatus(stats map[string]interface{}) {
104108
if instances, ok := status["instances"].(map[string]interface{}); ok {
105109
additional := make(map[string]interface{}, len(instances))
106110
for name, instance := range instances {
107-
if name == remoteconfig.DefaultStatusInstance {
111+
if name == defaultStatusInstance {
108112
continue
109113
}
110114
additional[name] = instance

pkg/clusteragent/autoscaling/workload/status.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,15 @@ func recordRemoteConfigUpdate(product string, timestamp time.Time, update map[st
8080
tracked.LastUpdate = timestamp
8181
tracked.ConfigCount = len(update)
8282
tracked.UpdateCount++
83+
84+
// An update carries the full current config set for the product, not a
85+
// delta, so both of these describe this update alone. Reset them rather
86+
// than accumulating: a stale high version or a recovered error would
87+
// otherwise be reported as current for the life of the process.
88+
tracked.LastVersion = 0
89+
tracked.LastError = ""
90+
tracked.LastErrorTime = time.Time{}
91+
8392
for _, rawConfig := range update {
8493
if rawConfig.Metadata.Version > tracked.LastVersion {
8594
tracked.LastVersion = rawConfig.Metadata.Version

pkg/clusteragent/autoscaling/workload/status_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,3 +122,40 @@ func TestWorkloadAutoscalingStatusWithUpdates(t *testing.T) {
122122
require.NoError(t, Provider{}.HTML(false, h))
123123
assert.Contains(t, h.String(), "DatadogPodAutoscalers: 2")
124124
}
125+
126+
// TestWorkloadAutoscalingStatusReportsOnlyTheLatestUpdate covers the two ways
127+
// stale state could leak into the status: an update carries the full config
128+
// set, so a version from a superseded config must not survive, and a recovered
129+
// product must not keep reporting an old error.
130+
func TestWorkloadAutoscalingStatusReportsOnlyTheLatestUpdate(t *testing.T) {
131+
resetStatus(t)
132+
cfg := configmock.New(t)
133+
cfg.SetInTest("autoscaling.workload.enabled", true)
134+
InitStatus(autoscalingstore.NewStore[model.PodAutoscalerInternal](), func() bool { return true }, "autoscaling")
135+
136+
now := time.Now()
137+
// First update: high version, and a config that fails to apply.
138+
recordRemoteConfigUpdate(data.ProductContainerAutoscalingSettings, now, map[string]state.RawConfig{
139+
"a": {Metadata: state.Metadata{Version: 99}},
140+
})
141+
recordRemoteConfigError(data.ProductContainerAutoscalingSettings, now, errors.New("bad spec"))
142+
out := renderText(t)
143+
assert.Contains(t, out, "Last config version: 99")
144+
assert.Contains(t, out, "Last error: bad spec")
145+
146+
// Second update: the high-version config is gone and nothing fails.
147+
recordRemoteConfigUpdate(data.ProductContainerAutoscalingSettings, now.Add(time.Minute), map[string]state.RawConfig{
148+
"b": {Metadata: state.Metadata{Version: 4}},
149+
})
150+
out = renderText(t)
151+
assert.Contains(t, out, "Last config version: 4", "version must describe this update, not the highest ever seen")
152+
assert.NotContains(t, out, "99")
153+
assert.NotContains(t, out, "Last error:", "a recovered product must not keep reporting an old error")
154+
assert.Contains(t, out, "Updates received: 2", "the cumulative counter still accumulates")
155+
156+
// An update that removes every config reports zero, not a stale version.
157+
recordRemoteConfigUpdate(data.ProductContainerAutoscalingSettings, now.Add(2*time.Minute), map[string]state.RawConfig{})
158+
out = renderText(t)
159+
assert.Contains(t, out, "Configs in last update: 0")
160+
assert.NotContains(t, out, "Last config version: 4")
161+
}

0 commit comments

Comments
 (0)