From 8f66a46c36bfdbbac7763dfea6360f5b70b1b734 Mon Sep 17 00:00:00 2001 From: huskcass <333684917+huskcass@users.noreply.github.com> Date: Fri, 23 Oct 2026 10:34:45 -0700 Subject: [PATCH 1/4] serving: stage LLMService deletion Delete the ModelRoute and wait for it to go, let routes settle, uninstall the release, then wait for its pods before dropping the finalizer. Pods past forceDeleteAfter are deleted with a zero grace period, and past maxWait the finalizer is dropped with outcome TimedOut. Every wait requeues after 5s, status is written before each action, and stages only move forward, so a restart resumes where it stopped. Orphan, foreign releases and suspend behave as before. spec.deletion holds maxWait (default 1h), routeSettle (default 60s) and forceDeleteAfter (off when unset). The annotations serving.modelsphere.dev/deletion-max-wait, route-settle and force-delete-after override them. helm.Release carries the manifest so the controller can find the release's ModelRoute, and RBAC gains delete on pods. Each test case is one YAML file under testdata/controller and testdata/helpers. --- api/serving/v1alpha1/llmservice_types.go | 204 ++++++- api/serving/v1alpha1/zz_generated.deepcopy.go | 106 ++++ .../serving.modelsphere.dev_llmservices.yaml | 154 ++++- config/rbac/role.yaml | 9 + .../llmservices.serving.modelsphere.dev.yaml | 154 ++++- dist/chart/templates/rbac/manager-role.yaml | 9 + dist/install.yaml | 163 +++++- internal/controller/serving/deletion.go | 525 +++++++++++++++++ .../serving/deletion_helpers_test.go | 247 ++++++++ internal/controller/serving/deletion_knobs.go | 125 ++++ internal/controller/serving/deletion_pods.go | 290 +++++++++ internal/controller/serving/deletion_route.go | 147 +++++ .../serving/llmservice_cases_test.go | 554 ++++++++++++++++-- .../serving/llmservice_controller.go | 40 +- internal/controller/serving/suite_test.go | 5 +- ...ase-is-kept-and-its-route-not-deleted.yaml | 40 ++ .../a-restart-resumes-the-recorded-stage.yaml | 39 ++ ...ation-added-mid-deletion-takes-effect.yaml | 43 ++ ...nnotation-route-settle-overrides-spec.yaml | 35 ++ ...tion-route-settle-zero-skips-the-wait.yaml | 31 + .../default-route-settle-is-60s.yaml | 28 + ...letes-the-modelroute-before-uninstall.yaml | 40 ++ ...orce-deletes-and-records-forcedeleted.yaml | 60 ++ .../ignores-succeeded-and-failed-pods.yaml | 25 + ...invalid-annotation-falls-back-to-spec.yaml | 34 ++ ...eases-with-timedout-while-pods-remain.yaml | 29 + ...modelroute-goes-straight-to-uninstall.yaml | 38 ++ .../orphan-skips-staged-deletion.yaml | 26 + ...uck-past-maxwait-leaves-the-finalizer.yaml | 37 ++ .../suspend-keeps-the-finalizer.yaml | 15 + ...on-a-notready-node-as-gone-past-grace.yaml | 46 ++ ...its-for-pods-then-drops-the-finalizer.yaml | 49 ++ ...e-to-be-notfound-then-for-routesettle.yaml | 53 ++ .../serving/testdata/crds/modelroute.yaml | 20 + .../annotation-overrides-spec.yaml | 15 + .../annotation-zero-is-kept.yaml | 15 + .../deletionKnobs/defaults-when-unset.yaml | 7 + .../empty-annotation-is-invalid.yaml | 12 + ...invalid-annotation-falls-back-to-spec.yaml | 17 + .../negative-annotation-falls-back.yaml | 15 + .../deletionKnobs/spec-force-delete-zero.yaml | 9 + .../helpers/deletionKnobs/spec-values.yaml | 11 + .../deletionKnobs/spec-zero-is-kept.yaml | 9 + .../modelRoutesFromManifest/empty.yaml | 7 + .../fills-an-empty-namespace.yaml | 14 + .../ignores-other-documents.yaml | 21 + .../keeps-a-gpucluster-namespace.yaml | 15 + ...jects-a-document-that-does-not-decode.yaml | 9 + .../rejects-a-route-without-a-name.yaml | 10 + .../selects-both-routing-groups.yaml | 30 + .../helpers/nextStage/empty-with-a-route.yaml | 6 + .../nextStage/empty-without-a-route.yaml | 5 + ...-deleting-releases-when-pods-are-gone.yaml | 7 + .../nextStage/force-deleting-stays.yaml | 8 + ...ods-force-due-moves-to-force-deleting.yaml | 8 + .../helpers/nextStage/pods-gone-releases.yaml | 6 + .../pods-past-the-deadline-releases.yaml | 9 + .../testdata/helpers/nextStage/pods-wait.yaml | 7 + .../helpers/nextStage/released-stays.yaml | 8 + .../route-gone-and-settled-uninstalls.yaml | 9 + .../nextStage/route-gone-starts-settling.yaml | 8 + .../route-removing-past-the-deadline.yaml | 8 + .../nextStage/route-removing-waits.yaml | 7 + .../nextStage/settling-done-uninstalls.yaml | 7 + ...settling-past-the-deadline-uninstalls.yaml | 7 + .../helpers/nextStage/settling-waits.yaml | 6 + ...stalling-does-not-return-to-the-route.yaml | 8 + .../nextStage/uninstalling-moves-to-pods.yaml | 7 + .../uninstalling-waits-for-helm.yaml | 7 + .../a-missing-node-past-grace-is-stuck.yaml | 22 + .../a-pod-that-is-not-deleting-counts.yaml | 20 + ...an-unassigned-pod-past-grace-is-stuck.yaml | 17 + ...ounts-within-grace-on-a-notready-node.yaml | 22 + .../ignores-succeeded-and-failed.yaml | 18 + ...ast-grace-on-a-notready-node-is-stuck.yaml | 22 + ...st-grace-on-a-ready-node-still-counts.yaml | 22 + .../past-grace-plus-5m-is-stuck.yaml | 22 + ...readable-nodes-use-only-the-time-rule.yaml | 19 + .../waiting-message-names-stuck-pods.yaml | 57 ++ internal/helm/helm.go | 4 + 80 files changed, 3927 insertions(+), 92 deletions(-) create mode 100644 internal/controller/serving/deletion.go create mode 100644 internal/controller/serving/deletion_helpers_test.go create mode 100644 internal/controller/serving/deletion_knobs.go create mode 100644 internal/controller/serving/deletion_pods.go create mode 100644 internal/controller/serving/deletion_route.go create mode 100644 internal/controller/serving/testdata/controller/a-foreign-release-is-kept-and-its-route-not-deleted.yaml create mode 100644 internal/controller/serving/testdata/controller/a-restart-resumes-the-recorded-stage.yaml create mode 100644 internal/controller/serving/testdata/controller/an-annotation-added-mid-deletion-takes-effect.yaml create mode 100644 internal/controller/serving/testdata/controller/annotation-route-settle-overrides-spec.yaml create mode 100644 internal/controller/serving/testdata/controller/annotation-route-settle-zero-skips-the-wait.yaml create mode 100644 internal/controller/serving/testdata/controller/default-route-settle-is-60s.yaml create mode 100644 internal/controller/serving/testdata/controller/deletes-the-modelroute-before-uninstall.yaml create mode 100644 internal/controller/serving/testdata/controller/force-delete-after-force-deletes-and-records-forcedeleted.yaml create mode 100644 internal/controller/serving/testdata/controller/ignores-succeeded-and-failed-pods.yaml create mode 100644 internal/controller/serving/testdata/controller/invalid-annotation-falls-back-to-spec.yaml create mode 100644 internal/controller/serving/testdata/controller/maxwait-timeout-releases-with-timedout-while-pods-remain.yaml create mode 100644 internal/controller/serving/testdata/controller/no-modelroute-goes-straight-to-uninstall.yaml create mode 100644 internal/controller/serving/testdata/controller/orphan-skips-staged-deletion.yaml create mode 100644 internal/controller/serving/testdata/controller/route-stuck-past-maxwait-leaves-the-finalizer.yaml create mode 100644 internal/controller/serving/testdata/controller/suspend-keeps-the-finalizer.yaml create mode 100644 internal/controller/serving/testdata/controller/treats-a-pod-stuck-on-a-notready-node-as-gone-past-grace.yaml create mode 100644 internal/controller/serving/testdata/controller/waits-for-pods-then-drops-the-finalizer.yaml create mode 100644 internal/controller/serving/testdata/controller/waits-for-the-route-to-be-notfound-then-for-routesettle.yaml create mode 100644 internal/controller/serving/testdata/crds/modelroute.yaml create mode 100644 internal/controller/serving/testdata/helpers/deletionKnobs/annotation-overrides-spec.yaml create mode 100644 internal/controller/serving/testdata/helpers/deletionKnobs/annotation-zero-is-kept.yaml create mode 100644 internal/controller/serving/testdata/helpers/deletionKnobs/defaults-when-unset.yaml create mode 100644 internal/controller/serving/testdata/helpers/deletionKnobs/empty-annotation-is-invalid.yaml create mode 100644 internal/controller/serving/testdata/helpers/deletionKnobs/invalid-annotation-falls-back-to-spec.yaml create mode 100644 internal/controller/serving/testdata/helpers/deletionKnobs/negative-annotation-falls-back.yaml create mode 100644 internal/controller/serving/testdata/helpers/deletionKnobs/spec-force-delete-zero.yaml create mode 100644 internal/controller/serving/testdata/helpers/deletionKnobs/spec-values.yaml create mode 100644 internal/controller/serving/testdata/helpers/deletionKnobs/spec-zero-is-kept.yaml create mode 100644 internal/controller/serving/testdata/helpers/modelRoutesFromManifest/empty.yaml create mode 100644 internal/controller/serving/testdata/helpers/modelRoutesFromManifest/fills-an-empty-namespace.yaml create mode 100644 internal/controller/serving/testdata/helpers/modelRoutesFromManifest/ignores-other-documents.yaml create mode 100644 internal/controller/serving/testdata/helpers/modelRoutesFromManifest/keeps-a-gpucluster-namespace.yaml create mode 100644 internal/controller/serving/testdata/helpers/modelRoutesFromManifest/rejects-a-document-that-does-not-decode.yaml create mode 100644 internal/controller/serving/testdata/helpers/modelRoutesFromManifest/rejects-a-route-without-a-name.yaml create mode 100644 internal/controller/serving/testdata/helpers/modelRoutesFromManifest/selects-both-routing-groups.yaml create mode 100644 internal/controller/serving/testdata/helpers/nextStage/empty-with-a-route.yaml create mode 100644 internal/controller/serving/testdata/helpers/nextStage/empty-without-a-route.yaml create mode 100644 internal/controller/serving/testdata/helpers/nextStage/force-deleting-releases-when-pods-are-gone.yaml create mode 100644 internal/controller/serving/testdata/helpers/nextStage/force-deleting-stays.yaml create mode 100644 internal/controller/serving/testdata/helpers/nextStage/pods-force-due-moves-to-force-deleting.yaml create mode 100644 internal/controller/serving/testdata/helpers/nextStage/pods-gone-releases.yaml create mode 100644 internal/controller/serving/testdata/helpers/nextStage/pods-past-the-deadline-releases.yaml create mode 100644 internal/controller/serving/testdata/helpers/nextStage/pods-wait.yaml create mode 100644 internal/controller/serving/testdata/helpers/nextStage/released-stays.yaml create mode 100644 internal/controller/serving/testdata/helpers/nextStage/route-gone-and-settled-uninstalls.yaml create mode 100644 internal/controller/serving/testdata/helpers/nextStage/route-gone-starts-settling.yaml create mode 100644 internal/controller/serving/testdata/helpers/nextStage/route-removing-past-the-deadline.yaml create mode 100644 internal/controller/serving/testdata/helpers/nextStage/route-removing-waits.yaml create mode 100644 internal/controller/serving/testdata/helpers/nextStage/settling-done-uninstalls.yaml create mode 100644 internal/controller/serving/testdata/helpers/nextStage/settling-past-the-deadline-uninstalls.yaml create mode 100644 internal/controller/serving/testdata/helpers/nextStage/settling-waits.yaml create mode 100644 internal/controller/serving/testdata/helpers/nextStage/uninstalling-does-not-return-to-the-route.yaml create mode 100644 internal/controller/serving/testdata/helpers/nextStage/uninstalling-moves-to-pods.yaml create mode 100644 internal/controller/serving/testdata/helpers/nextStage/uninstalling-waits-for-helm.yaml create mode 100644 internal/controller/serving/testdata/helpers/podCounts/a-missing-node-past-grace-is-stuck.yaml create mode 100644 internal/controller/serving/testdata/helpers/podCounts/a-pod-that-is-not-deleting-counts.yaml create mode 100644 internal/controller/serving/testdata/helpers/podCounts/an-unassigned-pod-past-grace-is-stuck.yaml create mode 100644 internal/controller/serving/testdata/helpers/podCounts/counts-within-grace-on-a-notready-node.yaml create mode 100644 internal/controller/serving/testdata/helpers/podCounts/ignores-succeeded-and-failed.yaml create mode 100644 internal/controller/serving/testdata/helpers/podCounts/past-grace-on-a-notready-node-is-stuck.yaml create mode 100644 internal/controller/serving/testdata/helpers/podCounts/past-grace-on-a-ready-node-still-counts.yaml create mode 100644 internal/controller/serving/testdata/helpers/podCounts/past-grace-plus-5m-is-stuck.yaml create mode 100644 internal/controller/serving/testdata/helpers/podCounts/unreadable-nodes-use-only-the-time-rule.yaml create mode 100644 internal/controller/serving/testdata/helpers/podCounts/waiting-message-names-stuck-pods.yaml diff --git a/api/serving/v1alpha1/llmservice_types.go b/api/serving/v1alpha1/llmservice_types.go index dcf4efc..b6d5e76 100644 --- a/api/serving/v1alpha1/llmservice_types.go +++ b/api/serving/v1alpha1/llmservice_types.go @@ -68,8 +68,47 @@ const ( // The only value is DeletionPolicyOrphan. AnnotationDeletionPolicy = "serving.modelsphere.dev/deletion-policy" // DeletionPolicyOrphan drops the finalizer without uninstalling the release. + // The ModelRoute and the pods are left as they are. DeletionPolicyOrphan = "Orphan" + // AnnotationDeletionMaxWait overrides spec.deletion.maxWait for one object. + // It is read on every reconcile while the object is deleting. + AnnotationDeletionMaxWait = "serving.modelsphere.dev/deletion-max-wait" + // AnnotationRouteSettle overrides spec.deletion.routeSettle. + AnnotationRouteSettle = "serving.modelsphere.dev/route-settle" + // AnnotationForceDeleteAfter overrides spec.deletion.forceDeleteAfter. + // A value of 0s force-deletes a pod as soon as it has a deletionTimestamp. + AnnotationForceDeleteAfter = "serving.modelsphere.dev/force-delete-after" + + // ConditionDeleting is true while a deleting LLMService still has its finalizer. + ConditionDeleting = "Deleting" + + // ReasonRouteRemoving means the ModelRoute from the release is being deleted. + ReasonRouteRemoving = "RouteRemoving" + // ReasonRouteSettling means the ModelRoute is gone and routes are settling. + ReasonRouteSettling = "RouteSettling" + // ReasonRouteStuck means the ModelRoute was still present when the budget elapsed. + // Its finalizer is left in place. + ReasonRouteStuck = "RouteStuck" + // ReasonNoRoute means the release manifest had no ModelRoute. + ReasonNoRoute = "NoRoute" + // ReasonUninstalled means the helm release was uninstalled. + ReasonUninstalled = "Uninstalled" + // ReasonWaitingForPods means release pods are still present. + ReasonWaitingForPods = "WaitingForPods" + // ReasonPodsForceDeleted means at least one pod was force-deleted. + ReasonPodsForceDeleted = "PodsForceDeleted" + // ReasonTimedOut means the deletion budget elapsed while pods were still present. + ReasonTimedOut = "TimedOut" + // ReasonReleased means staged deletion finished and the finalizer can drop. + ReasonReleased = "Released" + // ReasonInvalidAnnotation means a deletion annotation could not be parsed. + ReasonInvalidAnnotation = "InvalidAnnotation" + // ReasonPodForceDeleted means one pod was deleted with a zero grace period. + ReasonPodForceDeleted = "PodForceDeleted" + // ReasonKeptRelease means deletion left a helm release this object does not own. + ReasonKeptRelease = "KeptRelease" + // AnnotationForceConflicts, set to a generation, lets that generation's // apply take fields other managers own. Any other generation ignores it. AnnotationForceConflicts = "serving.modelsphere.dev/force-conflicts" @@ -99,6 +138,8 @@ const ( PhaseApplied Phase = "Applied" // PhaseFailed means the last reconcile stopped on an error. PhaseFailed Phase = "Failed" + // PhaseDeleting means the object is deleting and staged teardown is in progress. + PhaseDeleting Phase = "Deleting" ) // LocalObjectReference names an object in the LLMService's namespace. @@ -195,10 +236,84 @@ type LLMServiceSpec struct { Model *ModelSpec `json:"model,omitempty"` // suspend stops reconciliation. The status keeps the last outcome. + // Suspend wins over deletion: a suspended object that is deleting is left + // as it is, including its finalizer. // +optional Suspend bool `json:"suspend,omitempty"` + + // deletion tunes staged teardown. Annotations on the object override these + // fields, and an unset field uses the built-in default. An explicit 0s is + // a real zero (routeSettle 0s skips the settle wait). + // +optional + Deletion *DeletionSpec `json:"deletion,omitempty"` +} + +// DeletionSpec tunes staged deletion of an LLMService. +type DeletionSpec struct { + // maxWait is the budget for holding the finalizer, measured from + // status.deletion.startedAt. Past it, remaining waits are skipped and the + // finalizer is dropped. The annotation + // serving.modelsphere.dev/deletion-max-wait overrides this on every pass. + // +kubebuilder:default="1h" + // +kubebuilder:validation:Format=duration + // +kubebuilder:validation:XValidation:rule="duration(self) >= duration('0s')",message="maxWait must be a non-negative duration" + // +optional + MaxWait *metav1.Duration `json:"maxWait,omitempty"` + + // routeSettle is how long to wait after the ModelRoute is gone so the + // gateway can drop it before the Services disappear. 0, via the + // annotation serving.modelsphere.dev/route-settle, skips the wait. The + // built-in default is 60s. + // +kubebuilder:default="60s" + // +kubebuilder:validation:Format=duration + // +kubebuilder:validation:XValidation:rule="duration(self) >= duration('0s')",message="routeSettle must be a non-negative duration" + // +optional + RouteSettle *metav1.Duration `json:"routeSettle,omitempty"` + + // forceDeleteAfter force-deletes pods that have been terminating for this + // long, with gracePeriodSeconds 0. The wait starts when deletion was + // requested. Unset leaves the pods to the kubelet. The annotation + // serving.modelsphere.dev/force-delete-after overrides this. + // +kubebuilder:validation:Format=duration + // +kubebuilder:validation:XValidation:rule="duration(self) >= duration('0s')",message="forceDeleteAfter must be a non-negative duration" + // +optional + ForceDeleteAfter *metav1.Duration `json:"forceDeleteAfter,omitempty"` } +// DeletionStage is the monotonic stage of a staged deletion. +type DeletionStage string + +const ( + // DeletionStageRouteRemoving deletes the ModelRoute and waits for it to go. + DeletionStageRouteRemoving DeletionStage = "RouteRemoving" + // DeletionStageRouteSettling waits after the ModelRoute is NotFound. + DeletionStageRouteSettling DeletionStage = "RouteSettling" + // DeletionStageUninstalling uninstalls the helm release. + DeletionStageUninstalling DeletionStage = "Uninstalling" + // DeletionStagePodsTerminating waits for the release's pods to go. + DeletionStagePodsTerminating DeletionStage = "PodsTerminating" + // DeletionStageForceDeleting force-deletes pods past forceDeleteAfter. + DeletionStageForceDeleting DeletionStage = "ForceDeleting" + // DeletionStageReleased means the finalizer may be dropped. + DeletionStageReleased DeletionStage = "Released" +) + +// DeletionOutcome is how staged deletion finished. +type DeletionOutcome string + +const ( + // DeletionOutcomeClean means every stage finished inside the budget. + DeletionOutcomeClean DeletionOutcome = "Clean" + // DeletionOutcomeNoRoute means the release manifest had no ModelRoute. + DeletionOutcomeNoRoute DeletionOutcome = "NoRoute" + // DeletionOutcomeRouteStuck means the ModelRoute was still present at the deadline. + DeletionOutcomeRouteStuck DeletionOutcome = "RouteStuck" + // DeletionOutcomePodsForceDeleted means at least one pod was force-deleted. + DeletionOutcomePodsForceDeleted DeletionOutcome = "PodsForceDeleted" + // DeletionOutcomeTimedOut means the budget elapsed while pods were still present. + DeletionOutcomeTimedOut DeletionOutcome = "TimedOut" +) + // ChartStatus is the chart spec.chart.version resolved to. type ChartStatus struct { // name is the chart name that was resolved. @@ -270,8 +385,8 @@ type LLMServiceStatus struct { ObservedGeneration int64 `json:"observedGeneration,omitempty"` // phase is the current reconciliation phase: Pending, Applying, Applied, - // or Failed. - // +kubebuilder:validation:Enum=Pending;Applying;Applied;Failed + // Failed, or Deleting. + // +kubebuilder:validation:Enum=Pending;Applying;Applied;Failed;Deleting // +optional Phase Phase `json:"phase,omitempty"` @@ -311,6 +426,91 @@ type LLMServiceStatus struct { // recent DefaultHistoryLimit (10) entries. // +optional History []HistoryEntry `json:"history,omitempty"` + + // deletion is the staged teardown observed while the object is deleting. + // +optional + Deletion *DeletionStatus `json:"deletion,omitempty"` +} + +// DeletionStatus is the observed staged teardown of an LLMService. +type DeletionStatus struct { + // stage is the current teardown stage. It only moves forward. + // +kubebuilder:validation:Enum=RouteRemoving;RouteSettling;Uninstalling;PodsTerminating;ForceDeleting;Released + // +optional + Stage DeletionStage `json:"stage,omitempty"` + + // outcome is how teardown finished, or the non-clean result already known. + // +kubebuilder:validation:Enum=Clean;NoRoute;RouteStuck;PodsForceDeleted;TimedOut + // +optional + Outcome DeletionOutcome `json:"outcome,omitempty"` + + // startedAt is when staged deletion began. + // +optional + StartedAt *metav1.Time `json:"startedAt,omitempty"` + + // deadline is startedAt plus the effective maxWait. It is recomputed when + // the annotation or the spec changes. + // +optional + Deadline *metav1.Time `json:"deadline,omitempty"` + + // routeRemovedAt is when the ModelRoute was first observed NotFound. + // +optional + RouteRemovedAt *metav1.Time `json:"routeRemovedAt,omitempty"` + + // uninstalledAt is when the helm uninstall succeeded. + // +optional + UninstalledAt *metav1.Time `json:"uninstalledAt,omitempty"` + + // podsGoneAt is when no release pods were left to wait for. + // +optional + PodsGoneAt *metav1.Time `json:"podsGoneAt,omitempty"` + + // podsRemaining is how many release pods still block the finalizer. + // +optional + PodsRemaining int32 `json:"podsRemaining,omitempty"` + + // pods are the release pods still in the API, ignoring Succeeded and Failed. + // The list is capped at 20. + // +kubebuilder:validation:MaxItems=20 + // +optional + Pods []DeletionPod `json:"pods,omitempty"` + + // forceDeleted are pod names deleted with gracePeriodSeconds 0. Capped at 20. + // +kubebuilder:validation:MaxItems=20 + // +optional + ForceDeleted []string `json:"forceDeleted,omitempty"` + + // message is a human-readable line, for example + // "waiting for 3 pods (2 stuck on NotReady node gpu-7)". + // +optional + Message string `json:"message,omitempty"` +} + +// DeletionPod is one release pod observed during teardown. +type DeletionPod struct { + // name of the pod. + Name string `json:"name"` + + // phase is the pod phase. + // +optional + Phase string `json:"phase,omitempty"` + + // node is the node the pod is assigned to. + // +optional + Node string `json:"node,omitempty"` + + // deletionTimestamp is the pod's metadata.deletionTimestamp. + // +optional + DeletionTimestamp *metav1.Time `json:"deletionTimestamp,omitempty"` + + // grace is the pod's terminationGracePeriodSeconds. 30 when the pod does not set one. + // +optional + Grace int64 `json:"grace,omitempty"` + + // stuck is true when the pod is past its grace on a NotReady or missing + // node, or past grace plus 5m, and no longer blocks the finalizer. + // +optional + Stuck bool `json:"stuck,omitempty"` } // +kubebuilder:object:root=true diff --git a/api/serving/v1alpha1/zz_generated.deepcopy.go b/api/serving/v1alpha1/zz_generated.deepcopy.go index 0eb6875..6b9924e 100644 --- a/api/serving/v1alpha1/zz_generated.deepcopy.go +++ b/api/serving/v1alpha1/zz_generated.deepcopy.go @@ -61,6 +61,102 @@ func (in *ChartStatus) DeepCopy() *ChartStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DeletionPod) DeepCopyInto(out *DeletionPod) { + *out = *in + if in.DeletionTimestamp != nil { + in, out := &in.DeletionTimestamp, &out.DeletionTimestamp + *out = (*in).DeepCopy() + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DeletionPod. +func (in *DeletionPod) DeepCopy() *DeletionPod { + if in == nil { + return nil + } + out := new(DeletionPod) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DeletionSpec) DeepCopyInto(out *DeletionSpec) { + *out = *in + if in.MaxWait != nil { + in, out := &in.MaxWait, &out.MaxWait + *out = new(metav1.Duration) + **out = **in + } + if in.RouteSettle != nil { + in, out := &in.RouteSettle, &out.RouteSettle + *out = new(metav1.Duration) + **out = **in + } + if in.ForceDeleteAfter != nil { + in, out := &in.ForceDeleteAfter, &out.ForceDeleteAfter + *out = new(metav1.Duration) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DeletionSpec. +func (in *DeletionSpec) DeepCopy() *DeletionSpec { + if in == nil { + return nil + } + out := new(DeletionSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DeletionStatus) DeepCopyInto(out *DeletionStatus) { + *out = *in + if in.StartedAt != nil { + in, out := &in.StartedAt, &out.StartedAt + *out = (*in).DeepCopy() + } + if in.Deadline != nil { + in, out := &in.Deadline, &out.Deadline + *out = (*in).DeepCopy() + } + if in.RouteRemovedAt != nil { + in, out := &in.RouteRemovedAt, &out.RouteRemovedAt + *out = (*in).DeepCopy() + } + if in.UninstalledAt != nil { + in, out := &in.UninstalledAt, &out.UninstalledAt + *out = (*in).DeepCopy() + } + if in.PodsGoneAt != nil { + in, out := &in.PodsGoneAt, &out.PodsGoneAt + *out = (*in).DeepCopy() + } + if in.Pods != nil { + in, out := &in.Pods, &out.Pods + *out = make([]DeletionPod, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.ForceDeleted != nil { + in, out := &in.ForceDeleted, &out.ForceDeleted + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DeletionStatus. +func (in *DeletionStatus) DeepCopy() *DeletionStatus { + if in == nil { + return nil + } + out := new(DeletionStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HelmStatus) DeepCopyInto(out *HelmStatus) { *out = *in @@ -174,6 +270,11 @@ func (in *LLMServiceSpec) DeepCopyInto(out *LLMServiceSpec) { *out = new(ModelSpec) **out = **in } + if in.Deletion != nil { + in, out := &in.Deletion, &out.Deletion + *out = new(DeletionSpec) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LLMServiceSpec. @@ -205,6 +306,11 @@ func (in *LLMServiceStatus) DeepCopyInto(out *LLMServiceStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Deletion != nil { + in, out := &in.Deletion, &out.Deletion + *out = new(DeletionStatus) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LLMServiceStatus. diff --git a/config/crd/bases/serving.modelsphere.dev_llmservices.yaml b/config/crd/bases/serving.modelsphere.dev_llmservices.yaml index 95cf1d5..f3a2cc3 100644 --- a/config/crd/bases/serving.modelsphere.dev_llmservices.yaml +++ b/config/crd/bases/serving.modelsphere.dev_llmservices.yaml @@ -107,6 +107,48 @@ spec: - repo - version type: object + deletion: + description: |- + deletion tunes staged teardown. Annotations on the object override these + fields, and an unset field uses the built-in default. An explicit 0s is + a real zero (routeSettle 0s skips the settle wait). + properties: + forceDeleteAfter: + description: |- + forceDeleteAfter force-deletes pods that have been terminating for this + long, with gracePeriodSeconds 0. The wait starts when deletion was + requested. Unset leaves the pods to the kubelet. The annotation + serving.modelsphere.dev/force-delete-after overrides this. + format: duration + type: string + x-kubernetes-validations: + - message: forceDeleteAfter must be a non-negative duration + rule: duration(self) >= duration('0s') + maxWait: + default: 1h + description: |- + maxWait is the budget for holding the finalizer, measured from + status.deletion.startedAt. Past it, remaining waits are skipped and the + finalizer is dropped. The annotation + serving.modelsphere.dev/deletion-max-wait overrides this on every pass. + format: duration + type: string + x-kubernetes-validations: + - message: maxWait must be a non-negative duration + rule: duration(self) >= duration('0s') + routeSettle: + default: 60s + description: |- + routeSettle is how long to wait after the ModelRoute is gone so the + gateway can drop it before the Services disappear. 0, via the + annotation serving.modelsphere.dev/route-settle, skips the wait. The + built-in default is 60s. + format: duration + type: string + x-kubernetes-validations: + - message: routeSettle must be a non-negative duration + rule: duration(self) >= duration('0s') + type: object layers: description: |- layers are helm values documents merged in order, last writer wins. @@ -159,8 +201,10 @@ spec: type: string type: object suspend: - description: suspend stops reconciliation. The status keeps the last - outcome. + description: |- + suspend stops reconciliation. The status keeps the last outcome. + Suspend wins over deletion: a suspended object that is deleting is left + as it is, including its finalizer. type: boolean required: - chart @@ -252,6 +296,109 @@ spec: x-kubernetes-list-map-keys: - type x-kubernetes-list-type: map + deletion: + description: deletion is the staged teardown observed while the object + is deleting. + properties: + deadline: + description: |- + deadline is startedAt plus the effective maxWait. It is recomputed when + the annotation or the spec changes. + format: date-time + type: string + forceDeleted: + description: forceDeleted are pod names deleted with gracePeriodSeconds + 0. Capped at 20. + items: + type: string + maxItems: 20 + type: array + message: + description: |- + message is a human-readable line, for example + "waiting for 3 pods (2 stuck on NotReady node gpu-7)". + type: string + outcome: + description: outcome is how teardown finished, or the non-clean + result already known. + enum: + - Clean + - NoRoute + - RouteStuck + - PodsForceDeleted + - TimedOut + type: string + pods: + description: |- + pods are the release pods still in the API, ignoring Succeeded and Failed. + The list is capped at 20. + items: + description: DeletionPod is one release pod observed during + teardown. + properties: + deletionTimestamp: + description: deletionTimestamp is the pod's metadata.deletionTimestamp. + format: date-time + type: string + grace: + description: grace is the pod's terminationGracePeriodSeconds. + 30 when the pod does not set one. + format: int64 + type: integer + name: + description: name of the pod. + type: string + node: + description: node is the node the pod is assigned to. + type: string + phase: + description: phase is the pod phase. + type: string + stuck: + description: |- + stuck is true when the pod is past its grace on a NotReady or missing + node, or past grace plus 5m, and no longer blocks the finalizer. + type: boolean + required: + - name + type: object + maxItems: 20 + type: array + podsGoneAt: + description: podsGoneAt is when no release pods were left to wait + for. + format: date-time + type: string + podsRemaining: + description: podsRemaining is how many release pods still block + the finalizer. + format: int32 + type: integer + routeRemovedAt: + description: routeRemovedAt is when the ModelRoute was first observed + NotFound. + format: date-time + type: string + stage: + description: stage is the current teardown stage. It only moves + forward. + enum: + - RouteRemoving + - RouteSettling + - Uninstalling + - PodsTerminating + - ForceDeleting + - Released + type: string + startedAt: + description: startedAt is when staged deletion began. + format: date-time + type: string + uninstalledAt: + description: uninstalledAt is when the helm uninstall succeeded. + format: date-time + type: string + type: object helm: description: helm is the release revision and status last observed. properties: @@ -328,12 +475,13 @@ spec: phase: description: |- phase is the current reconciliation phase: Pending, Applying, Applied, - or Failed. + Failed, or Deleting. enum: - Pending - Applying - Applied - Failed + - Deleting type: string releaseUID: description: |- diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 6d722ab..e260171 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -27,11 +27,20 @@ rules: - create - get - patch +- apiGroups: + - "" + resources: + - nodes + verbs: + - get + - list + - watch - apiGroups: - "" resources: - pods verbs: + - delete - get - list - patch diff --git a/dist/chart/templates/crd/llmservices.serving.modelsphere.dev.yaml b/dist/chart/templates/crd/llmservices.serving.modelsphere.dev.yaml index 205fd6d..b24908a 100644 --- a/dist/chart/templates/crd/llmservices.serving.modelsphere.dev.yaml +++ b/dist/chart/templates/crd/llmservices.serving.modelsphere.dev.yaml @@ -110,6 +110,48 @@ spec: - repo - version type: object + deletion: + description: |- + deletion tunes staged teardown. Annotations on the object override these + fields, and an unset field uses the built-in default. An explicit 0s is + a real zero (routeSettle 0s skips the settle wait). + properties: + forceDeleteAfter: + description: |- + forceDeleteAfter force-deletes pods that have been terminating for this + long, with gracePeriodSeconds 0. The wait starts when deletion was + requested. Unset leaves the pods to the kubelet. The annotation + serving.modelsphere.dev/force-delete-after overrides this. + format: duration + type: string + x-kubernetes-validations: + - message: forceDeleteAfter must be a non-negative duration + rule: duration(self) >= duration('0s') + maxWait: + default: 1h + description: |- + maxWait is the budget for holding the finalizer, measured from + status.deletion.startedAt. Past it, remaining waits are skipped and the + finalizer is dropped. The annotation + serving.modelsphere.dev/deletion-max-wait overrides this on every pass. + format: duration + type: string + x-kubernetes-validations: + - message: maxWait must be a non-negative duration + rule: duration(self) >= duration('0s') + routeSettle: + default: 60s + description: |- + routeSettle is how long to wait after the ModelRoute is gone so the + gateway can drop it before the Services disappear. 0, via the + annotation serving.modelsphere.dev/route-settle, skips the wait. The + built-in default is 60s. + format: duration + type: string + x-kubernetes-validations: + - message: routeSettle must be a non-negative duration + rule: duration(self) >= duration('0s') + type: object layers: description: |- layers are helm values documents merged in order, last writer wins. @@ -162,8 +204,10 @@ spec: type: string type: object suspend: - description: suspend stops reconciliation. The status keeps the last - outcome. + description: |- + suspend stops reconciliation. The status keeps the last outcome. + Suspend wins over deletion: a suspended object that is deleting is left + as it is, including its finalizer. type: boolean required: - chart @@ -255,6 +299,109 @@ spec: x-kubernetes-list-map-keys: - type x-kubernetes-list-type: map + deletion: + description: deletion is the staged teardown observed while the object + is deleting. + properties: + deadline: + description: |- + deadline is startedAt plus the effective maxWait. It is recomputed when + the annotation or the spec changes. + format: date-time + type: string + forceDeleted: + description: forceDeleted are pod names deleted with gracePeriodSeconds + 0. Capped at 20. + items: + type: string + maxItems: 20 + type: array + message: + description: |- + message is a human-readable line, for example + "waiting for 3 pods (2 stuck on NotReady node gpu-7)". + type: string + outcome: + description: outcome is how teardown finished, or the non-clean + result already known. + enum: + - Clean + - NoRoute + - RouteStuck + - PodsForceDeleted + - TimedOut + type: string + pods: + description: |- + pods are the release pods still in the API, ignoring Succeeded and Failed. + The list is capped at 20. + items: + description: DeletionPod is one release pod observed during + teardown. + properties: + deletionTimestamp: + description: deletionTimestamp is the pod's metadata.deletionTimestamp. + format: date-time + type: string + grace: + description: grace is the pod's terminationGracePeriodSeconds. + 30 when the pod does not set one. + format: int64 + type: integer + name: + description: name of the pod. + type: string + node: + description: node is the node the pod is assigned to. + type: string + phase: + description: phase is the pod phase. + type: string + stuck: + description: |- + stuck is true when the pod is past its grace on a NotReady or missing + node, or past grace plus 5m, and no longer blocks the finalizer. + type: boolean + required: + - name + type: object + maxItems: 20 + type: array + podsGoneAt: + description: podsGoneAt is when no release pods were left to wait + for. + format: date-time + type: string + podsRemaining: + description: podsRemaining is how many release pods still block + the finalizer. + format: int32 + type: integer + routeRemovedAt: + description: routeRemovedAt is when the ModelRoute was first observed + NotFound. + format: date-time + type: string + stage: + description: stage is the current teardown stage. It only moves + forward. + enum: + - RouteRemoving + - RouteSettling + - Uninstalling + - PodsTerminating + - ForceDeleting + - Released + type: string + startedAt: + description: startedAt is when staged deletion began. + format: date-time + type: string + uninstalledAt: + description: uninstalledAt is when the helm uninstall succeeded. + format: date-time + type: string + type: object helm: description: helm is the release revision and status last observed. properties: @@ -331,12 +478,13 @@ spec: phase: description: |- phase is the current reconciliation phase: Pending, Applying, Applied, - or Failed. + Failed, or Deleting. enum: - Pending - Applying - Applied - Failed + - Deleting type: string releaseUID: description: |- diff --git a/dist/chart/templates/rbac/manager-role.yaml b/dist/chart/templates/rbac/manager-role.yaml index 041a979..51365ae 100644 --- a/dist/chart/templates/rbac/manager-role.yaml +++ b/dist/chart/templates/rbac/manager-role.yaml @@ -33,11 +33,20 @@ rules: - create - get - patch +- apiGroups: + - "" + resources: + - nodes + verbs: + - get + - list + - watch - apiGroups: - "" resources: - pods verbs: + - delete - get - list - patch diff --git a/dist/install.yaml b/dist/install.yaml index dec731e..a3dd946 100644 --- a/dist/install.yaml +++ b/dist/install.yaml @@ -516,6 +516,48 @@ spec: - repo - version type: object + deletion: + description: |- + deletion tunes staged teardown. Annotations on the object override these + fields, and an unset field uses the built-in default. An explicit 0s is + a real zero (routeSettle 0s skips the settle wait). + properties: + forceDeleteAfter: + description: |- + forceDeleteAfter force-deletes pods that have been terminating for this + long, with gracePeriodSeconds 0. The wait starts when deletion was + requested. Unset leaves the pods to the kubelet. The annotation + serving.modelsphere.dev/force-delete-after overrides this. + format: duration + type: string + x-kubernetes-validations: + - message: forceDeleteAfter must be a non-negative duration + rule: duration(self) >= duration('0s') + maxWait: + default: 1h + description: |- + maxWait is the budget for holding the finalizer, measured from + status.deletion.startedAt. Past it, remaining waits are skipped and the + finalizer is dropped. The annotation + serving.modelsphere.dev/deletion-max-wait overrides this on every pass. + format: duration + type: string + x-kubernetes-validations: + - message: maxWait must be a non-negative duration + rule: duration(self) >= duration('0s') + routeSettle: + default: 60s + description: |- + routeSettle is how long to wait after the ModelRoute is gone so the + gateway can drop it before the Services disappear. 0, via the + annotation serving.modelsphere.dev/route-settle, skips the wait. The + built-in default is 60s. + format: duration + type: string + x-kubernetes-validations: + - message: routeSettle must be a non-negative duration + rule: duration(self) >= duration('0s') + type: object layers: description: |- layers are helm values documents merged in order, last writer wins. @@ -568,8 +610,10 @@ spec: type: string type: object suspend: - description: suspend stops reconciliation. The status keeps the last - outcome. + description: |- + suspend stops reconciliation. The status keeps the last outcome. + Suspend wins over deletion: a suspended object that is deleting is left + as it is, including its finalizer. type: boolean required: - chart @@ -661,6 +705,109 @@ spec: x-kubernetes-list-map-keys: - type x-kubernetes-list-type: map + deletion: + description: deletion is the staged teardown observed while the object + is deleting. + properties: + deadline: + description: |- + deadline is startedAt plus the effective maxWait. It is recomputed when + the annotation or the spec changes. + format: date-time + type: string + forceDeleted: + description: forceDeleted are pod names deleted with gracePeriodSeconds + 0. Capped at 20. + items: + type: string + maxItems: 20 + type: array + message: + description: |- + message is a human-readable line, for example + "waiting for 3 pods (2 stuck on NotReady node gpu-7)". + type: string + outcome: + description: outcome is how teardown finished, or the non-clean + result already known. + enum: + - Clean + - NoRoute + - RouteStuck + - PodsForceDeleted + - TimedOut + type: string + pods: + description: |- + pods are the release pods still in the API, ignoring Succeeded and Failed. + The list is capped at 20. + items: + description: DeletionPod is one release pod observed during + teardown. + properties: + deletionTimestamp: + description: deletionTimestamp is the pod's metadata.deletionTimestamp. + format: date-time + type: string + grace: + description: grace is the pod's terminationGracePeriodSeconds. + 30 when the pod does not set one. + format: int64 + type: integer + name: + description: name of the pod. + type: string + node: + description: node is the node the pod is assigned to. + type: string + phase: + description: phase is the pod phase. + type: string + stuck: + description: |- + stuck is true when the pod is past its grace on a NotReady or missing + node, or past grace plus 5m, and no longer blocks the finalizer. + type: boolean + required: + - name + type: object + maxItems: 20 + type: array + podsGoneAt: + description: podsGoneAt is when no release pods were left to wait + for. + format: date-time + type: string + podsRemaining: + description: podsRemaining is how many release pods still block + the finalizer. + format: int32 + type: integer + routeRemovedAt: + description: routeRemovedAt is when the ModelRoute was first observed + NotFound. + format: date-time + type: string + stage: + description: stage is the current teardown stage. It only moves + forward. + enum: + - RouteRemoving + - RouteSettling + - Uninstalling + - PodsTerminating + - ForceDeleting + - Released + type: string + startedAt: + description: startedAt is when staged deletion began. + format: date-time + type: string + uninstalledAt: + description: uninstalledAt is when the helm uninstall succeeded. + format: date-time + type: string + type: object helm: description: helm is the release revision and status last observed. properties: @@ -737,12 +884,13 @@ spec: phase: description: |- phase is the current reconciliation phase: Pending, Applying, Applied, - or Failed. + Failed, or Deleting. enum: - Pending - Applying - Applied - Failed + - Deleting type: string releaseUID: description: |- @@ -910,11 +1058,20 @@ rules: - create - get - patch +- apiGroups: + - "" + resources: + - nodes + verbs: + - get + - list + - watch - apiGroups: - "" resources: - pods verbs: + - delete - get - list - patch diff --git a/internal/controller/serving/deletion.go b/internal/controller/serving/deletion.go new file mode 100644 index 0000000..a15e9b7 --- /dev/null +++ b/internal/controller/serving/deletion.go @@ -0,0 +1,525 @@ +/* +Copyright 2026. + +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 serving + +import ( + "context" + "time" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + logf "sigs.k8s.io/controller-runtime/pkg/log" + + servingv1alpha1 "github.com/modelsphere/llm-operator/api/serving/v1alpha1" + "github.com/modelsphere/llm-operator/internal/helm" +) + +// stageInput is the observation nextStage turns into one forward move. +type stageInput struct { + Stage servingv1alpha1.DeletionStage + HasRoute bool + RouteGone bool + SettleDone bool + Uninstalled bool + PodsRemaining int + PastDeadline bool + ForceDue bool +} + +// nextStage is the monotonic teardown transition. It never moves backward. +func nextStage(in stageInput) servingv1alpha1.DeletionStage { + want := in.Stage + switch in.Stage { + case "": + want = servingv1alpha1.DeletionStageUninstalling + if in.HasRoute { + want = servingv1alpha1.DeletionStageRouteRemoving + } + case servingv1alpha1.DeletionStageRouteRemoving: + want = routeRemovingNext(in) + case servingv1alpha1.DeletionStageRouteSettling: + want = servingv1alpha1.DeletionStageRouteSettling + if in.SettleDone || in.PastDeadline { + want = servingv1alpha1.DeletionStageUninstalling + } + case servingv1alpha1.DeletionStageUninstalling: + want = servingv1alpha1.DeletionStageUninstalling + if in.Uninstalled { + want = servingv1alpha1.DeletionStagePodsTerminating + } + case servingv1alpha1.DeletionStagePodsTerminating, servingv1alpha1.DeletionStageForceDeleting: + want = podsNext(in) + case servingv1alpha1.DeletionStageReleased: + want = servingv1alpha1.DeletionStageReleased + } + if stageRank(want) < stageRank(in.Stage) { + return in.Stage + } + return want +} + +func routeRemovingNext(in stageInput) servingv1alpha1.DeletionStage { + if !in.RouteGone { + if in.PastDeadline { + return servingv1alpha1.DeletionStageUninstalling + } + return servingv1alpha1.DeletionStageRouteRemoving + } + if !in.SettleDone { + return servingv1alpha1.DeletionStageRouteSettling + } + return servingv1alpha1.DeletionStageUninstalling +} + +func podsNext(in stageInput) servingv1alpha1.DeletionStage { + if in.PodsRemaining == 0 || in.PastDeadline { + return servingv1alpha1.DeletionStageReleased + } + if in.ForceDue { + return servingv1alpha1.DeletionStageForceDeleting + } + return in.Stage +} + +func stageRank(stage servingv1alpha1.DeletionStage) int { + switch stage { + case servingv1alpha1.DeletionStageRouteRemoving: + return 1 + case servingv1alpha1.DeletionStageRouteSettling: + return 2 + case servingv1alpha1.DeletionStageUninstalling: + return 3 + case servingv1alpha1.DeletionStagePodsTerminating: + return 4 + case servingv1alpha1.DeletionStageForceDeleting: + return 5 + case servingv1alpha1.DeletionStageReleased: + return 6 + default: + return 0 + } +} + +func (r *LLMServiceReconciler) now() time.Time { + if r.Now != nil { + return r.Now() + } + return time.Now() +} + +func (r *LLMServiceReconciler) reconcileStagedDelete(ctx context.Context, obj *servingv1alpha1.LLMService, rel *helm.Release) (ctrl.Result, error) { + now := r.now() + knobs := deletionKnobsFrom(obj) + r.warnInvalidKnobs(obj, knobs) + del := cloneDeletion(obj) + stampDeletionClock(del, now, knobs.maxWait) + past := !now.Before(del.Deadline.Time) + + if del.Stage == servingv1alpha1.DeletionStageReleased { + return r.finishReleased(ctx, obj, del) + } + if del.Stage == "" || del.Stage == servingv1alpha1.DeletionStageRouteRemoving || del.Stage == servingv1alpha1.DeletionStageRouteSettling { + if rel == nil { + // The release secret is already gone, so its manifest cannot name a route. + del.Stage = servingv1alpha1.DeletionStageUninstalling + if del.UninstalledAt == nil { + del.UninstalledAt = metav1Now(now) + } + } else { + res, cont, err := r.advanceRoutes(ctx, obj, rel, del, knobs, now, past) + if err != nil || !cont { + return res, err + } + } + } + if del.Stage == servingv1alpha1.DeletionStageUninstalling { + if err := r.advanceUninstall(ctx, obj, del, now); err != nil { + return ctrl.Result{}, err + } + } + return r.advancePods(ctx, obj, del, knobs, now, past) +} + +// deletionInProgress reports whether a previous pass already recorded a stage. +// The helm release is gone after uninstall, and that must not skip the pod wait. +func deletionInProgress(obj *servingv1alpha1.LLMService) bool { + return obj.Status.Deletion != nil && obj.Status.Deletion.Stage != "" +} + +func cloneDeletion(obj *servingv1alpha1.LLMService) *servingv1alpha1.DeletionStatus { + if obj.Status.Deletion == nil { + return &servingv1alpha1.DeletionStatus{} + } + return obj.Status.Deletion.DeepCopy() +} + +func stampDeletionClock(del *servingv1alpha1.DeletionStatus, now time.Time, maxWait time.Duration) { + if del.StartedAt == nil { + del.StartedAt = metav1Now(now) + } + deadline := del.StartedAt.Add(maxWait) + del.Deadline = metav1Now(deadline) +} + +func (r *LLMServiceReconciler) advanceRoutes(ctx context.Context, obj *servingv1alpha1.LLMService, rel *helm.Release, del *servingv1alpha1.DeletionStatus, knobs deletionKnobs, now time.Time, past bool) (ctrl.Result, bool, error) { + if del.Stage == "" { + routes, err := modelRoutesFromManifest(rel.Manifest, obj.Namespace) + if err != nil { + return ctrl.Result{}, false, err + } + del.Stage = nextStage(stageInput{HasRoute: len(routes) > 0}) + if del.Stage == servingv1alpha1.DeletionStageUninstalling { + setOutcome(del, servingv1alpha1.DeletionOutcomeNoRoute) + return ctrl.Result{}, true, nil + } + } + if del.Stage == servingv1alpha1.DeletionStageRouteRemoving { + res, err := r.advanceRouteRemoving(ctx, obj, rel, del, knobs, now, past) + if err != nil || del.Stage == servingv1alpha1.DeletionStageRouteRemoving { + return res, false, err + } + } + if del.Stage == servingv1alpha1.DeletionStageRouteSettling { + res, err := r.advanceRouteSettling(ctx, obj, del, knobs, now, past) + if err != nil || del.Stage == servingv1alpha1.DeletionStageRouteSettling { + return res, false, err + } + } + return ctrl.Result{}, true, nil +} + +func (r *LLMServiceReconciler) advanceRouteRemoving(ctx context.Context, obj *servingv1alpha1.LLMService, rel *helm.Release, del *servingv1alpha1.DeletionStatus, knobs deletionKnobs, now time.Time, past bool) (ctrl.Result, error) { + routes, err := modelRoutesFromManifest(rel.Manifest, obj.Namespace) + if err != nil { + return ctrl.Result{}, err + } + msg := routeRemovingMessage(routes) + if err := r.putDeletion(ctx, obj, del, servingv1alpha1.ReasonRouteRemoving, msg, corev1.EventTypeNormal); err != nil { + return ctrl.Result{}, err + } + if err := r.deleteModelRoutes(ctx, routes); err != nil { + return ctrl.Result{}, err + } + gone, err := r.modelRoutesGone(ctx, routes) + if err != nil { + return ctrl.Result{}, err + } + del.Stage = nextStage(stageInput{ + Stage: servingv1alpha1.DeletionStageRouteRemoving, + HasRoute: true, + RouteGone: gone, + SettleDone: gone && settleDone(del, knobs, now, past), + PastDeadline: past, + }) + if !gone && !past { + return ctrl.Result{RequeueAfter: deletionPoll}, nil + } + if gone && del.RouteRemovedAt == nil { + del.RouteRemovedAt = metav1Now(now) + } + if !gone && past { + setOutcome(del, servingv1alpha1.DeletionOutcomeRouteStuck) + } + return ctrl.Result{}, nil +} + +func (r *LLMServiceReconciler) advanceRouteSettling(ctx context.Context, obj *servingv1alpha1.LLMService, del *servingv1alpha1.DeletionStatus, knobs deletionKnobs, now time.Time, past bool) (ctrl.Result, error) { + if del.RouteRemovedAt == nil { + del.RouteRemovedAt = metav1Now(now) + } + if !settleDone(del, knobs, now, past) { + if err := r.putDeletion(ctx, obj, del, servingv1alpha1.ReasonRouteSettling, msgSettle, corev1.EventTypeNormal); err != nil { + return ctrl.Result{}, err + } + ready := del.RouteRemovedAt.Add(knobs.routeSettle) + return ctrl.Result{RequeueAfter: ready.Sub(now)}, nil + } + del.Stage = nextStage(stageInput{ + Stage: servingv1alpha1.DeletionStageRouteSettling, + SettleDone: true, + PastDeadline: past, + }) + return ctrl.Result{}, nil +} + +func settleDone(del *servingv1alpha1.DeletionStatus, knobs deletionKnobs, now time.Time, past bool) bool { + if past || del.RouteRemovedAt == nil { + return past + } + return !now.Before(del.RouteRemovedAt.Add(knobs.routeSettle)) +} + +func (r *LLMServiceReconciler) advanceUninstall(ctx context.Context, obj *servingv1alpha1.LLMService, del *servingv1alpha1.DeletionStatus, now time.Time) error { + if del.UninstalledAt != nil { + del.Stage = nextStage(stageInput{Stage: servingv1alpha1.DeletionStageUninstalling, Uninstalled: true}) + return nil + } + if err := r.writePreUninstall(ctx, obj, del); err != nil { + return err + } + if err := r.Helm.Uninstall(ctx, obj.Namespace, obj.Name); err != nil { + return err + } + del.UninstalledAt = metav1Now(now) + del.Stage = nextStage(stageInput{Stage: servingv1alpha1.DeletionStageUninstalling, Uninstalled: true}) + return r.putDeletion(ctx, obj, del, servingv1alpha1.ReasonUninstalled, msgUninstalled, corev1.EventTypeNormal) +} + +func (r *LLMServiceReconciler) writePreUninstall(ctx context.Context, obj *servingv1alpha1.LLMService, del *servingv1alpha1.DeletionStatus) error { + switch del.Outcome { + case servingv1alpha1.DeletionOutcomeNoRoute: + return r.putDeletion(ctx, obj, del, servingv1alpha1.ReasonNoRoute, msgNoRoute, corev1.EventTypeNormal) + case servingv1alpha1.DeletionOutcomeRouteStuck: + return r.putDeletion(ctx, obj, del, servingv1alpha1.ReasonRouteStuck, msgRouteStuck, corev1.EventTypeWarning) + default: + return r.putDeletion(ctx, obj, del, "", "", "") + } +} + +func (r *LLMServiceReconciler) advancePods(ctx context.Context, obj *servingv1alpha1.LLMService, del *servingv1alpha1.DeletionStatus, knobs deletionKnobs, now time.Time, past bool) (ctrl.Result, error) { + pods, nodes, nodesOK, err := r.observePods(ctx, obj) + if err != nil { + return ctrl.Result{}, err + } + forced, err := r.forceDuePods(ctx, obj, del, knobs, pods, now, past) + if err != nil { + return ctrl.Result{}, err + } + if forced { + pods, nodes, nodesOK, err = r.observePods(ctx, obj) + if err != nil { + return ctrl.Result{}, err + } + } + acct := accountPods(pods, nodes, nodesOK, now) + applyPodStatus(del, acct) + if acct.remaining == 0 { + return r.releasePods(ctx, obj, del, now) + } + if past { + return r.releasePastDeadline(ctx, obj, del) + } + return r.waitForPods(ctx, obj, del, acct) +} + +func (r *LLMServiceReconciler) observePods(ctx context.Context, obj *servingv1alpha1.LLMService) ([]corev1.Pod, map[string]bool, bool, error) { + pods, err := r.listReleasePods(ctx, obj) + if err != nil { + return nil, nil, false, err + } + nodes, ok := r.readyNodes(ctx) + return pods, nodes, ok, nil +} + +func (r *LLMServiceReconciler) forceDuePods(ctx context.Context, obj *servingv1alpha1.LLMService, del *servingv1alpha1.DeletionStatus, knobs deletionKnobs, pods []corev1.Pod, now time.Time, past bool) (bool, error) { + if past || knobs.forceAfter == nil { + return false, nil + } + due := dueForcePods(pods, *knobs.forceAfter, now) + if len(due) == 0 { + return false, nil + } + del.Stage = servingv1alpha1.DeletionStageForceDeleting + setOutcome(del, servingv1alpha1.DeletionOutcomePodsForceDeleted) + acct := accountPods(pods, nil, false, now) + applyPodStatus(del, acct) + del.Message = podWaitMessage(acct.remaining, acct.pods) + if err := r.putDeletion(ctx, obj, del, servingv1alpha1.ReasonPodsForceDeleted, del.Message, corev1.EventTypeWarning); err != nil { + return false, err + } + deleted := false + for i := range due { + ok, err := r.forceDeletePod(ctx, obj, &due[i]) + if err != nil { + return deleted, err + } + if !ok { + continue + } + deleted = true + del.ForceDeleted = appendUnique(del.ForceDeleted, due[i].Name) + if err := r.putDeletion(ctx, obj, del, servingv1alpha1.ReasonPodsForceDeleted, del.Message, corev1.EventTypeWarning); err != nil { + return true, err + } + } + return true, nil +} + +func applyPodStatus(del *servingv1alpha1.DeletionStatus, acct podAccount) { + del.PodsRemaining = int32(acct.remaining) + del.Pods = capPods(acct.pods) +} + +func (r *LLMServiceReconciler) releasePods(ctx context.Context, obj *servingv1alpha1.LLMService, del *servingv1alpha1.DeletionStatus, now time.Time) (ctrl.Result, error) { + if del.PodsGoneAt == nil { + del.PodsGoneAt = metav1Now(now) + } + if del.Outcome == "" { + setOutcome(del, servingv1alpha1.DeletionOutcomeClean) + } + del.Stage = nextStage(stageInput{Stage: del.Stage, PodsRemaining: 0}) + del.Message = releaseMessage(del) + return r.finishReleased(ctx, obj, del) +} + +func (r *LLMServiceReconciler) releasePastDeadline(ctx context.Context, obj *servingv1alpha1.LLMService, del *servingv1alpha1.DeletionStatus) (ctrl.Result, error) { + del.Stage = servingv1alpha1.DeletionStageReleased + if del.Outcome != servingv1alpha1.DeletionOutcomeRouteStuck { + setOutcome(del, servingv1alpha1.DeletionOutcomeTimedOut) + del.Message = msgTimedOut + if err := r.putDeletion(ctx, obj, del, servingv1alpha1.ReasonTimedOut, del.Message, corev1.EventTypeWarning); err != nil { + return ctrl.Result{}, err + } + } else { + del.Message = msgRouteStuck + } + return r.finishReleased(ctx, obj, del) +} + +func (r *LLMServiceReconciler) waitForPods(ctx context.Context, obj *servingv1alpha1.LLMService, del *servingv1alpha1.DeletionStatus, acct podAccount) (ctrl.Result, error) { + if del.Stage != servingv1alpha1.DeletionStageForceDeleting { + del.Stage = nextStage(stageInput{Stage: servingv1alpha1.DeletionStagePodsTerminating, PodsRemaining: acct.remaining}) + } + reason := servingv1alpha1.ReasonWaitingForPods + eventType := corev1.EventTypeNormal + if del.Stage == servingv1alpha1.DeletionStageForceDeleting { + reason = servingv1alpha1.ReasonPodsForceDeleted + eventType = corev1.EventTypeWarning + } + del.Message = podWaitMessage(acct.remaining, acct.pods) + if err := r.putDeletion(ctx, obj, del, reason, del.Message, eventType); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{RequeueAfter: deletionPoll}, nil +} + +func (r *LLMServiceReconciler) finishReleased(ctx context.Context, obj *servingv1alpha1.LLMService, del *servingv1alpha1.DeletionStatus) (ctrl.Result, error) { + if del.Message == "" { + del.Message = releaseMessage(del) + } + del.Stage = servingv1alpha1.DeletionStageReleased + if err := r.putDeletion(ctx, obj, del, servingv1alpha1.ReasonReleased, del.Message, corev1.EventTypeNormal); err != nil { + return ctrl.Result{}, err + } + var latest servingv1alpha1.LLMService + if err := r.Get(ctx, client.ObjectKeyFromObject(obj), &latest); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + return r.dropFinalizer(ctx, &latest) +} + +func releaseMessage(del *servingv1alpha1.DeletionStatus) string { + switch del.Outcome { + case servingv1alpha1.DeletionOutcomeTimedOut: + return msgTimedOut + case servingv1alpha1.DeletionOutcomeRouteStuck: + return msgRouteStuck + case servingv1alpha1.DeletionOutcomePodsForceDeleted: + return msgForceGone + case servingv1alpha1.DeletionOutcomeNoRoute: + return msgNoRoute + default: + return msgPodsGone + } +} + +func setOutcome(del *servingv1alpha1.DeletionStatus, next servingv1alpha1.DeletionOutcome) { + if next == servingv1alpha1.DeletionOutcomeTimedOut && del.Outcome == servingv1alpha1.DeletionOutcomeRouteStuck { + return + } + if outcomeRank(next) < outcomeRank(del.Outcome) { + return + } + del.Outcome = next +} + +func outcomeRank(outcome servingv1alpha1.DeletionOutcome) int { + switch outcome { + case servingv1alpha1.DeletionOutcomeClean: + return 1 + case servingv1alpha1.DeletionOutcomeNoRoute: + return 2 + case servingv1alpha1.DeletionOutcomeRouteStuck: + return 3 + case servingv1alpha1.DeletionOutcomePodsForceDeleted: + return 4 + case servingv1alpha1.DeletionOutcomeTimedOut: + return 5 + default: + return 0 + } +} + +// putDeletion writes phase Deleting and status.deletion before the action +// the reason names. An unchanged status is left alone so a status write does +// not retrigger the reconcile. reason empty keeps the current Deleting +// condition and does not emit an event. +func (r *LLMServiceReconciler) putDeletion(ctx context.Context, obj *servingv1alpha1.LLMService, del *servingv1alpha1.DeletionStatus, reason, message, eventType string) error { + if message != "" { + del.Message = message + } + condReason, condMessage := reason, message + if reason == "" { + condReason, condMessage = deletingCondition(obj) + if condReason == "" { + condReason = servingv1alpha1.ReasonUninstalled + } + } + if deletionUnchanged(obj, del, condReason, condMessage) { + return nil + } + prev, _ := deletingCondition(obj) + if reason != "" && prev != reason { + if eventType != "" { + r.event(obj, eventType, reason, message) + } + logf.FromContext(ctx).Info("Updated LLMService deletion", "stage", string(del.Stage), "reason", reason) + } + obj.Status.Phase = servingv1alpha1.PhaseDeleting + obj.Status.Deletion = del.DeepCopy() + meta.SetStatusCondition(&obj.Status.Conditions, metav1.Condition{ + Type: servingv1alpha1.ConditionDeleting, + Status: metav1.ConditionTrue, + Reason: condReason, + Message: condMessage, + ObservedGeneration: obj.Generation, + }) + return r.updateStatus(ctx, obj) +} + +func deletionUnchanged(obj *servingv1alpha1.LLMService, del *servingv1alpha1.DeletionStatus, reason, message string) bool { + if obj.Status.Phase != servingv1alpha1.PhaseDeleting { + return false + } + gotReason, gotMessage := deletingCondition(obj) + if gotReason != reason || gotMessage != message { + return false + } + return equality.Semantic.DeepEqual(obj.Status.Deletion, del) +} + +func deletingCondition(obj *servingv1alpha1.LLMService) (string, string) { + cond := meta.FindStatusCondition(obj.Status.Conditions, servingv1alpha1.ConditionDeleting) + if cond == nil { + return "", "" + } + return cond.Reason, cond.Message +} diff --git a/internal/controller/serving/deletion_helpers_test.go b/internal/controller/serving/deletion_helpers_test.go new file mode 100644 index 0000000..d2f1e99 --- /dev/null +++ b/internal/controller/serving/deletion_helpers_test.go @@ -0,0 +1,247 @@ +/* +Copyright 2026. + +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 serving + +import ( + "strings" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + servingv1alpha1 "github.com/modelsphere/llm-operator/api/serving/v1alpha1" + "github.com/modelsphere/llm-operator/internal/testutil/cases" +) + +type manifestCase struct { + Name string `json:"name,omitempty"` + Description string `json:"description,omitempty"` + Inputs struct { + Namespace string `json:"namespace"` + Manifest string `json:"manifest"` + } `json:"inputs"` + Expected struct { + Error bool `json:"error"` + Routes []struct { + APIVersion string `json:"apiVersion"` + Namespace string `json:"namespace"` + Name string `json:"name"` + } `json:"routes"` + } `json:"expected"` +} + +func TestModelRoutesFromManifest(t *testing.T) { + cases.Run(t, "testdata/helpers/modelRoutesFromManifest", func(t *testing.T, tc manifestCase) { + got, err := modelRoutesFromManifest(tc.Inputs.Manifest, tc.Inputs.Namespace) + if tc.Expected.Error { + if err == nil { + t.Fatal("expected an error") + } + return + } + if err != nil { + t.Fatalf("parse: %v", err) + } + if len(got) != len(tc.Expected.Routes) { + t.Fatalf("routes = %#v", got) + } + for i, want := range tc.Expected.Routes { + if got[i].APIVersion != want.APIVersion || got[i].Namespace != want.Namespace || got[i].Name != want.Name { + t.Fatalf("route %d = %#v, want %#v", i, got[i], want) + } + } + }) +} + +type podCountCase struct { + Name string `json:"name,omitempty"` + Description string `json:"description,omitempty"` + Inputs struct { + Now string `json:"now"` + NodesOK bool `json:"nodesOK"` + Nodes []struct { + Name string `json:"name"` + Ready bool `json:"ready"` + } `json:"nodes"` + Pods []struct { + Name string `json:"name"` + Phase string `json:"phase"` + Node string `json:"node"` + Grace *int64 `json:"grace"` + DeletionTimestamp string `json:"deletionTimestamp"` + } `json:"pods"` + } `json:"inputs"` + Expected struct { + Remaining int `json:"remaining"` + Message string `json:"message"` + Pods []struct { + Name string `json:"name"` + Stuck bool `json:"stuck"` + Phase string `json:"phase"` + Node string `json:"node"` + Grace int64 `json:"grace"` + } `json:"pods"` + } `json:"expected"` +} + +func TestPodCounts(t *testing.T) { + cases.Run(t, "testdata/helpers/podCounts", func(t *testing.T, tc podCountCase) { + now, err := time.Parse(time.RFC3339, tc.Inputs.Now) + if err != nil { + t.Fatalf("now: %v", err) + } + nodes := map[string]bool{} + for _, n := range tc.Inputs.Nodes { + nodes[n.Name] = n.Ready + } + pods := make([]corev1.Pod, len(tc.Inputs.Pods)) + for i, in := range tc.Inputs.Pods { + pod := corev1.Pod{} + pod.Name = in.Name + pod.Status.Phase = corev1.PodPhase(in.Phase) + pod.Spec.NodeName = in.Node + pod.Spec.TerminationGracePeriodSeconds = in.Grace + if in.DeletionTimestamp != "" { + ts, err := time.Parse(time.RFC3339, in.DeletionTimestamp) + if err != nil { + t.Fatalf("deletionTimestamp: %v", err) + } + pod.DeletionTimestamp = &metav1.Time{Time: ts} + } + pods[i] = pod + } + got := accountPods(pods, nodes, tc.Inputs.NodesOK, now) + if got.remaining != tc.Expected.Remaining { + t.Fatalf("remaining = %d, want %d", got.remaining, tc.Expected.Remaining) + } + if len(got.pods) != len(tc.Expected.Pods) { + t.Fatalf("pods = %#v", got.pods) + } + for i, want := range tc.Expected.Pods { + row := got.pods[i] + if row.Name != want.Name || row.Stuck != want.Stuck || row.Phase != want.Phase || row.Node != want.Node || row.Grace != want.Grace { + t.Fatalf("pod %d = %#v, want %#v", i, row, want) + } + } + if tc.Expected.Message != "" && podWaitMessage(got.remaining, got.pods) != tc.Expected.Message { + t.Fatalf("message = %q, want %q", podWaitMessage(got.remaining, got.pods), tc.Expected.Message) + } + }) +} + +type knobsCase struct { + Name string `json:"name,omitempty"` + Description string `json:"description,omitempty"` + Inputs struct { + Annotations map[string]string `json:"annotations"` + MaxWait *cases.Duration `json:"maxWait"` + RouteSettle *cases.Duration `json:"routeSettle"` + ForceDeleteAfter *cases.Duration `json:"forceDeleteAfter"` + ForceSet bool `json:"forceSet"` + } `json:"inputs"` + Expected struct { + MaxWait cases.Duration `json:"maxWait"` + RouteSettle cases.Duration `json:"routeSettle"` + ForceDeleteAfter *cases.Duration `json:"forceDeleteAfter"` + ForceOff bool `json:"forceOff"` + Invalid []struct { + Key string `json:"key"` + Contains string `json:"contains"` + } `json:"invalid"` + } `json:"expected"` +} + +func TestDeletionKnobs(t *testing.T) { + cases.Run(t, "testdata/helpers/deletionKnobs", func(t *testing.T, tc knobsCase) { + obj := &servingv1alpha1.LLMService{} + obj.Annotations = tc.Inputs.Annotations + if tc.Inputs.MaxWait != nil || tc.Inputs.RouteSettle != nil || tc.Inputs.ForceSet { + obj.Spec.Deletion = &servingv1alpha1.DeletionSpec{} + if tc.Inputs.MaxWait != nil { + obj.Spec.Deletion.MaxWait = &metav1.Duration{Duration: tc.Inputs.MaxWait.Std()} + } + if tc.Inputs.RouteSettle != nil { + obj.Spec.Deletion.RouteSettle = &metav1.Duration{Duration: tc.Inputs.RouteSettle.Std()} + } + if tc.Inputs.ForceSet { + d := metav1.Duration{} + if tc.Inputs.ForceDeleteAfter != nil { + d.Duration = tc.Inputs.ForceDeleteAfter.Std() + } + obj.Spec.Deletion.ForceDeleteAfter = &d + } + } + got := deletionKnobsFrom(obj) + if got.maxWait != tc.Expected.MaxWait.Std() || got.routeSettle != tc.Expected.RouteSettle.Std() { + t.Fatalf("maxWait %s routeSettle %s", got.maxWait, got.routeSettle) + } + if tc.Expected.ForceOff { + if got.forceAfter != nil { + t.Fatalf("forceAfter = %s, want off", *got.forceAfter) + } + } else if tc.Expected.ForceDeleteAfter != nil { + if got.forceAfter == nil || *got.forceAfter != tc.Expected.ForceDeleteAfter.Std() { + t.Fatalf("forceAfter = %v", got.forceAfter) + } + } + if len(got.invalid) != len(tc.Expected.Invalid) { + t.Fatalf("invalid = %#v", got.invalid) + } + for i, want := range tc.Expected.Invalid { + if got.invalid[i].key != want.Key || !strings.Contains(got.invalid[i].why, want.Contains) { + t.Fatalf("invalid %d = %#v", i, got.invalid[i]) + } + } + }) +} + +type stageCase struct { + Name string `json:"name,omitempty"` + Description string `json:"description,omitempty"` + Inputs struct { + Stage string `json:"stage"` + HasRoute bool `json:"hasRoute"` + RouteGone bool `json:"routeGone"` + SettleDone bool `json:"settleDone"` + Uninstalled bool `json:"uninstalled"` + PodsRemaining int `json:"podsRemaining"` + PastDeadline bool `json:"pastDeadline"` + ForceDue bool `json:"forceDue"` + } `json:"inputs"` + Expected struct { + Stage string `json:"stage"` + } `json:"expected"` +} + +func TestNextStage(t *testing.T) { + cases.Run(t, "testdata/helpers/nextStage", func(t *testing.T, tc stageCase) { + got := nextStage(stageInput{ + Stage: servingv1alpha1.DeletionStage(tc.Inputs.Stage), + HasRoute: tc.Inputs.HasRoute, + RouteGone: tc.Inputs.RouteGone, + SettleDone: tc.Inputs.SettleDone, + Uninstalled: tc.Inputs.Uninstalled, + PodsRemaining: tc.Inputs.PodsRemaining, + PastDeadline: tc.Inputs.PastDeadline, + ForceDue: tc.Inputs.ForceDue, + }) + if string(got) != tc.Expected.Stage { + t.Fatalf("stage = %s, want %s", got, tc.Expected.Stage) + } + }) +} diff --git a/internal/controller/serving/deletion_knobs.go b/internal/controller/serving/deletion_knobs.go new file mode 100644 index 0000000..a04c235 --- /dev/null +++ b/internal/controller/serving/deletion_knobs.go @@ -0,0 +1,125 @@ +/* +Copyright 2026. + +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 serving + +import ( + "fmt" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + servingv1alpha1 "github.com/modelsphere/llm-operator/api/serving/v1alpha1" +) + +const ( + deletionPoll = 5 * time.Second + defaultDeletionMaxWait = time.Hour + defaultRouteSettle = 60 * time.Second + stuckPastGrace = 5 * time.Minute + defaultPodGraceSeconds = int64(30) + deletionListCap = 20 +) + +const ( + msgNoRoute = "no ModelRoute in the helm manifest" + msgRouteStuck = "ModelRoute was still present at the deadline" + msgUninstalled = "Uninstalled helm release" + msgSettle = "waiting for the ModelRoute to settle" + msgTimedOut = "deletion budget elapsed with pods still present" + msgPodsGone = "release pods are gone" + msgForceGone = "force-deleted release pods" + msgNegativeWait = "duration is negative" +) + +// deletionKnobs is the effective teardown budget for one reconcile. +// forceAfter nil means force-delete is off. +type deletionKnobs struct { + maxWait time.Duration + routeSettle time.Duration + forceAfter *time.Duration + invalid []invalidAnnotation +} + +type invalidAnnotation struct { + key string + value string + why string +} + +// deletionKnobsFrom resolves annotation > spec > built-in default. +// An explicit 0s, in the spec or an annotation, is a real zero and skips +// that wait. An unparseable or negative +// annotation is reported and ignored. +func deletionKnobsFrom(obj *servingv1alpha1.LLMService) deletionKnobs { + var spec servingv1alpha1.DeletionSpec + if obj.Spec.Deletion != nil { + spec = *obj.Spec.Deletion + } + var out deletionKnobs + out.maxWait = resolveDuration(obj, servingv1alpha1.AnnotationDeletionMaxWait, spec.MaxWait, defaultDeletionMaxWait, &out) + out.routeSettle = resolveDuration(obj, servingv1alpha1.AnnotationRouteSettle, spec.RouteSettle, defaultRouteSettle, &out) + out.forceAfter = resolveForceAfter(obj, spec.ForceDeleteAfter, &out) + return out +} + +func resolveDuration(obj *servingv1alpha1.LLMService, key string, spec *metav1.Duration, fallback time.Duration, out *deletionKnobs) time.Duration { + if raw, ok := obj.Annotations[key]; ok { + d, bad := parseNonNegative(raw) + if bad == "" { + return d + } + out.invalid = append(out.invalid, invalidAnnotation{key: key, value: raw, why: bad}) + } + if spec != nil { + return spec.Duration + } + return fallback +} + +func resolveForceAfter(obj *servingv1alpha1.LLMService, spec *metav1.Duration, out *deletionKnobs) *time.Duration { + if raw, ok := obj.Annotations[servingv1alpha1.AnnotationForceDeleteAfter]; ok { + d, bad := parseNonNegative(raw) + if bad == "" { + return &d + } + out.invalid = append(out.invalid, invalidAnnotation{key: servingv1alpha1.AnnotationForceDeleteAfter, value: raw, why: bad}) + } + if spec == nil { + return nil + } + d := spec.Duration + return &d +} + +func parseNonNegative(raw string) (time.Duration, string) { + d, err := time.ParseDuration(raw) + if err != nil { + return 0, err.Error() + } + if d < 0 { + return 0, msgNegativeWait + } + return d, "" +} + +func (r *LLMServiceReconciler) warnInvalidKnobs(obj *servingv1alpha1.LLMService, knobs deletionKnobs) { + for _, inv := range knobs.invalid { + r.event(obj, corev1.EventTypeWarning, servingv1alpha1.ReasonInvalidAnnotation, + fmt.Sprintf("Ignored annotation %s=%q: %s", inv.key, inv.value, inv.why)) + } +} diff --git a/internal/controller/serving/deletion_pods.go b/internal/controller/serving/deletion_pods.go new file mode 100644 index 0000000..73d9b5a --- /dev/null +++ b/internal/controller/serving/deletion_pods.go @@ -0,0 +1,290 @@ +/* +Copyright 2026. + +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 serving + +import ( + "cmp" + "context" + "fmt" + "maps" + "slices" + "strings" + "time" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + logf "sigs.k8s.io/controller-runtime/pkg/log" + + servingv1alpha1 "github.com/modelsphere/llm-operator/api/serving/v1alpha1" +) + +// podAccount is the release pods still in the API, ignoring Succeeded and Failed. +type podAccount struct { + remaining int + pods []servingv1alpha1.DeletionPod +} + +// accountPods applies the stuck rules. Within grace a pod always counts. +// Past grace, a pod on a NotReady or missing node is gone and stuck. Any +// other pod is gone after grace plus 5m. When nodesOK is false only the +// time rule applies. pods are sorted by name. +func accountPods(pods []corev1.Pod, nodes map[string]bool, nodesOK bool, now time.Time) podAccount { + var rows []servingv1alpha1.DeletionPod + remaining := 0 + for i := range pods { + pod := &pods[i] + if pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed { + continue + } + grace := podGrace(pod) + stuck := podStuck(pod, grace, nodes, nodesOK, now) + if !stuck { + remaining++ + } + row := servingv1alpha1.DeletionPod{ + Name: pod.Name, + Phase: string(pod.Status.Phase), + Node: pod.Spec.NodeName, + DeletionTimestamp: pod.DeletionTimestamp, + Grace: grace, + Stuck: stuck, + } + rows = append(rows, row) + } + slices.SortFunc(rows, func(a, b servingv1alpha1.DeletionPod) int { + return cmp.Compare(a.Name, b.Name) + }) + return podAccount{remaining: remaining, pods: rows} +} + +func podGrace(pod *corev1.Pod) int64 { + if pod.Spec.TerminationGracePeriodSeconds != nil { + return *pod.Spec.TerminationGracePeriodSeconds + } + return defaultPodGraceSeconds +} + +// podStuck reports whether pod no longer blocks the finalizer. +func podStuck(pod *corev1.Pod, grace int64, nodes map[string]bool, nodesOK bool, now time.Time) bool { + if pod.DeletionTimestamp.IsZero() { + return false + } + deadline := graceDeadline(pod, grace) + if now.Before(deadline) { + return false + } + if nodesOK && !podNodeReady(pod, nodes) { + return true + } + return !now.Before(deadline.Add(stuckPastGrace)) +} + +// graceDeadline is when the pod's termination grace ends. +// The API sets deletionTimestamp to that instant and records the window in +// deletionGracePeriodSeconds. A timestamp without that field is the moment +// deletion was requested, and the grace comes from the pod spec. +func graceDeadline(pod *corev1.Pod, grace int64) time.Time { + if pod.DeletionGracePeriodSeconds != nil { + return pod.DeletionTimestamp.Time + } + return pod.DeletionTimestamp.Add(time.Duration(grace) * time.Second) +} + +// terminationStarted is when the pod was marked for deletion. +func terminationStarted(pod *corev1.Pod) time.Time { + if pod.DeletionGracePeriodSeconds != nil { + return pod.DeletionTimestamp.Add(-time.Duration(*pod.DeletionGracePeriodSeconds) * time.Second) + } + return pod.DeletionTimestamp.Time +} + +func podNodeReady(pod *corev1.Pod, nodes map[string]bool) bool { + if pod.Spec.NodeName == "" { + return false + } + ready, ok := nodes[pod.Spec.NodeName] + return ok && ready +} + +func podWaitMessage(remaining int, pods []servingv1alpha1.DeletionPod) string { + noun := "pods" + if remaining == 1 { + noun = "pod" + } + msg := fmt.Sprintf("waiting for %d %s", remaining, noun) + note := stuckNote(pods) + if note == "" { + return msg + } + return msg + " (" + note + ")" +} + +func stuckNote(pods []servingv1alpha1.DeletionPod) string { + byNode := map[string]int{} + missing := 0 + for _, pod := range pods { + if !pod.Stuck { + continue + } + if pod.Node == "" { + missing++ + continue + } + byNode[pod.Node]++ + } + if len(byNode) == 0 && missing == 0 { + return "" + } + var parts []string + names := slices.Sorted(maps.Keys(byNode)) + switch len(names) { + case 0: + case 1: + parts = append(parts, fmt.Sprintf("%d stuck on NotReady node %s", byNode[names[0]], names[0])) + default: + total := 0 + for _, name := range names { + total += byNode[name] + } + parts = append(parts, fmt.Sprintf("%d stuck on NotReady nodes %s", total, strings.Join(names, ", "))) + } + if missing > 0 { + parts = append(parts, fmt.Sprintf("%d stuck on a missing node", missing)) + } + return strings.Join(parts, ", ") +} + +func capPods(pods []servingv1alpha1.DeletionPod) []servingv1alpha1.DeletionPod { + if len(pods) > deletionListCap { + return pods[:deletionListCap] + } + return pods +} + +func appendUnique(in []string, name string) []string { + if slices.Contains(in, name) || len(in) >= deletionListCap { + return in + } + return append(in, name) +} + +func (r *LLMServiceReconciler) listReleasePods(ctx context.Context, obj *servingv1alpha1.LLMService) ([]corev1.Pod, error) { + selectors := []client.MatchingLabels{ + {"app.kubernetes.io/instance": obj.Name}, + } + if obj.Status.Chart.Name != "" { + selectors = append(selectors, client.MatchingLabels{"app": obj.Name + "-" + obj.Status.Chart.Name}) + } + seen := map[types.UID]struct{}{} + var out []corev1.Pod + for _, sel := range selectors { + var list corev1.PodList + if err := r.List(ctx, &list, client.InNamespace(obj.Namespace), sel); err != nil { + return nil, err + } + for _, pod := range list.Items { + if _, ok := seen[pod.UID]; ok { + continue + } + seen[pod.UID] = struct{}{} + out = append(out, pod) + } + } + return out, nil +} + +// readyNodes maps a node name to whether its Ready condition is True. +// The bool is false when the node list cannot be read. +func (r *LLMServiceReconciler) readyNodes(ctx context.Context) (map[string]bool, bool) { + var list corev1.NodeList + if err := r.List(ctx, &list); err != nil { + logf.FromContext(ctx).Error(err, "Could not list Nodes") + return nil, false + } + out := make(map[string]bool, len(list.Items)) + for i := range list.Items { + out[list.Items[i].Name] = nodeReady(&list.Items[i]) + } + return out, true +} + +func nodeReady(node *corev1.Node) bool { + for _, cond := range node.Status.Conditions { + if cond.Type == corev1.NodeReady { + return cond.Status == corev1.ConditionTrue + } + } + return false +} + +// dueForcePods are non-terminal pods whose deletionTimestamp is at least +// after old. Pods that are not deleting are never due. +func dueForcePods(pods []corev1.Pod, after time.Duration, now time.Time) []corev1.Pod { + var due []corev1.Pod + for i := range pods { + pod := &pods[i] + if pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed { + continue + } + if pod.DeletionTimestamp.IsZero() { + continue + } + if now.Before(terminationStarted(pod).Add(after)) { + continue + } + due = append(due, *pod) + } + slices.SortFunc(due, func(a, b corev1.Pod) int { return cmp.Compare(a.Name, b.Name) }) + return due +} + +// forceDeletePod deletes pod with grace 0. deleted is false when it was already gone. +// A grace-0 delete of a pod that is already terminating only shortens the grace +// period. The second delete removes the API object once that grace is 0. +func (r *LLMServiceReconciler) forceDeletePod(ctx context.Context, obj *servingv1alpha1.LLMService, pod *corev1.Pod) (bool, error) { + uid := pod.UID + existed, err := r.deletePodNow(ctx, pod, uid) + if err != nil || !existed { + return false, err + } + if _, err := r.deletePodNow(ctx, pod, uid); err != nil { + return false, err + } + r.event(obj, corev1.EventTypeWarning, servingv1alpha1.ReasonPodForceDeleted, + fmt.Sprintf("Force-deleted pod %s", pod.Name)) + return true, nil +} + +func (r *LLMServiceReconciler) deletePodNow(ctx context.Context, pod *corev1.Pod, uid types.UID) (bool, error) { + err := r.Delete(ctx, pod, client.GracePeriodSeconds(0), client.Preconditions{UID: &uid}) + if apierrors.IsNotFound(err) { + return false, nil + } + if err != nil { + return false, err + } + return true, nil +} + +// metav1Now keeps the monotonic instant so a wait computed in this pass is exact. +func metav1Now(now time.Time) *metav1.Time { + t := metav1.Time{Time: now} + return &t +} diff --git a/internal/controller/serving/deletion_route.go b/internal/controller/serving/deletion_route.go new file mode 100644 index 0000000..56a45d2 --- /dev/null +++ b/internal/controller/serving/deletion_route.go @@ -0,0 +1,147 @@ +/* +Copyright 2026. + +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 serving + +import ( + "bufio" + "bytes" + "context" + "fmt" + "io" + "strings" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + k8syaml "k8s.io/apimachinery/pkg/util/yaml" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const modelRouteKind = "ModelRoute" + +// modelRouteGroups are the API groups whose ModelRoute objects are drained +// before a release is uninstalled. +var modelRouteGroups = map[string]struct{}{ + "routing.modelsphere.dev": {}, + "routing.gpucluster.io": {}, +} + +// modelRouteRef is one ModelRoute rendered into a helm release manifest. +type modelRouteRef struct { + APIVersion string + Kind string + Namespace string + Name string +} + +// modelRoutesFromManifest returns ModelRoute objects in the routing groups. +// An empty namespace is filled from defaultNS. Other documents are ignored. +// A document that does not decode is an error. +func modelRoutesFromManifest(manifest, defaultNS string) ([]modelRouteRef, error) { + reader := k8syaml.NewYAMLReader(bufio.NewReader(strings.NewReader(manifest))) + var out []modelRouteRef + for { + doc, err := reader.Read() + if err == io.EOF { + return out, nil + } + if err != nil { + return nil, fmt.Errorf("decode manifest: %w", err) + } + if len(bytes.TrimSpace(doc)) == 0 { + continue + } + obj := map[string]any{} + if err := k8syaml.Unmarshal(doc, &obj); err != nil { + return nil, fmt.Errorf("decode manifest: %w", err) + } + if len(obj) == 0 { + continue + } + ref, ok, err := routeRef(&unstructured.Unstructured{Object: obj}, defaultNS) + if err != nil { + return nil, err + } + if ok { + out = append(out, ref) + } + } +} + +func routeRef(u *unstructured.Unstructured, defaultNS string) (modelRouteRef, bool, error) { + if u.GetKind() != modelRouteKind { + return modelRouteRef{}, false, nil + } + group, _, _ := strings.Cut(u.GetAPIVersion(), "/") + if _, ok := modelRouteGroups[group]; !ok { + return modelRouteRef{}, false, nil + } + if u.GetName() == "" { + return modelRouteRef{}, false, fmt.Errorf("modelroute is missing metadata.name") + } + ns := u.GetNamespace() + if ns == "" { + ns = defaultNS + } + return modelRouteRef{ + APIVersion: u.GetAPIVersion(), + Kind: u.GetKind(), + Namespace: ns, + Name: u.GetName(), + }, true, nil +} + +func routeObject(ref modelRouteRef) *unstructured.Unstructured { + u := &unstructured.Unstructured{} + u.SetAPIVersion(ref.APIVersion) + u.SetKind(ref.Kind) + u.SetNamespace(ref.Namespace) + u.SetName(ref.Name) + return u +} + +// deleteModelRoutes deletes each route and leaves its finalizers in place. +// NotFound is success. +func (r *LLMServiceReconciler) deleteModelRoutes(ctx context.Context, routes []modelRouteRef) error { + for _, ref := range routes { + if err := r.Delete(ctx, routeObject(ref)); err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("delete modelroute %s/%s: %w", ref.Namespace, ref.Name, err) + } + } + return nil +} + +// modelRoutesGone reports whether every route is NotFound. +func (r *LLMServiceReconciler) modelRoutesGone(ctx context.Context, routes []modelRouteRef) (bool, error) { + for _, ref := range routes { + err := r.Get(ctx, client.ObjectKey{Namespace: ref.Namespace, Name: ref.Name}, routeObject(ref)) + if apierrors.IsNotFound(err) { + continue + } + if err != nil { + return false, fmt.Errorf("get modelroute %s/%s: %w", ref.Namespace, ref.Name, err) + } + return false, nil + } + return true, nil +} + +func routeRemovingMessage(routes []modelRouteRef) string { + if len(routes) == 1 { + return fmt.Sprintf("waiting for ModelRoute %s to be removed", routes[0].Name) + } + return fmt.Sprintf("waiting for %d ModelRoutes to be removed", len(routes)) +} diff --git a/internal/controller/serving/llmservice_cases_test.go b/internal/controller/serving/llmservice_cases_test.go index da7988c..5749059 100644 --- a/internal/controller/serving/llmservice_cases_test.go +++ b/internal/controller/serving/llmservice_cases_test.go @@ -26,13 +26,16 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/events" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -52,28 +55,33 @@ type serviceCase struct { } type serviceStep struct { - Action string `json:"action"` - FreshNamespace bool `json:"freshNamespace,omitempty"` - Name string `json:"name,omitempty"` - Version string `json:"version,omitempty"` - Annotations map[string]string `json:"annotations,omitempty"` - Finalizers []string `json:"finalizers,omitempty"` - ChartVersion *string `json:"chartVersion,omitempty"` - Suspend *bool `json:"suspend,omitempty"` - Layers []layerSpec `json:"layers,omitempty"` - Release *releaseSpec `json:"release,omitempty"` - Entries []releaseSpec `json:"entries,omitempty"` - Put string `json:"put,omitempty"` - Versions []string `json:"versions,omitempty"` - ListErr *string `json:"listErr,omitempty"` - InstallErr *string `json:"installErr,omitempty"` - UpgradeErr *string `json:"upgradeErr,omitempty"` - GetErr *string `json:"getErr,omitempty"` - KeepFailed *bool `json:"keepFailed,omitempty"` - Repos []string `json:"repos,omitempty"` - Status *statusSpec `json:"status,omitempty"` - Capture []string `json:"capture,omitempty"` - Expect *expectSpec `json:"expect,omitempty"` + Action string `json:"action"` + FreshNamespace bool `json:"freshNamespace,omitempty"` + Name string `json:"name,omitempty"` + Version string `json:"version,omitempty"` + Annotations map[string]string `json:"annotations,omitempty"` + Finalizers []string `json:"finalizers,omitempty"` + ChartVersion *string `json:"chartVersion,omitempty"` + Suspend *bool `json:"suspend,omitempty"` + Layers []layerSpec `json:"layers,omitempty"` + Release *releaseSpec `json:"release,omitempty"` + Entries []releaseSpec `json:"entries,omitempty"` + Put string `json:"put,omitempty"` + Versions []string `json:"versions,omitempty"` + ListErr *string `json:"listErr,omitempty"` + InstallErr *string `json:"installErr,omitempty"` + UpgradeErr *string `json:"upgradeErr,omitempty"` + GetErr *string `json:"getErr,omitempty"` + KeepFailed *bool `json:"keepFailed,omitempty"` + Repos []string `json:"repos,omitempty"` + Status *statusSpec `json:"status,omitempty"` + Capture []string `json:"capture,omitempty"` + Expect *expectSpec `json:"expect,omitempty"` + Deletion *deletionSpecInput `json:"deletion,omitempty"` + Pod *podInput `json:"pod,omitempty"` + Node *nodeInput `json:"node,omitempty"` + Route *routeInput `json:"route,omitempty"` + Advance *cases.Duration `json:"advance,omitempty"` } type layerSpec struct { @@ -91,11 +99,50 @@ type releaseSpec struct { Owned bool `json:"owned,omitempty"` Slot bool `json:"slot,omitempty"` Updated *cases.Duration `json:"updated,omitempty"` + Manifest string `json:"manifest,omitempty"` +} + +type deletionSpecInput struct { + MaxWait *cases.Duration `json:"maxWait,omitempty"` + RouteSettle *cases.Duration `json:"routeSettle,omitempty"` + ForceDeleteAfter *cases.Duration `json:"forceDeleteAfter,omitempty"` +} + +type podInput struct { + Name string `json:"name"` + Phase string `json:"phase,omitempty"` + Node string `json:"node,omitempty"` + Grace *int64 `json:"grace,omitempty"` + DeletionTimestamp *cases.Duration `json:"deletionTimestamp,omitempty"` + Labels map[string]string `json:"labels,omitempty"` +} + +type nodeInput struct { + Name string `json:"name"` + Ready bool `json:"ready,omitempty"` +} + +type routeInput struct { + Name string `json:"name,omitempty"` + Finalizers []string `json:"finalizers,omitempty"` } type statusSpec struct { - Phase string `json:"phase,omitempty"` - Conditions []conditionSpec `json:"conditions,omitempty"` + Phase string `json:"phase,omitempty"` + ChartName string `json:"chartName,omitempty"` + Conditions []conditionSpec `json:"conditions,omitempty"` + Deletion *deletionStatusInput `json:"deletion,omitempty"` +} + +type deletionStatusInput struct { + Stage string `json:"stage,omitempty"` + Outcome string `json:"outcome,omitempty"` + Message string `json:"message,omitempty"` + StartedAt *cases.Duration `json:"startedAt,omitempty"` + RouteRemovedAt *cases.Duration `json:"routeRemovedAt,omitempty"` + UninstalledAt *cases.Duration `json:"uninstalledAt,omitempty"` + PodsGoneAt *cases.Duration `json:"podsGoneAt,omitempty"` + PodsRemaining *int32 `json:"podsRemaining,omitempty"` } type conditionSpec struct { @@ -108,41 +155,45 @@ type conditionSpec struct { } type expectSpec struct { - Mid *expectSpec `json:"mid,omitempty"` - Error *bool `json:"error,omitempty"` - ErrorContains string `json:"errorContains,omitempty"` - Installs *int `json:"installs,omitempty"` - Upgrades *int `json:"upgrades,omitempty"` - Gets *int `json:"gets,omitempty"` - Uninstalls *int `json:"uninstalls,omitempty"` - Rollbacks *int `json:"rollbacks,omitempty"` - LastRollback *int `json:"lastRollback,omitempty"` - HelmTouched *int `json:"helmTouched,omitempty"` - ListCalls *int `json:"listCalls,omitempty"` - LastForce *bool `json:"lastForce,omitempty"` - GetsUnchanged *bool `json:"getsUnchanged,omitempty"` - UpgradesUnchanged *bool `json:"upgradesUnchanged,omitempty"` - Phase string `json:"phase,omitempty"` - ChartVersion string `json:"chartVersion,omitempty"` - ReResolve *string `json:"reResolve,omitempty"` - AppliedHash *hashExpect `json:"appliedHash,omitempty"` - HelmRevision *int `json:"helmRevision,omitempty"` - ObservedGeneration string `json:"observedGeneration,omitempty"` - Generation *int64 `json:"generation,omitempty"` - History *historyExpect `json:"history,omitempty"` - Conditions []conditionSpec `json:"conditions,omitempty"` - ReleaseUID string `json:"releaseUID,omitempty"` - Release *releaseExpect `json:"release,omitempty"` - NotFound *bool `json:"notFound,omitempty"` - Message *messageExpect `json:"message,omitempty"` - DistinctRevisions *int `json:"distinctRevisions,omitempty"` - Snapshots []snapshotExpect `json:"snapshots,omitempty"` - Requeue *requeueExpect `json:"requeue,omitempty"` - EventContains string `json:"eventContains,omitempty"` - Finalizer string `json:"finalizer,omitempty"` - PlantedRevisionUsed *bool `json:"plantedRevisionUsed,omitempty"` - CapturedRevisionGone *bool `json:"capturedRevisionGone,omitempty"` - LatestRevisionExists *bool `json:"latestRevisionExists,omitempty"` + Mid *expectSpec `json:"mid,omitempty"` + Error *bool `json:"error,omitempty"` + ErrorContains string `json:"errorContains,omitempty"` + Installs *int `json:"installs,omitempty"` + Upgrades *int `json:"upgrades,omitempty"` + Gets *int `json:"gets,omitempty"` + Uninstalls *int `json:"uninstalls,omitempty"` + Rollbacks *int `json:"rollbacks,omitempty"` + LastRollback *int `json:"lastRollback,omitempty"` + HelmTouched *int `json:"helmTouched,omitempty"` + ListCalls *int `json:"listCalls,omitempty"` + LastForce *bool `json:"lastForce,omitempty"` + GetsUnchanged *bool `json:"getsUnchanged,omitempty"` + UpgradesUnchanged *bool `json:"upgradesUnchanged,omitempty"` + Phase string `json:"phase,omitempty"` + ChartVersion string `json:"chartVersion,omitempty"` + ReResolve *string `json:"reResolve,omitempty"` + AppliedHash *hashExpect `json:"appliedHash,omitempty"` + HelmRevision *int `json:"helmRevision,omitempty"` + ObservedGeneration string `json:"observedGeneration,omitempty"` + Generation *int64 `json:"generation,omitempty"` + History *historyExpect `json:"history,omitempty"` + Conditions []conditionSpec `json:"conditions,omitempty"` + ReleaseUID string `json:"releaseUID,omitempty"` + Release *releaseExpect `json:"release,omitempty"` + NotFound *bool `json:"notFound,omitempty"` + Message *messageExpect `json:"message,omitempty"` + DistinctRevisions *int `json:"distinctRevisions,omitempty"` + Snapshots []snapshotExpect `json:"snapshots,omitempty"` + Requeue *requeueExpect `json:"requeue,omitempty"` + EventContains string `json:"eventContains,omitempty"` + EventsContain []string `json:"eventsContain,omitempty"` + Finalizer string `json:"finalizer,omitempty"` + Deletion *deletionExpect `json:"deletion,omitempty"` + Route *routeExpect `json:"route,omitempty"` + ClusterPods []clusterPodExpect `json:"clusterPods,omitempty"` + PlantedRevisionUsed *bool `json:"plantedRevisionUsed,omitempty"` + CapturedRevisionGone *bool `json:"capturedRevisionGone,omitempty"` + LatestRevisionExists *bool `json:"latestRevisionExists,omitempty"` } type hashExpect struct { @@ -190,6 +241,41 @@ type snapshotExpect struct { ControllerOwner *bool `json:"controllerOwner,omitempty"` } +type deletionExpect struct { + Stage string `json:"stage,omitempty"` + Outcome string `json:"outcome,omitempty"` + Message *messageExpect `json:"message,omitempty"` + PodsRemaining *int `json:"podsRemaining,omitempty"` + ForceDeleted []string `json:"forceDeleted,omitempty"` + Pods []deletionPodExpect `json:"pods,omitempty"` + StartedAt *bool `json:"startedAt,omitempty"` + Deadline *bool `json:"deadline,omitempty"` + RouteRemovedAt *bool `json:"routeRemovedAt,omitempty"` + UninstalledAt *bool `json:"uninstalledAt,omitempty"` + PodsGoneAt *bool `json:"podsGoneAt,omitempty"` +} + +type deletionPodExpect struct { + Name string `json:"name"` + Phase string `json:"phase,omitempty"` + Node string `json:"node,omitempty"` + Grace *int64 `json:"grace,omitempty"` + Stuck *bool `json:"stuck,omitempty"` +} + +type routeExpect struct { + Name string `json:"name,omitempty"` + NotFound *bool `json:"notFound,omitempty"` + Deleting *bool `json:"deleting,omitempty"` + Finalizers []string `json:"finalizers,omitempty"` +} + +type clusterPodExpect struct { + Name string `json:"name"` + NotFound *bool `json:"notFound,omitempty"` + Deleting *bool `json:"deleting,omitempty"` +} + type requeueExpect struct { Exact *cases.Duration `json:"exact,omitempty"` AtLeast *cases.Duration `json:"atLeast,omitempty"` @@ -214,10 +300,11 @@ type caseRun struct { appliedHash string observedGeneration int64 planted string + nowOffset time.Duration } func runServiceCase(tc serviceCase) { - rec := events.NewFakeRecorder(32) + rec := events.NewFakeRecorder(64) h := &fakeHelm{} l := &versionList{versions: []string{ver086}} limit := tc.HistoryLimit @@ -238,6 +325,7 @@ func runServiceCase(tc serviceCase) { r: r, events: rec.Events, } + r.Now = run.clock Expect(tc.Steps).NotTo(BeEmpty(), tc.Name) for i, step := range tc.Steps { run.step(i, step) @@ -292,6 +380,22 @@ func (c *caseRun) step(i int, step serviceStep) { c.setStatus(step) case "setStatusReleaseUID": c.setStatusReleaseUID() + case "addPod": + c.addPod(step) + case "removePod": + c.removePod(step) + case "setNode": + c.setNode(step) + case "advanceClock": + c.advanceClock(step) + case "addRoute": + c.addRoute(step) + case "clearRouteFinalizer": + c.clearRouteFinalizer(step) + case "annotate": + c.annotate(step) + case "freshReconciler": + c.freshReconciler() default: Fail(fmt.Sprintf("%s: step %d unknown action %q", c.tc.Name, i+1, step.Action)) } @@ -311,6 +415,7 @@ func (c *caseRun) create(step serviceStep) { if step.Finalizers != nil { obj.Finalizers = append([]string(nil), step.Finalizers...) } + applyDeletionSpec(obj, step.Deletion) Expect(k8sClient.Create(ctx, obj)).To(Succeed()) } @@ -331,9 +436,164 @@ func (c *caseRun) update(step serviceStep) { if step.Layers != nil { obj.Spec.Layers = layersFrom(step.Layers) } + applyDeletionSpec(obj, step.Deletion) Expect(k8sClient.Update(ctx, obj)).To(Succeed()) } +func (c *caseRun) annotate(step serviceStep) { + Expect(step.Annotations).NotTo(BeEmpty()) + obj := fetch(c.ns, c.name) + if obj.Annotations == nil { + obj.Annotations = map[string]string{} + } + maps.Copy(obj.Annotations, step.Annotations) + Expect(k8sClient.Update(ctx, obj)).To(Succeed()) +} + +func applyDeletionSpec(obj *servingv1alpha1.LLMService, in *deletionSpecInput) { + if in == nil { + return + } + spec := &servingv1alpha1.DeletionSpec{} + if obj.Spec.Deletion != nil { + spec = obj.Spec.Deletion + } + if in.MaxWait != nil { + spec.MaxWait = &metav1.Duration{Duration: in.MaxWait.Std()} + } + if in.RouteSettle != nil { + spec.RouteSettle = &metav1.Duration{Duration: in.RouteSettle.Std()} + } + if in.ForceDeleteAfter != nil { + d := metav1.Duration{Duration: in.ForceDeleteAfter.Std()} + spec.ForceDeleteAfter = &d + } + obj.Spec.Deletion = spec +} + +func (c *caseRun) clock() time.Time { + return time.Now().Add(c.nowOffset) +} + +func (c *caseRun) advanceClock(step serviceStep) { + Expect(step.Advance).NotTo(BeNil()) + c.nowOffset += step.Advance.Std() +} + +func (c *caseRun) freshReconciler() { + rec := events.NewFakeRecorder(64) + next := newReconciler(c.h, c.l, c.r.HistoryLimit) + next.Recorder = rec + next.PendingTimeout = c.r.PendingTimeout + next.AllowedChartRepos = append([]string(nil), c.r.AllowedChartRepos...) + next.Now = c.clock + c.r = next + c.events = rec.Events +} + +func (c *caseRun) nodeName(short string) string { + if short == "" { + return "" + } + return c.ns + "-" + short +} + +func (c *caseRun) addPod(step serviceStep) { + Expect(step.Pod).NotTo(BeNil()) + Expect(step.Pod.Name).NotTo(BeEmpty()) + labels := step.Pod.Labels + if labels == nil { + labels = map[string]string{ + "app.kubernetes.io/instance": c.name, + "app": c.name + "-sglang", + } + } + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: step.Pod.Name, Namespace: c.ns, Labels: labels}, + Spec: corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyAlways, + NodeName: c.nodeName(step.Pod.Node), + Containers: []corev1.Container{{Name: "c", Image: "pause"}}, + }, + } + if step.Pod.Grace != nil { + pod.Spec.TerminationGracePeriodSeconds = step.Pod.Grace + } + Expect(k8sClient.Create(ctx, pod)).To(Succeed()) + if step.Pod.Phase != "" { + pod.Status.Phase = corev1.PodPhase(step.Pod.Phase) + Expect(k8sClient.Status().Update(ctx, pod)).To(Succeed()) + } + if step.Pod.DeletionTimestamp != nil { + Expect(k8sClient.Delete(ctx, pod)).To(Succeed()) + } +} + +func (c *caseRun) removePod(step serviceStep) { + Expect(step.Pod).NotTo(BeNil()) + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: step.Pod.Name, Namespace: c.ns}} + Expect(k8sClient.Delete(ctx, pod, client.GracePeriodSeconds(0))).To(Succeed()) +} + +func (c *caseRun) setNode(step serviceStep) { + Expect(step.Node).NotTo(BeNil()) + Expect(step.Node.Name).NotTo(BeEmpty()) + name := c.nodeName(step.Node.Name) + node := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: name}} + err := k8sClient.Get(ctx, types.NamespacedName{Name: name}, node) + if apierrors.IsNotFound(err) { + Expect(k8sClient.Create(ctx, node)).To(Succeed()) + } else { + Expect(err).NotTo(HaveOccurred()) + } + status := corev1.ConditionFalse + if step.Node.Ready { + status = corev1.ConditionTrue + } + node.Status.Conditions = []corev1.NodeCondition{{ + Type: corev1.NodeReady, + Status: status, + }} + Expect(k8sClient.Status().Update(ctx, node)).To(Succeed()) +} + +func (c *caseRun) addRoute(step serviceStep) { + name := c.routeName(step) + u := modelRouteObject(c.ns, name) + if step.Route != nil { + u.SetFinalizers(append([]string(nil), step.Route.Finalizers...)) + } + u.Object["spec"] = map[string]any{"model": name} + Expect(k8sClient.Create(ctx, u)).To(Succeed()) +} + +func (c *caseRun) clearRouteFinalizer(step serviceStep) { + name := c.routeName(step) + u := modelRouteObject(c.ns, name) + Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: c.ns, Name: name}, u)).To(Succeed()) + u.SetFinalizers(nil) + err := k8sClient.Update(ctx, u) + if err != nil && !apierrors.IsNotFound(err) { + Expect(err).NotTo(HaveOccurred()) + } +} + +func (c *caseRun) routeName(step serviceStep) string { + if step.Route != nil && step.Route.Name != "" { + return step.Route.Name + } + return c.name +} + +func modelRouteObject(ns, name string) *unstructured.Unstructured { + u := &unstructured.Unstructured{} + u.SetAPIVersion("routing.modelsphere.dev/v1alpha1") + u.SetKind("ModelRoute") + u.SetNamespace(ns) + u.SetName(name) + return u +} + func layersFrom(in []layerSpec) []servingv1alpha1.Layer { out := make([]servingv1alpha1.Layer, len(in)) for i, l := range in { @@ -418,7 +678,9 @@ func (c *caseRun) releaseFrom(in releaseSpec) helm.Release { if in.Updated != nil { updated = time.Now().Add(in.Updated.Std()) } - return *ownedRel(fetch(c.ns, c.name), in.Revision, in.Status, updated) + rel := *ownedRel(fetch(c.ns, c.name), in.Revision, in.Status, updated) + rel.Manifest = in.Manifest + return rel } rel := helm.Release{ Revision: in.Revision, @@ -427,6 +689,7 @@ func (c *caseRun) releaseFrom(in releaseSpec) helm.Release { ChartVersion: in.ChartVersion, Config: in.Config, Labels: in.Labels, + Manifest: in.Manifest, } if in.Updated != nil { rel.Updated = time.Now().Add(in.Updated.Std()) @@ -471,12 +734,42 @@ func (c *caseRun) setStatus(step serviceStep) { if step.Status.Phase != "" { obj.Status.Phase = servingv1alpha1.Phase(step.Status.Phase) } + if step.Status.ChartName != "" { + obj.Status.Chart.Name = step.Status.ChartName + } + if step.Status.Deletion != nil { + obj.Status.Deletion = deletionStatusFrom(step.Status.Deletion) + } for _, cond := range step.Status.Conditions { meta.SetStatusCondition(&obj.Status.Conditions, conditionFrom(obj, cond)) } Expect(k8sClient.Status().Update(ctx, obj)).To(Succeed()) } +func deletionStatusFrom(in *deletionStatusInput) *servingv1alpha1.DeletionStatus { + out := &servingv1alpha1.DeletionStatus{ + Stage: servingv1alpha1.DeletionStage(in.Stage), + Outcome: servingv1alpha1.DeletionOutcome(in.Outcome), + Message: in.Message, + } + out.StartedAt = offsetTime(in.StartedAt) + out.RouteRemovedAt = offsetTime(in.RouteRemovedAt) + out.UninstalledAt = offsetTime(in.UninstalledAt) + out.PodsGoneAt = offsetTime(in.PodsGoneAt) + if in.PodsRemaining != nil { + out.PodsRemaining = *in.PodsRemaining + } + return out +} + +func offsetTime(d *cases.Duration) *metav1.Time { + if d == nil { + return nil + } + t := metav1.NewTime(time.Now().Add(d.Std())) + return &t +} + func conditionFrom(obj *servingv1alpha1.LLMService, cond conditionSpec) metav1.Condition { out := metav1.Condition{ Type: cond.Type, @@ -549,6 +842,13 @@ func (c *caseRun) check(exp *expectSpec, result reconcile.Result, err error, bef if exp.EventContains != "" { Expect(c.events).To(Receive(ContainSubstring(exp.EventContains))) } + if len(exp.EventsContain) > 0 { + c.checkEvents(exp.EventsContain) + } + if exp.Route != nil { + c.checkRoute(exp.Route) + } + c.checkClusterPods(exp.ClusterPods) if exp.NotFound != nil && *exp.NotFound { c.checkMissing() } @@ -642,7 +942,7 @@ func (e *expectSpec) wantsObject() bool { if e.HelmRevision != nil || e.ObservedGeneration != "" || e.Generation != nil || e.History != nil { return true } - if len(e.Conditions) > 0 || e.ReleaseUID != "" || e.Message != nil || e.Finalizer != "" { + if len(e.Conditions) > 0 || e.ReleaseUID != "" || e.Message != nil || e.Finalizer != "" || e.Deletion != nil { return true } if e.DistinctRevisions != nil || len(e.Snapshots) > 0 || e.PlantedRevisionUsed != nil { @@ -670,6 +970,7 @@ func (c *caseRun) checkObject(exp *expectSpec, got *servingv1alpha1.LLMService) c.checkConditions(exp.Conditions, got) c.checkReleaseUID(exp.ReleaseUID, got) c.checkMessage(exp.Message, got) + c.checkDeletion(exp.Deletion, got) c.checkFinalizer(exp.Finalizer, got) c.checkRevisions(exp, got) } @@ -787,24 +1088,147 @@ func (c *caseRun) checkReleaseUID(want string, got *servingv1alpha1.LLMService) } func (c *caseRun) checkMessage(exp *messageExpect, got *servingv1alpha1.LLMService) { + checkText(exp, got.Status.Message) +} + +func checkText(exp *messageExpect, got string) { if exp == nil { return } if exp.Equals != "" { - Expect(got.Status.Message).To(Equal(exp.Equals)) + Expect(got).To(Equal(exp.Equals)) } // An empty substring matches every message, so it is rejected on its own. for _, sub := range exp.Contains { if sub == "" { Fail("message.contains entry is empty") } - Expect(got.Status.Message).To(ContainSubstring(sub)) + Expect(got).To(ContainSubstring(sub)) } for _, sub := range exp.NotContains { if sub == "" { Fail("message.notContains entry is empty") } - Expect(got.Status.Message).NotTo(ContainSubstring(sub)) + Expect(got).NotTo(ContainSubstring(sub)) + } +} + +func (c *caseRun) checkDeletion(exp *deletionExpect, got *servingv1alpha1.LLMService) { + if exp == nil { + return + } + Expect(got.Status.Deletion).NotTo(BeNil()) + del := got.Status.Deletion + if exp.Stage != "" { + Expect(string(del.Stage)).To(Equal(exp.Stage)) + } + if exp.Outcome != "" { + Expect(string(del.Outcome)).To(Equal(exp.Outcome)) + } + checkText(exp.Message, del.Message) + if exp.PodsRemaining != nil { + Expect(int(del.PodsRemaining)).To(Equal(*exp.PodsRemaining)) + } + if exp.ForceDeleted != nil { + Expect(del.ForceDeleted).To(Equal(exp.ForceDeleted)) + } + expectTimeSet(exp.StartedAt, del.StartedAt, "startedAt") + expectTimeSet(exp.Deadline, del.Deadline, "deadline") + expectTimeSet(exp.RouteRemovedAt, del.RouteRemovedAt, "routeRemovedAt") + expectTimeSet(exp.UninstalledAt, del.UninstalledAt, "uninstalledAt") + expectTimeSet(exp.PodsGoneAt, del.PodsGoneAt, "podsGoneAt") + if exp.Pods == nil { + return + } + Expect(del.Pods).To(HaveLen(len(exp.Pods))) + for i, want := range exp.Pods { + gotPod := del.Pods[i] + Expect(gotPod.Name).To(Equal(want.Name)) + if want.Phase != "" { + Expect(gotPod.Phase).To(Equal(want.Phase)) + } + if want.Node != "" { + Expect(gotPod.Node).To(Equal(c.nodeName(want.Node))) + } + if want.Grace != nil { + Expect(gotPod.Grace).To(Equal(*want.Grace)) + } + if want.Stuck != nil { + Expect(gotPod.Stuck).To(Equal(*want.Stuck)) + } + } +} + +func expectTimeSet(want *bool, got *metav1.Time, field string) { + if want == nil { + return + } + if *want { + Expect(got).NotTo(BeNil(), field) + return + } + Expect(got).To(BeNil(), field) +} + +func (c *caseRun) checkEvents(want []string) { + got := c.drainEvents() + for _, sub := range want { + Expect(got).To(ContainElement(ContainSubstring(sub)), "events %v", got) + } +} + +func (c *caseRun) drainEvents() []string { + var out []string + for { + select { + case e := <-c.events: + out = append(out, e) + default: + return out + } + } +} + +func (c *caseRun) checkRoute(exp *routeExpect) { + name := c.name + if exp.Name != "" { + name = exp.Name + } + u := modelRouteObject(c.ns, name) + err := k8sClient.Get(ctx, types.NamespacedName{Namespace: c.ns, Name: name}, u) + if exp.NotFound != nil && *exp.NotFound { + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + return + } + Expect(err).NotTo(HaveOccurred()) + if exp.Deleting != nil { + if *exp.Deleting { + Expect(u.GetDeletionTimestamp().IsZero()).To(BeFalse()) + } else { + Expect(u.GetDeletionTimestamp().IsZero()).To(BeTrue()) + } + } + if exp.Finalizers != nil { + Expect(u.GetFinalizers()).To(Equal(exp.Finalizers)) + } +} + +func (c *caseRun) checkClusterPods(pods []clusterPodExpect) { + for _, want := range pods { + pod := &corev1.Pod{} + err := k8sClient.Get(ctx, types.NamespacedName{Namespace: c.ns, Name: want.Name}, pod) + if want.NotFound != nil && *want.NotFound { + Expect(apierrors.IsNotFound(err)).To(BeTrue(), want.Name) + continue + } + Expect(err).NotTo(HaveOccurred()) + if want.Deleting != nil { + if *want.Deleting { + Expect(pod.DeletionTimestamp.IsZero()).To(BeFalse()) + } else { + Expect(pod.DeletionTimestamp.IsZero()).To(BeTrue()) + } + } } } diff --git a/internal/controller/serving/llmservice_controller.go b/internal/controller/serving/llmservice_controller.go index 44bed80..5a97654 100644 --- a/internal/controller/serving/llmservice_controller.go +++ b/internal/controller/serving/llmservice_controller.go @@ -82,6 +82,8 @@ type LLMServiceReconciler struct { // AllowedChartRepos are the chart repositories an LLMService may install // from; each allows itself and the paths under it. Empty allows any. AllowedChartRepos []string + // Now is the clock for staged deletion. Nil uses time.Now. Tests set it. + Now func() time.Time } // +kubebuilder:rbac:groups=serving.modelsphere.dev,resources=llmservices,verbs=get;list;watch;create;update;patch;delete @@ -90,7 +92,8 @@ type LLMServiceReconciler struct { // +kubebuilder:rbac:groups=apps,resources=controllerrevisions;deployments,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups="",resources=secrets;configmaps;services;serviceaccounts,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;create;patch -// +kubebuilder:rbac:groups="",resources=pods,verbs=get;list;watch;patch +// +kubebuilder:rbac:groups="",resources=pods,verbs=get;list;watch;patch;delete +// +kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch // +kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=get;list;watch;create;update;patch @@ -125,22 +128,29 @@ func (r *LLMServiceReconciler) Reconcile(ctx context.Context, req ctrl.Request) } func (r *LLMServiceReconciler) reconcileDelete(ctx context.Context, obj *servingv1alpha1.LLMService) (ctrl.Result, error) { - if obj.Annotations[servingv1alpha1.AnnotationDeletionPolicy] != servingv1alpha1.DeletionPolicyOrphan { - rel, err := r.Helm.Get(ctx, obj.Namespace, obj.Name) - if err != nil { - return ctrl.Result{}, err - } - switch { - case rel == nil: - case installedOrAdopted(obj, rel): - if err := r.Helm.Uninstall(ctx, obj.Namespace, obj.Name); err != nil { - return ctrl.Result{}, err - } - r.event(obj, corev1.EventTypeNormal, "Uninstalled", "Uninstalled helm release") - default: - r.event(obj, corev1.EventTypeNormal, "KeptRelease", "Kept helm release: this LLMService never installed or adopted it") + if obj.Annotations[servingv1alpha1.AnnotationDeletionPolicy] == servingv1alpha1.DeletionPolicyOrphan { + return r.dropFinalizer(ctx, obj) + } + rel, err := r.Helm.Get(ctx, obj.Namespace, obj.Name) + if err != nil { + return ctrl.Result{}, err + } + // Uninstall removes the release. A later pass still has to wait for pods, + // using the stage already stored on the object. + if rel == nil { + if deletionInProgress(obj) { + return r.reconcileStagedDelete(ctx, obj, nil) } + return r.dropFinalizer(ctx, obj) + } + if !installedOrAdopted(obj, rel) { + r.event(obj, corev1.EventTypeNormal, servingv1alpha1.ReasonKeptRelease, "Kept helm release: this LLMService never installed or adopted it") + return r.dropFinalizer(ctx, obj) } + return r.reconcileStagedDelete(ctx, obj, rel) +} + +func (r *LLMServiceReconciler) dropFinalizer(ctx context.Context, obj *servingv1alpha1.LLMService) (ctrl.Result, error) { if !controllerutil.ContainsFinalizer(obj, servingv1alpha1.Finalizer) { return ctrl.Result{}, nil } diff --git a/internal/controller/serving/suite_test.go b/internal/controller/serving/suite_test.go index 4a418a4..f997ffc 100644 --- a/internal/controller/serving/suite_test.go +++ b/internal/controller/serving/suite_test.go @@ -67,7 +67,10 @@ var _ = BeforeSuite(func() { By("bootstrapping test environment") testEnv = &envtest.Environment{ - CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "config", "crd", "bases")}, + CRDDirectoryPaths: []string{ + filepath.Join("..", "..", "..", "config", "crd", "bases"), + filepath.Join("testdata", "crds"), + }, ErrorIfCRDPathMissing: true, } diff --git a/internal/controller/serving/testdata/controller/a-foreign-release-is-kept-and-its-route-not-deleted.yaml b/internal/controller/serving/testdata/controller/a-foreign-release-is-kept-and-its-route-not-deleted.yaml new file mode 100644 index 0000000..7c8cf4e --- /dev/null +++ b/internal/controller/serving/testdata/controller/a-foreign-release-is-kept-and-its-route-not-deleted.yaml @@ -0,0 +1,40 @@ +name: a-foreign-release-is-kept-and-its-route-not-deleted +description: a foreign release is kept and its route not deleted +steps: + - action: putRelease + release: + revision: 3 + status: deployed + chartName: sglang + chartVersion: "0.8.6" + manifest: | + apiVersion: routing.modelsphere.dev/v1alpha1 + kind: ModelRoute + metadata: + name: qwen + - action: create + version: "0.8.6" + - action: reconcile + expect: + conditions: + - type: Adopted + reason: Drift + - action: addRoute + route: + name: qwen + finalizers: + - routing.modelsphere.dev/cleanup + - action: delete + - action: reconcile + expect: + uninstalls: 0 + notFound: true + release: + exists: true + route: + notFound: false + deleting: false + finalizers: + - routing.modelsphere.dev/cleanup + eventsContain: + - KeptRelease diff --git a/internal/controller/serving/testdata/controller/a-restart-resumes-the-recorded-stage.yaml b/internal/controller/serving/testdata/controller/a-restart-resumes-the-recorded-stage.yaml new file mode 100644 index 0000000..c60e689 --- /dev/null +++ b/internal/controller/serving/testdata/controller/a-restart-resumes-the-recorded-stage.yaml @@ -0,0 +1,39 @@ +name: a-restart-resumes-the-recorded-stage +description: a restart resumes the recorded stage +steps: + - action: create + version: "0.8.6" + finalizers: + - serving.modelsphere.dev/finalizer + - action: putRelease + release: + owned: true + revision: 1 + status: deployed + - action: setStatus + status: + phase: Deleting + chartName: sglang + deletion: + stage: PodsTerminating + outcome: NoRoute + startedAt: -1m + uninstalledAt: -30s + - action: addPod + pod: + name: engine + phase: Running + - action: delete + - action: freshReconciler + - action: reconcile + expect: + uninstalls: 0 + phase: Deleting + finalizer: present + requeue: + exact: 5s + deletion: + stage: PodsTerminating + outcome: NoRoute + podsRemaining: 1 + uninstalledAt: true diff --git a/internal/controller/serving/testdata/controller/an-annotation-added-mid-deletion-takes-effect.yaml b/internal/controller/serving/testdata/controller/an-annotation-added-mid-deletion-takes-effect.yaml new file mode 100644 index 0000000..87718e6 --- /dev/null +++ b/internal/controller/serving/testdata/controller/an-annotation-added-mid-deletion-takes-effect.yaml @@ -0,0 +1,43 @@ +name: an-annotation-added-mid-deletion-takes-effect +description: an annotation added mid-deletion takes effect on the next pass +steps: + - action: create + version: "0.8.6" + finalizers: + - serving.modelsphere.dev/finalizer + - action: putRelease + release: + owned: true + revision: 1 + status: deployed + manifest: | + apiVersion: routing.modelsphere.dev/v1alpha1 + kind: ModelRoute + metadata: + name: qwen + - action: addRoute + route: + name: qwen + finalizers: + - routing.modelsphere.dev/cleanup + - action: delete + - action: reconcile + expect: + uninstalls: 0 + deletion: + stage: RouteRemoving + requeue: + exact: 5s + - action: annotate + annotations: + serving.modelsphere.dev/deletion-max-wait: 0s + - action: reconcile + expect: + uninstalls: 1 + notFound: true + eventsContain: + - RouteStuck + route: + deleting: true + finalizers: + - routing.modelsphere.dev/cleanup diff --git a/internal/controller/serving/testdata/controller/annotation-route-settle-overrides-spec.yaml b/internal/controller/serving/testdata/controller/annotation-route-settle-overrides-spec.yaml new file mode 100644 index 0000000..64973d7 --- /dev/null +++ b/internal/controller/serving/testdata/controller/annotation-route-settle-overrides-spec.yaml @@ -0,0 +1,35 @@ +name: annotation-route-settle-overrides-spec +description: an annotation overrides spec.deletion.routeSettle +steps: + - action: create + version: "0.8.6" + finalizers: + - serving.modelsphere.dev/finalizer + annotations: + serving.modelsphere.dev/route-settle: 5s + deletion: + routeSettle: 30s + maxWait: 2h + - action: putRelease + release: + owned: true + revision: 1 + status: deployed + manifest: | + apiVersion: routing.modelsphere.dev/v1alpha1 + kind: ModelRoute + metadata: + name: qwen + - action: addRoute + route: + name: qwen + - action: delete + - action: reconcile + expect: + uninstalls: 0 + deletion: + stage: RouteSettling + requeue: + exact: 5s + route: + notFound: true diff --git a/internal/controller/serving/testdata/controller/annotation-route-settle-zero-skips-the-wait.yaml b/internal/controller/serving/testdata/controller/annotation-route-settle-zero-skips-the-wait.yaml new file mode 100644 index 0000000..f299281 --- /dev/null +++ b/internal/controller/serving/testdata/controller/annotation-route-settle-zero-skips-the-wait.yaml @@ -0,0 +1,31 @@ +name: annotation-route-settle-zero-skips-the-wait +description: an annotation of 0s skips the route settle wait +steps: + - action: create + version: "0.8.6" + finalizers: + - serving.modelsphere.dev/finalizer + annotations: + serving.modelsphere.dev/route-settle: 0s + deletion: + routeSettle: 30s + - action: putRelease + release: + owned: true + revision: 1 + status: deployed + manifest: | + apiVersion: routing.modelsphere.dev/v1alpha1 + kind: ModelRoute + metadata: + name: qwen + - action: addRoute + route: + name: qwen + - action: delete + - action: reconcile + expect: + uninstalls: 1 + notFound: true + eventsContain: + - Uninstalled diff --git a/internal/controller/serving/testdata/controller/default-route-settle-is-60s.yaml b/internal/controller/serving/testdata/controller/default-route-settle-is-60s.yaml new file mode 100644 index 0000000..8fce33c --- /dev/null +++ b/internal/controller/serving/testdata/controller/default-route-settle-is-60s.yaml @@ -0,0 +1,28 @@ +name: default-route-settle-is-60s +description: the default routeSettle is 60s +steps: + - action: create + version: "0.8.6" + finalizers: + - serving.modelsphere.dev/finalizer + - action: putRelease + release: + owned: true + revision: 1 + status: deployed + manifest: | + apiVersion: routing.modelsphere.dev/v1alpha1 + kind: ModelRoute + metadata: + name: qwen + - action: addRoute + route: + name: qwen + - action: delete + - action: reconcile + expect: + uninstalls: 0 + deletion: + stage: RouteSettling + requeue: + exact: 60s diff --git a/internal/controller/serving/testdata/controller/deletes-the-modelroute-before-uninstall.yaml b/internal/controller/serving/testdata/controller/deletes-the-modelroute-before-uninstall.yaml new file mode 100644 index 0000000..0a0ca70 --- /dev/null +++ b/internal/controller/serving/testdata/controller/deletes-the-modelroute-before-uninstall.yaml @@ -0,0 +1,40 @@ +name: deletes-the-modelroute-before-uninstall +description: deletes the ModelRoute before uninstall +steps: + - action: create + version: "0.8.6" + finalizers: + - serving.modelsphere.dev/finalizer + - action: putRelease + release: + owned: true + revision: 1 + status: deployed + manifest: | + apiVersion: routing.modelsphere.dev/v1alpha1 + kind: ModelRoute + metadata: + name: qwen + - action: addRoute + route: + name: qwen + finalizers: + - routing.modelsphere.dev/cleanup + - action: delete + - action: reconcile + expect: + uninstalls: 0 + phase: Deleting + finalizer: present + requeue: + exact: 5s + deletion: + stage: RouteRemoving + message: + equals: waiting for ModelRoute qwen to be removed + route: + deleting: true + finalizers: + - routing.modelsphere.dev/cleanup + eventsContain: + - RouteRemoving diff --git a/internal/controller/serving/testdata/controller/force-delete-after-force-deletes-and-records-forcedeleted.yaml b/internal/controller/serving/testdata/controller/force-delete-after-force-deletes-and-records-forcedeleted.yaml new file mode 100644 index 0000000..d2ebb33 --- /dev/null +++ b/internal/controller/serving/testdata/controller/force-delete-after-force-deletes-and-records-forcedeleted.yaml @@ -0,0 +1,60 @@ +name: force-delete-after-force-deletes-and-records-forcedeleted +description: force-delete-after force-deletes and records forceDeleted +steps: + - action: setNode + node: + name: gpu-7 + ready: true + - action: create + version: "0.8.6" + finalizers: + - serving.modelsphere.dev/finalizer + annotations: + serving.modelsphere.dev/force-delete-after: 1h + - action: putRelease + release: + owned: true + revision: 1 + status: deployed + - action: addPod + pod: + name: live + phase: Running + - action: addPod + pod: + name: stuck + phase: Running + node: gpu-7 + grace: 600 + deletionTimestamp: 0s + - action: delete + - action: reconcile + expect: + uninstalls: 1 + finalizer: present + deletion: + stage: PodsTerminating + outcome: NoRoute + podsRemaining: 2 + requeue: + exact: 5s + - action: annotate + annotations: + serving.modelsphere.dev/force-delete-after: 0s + - action: reconcile + expect: + uninstalls: 1 + finalizer: present + deletion: + stage: ForceDeleting + outcome: PodsForceDeleted + podsRemaining: 1 + forceDeleted: + - stuck + clusterPods: + - name: stuck + notFound: true + - name: live + notFound: false + eventsContain: + - PodForceDeleted diff --git a/internal/controller/serving/testdata/controller/ignores-succeeded-and-failed-pods.yaml b/internal/controller/serving/testdata/controller/ignores-succeeded-and-failed-pods.yaml new file mode 100644 index 0000000..16ef66a --- /dev/null +++ b/internal/controller/serving/testdata/controller/ignores-succeeded-and-failed-pods.yaml @@ -0,0 +1,25 @@ +name: ignores-succeeded-and-failed-pods +description: ignores Succeeded and Failed pods +steps: + - action: create + version: "0.8.6" + finalizers: + - serving.modelsphere.dev/finalizer + - action: putRelease + release: + owned: true + revision: 1 + status: deployed + - action: addPod + pod: + name: done + phase: Succeeded + - action: addPod + pod: + name: bad + phase: Failed + - action: delete + - action: reconcile + expect: + uninstalls: 1 + notFound: true diff --git a/internal/controller/serving/testdata/controller/invalid-annotation-falls-back-to-spec.yaml b/internal/controller/serving/testdata/controller/invalid-annotation-falls-back-to-spec.yaml new file mode 100644 index 0000000..36abbca --- /dev/null +++ b/internal/controller/serving/testdata/controller/invalid-annotation-falls-back-to-spec.yaml @@ -0,0 +1,34 @@ +name: invalid-annotation-falls-back-to-spec +description: an invalid annotation falls back to spec +steps: + - action: create + version: "0.8.6" + finalizers: + - serving.modelsphere.dev/finalizer + annotations: + serving.modelsphere.dev/route-settle: nope + deletion: + routeSettle: 15s + - action: putRelease + release: + owned: true + revision: 1 + status: deployed + manifest: | + apiVersion: routing.modelsphere.dev/v1alpha1 + kind: ModelRoute + metadata: + name: qwen + - action: addRoute + route: + name: qwen + - action: delete + - action: reconcile + expect: + uninstalls: 0 + deletion: + stage: RouteSettling + requeue: + exact: 15s + eventsContain: + - InvalidAnnotation diff --git a/internal/controller/serving/testdata/controller/maxwait-timeout-releases-with-timedout-while-pods-remain.yaml b/internal/controller/serving/testdata/controller/maxwait-timeout-releases-with-timedout-while-pods-remain.yaml new file mode 100644 index 0000000..963f92c --- /dev/null +++ b/internal/controller/serving/testdata/controller/maxwait-timeout-releases-with-timedout-while-pods-remain.yaml @@ -0,0 +1,29 @@ +name: maxwait-timeout-releases-with-timedout-while-pods-remain +description: maxWait timeout releases with TimedOut while pods remain +steps: + - action: create + version: "0.8.6" + finalizers: + - serving.modelsphere.dev/finalizer + annotations: + serving.modelsphere.dev/deletion-max-wait: 0s + - action: putRelease + release: + owned: true + revision: 1 + status: deployed + - action: addPod + pod: + name: engine + phase: Running + - action: delete + - action: reconcile + expect: + uninstalls: 1 + notFound: true + eventsContain: + - TimedOut + clusterPods: + - name: engine + notFound: false + deleting: false diff --git a/internal/controller/serving/testdata/controller/no-modelroute-goes-straight-to-uninstall.yaml b/internal/controller/serving/testdata/controller/no-modelroute-goes-straight-to-uninstall.yaml new file mode 100644 index 0000000..27b0de3 --- /dev/null +++ b/internal/controller/serving/testdata/controller/no-modelroute-goes-straight-to-uninstall.yaml @@ -0,0 +1,38 @@ +name: no-modelroute-goes-straight-to-uninstall +description: no ModelRoute in the manifest goes straight to uninstall +steps: + - action: create + version: "0.8.6" + finalizers: + - serving.modelsphere.dev/finalizer + - action: putRelease + release: + owned: true + revision: 1 + status: deployed + - action: setStatus + status: + chartName: sglang + - action: addPod + pod: + name: engine + phase: Running + - action: delete + - action: reconcile + expect: + uninstalls: 1 + phase: Deleting + finalizer: present + requeue: + exact: 5s + deletion: + stage: PodsTerminating + outcome: NoRoute + podsRemaining: 1 + uninstalledAt: true + message: + equals: waiting for 1 pod + eventsContain: + - NoRoute + - Uninstalled + - WaitingForPods diff --git a/internal/controller/serving/testdata/controller/orphan-skips-staged-deletion.yaml b/internal/controller/serving/testdata/controller/orphan-skips-staged-deletion.yaml new file mode 100644 index 0000000..55293b8 --- /dev/null +++ b/internal/controller/serving/testdata/controller/orphan-skips-staged-deletion.yaml @@ -0,0 +1,26 @@ +name: orphan-skips-staged-deletion +description: Orphan skips all stages +steps: + - action: create + version: "0.8.6" + - action: reconcile + expect: {} + - action: addRoute + route: + name: qwen + finalizers: + - routing.modelsphere.dev/cleanup + - action: update + annotations: + serving.modelsphere.dev/deletion-policy: Orphan + - action: delete + - action: reconcile + expect: + uninstalls: 0 + getsUnchanged: true + notFound: true + route: + notFound: false + deleting: false + finalizers: + - routing.modelsphere.dev/cleanup diff --git a/internal/controller/serving/testdata/controller/route-stuck-past-maxwait-leaves-the-finalizer.yaml b/internal/controller/serving/testdata/controller/route-stuck-past-maxwait-leaves-the-finalizer.yaml new file mode 100644 index 0000000..12efcaf --- /dev/null +++ b/internal/controller/serving/testdata/controller/route-stuck-past-maxwait-leaves-the-finalizer.yaml @@ -0,0 +1,37 @@ +name: route-stuck-past-maxwait-leaves-the-finalizer +description: a route stuck past maxWait gives RouteStuck, uninstalls, and leaves the ModelRoute finalizer +steps: + - action: create + version: "0.8.6" + finalizers: + - serving.modelsphere.dev/finalizer + annotations: + serving.modelsphere.dev/deletion-max-wait: 0s + - action: putRelease + release: + owned: true + revision: 1 + status: deployed + manifest: | + apiVersion: routing.modelsphere.dev/v1alpha1 + kind: ModelRoute + metadata: + name: qwen + - action: addRoute + route: + name: qwen + finalizers: + - routing.modelsphere.dev/cleanup + - action: delete + - action: reconcile + expect: + uninstalls: 1 + notFound: true + eventsContain: + - RouteStuck + - Uninstalled + route: + notFound: false + deleting: true + finalizers: + - routing.modelsphere.dev/cleanup diff --git a/internal/controller/serving/testdata/controller/suspend-keeps-the-finalizer.yaml b/internal/controller/serving/testdata/controller/suspend-keeps-the-finalizer.yaml new file mode 100644 index 0000000..ece174e --- /dev/null +++ b/internal/controller/serving/testdata/controller/suspend-keeps-the-finalizer.yaml @@ -0,0 +1,15 @@ +name: suspend-keeps-the-finalizer +description: suspend keeps the finalizer +steps: + - action: create + version: "0.8.6" + - action: reconcile + expect: {} + - action: update + suspend: true + - action: delete + - action: reconcile + expect: + uninstalls: 0 + getsUnchanged: true + finalizer: present diff --git a/internal/controller/serving/testdata/controller/treats-a-pod-stuck-on-a-notready-node-as-gone-past-grace.yaml b/internal/controller/serving/testdata/controller/treats-a-pod-stuck-on-a-notready-node-as-gone-past-grace.yaml new file mode 100644 index 0000000..5331c29 --- /dev/null +++ b/internal/controller/serving/testdata/controller/treats-a-pod-stuck-on-a-notready-node-as-gone-past-grace.yaml @@ -0,0 +1,46 @@ +name: treats-a-pod-stuck-on-a-notready-node-as-gone-past-grace +description: treats a pod stuck on a NotReady node as gone past grace +steps: + - action: setNode + node: + name: gpu-7 + ready: false + - action: create + version: "0.8.6" + finalizers: + - serving.modelsphere.dev/finalizer + - action: putRelease + release: + owned: true + revision: 1 + status: deployed + - action: addPod + pod: + name: engine + phase: Running + node: gpu-7 + grace: 600 + deletionTimestamp: 0s + - action: delete + - action: reconcile + expect: + uninstalls: 1 + finalizer: present + deletion: + stage: PodsTerminating + podsRemaining: 1 + pods: + - name: engine + node: gpu-7 + stuck: false + grace: 600 + requeue: + exact: 5s + - action: advanceClock + advance: 601s + - action: reconcile + expect: + notFound: true + clusterPods: + - name: engine + notFound: false diff --git a/internal/controller/serving/testdata/controller/waits-for-pods-then-drops-the-finalizer.yaml b/internal/controller/serving/testdata/controller/waits-for-pods-then-drops-the-finalizer.yaml new file mode 100644 index 0000000..7dc09a2 --- /dev/null +++ b/internal/controller/serving/testdata/controller/waits-for-pods-then-drops-the-finalizer.yaml @@ -0,0 +1,49 @@ +name: waits-for-pods-then-drops-the-finalizer +description: waits for pods, then drops the finalizer +steps: + - action: create + version: "0.8.6" + finalizers: + - serving.modelsphere.dev/finalizer + - action: putRelease + release: + owned: true + revision: 1 + status: deployed + - action: setStatus + status: + chartName: sglang + - action: addPod + pod: + name: app-only + phase: Running + labels: + app: qwen-sglang + - action: addPod + pod: + name: instance-only + phase: Running + labels: + app.kubernetes.io/instance: qwen + - action: delete + - action: reconcile + expect: + uninstalls: 1 + phase: Deleting + finalizer: present + requeue: + exact: 5s + deletion: + stage: PodsTerminating + outcome: NoRoute + podsRemaining: 2 + - action: removePod + pod: + name: app-only + - action: removePod + pod: + name: instance-only + - action: reconcile + expect: + uninstalls: 1 + notFound: true diff --git a/internal/controller/serving/testdata/controller/waits-for-the-route-to-be-notfound-then-for-routesettle.yaml b/internal/controller/serving/testdata/controller/waits-for-the-route-to-be-notfound-then-for-routesettle.yaml new file mode 100644 index 0000000..c9bfca4 --- /dev/null +++ b/internal/controller/serving/testdata/controller/waits-for-the-route-to-be-notfound-then-for-routesettle.yaml @@ -0,0 +1,53 @@ +name: waits-for-the-route-to-be-notfound-then-for-routesettle +description: waits for the route to be NotFound, then for routeSettle +steps: + - action: create + version: "0.8.6" + finalizers: + - serving.modelsphere.dev/finalizer + - action: putRelease + release: + owned: true + revision: 1 + status: deployed + manifest: | + apiVersion: routing.modelsphere.dev/v1alpha1 + kind: ModelRoute + metadata: + name: qwen + - action: addRoute + route: + name: qwen + finalizers: + - routing.modelsphere.dev/cleanup + - action: delete + - action: reconcile + expect: + uninstalls: 0 + deletion: + stage: RouteRemoving + - action: clearRouteFinalizer + route: + name: qwen + - action: reconcile + expect: + uninstalls: 0 + phase: Deleting + finalizer: present + requeue: + exact: 60s + deletion: + stage: RouteSettling + routeRemovedAt: true + message: + equals: waiting for the ModelRoute to settle + route: + notFound: true + - action: advanceClock + advance: 60s + - action: reconcile + expect: + uninstalls: 1 + notFound: true + eventsContain: + - Uninstalled diff --git a/internal/controller/serving/testdata/crds/modelroute.yaml b/internal/controller/serving/testdata/crds/modelroute.yaml new file mode 100644 index 0000000..26b7072 --- /dev/null +++ b/internal/controller/serving/testdata/crds/modelroute.yaml @@ -0,0 +1,20 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: modelroutes.routing.modelsphere.dev +spec: + group: routing.modelsphere.dev + names: + kind: ModelRoute + listKind: ModelRouteList + plural: modelroutes + singular: modelroute + scope: Namespaced + versions: + - name: v1alpha1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + x-kubernetes-preserve-unknown-fields: true diff --git a/internal/controller/serving/testdata/helpers/deletionKnobs/annotation-overrides-spec.yaml b/internal/controller/serving/testdata/helpers/deletionKnobs/annotation-overrides-spec.yaml new file mode 100644 index 0000000..1d2c42a --- /dev/null +++ b/internal/controller/serving/testdata/helpers/deletionKnobs/annotation-overrides-spec.yaml @@ -0,0 +1,15 @@ +name: annotation-overrides-spec +description: annotations override spec +inputs: + annotations: + serving.modelsphere.dev/deletion-max-wait: 5m + serving.modelsphere.dev/route-settle: 5s + serving.modelsphere.dev/force-delete-after: 1s + maxWait: 2h + routeSettle: 30s + forceSet: true + forceDeleteAfter: 10s +expected: + maxWait: 5m + routeSettle: 5s + forceDeleteAfter: 1s diff --git a/internal/controller/serving/testdata/helpers/deletionKnobs/annotation-zero-is-kept.yaml b/internal/controller/serving/testdata/helpers/deletionKnobs/annotation-zero-is-kept.yaml new file mode 100644 index 0000000..00c9704 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/deletionKnobs/annotation-zero-is-kept.yaml @@ -0,0 +1,15 @@ +name: annotation-zero-is-kept +description: an annotation of 0s is a real zero +inputs: + annotations: + serving.modelsphere.dev/deletion-max-wait: 0s + serving.modelsphere.dev/route-settle: 0s + serving.modelsphere.dev/force-delete-after: 0s + maxWait: 2h + routeSettle: 30s + forceSet: true + forceDeleteAfter: 10s +expected: + maxWait: 0s + routeSettle: 0s + forceDeleteAfter: 0s diff --git a/internal/controller/serving/testdata/helpers/deletionKnobs/defaults-when-unset.yaml b/internal/controller/serving/testdata/helpers/deletionKnobs/defaults-when-unset.yaml new file mode 100644 index 0000000..7df35fd --- /dev/null +++ b/internal/controller/serving/testdata/helpers/deletionKnobs/defaults-when-unset.yaml @@ -0,0 +1,7 @@ +name: defaults-when-unset +description: an empty spec uses the built-in defaults and leaves force-delete off +inputs: {} +expected: + maxWait: 1h + routeSettle: 60s + forceOff: true diff --git a/internal/controller/serving/testdata/helpers/deletionKnobs/empty-annotation-is-invalid.yaml b/internal/controller/serving/testdata/helpers/deletionKnobs/empty-annotation-is-invalid.yaml new file mode 100644 index 0000000..79b7c7a --- /dev/null +++ b/internal/controller/serving/testdata/helpers/deletionKnobs/empty-annotation-is-invalid.yaml @@ -0,0 +1,12 @@ +name: empty-annotation-is-invalid +description: an empty annotation falls back to the default +inputs: + annotations: + serving.modelsphere.dev/deletion-max-wait: "" +expected: + maxWait: 1h + routeSettle: 60s + forceOff: true + invalid: + - key: serving.modelsphere.dev/deletion-max-wait + contains: invalid duration diff --git a/internal/controller/serving/testdata/helpers/deletionKnobs/invalid-annotation-falls-back-to-spec.yaml b/internal/controller/serving/testdata/helpers/deletionKnobs/invalid-annotation-falls-back-to-spec.yaml new file mode 100644 index 0000000..ad5a231 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/deletionKnobs/invalid-annotation-falls-back-to-spec.yaml @@ -0,0 +1,17 @@ +name: invalid-annotation-falls-back-to-spec +description: an unparseable annotation falls back to spec +inputs: + annotations: + serving.modelsphere.dev/deletion-max-wait: nope + serving.modelsphere.dev/route-settle: 1d + maxWait: 2h + routeSettle: 15s +expected: + maxWait: 2h + routeSettle: 15s + forceOff: true + invalid: + - key: serving.modelsphere.dev/deletion-max-wait + contains: invalid duration + - key: serving.modelsphere.dev/route-settle + contains: unknown unit diff --git a/internal/controller/serving/testdata/helpers/deletionKnobs/negative-annotation-falls-back.yaml b/internal/controller/serving/testdata/helpers/deletionKnobs/negative-annotation-falls-back.yaml new file mode 100644 index 0000000..fc4d29c --- /dev/null +++ b/internal/controller/serving/testdata/helpers/deletionKnobs/negative-annotation-falls-back.yaml @@ -0,0 +1,15 @@ +name: negative-annotation-falls-back +description: a negative annotation falls back to the default +inputs: + annotations: + serving.modelsphere.dev/route-settle: -1s + serving.modelsphere.dev/force-delete-after: -5s +expected: + maxWait: 1h + routeSettle: 60s + forceOff: true + invalid: + - key: serving.modelsphere.dev/route-settle + contains: negative + - key: serving.modelsphere.dev/force-delete-after + contains: negative diff --git a/internal/controller/serving/testdata/helpers/deletionKnobs/spec-force-delete-zero.yaml b/internal/controller/serving/testdata/helpers/deletionKnobs/spec-force-delete-zero.yaml new file mode 100644 index 0000000..67544f5 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/deletionKnobs/spec-force-delete-zero.yaml @@ -0,0 +1,9 @@ +name: spec-force-delete-zero +description: a spec forceDeleteAfter of 0s force-deletes immediately +inputs: + forceSet: true + forceDeleteAfter: 0s +expected: + maxWait: 1h + routeSettle: 60s + forceDeleteAfter: 0s diff --git a/internal/controller/serving/testdata/helpers/deletionKnobs/spec-values.yaml b/internal/controller/serving/testdata/helpers/deletionKnobs/spec-values.yaml new file mode 100644 index 0000000..432dc39 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/deletionKnobs/spec-values.yaml @@ -0,0 +1,11 @@ +name: spec-values +description: spec values replace the built-in defaults +inputs: + maxWait: 2h + routeSettle: 30s + forceSet: true + forceDeleteAfter: 10s +expected: + maxWait: 2h + routeSettle: 30s + forceDeleteAfter: 10s diff --git a/internal/controller/serving/testdata/helpers/deletionKnobs/spec-zero-is-kept.yaml b/internal/controller/serving/testdata/helpers/deletionKnobs/spec-zero-is-kept.yaml new file mode 100644 index 0000000..1e3e195 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/deletionKnobs/spec-zero-is-kept.yaml @@ -0,0 +1,9 @@ +name: spec-zero-is-kept +description: an explicit 0s spec maxWait or routeSettle is a real zero, not the default +inputs: + maxWait: 0s + routeSettle: 0s +expected: + maxWait: 0s + routeSettle: 0s + forceOff: true diff --git a/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/empty.yaml b/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/empty.yaml new file mode 100644 index 0000000..453b0ac --- /dev/null +++ b/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/empty.yaml @@ -0,0 +1,7 @@ +name: empty +description: an empty manifest has no ModelRoute +inputs: + namespace: ns + manifest: "" +expected: + routes: [] diff --git a/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/fills-an-empty-namespace.yaml b/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/fills-an-empty-namespace.yaml new file mode 100644 index 0000000..0d5a8d5 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/fills-an-empty-namespace.yaml @@ -0,0 +1,14 @@ +name: fills-an-empty-namespace +description: a modelsphere ModelRoute with no namespace uses the service namespace +inputs: + namespace: ns + manifest: | + apiVersion: routing.modelsphere.dev/v1alpha1 + kind: ModelRoute + metadata: + name: qwen +expected: + routes: + - apiVersion: routing.modelsphere.dev/v1alpha1 + namespace: ns + name: qwen diff --git a/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/ignores-other-documents.yaml b/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/ignores-other-documents.yaml new file mode 100644 index 0000000..12c105e --- /dev/null +++ b/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/ignores-other-documents.yaml @@ -0,0 +1,21 @@ +name: ignores-other-documents +description: other kinds and groups are ignored +inputs: + namespace: ns + manifest: | + apiVersion: v1 + kind: ConfigMap + metadata: + name: cm + --- + apiVersion: apps/v1 + kind: Deployment + metadata: + name: qwen-sglang + --- + apiVersion: example.com/v1 + kind: ModelRoute + metadata: + name: foreign +expected: + routes: [] diff --git a/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/keeps-a-gpucluster-namespace.yaml b/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/keeps-a-gpucluster-namespace.yaml new file mode 100644 index 0000000..80b1e0c --- /dev/null +++ b/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/keeps-a-gpucluster-namespace.yaml @@ -0,0 +1,15 @@ +name: keeps-a-gpucluster-namespace +description: a gpucluster ModelRoute keeps the namespace from the manifest +inputs: + namespace: ns + manifest: | + apiVersion: routing.gpucluster.io/v1alpha1 + kind: ModelRoute + metadata: + name: legacy + namespace: other +expected: + routes: + - apiVersion: routing.gpucluster.io/v1alpha1 + namespace: other + name: legacy diff --git a/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/rejects-a-document-that-does-not-decode.yaml b/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/rejects-a-document-that-does-not-decode.yaml new file mode 100644 index 0000000..be472c7 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/rejects-a-document-that-does-not-decode.yaml @@ -0,0 +1,9 @@ +name: rejects-a-document-that-does-not-decode +description: a document that does not decode is an error +inputs: + namespace: ns + manifest: | + apiVersion: v1 + kind: [broken +expected: + error: true diff --git a/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/rejects-a-route-without-a-name.yaml b/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/rejects-a-route-without-a-name.yaml new file mode 100644 index 0000000..76bb088 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/rejects-a-route-without-a-name.yaml @@ -0,0 +1,10 @@ +name: rejects-a-route-without-a-name +description: a ModelRoute without a name is an error +inputs: + namespace: ns + manifest: | + apiVersion: routing.modelsphere.dev/v1alpha1 + kind: ModelRoute + metadata: {} +expected: + error: true diff --git a/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/selects-both-routing-groups.yaml b/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/selects-both-routing-groups.yaml new file mode 100644 index 0000000..a0b2e9b --- /dev/null +++ b/internal/controller/serving/testdata/helpers/modelRoutesFromManifest/selects-both-routing-groups.yaml @@ -0,0 +1,30 @@ +name: selects-both-routing-groups +description: both routing groups are selected around blank documents +inputs: + namespace: ns + manifest: | + --- + apiVersion: v1 + kind: Service + metadata: + name: qwen + --- + apiVersion: routing.modelsphere.dev/v1alpha1 + kind: ModelRoute + metadata: + name: a + --- + --- + apiVersion: routing.gpucluster.io/v1alpha1 + kind: ModelRoute + metadata: + name: b + namespace: other +expected: + routes: + - apiVersion: routing.modelsphere.dev/v1alpha1 + namespace: ns + name: a + - apiVersion: routing.gpucluster.io/v1alpha1 + namespace: other + name: b diff --git a/internal/controller/serving/testdata/helpers/nextStage/empty-with-a-route.yaml b/internal/controller/serving/testdata/helpers/nextStage/empty-with-a-route.yaml new file mode 100644 index 0000000..167de35 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/nextStage/empty-with-a-route.yaml @@ -0,0 +1,6 @@ +name: empty-with-a-route +description: a manifest with a ModelRoute starts at RouteRemoving +inputs: + hasRoute: true +expected: + stage: RouteRemoving diff --git a/internal/controller/serving/testdata/helpers/nextStage/empty-without-a-route.yaml b/internal/controller/serving/testdata/helpers/nextStage/empty-without-a-route.yaml new file mode 100644 index 0000000..e1c0759 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/nextStage/empty-without-a-route.yaml @@ -0,0 +1,5 @@ +name: empty-without-a-route +description: a manifest without a ModelRoute starts at Uninstalling +inputs: {} +expected: + stage: Uninstalling diff --git a/internal/controller/serving/testdata/helpers/nextStage/force-deleting-releases-when-pods-are-gone.yaml b/internal/controller/serving/testdata/helpers/nextStage/force-deleting-releases-when-pods-are-gone.yaml new file mode 100644 index 0000000..c3f7cf9 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/nextStage/force-deleting-releases-when-pods-are-gone.yaml @@ -0,0 +1,7 @@ +name: force-deleting-releases-when-pods-are-gone +description: ForceDeleting releases when no pods remain +inputs: + stage: ForceDeleting + forceDue: true +expected: + stage: Released diff --git a/internal/controller/serving/testdata/helpers/nextStage/force-deleting-stays.yaml b/internal/controller/serving/testdata/helpers/nextStage/force-deleting-stays.yaml new file mode 100644 index 0000000..3bdc0f5 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/nextStage/force-deleting-stays.yaml @@ -0,0 +1,8 @@ +name: force-deleting-stays +description: ForceDeleting stays while pods remain inside the deadline +inputs: + stage: ForceDeleting + podsRemaining: 1 + forceDue: true +expected: + stage: ForceDeleting diff --git a/internal/controller/serving/testdata/helpers/nextStage/pods-force-due-moves-to-force-deleting.yaml b/internal/controller/serving/testdata/helpers/nextStage/pods-force-due-moves-to-force-deleting.yaml new file mode 100644 index 0000000..a1f023b --- /dev/null +++ b/internal/controller/serving/testdata/helpers/nextStage/pods-force-due-moves-to-force-deleting.yaml @@ -0,0 +1,8 @@ +name: pods-force-due-moves-to-force-deleting +description: a due force-delete moves to ForceDeleting +inputs: + stage: PodsTerminating + podsRemaining: 1 + forceDue: true +expected: + stage: ForceDeleting diff --git a/internal/controller/serving/testdata/helpers/nextStage/pods-gone-releases.yaml b/internal/controller/serving/testdata/helpers/nextStage/pods-gone-releases.yaml new file mode 100644 index 0000000..1b21a02 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/nextStage/pods-gone-releases.yaml @@ -0,0 +1,6 @@ +name: pods-gone-releases +description: no remaining pods releases +inputs: + stage: PodsTerminating +expected: + stage: Released diff --git a/internal/controller/serving/testdata/helpers/nextStage/pods-past-the-deadline-releases.yaml b/internal/controller/serving/testdata/helpers/nextStage/pods-past-the-deadline-releases.yaml new file mode 100644 index 0000000..70c9af1 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/nextStage/pods-past-the-deadline-releases.yaml @@ -0,0 +1,9 @@ +name: pods-past-the-deadline-releases +description: pods still present past the deadline releases +inputs: + stage: PodsTerminating + podsRemaining: 2 + pastDeadline: true + forceDue: true +expected: + stage: Released diff --git a/internal/controller/serving/testdata/helpers/nextStage/pods-wait.yaml b/internal/controller/serving/testdata/helpers/nextStage/pods-wait.yaml new file mode 100644 index 0000000..9041743 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/nextStage/pods-wait.yaml @@ -0,0 +1,7 @@ +name: pods-wait +description: pods still present stay in PodsTerminating +inputs: + stage: PodsTerminating + podsRemaining: 1 +expected: + stage: PodsTerminating diff --git a/internal/controller/serving/testdata/helpers/nextStage/released-stays.yaml b/internal/controller/serving/testdata/helpers/nextStage/released-stays.yaml new file mode 100644 index 0000000..1839510 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/nextStage/released-stays.yaml @@ -0,0 +1,8 @@ +name: released-stays +description: Released does not move +inputs: + stage: Released + hasRoute: true + podsRemaining: 3 +expected: + stage: Released diff --git a/internal/controller/serving/testdata/helpers/nextStage/route-gone-and-settled-uninstalls.yaml b/internal/controller/serving/testdata/helpers/nextStage/route-gone-and-settled-uninstalls.yaml new file mode 100644 index 0000000..8c562cc --- /dev/null +++ b/internal/controller/serving/testdata/helpers/nextStage/route-gone-and-settled-uninstalls.yaml @@ -0,0 +1,9 @@ +name: route-gone-and-settled-uninstalls +description: a removed route whose settle wait is over uninstalls +inputs: + stage: RouteRemoving + hasRoute: true + routeGone: true + settleDone: true +expected: + stage: Uninstalling diff --git a/internal/controller/serving/testdata/helpers/nextStage/route-gone-starts-settling.yaml b/internal/controller/serving/testdata/helpers/nextStage/route-gone-starts-settling.yaml new file mode 100644 index 0000000..d19a08f --- /dev/null +++ b/internal/controller/serving/testdata/helpers/nextStage/route-gone-starts-settling.yaml @@ -0,0 +1,8 @@ +name: route-gone-starts-settling +description: a removed route waits for routeSettle +inputs: + stage: RouteRemoving + hasRoute: true + routeGone: true +expected: + stage: RouteSettling diff --git a/internal/controller/serving/testdata/helpers/nextStage/route-removing-past-the-deadline.yaml b/internal/controller/serving/testdata/helpers/nextStage/route-removing-past-the-deadline.yaml new file mode 100644 index 0000000..31ff98a --- /dev/null +++ b/internal/controller/serving/testdata/helpers/nextStage/route-removing-past-the-deadline.yaml @@ -0,0 +1,8 @@ +name: route-removing-past-the-deadline +description: a route still present past the deadline moves to Uninstalling +inputs: + stage: RouteRemoving + hasRoute: true + pastDeadline: true +expected: + stage: Uninstalling diff --git a/internal/controller/serving/testdata/helpers/nextStage/route-removing-waits.yaml b/internal/controller/serving/testdata/helpers/nextStage/route-removing-waits.yaml new file mode 100644 index 0000000..cfaf5db --- /dev/null +++ b/internal/controller/serving/testdata/helpers/nextStage/route-removing-waits.yaml @@ -0,0 +1,7 @@ +name: route-removing-waits +description: RouteRemoving waits while the route is still present +inputs: + stage: RouteRemoving + hasRoute: true +expected: + stage: RouteRemoving diff --git a/internal/controller/serving/testdata/helpers/nextStage/settling-done-uninstalls.yaml b/internal/controller/serving/testdata/helpers/nextStage/settling-done-uninstalls.yaml new file mode 100644 index 0000000..7114a67 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/nextStage/settling-done-uninstalls.yaml @@ -0,0 +1,7 @@ +name: settling-done-uninstalls +description: RouteSettling moves to Uninstalling when the wait is over +inputs: + stage: RouteSettling + settleDone: true +expected: + stage: Uninstalling diff --git a/internal/controller/serving/testdata/helpers/nextStage/settling-past-the-deadline-uninstalls.yaml b/internal/controller/serving/testdata/helpers/nextStage/settling-past-the-deadline-uninstalls.yaml new file mode 100644 index 0000000..5a1467d --- /dev/null +++ b/internal/controller/serving/testdata/helpers/nextStage/settling-past-the-deadline-uninstalls.yaml @@ -0,0 +1,7 @@ +name: settling-past-the-deadline-uninstalls +description: RouteSettling skips the rest of the wait past the deadline +inputs: + stage: RouteSettling + pastDeadline: true +expected: + stage: Uninstalling diff --git a/internal/controller/serving/testdata/helpers/nextStage/settling-waits.yaml b/internal/controller/serving/testdata/helpers/nextStage/settling-waits.yaml new file mode 100644 index 0000000..964dfa3 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/nextStage/settling-waits.yaml @@ -0,0 +1,6 @@ +name: settling-waits +description: RouteSettling stays until the wait is over +inputs: + stage: RouteSettling +expected: + stage: RouteSettling diff --git a/internal/controller/serving/testdata/helpers/nextStage/uninstalling-does-not-return-to-the-route.yaml b/internal/controller/serving/testdata/helpers/nextStage/uninstalling-does-not-return-to-the-route.yaml new file mode 100644 index 0000000..b25fd09 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/nextStage/uninstalling-does-not-return-to-the-route.yaml @@ -0,0 +1,8 @@ +name: uninstalling-does-not-return-to-the-route +description: Uninstalling does not re-enter the route stages +inputs: + stage: Uninstalling + hasRoute: true + routeGone: false +expected: + stage: Uninstalling diff --git a/internal/controller/serving/testdata/helpers/nextStage/uninstalling-moves-to-pods.yaml b/internal/controller/serving/testdata/helpers/nextStage/uninstalling-moves-to-pods.yaml new file mode 100644 index 0000000..ea39b10 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/nextStage/uninstalling-moves-to-pods.yaml @@ -0,0 +1,7 @@ +name: uninstalling-moves-to-pods +description: a recorded uninstall moves to PodsTerminating +inputs: + stage: Uninstalling + uninstalled: true +expected: + stage: PodsTerminating diff --git a/internal/controller/serving/testdata/helpers/nextStage/uninstalling-waits-for-helm.yaml b/internal/controller/serving/testdata/helpers/nextStage/uninstalling-waits-for-helm.yaml new file mode 100644 index 0000000..447f030 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/nextStage/uninstalling-waits-for-helm.yaml @@ -0,0 +1,7 @@ +name: uninstalling-waits-for-helm +description: Uninstalling stays until helm uninstall has been recorded +inputs: + stage: Uninstalling + hasRoute: true +expected: + stage: Uninstalling diff --git a/internal/controller/serving/testdata/helpers/podCounts/a-missing-node-past-grace-is-stuck.yaml b/internal/controller/serving/testdata/helpers/podCounts/a-missing-node-past-grace-is-stuck.yaml new file mode 100644 index 0000000..6bb31e4 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/podCounts/a-missing-node-past-grace-is-stuck.yaml @@ -0,0 +1,22 @@ +name: a-missing-node-past-grace-is-stuck +description: past grace a pod whose node is missing is stuck +inputs: + now: "2026-01-02T03:04:05Z" + nodesOK: true + nodes: + - name: other + ready: true + pods: + - name: engine + phase: Running + node: gpu-7 + grace: 30 + deletionTimestamp: "2026-01-02T03:03:00Z" +expected: + remaining: 0 + pods: + - name: engine + stuck: true + phase: Running + node: gpu-7 + grace: 30 diff --git a/internal/controller/serving/testdata/helpers/podCounts/a-pod-that-is-not-deleting-counts.yaml b/internal/controller/serving/testdata/helpers/podCounts/a-pod-that-is-not-deleting-counts.yaml new file mode 100644 index 0000000..8aecc00 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/podCounts/a-pod-that-is-not-deleting-counts.yaml @@ -0,0 +1,20 @@ +name: a-pod-that-is-not-deleting-counts +description: a pod with no deletionTimestamp counts and uses the default grace +inputs: + now: "2026-01-02T03:04:05Z" + nodesOK: true + nodes: + - name: gpu-7 + ready: false + pods: + - name: engine + phase: Running + node: gpu-7 +expected: + remaining: 1 + pods: + - name: engine + stuck: false + phase: Running + node: gpu-7 + grace: 30 diff --git a/internal/controller/serving/testdata/helpers/podCounts/an-unassigned-pod-past-grace-is-stuck.yaml b/internal/controller/serving/testdata/helpers/podCounts/an-unassigned-pod-past-grace-is-stuck.yaml new file mode 100644 index 0000000..7b07292 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/podCounts/an-unassigned-pod-past-grace-is-stuck.yaml @@ -0,0 +1,17 @@ +name: an-unassigned-pod-past-grace-is-stuck +description: past grace a pod with no node is stuck +inputs: + now: "2026-01-02T03:04:05Z" + nodesOK: true + pods: + - name: engine + phase: Pending + grace: 30 + deletionTimestamp: "2026-01-02T03:03:00Z" +expected: + remaining: 0 + pods: + - name: engine + stuck: true + phase: Pending + grace: 30 diff --git a/internal/controller/serving/testdata/helpers/podCounts/counts-within-grace-on-a-notready-node.yaml b/internal/controller/serving/testdata/helpers/podCounts/counts-within-grace-on-a-notready-node.yaml new file mode 100644 index 0000000..db45766 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/podCounts/counts-within-grace-on-a-notready-node.yaml @@ -0,0 +1,22 @@ +name: counts-within-grace-on-a-notready-node +description: within grace a pod counts even on a NotReady node +inputs: + now: "2026-01-02T03:04:05Z" + nodesOK: true + nodes: + - name: gpu-7 + ready: false + pods: + - name: engine + phase: Running + node: gpu-7 + grace: 30 + deletionTimestamp: "2026-01-02T03:03:50Z" +expected: + remaining: 1 + pods: + - name: engine + stuck: false + phase: Running + node: gpu-7 + grace: 30 diff --git a/internal/controller/serving/testdata/helpers/podCounts/ignores-succeeded-and-failed.yaml b/internal/controller/serving/testdata/helpers/podCounts/ignores-succeeded-and-failed.yaml new file mode 100644 index 0000000..7c11647 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/podCounts/ignores-succeeded-and-failed.yaml @@ -0,0 +1,18 @@ +name: ignores-succeeded-and-failed +description: Succeeded and Failed pods are ignored +inputs: + now: "2026-01-02T03:04:05Z" + nodesOK: true + nodes: + - name: gpu-7 + ready: true + pods: + - name: bad + phase: Failed + node: gpu-7 + - name: done + phase: Succeeded + node: gpu-7 +expected: + remaining: 0 + pods: [] diff --git a/internal/controller/serving/testdata/helpers/podCounts/past-grace-on-a-notready-node-is-stuck.yaml b/internal/controller/serving/testdata/helpers/podCounts/past-grace-on-a-notready-node-is-stuck.yaml new file mode 100644 index 0000000..7f5b887 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/podCounts/past-grace-on-a-notready-node-is-stuck.yaml @@ -0,0 +1,22 @@ +name: past-grace-on-a-notready-node-is-stuck +description: past grace a pod on a NotReady node is gone and stuck +inputs: + now: "2026-01-02T03:04:05Z" + nodesOK: true + nodes: + - name: gpu-7 + ready: false + pods: + - name: engine + phase: Running + node: gpu-7 + grace: 30 + deletionTimestamp: "2026-01-02T03:03:00Z" +expected: + remaining: 0 + pods: + - name: engine + stuck: true + phase: Running + node: gpu-7 + grace: 30 diff --git a/internal/controller/serving/testdata/helpers/podCounts/past-grace-on-a-ready-node-still-counts.yaml b/internal/controller/serving/testdata/helpers/podCounts/past-grace-on-a-ready-node-still-counts.yaml new file mode 100644 index 0000000..0f68ec2 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/podCounts/past-grace-on-a-ready-node-still-counts.yaml @@ -0,0 +1,22 @@ +name: past-grace-on-a-ready-node-still-counts +description: past grace a pod on a Ready node still counts until grace plus 5m +inputs: + now: "2026-01-02T03:04:05Z" + nodesOK: true + nodes: + - name: gpu-7 + ready: true + pods: + - name: engine + phase: Running + node: gpu-7 + grace: 30 + deletionTimestamp: "2026-01-02T03:03:00Z" +expected: + remaining: 1 + pods: + - name: engine + stuck: false + phase: Running + node: gpu-7 + grace: 30 diff --git a/internal/controller/serving/testdata/helpers/podCounts/past-grace-plus-5m-is-stuck.yaml b/internal/controller/serving/testdata/helpers/podCounts/past-grace-plus-5m-is-stuck.yaml new file mode 100644 index 0000000..10f7b4a --- /dev/null +++ b/internal/controller/serving/testdata/helpers/podCounts/past-grace-plus-5m-is-stuck.yaml @@ -0,0 +1,22 @@ +name: past-grace-plus-5m-is-stuck +description: a pod is gone after grace plus 5m +inputs: + now: "2026-01-02T03:10:00Z" + nodesOK: true + nodes: + - name: gpu-7 + ready: true + pods: + - name: engine + phase: Running + node: gpu-7 + grace: 30 + deletionTimestamp: "2026-01-02T03:04:00Z" +expected: + remaining: 0 + pods: + - name: engine + stuck: true + phase: Running + node: gpu-7 + grace: 30 diff --git a/internal/controller/serving/testdata/helpers/podCounts/unreadable-nodes-use-only-the-time-rule.yaml b/internal/controller/serving/testdata/helpers/podCounts/unreadable-nodes-use-only-the-time-rule.yaml new file mode 100644 index 0000000..0978b1a --- /dev/null +++ b/internal/controller/serving/testdata/helpers/podCounts/unreadable-nodes-use-only-the-time-rule.yaml @@ -0,0 +1,19 @@ +name: unreadable-nodes-use-only-the-time-rule +description: when nodes cannot be listed only the time rule applies +inputs: + now: "2026-01-02T03:04:05Z" + nodesOK: false + pods: + - name: engine + phase: Running + node: gpu-7 + grace: 30 + deletionTimestamp: "2026-01-02T03:03:00Z" +expected: + remaining: 1 + pods: + - name: engine + stuck: false + phase: Running + node: gpu-7 + grace: 30 diff --git a/internal/controller/serving/testdata/helpers/podCounts/waiting-message-names-stuck-pods.yaml b/internal/controller/serving/testdata/helpers/podCounts/waiting-message-names-stuck-pods.yaml new file mode 100644 index 0000000..4bac282 --- /dev/null +++ b/internal/controller/serving/testdata/helpers/podCounts/waiting-message-names-stuck-pods.yaml @@ -0,0 +1,57 @@ +name: waiting-message-names-stuck-pods +description: the wait message counts blockers and names stuck pods +inputs: + now: "2026-01-02T03:04:05Z" + nodesOK: true + nodes: + - name: gpu-7 + ready: false + pods: + - name: c + phase: Running + node: gpu-7 + - name: a + phase: Running + node: gpu-7 + grace: 30 + deletionTimestamp: "2026-01-02T03:03:00Z" + - name: b + phase: Running + node: gpu-7 + grace: 30 + deletionTimestamp: "2026-01-02T03:03:00Z" + - name: d + phase: Running + node: gpu-7 + - name: e + phase: Running + node: gpu-7 +expected: + remaining: 3 + message: waiting for 3 pods (2 stuck on NotReady node gpu-7) + pods: + - name: a + stuck: true + phase: Running + node: gpu-7 + grace: 30 + - name: b + stuck: true + phase: Running + node: gpu-7 + grace: 30 + - name: c + stuck: false + phase: Running + node: gpu-7 + grace: 30 + - name: d + stuck: false + phase: Running + node: gpu-7 + grace: 30 + - name: e + stuck: false + phase: Running + node: gpu-7 + grace: 30 diff --git a/internal/helm/helm.go b/internal/helm/helm.go index 3e58e2c..cd9e224 100644 --- a/internal/helm/helm.go +++ b/internal/helm/helm.go @@ -53,6 +53,9 @@ type Release struct { Labels map[string]string // Updated is the release info last-deployed time. Updated time.Time + // Manifest is the rendered manifest of the release. Deletion reads + // ModelRoute objects from it. + Manifest string } // ChartRef selects one exact chart version in a repository. @@ -275,6 +278,7 @@ func toRelease(rel release.Releaser) (*Release, error) { Revision: v1rel.Version, Config: v1rel.Config, Labels: maps.Clone(v1rel.Labels), + Manifest: v1rel.Manifest, } if v1rel.Info != nil { out.Status = string(v1rel.Info.Status) From c18d017fe2e2bea4bad6bc5a7a4bf0e1e2e014d8 Mon Sep 17 00:00:00 2001 From: huskcass <333684917+huskcass@users.noreply.github.com> Date: Fri, 23 Oct 2026 10:34:45 -0700 Subject: [PATCH 2/4] e2e, docs: cover staged deletion on kind The sglang-mock chart can render a ModelRoute and a single mock pod with a preStop and grace period. The e2e suite checks route-before-Deployment ordering, a 20s preStop, force delete of a pod that ignores SIGTERM, and RouteStuck. docs/llmservice.md describes the stages, knobs and status. --- docs/llmservice.md | 61 ++- .../sglang-mock/templates/deployment.yaml | 2 + .../templates/mock-deployment.yaml | 31 ++ .../sglang-mock/templates/modelroute.yaml | 9 + test/charts/sglang-mock/values.yaml | 20 + test/e2e/deletion_test.go | 407 ++++++++++++++++++ test/e2e/e2e_test.go | 2 + test/e2e/testdata/deletion-force-values.yaml | 15 + .../e2e/testdata/deletion-prestop-values.yaml | 15 + test/e2e/testdata/deletion-route-values.yaml | 15 + .../testdata/deletion-routestuck-values.yaml | 15 + test/e2e/testdata/modelroute-crd.yaml | 20 + 12 files changed, 604 insertions(+), 8 deletions(-) create mode 100644 test/charts/sglang-mock/templates/mock-deployment.yaml create mode 100644 test/charts/sglang-mock/templates/modelroute.yaml create mode 100644 test/e2e/deletion_test.go create mode 100644 test/e2e/testdata/deletion-force-values.yaml create mode 100644 test/e2e/testdata/deletion-prestop-values.yaml create mode 100644 test/e2e/testdata/deletion-route-values.yaml create mode 100644 test/e2e/testdata/deletion-routestuck-values.yaml create mode 100644 test/e2e/testdata/modelroute-crd.yaml diff --git a/docs/llmservice.md b/docs/llmservice.md index 5fd2c50..ec95789 100644 --- a/docs/llmservice.md +++ b/docs/llmservice.md @@ -25,6 +25,7 @@ | `layers` | []object | | — | Helm values documents, merged in order. Empty or unset leaves the chart defaults. See below. | | `model` | object | | — | Descriptive metadata. Not rendered into the chart. See below. | | `suspend` | bool | | `false` | Stop reconciling: no status write and no Helm call. Suspend wins over deletion: a suspended object that is deleting is not uninstalled and keeps its finalizer. | +| `deletion` | object | | — | Staged teardown budget. See [Staged deletion](#staged-deletion). | ### `spec.chart` @@ -60,7 +61,7 @@ All fields are optional strings. They are descriptive only: labels, printer colu | Field | Type | Description | | --- | --- | --- | -| `phase` | string | `Applying`, `Applied`, or `Failed`. The enum also allows `Pending`, which the controller does not set. | +| `phase` | string | `Applying`, `Applied`, `Failed`, or `Deleting`. The enum also allows `Pending`, which the controller does not set. | | `observedGeneration` | int64 | `metadata.generation` last reconciled to a terminal outcome. It stays at the previous generation while `Applying`. | | `conditions` | []metav1.Condition | `Applied` and `Adopted`. See [Conditions and events](#conditions-and-events). | | `message` | string | The error when `phase` is `Failed`; empty otherwise. | @@ -70,6 +71,7 @@ All fields are optional strings. They are descriptive only: labels, printer colu | `releaseUID` | string | UID of the LLMService that claimed the release. See [Ownership](#ownership). | | `helm.revision`, `helm.status` | int, string | The Helm release revision and status last observed. | | `history` | []object | Successful applies, newest first, capped by `--llmservice-history-limit`. See below. | +| `deletion` | object | Staged teardown while the object is deleting. See [Staged deletion](#staged-deletion). | ### `status.history[]` @@ -201,6 +203,9 @@ A layer with no `values`, or `values: null`, contributes nothing. | Annotation | Effect | | --- | --- | | `serving.modelsphere.dev/deletion-policy: Orphan` | On delete, drop the finalizer and do not uninstall. | +| `serving.modelsphere.dev/deletion-max-wait` | Overrides `spec.deletion.maxWait`. Read again on every pass. `0s` stops waiting. | +| `serving.modelsphere.dev/route-settle` | Overrides `spec.deletion.routeSettle`. `0s` skips the settle wait. | +| `serving.modelsphere.dev/force-delete-after` | Overrides `spec.deletion.forceDeleteAfter`. `0s` force-deletes a terminating pod on the next pass. | | `serving.modelsphere.dev/force-conflicts: ""` | Force field-manager conflicts on the apply for that generation only. Any other value is ignored. Recorded as `forceConflicts` in `status.history`. | | `serving.modelsphere.dev/re-resolve: ""` | Drop the pinned chart version once and resolve `spec.chart.version` to the newest version in range. See [Chart version](#chart-version). | | `swiss.modelsphere.dev/*` | Copied into each history entry and ControllerRevision. Otherwise ignored. | @@ -223,11 +228,7 @@ A chart-list, credentials, or Helm read error retries with backoff and does not ## Reconcile 1. `spec.suspend` is true: return without writing status or calling Helm. -2. The object is deleting: - - `serving.modelsphere.dev/deletion-policy: Orphan`: drop the finalizer and leave the release. - - No release: drop the finalizer. - - A release this object installed or adopted (see [Ownership](#ownership)): uninstall it, emit `Uninstalled`, drop the finalizer. - - Any other release: keep it, emit `KeptRelease`, drop the finalizer. +2. The object is deleting. See [Staged deletion](#staged-deletion). Orphan and a release this object never installed or adopted still skip that and drop the finalizer immediately. A kept release emits `KeptRelease`. 3. Add finalizer `serving.modelsphere.dev/finalizer`. 4. `spec.chart.repo` is not allowed by `--llmservice-allowed-chart-repos`: `phase: Failed`, `Applied=False` reason `ChartRepoNotAllowed`. Nothing is fetched or applied, and nothing is retried until the spec or annotations change. 5. Read the credentials Secret, then resolve `spec.chart.version` with the same rule as swiss `chart.Resolve`: an exact version; else the pinned `status.chart.version` while the range allows it, unless a new `serving.modelsphere.dev/re-resolve` value skips the pin; else the newest version the repository lists. An error sets `phase: Failed`, `Applied=False` reason `ChartResolveFailed`, and retries with backoff. @@ -245,6 +246,50 @@ A chart-list, credentials, or Helm read error retries with backoff and does not A successful install, upgrade, or adoption writes a ControllerRevision, prepends a `status.history` entry, and sets `status.chart`, `appliedHash`, `releaseUID`, `helm`, `phase: Applied`, and `observedGeneration`. +## Staged deletion + +An LLMService that installed or adopted its release keeps `serving.modelsphere.dev/finalizer` and tears the release down in order. Each pass requeues after 5 seconds and writes status before the step it records. The stage only moves forward, and a restart resumes from the recorded stage and timestamps. + +1. Delete every `ModelRoute` in the Helm release manifest (`routing.modelsphere.dev` or `routing.gpucluster.io`) and wait until it is NotFound. The operator does not remove the route finalizer. +2. Wait `routeSettle` so the gateway can drop the route. +3. Uninstall the Helm release (`Uninstalled`). +4. Wait until the release pods are gone. The selectors are `app.kubernetes.io/instance=` and `app=-`. Succeeded and Failed pods are ignored. A pod inside its grace period always counts. Past grace, a pod on a NotReady or missing node counts as gone (`stuck: true`). Any other pod counts as gone after grace plus 5 minutes. If the node list cannot be read, only the time rule applies. +5. Drop the finalizer. + +No ModelRoute in the manifest skips straight to uninstall with outcome `NoRoute`. Past `maxWait`, the finalizer is dropped. The outcome is `TimedOut` when pods are still present, unless it is already `RouteStuck`: the route was still there at the deadline, its finalizer stays, and the release is uninstalled anyway. `forceDeleteAfter`, when set, deletes a pod that has been terminating that long with `gracePeriodSeconds: 0` and a UID precondition. + +`spec.suspend` still wins: the finalizer stays and nothing is deleted. `serving.modelsphere.dev/deletion-policy: Orphan` skips every stage and leaves the release. A release this object never installed or adopted is kept, with `KeptRelease`, and its ModelRoute is not deleted. + +### Knobs + +Precedence is annotation, then `spec.deletion`, then the built-in default. Annotations are read on every pass, including ones added after the delete. `deadline` is `startedAt` plus the effective `maxWait`. An invalid annotation emits Warning `InvalidAnnotation` and falls back to the spec or the default. An explicit `0s`, in the spec or an annotation, is a real zero: `routeSettle: 0s` skips the settle wait and `maxWait: 0s` releases on the next pass. + +| Spec | Annotation | Default | Effect | +| --- | --- | --- | --- | +| `maxWait` | `serving.modelsphere.dev/deletion-max-wait` | `1h` | How long the finalizer may be held. Past it, remaining waits are skipped. | +| `routeSettle` | `serving.modelsphere.dev/route-settle` | `60s` | Wait after the ModelRoute is NotFound. `0s` skips it. | +| `forceDeleteAfter` | `serving.modelsphere.dev/force-delete-after` | unset (off) | Force-delete pods that have been terminating this long. `0s` does it on the next pass. | + +### `status.deletion` + +`status.phase` is `Deleting` while the finalizer is held. `status.message` stays the Helm error field; the human line is `status.deletion.message`. + +| Field | Meaning | +| --- | --- | +| `stage` | `RouteRemoving`, `RouteSettling`, `Uninstalling`, `PodsTerminating`, `ForceDeleting`, `Released`. | +| `outcome` | `Clean`, `NoRoute`, `RouteStuck`, `PodsForceDeleted`, `TimedOut`. | +| `startedAt` | When staged deletion began. | +| `deadline` | `startedAt` plus the effective `maxWait`, recomputed each pass. | +| `routeRemovedAt` | When every ModelRoute was first observed NotFound. | +| `uninstalledAt` | When the Helm uninstall succeeded. | +| `podsGoneAt` | When no release pods were left to wait for. | +| `podsRemaining` | How many pods still block the finalizer. | +| `pods` | Up to 20 pods still in the API, ignoring Succeeded and Failed. Each has `name`, `phase`, `node`, `deletionTimestamp`, `grace`, and `stuck`. | +| `forceDeleted` | Up to 20 pod names deleted with a zero grace period. | +| `message` | For example `waiting for 3 pods (2 stuck on NotReady node gpu-7)`. | + +Condition `Deleting` uses the same reasons as the events: `RouteRemoving`, `RouteSettling`, `RouteStuck`, `NoRoute`, `Uninstalled`, `WaitingForPods`, `PodsForceDeleted`, `TimedOut`, `Released`. `PodForceDeleted` is one Warning per pod. `InvalidAnnotation` is a Warning whenever an annotation value cannot be parsed. + ## Conditions and events | Reason | Condition | Phase | Retry | Event | @@ -261,7 +306,7 @@ A successful install, upgrade, or adoption writes a ControllerRevision, prepends | `OperationInProgress` | `Applied=False` | `Applying` | After the time remaining, at least 15s | — | | `RecoveredPendingRelease` | `Applied=False` | `Applying` | Applies in the same reconcile | `RecoveredPendingRelease` | -`Failed` means the layers could not be merged or hashed. Deletion emits `Uninstalled` or `KeptRelease`. +`Failed` means the layers could not be merged or hashed. Deletion also emits the reasons in [Staged deletion](#staged-deletion). `Uninstalled` and `KeptRelease` are unchanged. ## Revisions @@ -289,4 +334,4 @@ A ControllerRevision is never updated. A name that comes round again, from the s ## RBAC -The manager role may manage `llmservices`, their status and finalizers, `controllerrevisions`, Helm release Secrets, and `events.k8s.io` events. It also includes the rules the sglang and vllm charts render (secrets, configmaps, services, serviceaccounts, deployments, pod disruption budgets, roles, role bindings, leases, pods, servicemonitors, leaderworkersets, llmscalers, llmslorequirements, modelroutes) plus `namespaces` get, create, and patch. +The manager role may manage `llmservices`, their status and finalizers, `controllerrevisions`, Helm release Secrets, and `events.k8s.io` events. It also includes the rules the sglang and vllm charts render (secrets, configmaps, services, serviceaccounts, deployments, pod disruption budgets, roles, role bindings, leases, pods, servicemonitors, leaderworkersets, llmscalers, llmslorequirements, modelroutes) plus `namespaces` get, create, and patch. Pods include `delete`, used by force-delete. Nodes are `get`, `list`, and `watch`, used to tell a NotReady node from a slow pod. diff --git a/test/charts/sglang-mock/templates/deployment.yaml b/test/charts/sglang-mock/templates/deployment.yaml index faca298..9866d1d 100644 --- a/test/charts/sglang-mock/templates/deployment.yaml +++ b/test/charts/sglang-mock/templates/deployment.yaml @@ -1,3 +1,4 @@ +{{- if not .Values.mockPod.enabled }} {{/* Everything the shutdown does has to fit inside terminationGracePeriodSeconds. That timer starts when the pod is marked Terminating and covers BOTH the @@ -217,3 +218,4 @@ spec: tolerations: {{- toYaml . | nindent 8 }} {{- end }} +{{- end }} diff --git a/test/charts/sglang-mock/templates/mock-deployment.yaml b/test/charts/sglang-mock/templates/mock-deployment.yaml new file mode 100644 index 0000000..38ad3ca --- /dev/null +++ b/test/charts/sglang-mock/templates/mock-deployment.yaml @@ -0,0 +1,31 @@ +{{- if .Values.mockPod.enabled }} +apiVersion: apps/v1 +kind: Deployment +metadata: + name: {{ .Release.Name }}-sglang + labels: + app: {{ .Release.Name }}-sglang + app.kubernetes.io/instance: {{ .Release.Name }} +spec: + replicas: {{ .Values.replicaCount }} + selector: + matchLabels: + app: {{ .Release.Name }}-sglang + template: + metadata: + labels: + app: {{ .Release.Name }}-sglang + app.kubernetes.io/instance: {{ .Release.Name }} + spec: + terminationGracePeriodSeconds: {{ .Values.mockPod.terminationGracePeriodSeconds }} + containers: + - name: sglang + image: {{ .Values.mockPod.image | quote }} + command: ["sh", "-c", {{ .Values.mockPod.command | quote }}] + {{- with .Values.mockPod.preStop }} + lifecycle: + preStop: + exec: + command: ["sh", "-c", {{ . | quote }}] + {{- end }} +{{- end }} diff --git a/test/charts/sglang-mock/templates/modelroute.yaml b/test/charts/sglang-mock/templates/modelroute.yaml new file mode 100644 index 0000000..e217811 --- /dev/null +++ b/test/charts/sglang-mock/templates/modelroute.yaml @@ -0,0 +1,9 @@ +{{- if .Values.modelRoute.enabled }} +apiVersion: {{ .Values.modelRoute.apiVersion }} +kind: ModelRoute +metadata: + name: {{ .Values.modelRoute.name | default .Release.Name }} + finalizers: + - routing.modelsphere.dev/cleanup +spec: {} +{{- end }} diff --git a/test/charts/sglang-mock/values.yaml b/test/charts/sglang-mock/values.yaml index 6b7e974..c5e7a10 100644 --- a/test/charts/sglang-mock/values.yaml +++ b/test/charts/sglang-mock/values.yaml @@ -272,3 +272,23 @@ nodeSelector: {} tolerations: [] affinity: {} + +# Test stand-in for the engine Deployment. When enabled, templates/deployment.yaml +# is skipped and templates/mock-deployment.yaml runs a single container with a +# chosen command, preStop, and grace period. Leave this off for the SGLang shape. +mockPod: + enabled: false + image: "busybox:1.36" + # Shell command. The default stays up until the pod is deleted. + command: "sleep 3600" + # Shell command for a preStop hook. Empty disables the hook. + preStop: "" + terminationGracePeriodSeconds: 60 + +# Optional ModelRoute in the release, so staged deletion can remove the route +# before the Deployment. Leave this off unless that CRD is installed. +modelRoute: + enabled: false + # Empty uses the release name. + name: "" + apiVersion: routing.modelsphere.dev/v1alpha1 diff --git a/test/e2e/deletion_test.go b/test/e2e/deletion_test.go new file mode 100644 index 0000000..7d2eb9c --- /dev/null +++ b/test/e2e/deletion_test.go @@ -0,0 +1,407 @@ +//go:build e2e + +/* +Copyright 2026. + +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 e2e + +import ( + "encoding/json" + "os/exec" + "strings" + "sync" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/modelsphere/llm-operator/test/utils" +) + +const ( + deletionNamespace = "llmop-deletion-e2e" + routeFinalizer = "routing.modelsphere.dev/cleanup" + chartRepo = "oci://example.com/charts" +) + +// kubectlMu keeps utils.Run's process-wide chdir from racing the autoconfig poll. +var kubectlMu sync.Mutex + +func runCmd(cmd *exec.Cmd) (string, error) { + kubectlMu.Lock() + defer kubectlMu.Unlock() + return utils.Run(cmd) +} + +func kubectl(args ...string) (string, error) { + return runCmd(exec.Command("kubectl", args...)) +} + +type routeCleaner struct { + stopOnce sync.Once + stopCh chan struct{} + done chan struct{} +} + +func startRouteCleaner() *routeCleaner { + c := &routeCleaner{stopCh: make(chan struct{}), done: make(chan struct{})} + go func() { + defer close(c.done) + ticker := time.NewTicker(time.Second) + defer ticker.Stop() + for { + clearDeletingRouteFinalizers() + select { + case <-c.stopCh: + return + case <-ticker.C: + } + } + }() + return c +} + +func (c *routeCleaner) stop() { + if c == nil { + return + } + c.stopOnce.Do(func() { close(c.stopCh) }) + <-c.done +} + +func clearDeletingRouteFinalizers() { + out, err := kubectl("get", "modelroutes", "-n", deletionNamespace, "-o", "json") + if err != nil { + return + } + var list struct { + Items []struct { + Metadata struct { + Name string `json:"name"` + DeletionTimestamp *string `json:"deletionTimestamp"` + Finalizers []string `json:"finalizers"` + } `json:"metadata"` + } `json:"items"` + } + if json.Unmarshal([]byte(out), &list) != nil { + return + } + for _, item := range list.Items { + if item.Metadata.DeletionTimestamp == nil || len(item.Metadata.Finalizers) == 0 { + continue + } + kept := make([]string, 0, len(item.Metadata.Finalizers)) + for _, f := range item.Metadata.Finalizers { + if f != routeFinalizer { + kept = append(kept, f) + } + } + if len(kept) == len(item.Metadata.Finalizers) { + continue + } + body, err := json.Marshal(map[string]any{"metadata": map[string]any{"finalizers": kept}}) + if err != nil { + continue + } + _, _ = kubectl("patch", "modelroute", item.Metadata.Name, "-n", deletionNamespace, "--type=merge", "-p", string(body)) + } +} + +func registerDeletionTests() { + var ( + release string + cleaner *routeCleaner + ) + + Context("staged deletion", Ordered, func() { + BeforeAll(func() { + By("installing helm 4 when the CLI is missing") + _, err := runCmd(exec.Command("make", "install-helm")) + Expect(err).NotTo(HaveOccurred()) + + By("creating the deletion namespace without a restricted policy") + _, err = kubectl("get", "ns", deletionNamespace) + if err != nil { + _, err = kubectl("create", "ns", deletionNamespace) + Expect(err).NotTo(HaveOccurred()) + } + + By("installing the ModelRoute CRD") + _, err = kubectl("apply", "-f", "test/e2e/testdata/modelroute-crd.yaml") + Expect(err).NotTo(HaveOccurred()) + Eventually(func(g Gomega) { + out, err := kubectl("get", "crd", "modelroutes.routing.modelsphere.dev", + "-o", "jsonpath={.status.conditions[?(@.type=='Established')].status}") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(out).To(Equal("True")) + }, time.Minute, time.Second).Should(Succeed()) + }) + + AfterAll(func() { + _, _ = kubectl("delete", "ns", deletionNamespace, "--wait=false", "--ignore-not-found") + _, _ = kubectl("delete", "-f", "test/e2e/testdata/modelroute-crd.yaml", "--ignore-not-found", "--wait=false") + }) + + AfterEach(func() { + if cleaner != nil { + cleaner.stop() + cleaner = nil + } + if release == "" { + return + } + _, _ = kubectl("patch", "modelroute", release, "-n", deletionNamespace, "--type=json", + "-p", `[{"op":"remove","path":"/metadata/finalizers"}]`) + _, _ = runCmd(exec.Command("helm", "uninstall", release, "-n", deletionNamespace)) + _, _ = kubectl("delete", "llmsvc", release, "-n", deletionNamespace, "--ignore-not-found", "--wait=false") + _, _ = kubectl("delete", "modelroute", release, "-n", deletionNamespace, "--ignore-not-found", "--wait=false") + release = "" + }) + + It("removes the ModelRoute before the Deployment", func() { + release = "del-route" + cleaner = startRouteCleaner() + installChart(release, "test/e2e/testdata/deletion-route-values.yaml") + adoptDeletingService(release, map[string]string{ + "serving.modelsphere.dev/route-settle": "8s", + }) + + Eventually(func(g Gomega) { + g.Expect(modelRouteExists(release)).To(BeFalse()) + g.Expect(deploymentExists(release)).To(BeTrue()) + }, 2*time.Minute, time.Second).Should(Succeed()) + + Eventually(func(g Gomega) { + g.Expect(deploymentExists(release)).To(BeFalse()) + g.Expect(serviceExists(release)).To(BeFalse()) + }, 2*time.Minute, time.Second).Should(Succeed()) + }) + + It("keeps the service in PodsTerminating through a 20s preStop", func() { + release = "del-prestop" + installChart(release, "test/e2e/testdata/deletion-prestop-values.yaml") + waitPodRunning(release) + adoptDeletingService(release, nil) + + Eventually(func(g Gomega) { + g.Expect(servicePhase(release)).To(Equal("Deleting")) + g.Expect(deletionStage(release)).To(Equal("PodsTerminating")) + }, time.Minute, time.Second).Should(Succeed()) + + Consistently(func(g Gomega) { + g.Expect(serviceExists(release)).To(BeTrue()) + g.Expect(deletionStage(release)).To(Equal("PodsTerminating")) + }, 12*time.Second, time.Second).Should(Succeed()) + + Eventually(func(g Gomega) { + g.Expect(serviceExists(release)).To(BeFalse()) + }, 2*time.Minute, time.Second).Should(Succeed()) + }) + + It("force-deletes a pod that ignores SIGTERM", func() { + release = "del-force" + installChart(release, "test/e2e/testdata/deletion-force-values.yaml") + waitPodRunning(release) + adoptDeletingService(release, map[string]string{ + "serving.modelsphere.dev/force-delete-after": "5s", + }) + + Eventually(func(g Gomega) { + g.Expect(podNames(release)).To(BeEmpty()) + }, 30*time.Second, time.Second).Should(Succeed()) + Eventually(func(g Gomega) { + g.Expect(eventsContain(release, "PodForceDeleted")).To(BeTrue()) + }, time.Minute, time.Second).Should(Succeed()) + Eventually(func(g Gomega) { + g.Expect(serviceExists(release)).To(BeFalse()) + }, 2*time.Minute, time.Second).Should(Succeed()) + }) + + It("reaches RouteStuck when the route finalizer is never cleared", func() { + release = "del-stuck" + installChart(release, "test/e2e/testdata/deletion-routestuck-values.yaml") + adoptDeletingService(release, map[string]string{ + "serving.modelsphere.dev/deletion-max-wait": "15s", + }) + + seen := false + Eventually(func(g Gomega) { + outcome, exists := deletionOutcome(release) + if exists && outcome == "RouteStuck" { + seen = true + } + g.Expect(seen).To(BeTrue()) + }, 2*time.Minute, 200*time.Millisecond).Should(Succeed()) + + Eventually(func(g Gomega) { + g.Expect(serviceExists(release)).To(BeFalse()) + }, 2*time.Minute, time.Second).Should(Succeed()) + Eventually(func(g Gomega) { + g.Expect(modelRouteFinalizers(release)).To(ContainElement(routeFinalizer)) + }, time.Minute, time.Second).Should(Succeed()) + }) + }) +} + +func installChart(name, values string) { + By("installing " + name) + _, err := runCmd(exec.Command("helm", "upgrade", "--install", name, "test/charts/sglang-mock", + "-n", deletionNamespace, "-f", values)) + Expect(err).NotTo(HaveOccurred()) +} + +func adoptDeletingService(name string, annotations map[string]string) { + By("adopting " + name + " and deleting it while suspended") + meta := map[string]any{ + "name": name, + "namespace": deletionNamespace, + "finalizers": []string{"serving.modelsphere.dev/finalizer"}, + } + if len(annotations) > 0 { + meta["annotations"] = annotations + } + obj := map[string]any{ + "apiVersion": "serving.modelsphere.dev/v1alpha1", + "kind": "LLMService", + "metadata": meta, + "spec": map[string]any{ + "suspend": true, + "chart": map[string]any{ + "name": "sglang", + "repo": chartRepo, + "version": "0.1.0", + }, + }, + } + raw, err := json.Marshal(obj) + Expect(err).NotTo(HaveOccurred()) + cmd := exec.Command("kubectl", "apply", "-f", "-") + cmd.Stdin = strings.NewReader(string(raw)) + _, err = runCmd(cmd) + Expect(err).NotTo(HaveOccurred()) + + status := map[string]any{ + "status": map[string]any{ + "chart": map[string]any{"name": "sglang", "version": "0.1.0"}, + "history": []map[string]any{{ + "revision": 1, + "hash": "sha256:e2e", + "appliedAt": "2026-01-02T03:04:05Z", + "controllerRevision": "e2e", + }}, + }, + } + body, err := json.Marshal(status) + Expect(err).NotTo(HaveOccurred()) + _, err = kubectl("patch", "llmsvc", name, "-n", deletionNamespace, "--subresource=status", "--type=merge", "-p", string(body)) + Expect(err).NotTo(HaveOccurred()) + + _, err = kubectl("delete", "llmsvc", name, "-n", deletionNamespace, "--wait=false") + Expect(err).NotTo(HaveOccurred()) + _, err = kubectl("patch", "llmsvc", name, "-n", deletionNamespace, "--type=merge", "-p", `{"spec":{"suspend":false}}`) + Expect(err).NotTo(HaveOccurred()) +} + +func waitPodRunning(name string) { + Eventually(func(g Gomega) { + g.Expect(podPhase(name)).To(Equal("Running")) + }, 3*time.Minute, 2*time.Second).Should(Succeed()) +} + +func podPhase(name string) string { + out, err := kubectl("get", "pods", "-n", deletionNamespace, "-l", "app="+name+"-sglang", + "-o", "jsonpath={.items[0].status.phase}") + if err != nil { + return "" + } + return out +} + +func podNames(name string) string { + out, err := kubectl("get", "pods", "-n", deletionNamespace, "-l", "app="+name+"-sglang", + "-o", "jsonpath={.items[*].metadata.name}") + if err != nil { + return "error" + } + return strings.TrimSpace(out) +} + +func deploymentExists(name string) bool { + _, err := kubectl("get", "deploy", name+"-sglang", "-n", deletionNamespace) + return err == nil +} + +func modelRouteExists(name string) bool { + _, err := kubectl("get", "modelroute", name, "-n", deletionNamespace) + return err == nil +} + +func modelRouteFinalizers(name string) []string { + out, err := kubectl("get", "modelroute", name, "-n", deletionNamespace, "-o", "json") + if err != nil { + return nil + } + var obj struct { + Metadata struct { + Finalizers []string `json:"finalizers"` + } `json:"metadata"` + } + if json.Unmarshal([]byte(out), &obj) != nil { + return nil + } + return obj.Metadata.Finalizers +} + +func serviceExists(name string) bool { + _, exists := serviceField(name, "{.metadata.name}") + return exists +} + +func servicePhase(name string) string { + out, exists := serviceField(name, "{.status.phase}") + if !exists { + return "" + } + return out +} + +func deletionStage(name string) string { + out, exists := serviceField(name, "{.status.deletion.stage}") + if !exists { + return "" + } + return out +} + +func deletionOutcome(name string) (string, bool) { + return serviceField(name, "{.status.deletion.outcome}") +} + +func serviceField(name, path string) (string, bool) { + out, err := kubectl("get", "llmsvc", name, "-n", deletionNamespace, "-o", "jsonpath="+path) + if err != nil { + return "", false + } + return out, true +} + +func eventsContain(name, reason string) bool { + out, err := kubectl("get", "events", "-n", deletionNamespace, "--field-selector", "involvedObject.name="+name, "-o", "json") + if err != nil { + return false + } + return strings.Contains(out, reason) +} diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 636e456..41f592e 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -406,6 +406,8 @@ spec: }) }) + + registerDeletionTests() }) // serviceAccountToken returns a token for the specified service account in the given namespace. diff --git a/test/e2e/testdata/deletion-force-values.yaml b/test/e2e/testdata/deletion-force-values.yaml new file mode 100644 index 0000000..99998d6 --- /dev/null +++ b/test/e2e/testdata/deletion-force-values.yaml @@ -0,0 +1,15 @@ +replicaCount: 1 +scaler: + enabled: false +serviceMonitor: + enabled: false +modelCheck: + enabled: false +modelRoute: + enabled: false +mockPod: + enabled: true + image: busybox:1.36 + command: "trap '' TERM INT; while true; do sleep 1; done" + preStop: "" + terminationGracePeriodSeconds: 600 diff --git a/test/e2e/testdata/deletion-prestop-values.yaml b/test/e2e/testdata/deletion-prestop-values.yaml new file mode 100644 index 0000000..0884c6a --- /dev/null +++ b/test/e2e/testdata/deletion-prestop-values.yaml @@ -0,0 +1,15 @@ +replicaCount: 1 +scaler: + enabled: false +serviceMonitor: + enabled: false +modelCheck: + enabled: false +modelRoute: + enabled: false +mockPod: + enabled: true + image: busybox:1.36 + command: "sleep 3600" + preStop: "sleep 20" + terminationGracePeriodSeconds: 60 diff --git a/test/e2e/testdata/deletion-route-values.yaml b/test/e2e/testdata/deletion-route-values.yaml new file mode 100644 index 0000000..c72a56d --- /dev/null +++ b/test/e2e/testdata/deletion-route-values.yaml @@ -0,0 +1,15 @@ +replicaCount: 1 +scaler: + enabled: false +serviceMonitor: + enabled: false +modelCheck: + enabled: false +lifecycle: + preStop: + enabled: false +terminationGracePeriodSeconds: 60 +modelRoute: + enabled: true +mockPod: + enabled: false diff --git a/test/e2e/testdata/deletion-routestuck-values.yaml b/test/e2e/testdata/deletion-routestuck-values.yaml new file mode 100644 index 0000000..c72a56d --- /dev/null +++ b/test/e2e/testdata/deletion-routestuck-values.yaml @@ -0,0 +1,15 @@ +replicaCount: 1 +scaler: + enabled: false +serviceMonitor: + enabled: false +modelCheck: + enabled: false +lifecycle: + preStop: + enabled: false +terminationGracePeriodSeconds: 60 +modelRoute: + enabled: true +mockPod: + enabled: false diff --git a/test/e2e/testdata/modelroute-crd.yaml b/test/e2e/testdata/modelroute-crd.yaml new file mode 100644 index 0000000..26b7072 --- /dev/null +++ b/test/e2e/testdata/modelroute-crd.yaml @@ -0,0 +1,20 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: modelroutes.routing.modelsphere.dev +spec: + group: routing.modelsphere.dev + names: + kind: ModelRoute + listKind: ModelRouteList + plural: modelroutes + singular: modelroute + scope: Namespaced + versions: + - name: v1alpha1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + x-kubernetes-preserve-unknown-fields: true From 9c49f35a9110d441eb9c3928abebe036c7a24fa7 Mon Sep 17 00:00:00 2001 From: huskcass <333684917+huskcass@users.noreply.github.com> Date: Fri, 23 Oct 2026 10:50:28 -0700 Subject: [PATCH 3/4] test: split deletion step actions and wrap long e2e lines for lint --- .../serving/llmservice_cases_test.go | 20 +++++++++++++++---- test/e2e/deletion_test.go | 6 ++++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/internal/controller/serving/llmservice_cases_test.go b/internal/controller/serving/llmservice_cases_test.go index 5749059..14e9818 100644 --- a/internal/controller/serving/llmservice_cases_test.go +++ b/internal/controller/serving/llmservice_cases_test.go @@ -380,6 +380,20 @@ func (c *caseRun) step(i int, step serviceStep) { c.setStatus(step) case "setStatusReleaseUID": c.setStatusReleaseUID() + default: + if !c.deletionStep(step) { + Fail(fmt.Sprintf("%s: step %d unknown action %q", c.tc.Name, i+1, step.Action)) + } + } + if step.Expect != nil && step.Action != "reconcile" && step.Action != "reconcileHeld" { + c.check(step.Expect, reconcile.Result{}, nil, helmSnap{}, false) + } +} + +// deletionStep runs the staged-deletion step actions. It reports false for +// an action it does not know. +func (c *caseRun) deletionStep(step serviceStep) bool { + switch step.Action { case "addPod": c.addPod(step) case "removePod": @@ -397,11 +411,9 @@ func (c *caseRun) step(i int, step serviceStep) { case "freshReconciler": c.freshReconciler() default: - Fail(fmt.Sprintf("%s: step %d unknown action %q", c.tc.Name, i+1, step.Action)) - } - if step.Expect != nil && step.Action != "reconcile" && step.Action != "reconcileHeld" { - c.check(step.Expect, reconcile.Result{}, nil, helmSnap{}, false) + return false } + return true } func stepRequiresExpect(action string) bool { diff --git a/test/e2e/deletion_test.go b/test/e2e/deletion_test.go index 7d2eb9c..76ca87d 100644 --- a/test/e2e/deletion_test.go +++ b/test/e2e/deletion_test.go @@ -306,7 +306,8 @@ func adoptDeletingService(name string, annotations map[string]string) { } body, err := json.Marshal(status) Expect(err).NotTo(HaveOccurred()) - _, err = kubectl("patch", "llmsvc", name, "-n", deletionNamespace, "--subresource=status", "--type=merge", "-p", string(body)) + _, err = kubectl("patch", "llmsvc", name, "-n", deletionNamespace, + "--subresource=status", "--type=merge", "-p", string(body)) Expect(err).NotTo(HaveOccurred()) _, err = kubectl("delete", "llmsvc", name, "-n", deletionNamespace, "--wait=false") @@ -399,7 +400,8 @@ func serviceField(name, path string) (string, bool) { } func eventsContain(name, reason string) bool { - out, err := kubectl("get", "events", "-n", deletionNamespace, "--field-selector", "involvedObject.name="+name, "-o", "json") + out, err := kubectl("get", "events", "-n", deletionNamespace, + "--field-selector", "involvedObject.name="+name, "-o", "json") if err != nil { return false } From d08bba310bf1e266462fc516bd25fb9d426a9880 Mon Sep 17 00:00:00 2001 From: huskcass <333684917+huskcass@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:39:48 -0700 Subject: [PATCH 4/4] serving: end the route settle wait at the deletion deadline A routeSettle longer than maxWait held the finalizer past the budget. The settle now ends at the earlier of routeRemovedAt+routeSettle and the deadline, then uninstalls. The docs note that annotation changes during deletion apply on the next pass. --- docs/llmservice.md | 2 +- internal/controller/serving/deletion.go | 16 +++++-- ...n-maxwait-requeues-until-the-deadline.yaml | 46 +++++++++++++++++++ 3 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 internal/controller/serving/testdata/controller/route-settle-longer-than-maxwait-requeues-until-the-deadline.yaml diff --git a/docs/llmservice.md b/docs/llmservice.md index ec95789..b5a0035 100644 --- a/docs/llmservice.md +++ b/docs/llmservice.md @@ -262,7 +262,7 @@ No ModelRoute in the manifest skips straight to uninstall with outcome `NoRoute` ### Knobs -Precedence is annotation, then `spec.deletion`, then the built-in default. Annotations are read on every pass, including ones added after the delete. `deadline` is `startedAt` plus the effective `maxWait`. An invalid annotation emits Warning `InvalidAnnotation` and falls back to the spec or the default. An explicit `0s`, in the spec or an annotation, is a real zero: `routeSettle: 0s` skips the settle wait and `maxWait: 0s` releases on the next pass. +Precedence is annotation, then `spec.deletion`, then the built-in default. Annotations are read on every pass, including ones added after the delete. Annotation changes during deletion, such as lowering `serving.modelsphere.dev/deletion-max-wait`, setting `serving.modelsphere.dev/force-delete-after: 0s`, or adding `serving.modelsphere.dev/deletion-policy: Orphan`, take effect on the next pass. `deadline` is `startedAt` plus the effective `maxWait`. An invalid annotation emits Warning `InvalidAnnotation` and falls back to the spec or the default. An explicit `0s`, in the spec or an annotation, is a real zero: `routeSettle: 0s` skips the settle wait and `maxWait: 0s` releases on the next pass. | Spec | Annotation | Default | Effect | | --- | --- | --- | --- | diff --git a/internal/controller/serving/deletion.go b/internal/controller/serving/deletion.go index a15e9b7..d339736 100644 --- a/internal/controller/serving/deletion.go +++ b/internal/controller/serving/deletion.go @@ -249,8 +249,7 @@ func (r *LLMServiceReconciler) advanceRouteSettling(ctx context.Context, obj *se if err := r.putDeletion(ctx, obj, del, servingv1alpha1.ReasonRouteSettling, msgSettle, corev1.EventTypeNormal); err != nil { return ctrl.Result{}, err } - ready := del.RouteRemovedAt.Add(knobs.routeSettle) - return ctrl.Result{RequeueAfter: ready.Sub(now)}, nil + return ctrl.Result{RequeueAfter: settleReadyAt(del, knobs).Sub(now)}, nil } del.Stage = nextStage(stageInput{ Stage: servingv1alpha1.DeletionStageRouteSettling, @@ -260,11 +259,22 @@ func (r *LLMServiceReconciler) advanceRouteSettling(ctx context.Context, obj *se return ctrl.Result{}, nil } +// settleReadyAt is the end of the route-settle wait: the earlier of +// routeRemovedAt+routeSettle and the deletion deadline. A longer routeSettle +// must not hold the finalizer past maxWait. +func settleReadyAt(del *servingv1alpha1.DeletionStatus, knobs deletionKnobs) time.Time { + ready := del.RouteRemovedAt.Add(knobs.routeSettle) + if del.Deadline != nil && del.Deadline.Time.Before(ready) { + return del.Deadline.Time + } + return ready +} + func settleDone(del *servingv1alpha1.DeletionStatus, knobs deletionKnobs, now time.Time, past bool) bool { if past || del.RouteRemovedAt == nil { return past } - return !now.Before(del.RouteRemovedAt.Add(knobs.routeSettle)) + return !now.Before(settleReadyAt(del, knobs)) } func (r *LLMServiceReconciler) advanceUninstall(ctx context.Context, obj *servingv1alpha1.LLMService, del *servingv1alpha1.DeletionStatus, now time.Time) error { diff --git a/internal/controller/serving/testdata/controller/route-settle-longer-than-maxwait-requeues-until-the-deadline.yaml b/internal/controller/serving/testdata/controller/route-settle-longer-than-maxwait-requeues-until-the-deadline.yaml new file mode 100644 index 0000000..b65b8f6 --- /dev/null +++ b/internal/controller/serving/testdata/controller/route-settle-longer-than-maxwait-requeues-until-the-deadline.yaml @@ -0,0 +1,46 @@ +name: route-settle-longer-than-maxwait-requeues-until-the-deadline +description: routeSettle longer than maxWait requeues until the deadline, then uninstalls +steps: + - action: create + version: "0.8.6" + finalizers: + - serving.modelsphere.dev/finalizer + deletion: + maxWait: 1h + routeSettle: 2h + - action: putRelease + release: + owned: true + revision: 1 + status: deployed + manifest: | + apiVersion: routing.modelsphere.dev/v1alpha1 + kind: ModelRoute + metadata: + name: qwen + - action: addRoute + route: + name: qwen + - action: delete + - action: reconcile + expect: + uninstalls: 0 + phase: Deleting + finalizer: present + requeue: + exact: 1h + deletion: + stage: RouteSettling + routeRemovedAt: true + message: + equals: waiting for the ModelRoute to settle + route: + notFound: true + - action: advanceClock + advance: 1h + - action: reconcile + expect: + uninstalls: 1 + notFound: true + eventsContain: + - Uninstalled