Skip to content

[clusteragent] Fix wrong computation of the init container resources #35588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Apr 3, 2025
Merged

[clusteragent] Fix wrong computation of the init container resources #35588

merged 4 commits into from
Apr 3, 2025

Conversation

iamluc
Copy link
Contributor

@iamluc iamluc commented Mar 28, 2025 •
edited
Loading

What does this PR do?

Fix wrong computation of the init container resources, introduced in #30266

If a request was set but not the limit, the computed request for the init containers could be higher than the computed limit. This is invalid and causes the deployment to fail.

To fix that, in case there is a request but no limit, we use the request as the limit.

Motivation

Fix #35170 / https://datadoghq.atlassian.net/browse/CONS-7154 / https://datadoghq.atlassian.net/browse/INPLAT-529

Describe how you validated your changes

Unit test
+
manually using

# values.yaml
datadog:
  logLevel: DEBUG
  clusterName: my-test-cluster
  apiKeyExistingSecret: datadog-secret
  appKeyExistingSecret: datadog-secret
  logs:
    enabled: true
    containerCollectAll: true
    containerCollectUsingFiles: true
  kubelet:
    tlsVerify: false
  apm:
    instrumentation:
      enabled: true
clusterAgent:
  admissionController:
    enabled: true


# deploy.yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  name: demo
  namespace: init
  labels:
    app: init-container-test
spec:
  replicas: 1
  selector:
    matchLabels:
      app: init-container-test
  template:
    metadata:
      labels:
        app: init-container-test
    spec:
      containers:
        - name: container1
          image: busybox
          command: ["sleep", "3600"]
          resources:
            limits:
              cpu: 100m
              memory: 128Mi
            requests:
              cpu: 100m
              memory: 128Mi
        - name: container2
          image: busybox
          command: ["sleep", "3600"]
          resources:
            limits:
              # cpu: 50m    <- uncommenting this allows the pod to be created
              memory: 128Mi
            requests:
              cpu: 50m
              memory: 128Mi

With cluster agent 7.64, the deployment of the app results in a failure:

Pod "demo-57667dd664-q8h7z" is invalid: [spec.initContainers[0].resources.requests: Invalid value: "150m": must be less than or equal to cpu limit of 100m, spec.initContainers[1].resources.requests: Invalid value: "150m": must be less than or equal to cpu limit of 100m,

With this PR, the deployment is successful.

Possible Drawbacks / Trade-offs

Additional Notes

@github-actions github-actions bot added team/container-platform The Container Platform Team team/injection-platform short review PR is simple enough to be reviewed quickly labels Mar 28, 2025
@agent-platform-auto-pr
Copy link
Contributor

agent-platform-auto-pr bot commented Mar 28, 2025 •
edited
Loading

Test changes on VM

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

dda inv aws.create-vm --pipeline-id=60917057 --os-family=ubuntu

Note: This applies to commit 2fc932f

@agent-platform-auto-pr
Copy link
Contributor

agent-platform-auto-pr bot commented Mar 28, 2025 •
edited
Loading

Uncompressed package size comparison

Comparison with ancestor 30ac12f04f21d389772471a50746d0eebb0ab225

Size reduction summary
package diff status size ancestor threshold
datadog-agent-amd64-deb -0.00MB ✅ 796.98MB 796.98MB 0.50MB
datadog-agent-x86_64-rpm -0.00MB ✅ 806.85MB 806.85MB 0.50MB
datadog-agent-x86_64-suse -0.00MB ✅ 806.85MB 806.85MB 0.50MB
Diff per package
package diff status size ancestor threshold
datadog-agent-aarch64-rpm 0.00MB ✅ 797.60MB 797.60MB 0.50MB
datadog-agent-arm64-deb 0.00MB ✅ 787.75MB 787.75MB 0.50MB
datadog-dogstatsd-amd64-deb 0.00MB ✅ 39.71MB 39.71MB 0.50MB
datadog-dogstatsd-x86_64-rpm 0.00MB ✅ 39.79MB 39.79MB 0.50MB
datadog-dogstatsd-x86_64-suse 0.00MB ✅ 39.79MB 39.79MB 0.50MB
datadog-dogstatsd-arm64-deb 0.00MB ✅ 38.22MB 38.22MB 0.50MB
datadog-heroku-agent-amd64-deb 0.00MB ✅ 440.94MB 440.94MB 0.50MB
datadog-iot-agent-amd64-deb 0.00MB ✅ 60.48MB 60.48MB 0.50MB
datadog-iot-agent-x86_64-rpm 0.00MB ✅ 60.55MB 60.55MB 0.50MB
datadog-iot-agent-x86_64-suse 0.00MB ✅ 60.55MB 60.55MB 0.50MB
datadog-iot-agent-arm64-deb 0.00MB ✅ 57.80MB 57.80MB 0.50MB
datadog-iot-agent-aarch64-rpm 0.00MB ✅ 57.87MB 57.87MB 0.50MB

Decision

✅ Passed

@cit-pr-commenter cit-pr-commenter
Copy link

cit-pr-commenter bot commented Mar 28, 2025 •
edited
Loading

Regression Detector

Regression Detector Results

Metrics dashboard
Target profiles
Run ID: 92ddf7ad-b12a-4e44-ad5a-9cc90cdc2103

Baseline: 30ac12f
Comparison: 2fc932f
Diff

Optimization Goals: ✅ No significant changes detected

Fine details of change detection per experiment

perf experiment goal Δ mean % Δ mean % CI trials links
➖ otlp_ingest_metrics memory utilization +0.36 [+0.22, +0.50] 1 Logs
➖ file_tree memory utilization +0.35 [+0.20, +0.50] 1 Logs
➖ file_to_blackhole_1000ms_latency_linear_load egress throughput +0.24 [-0.24, +0.71] 1 Logs
➖ otlp_ingest_logs memory utilization +0.06 [-0.10, +0.22] 1 Logs
➖ file_to_blackhole_0ms_latency_http2 egress throughput +0.04 [-0.78, +0.86] 1 Logs
➖ file_to_blackhole_1000ms_latency egress throughput +0.01 [-0.75, +0.76] 1 Logs
➖ tcp_dd_logs_filter_exclude ingress throughput +0.00 [-0.01, +0.01] 1 Logs
➖ file_to_blackhole_100ms_latency egress throughput -0.00 [-0.73, +0.73] 1 Logs
➖ uds_dogstatsd_to_api ingress throughput -0.00 [-0.27, +0.26] 1 Logs
➖ file_to_blackhole_300ms_latency egress throughput -0.01 [-0.64, +0.63] 1 Logs
➖ file_to_blackhole_0ms_latency egress throughput -0.01 [-0.78, +0.75] 1 Logs
➖ file_to_blackhole_0ms_latency_http1 egress throughput -0.02 [-0.88, +0.84] 1 Logs
➖ file_to_blackhole_500ms_latency egress throughput -0.02 [-0.81, +0.76] 1 Logs
➖ quality_gate_idle memory utilization -0.13 [-0.19, -0.07] 1 Logs bounds checks dashboard
➖ otlp_ingest_traces memory utilization -0.21 [-0.60, +0.17] 1 Logs
➖ quality_gate_idle_all_features memory utilization -0.31 [-0.40, -0.21] 1 Logs bounds checks dashboard
➖ tcp_syslog_to_blackhole ingress throughput -0.38 [-0.44, -0.32] 1 Logs
➖ quality_gate_logs % cpu utilization -0.64 [-3.39, +2.12] 1 Logs
➖ uds_dogstatsd_20mb_12k_contexts_20_senders memory utilization -0.67 [-0.71, -0.63] 1 Logs
➖ uds_dogstatsd_to_api_cpu % cpu utilization -0.77 [-1.58, +0.03] 1 Logs

Bounds Checks: ✅ Passed

perf experiment bounds_check_name replicates_passed links
✅ file_to_blackhole_0ms_latency lost_bytes 10/10
✅ file_to_blackhole_0ms_latency memory_usage 10/10
✅ file_to_blackhole_0ms_latency_http1 lost_bytes 10/10
✅ file_to_blackhole_0ms_latency_http1 memory_usage 10/10
✅ file_to_blackhole_0ms_latency_http2 lost_bytes 10/10
✅ file_to_blackhole_0ms_latency_http2 memory_usage 10/10
✅ file_to_blackhole_1000ms_latency memory_usage 10/10
✅ file_to_blackhole_1000ms_latency_linear_load memory_usage 10/10
✅ file_to_blackhole_100ms_latency lost_bytes 10/10
✅ file_to_blackhole_100ms_latency memory_usage 10/10
✅ file_to_blackhole_300ms_latency lost_bytes 10/10
✅ file_to_blackhole_300ms_latency memory_usage 10/10
✅ file_to_blackhole_500ms_latency lost_bytes 10/10
✅ file_to_blackhole_500ms_latency memory_usage 10/10
✅ quality_gate_idle intake_connections 10/10 bounds checks dashboard
✅ quality_gate_idle memory_usage 10/10 bounds checks dashboard
✅ quality_gate_idle_all_features intake_connections 10/10 bounds checks dashboard
✅ quality_gate_idle_all_features memory_usage 10/10 bounds checks dashboard
✅ quality_gate_logs intake_connections 10/10
✅ quality_gate_logs lost_bytes 10/10
✅ quality_gate_logs memory_usage 10/10

Explanation

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

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

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:

  1. Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.

  2. 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.

  3. Its configuration does not mark it "erratic".

CI Pass/Fail Decision

✅ Passed. All Quality Gates passed.

  • quality_gate_logs, bounds check memory_usage: 10/10 replicas passed. Gate passed.
  • quality_gate_logs, bounds check lost_bytes: 10/10 replicas passed. Gate passed.
  • quality_gate_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
  • quality_gate_idle, bounds check intake_connections: 10/10 replicas passed. Gate passed.
  • quality_gate_idle, bounds check memory_usage: 10/10 replicas passed. Gate passed.
  • quality_gate_idle_all_features, bounds check memory_usage: 10/10 replicas passed. Gate passed.
  • quality_gate_idle_all_features, bounds check intake_connections: 10/10 replicas passed. Gate passed.

@agent-platform-auto-pr
Copy link
Contributor

agent-platform-auto-pr bot commented Mar 28, 2025 •
edited
Loading

Static quality checks ✅

Please find below the results from static quality gates

Successful checks

Info

Result Quality gate On disk size On disk size limit On wire size On wire size limit
✅ static_quality_gate_agent_deb_amd64 771.75 MiB 780.74 MiB 189.33 MiB 190.85 MiB
✅ static_quality_gate_agent_deb_amd64_fips 769.68 MiB 778.65 MiB 188.61 MiB 190.66 MiB
✅ static_quality_gate_agent_heroku_amd64 428.58 MiB 428.97 MiB 112.78 MiB 113.11 MiB
✅ static_quality_gate_agent_msi 976.92 MiB 977.0 MiB 149.39 MiB 149.85 MiB
✅ static_quality_gate_agent_rpm_amd64 771.73 MiB 780.59 MiB 190.95 MiB 193.37 MiB
✅ static_quality_gate_agent_rpm_amd64_fips 769.72 MiB 778.69 MiB 190.66 MiB 192.47 MiB
✅ static_quality_gate_agent_rpm_arm64 762.9 MiB 772.07 MiB 173.63 MiB 174.47 MiB
✅ static_quality_gate_agent_rpm_arm64_fips 761.05 MiB 770.27 MiB 172.33 MiB 173.63 MiB
✅ static_quality_gate_agent_suse_amd64 771.76 MiB 780.7 MiB 190.95 MiB 193.37 MiB
✅ static_quality_gate_agent_suse_amd64_fips 769.69 MiB 778.59 MiB 190.66 MiB 192.47 MiB
✅ static_quality_gate_agent_suse_arm64 762.86 MiB 772.02 MiB 173.63 MiB 174.47 MiB
✅ static_quality_gate_agent_suse_arm64_fips 761.08 MiB 770.2 MiB 172.33 MiB 173.63 MiB
✅ static_quality_gate_docker_agent_amd64 856.29 MiB 865.3 MiB 289.29 MiB 291.35 MiB
✅ static_quality_gate_docker_agent_arm64 870.75 MiB 880.0 MiB 275.8 MiB 277.86 MiB
✅ static_quality_gate_docker_agent_jmx_amd64 856.29 MiB 865.3 MiB 289.29 MiB 291.35 MiB
✅ static_quality_gate_docker_agent_jmx_arm64 870.75 MiB 880.0 MiB 275.8 MiB 277.86 MiB
✅ static_quality_gate_docker_agent_windows1809 856.29 MiB 865.3 MiB 289.29 MiB 291.35 MiB
✅ static_quality_gate_docker_agent_windows1809_core 856.29 MiB 865.3 MiB 289.29 MiB 291.35 MiB
✅ static_quality_gate_docker_agent_windows1809_core_jmx 856.29 MiB 865.3 MiB 289.29 MiB 291.35 MiB
✅ static_quality_gate_docker_agent_windows1809_jmx 856.29 MiB 865.3 MiB 289.29 MiB 291.35 MiB
✅ static_quality_gate_docker_agent_windows2022 856.29 MiB 865.3 MiB 289.29 MiB 291.35 MiB
✅ static_quality_gate_docker_agent_windows2022_core 856.29 MiB 865.3 MiB 289.29 MiB 291.35 MiB
✅ static_quality_gate_docker_agent_windows2022_core_jmx 856.29 MiB 865.3 MiB 289.29 MiB 291.35 MiB
✅ static_quality_gate_docker_agent_windows2022_jmx 856.29 MiB 865.3 MiB 289.29 MiB 291.35 MiB
✅ static_quality_gate_docker_cluster_agent_amd64 263.13 MiB 263.29 MiB 105.67 MiB 106.03 MiB
✅ static_quality_gate_docker_cluster_agent_arm64 279.08 MiB 279.25 MiB 100.53 MiB 100.87 MiB
✅ static_quality_gate_docker_cws_instrumentation_amd64 6.65 MiB 7.12 MiB 2.82 MiB 3.29 MiB
✅ static_quality_gate_docker_cws_instrumentation_arm64 6.44 MiB 6.92 MiB 2.6 MiB 3.07 MiB
✅ static_quality_gate_docker_dogstatsd_amd64 46.09 MiB 46.37 MiB 17.38 MiB 17.78 MiB
✅ static_quality_gate_docker_dogstatsd_arm64 44.71 MiB 44.99 MiB 16.24 MiB 16.65 MiB
✅ static_quality_gate_dogstatsd_deb_amd64 37.95 MiB 38.23 MiB 9.84 MiB 10.26 MiB
✅ static_quality_gate_dogstatsd_deb_arm64 36.53 MiB 36.82 MiB 8.54 MiB 8.96 MiB
✅ static_quality_gate_dogstatsd_rpm_amd64 37.95 MiB 38.23 MiB 9.85 MiB 10.27 MiB
✅ static_quality_gate_dogstatsd_suse_amd64 37.95 MiB 38.23 MiB 9.85 MiB 10.27 MiB
✅ static_quality_gate_iot_agent_deb_amd64 57.75 MiB 58.19 MiB 14.56 MiB 15.02 MiB
✅ static_quality_gate_iot_agent_deb_arm64 55.2 MiB 55.63 MiB 12.58 MiB 13.05 MiB
✅ static_quality_gate_iot_agent_deb_armhf 53.89 MiB 54.32 MiB 12.58 MiB 13.05 MiB
✅ static_quality_gate_iot_agent_rpm_amd64 57.75 MiB 58.19 MiB 14.58 MiB 15.04 MiB
✅ static_quality_gate_iot_agent_rpm_arm64 55.2 MiB 55.63 MiB 12.61 MiB 13.07 MiB
✅ static_quality_gate_iot_agent_suse_amd64 57.75 MiB 58.19 MiB 14.58 MiB 15.04 MiB

@iamluc iamluc force-pushed the luc/clusteragent-fix-resources-computation branch from 2097b1c to c05297a Compare March 28, 2025 14:18
@iamluc iamluc added the qa/done QA done before merge and regressions are covered by tests label Mar 28, 2025
@github-actions github-actions bot added medium review PR review might take time and removed short review PR is simple enough to be reviewed quickly labels Mar 28, 2025
@iamluc iamluc force-pushed the luc/clusteragent-fix-resources-computation branch 2 times, most recently from 800d457 to 71f95c3 Compare March 28, 2025 16:00
@iamluc iamluc marked this pull request as ready for review March 28, 2025 16:33
@iamluc iamluc requested review from a team as code owners March 28, 2025 16:33
@iamluc iamluc requested a review from natitsechanski March 28, 2025 16:33
@iamluc iamluc force-pushed the luc/clusteragent-fix-resources-computation branch from 71f95c3 to 2fc932f Compare April 3, 2025 08:11
@adel121 adel121 removed the team/container-platform The Container Platform Team label Apr 3, 2025
adel121
adel121 approved these changes Apr 3, 2025
View reviewed changes
Copy link
Contributor

@adel121 adel121 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks for adding a QA section. 🙇

  • Would be good to include the expected outcome in the QA to make it easier during the QA process later.
  • I also removed team/container-platform label from the labels because it will generate a QA card for us too, while this should be owned only by #injection-platform.
  • Additionally, qa/done means that there is no need to QA during the release process once this is merged on main. I think this should still be QA'ed after being merged on main. For this we need the label qa/rc-required. But I will leave it to your to decide on this.

@iamluc iamluc removed the qa/done QA done before merge and regressions are covered by tests label Apr 3, 2025
@iamluc iamluc added the qa/rc-required Only for a PR that requires validation on the Release Candidate label Apr 3, 2025
@iamluc
Copy link
Contributor Author

iamluc commented Apr 3, 2025

Thanks!

  • Would be good to include the expected outcome in the QA to make it easier during the QA process later.

QA section updated

  • Additionally, qa/done means that there is no need to QA during the release process once this is merged on main. I think this should still be QA'ed after being merged on main. For this we need the label qa/rc-required. But I will leave it to your to decide on this.

Sounds good. I changed qa/done to qa/rc-required.

@iamluc
Copy link
Contributor Author

iamluc commented Apr 3, 2025

/merge

@dd-devflow
Copy link

dd-devflow bot commented Apr 3, 2025 •
edited
Loading

View all feedbacks in Devflow UI.

2025-04-03 13:57:46 UTC ℹ️ Start processing command /merge

2025-04-03 13:58:06 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2025-04-03 13:59:22 UTC ℹ️ MergeQueue: merge request added to the queue

The expected merge time in main is approximately 46m (p90).

2025-04-03 14:55:37 UTC ℹ️ MergeQueue: This merge request was merged

@dd-mergequeue dd-mergequeue bot merged commit 3729a76 into main Apr 3, 2025
279 checks passed
@dd-mergequeue dd-mergequeue bot deleted the luc/clusteragent-fix-resources-computation branch April 3, 2025 14:55
@github-actions github-actions bot added this to the 7.66.0 milestone Apr 3, 2025
ogaca-dd pushed a commit that referenced this pull request Apr 9, 2025
@iamluc @ogaca-dd
[clusteragent] Fix wrong computation of the init container resources (#…
c2ba9a0 
…35588)
@Dasio
Copy link

Dasio commented May 6, 2025

Hello, can this be backported to current latest release? I see it's on 7.66 milestone, but current version is just 7.64.x

@stanistan stanistan added the backport/7.65.x Automatically create a backport PR to 7.65.x label May 7, 2025
agent-platform-auto-pr bot pushed a commit that referenced this pull request May 7, 2025
@iamluc @github-actions
[clusteragent] Fix wrong computation of the init container resources (#…
148d652 
…35588)

(cherry picked from commit 3729a76)
@agent-platform-auto-pr agent-platform-auto-pr bot mentioned this pull request May 7, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paullegranddc paullegranddc paullegranddc approved these changes

@adel121 adel121 adel121 approved these changes

@michaelcretzman michaelcretzman michaelcretzman approved these changes

@natitsechanski natitsechanski Awaiting requested review from natitsechanski natitsechanski is a code owner automatically assigned from DataDog/injection-platform

Assignees
No one assigned
Labels
backport/7.65.x Automatically create a backport PR to 7.65.x medium review PR review might take time qa/rc-required Only for a PR that requires validation on the Release Candidate team/injection-platform
Projects
None yet
Milestone
7.66.0
Development

Successfully merging this pull request may close these issues.

[BUG] Limit for autoinstrumentation init container calculated wrong
6 participants
@iamluc @Dasio @adel121 @michaelcretzman @paullegranddc @stanistan