[clusteragent/autoscaling] Check that autoscaling target is not cluster agent #28723

jennchenn · 2024-08-23T22:29:55Z

What does this PR do?

Check that the autoscaling target is not set to the cluster agent. If it is, set an error on the corresponding DatadogPodAutoscaler.

Motivation

We do not want the cluster agent to be taken down due to autoscaler failures.

Additional Notes

If CLUSTER_AGENT_DEPLOYMENT is not set, it defaults to assuming datadog-agent-cluster-agent as the deployment name.

Do we want to add CLUSTER_AGENT_DEPLOYMENT to the agent config rather than reading the environment variable directly?

Possible Drawbacks / Trade-offs

Describe how to test/QA your changes

Create a DatadogPodAutoscalerresource that targets the cluster agent
Verify that an error status is set and the cluster agent is not scaled by it

❯ k describe datadogpodautoscaler cluster-agent

...
Status:
  Conditions:
    Last Transition Time:  2024-08-23T21:17:55Z
    Reason:                Autoscaling target cannot be set to the cluster agent
    Status:                True
    Type:                  Error
    Last Transition Time:  2024-08-23T21:17:55Z
...

pr-commenter · 2024-08-23T22:50:28Z

Test changes on VM

Use this command from test-infra-definitions to manually test this PR changes on a VM:

inv create-vm --pipeline-id=43465304 --os-family=ubuntu

Note: This applies to commit 2182fa3

pr-commenter · 2024-08-23T23:12:30Z

Regression Detector

Regression Detector Results

Run ID: a17d9b0e-b178-499f-9ba5-f91b3b272426 Metrics dashboard Target profiles

Baseline: 0cce24d
Comparison: 2182fa3

Performance changes are noted in the perf column of each table:

✅ = significantly better comparison variant performance
❌ = significantly worse comparison variant performance
➖ = no significant change in performance

No significant changes in experiment optimization goals

Confidence level: 90.00%
Effect size tolerance: |Δ mean %| ≥ 5.00%

There were no significant changes in experiment optimization goals at this confidence level and effect size tolerance.

Fine details of change detection per experiment

perf	experiment	goal	Δ mean %	Δ mean % CI	trials	links
➖	uds_dogstatsd_to_api_cpu	% cpu utilization	+0.48	[-0.39, +1.35]	1	Logs
➖	otel_to_otel_logs	ingress throughput	+0.38	[-0.43, +1.19]	1	Logs
➖	file_tree	memory utilization	+0.14	[+0.10, +0.17]	1	Logs
➖	idle	memory utilization	+0.05	[+0.01, +0.08]	1	Logs
➖	tcp_dd_logs_filter_exclude	ingress throughput	+0.00	[-0.01, +0.01]	1	Logs
➖	uds_dogstatsd_to_api	ingress throughput	-0.00	[-0.00, +0.00]	1	Logs
➖	pycheck_lots_of_tags	% cpu utilization	-0.39	[-2.75, +1.97]	1	Logs
➖	basic_py_check	% cpu utilization	-1.04	[-3.98, +1.90]	1	Logs
➖	tcp_syslog_to_blackhole	ingress throughput	-4.14	[-16.83, +8.55]	1	Logs

Bounds Checks

perf	experiment	bounds_check_name	replicates_passed
❌	idle	memory_usage	0/10

Explanation

A regression test is an A/B test of target performance in a repeatable rig, where "performance" is measured as "comparison variant minus baseline variant" for an optimization goal (e.g., ingress throughput). Due to intrinsic variability in measuring that goal, we can only estimate its mean value for each experiment; we report uncertainty in that value as a 90.00% confidence interval denoted "Δ mean % CI".

For each experiment, we decide whether a change in performance is a "regression" -- a change worth investigating further -- if all of the following criteria are true:

Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.
Its 90.00% confidence interval "Δ mean % CI" does not contain zero, indicating that if our statistical model is accurate, there is at least a 90.00% chance there is a difference in performance between baseline and comparison variants.
Its configuration does not mark it "erratic".

vboulineau · 2024-08-29T10:07:40Z

pkg/clusteragent/autoscaling/workload/controller.go

@@ -389,3 +408,10 @@ func (c *Controller) deletePodAutoscaler(ns, name string) error {
 	}
 	return nil
 }
+
+func getDeploymentName() string {


Tooling already exists for that: https://github.com/DataDog/datadog-agent/blob/main/pkg/util/kubernetes/apiserver/common/common_linux.go

It reads DD_POD_NAME which is already set and defaults to os.hostname() (should work for DCA). From there you can use a variant of https://github.com/DataDog/datadog-agent/blob/main/pkg/util/kubernetes/helpers.go#L22-L40 to get Deployment name from POD Name

Thanks! Made a new helper to get the deployment name from the pod name which reuses the code from the helper you shared here

vboulineau · 2024-08-29T10:13:39Z

pkg/clusteragent/autoscaling/workload/controller.go

@@ -272,6 +274,23 @@ func (c *Controller) syncPodAutoscaler(ctx context.Context, key, ns, name string
 	// Reaching this point, we had an error in processing, clearing up global error
 	podAutoscalerInternal.SetError(nil)

+	// Check that targetRef is not set to the cluster agent
+	clusterAgentRef := autoscalingv2.CrossVersionObjectReference{


You also need to check autoscaler namespace vs cluster agent namespace: https://github.com/DataDog/datadog-agent/blob/main/pkg/util/kubernetes/apiserver/common/common.go#L40-L49

Also not sure you need to compare the full CrossVersionObjectReference. Basically if something has some name and namespace than DCA it's suspicious enough.

Thinking more about it, you should try to cover both Cluster Agent Deployment AND direct ReplicaSet

vboulineau · 2024-08-29T10:15:46Z

pkg/clusteragent/autoscaling/workload/controller.go

+		APIVersion: "apps/v1",
+	}
+
+	if podAutoscaler.Spec.TargetRef == clusterAgentRef {


nit: The code should probably go to a dedicated function like validateAutoscaler or validateRequirements, the max 100 could also go there.

vboulineau · 2024-08-30T15:32:33Z

pkg/clusteragent/autoscaling/workload/controller.go

+	return nil
+}
+
+func (c *Controller) updateAutoscalerStatus(ctx context.Context, key, ns, name string, err error, podAutoscalerInternal model.PodAutoscalerInternal, podAutoscaler *datadoghq.DatadogPodAutoscaler) error {


I'd expect the method to highlight that it unlocks store and one line it:

return autoscaling.NoRequeue, c.updateAndUnlock(ctx, key, ns, name, validationErr, podAutoscalerInternal, podAutoscaler)

There's probably a better name to find for it!

…arget

davidor

👍 for container-platform files

jennchenn · 2024-09-05T13:24:06Z

/merge

dd-devflow · 2024-09-05T13:24:13Z

🚂 MergeQueue: pull request added to the queue

The median merge time in main is 23m.

Use /merge -c to cancel this operation!

Check that autoscaling target is not cluster agent

69a987b

jennchenn added team/containers changelog/no-changelog labels Aug 23, 2024

jennchenn requested a review from a team as a code owner August 23, 2024 22:29

vboulineau reviewed Aug 29, 2024

View reviewed changes

jennchenn added 2 commits August 29, 2024 13:12

Add helper function to parse deployment name from pod name

d127013

Extract validation logic into separate function

b03235e

jennchenn requested review from a team as code owners August 29, 2024 18:58

vboulineau reviewed Aug 30, 2024

View reviewed changes

jennchenn added 3 commits August 30, 2024 11:41

Check DCA replicaset in validation logic

ab500df

Rename function to highlight store unlock logic

807e791

fixup! Check DCA replicaset in validation logic

f35a065

This was referenced Aug 30, 2024

[clusteragent/autoscaling] Use PodWatcher to update current replicas in status #28857

Merged

[clusteragent/autoscaling] Impose 100 DatadogPodAutoscaler limit in cluster agent #28684

Merged

jennchenn and others added 2 commits September 3, 2024 13:50

Merge branch 'main' into jenn/CASCL-49_autoscaling-prevent-ca-being-t…

cfef46e

…arget

Dynamically set test namespace

995292e

jennchenn added the qa/done QA done before merge and regressions are covered by tests label Sep 3, 2024

Add test for targeting cluster agent replicaset

2182fa3

sblumenthal approved these changes Sep 4, 2024

View reviewed changes

davidor approved these changes Sep 5, 2024

View reviewed changes

mfpierre approved these changes Sep 5, 2024

View reviewed changes

dd-mergequeue bot merged commit c4e3a15 into main Sep 5, 2024
231 checks passed

dd-mergequeue bot deleted the jenn/CASCL-49_autoscaling-prevent-ca-being-target branch September 5, 2024 13:49

github-actions bot added this to the 7.58.0 milestone Sep 5, 2024

jennchenn added the component/autoscaling label Nov 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clusteragent/autoscaling] Check that autoscaling target is not cluster agent #28723

[clusteragent/autoscaling] Check that autoscaling target is not cluster agent #28723

jennchenn commented Aug 23, 2024 •

edited

Loading

pr-commenter bot commented Aug 23, 2024 •

edited

Loading

pr-commenter bot commented Aug 23, 2024 •

edited

Loading

Fine details of change detection per experiment

Explanation

vboulineau Aug 29, 2024

jennchenn Aug 29, 2024

vboulineau Aug 29, 2024

vboulineau Aug 30, 2024

vboulineau Aug 29, 2024

vboulineau Aug 30, 2024

davidor left a comment

jennchenn commented Sep 5, 2024

dd-devflow bot commented Sep 5, 2024

[clusteragent/autoscaling] Check that autoscaling target is not cluster agent #28723

[clusteragent/autoscaling] Check that autoscaling target is not cluster agent #28723

Conversation

jennchenn commented Aug 23, 2024 • edited Loading

What does this PR do?

Motivation

Additional Notes

Possible Drawbacks / Trade-offs

Describe how to test/QA your changes

pr-commenter bot commented Aug 23, 2024 • edited Loading

Test changes on VM

pr-commenter bot commented Aug 23, 2024 • edited Loading

Regression Detector

Regression Detector Results

No significant changes in experiment optimization goals

Fine details of change detection per experiment

Bounds Checks

Explanation

vboulineau Aug 29, 2024

Choose a reason for hiding this comment

jennchenn Aug 29, 2024

Choose a reason for hiding this comment

vboulineau Aug 29, 2024

Choose a reason for hiding this comment

vboulineau Aug 30, 2024

Choose a reason for hiding this comment

vboulineau Aug 29, 2024

Choose a reason for hiding this comment

vboulineau Aug 30, 2024

Choose a reason for hiding this comment

davidor left a comment

Choose a reason for hiding this comment

jennchenn commented Sep 5, 2024

dd-devflow bot commented Sep 5, 2024

jennchenn commented Aug 23, 2024 •

edited

Loading

pr-commenter bot commented Aug 23, 2024 •

edited

Loading

pr-commenter bot commented Aug 23, 2024 •

edited

Loading