Add env var overrides for controller enable flags#3181
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3181 +/- ##
==========================================
+ Coverage 44.03% 44.27% +0.23%
==========================================
Files 377 377
Lines 30713 30763 +50
==========================================
+ Hits 13525 13620 +95
+ Misses 16300 16255 -45
Partials 888 888
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:
|
![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: a050c224b2
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | Pprof | `--pprof` | `DD_PPROF_ENABLED` | `false` | | ||
| | Leader election | `--enable-leader-election` | `DD_LEADER_ELECTION_ENABLED` | `true` | |
There was a problem hiding this comment.
In the default manager/OLM manifests I checked (config/manager/manager.yaml:39-41 and bundle/manifests/datadog-operator.clusterserviceversion.yaml:913-915), the operator is always started with --enable-leader-election and --pprof. Since the new parser applies env values before flag.Parse() and explicitly gives CLI args precedence, setting DD_LEADER_ELECTION_ENABLED=false or DD_PPROF_ENABLED=false in an OLM Subscription will be overwritten by those existing args, so users following this table cannot actually disable those options without patching container args.
Useful? React with 👍 / 👎.
janine-c
left a comment
janine-c
left a comment
There was a problem hiding this comment.
Looks great! I just have some very minor writing suggestions I noticed.
For full compliance with the docs team writing guide, I'd get an agent to go through and replace the word via with something like using or through, but I don't think that's a showstopper in this context.
>[!NOTE] > This PR is created by pushing locally the commits from #3097 for the purpose of CI. Original commits are preserved with authorship. >[!IMPORTANT] > This PR now goes beyond the original controller-only boolean env-var fallback. It centralizes environment override parsing through a small typed helper so options can share the same precedence model: compiled default < environment variable < CLI flag. > > The updated implementation supports boolean, string, integer, and duration env vars, while still applying them before `flag.Parse()` so explicit CLI arguments win. In addition to the controller enable flags, it now covers selected non-EDS startup options such as metrics binding, secure metrics, profiling, pprof, leader election, Cilium support, maximum goroutines, controller revisions, and the untaint controller's CSI readiness gate. > > EDS flags, log flags, secret backend flags, and `--version` remain CLI-only. Tests were added for env-over-default behavior, CLI-over-env precedence, and invalid env values preserving defaults. ### `cmd/main.go` - Added `boolFromEnv` helper using `os.LookupEnv` + `strconv.ParseBool` - 12 env var overrides applied before `flag.Parse()` so CLI flags take precedence - Invalid values logged to stderr (logger is not initialized at parse time) ### Documentation | Doc | Env var documented | |-----|--------------------| | `docs/installation.md` | Full reference table with all 12 flags | | `docs/untaint_controller.md` | `DD_UNTAINT_CONTROLLER_ENABLED` | | `docs/datadog_agent_profiles.md` | `DD_AGENT_PROFILE_CONTROLLER_ENABLED` | | `docs/introspection.md` | `DD_INTROSPECTION_ENABLED` | | `docs/control_plane_monitoring.md` | `DD_INTROSPECTION_ENABLED` (OLM alternative to CSV patch) | | `docs/datadog_dashboard.md` | `DD_DASHBOARD_CONTROLLER_ENABLED` | | `docs/datadog_monitor.md` | `DD_MONITOR_CONTROLLER_ENABLED` | | `docs/datadog_slo.md` | `DD_SLO_CONTROLLER_ENABLED` | | `docs/datadog_generic_resource.md` | `DD_GENERIC_RESOURCE_CONTROLLER_ENABLED` | ## Test plan - [x] `go build ./cmd/main.go` passes - [x] `go test ./cmd/...` passes - [x] `go vet ./cmd/...` clean - [ ] Set `DD_DASHBOARD_CONTROLLER_ENABLED=true` on operator pod, verify controller starts - [ ] Set invalid value (e.g. `yes`), verify stderr error message - [ ] Set env var + CLI flag, verify CLI flag wins Original commits: * f899bf9 Add env var overrides for controller enable flags * bbdc4b5 Add more options + use small generic helper to parse multiple types + tests * a050c22 Merge branch 'main' into savec80/feat/env-var-controller-flags * c04e806 Update docs/control_plane_monitoring.md * c226883 Update docs/installation.md * 06f778f Update docs/installation.md * ea9586c Update docs/installation.md * b6e7ad0 Update docs/control_plane_monitoring.md * 7ab35d3 Merge branch 'main' into savec80/feat/env-var-controller-flags Signed-off-by: Timothée Bavelier <[email protected]>
7ab35d3 to
03d1e1a
Compare
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: done
and removed
mergequeue-status: in_progress
labels
4 participants
Note
This PR is created by pushing locally the commits from #3097 for the purpose of CI. Original commits are preserved with authorship.
Important
This PR now goes beyond the original controller-only boolean env-var fallback. It centralizes environment override parsing through a small typed helper so options can share the same precedence model: compiled default < environment variable < CLI flag.
The updated implementation supports boolean, string, integer, and duration env vars, while still applying them before
flag.Parse()so explicit CLI arguments win. In addition to the controller enable flags, it now covers selected non-EDS startup options such as metrics binding, secure metrics, profiling, pprof, leader election, Cilium support, maximum goroutines, controller revisions, and the untaint controller's CSI readiness gate.EDS flags, log flags, secret backend flags, and
--versionremain CLI-only. Tests were added for env-over-default behavior, CLI-over-env precedence, and invalid env values preserving defaults.Changes
cmd/main.goboolFromEnvhelper usingos.LookupEnv+strconv.ParseBoolflag.Parse()so CLI flags take precedenceDocumentation
docs/installation.mddocs/untaint_controller.mdDD_UNTAINT_CONTROLLER_ENABLEDdocs/datadog_agent_profiles.mdDD_AGENT_PROFILE_CONTROLLER_ENABLEDdocs/introspection.mdDD_INTROSPECTION_ENABLEDdocs/control_plane_monitoring.mdDD_INTROSPECTION_ENABLED(OLM alternative to CSV patch)docs/datadog_dashboard.mdDD_DASHBOARD_CONTROLLER_ENABLEDdocs/datadog_monitor.mdDD_MONITOR_CONTROLLER_ENABLEDdocs/datadog_slo.mdDD_SLO_CONTROLLER_ENABLEDdocs/datadog_generic_resource.mdDD_GENERIC_RESOURCE_CONTROLLER_ENABLEDTest plan
go build ./cmd/main.gopassesgo test ./cmd/...passesgo vet ./cmd/...cleanDD_DASHBOARD_CONTROLLER_ENABLED=trueon operator pod, verify controller startsyes), verify stderr error message