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 (only once Evicting=False)
and block service deletion in the offboarding controller until it is
True. Poll with RequeueAfter since pods are not watched.

Author: fwiesel

api/v1/hypervisor_types.go

@@ -71,6 +71,11 @@ const (
 
 	// ConditionTypeAggregatesUpdated is the type of condition for aggregates updated status of a hypervisor
 	ConditionTypeAggregatesUpdated = "AggregatesUpdated"
+
+	// ConditionTypeAgentPodsEvicted gates compute-service deletion during
+	// offboarding: a running nova-compute pod would otherwise re-register
+	// the service we are about to delete.
+	ConditionTypeAgentPodsEvicted = "AgentPodsEvicted"
 )
 
 // Condition Reasons

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

@@ -20,6 +20,14 @@ rules:
   - nodes/status
   verbs:
   - get
+- apiGroups:
+  - ""
+  resources:
+  - pods
+  verbs:
+  - get
+  - list
+  - watch
 - apiGroups:
   - apps
   resources:

go.mod

@@ -43,7 +43,7 @@ require (
 	github.com/felixge/httpsnoop v1.0.4 // indirect
 	github.com/fsnotify/fsnotify v1.9.0 // indirect
 	github.com/fxamacker/cbor/v2 v2.9.0 // indirect
-	github.com/go-logr/logr v1.4.3 // indirect
+	github.com/go-logr/logr v1.4.3
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/go-logr/zapr v1.3.0 // indirect
 	github.com/go-openapi/jsonpointer v0.22.4 // indirect

internal/controller/hypervisor_controller.go

@@ -21,13 +21,16 @@ import (
 	"context"
 	"fmt"
 	"slices"
+	"sort"
 	"strings"
 
+	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	"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"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -69,6 +72,7 @@ type HypervisorController struct {
 
 // +kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch
 // +kubebuilder:rbac:groups="",resources=nodes/status,verbs=get
+// +kubebuilder:rbac:groups="",resources=pods,verbs=get;list;watch
 // +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors/status,verbs=get;update;patch
 
@@ -123,16 +127,40 @@ 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) {
+			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)
+			}
+			meta.SetStatusCondition(&hypervisor.Status.Conditions, cond)
+			if cond.Status == metav1.ConditionFalse {
+				// No pod watch — rely on periodic requeue.
+				if err := utils.PatchHypervisorStatusWithRetry(ctx, hv.Client, hypervisor.Name, HypervisorControllerName, func(h *kvmv1.Hypervisor) {
+					meta.SetStatusCondition(&h.Status.Conditions, cond)
+				}); err != nil {
+					return ctrl.Result{}, err
+				}
+				return ctrl.Result{RequeueAfter: defaultPollTime}, nil
+			}
+		}
+
 		if !equality.Semantic.DeepEqual(hypervisor, base) {
 			// Capture values to apply - only mutate fields this controller owns
 			newInternalIP := hypervisor.Status.InternalIP
 			terminatingCondition := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeTerminating)
+			agentPodsCondition := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeAgentPodsEvicted)
 
 			return ctrl.Result{}, utils.PatchHypervisorStatusWithRetry(ctx, hv.Client, hypervisor.Name, HypervisorControllerName, func(h *kvmv1.Hypervisor) {
 				h.Status.InternalIP = newInternalIP
 				if terminatingCondition != nil {
 					meta.SetStatusCondition(&h.Status.Conditions, *terminatingCondition)
 				}
+				if agentPodsCondition != nil {
+					meta.SetStatusCondition(&h.Status.Conditions, *agentPodsCondition)
+				}
 			})
 		}
 
@@ -262,3 +290,68 @@ func transportLabels(source, destination *metav1.ObjectMeta) {
 		}
 	}
 }
+
+// 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
+		}
+		agentPods = append(agentPods, pod.Namespace+"/"+pod.Name)
+	}
+
+	if len(agentPods) == 0 {
+		return metav1.Condition{
+			Type:    kvmv1.ConditionTypeAgentPodsEvicted,
+			Status:  metav1.ConditionTrue,
+			Reason:  "NoAgentPods",
+			Message: "No agent pods are running on this node",
+		}, nil
+	}
+
+	sort.Strings(agentPods)
+	return metav1.Condition{
+		Type:    kvmv1.ConditionTypeAgentPodsEvicted,
+		Status:  metav1.ConditionFalse,
+		Reason:  "AgentPodsRunning",
+		Message: fmt.Sprintf("%d agent pod(s) still running on node: %s", len(agentPods), strings.Join(agentPods, ", ")),
+	}, nil
+}
+
+// podToleratesTaint reports whether the pod tolerates the taint indefinitely.
+// Tolerations with a finite TolerationSeconds are excluded: the pod will
+// eventually be evicted and must not be treated as safe to ignore.
+func podToleratesTaint(log logr.Logger, pod *corev1.Pod, taint *corev1.Taint) bool {
+	for i := range pod.Spec.Tolerations {
+		t := &pod.Spec.Tolerations[i]
+		if t.TolerationSeconds != nil {
+			continue
+		}
+		if t.ToleratesTaint(log, taint, false) {
+			return true
+		}
+	}
+	return false
+}

internal/controller/hypervisor_controller_test.go

@@ -18,9 +18,13 @@ limitations under the License.
 package controller
 
 import (
+	"fmt"
+	"sync/atomic"
+
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -29,6 +33,10 @@ import (
 	kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1"
 )
 
+// envtest does not actually GC pods when their namespace is deleted, so
+// each spec gets a fresh namespace via this counter to keep them isolated.
+var agentNamespaceCounter atomic.Uint64
+
 var _ = Describe("Hypervisor Controller", func() {
 	const (
 		resourceName      = "other-node"
@@ -402,4 +410,164 @@ var _ = Describe("Hypervisor Controller", func() {
 			Expect(hypervisor.Status.InternalIP).To(Equal("192.168.1.100"))
 		})
 	})
+
+	Context("AgentPodsEvicted condition", func() {
+		var agentNamespace string
+
+		BeforeEach(func(ctx SpecContext) {
+			agentNamespace = fmt.Sprintf("agent-ns-%d", agentNamespaceCounter.Add(1))
+			ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: agentNamespace}}
+			Expect(client.IgnoreAlreadyExists(k8sClient.Create(ctx, ns))).To(Succeed())
+			DeferCleanup(func(ctx SpecContext) {
+				Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, ns))).To(Succeed())
+			})
+
+			// The condition is only computed during termination.
+			_, err := hypervisorController.Reconcile(ctx, ctrl.Request{
+				NamespacedName: types.NamespacedName{Name: resource.Name},
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			hypervisor := &kvmv1.Hypervisor{}
+			Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
+			hypervisor.Spec.Maintenance = kvmv1.MaintenanceTermination
+			hypervisor.Spec.LifecycleEnabled = true
+			Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed())
+
+			// Default for these specs: VM eviction is done. Subcontexts
+			// that exercise the "not yet done" path override this.
+			Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
+			meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
+				Type:    kvmv1.ConditionTypeEvicting,
+				Status:  metav1.ConditionFalse,
+				Reason:  "Succeeded",
+				Message: "All VMs evicted",
+			})
+			Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
+		})
+
+		createPod := func(ctx SpecContext, name, namespace string, phase corev1.PodPhase, tolerations ...corev1.Toleration) *corev1.Pod {
+			pod := &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      name,
+					Namespace: namespace,
+				},
+				Spec: corev1.PodSpec{
+					NodeName: resource.Name,
+					Containers: []corev1.Container{
+						{Name: "main", Image: "registry.example.com/whatever:latest"},
+					},
+					Tolerations: tolerations,
+				},
+				Status: corev1.PodStatus{Phase: phase},
+			}
+			Expect(k8sClient.Create(ctx, pod)).To(Succeed())
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, pod)).To(Succeed())
+			pod.Status.Phase = phase
+			Expect(k8sClient.Status().Update(ctx, pod)).To(Succeed())
+			DeferCleanup(func(ctx SpecContext) {
+				Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, pod))).To(Succeed())
+			})
+			return pod
+		}
+
+		When("only pods that tolerate the offboarding taint are running", func() {
+			BeforeEach(func(ctx SpecContext) {
+				createPod(ctx, "tolerator", agentNamespace, corev1.PodRunning, corev1.Toleration{
+					Key:      taintKeyOffboarding,
+					Operator: corev1.TolerationOpExists,
+					Effect:   corev1.TaintEffectNoExecute,
+				})
+				// Phase=Succeeded must not count regardless of tolerations.
+				createPod(ctx, "old-job", agentNamespace, corev1.PodSucceeded)
+			})
+
+			It("should set AgentPodsEvicted=True without requeue", func(ctx SpecContext) {
+				result, err := hypervisorController.Reconcile(ctx, ctrl.Request{
+					NamespacedName: types.NamespacedName{Name: resource.Name},
+				})
+				Expect(err).NotTo(HaveOccurred())
+				Expect(result.RequeueAfter).To(BeZero())
+
+				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"),
+					),
+				))
+			})
+		})
+
+		When("an agent pod is running on the node and VM eviction is done", func() {
+			BeforeEach(func(ctx SpecContext) {
+				createPod(ctx, "nova-compute-xyz", agentNamespace, corev1.PodRunning)
+			})
+
+			It("should set AgentPodsEvicted=False with reason AgentPodsRunning and requeue", 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.ConditionFalse),
+						HaveField("Reason", "AgentPodsRunning"),
+						HaveField("Message", ContainSubstring("nova-compute-xyz")),
+					),
+				))
+			})
+		})
+
+		When("the only non-tolerating pod is already being deleted", func() {
+			// A finalizer keeps the API object around with DeletionTimestamp
+			// set, simulating a pod whose containers are shutting down but
+			// whose deletion is blocked on some unrelated finalizer.
+			const finalizer = "test.kvm.cloud.sap/keep-alive"
+
+			BeforeEach(func(ctx SpecContext) {
+				pod := createPod(ctx, "nova-compute-deleting", agentNamespace, corev1.PodRunning)
+				pod.Finalizers = []string{finalizer}
+				Expect(k8sClient.Update(ctx, pod)).To(Succeed())
+				Expect(k8sClient.Delete(ctx, pod)).To(Succeed())
+
+				DeferCleanup(func(ctx SpecContext) {
+					fresh := &corev1.Pod{}
+					if err := k8sClient.Get(ctx, types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}, fresh); err == nil {
+						fresh.Finalizers = nil
+						Expect(client.IgnoreNotFound(k8sClient.Update(ctx, fresh))).To(Succeed())
+					}
+				})
+
+				// Sanity: the pod must still exist with DeletionTimestamp.
+				fresh := &corev1.Pod{}
+				Expect(k8sClient.Get(ctx, types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}, fresh)).To(Succeed())
+				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{
+					NamespacedName: types.NamespacedName{Name: resource.Name},
+				})
+				Expect(err).NotTo(HaveOccurred())
+
+				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"),
+					),
+				))
+			})
+		})
+	})
 })

internal/controller/offboarding_controller.go

@@ -127,7 +127,16 @@ func (r *HypervisorOffboardingReconciler) Reconcile(ctx context.Context, req ctr
 		return r.setOffboardingCondition(ctx, hv, msg)
 	}
 
-	// Deleting and evicted, so better delete the service
+	// A still-running nova-compute agent would re-register the service we
+	// are about to delete, undoing the offboarding.
+	if !meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeAgentPodsEvicted) {
+		msg := "Waiting for agent pods (nova-compute, neutron) to be evicted from node"
+		if cond := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeAgentPodsEvicted); cond != nil && cond.Message != "" {
+			msg = "Waiting for agent pods to be evicted: " + cond.Message
+		}
+		return r.setOffboardingCondition(ctx, hv, msg)
+	}
+
 	err = services.Delete(ctx, r.computeClient, hypervisor.Service.ID).ExtractErr()
 	if err != nil && !gophercloud.ResponseCodeIs(err, http.StatusNotFound) {
 		msg := fmt.Sprintf("cannot delete service %s due to %v", hypervisor.Service.ID, err)