Gate compute-service deletion on agent pods being evicted

A running nova-compute pod would re-register the service immediately
after deletion, undoing the offboarding. Introduce AgentPodsEvicted
condition computed by HypervisorController and block service deletion
in the offboarding controller until it is True.

The condition is only evaluated once VM eviction has finished
(Evicting=False) and the offboarding taint is present on the node.
Pods are listed per-namespace via --agent-namespaces (chart default:
monsoon3; the operator refuses to start without it) in pages of 100,
with the pod cache disabled so the operator does not start a
cluster-wide pod informer. RequeueAfter is used to poll since pods
are not watched.

Author: fwiesel

charts/openstack-hypervisor-operator/templates/deployment.yaml

@@ -45,6 +45,8 @@ spec:
           value: {{ quote .Values.controllerManager.manager.env.certificateIssuerName }}
         - name: LABEL_SELECTOR
           value: {{ quote .Values.controllerManager.manager.env.labelSelector }}
+        - name: AGENT_NAMESPACES
+          value: {{ quote .Values.controllerManager.manager.env.agentNamespaces }}
         - name: KUBERNETES_CLUSTER_DOMAIN
           value: {{ quote .Values.kubernetesClusterDomain }}
         image: {{ .Values.controllerManager.manager.image.repository }}:{{ .Chart.AppVersion }}

charts/openstack-hypervisor-operator/values.yaml

@@ -7,6 +7,7 @@ controllerManager:
     - --certificate-namespace=$(CERTIFICATE_NAMESPACE)
     - --certificate-issuer-name=$(CERTIFICATE_ISSUER_NAME)
     - --label-selector=$(LABEL_SELECTOR)
+    - --agent-namespaces=$(AGENT_NAMESPACES)
     containerSecurityContext:
       allowPrivilegeEscalation: false
       capabilities:
@@ -16,6 +17,7 @@ controllerManager:
       certificateIssuerName: nova-hypervisor-agents-ca-issuer
       certificateNamespace: monsoon3
       labelSelector: ""
+      agentNamespaces: "monsoon3"
       osAuthUrl: ""
       osProjectDomainName: ""
       osProjectName: ""

cmd/main.go

@@ -25,6 +25,7 @@ import (
 	"fmt"
 	"os"
 	gruntime "runtime"
+	"strings"
 
 	"github.com/sapcc/go-api-declarations/bininfo"
 	// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
@@ -97,6 +98,10 @@ func main() {
 		"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.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.")
@@ -111,6 +116,19 @@ func main() {
 	opts.BindFlags(flag.CommandLine)
 	flag.Parse()
 
+	if agentNamespacesFlag != "" {
+		for ns := range strings.SplitSeq(agentNamespacesFlag, ",") {
+			if ns = strings.TrimSpace(ns); ns != "" {
+				global.AgentNamespaces = append(global.AgentNamespaces, ns)
+			}
+		}
+	}
+
+	if len(global.AgentNamespaces) == 0 {
+		setupLog.Error(nil, "--agent-namespaces is required")
+		os.Exit(1)
+	}
+
 	if version {
 		fmt.Printf("%s %s (%s/%s) %s\n",
 			bininfo.Component(), bininfo.VersionOr("devel"), gruntime.GOOS, gruntime.GOARCH,
@@ -226,6 +244,13 @@ func main() {
 
 		// Optionally configure the cache to listen/watch for specific labeled resources only
 		Cache: cacheOptions,
+		// Pods are listed directly (not cached) to avoid a cluster-wide pod
+		// informer — a single large namespace would cause an OOM on startup.
+		Client: client.Options{
+			Cache: &client.CacheOptions{
+				DisableFor: []client.Object{&corev1.Pod{}},
+			},
+		},
 	})
 
 	if err != nil {

internal/controller/hypervisor_controller.go

@@ -130,7 +130,8 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request)
 		// Only evaluate after VM eviction; a spurious True on a fresh node
 		// (agents not yet scheduled) would be misleading.
 		if hypervisor.Spec.Maintenance == kvmv1.MaintenanceTermination &&
-			meta.IsStatusConditionFalse(hypervisor.Status.Conditions, kvmv1.ConditionTypeEvicting) {
+			meta.IsStatusConditionFalse(hypervisor.Status.Conditions, kvmv1.ConditionTypeEvicting) &&
+			nodeHasOffboardingTaint(node) {
 			cond, err := hv.computeAgentPodsEvictedCondition(ctx, log, node.Name)
 			if err != nil {
 				return ctrl.Result{}, fmt.Errorf("failed to compute %s condition: %w", kvmv1.ConditionTypeAgentPodsEvicted, err)
@@ -291,35 +292,56 @@ func transportLabels(source, destination *metav1.ObjectMeta) {
 	}
 }
 
+// nodeHasOffboardingTaint reports whether the offboarding NoExecute taint has
+// been applied to the node. The pod list is only meaningful after that point —
+// before it, no agents have been evicted yet.
+func nodeHasOffboardingTaint(node *corev1.Node) bool {
+	for _, t := range node.Spec.Taints {
+		if t.Key == taintKeyOffboarding && t.Effect == corev1.TaintEffectNoExecute {
+			return true
+		}
+	}
+	return false
+}
+
 // computeAgentPodsEvictedCondition checks whether pods that would be evicted
 // by the offboarding taint are still running. Only call once Evicting=False.
 func (hv *HypervisorController) computeAgentPodsEvictedCondition(ctx context.Context, log logr.Logger, nodeName string) (metav1.Condition, error) {
-	pods := &corev1.PodList{}
-	if err := hv.List(ctx, pods, k8sclient.MatchingFieldsSelector{
-		Selector: fields.OneTermEqualSelector("spec.nodeName", nodeName),
-	}); err != nil {
-		return metav1.Condition{}, fmt.Errorf("failed to list pods on node %s: %w", nodeName, err)
-	}
-
 	offboardingTaint := corev1.Taint{
 		Key:    taintKeyOffboarding,
 		Effect: corev1.TaintEffectNoExecute,
 	}
 
 	var agentPods []string
-	for _, pod := range pods.Items {
-		if pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed {
-			continue
-		}
-		// Deletion in progress; don't wait for API object removal
-		// (may be blocked on unrelated finalizers).
-		if pod.DeletionTimestamp != nil {
-			continue
-		}
-		if podToleratesTaint(log, &pod, &offboardingTaint) {
-			continue
+	for _, ns := range global.AgentNamespaces {
+		var continueToken string
+		for {
+			pods := &corev1.PodList{}
+			if err := hv.List(ctx, pods,
+				k8sclient.InNamespace(ns),
+				k8sclient.MatchingFieldsSelector{
+					Selector: fields.OneTermEqualSelector("spec.nodeName", nodeName),
+				},
+				&k8sclient.ListOptions{Limit: 100, Continue: continueToken},
+			); err != nil {
+				return metav1.Condition{}, fmt.Errorf("failed to list pods on node %s in namespace %q: %w", nodeName, ns, err)
+			}
+
+			for _, pod := range pods.Items {
+				if pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed {
+					continue
+				}
+				if podToleratesTaint(log, &pod, &offboardingTaint) {
+					continue
+				}
+				agentPods = append(agentPods, pod.Namespace+"/"+pod.Name)
+			}
+
+			if pods.Continue == "" {
+				break
+			}
+			continueToken = pods.Continue
 		}
-		agentPods = append(agentPods, pod.Namespace+"/"+pod.Name)
 	}
 
 	if len(agentPods) == 0 {

internal/controller/hypervisor_controller_test.go

@@ -26,11 +26,14 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/types"
+	k8sscheme "k8s.io/client-go/kubernetes/scheme"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"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
@@ -422,6 +425,10 @@ var _ = Describe("Hypervisor Controller", func() {
 				Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, ns))).To(Succeed())
 			})
 
+			// Restrict pod listing to the test namespace.
+			global.AgentNamespaces = []string{agentNamespace}
+			DeferCleanup(func() { global.AgentNamespaces = nil })
+
 			// The condition is only computed during termination.
 			_, err := hypervisorController.Reconcile(ctx, ctrl.Request{
 				NamespacedName: types.NamespacedName{Name: resource.Name},
@@ -444,6 +451,30 @@ var _ = Describe("Hypervisor Controller", func() {
 				Message: "All VMs evicted",
 			})
 			Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
+
+			// The pod list is only issued once the offboarding taint is on the node.
+			node := &corev1.Node{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: resource.Name}, node)).To(Succeed())
+			base := node.DeepCopy()
+			node.Spec.Taints = append(node.Spec.Taints, corev1.Taint{
+				Key:    taintKeyOffboarding,
+				Effect: corev1.TaintEffectNoExecute,
+			})
+			Expect(k8sClient.Patch(ctx, node, client.MergeFrom(base))).To(Succeed())
+			DeferCleanup(func(ctx SpecContext) {
+				fresh := &corev1.Node{}
+				if err := k8sClient.Get(ctx, types.NamespacedName{Name: resource.Name}, fresh); err == nil {
+					base := fresh.DeepCopy()
+					taints := fresh.Spec.Taints[:0]
+					for _, t := range fresh.Spec.Taints {
+						if t.Key != taintKeyOffboarding {
+							taints = append(taints, t)
+						}
+					}
+					fresh.Spec.Taints = taints
+					Expect(client.IgnoreNotFound(k8sClient.Patch(ctx, fresh, client.MergeFrom(base)))).To(Succeed())
+				}
+			})
 		})
 
 		createPod := func(ctx SpecContext, name, namespace string, phase corev1.PodPhase, tolerations ...corev1.Toleration) *corev1.Pod {
@@ -552,22 +583,86 @@ var _ = Describe("Hypervisor Controller", func() {
 				Expect(fresh.DeletionTimestamp).NotTo(BeNil())
 			})
 
-			It("should set AgentPodsEvicted=True (deletion-pending pod is treated as gone)", func(ctx SpecContext) {
-				_, err := hypervisorController.Reconcile(ctx, ctrl.Request{
+			It("should set AgentPodsEvicted=False (deletion-pending pod still counts as running)", func(ctx SpecContext) {
+				result, err := hypervisorController.Reconcile(ctx, ctrl.Request{
 					NamespacedName: types.NamespacedName{Name: resource.Name},
 				})
 				Expect(err).NotTo(HaveOccurred())
+				Expect(result.RequeueAfter).To(Equal(defaultPollTime))
 
 				hypervisor := &kvmv1.Hypervisor{}
 				Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
 				Expect(hypervisor.Status.Conditions).To(ContainElement(
 					SatisfyAll(
 						HaveField("Type", kvmv1.ConditionTypeAgentPodsEvicted),
-						HaveField("Status", metav1.ConditionTrue),
-						HaveField("Reason", "NoAgentPods"),
+						HaveField("Status", metav1.ConditionFalse),
+						HaveField("Reason", "AgentPodsRunning"),
+						HaveField("Message", ContainSubstring("nova-compute-deleting")),
 					),
 				))
 			})
 		})
 	})
 })
+
+var _ = Describe("computeAgentPodsEvictedCondition field selector", func() {
+	// This test verifies that the pod list issued by computeAgentPodsEvictedCondition
+	// uses the spec.nodeName field selector so that only pods on the target node
+	// are returned, not all pods in the cluster.
+	It("should only list pods scheduled on the target node", func(ctx SpecContext) {
+		// Use a client with DisableFor pods, mirroring production config.
+		uncachedClient, err := client.New(cfg, client.Options{
+			Scheme: k8sscheme.Scheme,
+			Cache: &client.CacheOptions{
+				DisableFor: []client.Object{&corev1.Pod{}},
+			},
+		})
+		Expect(err).NotTo(HaveOccurred())
+
+		ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "field-selector-test"}}
+		Expect(client.IgnoreAlreadyExists(k8sClient.Create(ctx, ns))).To(Succeed())
+		DeferCleanup(func(ctx SpecContext) {
+			Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, ns))).To(Succeed())
+		})
+
+		// Create a pod on "target-node".
+		onTarget := &corev1.Pod{
+			ObjectMeta: metav1.ObjectMeta{Name: "on-target", Namespace: ns.Name},
+			Spec: corev1.PodSpec{
+				NodeName:   "target-node",
+				Containers: []corev1.Container{{Name: "c", Image: "registry.example.com/img:latest"}},
+			},
+		}
+		Expect(k8sClient.Create(ctx, onTarget)).To(Succeed())
+		DeferCleanup(func(ctx SpecContext) {
+			Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, onTarget))).To(Succeed())
+		})
+
+		// Create a pod on a different node — must not appear in results.
+		onOther := &corev1.Pod{
+			ObjectMeta: metav1.ObjectMeta{Name: "on-other", Namespace: ns.Name},
+			Spec: corev1.PodSpec{
+				NodeName:   "other-node",
+				Containers: []corev1.Container{{Name: "c", Image: "registry.example.com/img:latest"}},
+			},
+		}
+		Expect(k8sClient.Create(ctx, onOther)).To(Succeed())
+		DeferCleanup(func(ctx SpecContext) {
+			Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, onOther))).To(Succeed())
+		})
+
+		pods := &corev1.PodList{}
+		Expect(uncachedClient.List(ctx, pods,
+			client.MatchingFieldsSelector{
+				Selector: fields.OneTermEqualSelector("spec.nodeName", "target-node"),
+			},
+		)).To(Succeed())
+
+		names := make([]string, len(pods.Items))
+		for i, p := range pods.Items {
+			names[i] = p.Name
+		}
+		Expect(names).To(ConsistOf("on-target"),
+			"field selector spec.nodeName must filter server-side; got pods: %v", names)
+	})
+})

internal/global/global.go

@@ -20,4 +20,9 @@ package global
 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
 )