Refactor GKE Autopilot support to use capabilities framework#3137
Conversation
🛑 Gate Violations
ℹ️ Info🎯 Code Coverage (details) Useful? React with 👍 / 👎 This comment will be updated automatically if new data arrives.🔗 Commit SHA: fc752ff | Docs | Datadog PR Page | Give us feedback! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb4c2c75fc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| // Apply provider-conditional global (default-layer) mutations. Runs after | ||
| // global settings. | ||
| global.ApplyGlobalNodeAgentSpec(podManagers, provider) |
There was a problem hiding this comment.
When GKE Autopilot is enabled and a user has a nodeAgent override that adds one of the volumes or mounts Autopilot strips (for example runtimesocketdir or datadog-agent-auth), this call now runs before the override loop below, so the later override.PodTemplateSpec call reintroduces forbidden hostPath/auth mounts into the rendered DaemonSet. Previously the experimental Autopilot override ran after nodeAgent overrides and stripped those entries, so existing Autopilot specs could still render pods accepted by the WorkloadAllowlist. Please move or reapply the Autopilot removals after nodeAgent overrides, and mirror the ordering fix in the ExtendedDaemonSet branch.
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (57.62%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #3137 +/- ##
==========================================
- Coverage 43.96% 43.92% -0.05%
==========================================
Files 377 378 +1
Lines 30774 30711 -63
==========================================
- Hits 13530 13490 -40
+ Misses 16327 16312 -15
+ Partials 917 909 -8
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
tbavelier
left a comment
tbavelier
left a comment
There was a problem hiding this comment.
Autopilot part looks great, thank you, this is much clearer than previous, great framework.
Could we move the un-related files out of this PR in a different one, e.g. comprehensive-aks, eks, etc. ?
3 participants
What does this PR do?
CONTP-1743
Refactoring, should be non-functional change.
9842240 adds golden files for Operator generated k8s resources (besides allowlist).
comprehensive-dda.yamlisn't working on actual Autopilot.eb4c2c7 actual refactor including golden file diffs which are no-ops just several rearranged env vars.
Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
Minimum Agent Versions
Are there minimum versions of the Datadog Agent and/or Cluster Agent required?
Describe your test plan
https://github.com/DataDog/datadog-operator/blob/main/docs/gke_autopilot/external.md
Checklist
bug,enhancement,refactoring,documentation,tooling, and/ordependenciesqa/skip-qalabel