Skip to content

Parse configuration into typed options structs with cobra - #379

Draft
fwiesel wants to merge 2 commits into
mainfrom
config/cobra
Draft

fwiesel wants to merge 2 commits into
mainfrom
config/cobra

Conversation

@fwiesel

@fwiesel fwiesel commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Keep operator parameters in module-owned typed options structs.
Adding a parameter changes its module and the chart environment values,
not a central parameter registry.

Cobra and pflag parse the flags.
Viper supplies environment values.
Explicit flags take precedence. --help documents the environment names.
Invalid configuration fails at startup.

Trait concurrency uses map[string]int and native StringToIntVar parsing.
Malformed integers fail during parsing. Validation rejects limits below 1
and empty or whitespace-padded trait names. The lowest matching limit wins,
with the default as the cap and a minimum result of 1.

The chart removes six --flag=$(ENV_VAR) arguments in a separate commit.

Alternative to #373, which uses go-flagsfiller instead of cobra and viper.
Existing tests retain the option wiring changes. No new repository tests.

Relates to #13.

@fwiesel
fwiesel force-pushed the config/cobra branch 2 times, most recently from 3c78f30 to 47e0676 Compare October 30, 2026 16:16
A controller parameter should not need a central configuration registry.
Let each module declare typed options and flags. Use cobra and viper for
command-line parsing and environment values. Flags take precedence.
Keep the chart environment values and document them in --help.

Use native integer trait limits so malformed values fail during parsing.
Keep concurrency guards and the existing certificate namespace wiring.

Signed-off-by: Fabian Wiesel <[email protected]>
The manager reads its settings from the environment, so drop the
--flag=$(ENV_VAR) container arguments.

Signed-off-by: Fabian Wiesel <[email protected]>
@github-actions

Copy link
Copy Markdown

Merging this branch will decrease overall coverage

Impacted Packages Coverage Δ 🤖
github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/config 0.00% (ø)
github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller 74.09% (-0.68%) 👎
github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/eviction 64.45% (-3.42%) 👎
github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/global 0.00% (-100.00%) 💀 💀 💀 💀 💀
Coverage details

Compared to commit f5172c4 (run #41086905849)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/cobaltcore-dev/openstack-hypervisor-operator/cmd/main.go 0.00% (ø) 0 0 0
github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/config/options.go 0.00% (ø) 12 (+12) 0 12 (+12)
github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/eviction/controller.go 67.53% (-0.35%) 194 (+1) 131 63 (+1) 👎
github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/eviction/options.go 29.41% (+29.41%) 17 (+17) 5 (+5) 12 (+12) 🌟
github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/hypervisor_controller.go 79.34% (-0.66%) 121 (+1) 96 25 (+1) 👎
github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/node_certificate_controller.go 80.95% (ø) 42 34 8
github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/options.go 0.00% (ø) 7 (+7) 0 7 (+7)
github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/global/global.go 0.00% (-100.00%) 0 (-7) 0 (-7) 0 💀 💀 💀 💀 💀

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files:

  • github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/eviction/controller_test.go
  • github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/hypervisor_controller_test.go
  • github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/global/global_test.go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant