Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
8 changes: 4 additions & 4 deletions pkg/clusteragent/autoscaling/workload/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,10 +393,10 @@ func (c *Controller) syncPodAutoscaler(ctx context.Context, key, ns, name string

// Fall through to normal scaling logic.
} else if podAutoscalerInternal.Spec().Owner == datadoghqcommon.DatadogPodAutoscalerLocalOwner {
// Implement sync logic for local ownership, source of truth is Kubernetes
if podAutoscalerInternal.Generation() != podAutoscaler.Generation {
podAutoscalerInternal.UpdateFromPodAutoscaler(podAutoscaler)
}
// Sync logic for local ownership: Kubernetes is the source of truth.
// UpdateFromPodAutoscaler internally short-circuits when neither .metadata.generation
// nor any of the watched labels/annotations has changed.
podAutoscalerInternal.UpdateFromPodAutoscaler(podAutoscaler)
}

// Reaching this point, we had no errors in processing, clearing up global error
Expand Down
44 changes: 43 additions & 1 deletion pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/DataDog/datadog-agent/pkg/clusteragent/autoscaling"
"github.com/DataDog/datadog-agent/pkg/util/pointer"

"github.com/twmb/murmur3"
autoscalingv2 "k8s.io/api/autoscaling/v2"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -45,6 +46,18 @@ const (
CustomRecommenderAnnotationKey = "autoscaling.datadoghq.com/custom-recommender"
)

// resyncLabelKeysFromPodAutoscaler lists the label keys read by UpdateFromPodAutoscaler.
var resyncLabelKeysFromPodAutoscaler = []string{
ProfileLabelKey,
}

// resyncAnnotationKeysFromPodAutoscaler lists the annotation keys read by UpdateFromPodAutoscaler.
var resyncAnnotationKeysFromPodAutoscaler = []string{
PreviewAnnotationKey,
ProfileTemplateHashAnnotation,
CustomRecommenderAnnotationKey,
}

// PodAutoscalerInternal holds the necessary data to work with the `DatadogPodAutoscaler` CRD.
type PodAutoscalerInternal struct {
// namespace is the namespace of the PodAutoscaler
Expand Down Expand Up @@ -83,6 +96,11 @@ type PodAutoscalerInternal struct {
// previewOptions holds the parsed preview feature flags from the DPA annotations
previewOptions previewOptions

// metadataHash fingerprints the K8s state read by UpdateFromPodAutoscaler
// (.metadata.generation plus watched labels/annotations), so that a single
// equality check captures both spec changes and out-of-spec edits.
metadataHash uint64

// scalingValues represents the active scaling values that should be used
scalingValues ScalingValues

Expand Down Expand Up @@ -293,8 +311,17 @@ func (p *PodAutoscalerInternal) UpdateFromProfile(
p.horizontalEventsRetention, p.horizontalRecommendationsRetention = getHorizontalRetentionValues(dpaSpec.ApplyPolicy)
}

// UpdateFromPodAutoscaler updates the PodAutoscalerInternal from a PodAutoscaler object inside K8S
// UpdateFromPodAutoscaler updates the PodAutoscalerInternal from a PodAutoscaler object inside K8S.
// For local-owner DPAs it short-circuits when the cached metadata fingerprint is unchanged.
func (p *PodAutoscalerInternal) UpdateFromPodAutoscaler(podAutoscaler *datadoghq.DatadogPodAutoscaler) {
if podAutoscaler.Spec.Owner == datadoghqcommon.DatadogPodAutoscalerLocalOwner {
newHash := computePodAutoscalerMetadataHash(podAutoscaler)
if p.metadataHash == newHash {
return
}
p.metadataHash = newHash
}

if v, ok := podAutoscaler.Labels[ProfileLabelKey]; ok {
p.profileName = v
}
Expand Down Expand Up @@ -1340,3 +1367,18 @@ func parseCustomConfigurationAnnotation(annotations map[string]string) (*Recomme

return &customConfiguration, nil
}

// computePodAutoscalerMetadataHash returns a fingerprint of the K8s state
// read by UpdateFromPodAutoscaler — .metadata.generation plus the watched
// labels/annotations — used to identify any change worth re-syncing.
func computePodAutoscalerMetadataHash(podAutoscaler *datadoghq.DatadogPodAutoscaler) uint64 {
hasher := murmur3.New64()
_, _ = fmt.Fprintf(hasher, "G\x00%d\x00", podAutoscaler.Generation)
for _, key := range resyncLabelKeysFromPodAutoscaler {
_, _ = hasher.Write([]byte("L\x00" + key + "\x00" + podAutoscaler.Labels[key] + "\x00"))
}
for _, key := range resyncAnnotationKeysFromPodAutoscaler {
_, _ = hasher.Write([]byte("A\x00" + key + "\x00" + podAutoscaler.Annotations[key] + "\x00"))
}
return hasher.Sum64()
}
Original file line number Diff line number Diff line change
Expand Up @@ -847,3 +847,31 @@ func TestContainerResourcesForStatus(t *testing.T) {
})
}
}

// TestUpdateFromPodAutoscalerResyncsOnWatchedMetadata verifies that, for local-owner DPAs,
// UpdateFromPodAutoscaler picks up changes to the watched labels/annotations even when
// .metadata.generation is unchanged (e.g. an annotation-only edit to PreviewAnnotationKey).
func TestUpdateFromPodAutoscalerResyncsOnWatchedMetadata(t *testing.T) {
dpa := &datadoghq.DatadogPodAutoscaler{
ObjectMeta: metav1.ObjectMeta{Name: "dpa", Namespace: "default", Generation: 1},
Spec: datadoghq.DatadogPodAutoscalerSpec{Owner: datadoghqcommon.DatadogPodAutoscalerLocalOwner},
}

pai := NewPodAutoscalerInternal(dpa)
assert.False(t, pai.IsBurstable())

// Annotation-only edit: same generation, new preview annotation.
dpa.Annotations = map[string]string{PreviewAnnotationKey: `{"burstable":true}`}
pai.UpdateFromPodAutoscaler(dpa)
assert.True(t, pai.IsBurstable(), "annotation-only edit must be picked up")

// Calling again with the same object is a no-op (gate kicks in).
previousHash := pai.metadataHash
pai.UpdateFromPodAutoscaler(dpa)
assert.Equal(t, previousHash, pai.metadataHash)

// An unrelated annotation change does not retrigger the parse but is harmless.
dpa.Annotations["unrelated"] = "x"
pai.UpdateFromPodAutoscaler(dpa)
assert.True(t, pai.IsBurstable())
}
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,21 @@ func (f FakePodAutoscalerInternal) Build() PodAutoscalerInternal {
upstreamCR.Annotations[PreviewAnnotationKey] = f.PreviewAnnotationKey
}

// Mirror UpdateFromPodAutoscaler: cache the metadata fingerprint so equality checks
// against a PodAutoscalerInternal that came through the real code path match.
// Only populate when the test passes an explicit local-owner UpstreamCR — settings-driven
// shells (and non-local owners) do not go through the gated path in production.
var metadataHash uint64
if f.UpstreamCR != nil && f.UpstreamCR.Spec.Owner == datadoghqcommon.DatadogPodAutoscalerLocalOwner {
metadataHash = computePodAutoscalerMetadataHash(upstreamCR)
}

return PodAutoscalerInternal{
namespace: f.Namespace,
name: f.Name,
generation: f.Generation,
upstreamCR: upstreamCR,
metadataHash: metadataHash,
settingsTimestamp: f.SettingsTimestamp,
creationTimestamp: f.CreationTimestamp,
scalingValues: f.ScalingValues,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
fixes:
- |
Fix a bug in the workload autoscaling controller where annotation-only edits
(e.g. ``autoscaling.datadoghq.com/preview``) on a locally-owned
``DatadogPodAutoscaler`` were not picked up until the next ``.spec`` change or
cluster-agent restart, because the controller gated re-sync on
``.metadata.generation`` (which annotations do not bump). Toggling burstable
mode via the preview annotation now takes effect on the next reconcile.
Loading