fix(ssi): decoupling admission controller webhooks #33599

betterengineering · 2025-01-30T22:47:24Z

What does this PR do?

fix(ssi): migrate injection filter to mutate common
- This commit migrates the injection filter into the mutate/common package given it's used by multiple webhooks.
fix(ssi): remove config from injection filter
- This commit moves config fetching out of the NewInjectorFIlter method and instead requires it to be passed in.
fix(ssi): update mutating webhooks to use their own config
- BREAKING CHANGE: This commit updates the config webhook and the tags to labels webhook to use their own configs when constructing their injection filter. While there were no substantial changes to the tests, this change could certainly have subtle consequences.
fix(ssi): create injector interface and refactor webhooks
- This commit adds an injector interface and refactors the tagsfromlabels and the config webhooks to use it. This will allow us to use the injector logic without needing to instantiate a webhook.

Motivation

The short version is that we have three webooks that are tightly coupled, and they all rely on configuration for SSI. We're looking to substantially modify the SSI configuration in Kubernetes SSI | Workload Selection 🎯. Decoupling these webhooks is a necessary step to safely modifying the single step configuration. See Decoupling Admission Controller Webhooks for more details.

Describe how you validated your changes

Possible Drawbacks / Trade-offs

Additional Notes

This commit migrates the injection filter into the mutate/common package given it's used by multiple webhooks.

This commit moves config fetching out of the NewInjectorFIlter method and instead requires it to be passed in.

BREAKING CHANGE: This commit updates the config webhook and the tags to labels webhook to use their own configs when constructing their injection filter. While there were no substantial changes to the tests, this change could certainly have subtle consequences.

agent-platform-auto-pr · 2025-01-30T23:04:52Z

Uncompressed package size comparison

Comparison with ancestor 78991f6b4f90368bb9b7d51816beb5dd16b89d4a

Diff per package

package	diff	status	size	ancestor	threshold
datadog-agent-amd64-deb	0.00MB	✅	882.18MB	882.18MB	0.50MB
datadog-agent-x86_64-rpm	0.00MB	✅	891.92MB	891.92MB	0.50MB
datadog-agent-x86_64-suse	0.00MB	✅	891.92MB	891.92MB	0.50MB
datadog-iot-agent-aarch64-rpm	0.00MB	✅	89.94MB	89.94MB	0.50MB
datadog-agent-arm64-deb	0.00MB	✅	869.97MB	869.97MB	0.50MB
datadog-agent-aarch64-rpm	0.00MB	✅	879.69MB	879.69MB	0.50MB
datadog-dogstatsd-amd64-deb	0.00MB	✅	59.02MB	59.02MB	0.50MB
datadog-heroku-agent-amd64-deb	0.00MB	✅	461.46MB	461.46MB	0.50MB
datadog-iot-agent-amd64-deb	0.00MB	✅	93.81MB	93.81MB	0.50MB
datadog-iot-agent-arm64-deb	0.00MB	✅	89.87MB	89.87MB	0.50MB
datadog-iot-agent-x86_64-rpm	-0.00MB	✅	93.88MB	93.88MB	0.50MB
datadog-iot-agent-x86_64-suse	-0.00MB	✅	93.88MB	93.88MB	0.50MB
datadog-dogstatsd-x86_64-rpm	-0.00MB	✅	59.10MB	59.10MB	0.50MB
datadog-dogstatsd-x86_64-suse	-0.00MB	✅	59.10MB	59.10MB	0.50MB
datadog-dogstatsd-arm64-deb	-0.00MB	✅	56.50MB	56.50MB	0.50MB

Decision

✅ Passed

agent-platform-auto-pr · 2025-01-30T23:06:14Z

Test changes on VM

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

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

Note: This applies to commit 748d0b8

cit-pr-commenter · 2025-01-30T23:33:19Z

Regression Detector

Regression Detector Results

Metrics dashboard
Target profiles
Run ID: 7bd96955-e743-4eda-9e15-da42dbea837a

Baseline: 78991f6
Comparison: 5ed5b03
Diff

Optimization Goals: ✅ No significant changes detected

Fine details of change detection per experiment

perf	experiment	goal	Δ mean %	Δ mean % CI	trials	links
➖	quality_gate_logs	% cpu utilization	+0.56	[-2.50, +3.61]	1	Logs
➖	quality_gate_idle	memory utilization	+0.46	[+0.42, +0.50]	1	Logs bounds checks dashboard
➖	file_to_blackhole_1000ms_latency_linear_load	egress throughput	+0.40	[-0.07, +0.87]	1	Logs
➖	tcp_syslog_to_blackhole	ingress throughput	+0.10	[+0.02, +0.17]	1	Logs
➖	file_to_blackhole_0ms_latency_http2	egress throughput	+0.07	[-0.81, +0.94]	1	Logs
➖	tcp_dd_logs_filter_exclude	ingress throughput	+0.00	[-0.01, +0.02]	1	Logs
➖	uds_dogstatsd_to_api	ingress throughput	-0.00	[-0.29, +0.28]	1	Logs
➖	file_to_blackhole_300ms_latency	egress throughput	-0.01	[-0.65, +0.64]	1	Logs
➖	file_to_blackhole_100ms_latency	egress throughput	-0.01	[-0.75, +0.74]	1	Logs
➖	file_to_blackhole_0ms_latency	egress throughput	-0.03	[-0.84, +0.78]	1	Logs
➖	file_to_blackhole_500ms_latency	egress throughput	-0.03	[-0.81, +0.75]	1	Logs
➖	file_to_blackhole_0ms_latency_http1	egress throughput	-0.04	[-0.84, +0.75]	1	Logs
➖	quality_gate_idle_all_features	memory utilization	-0.14	[-0.22, -0.07]	1	Logs bounds checks dashboard
➖	file_to_blackhole_1000ms_latency	egress throughput	-0.34	[-1.13, +0.45]	1	Logs
➖	file_tree	memory utilization	-0.34	[-0.41, -0.27]	1	Logs
➖	uds_dogstatsd_to_api_cpu	% cpu utilization	-0.38	[-1.27, +0.50]	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:

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

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 intake_connections: 10/10 replicas passed. Gate passed.
quality_gate_logs, bounds check lost_bytes: 10/10 replicas passed. Gate passed.
quality_gate_idle, bounds check memory_usage: 10/10 replicas passed. Gate passed.
quality_gate_idle, bounds check intake_connections: 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.

pkg/clusteragent/admission/mutate/common/injection_filter.go

pkg/clusteragent/admission/mutate/tagsfromlabels/tags.go

pkg/clusteragent/admission/mutate/config/config.go

Co-authored-by: Adel Haj Hassan <41540817+adel121@users.noreply.github.com>

This commit adds an injector interface and refactors the tagsfromlabels and the config webhooks to use it. This will allow us to use the injector logic without needing to instantiate a webhook.

betterengineering added 6 commits January 30, 2025 15:46

fix(ssi): migrate injection filter to mutate common

d356c6b

This commit migrates the injection filter into the mutate/common package given it's used by multiple webhooks.

fix(ssi): remove config from injection filter

7b4bcc5

This commit moves config fetching out of the NewInjectorFIlter method and instead requires it to be passed in.

Fixup injection_filter

431f019

Fixup namespace filter naming

181ff35

Fixup log line

dca9b2f

github-actions bot added team/container-platform The Container Platform Team medium review PR review might take time labels Jan 30, 2025

adel121 reviewed Jan 31, 2025

View reviewed changes

pkg/clusteragent/admission/mutate/common/injection_filter.go Outdated Show resolved Hide resolved

adel121 reviewed Jan 31, 2025

View reviewed changes

pkg/clusteragent/admission/mutate/tagsfromlabels/tags.go Outdated Show resolved Hide resolved

adel121 reviewed Jan 31, 2025

View reviewed changes

pkg/clusteragent/admission/mutate/tagsfromlabels/tags.go Show resolved Hide resolved

adel121 reviewed Jan 31, 2025

View reviewed changes

pkg/clusteragent/admission/mutate/config/config.go Show resolved Hide resolved

betterengineering and others added 2 commits January 31, 2025 10:14

Update pkg/clusteragent/admission/mutate/tagsfromlabels/tags.go

081bdad

Co-authored-by: Adel Haj Hassan <41540817+adel121@users.noreply.github.com>

Address PR feedback

748d0b8

betterengineering added the team/injection-platform label Jan 31, 2025

fix(ssi): create injector interface and refactor webhooks

5ed5b03

This commit adds an injector interface and refactors the tagsfromlabels and the config webhooks to use it. This will allow us to use the injector logic without needing to instantiate a webhook.

github-actions bot added long review PR is complex, plan time to review it and removed medium review PR review might take time labels Jan 31, 2025

betterengineering changed the title ~~fix(ssi): separate out usages of InjectionFilter~~ fix(ssi): decoupling admission controller webhooks Jan 31, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(ssi): decoupling admission controller webhooks #33599

fix(ssi): decoupling admission controller webhooks #33599

betterengineering commented Jan 30, 2025 •

edited

Loading

agent-platform-auto-pr bot commented Jan 30, 2025 •

edited

Loading

agent-platform-auto-pr bot commented Jan 30, 2025 •

edited

Loading

cit-pr-commenter bot commented Jan 30, 2025 •

edited

Loading

Fine details of change detection per experiment

Bounds Checks: ✅ Passed

Explanation

fix(ssi): decoupling admission controller webhooks #33599

Are you sure you want to change the base?

fix(ssi): decoupling admission controller webhooks #33599

Conversation

betterengineering commented Jan 30, 2025 • edited Loading

What does this PR do?

Motivation

Describe how you validated your changes

Possible Drawbacks / Trade-offs

Additional Notes

agent-platform-auto-pr bot commented Jan 30, 2025 • edited Loading

Uncompressed package size comparison

Decision

agent-platform-auto-pr bot commented Jan 30, 2025 • edited Loading

Test changes on VM

cit-pr-commenter bot commented Jan 30, 2025 • edited Loading

Regression Detector

Regression Detector Results

Optimization Goals: ✅ No significant changes detected

Fine details of change detection per experiment

Bounds Checks: ✅ Passed

Explanation

CI Pass/Fail Decision

betterengineering commented Jan 30, 2025 •

edited

Loading

agent-platform-auto-pr bot commented Jan 30, 2025 •

edited

Loading

agent-platform-auto-pr bot commented Jan 30, 2025 •

edited

Loading

cit-pr-commenter bot commented Jan 30, 2025 •

edited

Loading