diff --git a/charts/openstack-hypervisor-operator/values.yaml b/charts/openstack-hypervisor-operator/values.yaml index 22f61295..a80526f8 100644 --- a/charts/openstack-hypervisor-operator/values.yaml +++ b/charts/openstack-hypervisor-operator/values.yaml @@ -4,12 +4,6 @@ controllerManager: - --metrics-bind-address=:8443 - --leader-elect - --health-probe-bind-address=:8081 - - --certificate-namespace=$(CERTIFICATE_NAMESPACE) - - --certificate-issuer-name=$(CERTIFICATE_ISSUER_NAME) - - --label-selector=$(LABEL_SELECTOR) - - --agent-namespaces=$(AGENT_NAMESPACES) - - --eviction-concurrency=$(EVICTION_CONCURRENCY) - - --eviction-trait-concurrency=$(EVICTION_TRAIT_CONCURRENCY) containerSecurityContext: allowPrivilegeEscalation: false capabilities: diff --git a/cmd/main.go b/cmd/main.go index 41982a56..263b44a7 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -20,15 +20,17 @@ package main import ( "crypto/sha256" "crypto/tls" - "errors" "flag" "fmt" "os" gruntime "runtime" - "strconv" "strings" "github.com/sapcc/go-api-declarations/bininfo" + "github.com/spf13/cobra" + "github.com/spf13/pflag" + "github.com/spf13/viper" + // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. _ "k8s.io/client-go/plugin/pkg/client/auth" @@ -52,10 +54,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/config" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/eviction" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/ready" - "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/global" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/logger" // +kubebuilder:scaffold:imports @@ -77,19 +79,20 @@ func init() { utilruntime.Must(cmapi.AddToScheme(scheme)) } +type options struct { + Shared config.Shared + Certificate controller.CertificateOptions + Eviction eviction.Options +} + func main() { - var version bool var metricsAddr string var enableLeaderElection bool var probeAddr string var secureMetrics bool var enableHTTP2 bool var tlsOpts []func(*tls.Config) - var certificateNamespace string - var certificateIssuerName string - flag.BoolVar(&version, "version", false, - "Print the version and exit.") flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address the metrics endpoint binds to. "+ "Use :8443 for HTTPS or :8080 for HTTP, or leave as 0 to disable the metrics service.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") @@ -100,23 +103,8 @@ func main() { "If set, the metrics endpoint is served securely via HTTPS. Use --metrics-secure=false to use HTTP instead.") flag.BoolVar(&enableHTTP2, "enable-http2", false, "If set, HTTP/2 will be enabled for the metrics and webhook servers") - flag.StringVar(&global.LabelSelector, "label-selector", "", "Label selector to filter watched resources (namely nodes).") - - var agentNamespacesFlag string - flag.StringVar(&agentNamespacesFlag, "agent-namespaces", "", - "Comma-separated list of namespaces to search for agent pods (nova-compute, neutron) during offboarding.") - flag.IntVar(&global.EvictionConcurrency, "eviction-concurrency", 1, - "Default maximum number of VM migrations to run concurrently while draining a hypervisor. Defaults to 1 (serial).") - - var evictionTraitConcurrencyFlag string - flag.StringVar(&evictionTraitConcurrencyFlag, "eviction-trait-concurrency", "", - "Comma-separated TRAIT=N overrides for per-host migration concurrency, e.g. "+ - "CUSTOM_HANA_EXCLUSIVE_HOST=1,CUSTOM_FOO=2. The lowest matching trait limit wins.") - - flag.StringVar(&certificateNamespace, "certificate-namespace", "monsoon3", "The namespace for the certificates. ") - flag.StringVar(&certificateIssuerName, "certificate-issuer-name", "nova-hypervisor-agents-ca-issuer", - "Name of the certificate issuer.") + var cfg options opts := ctrlzap.Options{ Development: true, @@ -126,73 +114,84 @@ func main() { } opts.BindFlags(flag.CommandLine) - flag.Parse() - - // Handle --version before validating required runtime flags so that - // `./manager --version` works without providing --agent-namespaces. - if version { - fmt.Printf("%s %s (%s/%s) %s\n", - bininfo.Component(), bininfo.VersionOr("devel"), gruntime.GOOS, gruntime.GOARCH, - bininfo.CommitOr("edge")) - os.Exit(0) + + rootCmd := &cobra.Command{ + Use: "manager", + Version: bininfo.VersionOr("devel"), + SilenceUsage: true, + Run: func(cmd *cobra.Command, args []string) { + run(cfg, metricsAddr, probeAddr, enableLeaderElection, secureMetrics, enableHTTP2, tlsOpts, opts) + }, } - if agentNamespacesFlag != "" { - // Deduplicate. Without this, repeated entries (e.g. from templated - // values) would cause redundant pod-list calls and inflate the - // AgentPodsEvicted condition message with duplicate pod names. - seen := map[string]struct{}{} - for ns := range strings.SplitSeq(agentNamespacesFlag, ",") { - ns = strings.TrimSpace(ns) - if ns == "" { - continue + rootCmd.SetVersionTemplate(fmt.Sprintf("%s %s (%s/%s) %s\n", + bininfo.Component(), bininfo.VersionOr("devel"), gruntime.GOOS, gruntime.GOARCH, + bininfo.CommitOr("edge"))) + + flags := rootCmd.Flags() + cfg.Shared.AddFlags(flags) + cfg.Certificate.AddFlags(flags) + cfg.Eviction.AddFlags(flags) + // Import the flags registered on the standard flag set (manager and + // logging flags) into the command. + flags.AddGoFlagSet(flag.CommandLine) + + viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_")) + viper.AutomaticEnv() + if err := viper.BindPFlags(flags); err != nil { + fmt.Fprintf(os.Stderr, "invalid configuration: %v\n", err) + os.Exit(1) + } + + // Document each flag's environment variable. + flags.VisitAll(func(f *pflag.Flag) { + f.Usage += fmt.Sprintf(" (env %s)", strings.ToUpper(strings.ReplaceAll(f.Name, "-", "_"))) + }) + + // Apply environment values to flags not given on the command line. + rootCmd.PreRun = func(cmd *cobra.Command, args []string) { + flags.VisitAll(func(f *pflag.Flag) { + if f.Name == "version" || f.Name == "help" { + return } - if _, ok := seen[ns]; ok { - continue + if f.Changed { + return } - seen[ns] = struct{}{} - global.AgentNamespaces = append(global.AgentNamespaces, ns) - } + val := viper.GetString(f.Name) + if val == "" || val == f.DefValue { + return + } + if err := flags.Set(f.Name, val); err != nil { + fmt.Fprintf(os.Stderr, "invalid configuration: %v\n", err) + os.Exit(1) + } + }) } - if len(global.AgentNamespaces) == 0 { - setupLog.Error(errors.New("--agent-namespaces is required"), "invalid configuration") + if err := rootCmd.Execute(); err != nil { os.Exit(1) } +} - if global.EvictionConcurrency < 1 { - setupLog.Error(errors.New("--eviction-concurrency must be >= 1"), "invalid configuration") - os.Exit(1) - } +// run validates the configuration and runs the controller manager. +func run(cfg options, metricsAddr, probeAddr string, enableLeaderElection, secureMetrics, enableHTTP2 bool, tlsOpts []func(*tls.Config), opts ctrlzap.Options) { + ctrl.SetLogger(ctrlzap.New(ctrlzap.UseFlagOptions(&opts))) - for entry := range strings.SplitSeq(evictionTraitConcurrencyFlag, ",") { - entry = strings.TrimSpace(entry) - if entry == "" { - continue - } - trait, value, ok := strings.Cut(entry, "=") - trait = strings.TrimSpace(trait) - n, err := strconv.Atoi(strings.TrimSpace(value)) - if !ok || trait == "" || err != nil || n < 1 { - setupLog.Error(errors.New("invalid --eviction-trait-concurrency entry: "+entry), - "invalid configuration", "expected", "TRAIT=N with N >= 1") - os.Exit(1) - } - global.EvictionTraitConcurrency[trait] = n + if err := cfg.Shared.Validate(); err != nil { + setupLog.Error(err, "invalid configuration") + os.Exit(1) } - if certificateIssuerName == "" { - setupLog.Error(errors.New("certificate-issuer-name cannot be empty"), "invalid certificate issuer name") + if err := cfg.Eviction.Validate(); err != nil { + setupLog.Error(err, "invalid configuration") os.Exit(1) } - if certificateNamespace == "" { - setupLog.Error(errors.New("certificate-namespace cannot be empty"), "invalid certificate namespace") + if err := cfg.Certificate.Validate(); err != nil { + setupLog.Error(err, "invalid configuration") os.Exit(1) } - ctrl.SetLogger(ctrlzap.New(ctrlzap.UseFlagOptions(&opts))) - // if the enable-http2 flag is false (the default), http/2 should be disabled // due to its vulnerabilities. More specifically, disabling http/2 will // prevent from being vulnerable to the HTTP/2 Stream Cancellation and @@ -260,9 +259,9 @@ func main() { &policyv1.PodDisruptionBudget{}: maintenanceCacheConfig, }, } - if global.LabelSelector != "" { - setupLog.Info("setting up cache with label selector", "selector", global.LabelSelector) - selector, err := labels.Parse(global.LabelSelector) + if cfg.Shared.LabelSelector != "" { + setupLog.Info("setting up cache with label selector", "selector", cfg.Shared.LabelSelector) + selector, err := labels.Parse(cfg.Shared.LabelSelector) if err != nil { setupLog.Error(err, "unable to parse label selector") os.Exit(1) @@ -273,7 +272,7 @@ func main() { h := sha256.New() h.Write([]byte(leaderElectionID)) // Seed it with something "unique" to the project - h.Write([]byte(global.LabelSelector)) + h.Write([]byte(cfg.Shared.LabelSelector)) leaderElectionID = fmt.Sprintf("%x.cloud.sap", h.Sum(nil)) } @@ -315,7 +314,7 @@ func main() { if err = (&controller.HypervisorController{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), - }).SetupWithManager(mgr); err != nil { + }).SetupWithManager(mgr, cfg.Shared); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Hypervisor") os.Exit(1) } @@ -331,7 +330,7 @@ func main() { if err = (&eviction.EvictionReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), - }).SetupWithManager(mgr); err != nil { + }).SetupWithManager(mgr, cfg.Eviction); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Eviction") os.Exit(1) } @@ -355,7 +354,7 @@ func main() { if err = (&controller.NodeCertificateController{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), - }).SetupWithManager(mgr, certificateNamespace, certificateIssuerName); err != nil { + }).SetupWithManager(mgr, cfg.Certificate); err != nil { setupLog.Error(err, "unable to create controller", "certificate", "NodeCertificate") os.Exit(1) } @@ -363,7 +362,7 @@ func main() { if err = (&controller.GardenerNodeLifecycleController{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), - }).SetupWithManager(mgr, certificateNamespace); err != nil { + }).SetupWithManager(mgr, cfg.Certificate.Namespace); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Maintenance") os.Exit(1) } diff --git a/go.mod b/go.mod index ef3922c4..75ccc3a5 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,9 @@ require ( github.com/onsi/ginkgo/v2 v2.33.1 github.com/onsi/gomega v1.44.0 github.com/sapcc/go-api-declarations v1.25.1 + github.com/spf13/cobra v1.10.2 + github.com/spf13/pflag v1.0.10 + github.com/spf13/viper v1.21.0 go.uber.org/zap v1.28.0 golang.org/x/sync v0.24.0 k8s.io/api v0.37.1 @@ -54,6 +57,7 @@ require ( github.com/go-openapi/swag/typeutils v0.27.1 // indirect github.com/go-openapi/swag/yamlutils v0.27.1 // indirect github.com/go-task/slim-sprig/v3 v3.0.0 // indirect + github.com/go-viper/mapstructure/v2 v2.4.0 // indirect github.com/gofrs/uuid/v5 v5.5.1 // indirect github.com/google/cel-go v0.30.0 // indirect github.com/google/gnostic-models v0.7.1 // indirect @@ -68,13 +72,17 @@ require ( github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pandatix/go-abnf v0.5.1 // indirect + github.com/pelletier/go-toml/v2 v2.2.4 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/client_golang v1.25.0 // indirect github.com/prometheus/client_model v0.6.3 // indirect github.com/prometheus/common v0.73.0 // indirect github.com/prometheus/procfs v0.22.0 // indirect - github.com/spf13/cobra v1.10.2 // indirect - github.com/spf13/pflag v1.0.10 // indirect + github.com/sagikazarmark/locafero v0.11.0 // indirect + github.com/sourcegraph/conc v0.3.1-0.20240121214520-5f936abd7ae8 // indirect + github.com/spf13/afero v1.15.0 // indirect + github.com/spf13/cast v1.10.0 // indirect + github.com/subosito/gotenv v1.6.0 // indirect github.com/x448/float16 v0.8.4 // indirect go.opentelemetry.io/auto/sdk v1.2.1 // indirect go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.69.0 // indirect diff --git a/go.sum b/go.sum index b5f8236b..433878fe 100644 --- a/go.sum +++ b/go.sum @@ -27,6 +27,8 @@ github.com/evanphx/json-patch/v5 v5.9.11 h1:/8HVnzMq13/3x9TPvjG08wUGqBTmZBsCWzjT github.com/evanphx/json-patch/v5 v5.9.11/go.mod h1:3j+LviiESTElxA4p3EMKAB9HXj3/XEtnUf6OZxqIQTM= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= +github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= +github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.9.0 h1:2Ml+OJNzbYCTzsxtv8vKSFD9PbJjmhYF14k/jKC7S9k= github.com/fsnotify/fsnotify v1.9.0/go.mod h1:8jBTzvmWwFyi3Pb8djgCCO5IBqzKJ/Jwo8TRcHyHii0= github.com/fxamacker/cbor/v2 v2.9.1 h1:2rWm8B193Ll4VdjsJY28jxs70IdDsHRWgQYAI80+rMQ= @@ -80,6 +82,8 @@ github.com/go-openapi/testify/v2 v2.6.0 h1:5PKH2HE7YJ/LuRPQGvSxBRlFXNQhSetBLlGAg github.com/go-openapi/testify/v2 v2.6.0/go.mod h1:SgsVHtfooshd0tublTtJ50FPKhujf47YRqauXXOUxfw= github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI= github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= +github.com/go-viper/mapstructure/v2 v2.4.0 h1:EBsztssimR/CONLSZZ04E8qAkxNYq4Qp9LvH92wZUgs= +github.com/go-viper/mapstructure/v2 v2.4.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= github.com/goccy/go-yaml v1.18.0 h1:8W7wMFS12Pcas7KU+VVkaiCng+kG8QiFeFwzFb+rwuw= github.com/goccy/go-yaml v1.18.0/go.mod h1:XBurs7gK8ATbW4ZPGKgcbrY1Br56PdM69F7LkFRi1kA= github.com/gofrs/uuid/v5 v5.5.1 h1:z1Ce19/JwNidXpy3tOQc3241lnJLKdKyq/xlNvlD4Ng= @@ -139,6 +143,8 @@ github.com/onsi/gomega v1.44.0 h1:eAiGl3Pw5jz5GQdDff0BcxYpAX1JxW8xD7mFUuwNfZQ= github.com/onsi/gomega v1.44.0/go.mod h1:e/C2HwaZ1DhvjzXXuFhcR7hY7Sh9pl7MmoWKEjzwcdA= github.com/pandatix/go-abnf v0.5.1 h1:iyLvSu2Aq12iE0r4n948Qw21BHDuckLq/8WIXAAF+HI= github.com/pandatix/go-abnf v0.5.1/go.mod h1:VBmbtpFgQNEaQpyAvmi+Ecd9wAYJKzA3TmiB3L93sVg= +github.com/pelletier/go-toml/v2 v2.2.4 h1:mye9XuhQ6gvn5h28+VilKrrPoQVanw5PMw/TB0t5Ec4= +github.com/pelletier/go-toml/v2 v2.2.4/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= @@ -155,19 +161,31 @@ github.com/prometheus/procfs v0.22.0/go.mod h1:CvmFr/GVhIjIvWJZW3tgkODBQMRIf0EyW github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= +github.com/sagikazarmark/locafero v0.11.0 h1:1iurJgmM9G3PA/I+wWYIOw/5SyBtxapeHDcg+AAIFXc= +github.com/sagikazarmark/locafero v0.11.0/go.mod h1:nVIGvgyzw595SUSUE6tvCp3YYTeHs15MvlmU87WwIik= github.com/sapcc/go-api-declarations v1.25.1 h1:Lrbf8TZY/ARg0J9IrlrObqnXjrzSzmYKQd/L/a71Lsg= github.com/sapcc/go-api-declarations v1.25.1/go.mod h1:bJkK8wphzlmUmN5lxBv8BbfxUXMOkW+UiC10xBHXsFs= +github.com/sourcegraph/conc v0.3.1-0.20240121214520-5f936abd7ae8 h1:+jumHNA0Wrelhe64i8F6HNlS8pkoyMv5sreGx2Ry5Rw= +github.com/sourcegraph/conc v0.3.1-0.20240121214520-5f936abd7ae8/go.mod h1:3n1Cwaq1E1/1lhQhtRK2ts/ZwZEhjcQeJQ1RuC6Q/8U= +github.com/spf13/afero v1.15.0 h1:b/YBCLWAJdFWJTN9cLhiXXcD7mzKn9Dm86dNnfyQw1I= +github.com/spf13/afero v1.15.0/go.mod h1:NC2ByUVxtQs4b3sIUphxK0NioZnmxgyCrfzeuq8lxMg= +github.com/spf13/cast v1.10.0 h1:h2x0u2shc1QuLHfxi+cTJvs30+ZAHOGRic8uyGTDWxY= +github.com/spf13/cast v1.10.0/go.mod h1:jNfB8QC9IA6ZuY2ZjDp0KtFO2LZZlg4S/7bzP6qqeHo= github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU= github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4= github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/spf13/viper v1.21.0 h1:x5S+0EU27Lbphp4UKm1C+1oQO+rKx36vfCoaVebLFSU= +github.com/spf13/viper v1.21.0/go.mod h1:P0lhsswPGWD/1lZJ9ny3fYnVqxiegrlNrEmgLjbTCAY= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.5.3 h1:jmXUvGomnU1o3W/V5h2VEradbpJDwGrzugQQvL0POH4= github.com/stretchr/objx v0.5.3/go.mod h1:rDQraq+vQZU7Fde9LOZLr8Tax6zZvy4kuNKF+QYS+U0= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.12.1 h1:EuwCh5fleGS7H32xRwO3wRGT7DxrDhLAT6FF8MpWDWE= github.com/stretchr/testify v1.12.1/go.mod h1:MDEgiDPPsNp5cuIrHPPCyornHKgEVbtFUmoNlxoYthg= +github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8= +github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU= github.com/tidwall/gjson v1.18.0 h1:FIDeeyB800efLX89e5a8Y0BNH+LOngJyGrIWxG2FKQY= github.com/tidwall/gjson v1.18.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA= diff --git a/internal/config/options.go b/internal/config/options.go new file mode 100644 index 00000000..96494ab2 --- /dev/null +++ b/internal/config/options.go @@ -0,0 +1,60 @@ +/* +SPDX-FileCopyrightText: Copyright 2025 SAP SE or an SAP affiliate company and cobaltcore-dev contributors +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package config + +import ( + "errors" + "slices" + "strings" + + "github.com/spf13/pflag" +) + +// Shared contains configuration options used by more than one controller. +type Shared struct { + LabelSelector string + AgentNamespaces []string +} + +// AddFlags registers the shared options on the flag set. +func (s *Shared) AddFlags(fs *pflag.FlagSet) { + fs.StringVar(&s.LabelSelector, "label-selector", "", + "Label selector to filter watched resources (namely nodes).") + fs.StringSliceVar(&s.AgentNamespaces, "agent-namespaces", nil, + "Comma-separated list of namespaces to search for agent pods (nova-compute, neutron) during offboarding.") +} + +// Validate normalizes the agent namespace list and requires at least one +// namespace to remain. +func (s *Shared) Validate() error { + namespaces := make([]string, 0, len(s.AgentNamespaces)) + for _, namespace := range s.AgentNamespaces { + namespace = strings.TrimSpace(namespace) + if namespace == "" || slices.Contains(namespaces, namespace) { + continue + } + namespaces = append(namespaces, namespace) + } + s.AgentNamespaces = namespaces + + if len(namespaces) == 0 { + return errors.New("--agent-namespaces (AGENT_NAMESPACES) must contain at least one namespace") + } + + return nil +} diff --git a/internal/controller/eviction/controller.go b/internal/controller/eviction/controller.go index 8e80c15f..9d47042a 100644 --- a/internal/controller/eviction/controller.go +++ b/internal/controller/eviction/controller.go @@ -47,6 +47,7 @@ type EvictionReconciler struct { client.Client Scheme *runtime.Scheme computeClient *gophercloud.ServiceClient + Options Options } const ( @@ -173,7 +174,7 @@ func (r *EvictionReconciler) handleRunning(ctx context.Context, eviction *kvmv1. // That should leave us with "Running" and the hypervisor should be deactivated if len(eviction.Status.OutstandingInstances) > 0 { - limit := global.ResolveConcurrency(hypervisor.Status.Traits) + limit := r.Options.ResolveConcurrency(hypervisor.Status.Traits) return r.evictNext(ctx, eviction, limit) } @@ -533,7 +534,9 @@ func (r *EvictionReconciler) registerWithManager(mgr ctrl.Manager) error { } // SetupWithManager sets up the controller with the Manager. -func (r *EvictionReconciler) SetupWithManager(mgr ctrl.Manager) error { +func (r *EvictionReconciler) SetupWithManager(mgr ctrl.Manager, options Options) error { + r.Options = options + ctx := context.Background() var err error diff --git a/internal/controller/eviction/controller_test.go b/internal/controller/eviction/controller_test.go index 4be3a0ef..5ed875af 100644 --- a/internal/controller/eviction/controller_test.go +++ b/internal/controller/eviction/controller_test.go @@ -33,7 +33,6 @@ import ( ctrl "sigs.k8s.io/controller-runtime" kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" - "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/global" ) var _ = Describe("Instance slice helpers", func() { @@ -663,9 +662,9 @@ var _ = Describe("Eviction Controller", func() { } It("starts at most `limit` migrations per pass and never exceeds it in flight", func(ctx SpecContext) { - orig := global.EvictionConcurrency - global.EvictionConcurrency = 3 - DeferCleanup(func() { global.EvictionConcurrency = orig }) + orig := evictionReconciler.Options.Concurrency + evictionReconciler.Options.Concurrency = 3 + DeferCleanup(func() { evictionReconciler.Options.Concurrency = orig }) seedVMs(ctx, 5) resource := &kvmv1.Eviction{} @@ -704,9 +703,9 @@ var _ = Describe("Eviction Controller", func() { // with task_state "migrating" for a while before Status becomes // MIGRATING. If those aren't counted as in-flight, the loop keeps // triggering and exceeds the concurrency limit. - orig := global.EvictionConcurrency - global.EvictionConcurrency = 2 - DeferCleanup(func() { global.EvictionConcurrency = orig }) + orig := evictionReconciler.Options.Concurrency + evictionReconciler.Options.Concurrency = 2 + DeferCleanup(func() { evictionReconciler.Options.Concurrency = orig }) const tmpl = `{ "server": { @@ -798,13 +797,13 @@ var _ = Describe("Eviction Controller", func() { }) It("forces serial migration for hosts carrying the exclusive trait", func(ctx SpecContext) { - orig := global.EvictionConcurrency - origMap := global.EvictionTraitConcurrency - global.EvictionConcurrency = 4 - global.EvictionTraitConcurrency = map[string]int{"CUSTOM_HANA_EXCLUSIVE_HOST": 1} + orig := evictionReconciler.Options.Concurrency + origMap := evictionReconciler.Options.TraitConcurrency + evictionReconciler.Options.Concurrency = 4 + evictionReconciler.Options.TraitConcurrency = map[string]int{"CUSTOM_HANA_EXCLUSIVE_HOST": 1} DeferCleanup(func() { - global.EvictionConcurrency = orig - global.EvictionTraitConcurrency = origMap + evictionReconciler.Options.Concurrency = orig + evictionReconciler.Options.TraitConcurrency = origMap }) By("Marking the hypervisor with the exclusive trait") diff --git a/internal/controller/eviction/options.go b/internal/controller/eviction/options.go new file mode 100644 index 00000000..7eb116f6 --- /dev/null +++ b/internal/controller/eviction/options.go @@ -0,0 +1,76 @@ +/* +SPDX-FileCopyrightText: Copyright 2025 SAP SE or an SAP affiliate company and cobaltcore-dev contributors +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package eviction + +import ( + "errors" + "fmt" + "strings" + + "github.com/spf13/pflag" +) + +// Options configures the eviction controller. +type Options struct { + Concurrency int + TraitConcurrency map[string]int +} + +// AddFlags registers the eviction options on the flag set. +func (o *Options) AddFlags(fs *pflag.FlagSet) { + fs.IntVar(&o.Concurrency, "eviction-concurrency", 1, + "Default maximum number of VM migrations to run concurrently while draining a hypervisor. Defaults to 1 (serial).") + fs.StringToIntVar(&o.TraitConcurrency, "eviction-trait-concurrency", nil, + "Comma-separated TRAIT=N overrides for per-host migration concurrency, e.g. "+ + "CUSTOM_HANA_EXCLUSIVE_HOST=1,CUSTOM_FOO=2. The lowest matching trait limit wins.") +} + +// ResolveConcurrency returns the maximum number of concurrent migrations +// for a host given its traits: the default, lowered by the lowest matching +// trait limit, and never below 1. +func (o Options) ResolveConcurrency(traits []string) int { + limit := o.Concurrency + for _, trait := range traits { + if traitLimit, ok := o.TraitConcurrency[trait]; ok && traitLimit < limit { + limit = traitLimit + } + } + return max(limit, 1) +} + +// Validate checks that the concurrency and every trait limit are >= 1 and +// that trait names are non-empty and carry no surrounding whitespace. +func (o Options) Validate() error { + if o.Concurrency < 1 { + return fmt.Errorf("--eviction-concurrency (EVICTION_CONCURRENCY) must be >= 1, got %d", o.Concurrency) + } + + for trait, limit := range o.TraitConcurrency { + if limit < 1 { + return fmt.Errorf("--eviction-trait-concurrency (EVICTION_TRAIT_CONCURRENCY): trait %q must have an integer limit >= 1, got %d", trait, limit) + } + if trait == "" { + return errors.New("--eviction-trait-concurrency (EVICTION_TRAIT_CONCURRENCY): trait name must not be empty") + } + if trait != strings.TrimSpace(trait) { + return fmt.Errorf("--eviction-trait-concurrency (EVICTION_TRAIT_CONCURRENCY): trait name %q must not contain surrounding whitespace", trait) + } + } + + return nil +} diff --git a/internal/controller/hypervisor_controller.go b/internal/controller/hypervisor_controller.go index 76685d90..3c79d8f2 100644 --- a/internal/controller/hypervisor_controller.go +++ b/internal/controller/hypervisor_controller.go @@ -41,7 +41,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" - "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/global" + "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/config" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils" ) @@ -68,7 +68,8 @@ var transferLabels = append([]string{}, defaultTransferLabels...) type HypervisorController struct { k8sclient.Client - Scheme *runtime.Scheme + Scheme *runtime.Scheme + Options config.Shared } // +kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch @@ -213,7 +214,9 @@ func syncLabelsAndAnnotations(nodeLabels labels.Set, hypervisor *kvmv1.Hyperviso transportAggregatesAndTraits(&node.ObjectMeta, hypervisor) } -func (hv *HypervisorController) SetupWithManager(mgr ctrl.Manager) error { +func (hv *HypervisorController) SetupWithManager(mgr ctrl.Manager, options config.Shared) error { + hv.Options = options + novaVirtLabeledPredicate, err := predicate.LabelSelectorPredicate(metav1.LabelSelector{ MatchExpressions: []metav1.LabelSelectorRequirement{ { @@ -227,8 +230,8 @@ func (hv *HypervisorController) SetupWithManager(mgr ctrl.Manager) error { } // append the custom label selector from global config - if global.LabelSelector != "" { - requirements, err := labels.ParseToRequirements(global.LabelSelector) + if hv.Options.LabelSelector != "" { + requirements, err := labels.ParseToRequirements(hv.Options.LabelSelector) if err != nil { return fmt.Errorf("failed to parse global label selector: %w", err) } @@ -320,7 +323,7 @@ func (hv *HypervisorController) computeAgentPodsEvictedCondition(ctx context.Con } var agentPods []string - for _, ns := range global.AgentNamespaces { + for _, ns := range hv.Options.AgentNamespaces { var continueToken string for { pods := &corev1.PodList{} diff --git a/internal/controller/hypervisor_controller_test.go b/internal/controller/hypervisor_controller_test.go index c0f45899..46508b93 100644 --- a/internal/controller/hypervisor_controller_test.go +++ b/internal/controller/hypervisor_controller_test.go @@ -33,7 +33,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" - "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/global" ) // envtest does not actually GC pods when their namespace is deleted, so @@ -455,8 +454,8 @@ var _ = Describe("Hypervisor Controller", func() { }) // Restrict pod listing to the test namespace. - global.AgentNamespaces = []string{agentNamespace} - DeferCleanup(func() { global.AgentNamespaces = nil }) + hypervisorController.Options.AgentNamespaces = []string{agentNamespace} + DeferCleanup(func() { hypervisorController.Options.AgentNamespaces = nil }) // The condition is only computed during termination. _, err := hypervisorController.Reconcile(ctx, ctrl.Request{ diff --git a/internal/controller/node_certificate_controller.go b/internal/controller/node_certificate_controller.go index 8287c482..d1d68d69 100644 --- a/internal/controller/node_certificate_controller.go +++ b/internal/controller/node_certificate_controller.go @@ -156,9 +156,9 @@ func (r *NodeCertificateController) Reconcile(ctx context.Context, req ctrl.Requ } // SetupWithManager sets up the controller with the Manager. -func (r *NodeCertificateController) SetupWithManager(mgr ctrl.Manager, namespace, issuerName string) error { - r.namespace = namespace - r.issuerName = issuerName +func (r *NodeCertificateController) SetupWithManager(mgr ctrl.Manager, options CertificateOptions) error { + r.namespace = options.Namespace + r.issuerName = options.IssuerName novaVirtLabeledPredicate, err := predicate.LabelSelectorPredicate(metav1.LabelSelector{ MatchExpressions: []metav1.LabelSelectorRequirement{ diff --git a/internal/controller/options.go b/internal/controller/options.go new file mode 100644 index 00000000..662f1a05 --- /dev/null +++ b/internal/controller/options.go @@ -0,0 +1,49 @@ +/* +SPDX-FileCopyrightText: Copyright 2025 SAP SE or an SAP affiliate company and cobaltcore-dev contributors +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "errors" + + "github.com/spf13/pflag" +) + +// CertificateOptions configures the node certificate controller. +type CertificateOptions struct { + Namespace string + IssuerName string +} + +// AddFlags registers the certificate options on the flag set. +func (o *CertificateOptions) AddFlags(fs *pflag.FlagSet) { + fs.StringVar(&o.Namespace, "certificate-namespace", "monsoon3", "The namespace for the certificates.") + fs.StringVar(&o.IssuerName, "certificate-issuer-name", "nova-hypervisor-agents-ca-issuer", + "Name of the certificate issuer.") +} + +// Validate checks that the certificate controller has the required settings. +func (o CertificateOptions) Validate() error { + if o.Namespace == "" { + return errors.New("--certificate-namespace (CERTIFICATE_NAMESPACE) cannot be empty") + } + if o.IssuerName == "" { + return errors.New("--certificate-issuer-name (CERTIFICATE_ISSUER_NAME) cannot be empty") + } + + return nil +} diff --git a/internal/global/global.go b/internal/global/global.go index 9e40d29e..ec63c20b 100644 --- a/internal/global/global.go +++ b/internal/global/global.go @@ -30,44 +30,3 @@ const ( // poll cycle. ShortRetryTime = 1 * time.Second ) - -var ( - // LabelSelector is a custom label that is used to select resources managed by the operator. - LabelSelector = "" - - // AgentNamespaces is the list of namespaces in which agent pods (nova-compute, - // neutron) are scheduled. The pod list during offboarding is restricted to - // these namespaces. Must be non-empty; set via --agent-namespaces. - AgentNamespaces []string - - // EvictionConcurrency is the default maximum number of VM migrations that may - // run concurrently while draining a single hypervisor. Defaults to 1 (serial), - // which preserves the historical one-at-a-time behavior. Set via - // --eviction-concurrency. - EvictionConcurrency = 1 - - // EvictionTraitConcurrency maps a Placement trait name to a maximum number of - // concurrent migrations for hosts carrying that trait. It overrides - // EvictionConcurrency for matching hosts. Typical use is forcing exclusive host - // classes (e.g. CUSTOM_HANA_EXCLUSIVE_HOST) to migrate serially. Set via - // --eviction-trait-concurrency. - EvictionTraitConcurrency = map[string]int{} -) - -// ResolveConcurrency returns the maximum number of concurrent migrations for a -// host given its traits. It starts from EvictionConcurrency and, for every trait -// present in EvictionTraitConcurrency, keeps the lowest configured limit. The -// result is always clamped to at least 1 so a misconfiguration can never stall a -// drain entirely. -func ResolveConcurrency(traits []string) int { - limit := EvictionConcurrency - for _, t := range traits { - if traitLimit, ok := EvictionTraitConcurrency[t]; ok && traitLimit < limit { - limit = traitLimit - } - } - if limit < 1 { - limit = 1 - } - return limit -} diff --git a/internal/global/global_test.go b/internal/global/global_test.go index 167cdfc0..9d0f9256 100644 --- a/internal/global/global_test.go +++ b/internal/global/global_test.go @@ -15,19 +15,15 @@ See the License for the specific language governing permissions and limitations under the License. */ -package global +package global_test -import "testing" +import ( + "testing" -func TestResolveConcurrency(t *testing.T) { - // Save and restore the package globals so tests don't leak into each other. - origDefault := EvictionConcurrency - origMap := EvictionTraitConcurrency - t.Cleanup(func() { - EvictionConcurrency = origDefault - EvictionTraitConcurrency = origMap - }) + "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/eviction" +) +func TestResolveConcurrency(t *testing.T) { tests := []struct { name string defaultVal int @@ -81,9 +77,11 @@ func TestResolveConcurrency(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - EvictionConcurrency = tt.defaultVal - EvictionTraitConcurrency = tt.traitMap - if got := ResolveConcurrency(tt.traits); got != tt.want { + options := eviction.Options{ + Concurrency: tt.defaultVal, + TraitConcurrency: tt.traitMap, + } + if got := options.ResolveConcurrency(tt.traits); got != tt.want { t.Errorf("ResolveConcurrency(%v) = %d, want %d", tt.traits, got, tt.want) } })