From 44d93ab9b06a0e7661f66c8b54c63153c3a87677 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Fri, 30 Oct 2026 18:45:05 +0100 Subject: [PATCH 1/4] Read operator settings from env into typed options structs Kubernetes uses the environment as the primary configuration interface. Module-owned options keep each environment name and default with its field. kelseyhightower/envconfig fills these structs at startup through separate module calls. Explicitly empty values do not restore defaults. Use the native integer map handler instead of a custom trait parser. Environment maps use comma-separated TRAIT:N entries and base-0 integer values. Keep validation of final positive limits and trait names with the eviction module. Keep the resolver's cap and minimum-one guard. Flag generation and descriptions belong to the later CLI commit. New settings remain fields with library-owned tags, not registry entries. Signed-off-by: Fabian Wiesel --- cmd/main.go | 120 +++++++----------- go.mod | 1 + go.sum | 2 + internal/config/options.go | 50 ++++++++ internal/controller/eviction/controller.go | 7 +- .../controller/eviction/controller_test.go | 25 ++-- internal/controller/eviction/options.go | 68 ++++++++++ internal/controller/hypervisor_controller.go | 15 ++- .../controller/hypervisor_controller_test.go | 5 +- .../controller/node_certificate_controller.go | 6 +- internal/controller/options.go | 38 ++++++ internal/global/global.go | 41 ------ internal/global/global_test.go | 38 +++--- 13 files changed, 252 insertions(+), 164 deletions(-) create mode 100644 internal/config/options.go create mode 100644 internal/controller/eviction/options.go create mode 100644 internal/controller/options.go diff --git a/cmd/main.go b/cmd/main.go index 41982a56..7b29fcd0 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/kelseyhightower/envconfig" "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" @@ -52,10 +51,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,6 +76,27 @@ func init() { utilruntime.Must(cmapi.AddToScheme(scheme)) } +type options struct { + Shared config.Shared + Certificate controller.CertificateOptions + Eviction eviction.Options +} + +func (o *options) loadFromEnv() error { + // Each module owns its settings; process each struct with an empty prefix + // so env names come from the envconfig tags verbatim. + if err := envconfig.Process("", &o.Shared); err != nil { + return err + } + if err := envconfig.Process("", &o.Certificate); err != nil { + return err + } + if err := envconfig.Process("", &o.Eviction); err != nil { + return err + } + return nil +} + func main() { var version bool var metricsAddr string @@ -85,8 +105,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 +118,12 @@ 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 + if err := cfg.loadFromEnv(); err != nil { + fmt.Fprintf(os.Stderr, "invalid configuration: %v\n", err) + os.Exit(1) + } opts := ctrlzap.Options{ Development: true, @@ -137,62 +144,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.Shared.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 +228,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 +241,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 +283,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 +299,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 +323,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 +331,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..d2a8ad53 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,7 @@ require ( github.com/go-logr/logr v1.4.4 github.com/gophercloud/gophercloud/v2 v2.15.0 github.com/gophercloud/utils/v2 v2.0.0-20260922174841-b4759ea0529b + github.com/kelseyhightower/envconfig v1.4.0 github.com/onsi/ginkgo/v2 v2.33.1 github.com/onsi/gomega v1.44.0 github.com/sapcc/go-api-declarations v1.25.1 diff --git a/go.sum b/go.sum index b5f8236b..42b7b3b0 100644 --- a/go.sum +++ b/go.sum @@ -113,6 +113,8 @@ github.com/joshdk/go-junit v1.0.0 h1:S86cUKIdwBHWwA6xCmFlf3RTLfVXYQfvanM5Uh+K6GE github.com/joshdk/go-junit v1.0.0/go.mod h1:TiiV0PqkaNfFXjEiyjWM3XXrhVyCa1K4Zfga6W52ung= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= +github.com/kelseyhightower/envconfig v1.4.0 h1:Im6hONhd3pLkfDFsbRgu68RDNkGF1r3dvMUtDTo2cv8= +github.com/kelseyhightower/envconfig v1.4.0/go.mod h1:cccZRl6mQpaq41TPp5QxidR+Sa3axMbJDNb//FQX6Gg= github.com/klauspost/compress v1.20.1 h1:T7kKElXUMXrUJ2E9QhQhxFtcK5rPyLdsGZvdbLMPdiQ= github.com/klauspost/compress v1.20.1/go.mod h1:LUdAzn7YLVvxLpc7y3V1m40wESHTgc1422pwwBSKYuI= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= diff --git a/internal/config/options.go b/internal/config/options.go new file mode 100644 index 00000000..33d798f9 --- /dev/null +++ b/internal/config/options.go @@ -0,0 +1,50 @@ +/* +SPDX-FileCopyrightText: Copyright 2026 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" +) + +// Shared contains configuration options used by more than one controller. +type Shared struct { + LabelSelector string `envconfig:"LABEL_SELECTOR"` + AgentNamespaces []string `envconfig:"AGENT_NAMESPACES"` +} + +// 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..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/eviction/options.go b/internal/controller/eviction/options.go new file mode 100644 index 00000000..277d78ef --- /dev/null +++ b/internal/controller/eviction/options.go @@ -0,0 +1,68 @@ +/* +SPDX-FileCopyrightText: Copyright 2026 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" +) + +// Options configures the eviction controller. +type Options struct { + Concurrency int `envconfig:"EVICTION_CONCURRENCY" default:"1"` + TraitConcurrency map[string]int `envconfig:"EVICTION_TRAIT_CONCURRENCY"` +} + +// TraitConcurrency maps Placement trait names to migration concurrency limits. +type TraitConcurrency = map[string]int + +// 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 +} 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..ce26c648 --- /dev/null +++ b/internal/controller/options.go @@ -0,0 +1,38 @@ +/* +SPDX-FileCopyrightText: Copyright 2026 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" + +// CertificateOptions configures the node certificate controller. +type CertificateOptions struct { + Namespace string `envconfig:"CERTIFICATE_NAMESPACE" default:"monsoon3"` + IssuerName string `envconfig:"CERTIFICATE_ISSUER_NAME" default:"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 +} 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) } }) From efb82206def4e47d5855c834b373e15a312ea6ed Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Fri, 30 Oct 2026 18:46:54 +0100 Subject: [PATCH 2/4] Read manager settings from the environment in the chart The deployment already supplies manager settings through environment variables. Remove the six duplicated --flag=$(ENV_VAR) arguments so the chart uses that configuration interface directly. Explain the native map format where operators configure trait limits. Environment values use comma-separated TRAIT:N entries. CLI overrides use repeated flags for individual keys. Document base-0 integers and the difference between empty environment and empty CLI values. Signed-off-by: Fabian Wiesel --- charts/openstack-hypervisor-operator/values.yaml | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/charts/openstack-hypervisor-operator/values.yaml b/charts/openstack-hypervisor-operator/values.yaml index 22f61295..18a03a69 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,13 @@ 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 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". + # Native integers use base 0: 0x10 is 16, and 010 is 8. + # To override individual keys, repeat CLI flags in manager.args: + # --eviction-trait-concurrency=CUSTOM_HANA_EXCLUSIVE_HOST:1 + # --eviction-trait-concurrency=CUSTOM_FOO:2 + # Empty CLI values are invalid. An empty environment value supplies no overrides. evictionTraitConcurrency: "" osAuthUrl: "" osProjectDomainName: "" From db42c613348d54f6bf1be01a2854dbdd5e3e9301 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Fri, 30 Oct 2026 18:48:57 +0100 Subject: [PATCH 3/4] Add CLI flags overriding the env settings Operators need to inspect and override settings from the command line. Register the aggregate options once with sflags. An anonymous shared member retains the existing unprefixed flags. Named controller members supply the certificate and eviction prefixes. Descriptions remain on the module-owned fields, beside their environment tags and defaults. Use native handlers for maps and slices so new fields need no registry or custom value methods. Trait maps use repeated TRAIT:N flags for per-key overrides of environment values. Unmentioned keys remain. Empty CLI map values are invalid. Environment maps use commas between TRAIT:N entries. Both handlers use base-0 integers. Scalar CLI values replace environment-derived defaults. Environment input must parse before CLI registration. Final domain validation stays with each module. This preserves CLI, environment, and built-in default precedence without a custom map parser or rendering method. Signed-off-by: Fabian Wiesel --- cmd/main.go | 21 +++++++++++++++------ go.mod | 1 + go.sum | 2 ++ internal/config/options.go | 4 ++-- internal/controller/eviction/options.go | 4 ++-- internal/controller/options.go | 4 ++-- 6 files changed, 24 insertions(+), 12 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 7b29fcd0..d627d7e9 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -26,6 +26,7 @@ import ( gruntime "runtime" "github.com/kelseyhightower/envconfig" + "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.) @@ -77,7 +78,7 @@ func init() { } type options struct { - Shared config.Shared + config.Shared Certificate controller.CertificateOptions Eviction eviction.Options } @@ -125,6 +126,14 @@ func main() { os.Exit(1) } + // 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, TimeEncoder: zapcore.ISO8601TimeEncoder, @@ -146,7 +155,7 @@ func main() { ctrl.SetLogger(ctrlzap.New(ctrlzap.UseFlagOptions(&opts))) - if err := cfg.Shared.Validate(); err != nil { + if err := cfg.Validate(); err != nil { setupLog.Error(err, "invalid configuration") os.Exit(1) } @@ -228,9 +237,9 @@ func main() { &policyv1.PodDisruptionBudget{}: maintenanceCacheConfig, }, } - if cfg.Shared.LabelSelector != "" { - setupLog.Info("setting up cache with label selector", "selector", cfg.Shared.LabelSelector) - selector, err := labels.Parse(cfg.Shared.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) @@ -241,7 +250,7 @@ func main() { h := sha256.New() h.Write([]byte(leaderElectionID)) // Seed it with something "unique" to the project - h.Write([]byte(cfg.Shared.LabelSelector)) + h.Write([]byte(cfg.LabelSelector)) leaderElectionID = fmt.Sprintf("%x.cloud.sap", h.Sum(nil)) } diff --git a/go.mod b/go.mod index d2a8ad53..92a57bc2 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/gophercloud/gophercloud/v2 v2.15.0 github.com/gophercloud/utils/v2 v2.0.0-20260922174841-b4759ea0529b github.com/kelseyhightower/envconfig v1.4.0 + 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 diff --git a/go.sum b/go.sum index 42b7b3b0..d2477416 100644 --- a/go.sum +++ b/go.sum @@ -135,6 +135,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/config/options.go b/internal/config/options.go index 33d798f9..6ee633f8 100644 --- a/internal/config/options.go +++ b/internal/config/options.go @@ -25,8 +25,8 @@ import ( // Shared contains configuration options used by more than one controller. type Shared struct { - LabelSelector string `envconfig:"LABEL_SELECTOR"` - AgentNamespaces []string `envconfig:"AGENT_NAMESPACES"` + LabelSelector string `envconfig:"LABEL_SELECTOR" desc:"Label selector to filter watched resources (namely nodes)."` + AgentNamespaces []string `envconfig:"AGENT_NAMESPACES" desc:"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 diff --git a/internal/controller/eviction/options.go b/internal/controller/eviction/options.go index 277d78ef..02c8ff7c 100644 --- a/internal/controller/eviction/options.go +++ b/internal/controller/eviction/options.go @@ -25,8 +25,8 @@ import ( // Options configures the eviction controller. type Options struct { - Concurrency int `envconfig:"EVICTION_CONCURRENCY" default:"1"` - TraitConcurrency map[string]int `envconfig:"EVICTION_TRAIT_CONCURRENCY"` + Concurrency int `envconfig:"EVICTION_CONCURRENCY" default:"1" desc:"Default maximum number of VM migrations to run concurrently while draining a hypervisor. Defaults to 1 (serial)."` + TraitConcurrency map[string]int `envconfig:"EVICTION_TRAIT_CONCURRENCY" desc:"Per-host migration concurrency overrides. Set EVICTION_TRAIT_CONCURRENCY to comma-separated TRAIT:N entries, e.g. CUSTOM_HANA_EXCLUSIVE_HOST:1,CUSTOM_FOO:2. Integers use base 0 (0x10 is 16, 010 is 8). Repeat --eviction-trait-concurrency=TRAIT:N for CLI overrides of individual keys. Empty CLI values are invalid. The lowest matching trait limit wins."` } // TraitConcurrency maps Placement trait names to migration concurrency limits. diff --git a/internal/controller/options.go b/internal/controller/options.go index ce26c648..62563c8f 100644 --- a/internal/controller/options.go +++ b/internal/controller/options.go @@ -21,8 +21,8 @@ import "errors" // CertificateOptions configures the node certificate controller. type CertificateOptions struct { - Namespace string `envconfig:"CERTIFICATE_NAMESPACE" default:"monsoon3"` - IssuerName string `envconfig:"CERTIFICATE_ISSUER_NAME" default:"nova-hypervisor-agents-ca-issuer"` + Namespace string `envconfig:"CERTIFICATE_NAMESPACE" default:"monsoon3" desc:"The namespace for the certificates."` + IssuerName string `envconfig:"CERTIFICATE_ISSUER_NAME" default:"nova-hypervisor-agents-ca-issuer" desc:"Name of the certificate issuer."` } // Validate checks that the certificate controller has the required settings. From 2f832331ddc4192c8ee591981e86928050617d53 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Sun, 1 Nov 2026 17:52:51 +0100 Subject: [PATCH 4/4] Colocate options with their owning controllers Settings now live next to their owning controllers, so adding a setting touches one file per controller plus the chart. Signed-off-by: Fabian Wiesel --- cmd/main.go | 3 +- internal/config/options.go | 50 -------------- internal/controller/eviction/controller.go | 44 ++++++++++++ internal/controller/eviction/options.go | 68 ------------------- internal/controller/hypervisor_controller.go | 36 ++++++++-- .../controller/node_certificate_controller.go | 19 ++++++ internal/controller/options.go | 38 ----------- 7 files changed, 95 insertions(+), 163 deletions(-) delete mode 100644 internal/config/options.go delete mode 100644 internal/controller/eviction/options.go delete mode 100644 internal/controller/options.go diff --git a/cmd/main.go b/cmd/main.go index d627d7e9..2847626e 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -52,7 +52,6 @@ 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" @@ -78,7 +77,7 @@ func init() { } type options struct { - config.Shared + controller.Shared Certificate controller.CertificateOptions Eviction eviction.Options } diff --git a/internal/config/options.go b/internal/config/options.go deleted file mode 100644 index 6ee633f8..00000000 --- a/internal/config/options.go +++ /dev/null @@ -1,50 +0,0 @@ -/* -SPDX-FileCopyrightText: Copyright 2026 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" -) - -// Shared contains configuration options used by more than one controller. -type Shared struct { - LabelSelector string `envconfig:"LABEL_SELECTOR" desc:"Label selector to filter watched resources (namely nodes)."` - AgentNamespaces []string `envconfig:"AGENT_NAMESPACES" desc:"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 9d47042a..82e406a1 100644 --- a/internal/controller/eviction/controller.go +++ b/internal/controller/eviction/controller.go @@ -42,6 +42,50 @@ import ( "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils" ) +// Options configures the eviction controller. +type Options struct { + Concurrency int `envconfig:"EVICTION_CONCURRENCY" default:"1" desc:"Default maximum number of VM migrations to run concurrently while draining a hypervisor. Defaults to 1 (serial)."` + TraitConcurrency map[string]int `envconfig:"EVICTION_TRAIT_CONCURRENCY" desc:"Per-host migration concurrency overrides. Set EVICTION_TRAIT_CONCURRENCY to comma-separated TRAIT:N entries, e.g. CUSTOM_HANA_EXCLUSIVE_HOST:1,CUSTOM_FOO:2. Integers use base 0 (0x10 is 16, 010 is 8). Repeat --eviction-trait-concurrency=TRAIT:N for CLI overrides of individual keys. Empty CLI values are invalid. The lowest matching trait limit wins."` +} + +// TraitConcurrency maps Placement trait names to migration concurrency limits. +type TraitConcurrency = map[string]int + +// 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 diff --git a/internal/controller/eviction/options.go b/internal/controller/eviction/options.go deleted file mode 100644 index 02c8ff7c..00000000 --- a/internal/controller/eviction/options.go +++ /dev/null @@ -1,68 +0,0 @@ -/* -SPDX-FileCopyrightText: Copyright 2026 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" -) - -// Options configures the eviction controller. -type Options struct { - Concurrency int `envconfig:"EVICTION_CONCURRENCY" default:"1" desc:"Default maximum number of VM migrations to run concurrently while draining a hypervisor. Defaults to 1 (serial)."` - TraitConcurrency map[string]int `envconfig:"EVICTION_TRAIT_CONCURRENCY" desc:"Per-host migration concurrency overrides. Set EVICTION_TRAIT_CONCURRENCY to comma-separated TRAIT:N entries, e.g. CUSTOM_HANA_EXCLUSIVE_HOST:1,CUSTOM_FOO:2. Integers use base 0 (0x10 is 16, 010 is 8). Repeat --eviction-trait-concurrency=TRAIT:N for CLI overrides of individual keys. Empty CLI values are invalid. The lowest matching trait limit wins."` -} - -// TraitConcurrency maps Placement trait names to migration concurrency limits. -type TraitConcurrency = map[string]int - -// 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 -} diff --git a/internal/controller/hypervisor_controller.go b/internal/controller/hypervisor_controller.go index 3c79d8f2..87aaddbf 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,35 @@ 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/config" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils" ) +// Shared contains configuration options used by more than one controller. +type Shared struct { + LabelSelector string `envconfig:"LABEL_SELECTOR" desc:"Label selector to filter watched resources (namely nodes)."` + AgentNamespaces []string `envconfig:"AGENT_NAMESPACES" desc:"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 +} + const ( labelLifecycleMode = "cobaltcore.cloud.sap/node-hypervisor-lifecycle" annotationAggregates = "nova.openstack.cloud.sap/aggregates" @@ -69,7 +95,7 @@ var transferLabels = append([]string{}, defaultTransferLabels...) type HypervisorController struct { k8sclient.Client Scheme *runtime.Scheme - Options config.Shared + Options Shared } // +kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch @@ -99,7 +125,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 @@ -214,7 +240,7 @@ func syncLabelsAndAnnotations(nodeLabels labels.Set, hypervisor *kvmv1.Hyperviso transportAggregatesAndTraits(&node.ObjectMeta, hypervisor) } -func (hv *HypervisorController) SetupWithManager(mgr ctrl.Manager, options config.Shared) error { +func (hv *HypervisorController) SetupWithManager(mgr ctrl.Manager, options Shared) error { hv.Options = options novaVirtLabeledPredicate, err := predicate.LabelSelectorPredicate(metav1.LabelSelector{ diff --git a/internal/controller/node_certificate_controller.go b/internal/controller/node_certificate_controller.go index d1d68d69..6d16baae 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,24 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" ) +// CertificateOptions configures the node certificate controller. +type CertificateOptions struct { + Namespace string `envconfig:"CERTIFICATE_NAMESPACE" default:"monsoon3" desc:"The namespace for the certificates."` + IssuerName string `envconfig:"CERTIFICATE_ISSUER_NAME" default:"nova-hypervisor-agents-ca-issuer" desc:"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 +} + const ( NodeCertificateControllerName = "certificate" ) diff --git a/internal/controller/options.go b/internal/controller/options.go deleted file mode 100644 index 62563c8f..00000000 --- a/internal/controller/options.go +++ /dev/null @@ -1,38 +0,0 @@ -/* -SPDX-FileCopyrightText: Copyright 2026 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" - -// CertificateOptions configures the node certificate controller. -type CertificateOptions struct { - Namespace string `envconfig:"CERTIFICATE_NAMESPACE" default:"monsoon3" desc:"The namespace for the certificates."` - IssuerName string `envconfig:"CERTIFICATE_ISSUER_NAME" default:"nova-hypervisor-agents-ca-issuer" desc:"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 -}