diff --git a/charts/openstack-hypervisor-operator/values.yaml b/charts/openstack-hypervisor-operator/values.yaml index 22f61295..bc0a6615 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: @@ -23,8 +17,10 @@ controllerManager: # Default maximum number of VM migrations to run concurrently while draining a hypervisor. # Keep this quoted because it is rendered as an environment variable. evictionConcurrency: "1" - # Optional comma-separated TRAIT=N overrides for per-host migration concurrency. - # The lowest matching trait limit wins, e.g. "CUSTOM_HANA_EXCLUSIVE_HOST=1,CUSTOM_FOO=2". + # Optional JSON object with integer per-host migration limits >= 1. + # Example: '{"CUSTOM_HANA_EXCLUSIVE_HOST":1,"CUSTOM_FOO":2}'. Empty text clears the map. + # Trait names must be nonempty without surrounding whitespace. The lowest matching limit wins. + # CLI flags repeat --eviction-trait-concurrency=TRAIT:N to merge keys. Empty CLI input is invalid. evictionTraitConcurrency: "" osAuthUrl: "" osProjectDomainName: "" diff --git a/cmd/config.go b/cmd/config.go new file mode 100644 index 00000000..cafedcc9 --- /dev/null +++ b/cmd/config.go @@ -0,0 +1,86 @@ +/* +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 main + +import ( + "encoding/json" + "reflect" + "strings" + + "github.com/go-viper/mapstructure/v2" + "github.com/knadh/koanf/providers/env" + "github.com/knadh/koanf/v2" + + "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller" + "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/eviction" +) + +type options struct { + controller.Shared `koanf:",squash"` + Certificate controller.CertificateOptions `koanf:",squash"` + Eviction eviction.Options `koanf:",squash"` +} + +// loadOptions reads the operator settings from the environment, starting from +// the built-in defaults for settings whose env var is not set. Each module +// owns its options struct and defaults. A new tagged field needs no loader +// changes or flag registration. +func loadOptions() (options, error) { + cfg := options{ + Shared: controller.DefaultOptions(), + Certificate: controller.DefaultCertificateOptions(), + Eviction: eviction.DefaultOptions(), + } + k := koanf.New(".") + + // Environment variables use the same names as before, lowercased as keys. + if err := k.Load(env.Provider("", ".", strings.ToLower), nil); err != nil { + return cfg, err + } + + // Decode lists and JSON maps by target type. Keep text unmarshaling for + // other types. Plain strings, including label selectors, stay unchanged. + decoderConfig := &mapstructure.DecoderConfig{ + DecodeHook: mapstructure.ComposeDecodeHookFunc( + mapstructure.StringToSliceHookFunc(","), + stringToMapJSONHook, + mapstructure.TextUnmarshallerHookFunc(), + ), + WeaklyTypedInput: true, + } + + err := k.UnmarshalWithConf("", &cfg, koanf.UnmarshalConf{ + DecoderConfig: decoderConfig, + }) + return cfg, err +} + +// stringToMapJSONHook decodes JSON into a fresh map of the target type. +// An empty environment value clears the map. Errors do not change the target. +func stringToMapJSONHook(from, to reflect.Type, data any) (any, error) { + if from.Kind() != reflect.String || to.Kind() != reflect.Map { + return data, nil + } + result := reflect.New(to) + if text := data.(string); text != "" { + if err := json.Unmarshal([]byte(text), result.Interface()); err != nil { + return nil, err + } + } + return result.Elem().Interface(), nil +} diff --git a/cmd/main.go b/cmd/main.go index 41982a56..49be90b5 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -20,15 +20,14 @@ package main import ( "crypto/sha256" "crypto/tls" - "errors" "flag" "fmt" "os" gruntime "runtime" - "strconv" - "strings" + "github.com/octago/sflags/gen/gflag" "github.com/sapcc/go-api-declarations/bininfo" + // 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" @@ -55,7 +54,6 @@ import ( "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 @@ -85,8 +83,6 @@ func main() { 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.") @@ -100,23 +96,20 @@ 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.") + cfg, err := loadOptions() + if err != nil { + fmt.Fprintf(os.Stderr, "invalid configuration: %v\n", err) + os.Exit(1) + } - 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.") + // Register CLI flags from the env-filled structs so that explicit + // command-line arguments override environment values; the current + // (env-derived) field values become the flag defaults. + if err := gflag.ParseTo(&cfg, flag.CommandLine); err != nil { + fmt.Fprintf(os.Stderr, "invalid configuration: %v\n", err) + os.Exit(1) + } opts := ctrlzap.Options{ Development: true, @@ -137,62 +130,23 @@ func main() { os.Exit(0) } - 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 - } - if _, ok := seen[ns]; ok { - continue - } - seen[ns] = struct{}{} - global.AgentNamespaces = append(global.AgentNamespaces, ns) - } - } - - if len(global.AgentNamespaces) == 0 { - setupLog.Error(errors.New("--agent-namespaces is required"), "invalid configuration") - os.Exit(1) - } + ctrl.SetLogger(ctrlzap.New(ctrlzap.UseFlagOptions(&opts))) - if global.EvictionConcurrency < 1 { - setupLog.Error(errors.New("--eviction-concurrency must be >= 1"), "invalid configuration") + if err := cfg.Validate(); err != nil { + setupLog.Error(err, "invalid configuration") os.Exit(1) } - 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 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 +214,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.LabelSelector != "" { + setupLog.Info("setting up cache with label selector", "selector", cfg.LabelSelector) + selector, err := labels.Parse(cfg.LabelSelector) if err != nil { setupLog.Error(err, "unable to parse label selector") os.Exit(1) @@ -273,7 +227,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.LabelSelector)) leaderElectionID = fmt.Sprintf("%x.cloud.sap", h.Sum(nil)) } @@ -315,7 +269,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 +285,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 +309,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 +317,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..5550e0de 100644 --- a/go.mod +++ b/go.mod @@ -8,8 +8,12 @@ go 1.27 require ( github.com/cert-manager/cert-manager v1.21.2 github.com/go-logr/logr v1.4.4 + github.com/go-viper/mapstructure/v2 v2.4.0 github.com/gophercloud/gophercloud/v2 v2.15.0 github.com/gophercloud/utils/v2 v2.0.0-20260922174841-b4759ea0529b + github.com/knadh/koanf/providers/env v1.1.0 + github.com/knadh/koanf/v2 v2.4.1 + github.com/octago/sflags v0.3.1 github.com/onsi/ginkgo/v2 v2.33.1 github.com/onsi/gomega v1.44.0 github.com/sapcc/go-api-declarations v1.25.1 @@ -64,6 +68,9 @@ require ( github.com/hashicorp/go-uuid v1.0.3 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/json-iterator/go v1.1.12 // indirect + github.com/knadh/koanf/maps v0.1.2 // indirect + github.com/mitchellh/copystructure v1.2.0 // indirect + github.com/mitchellh/reflectwalk v1.0.2 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect diff --git a/go.sum b/go.sum index b5f8236b..04a1f9e7 100644 --- a/go.sum +++ b/go.sum @@ -80,6 +80,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= @@ -115,6 +117,12 @@ github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnr github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/klauspost/compress v1.20.1 h1:T7kKElXUMXrUJ2E9QhQhxFtcK5rPyLdsGZvdbLMPdiQ= github.com/klauspost/compress v1.20.1/go.mod h1:LUdAzn7YLVvxLpc7y3V1m40wESHTgc1422pwwBSKYuI= +github.com/knadh/koanf/maps v0.1.2 h1:RBfmAW5CnZT+PJ1CVc1QSJKf4Xu9kxfQgYVQSu8hpbo= +github.com/knadh/koanf/maps v0.1.2/go.mod h1:npD/QZY3V6ghQDdcQzl1W4ICNVTkohC8E73eI2xW4yI= +github.com/knadh/koanf/providers/env v1.1.0 h1:U2VXPY0f+CsNDkvdsG8GcsnK4ah85WwWyJgef9oQMSc= +github.com/knadh/koanf/providers/env v1.1.0/go.mod h1:QhHHHZ87h9JxJAn2czdEl6pdkNnDh/JS1Vtsyt65hTY= +github.com/knadh/koanf/v2 v2.4.1 h1:MK2JDkLy4UGzEUkW1CCCKQftd9+M+NX8hAZ2axbf/jo= +github.com/knadh/koanf/v2 v2.4.1/go.mod h1:gRb40VRAbd4iJMYYD5IxZ6hfuopFcXBpc9bbQpZwo28= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= @@ -125,6 +133,10 @@ github.com/maruel/natural v1.1.1 h1:Hja7XhhmvEFhcByqDoHz9QZbkWey+COd9xWfCfn1ioo= github.com/maruel/natural v1.1.1/go.mod h1:v+Rfd79xlw1AgVBjbO0BEQmptqb5HvL/k9GRHB7ZKEg= github.com/mfridman/tparse v0.18.0 h1:wh6dzOKaIwkUGyKgOntDW4liXSo37qg5AXbIhkMV3vE= github.com/mfridman/tparse v0.18.0/go.mod h1:gEvqZTuCgEhPbYk/2lS3Kcxg1GmTxxU7kTC8DvP0i/A= +github.com/mitchellh/copystructure v1.2.0 h1:vpKXTN4ewci03Vljg/q9QvCGUDttBOGBIa15WveJJGw= +github.com/mitchellh/copystructure v1.2.0/go.mod h1:qLl+cE2AmVv+CoeAwDPye/v+N2HKCj9FbZEVFJRxO9s= +github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zxSIeXaQ= +github.com/mitchellh/reflectwalk v1.0.2/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw= github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= @@ -133,6 +145,8 @@ github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee h1:W5t00kpgFd github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= +github.com/octago/sflags v0.3.1 h1:LW65z20iAQKteEyjsnnc+/lyoCUnIoRuAocggr6RB6A= +github.com/octago/sflags v0.3.1/go.mod h1:hVUkbnYwMU9kZiZJyOAIVN56YiVMMPxgJ46kRZ19jh0= github.com/onsi/ginkgo/v2 v2.33.1 h1:62XfWJbEIjPSCSDYrsCPP/SHcmH7O0M8ne8zfqQ2iz4= github.com/onsi/ginkgo/v2 v2.33.1/go.mod h1:+aXOY+vzZ5mu2iI2HpTZUPmM//oQfsNFX6gU9kNcA44= github.com/onsi/gomega v1.44.0 h1:eAiGl3Pw5jz5GQdDff0BcxYpAX1JxW8xD7mFUuwNfZQ= diff --git a/internal/controller/eviction/controller.go b/internal/controller/eviction/controller.go index 8e80c15f..cb2cb0a9 100644 --- a/internal/controller/eviction/controller.go +++ b/internal/controller/eviction/controller.go @@ -42,11 +42,61 @@ import ( "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils" ) +// Options configures the eviction controller. +type Options struct { + Concurrency int `koanf:"eviction_concurrency" desc:"Default maximum number of VM migrations to run concurrently while draining a hypervisor. Defaults to 1 (serial)."` + TraitConcurrency map[string]int `koanf:"eviction_trait_concurrency" desc:"Per-host migration limits. Env: JSON object, e.g. {\"CUSTOM_HANA_EXCLUSIVE_HOST\":1,\"CUSTOM_FOO\":2}. Empty env clears. CLI: repeat TRAIT:N flags to merge keys, e.g. CUSTOM_FOO:2. Empty CLI is invalid. Names must be nonempty without surrounding whitespace. Limits must be >= 1. The lowest matching limit wins."` +} + +// TraitConcurrency preserves the existing name for an ordinary integer map. +type TraitConcurrency = map[string]int + +// DefaultOptions returns the eviction controller defaults. +func DefaultOptions() Options { + return Options{Concurrency: 1} +} + +// 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 != strings.TrimSpace(trait) { + return fmt.Errorf("--eviction-trait-concurrency (EVICTION_TRAIT_CONCURRENCY): trait name %q must not contain surrounding whitespace", trait) + } + if trait == "" { + return errors.New("--eviction-trait-concurrency (EVICTION_TRAIT_CONCURRENCY): trait name must not be empty") + } + } + + return nil +} + // EvictionReconciler reconciles a Eviction object type EvictionReconciler struct { client.Client Scheme *runtime.Scheme computeClient *gophercloud.ServiceClient + Options Options } const ( @@ -173,7 +223,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 +583,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..ef905faa 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 = TraitConcurrency{"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/hypervisor_controller.go b/internal/controller/hypervisor_controller.go index 76685d90..a4516398 100644 --- a/internal/controller/hypervisor_controller.go +++ b/internal/controller/hypervisor_controller.go @@ -19,6 +19,7 @@ package controller import ( "context" + "errors" "fmt" "slices" "sort" @@ -28,7 +29,7 @@ import ( "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" - "k8s.io/apimachinery/pkg/api/errors" + k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" @@ -41,10 +42,40 @@ 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/utils" ) +// Shared contains configuration options used by more than one controller. +type Shared struct { + LabelSelector string `koanf:"label_selector" desc:"Label selector to filter watched resources (namely nodes)."` + AgentNamespaces []string `koanf:"agent_namespaces" desc:"Comma-separated list of namespaces to search for agent pods (nova-compute, neutron) during offboarding."` +} + +// DefaultOptions returns the shared controller defaults. +func DefaultOptions() Shared { + return Shared{} +} + +// 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 +} + const ( labelLifecycleMode = "cobaltcore.cloud.sap/node-hypervisor-lifecycle" annotationAggregates = "nova.openstack.cloud.sap/aggregates" @@ -68,7 +99,8 @@ var transferLabels = append([]string{}, defaultTransferLabels...) type HypervisorController struct { k8sclient.Client - Scheme *runtime.Scheme + Scheme *runtime.Scheme + Options Shared } // +kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch @@ -98,7 +130,7 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request) // Check if hypervisor already exists if err := hv.Get(ctx, k8sclient.ObjectKeyFromObject(hypervisor), hypervisor); err != nil { - if !errors.IsNotFound(err) { + if !k8serrors.IsNotFound(err) { return ctrl.Result{}, fmt.Errorf("failed to get hypervisor: %w", err) } // continue with creation @@ -213,7 +245,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 Shared) error { + hv.Options = options + novaVirtLabeledPredicate, err := predicate.LabelSelectorPredicate(metav1.LabelSelector{ MatchExpressions: []metav1.LabelSelectorRequirement{ { @@ -227,8 +261,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 +354,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..c417d07a 100644 --- a/internal/controller/node_certificate_controller.go +++ b/internal/controller/node_certificate_controller.go @@ -19,6 +19,7 @@ package controller import ( "context" + "errors" "fmt" "slices" "time" @@ -36,6 +37,32 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" ) +// CertificateOptions configures the node certificate controller. +type CertificateOptions struct { + Namespace string `koanf:"certificate_namespace" desc:"The namespace for the certificates."` + IssuerName string `koanf:"certificate_issuer_name" desc:"Name of the certificate issuer."` +} + +// DefaultCertificateOptions returns the certificate controller defaults. +func DefaultCertificateOptions() CertificateOptions { + return CertificateOptions{ + Namespace: "monsoon3", + IssuerName: "nova-hypervisor-agents-ca-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 +} + const ( NodeCertificateControllerName = "certificate" ) @@ -156,9 +183,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/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..692567f0 100644 --- a/internal/global/global_test.go +++ b/internal/global/global_test.go @@ -15,65 +15,61 @@ 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 - traitMap map[string]int + traitMap eviction.TraitConcurrency traits []string want int }{ { name: "no traits falls back to default", defaultVal: 3, - traitMap: map[string]int{"CUSTOM_HANA_EXCLUSIVE_HOST": 1}, + traitMap: eviction.TraitConcurrency{"CUSTOM_HANA_EXCLUSIVE_HOST": 1}, traits: nil, want: 3, }, { name: "non-matching trait falls back to default", defaultVal: 3, - traitMap: map[string]int{"CUSTOM_HANA_EXCLUSIVE_HOST": 1}, + traitMap: eviction.TraitConcurrency{"CUSTOM_HANA_EXCLUSIVE_HOST": 1}, traits: []string{"CUSTOM_SOMETHING_ELSE"}, want: 3, }, { name: "matching trait overrides default", defaultVal: 5, - traitMap: map[string]int{"CUSTOM_HANA_EXCLUSIVE_HOST": 1}, + traitMap: eviction.TraitConcurrency{"CUSTOM_HANA_EXCLUSIVE_HOST": 1}, traits: []string{"CUSTOM_HANA_EXCLUSIVE_HOST"}, want: 1, }, { name: "lowest matching trait wins", defaultVal: 10, - traitMap: map[string]int{"CUSTOM_A": 4, "CUSTOM_B": 2, "CUSTOM_C": 7}, + traitMap: eviction.TraitConcurrency{"CUSTOM_A": 4, "CUSTOM_B": 2, "CUSTOM_C": 7}, traits: []string{"CUSTOM_A", "CUSTOM_B", "CUSTOM_C"}, want: 2, }, { name: "trait limit above default is ignored (default is the cap)", defaultVal: 2, - traitMap: map[string]int{"CUSTOM_BIG": 8}, + traitMap: eviction.TraitConcurrency{"CUSTOM_BIG": 8}, traits: []string{"CUSTOM_BIG"}, want: 2, }, { name: "result is clamped to at least 1", defaultVal: 0, - traitMap: map[string]int{}, + traitMap: eviction.TraitConcurrency{}, traits: nil, want: 1, }, @@ -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) } })