diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 93f3037..812ef8e 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -825,14 +825,24 @@ paths: delete: tags: [releases] summary: Uninstall - description: Removes the release first, then its plan record, so a failure never leaves a live release untracked. + description: >- + Removes the release. Helm uninstalls and keeps the plan until pods + finish terminating. An llmsvc release returns 200 as soon as the + LLMService delete is accepted; the operator removes the route, + uninstalls the chart, and waits for pods. The release stays listed + with phase uninstalling, and deletion passes through the operator's + stage, until the LLMService is gone. operationId: uninstall parameters: - $ref: "#/components/parameters/namespace" - $ref: "#/components/parameters/release" responses: "200": - description: Uninstalled. + description: >- + Uninstalled, or output is "uninstall already in progress" when the + release is already uninstalling. An llmsvc release returns that + in-progress body as soon as the delete is accepted; the operator + finishes the teardown. content: application/json: schema: @@ -846,6 +856,11 @@ paths: "401": { $ref: "#/components/responses/Unauthorized" } "403": { $ref: "#/components/responses/Forbidden" } "404": { $ref: "#/components/responses/NotFound" } + "409": + description: An apply on this release is in progress. + content: + application/json: + schema: { $ref: "#/components/schemas/Error" } "500": { $ref: "#/components/responses/ServerError" } "502": { $ref: "#/components/responses/BadGateway" } @@ -1877,26 +1892,55 @@ components: Deployment: type: object - required: [release, namespace, revision, backend] + required: [release, namespace, backend] properties: release: { type: string } namespace: { type: string } chart: { type: string, description: "`name-version`." } status: { type: string, description: "helm's status." } - revision: { type: integer } + revision: { type: integer, description: "helm's revision. Omitted when there is no revision." } updated: { type: string, format: date-time } model: { type: string } variant: { type: string } catalogRef: { type: string } catalog: { type: string, description: "The configured catalog it came from; absent when the site no longer lists it." } version: { type: string } - phase: { type: string, description: "swiss's apply phase: applying, applied, failed (or the operator's)." } - drift: { type: string, description: "What a person should look at." } + phase: { type: string, description: "swiss's apply phase: applying, applied, failed, uninstalling (or the operator's). uninstalling is also set on an LLMService while metadata.deletionTimestamp is set or status.phase is Deleting." } + drift: { type: string, description: "What a person should look at. An uninstall in progress names the operator's stage when there is one." } + deletion: + $ref: "#/components/schemas/LLMDeletion" route: { type: string } backend: { $ref: "#/components/schemas/ApplyWith" } external: { type: boolean, description: "An LLMService swiss did not write." } cleanupPending: { type: boolean, description: "Both a plan ConfigMap and an LLMService exist." } + LLMDeletion: + type: object + description: Operator staged teardown of an LLMService. Swiss passes the fields through. + properties: + stage: { type: string, description: "RouteRemoving, RouteSettling, Uninstalling, PodsTerminating, ForceDeleting, or Released." } + outcome: { type: string, description: "Clean, NoRoute, RouteStuck, PodsForceDeleted, or TimedOut." } + podsRemaining: { type: integer } + deadline: { type: string, format: date-time } + startedAt: { type: string, format: date-time } + message: { type: string } + pods: + type: array + items: { $ref: "#/components/schemas/LLMDeletionPod" } + forceDeleted: + type: array + items: { type: string } + + LLMDeletionPod: + type: object + properties: + name: { type: string } + phase: { type: string } + node: { type: string } + deletionTimestamp: { type: string, format: date-time } + grace: { type: integer } + stuck: { type: boolean } + ReleaseStatus: type: object required: [release, namespace, exists, revision, pods, ready, total, authPrefix] @@ -1933,7 +1977,7 @@ components: description: Helm releases. The last apply as swiss recorded it. required: [phase] properties: - phase: { type: string, enum: [applying, applied, failed] } + phase: { type: string, enum: [applying, applied, failed, uninstalling] } action: { type: string } revision: { type: integer } startedAt: { type: string, format: date-time } @@ -1947,9 +1991,12 @@ components: type: object description: LLMService releases. The operator's status. properties: - phase: { type: string, examples: [Applying, Applied, Failed] } + phase: { type: string, description: "uninstalling while metadata.deletionTimestamp is set or status.phase is Deleting; otherwise the operator's phase.", examples: [Applying, Applied, Failed, uninstalling] } + operatorPhase: { type: string, description: "status.phase as the operator wrote it, including Deleting.", examples: [Applying, Applied, Failed, Deleting] } message: { type: string } revision: { type: integer } + deletion: + $ref: "#/components/schemas/LLMDeletion" conditions: type: array items: diff --git a/docs/swiss-design.md b/docs/swiss-design.md index c7821f8..67519b3 100644 --- a/docs/swiss-design.md +++ b/docs/swiss-design.md @@ -52,13 +52,54 @@ form / flags ─┘ (hash) ↓ | `POST /api/diff` | diff a stored plan against the live release | | `POST /api/apply` | upgrade, asserting the diffed revision when one is supplied | | `POST /api/install` | create | -| `DELETE /api/releases/{ns}/{release}` | uninstall | +| `DELETE /api/releases/{ns}/{release}` | uninstall; a helm release stays listed as `uninstalling` until helm has removed it and its pods are gone | | `POST /api/releases/{ns}/{release}/chat` | health check: one real inference request | | `GET /api/runs`, `GET /api/runs/{id}` | the operation log; output is per row, not in the list | | `GET /api/nodes` | the GPU inventory: type, allocatable, in use, and what holds it | | `GET /api/profile` | the site profile as parsed, after defaults | +## Uninstalling + +A helm release stays on `GET /api/deployments` with `phase: uninstalling` until helm has removed the release and its pods are gone. `DELETE` returns when helm does; it does not wait for those pods to finish terminating. + +| | | +| --- | --- | +| marker | plan ConfigMap `status.yaml` `phase: uninstalling`, plus label `swiss.modelsphere.dev/phase=uninstalling` so the metadata-only list can see it | +| memory | `namespace/release` on the process; `inflight` while that helm call has not returned, so a list cannot delete the plan underneath it | +| rebuild | the background loop at startup, and the first deployments list, fill that set from ConfigMaps carrying the label | +| drop | helm no longer has the release, and no pod matches `app=-` (when the plan names a chart) or `app.kubernetes.io/instance=` | +| check | every 5s from `Server.Run`, and again when the deployments list or that release's status is read | + +``` +DELETE + ├─ no plan ConfigMap ─► helm uninstall ─► delete the ConfigMap + └─ plan ConfigMap + write phase + label + helm uninstall + ├─ error ─► phase failed, clear the label, keep the ConfigMap + └─ ok ─► delete archive Secrets + helm release still recorded, or pods match either selector + ├─ yes ─► stay listed (phase uninstalling) + └─ no ─► delete the ConfigMap and drop the row +``` + +| situation | result | +| --- | --- | +| helm uninstall fails | `phase: failed` and the error; the plan stays; the row does not stay `uninstalling` | +| DELETE again, helm release already gone | 200, `output` is `uninstall already in progress` | +| DELETE again, helm release still recorded | helm uninstall runs again | +| install, upgrade, or rollback of that name | 409 `release / is uninstalling` | +| ConfigMap delete fails | the row stays; the next check retries | +| pod list fails, or reading the helm release fails | the row stays | +| helm release still recorded and no pods (restart mid-uninstall) | the row stays `uninstalling`; the next DELETE runs helm again | +| swissd restarts | the label is read back; the row stays until the helm release and its pods are both gone | + +An LLMService with `metadata.deletionTimestamp` set reports `phase: uninstalling` on the deployments row and `llmservice.phase` on status, whatever `status.phase` says. Install, upgrade, and rollback of that name are 409. `DELETE` of an LLMService still waits until the object is gone. + +`phase` gains the value `uninstalling` on a deployment row, on helm `planStatus.phase`, and on `llmservice.phase`. No other field on those objects changes. + + ## Catalog See [model-catalog](https://github.com/modelsphere/model-catalog). diff --git a/internal/autoscale/decide.go b/internal/autoscale/decide.go index cbb1571..9357502 100644 --- a/internal/autoscale/decide.go +++ b/internal/autoscale/decide.go @@ -26,6 +26,9 @@ const ( // HoldReleaseFailed is a helm or plan failure this loop did not record. // The tick holds instead of applying again. HoldReleaseFailed = "release failed" + // HoldUninstalling is a release on its way out: the plan is marked + // uninstalling, or the LLMService is being deleted. Nothing is applied. + HoldUninstalling = "uninstalling" backoffBase = time.Minute backoffCap = 30 * time.Minute @@ -73,6 +76,8 @@ type Live struct { Roles []Role `json:"roles,omitempty" yaml:"roles,omitempty"` Routers []Router `json:"routers,omitempty" yaml:"routers,omitempty"` LockBusy bool `json:"lockBusy,omitempty" yaml:"lockBusy,omitempty"` + // LLMDeleting is an LLMService with a deletionTimestamp or status.phase Deleting. + LLMDeleting bool `json:"llmDeleting,omitempty" yaml:"llmDeleting,omitempty"` } // Role is one DisaggregatedSet roleStatuses entry. @@ -265,6 +270,9 @@ func current(live Live) int { // rolloutHold is a change still landing. It waits in either direction. func rolloutHold(live Live) string { + if live.PlanPhase == "uninstalling" || live.LLMDeleting || live.LLMPhase == "Deleting" { + return HoldUninstalling + } if live.LockBusy { return HoldApplyInProgress } diff --git a/internal/autoscale/testdata/decide/guard-llm-deleting.yaml b/internal/autoscale/testdata/decide/guard-llm-deleting.yaml new file mode 100644 index 0000000..427a20c --- /dev/null +++ b/internal/autoscale/testdata/decide/guard-llm-deleting.yaml @@ -0,0 +1,23 @@ +name: guard-llm-deleting +description: an LLMService being deleted holds, even on a scale-down +config: + enabled: true + minSlices: 1 + maxSlices: 4 +live: + backend: llmsvc + llmPhase: Applied + llmGeneration: 1 + llmObservedGeneration: 1 + llmDeleting: true + pd: true + planReplicaCount: 3 +decision: + ok: true + replicas: 1 +wantHold: "uninstalling" +wantScale: false +wantCurrent: 3 +want: 0 +wantFailed: false +hasDecision: true diff --git a/internal/autoscale/testdata/decide/guard-llm-phase-deleting.yaml b/internal/autoscale/testdata/decide/guard-llm-phase-deleting.yaml new file mode 100644 index 0000000..62a6cef --- /dev/null +++ b/internal/autoscale/testdata/decide/guard-llm-phase-deleting.yaml @@ -0,0 +1,22 @@ +name: guard-llm-phase-deleting +description: status.phase Deleting holds, even when deletionTimestamp is not set +config: + enabled: true + minSlices: 1 + maxSlices: 4 +live: + backend: llmsvc + llmPhase: Deleting + llmGeneration: 1 + llmObservedGeneration: 1 + pd: true + planReplicaCount: 3 +decision: + ok: true + replicas: 1 +wantHold: "uninstalling" +wantScale: false +wantCurrent: 3 +want: 0 +wantFailed: false +hasDecision: true diff --git a/internal/autoscale/testdata/decide/guard-uninstalling.yaml b/internal/autoscale/testdata/decide/guard-uninstalling.yaml new file mode 100644 index 0000000..e64e65d --- /dev/null +++ b/internal/autoscale/testdata/decide/guard-uninstalling.yaml @@ -0,0 +1,22 @@ +name: guard-uninstalling +description: a plan marked uninstalling holds before any other guard +config: + enabled: true + minSlices: 1 + maxSlices: 4 +live: + backend: helm + helmStatus: deployed + planPhase: uninstalling + pd: true + planReplicaCount: 1 + lockBusy: true +decision: + ok: true + replicas: 2 +wantHold: "uninstalling" +wantScale: false +wantCurrent: 1 +want: 0 +wantFailed: false +hasDecision: true diff --git a/internal/cluster/kube.go b/internal/cluster/kube.go index 74cff1d..eab8875 100644 --- a/internal/cluster/kube.go +++ b/internal/cluster/kube.go @@ -62,6 +62,10 @@ type Kube struct { // PlanConfigMapPrefix names the ConfigMap holding a release's live plan. const PlanConfigMapPrefix = "swiss-plan-" +// LabelPhase is the plan ConfigMap label whose value is the release phase when +// that phase has to be visible without reading the ConfigMap body. +const LabelPhase = "swiss.modelsphere.dev/phase" + // PlanSecretPrefix names the Secrets holding previous plans, one per revision: // swiss.plan.v1..v, the shape helm uses for its own. const PlanSecretPrefix = "swiss.plan.v1." @@ -264,7 +268,7 @@ func (k *Kube) ManagedRefs(ctx context.Context) ([]ManagedRef, error) { if !ok || name == "" { continue } - out = append(out, ManagedRef{Namespace: item.Namespace, Name: name}) + out = append(out, ManagedRef{Namespace: item.Namespace, Name: name, Labels: maps.Clone(item.GetLabels())}) } } sort.Slice(out, func(i, j int) bool { @@ -615,16 +619,22 @@ var ( ) // PutConfigMap creates or replaces a ConfigMap given as "namespace/name". -func (k *Kube) PutConfigMap(ctx context.Context, ref string, data map[string]string) error { +func (k *Kube) PutConfigMap(ctx context.Context, ref string, data, labels map[string]string) error { ns, name, err := SplitRef(ref) if err != nil { return err } + labs := map[string]string{"app.kubernetes.io/managed-by": "swiss"} + for k, v := range labels { + if v != "" { + labs[k] = v + } + } cm := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns, Name: name, - Labels: map[string]string{"app.kubernetes.io/managed-by": "swiss"}, + Labels: labs, }, Data: data, } @@ -731,6 +741,18 @@ func (k *Kube) Pods(ctx context.Context, namespace, selector string) ([]Pod, err Phase: string(p.Status.Phase), Node: p.Spec.NodeName, } + if p.DeletionTimestamp != nil && !p.DeletionTimestamp.IsZero() { + t := p.DeletionTimestamp.Time + pod.DeletionTimestamp = &t + } + if p.DeletionGracePeriodSeconds != nil { + v := *p.DeletionGracePeriodSeconds + pod.DeletionGracePeriodSeconds = &v + } + if p.Spec.TerminationGracePeriodSeconds != nil { + v := *p.Spec.TerminationGracePeriodSeconds + pod.TerminationGracePeriodSeconds = &v + } if !p.CreationTimestamp.IsZero() { pod.AgeSecond = int64(time.Since(p.CreationTimestamp.Time).Seconds()) } diff --git a/internal/cluster/kube_test.go b/internal/cluster/kube_test.go index e06876d..15fb0a0 100644 --- a/internal/cluster/kube_test.go +++ b/internal/cluster/kube_test.go @@ -5,8 +5,11 @@ import ( "compress/gzip" "context" "encoding/base64" + "encoding/json" "fmt" + "strings" "testing" + "time" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -130,6 +133,65 @@ func TestAnUnreadableReleaseIsStillReported(t *testing.T) { } } +func TestPodsCarryDeletion(t *testing.T) { + grace := int64(45) + term := int64(90) + when := time.Date(2026, 10, 22, 12, 0, 0, 0, time.UTC) + ts := metav1.NewTime(when) + cs := fake.NewSimpleClientset( + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "models", Name: "engine-0", + Labels: map[string]string{"app": "glm"}, + DeletionTimestamp: &ts, + DeletionGracePeriodSeconds: &grace, + }, + Spec: corev1.PodSpec{ + NodeName: "gpu-1", + TerminationGracePeriodSeconds: &term, + }, + Status: corev1.PodStatus{Phase: corev1.PodRunning}, + }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "models", Name: "idle", + Labels: map[string]string{"app": "glm"}, + }, + Spec: corev1.PodSpec{NodeName: "gpu-2"}, + Status: corev1.PodStatus{Phase: corev1.PodRunning}, + }, + ) + got, err := NewKubeWithClient(cs).Pods(context.Background(), "models", "app=glm") + if err != nil { + t.Fatal(err) + } + byName := map[string]Pod{} + for _, p := range got { + byName[p.Name] = p + } + engine := byName["engine-0"] + if engine.Node != "gpu-1" || engine.DeletionTimestamp == nil || !engine.DeletionTimestamp.Equal(when) { + t.Fatalf("deletion timestamp: %+v", engine) + } + if engine.DeletionGracePeriodSeconds == nil || *engine.DeletionGracePeriodSeconds != 45 { + t.Fatalf("deletion grace: %+v", engine.DeletionGracePeriodSeconds) + } + if engine.TerminationGracePeriodSeconds == nil || *engine.TerminationGracePeriodSeconds != 90 { + t.Fatalf("termination grace: %+v", engine.TerminationGracePeriodSeconds) + } + idle := byName["idle"] + if idle.DeletionTimestamp != nil || idle.DeletionGracePeriodSeconds != nil || idle.TerminationGracePeriodSeconds != nil { + t.Fatalf("a running pod carries no deletion fields: %+v", idle) + } + raw, err := json.Marshal(idle) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(raw), "deletion") || strings.Contains(string(raw), "Grace") { + t.Fatalf("deletion fields leaked into pod JSON: %s", raw) + } +} + func TestNodesReadGPUFacts(t *testing.T) { cs := fake.NewSimpleClientset(&corev1.Node{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/cluster/managed_test.go b/internal/cluster/managed_test.go index 5413ee8..750d80b 100644 --- a/internal/cluster/managed_test.go +++ b/internal/cluster/managed_test.go @@ -65,6 +65,67 @@ func TestManagedRefsListsPlansAndNotTheProfile(t *testing.T) { } } +func TestManagedRefsReturnsThePhaseLabel(t *testing.T) { + md := metaClient(planMetaLabeled("models", PlanConfigMapPrefix+"glm-53", map[string]string{ + LabelPhase: "uninstalling", + })) + k := NewKubeWithClients(fake.NewSimpleClientset(), md) + + refs, err := k.ManagedRefs(context.Background()) + if err != nil { + t.Fatal(err) + } + if len(refs) != 1 || refs[0].Namespace != "models" || refs[0].Name != "glm-53" { + t.Fatalf("got %+v", refs) + } + if refs[0].Labels[LabelPhase] != "uninstalling" { + t.Fatalf("the phase label is what a restart reads, got %+v", refs[0].Labels) + } +} + +// PutConfigMap replaces the label set. A later write with no phase label has +// to drop the uninstalling marker, or a finished release stays in that set. +func TestPutConfigMapReplacesPhaseLabel(t *testing.T) { + cs := fake.NewSimpleClientset() + k := NewKubeWithClient(cs) + ctx := context.Background() + ref := "models/" + PlanConfigMapPrefix + "glm-53" + data := map[string]string{planKey: "source:\n model: glm\n"} + + if err := k.PutConfigMap(ctx, ref, data, map[string]string{LabelPhase: "uninstalling"}); err != nil { + t.Fatal(err) + } + cm, err := cs.CoreV1().ConfigMaps("models").Get(ctx, PlanConfigMapPrefix+"glm-53", metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + if cm.Labels["app.kubernetes.io/managed-by"] != "swiss" || cm.Labels[LabelPhase] != "uninstalling" { + t.Fatalf("labels %+v", cm.Labels) + } + + if err := k.PutConfigMap(ctx, ref, data, nil); err != nil { + t.Fatal(err) + } + cm, err = cs.CoreV1().ConfigMaps("models").Get(ctx, PlanConfigMapPrefix+"glm-53", metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + if cm.Labels["app.kubernetes.io/managed-by"] != "swiss" { + t.Fatalf("managed-by must stay, labels %+v", cm.Labels) + } + if _, ok := cm.Labels[LabelPhase]; ok { + t.Fatalf("nil labels must clear the phase marker, labels %+v", cm.Labels) + } +} + +func planMetaLabeled(namespace, name string, extra map[string]string) *metav1.PartialObjectMetadata { + o := planMeta(namespace, name) + for k, v := range extra { + o.Labels[k] = v + } + return o +} + func TestManagedRefsNeedsAMetadataClient(t *testing.T) { k := NewKubeWithClient(fake.NewSimpleClientset()) if _, err := k.ManagedRefs(context.Background()); err == nil { diff --git a/internal/cluster/probe.go b/internal/cluster/probe.go index 63680b4..7215e51 100644 --- a/internal/cluster/probe.go +++ b/internal/cluster/probe.go @@ -46,6 +46,10 @@ type Release struct { type ManagedRef struct { Namespace string Name string // the release name, not the ConfigMap name + // Labels are the plan ConfigMap's labels. Metadata only: the uninstalling + // marker is one of them, so a restart can find those releases without + // opening the values documents. + Labels map[string]string } // Node is what a fit check needs, plus what a node view shows. @@ -91,6 +95,10 @@ type GPUPod struct { // Pod is enough to tell loading from broken: on this workload a pod that is // scheduled and not ready has usually been reading weights for twenty minutes. +// +// The deletion fields are how an uninstall tells a pod that is still finishing +// from one that is stuck. They are not part of the status JSON, so a pod that +// is not being deleted looks the same as it did before they existed. type Pod struct { Name string `json:"name"` Phase string `json:"phase"` @@ -99,6 +107,10 @@ type Pod struct { Node string `json:"node,omitempty"` Message string `json:"message,omitempty"` AgeSecond int64 `json:"ageSeconds"` + + DeletionTimestamp *time.Time `json:"-"` + DeletionGracePeriodSeconds *int64 `json:"-"` + TerminationGracePeriodSeconds *int64 `json:"-"` } type Probe interface { @@ -447,7 +459,10 @@ func (f Fake) Secret(_ context.Context, ref string) (map[string]string, error) { // Writer is the write half, kept separate so a read-only swissd can hold a // Probe and nothing else. type Writer interface { - PutConfigMap(ctx context.Context, ref string, data map[string]string) error + // PutConfigMap creates or replaces a ConfigMap given as "namespace/name". + // labels are set in addition to app.kubernetes.io/managed-by=swiss, and the + // label set is replaced rather than merged: nil leaves only that one label. + PutConfigMap(ctx context.Context, ref string, data, labels map[string]string) error // DeleteConfigMap removes one, and succeeds when it is already gone. // Uninstall is a cleanup path: it has to be safe to run twice. DeleteConfigMap(ctx context.Context, ref string) error diff --git a/internal/llmsvc/client.go b/internal/llmsvc/client.go index 7d4896e..9266f1d 100644 --- a/internal/llmsvc/client.go +++ b/internal/llmsvc/client.go @@ -23,7 +23,7 @@ type Client struct { disco discovery.DiscoveryInterface kube kubernetes.Interface - // Interval is how often WaitApplied and WaitGone poll. Zero means one second. + // Interval is how often WaitApplied polls. Zero means one second. Interval time.Duration } @@ -144,18 +144,6 @@ func (c *Client) WaitApplied(ctx context.Context, ns, name string, generation in return got, nil } -// WaitGone returns once the object is NotFound. A finalizer keeps it visible -// until the operator drops it, so a successful Delete is not yet gone. -func (c *Client) WaitGone(ctx context.Context, ns, name string) error { - return c.poll(ctx, func() (bool, error) { - _, err := c.Get(ctx, ns, name) - if apierrors.IsNotFound(err) { - return true, nil - } - return false, err - }) -} - // ControllerRevisions reads history snapshots by name, in the order given. func (c *Client) ControllerRevisions(ctx context.Context, ns string, names []string) ([]*appsv1.ControllerRevision, error) { if c.kube == nil { diff --git a/internal/llmsvc/client_test.go b/internal/llmsvc/client_test.go index 70fe37b..91c7e26 100644 --- a/internal/llmsvc/client_test.go +++ b/internal/llmsvc/client_test.go @@ -224,30 +224,6 @@ func TestWaitAppliedTimesOut(t *testing.T) { } } -func TestWaitGone(t *testing.T) { - ctx, cancel := context.WithTimeout(t.Context(), 2*time.Second) - defer cancel() - c := New(newDyn(), nil, nil) - c.Interval = 5 * time.Millisecond - if err := c.WaitGone(ctx, "models", "missing"); err != nil { - t.Fatal(err) - } - if _, err := c.Create(ctx, obj("models", "qwen")); err != nil { - t.Fatal(err) - } - errc := make(chan error, 1) - go func() { - time.Sleep(30 * time.Millisecond) - errc <- c.Delete(ctx, "models", "qwen") - }() - if err := c.WaitGone(ctx, "models", "qwen"); err != nil { - t.Fatal(err) - } - if err := <-errc; err != nil { - t.Fatal(err) - } -} - func TestListHonoursNamespaceScope(t *testing.T) { ctx := t.Context() dyn := newDyn() diff --git a/internal/llmsvc/map_test.go b/internal/llmsvc/map_test.go index dbf05b6..f325c47 100644 --- a/internal/llmsvc/map_test.go +++ b/internal/llmsvc/map_test.go @@ -27,10 +27,10 @@ func TestWireContract(t *testing.T) { if GVR != (schema.GroupVersionResource{Group: "serving.modelsphere.dev", Version: "v1alpha1", Resource: "llmservices"}) { t.Fatalf("GVR %v", GVR) } - if PhasePending != "Pending" || PhaseApplying != "Applying" || PhaseApplied != "Applied" || PhaseFailed != "Failed" { + if PhasePending != "Pending" || PhaseApplying != "Applying" || PhaseApplied != "Applied" || PhaseFailed != "Failed" || PhaseDeleting != "Deleting" { t.Fatal("phases") } - if ConditionApplied != "Applied" || ConditionAdopted != "Adopted" { + if ConditionApplied != "Applied" || ConditionAdopted != "Adopted" || ConditionDeleting != "Deleting" { t.Fatal("conditions") } if AnnDeletionPolicy != "serving.modelsphere.dev/deletion-policy" || AnnForceConflicts != "serving.modelsphere.dev/force-conflicts" { @@ -365,6 +365,12 @@ func TestStatusRoundTripsThroughUnstructured(t *testing.T) { Annotations: map[string]string{AnnAction: "upgrade", AnnNote: "bump to 1.1.0"}, ForceConflicts: true, Action: "upgrade", }}, + Deletion: &DeletionStatus{ + Stage: "PodsTerminating", Outcome: "Clean", StartedAt: ts, Deadline: ts, + PodsRemaining: 2, Message: "waiting", + Pods: []DeletionPod{{Name: "qwen-0", Phase: "Running", Node: "gpu-1", Grace: 30, Stuck: true, DeletionTimestamp: ts}}, + ForceDeleted: []string{"qwen-old"}, + }, }, } in.APIVersion = APIVersion @@ -386,6 +392,9 @@ func TestStatusRoundTripsThroughUnstructured(t *testing.T) { `"appliedHash":"sha256:abc"`, `"suspend":true`, `"action":"upgrade"`, + `"stage":"PodsTerminating"`, + `"podsRemaining":2`, + `"forceDeleted":["qwen-old"]`, } { if !strings.Contains(s, needle) { t.Errorf("missing %s in %s", needle, s) @@ -427,6 +436,16 @@ func TestStatusRoundTripsThroughUnstructured(t *testing.T) { if !h.AppliedAt.Time.Equal(ts.Time) { t.Fatalf("appliedAt %s", h.AppliedAt) } + d := got.Status.Deletion + if d == nil || d.Stage != "PodsTerminating" || d.Outcome != "Clean" || d.PodsRemaining != 2 || d.Message != "waiting" { + t.Fatalf("deletion %+v", d) + } + if len(d.Pods) != 1 || d.Pods[0].Name != "qwen-0" || d.Pods[0].Grace != 30 || !d.Pods[0].Stuck || d.Pods[0].Node != "gpu-1" { + t.Fatalf("deletion pods %+v", d.Pods) + } + if len(d.ForceDeleted) != 1 || d.ForceDeleted[0] != "qwen-old" || !d.StartedAt.Time.Equal(ts.Time) || !d.Deadline.Time.Equal(ts.Time) { + t.Fatalf("deletion times %+v", d) + } } func realistic(t *testing.T) *plan.Plan { diff --git a/internal/llmsvc/types.go b/internal/llmsvc/types.go index c8eb652..ebb0b25 100644 --- a/internal/llmsvc/types.go +++ b/internal/llmsvc/types.go @@ -26,9 +26,11 @@ const ( PhaseApplying = "Applying" PhaseApplied = "Applied" PhaseFailed = "Failed" + PhaseDeleting = "Deleting" - ConditionApplied = "Applied" - ConditionAdopted = "Adopted" + ConditionApplied = "Applied" + ConditionAdopted = "Adopted" + ConditionDeleting = "Deleting" SwissAnnotationPrefix = "swiss.modelsphere.dev/" @@ -98,6 +100,35 @@ type Status struct { AppliedHash string `json:"appliedHash,omitempty"` Helm *HelmStatus `json:"helm,omitempty"` History []HistoryEntry `json:"history,omitempty"` + // Deletion is the operator's staged teardown. Swiss reports it and does + // not choose the stages. + Deletion *DeletionStatus `json:"deletion,omitempty"` +} + +// DeletionStatus is status.deletion while the operator is removing an +// LLMService. Times are omitempty; a zero time is not a real instant. +type DeletionStatus struct { + Stage string `json:"stage,omitempty"` + Outcome string `json:"outcome,omitempty"` + StartedAt metav1.Time `json:"startedAt,omitempty,omitzero"` + Deadline metav1.Time `json:"deadline,omitempty,omitzero"` + RouteRemovedAt metav1.Time `json:"routeRemovedAt,omitempty,omitzero"` + UninstalledAt metav1.Time `json:"uninstalledAt,omitempty,omitzero"` + PodsGoneAt metav1.Time `json:"podsGoneAt,omitempty,omitzero"` + PodsRemaining int `json:"podsRemaining,omitempty"` + Pods []DeletionPod `json:"pods,omitempty"` + ForceDeleted []string `json:"forceDeleted,omitempty"` + Message string `json:"message,omitempty"` +} + +// DeletionPod is one pod the operator still sees during teardown. +type DeletionPod struct { + Name string `json:"name,omitempty"` + Phase string `json:"phase,omitempty"` + Node string `json:"node,omitempty"` + DeletionTimestamp metav1.Time `json:"deletionTimestamp,omitempty,omitzero"` + Grace int `json:"grace,omitempty"` + Stuck bool `json:"stuck,omitempty"` } type ChartStatus struct { diff --git a/internal/server/api.go b/internal/server/api.go index 26c82d0..db0a5c9 100644 --- a/internal/server/api.go +++ b/internal/server/api.go @@ -455,7 +455,7 @@ func (s *Server) handleSaveProfile(w http.ResponseWriter, r *http.Request) { } data := map[string]string{s.cfg.Cluster.Profile.Key: body} - if err := s.writer.PutConfigMap(ctx, ref, data); err != nil { + if err := s.writer.PutConfigMap(ctx, ref, data, nil); err != nil { writeError(w, http.StatusBadGateway, err.Error()) return } @@ -563,11 +563,13 @@ func (s *Server) handleNodes(w http.ResponseWriter, r *http.Request) { } type deployment struct { - Release string `json:"release"` - Namespace string `json:"namespace"` - Chart string `json:"chart,omitempty"` - Status string `json:"status,omitempty"` - Revision int `json:"revision"` + Release string `json:"release"` + Namespace string `json:"namespace"` + Chart string `json:"chart,omitempty"` + Status string `json:"status,omitempty"` + // Revision is helm's. Omitted when there is none, so an uninstalling row + // whose release is already gone does not report 0. + Revision int `json:"revision,omitempty"` Updated string `json:"updated,omitempty"` Model string `json:"model,omitempty"` Variant string `json:"variant,omitempty"` @@ -584,9 +586,10 @@ type deployment struct { Route string `json:"route,omitempty"` // Backend is helm or llmsvc. External is an LLMService swiss did not // write. CleanupPending means both records still exist. - Backend string `json:"backend"` - External bool `json:"external,omitempty"` - CleanupPending bool `json:"cleanupPending,omitempty"` + Backend string `json:"backend"` + External bool `json:"external,omitempty"` + CleanupPending bool `json:"cleanupPending,omitempty"` + Deletion *llmDeletionView `json:"deletion,omitempty"` // What the plan recorded about its catalog, for finding the configured // catalog the release belongs to. @@ -674,10 +677,15 @@ func (s *Server) deploymentFromService(ctx context.Context, ref deployRef) *depl s.log.WarnContext(ctx, "llmservice does not decode", "namespace", u.GetNamespace(), "release", u.GetName(), "err", err) d.Drift = "plan unreadable: " + err.Error() + if llmDeleting(u, nil) { + d.Phase = phaseUninstalling + d.Drift = uninstallDrift("") + } return d } if obj.Status != nil { d.Phase = obj.Status.Phase + d.Deletion = deletionView(obj.Status.Deletion) if obj.Status.Helm != nil { d.Revision = int(obj.Status.Helm.Revision) d.Status = obj.Status.Helm.Status @@ -692,6 +700,12 @@ func (s *Server) deploymentFromService(ctx context.Context, ref deployRef) *depl d.Drift = "an apply is in progress" } } + // deletionTimestamp or status.phase Deleting wins: the object is still + // listed while the operator's finalizer keeps it, and that is the uninstall. + if llmDeleting(u, obj) { + d.Phase = phaseUninstalling + d.Drift = uninstallDrift(deletionStage(d.Deletion)) + } if d.Chart == "" { d.Chart = chartLabel(obj.Spec.Chart.Name, obj.Spec.Chart.Version) } @@ -700,6 +714,10 @@ func (s *Server) deploymentFromService(ctx context.Context, ref deployRef) *depl s.log.WarnContext(ctx, "llmservice plan does not decode", "namespace", u.GetNamespace(), "release", u.GetName(), "err", err) d.Drift = "plan unreadable: " + err.Error() + if llmDeleting(u, obj) { + d.Phase = phaseUninstalling + d.Drift = uninstallDrift(deletionStage(d.Deletion)) + } return d } d.Route = routeOf(p) @@ -725,6 +743,10 @@ func (s *Server) handleDeployments(w http.ResponseWriter, r *http.Request) { ctx, cancel := contextWithTimeout(r, 30*time.Second) defer cancel() + // A release whose pods have finished terminating drops out of this page + // rather than on the next poll interval. + s.reapUninstalling(ctx) + refs, err := s.deploymentRefs(ctx) if err != nil { writeError(w, http.StatusBadGateway, err.Error()) @@ -845,6 +867,8 @@ func (s *Server) deploymentRow(ctx context.Context, rel cluster.Release) *deploy d.Drift = "last apply failed: " + st.Error case phaseApplying: d.Drift = "an apply was started and never completed" + case phaseUninstalling: + d.Drift = "uninstall in progress" } } // The route is read off the composed values rather than the summary: it is a diff --git a/internal/server/autoscale_api_test.go b/internal/server/autoscale_api_test.go index 683cc9d..e2d00df 100644 --- a/internal/server/autoscale_api_test.go +++ b/internal/server/autoscale_api_test.go @@ -54,8 +54,8 @@ type mirrorWriter struct { mapLabels map[string]map[string]string } -func (m *mirrorWriter) PutConfigMap(ctx context.Context, ref string, data map[string]string) error { - if err := m.fakeWriter.PutConfigMap(ctx, ref, data); err != nil { +func (m *mirrorWriter) PutConfigMap(ctx context.Context, ref string, data, labels map[string]string) error { + if err := m.fakeWriter.PutConfigMap(ctx, ref, data, labels); err != nil { return err } m.remember(ref, m.written[ref], nil) @@ -174,6 +174,7 @@ func runUninstallCase(t *testing.T, tc apiCase) { ref := autoscaleRef(ns, release) var srv *httptest.Server var w *fakeWriter + var rig *llmRig if tc.Backend == backendLLMSVC { probe := llmProbe() probe.MapLabels = map[string]map[string]string{} @@ -181,7 +182,7 @@ func runUninstallCase(t *testing.T, tc apiCase) { probe.DSs = map[string]cluster.DisaggregatedSet{} probe.Maps[ref] = map[string]string{"config.yaml": "enabled: true\nminSlices: 1\nmaxSlices: 2\n"} probe.MapLabels[ref] = map[string]string{"owner": "swiss", "name": release} - rig := startRig(t, probe, true, false, "") + rig = startRig(t, probe, true, false, "") p, _ := pdPlan(t, ns, release, release, 1, true, false, "v2") u, err := llmsvc.FromPlan(p, "install", "seed") if err != nil { @@ -207,6 +208,11 @@ func runUninstallCase(t *testing.T, tc apiCase) { if code != http.StatusOK { t.Fatalf("status %d: %v", code, out) } + // The request returns once the delete is accepted. The autoscale ConfigMap + // goes when the reap sees the LLMService is gone. + if rig != nil { + rig.s.reapUninstalling(context.Background()) + } found := false for _, d := range w.deleted { if d == ref { diff --git a/internal/server/autoscale_loop.go b/internal/server/autoscale_loop.go index 0602e84..20efbca 100644 --- a/internal/server/autoscale_loop.go +++ b/internal/server/autoscale_loop.go @@ -223,17 +223,21 @@ func (s *Server) queryDecision(ctx context.Context, ns, serviceID string) autosc func (s *Server) autoscaleLive(ctx context.Context, ns, release, backend string, p *plan.Plan) (autoscale.Live, int, error) { live := autoscale.Live{Backend: backend, PD: disaggregated(p), PlanReplicaCount: planReplicaCount(p)} - if !s.tryApplyLock(ns, release) { - live.LockBusy = true - } else { - s.unlockApply(ns, release) - } - if data, err := s.probe.ConfigMap(ctx, planRef(ns, release)); err != nil && !apierrors.IsNotFound(err) { return live, 0, err } else if err == nil { live.PlanPhase = parseStatus([]byte(data["status.yaml"])).Phase } + // An uninstalling release holds in Decide. Its lock may be the + // uninstall's, and a probe would only contend with a repeat DELETE. + if live.PlanPhase == phaseUninstalling { + return live, 0, nil + } + if !s.tryApplyLock(ns, release) { + live.LockBusy = true + } else { + s.unlockApply(ns, release) + } rev := 0 switch backend { @@ -265,6 +269,12 @@ func (s *Server) autoscaleLive(ctx context.Context, ns, release, backend string, rev = int(obj.Status.Helm.Revision) } } + // A deletionTimestamp or status.phase Deleting is the operator tearing + // the release down. Probing workloads would only race that. + if !u.GetDeletionTimestamp().IsZero() || live.LLMPhase == llmsvc.PhaseDeleting { + live.LLMDeleting = true + return live, 0, nil + } } name := fullnameOf(p, release) @@ -359,6 +369,11 @@ func (s *Server) scaleTo(ctx context.Context, ns, release string, from, want, ex Action: "autoscale", Note: fmt.Sprintf("autoscale %d -> %d (decision-gen)", from, want), }) + // An uninstall that started after Decide stops the tick the same way: + // applyLocked refused it under the lock, before anything was applied. + if errors.Is(err, errUninstalling) { + return 0, errAutoscaleInactive + } if err != nil { return 0, err } diff --git a/internal/server/autoscale_loop_test.go b/internal/server/autoscale_loop_test.go index 7490466..ed822f2 100644 --- a/internal/server/autoscale_loop_test.go +++ b/internal/server/autoscale_loop_test.go @@ -18,6 +18,7 @@ import ( "github.com/modelsphere/swiss/internal/llmsvc" "github.com/modelsphere/swiss/internal/store" "github.com/modelsphere/swiss/internal/testutil/cases" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" k8stesting "k8s.io/client-go/testing" @@ -46,6 +47,13 @@ type loopCase struct { WantRetryDelta string `json:"wantRetryDelta,omitempty"` WantLastFailedWant *int `json:"wantLastFailedWant,omitempty"` WantCleared bool `json:"wantCleared,omitempty"` + // PlanPhase seeds status.yaml in the helm plan ConfigMap. + PlanPhase string `json:"planPhase,omitempty"` + // Deleting gives the LLMService a deletionTimestamp. + Deleting bool `json:"deleting,omitempty"` + // LLMPhase overrides status.phase. Deleting holds without a deletionTimestamp, + // and the rig operator stays off so it does not rewrite that phase. + LLMPhase string `json:"llmPhase,omitempty"` } func TestAutoscaleLoop(t *testing.T) { @@ -80,6 +88,9 @@ func runHelmLoop(t *testing.T, tc loopCase) { } probe.Maps[autoscaleRef(ns, release)][autoscaleStatusKey] = string(b) } + if tc.PlanPhase != "" { + seed[statusFile] = "phase: " + tc.PlanPhase + "\naction: uninstall\nrevision: 2\n" + } probe.Maps[planRef(ns, release)] = seed rev := tc.Revision if rev < 1 { @@ -150,7 +161,11 @@ func runLLMLoop(t *testing.T, tc loopCase) { probe.SecretLabels = map[string]map[string]string{} seedAutoscaleCM(&probe, ns, release, true, tc.Min, tc.Max) - rig := startRig(t, probe, true, !tc.Conflict, "") + phase := llmsvc.PhaseApplied + if tc.LLMPhase != "" { + phase = tc.LLMPhase + } + rig := startRig(t, probe, true, !tc.Conflict && phase != llmsvc.PhaseDeleting, "") u, err := llmsvc.FromPlan(p, "install", "seed") if err != nil { t.Fatal(err) @@ -161,12 +176,16 @@ func runLLMLoop(t *testing.T, tc loopCase) { } if err := unstructured.SetNestedMap(u.Object, map[string]any{ "observedGeneration": int64(1), - "phase": llmsvc.PhaseApplied, + "phase": phase, "helm": map[string]any{"revision": int64(rev), "status": "deployed"}, }, "status"); err != nil { t.Fatal(err) } u.SetGeneration(1) + if tc.Deleting { + ts := metav1.Now() + u.SetDeletionTimestamp(&ts) + } if _, err := rig.c.Create(t.Context(), u); err != nil { t.Fatal(err) } @@ -178,7 +197,7 @@ func runLLMLoop(t *testing.T, tc loopCase) { if err != nil { t.Fatal(err) } - if obj.Status == nil || obj.Status.Phase != llmsvc.PhaseApplied || obj.Status.Helm == nil || obj.Status.Helm.Revision != int64(rev) { + if obj.Status == nil || obj.Status.Phase != phase || obj.Status.Helm == nil || obj.Status.Helm.Revision != int64(rev) { t.Fatalf("seed status did not stick: %+v gen=%d", obj.Status, seeded.GetGeneration()) } if obj.Status.ObservedGeneration < seeded.GetGeneration() { @@ -224,8 +243,10 @@ func runLLMLoop(t *testing.T, tc loopCase) { t.Fatal(err) } if tc.Conflict { - if gotGets != 7 { - t.Errorf("Get calls %d, want 7", gotGets) + // One more than the loop alone: applyLocked reads the LLMService + // once to refuse a release that is being deleted. + if gotGets != 8 { + t.Errorf("Get calls %d, want 8", gotGets) } if st.LastError == "" || !strings.Contains(st.LastError, tc.ErrorContains) { t.Fatalf("lastError %q, want substring %q (gets %d)", st.LastError, tc.ErrorContains, gets.Load()) @@ -236,6 +257,19 @@ func runLLMLoop(t *testing.T, tc loopCase) { assertLoopBackoff(t, st, tc) return } + if tc.WantHold != "" { + if st.HoldReason != tc.WantHold { + t.Fatalf("hold %q, want %q", st.HoldReason, tc.WantHold) + } + if st.LastError != "" { + t.Fatalf("hold recorded error %q", st.LastError) + } + if got.GetAnnotations()[llmsvc.AnnAction] == "autoscale" { + t.Fatalf("held tick updated the object: %v", got.GetAnnotations()) + } + assertLoopBackoff(t, st, tc) + return + } if got.GetAnnotations()[llmsvc.AnnAction] != "autoscale" { t.Fatalf("action %q", got.GetAnnotations()[llmsvc.AnnAction]) } diff --git a/internal/server/backend_helm.go b/internal/server/backend_helm.go index 4598148..7380371 100644 --- a/internal/server/backend_helm.go +++ b/internal/server/backend_helm.go @@ -165,12 +165,31 @@ func (b helmBackend) Apply(ctx context.Context, p *plan.Plan, mode exec.Mode, o func (b helmBackend) Uninstall(ctx context.Context, ns, release string) (uninstallResult, error) { s := b.s + if s.uninstallInflight(ns, release) { + return uninstallResult{Output: uninstallInProgress}, nil + } st, err := exec.Lookup(ctx, s.probe, ns, release) if err != nil { return uninstallResult{}, failErr(http.StatusBadGateway, err) } - if err := exec.CheckUninstall(st, ns, release); err != nil { - return uninstallResult{}, failErr(http.StatusNotFound, err) + // Already marked, and helm has deleted the release: pods may still be + // terminating. A second DELETE does not run helm again. Archive Secrets + // are removed here too: the first DELETE may have stopped before it + // reached them. + phase := s.planPhase(ctx, ns, release) + if phase == phaseUninstalling && !st.Exists { + s.trackUninstalling(ns, release) + var planErr string + if s.writer != nil { + s.deleteArchives(ctx, ns, release) + planErr = s.reapRelease(ctx, ns, release) + } + return uninstallResult{Output: uninstallInProgress, PlanError: planErr}, nil + } + if phase != phaseUninstalling { + if err := exec.CheckUninstall(st, ns, release); err != nil { + return uninstallResult{}, failErr(http.StatusNotFound, err) + } } // Read the plan before the release goes, only to name it in the audit row. @@ -180,8 +199,25 @@ func (b helmBackend) Uninstall(ctx context.Context, ns, release string) (uninsta planHash = p.Hash } + // Inflight before the marker so a list cannot drop the plan while helm is + // still running. A marker with no inflight is a destroy that did not finish + // (this process, or the one before a restart); helm runs again. + if !s.beginUninstall(ns, release) { + return uninstallResult{Output: uninstallInProgress}, nil + } + hadPlan := phase == phaseUninstalling + if !hadPlan { + var merr error + hadPlan, merr = s.markUninstalling(ctx, ns, release) + if merr != nil { + s.forgetUninstalling(ns, release) + return uninstallResult{}, fail(http.StatusInternalServerError, "plan not marked, nothing uninstalled: %s", merr.Error()) + } + } + // The uninstall outlives the request, like an apply: a browser navigating - // away must not SIGKILL helm halfway through deleting a release. + // away must not SIGKILL helm halfway through deleting a release. It does + // not wait for pods to finish terminating. started := time.Now() ctx, cancel := detach(ctx) defer cancel() @@ -189,34 +225,47 @@ func (b helmBackend) Uninstall(ctx context.Context, ns, release string) (uninsta res, uninstallErr := s.runner().Uninstall(ctx, ns, release) s.recordRelease(ctx, "uninstall", ns, release, planHash, res, uninstallErr, started, "", 0) if uninstallErr != nil { + if hadPlan { + if err := s.finishUninstallFailed(ctx, ns, release, uninstallErr); err != nil { + s.log.ErrorContext(ctx, "uninstalling phase not cleared", "release", release, "err", err) + s.endUninstall(ns, release) + return uninstallResult{}, failErr(http.StatusInternalServerError, uninstallErr) + } + } + s.forgetUninstalling(ns, release) return uninstallResult{}, failErr(http.StatusInternalServerError, uninstallErr) } - // Best effort, and reported rather than swallowed: the release is gone - // either way, and a stray plan ConfigMap shows up in the deployments view - // as a plan with no release rather than as anything dangerous. + s.endUninstall(ns, release) var planErr string if s.writer != nil { // The history goes with it: one Secret per revision would otherwise be - // left behind for a release that no longer exists. - if revs, err := s.archivedRevisions(ctx, ns, release); err == nil { - for _, r := range revs { - if err := s.writer.DeleteSecret(ctx, archiveRef(ns, release, r)); err != nil { - s.log.WarnContext(ctx, "archived plan not removed", "release", release, "revision", r, "err", err) - } + // left behind for a release that no longer exists. The plan ConfigMap + // stays until the pods are gone, which is what keeps the row listed. + // This is also the path a restart takes when the marker is set, nothing + // is inflight, and helm runs again. + s.deleteArchives(ctx, ns, release) + // With a plan, the autoscale ConfigMap goes in reapRelease, once the + // release is gone. Without one nothing keeps the release listed, so it + // goes now. + if hadPlan { + planErr = s.reapRelease(ctx, ns, release) + } else { + if err := s.writer.DeleteConfigMap(ctx, planRef(ns, release)); err != nil { + planErr = err.Error() + s.log.ErrorContext(ctx, "plan configmap not removed", "release", release, "err", err) } - } - if err := s.writer.DeleteConfigMap(ctx, planRef(ns, release)); err != nil { - planErr = err.Error() - s.log.ErrorContext(ctx, "plan configmap not removed", "release", release, "err", err) - } - if err := s.removeAutoscale(ctx, ns, release); err != nil { - if planErr != "" { - planErr += "; " + if err := s.removeAutoscale(ctx, ns, release); err != nil { + if planErr != "" { + planErr += "; " + } + planErr += err.Error() + s.log.ErrorContext(ctx, "autoscale configmap not removed", "release", release, "err", err) } - planErr += err.Error() - s.log.ErrorContext(ctx, "autoscale configmap not removed", "release", release, "err", err) } } + if !hadPlan { + s.forgetUninstalling(ns, release) + } return uninstallResult{Output: res.Output, PlanError: planErr}, nil } diff --git a/internal/server/backend_llm.go b/internal/server/backend_llm.go index 78c8998..edcee7c 100644 --- a/internal/server/backend_llm.go +++ b/internal/server/backend_llm.go @@ -308,6 +308,24 @@ func (b llmBackend) Uninstall(ctx context.Context, ns, release string) (uninstal if err != nil { return uninstallResult{}, failErr(http.StatusBadGateway, err) } + if s.llms == nil { + return uninstallResult{}, fail(http.StatusBadGateway, "no LLMService client") + } + u, err := s.llms.Get(ctx, ns, release) + if apierrors.IsNotFound(err) { + return uninstallResult{}, fail(http.StatusNotFound, "no release %s/%s", ns, release) + } + if err != nil { + return uninstallResult{}, failErr(http.StatusBadGateway, err) + } + // The operator's finalizer holds the object through route removal and pod + // termination. This call returns once that delete is accepted; the reap + // drops swiss's records when the object is finally gone. + if !u.GetDeletionTimestamp().IsZero() { + s.trackLLMUninstall(ns, release, u.GetAnnotations()[llmsvc.AnnPlanHash], u.GetDeletionTimestamp().Time) + return uninstallResult{Output: uninstallInProgress}, nil + } + planHash := "" if p, err := b.Current(ctx, ns, release); err == nil { planHash = p.Hash @@ -315,34 +333,41 @@ func (b llmBackend) Uninstall(ctx context.Context, ns, release string) (uninstal return uninstallResult{}, fail(http.StatusNotFound, "no release %s/%s", ns, release) } - started := time.Now() - ctx, cancel := detach(ctx) - defer cancel() + prev, err := s.snapshotPlan(ctx, ns, release) + if err != nil { + return uninstallResult{}, fail(http.StatusInternalServerError, "plan not marked, nothing uninstalled: %s", err.Error()) + } + hadPlan, err := s.markUninstalling(ctx, ns, release) + if err != nil { + return uninstallResult{}, fail(http.StatusInternalServerError, "plan not marked, nothing uninstalled: %s", err.Error()) + } + started := time.Now() delErr := s.llms.Delete(ctx, ns, release) if apierrors.IsNotFound(delErr) { + if hadPlan { + if rerr := s.restorePlan(ctx, ns, release, prev); rerr != nil { + s.log.ErrorContext(ctx, "plan not restored after delete NotFound", "namespace", ns, "release", release, "err", rerr) + } + } + s.forgetUninstalling(ns, release) return uninstallResult{}, fail(http.StatusNotFound, "no release %s/%s", ns, release) } - var uninstallErr error + out := llmDeleteRequested(ns, release) if delErr != nil { - uninstallErr = delErr - } else if err := s.llms.WaitGone(ctx, ns, release); err != nil { - uninstallErr = err - } - out := fmt.Sprintf("deleted LLMService %s/%s", ns, release) - res := exec.Result{Output: out} - s.recordRelease(ctx, "uninstall", ns, release, planHash, res, uninstallErr, started, "", 0) - if uninstallErr != nil { - return uninstallResult{}, failErr(http.StatusInternalServerError, uninstallErr) - } - var planErr string - if s.writer != nil { - if err := s.removeAutoscale(ctx, ns, release); err != nil { - planErr = err.Error() - s.log.ErrorContext(ctx, "autoscale configmap not removed", "release", release, "err", err) + s.recordRelease(ctx, "uninstall", ns, release, planHash, exec.Result{Output: out}, delErr, started, "", 0) + if hadPlan { + if ferr := s.finishUninstallFailed(ctx, ns, release, delErr); ferr != nil { + s.log.ErrorContext(ctx, "plan not marked failed", "namespace", ns, "release", release, "err", ferr) + } } + return uninstallResult{}, failErr(http.StatusInternalServerError, delErr) } - return uninstallResult{Output: out, PlanError: planErr}, nil + // Record before tracking. The object may already be gone, and a reap that + // sees the tracking first would write the final row ahead of this one. + s.recordRelease(ctx, "uninstall", ns, release, planHash, exec.Result{Output: out}, nil, started, "", 0) + s.trackLLMUninstall(ns, release, planHash, started) + return uninstallResult{Output: uninstallInProgress}, nil } func (b llmBackend) helmRevision(ctx context.Context, ns, name string) int { diff --git a/internal/server/backend_test.go b/internal/server/backend_test.go index 3943af6..d01c2c0 100644 --- a/internal/server/backend_test.go +++ b/internal/server/backend_test.go @@ -358,7 +358,10 @@ func TestLLMListErrorIsStillBadGateway(t *testing.T) { if code != http.StatusBadGateway { t.Fatalf("status %d %v", code, body) } - if _, _ = get(t, g.srv, "/api/deployments"); g.calls.Load() != 2 { + // Each deployments read lists twice: once while rebuilding uninstalling + // LLMServices, and once for the page. A timeout is not cached, so the + // second read does both again. + if _, _ = get(t, g.srv, "/api/deployments"); g.calls.Load() != 4 { t.Fatalf("a non-forbidden list error was cached: calls %d", g.calls.Load()) } } diff --git a/internal/server/deploy.go b/internal/server/deploy.go index ae8a459..a6d4ab2 100644 --- a/internal/server/deploy.go +++ b/internal/server/deploy.go @@ -646,7 +646,14 @@ func (s *Server) apply(ctx context.Context, p *plan.Plan, mode exec.Mode, applyW return s.applyLocked(ctx, p, mode, applyWith, o) } +// applyLocked is where install, upgrade, rollback and the autoscaler refuse an +// uninstalling release. It runs under the release's lock, as the uninstall +// marker is written, so an apply cannot write its applying phase over the +// marker and an uninstall cannot start in the middle of an apply. func (s *Server) applyLocked(ctx context.Context, p *plan.Plan, mode exec.Mode, applyWith string, o applyOpts) (applyResult, error) { + if err := s.refuseUninstalling(ctx, p.Release.Namespace, p.Release.Name); err != nil { + return applyResult{}, err + } b, err := s.backendForApply(ctx, p, mode, applyWith) if err != nil { return applyResult{}, err @@ -677,20 +684,28 @@ func (s *Server) applyPlan(ctx context.Context, w http.ResponseWriter, p *plan.P }) } -// handleUninstall removes a release and the plan recorded beside it. -// -// The order is deliberate and is the write-ahead in reverse. An apply records -// the plan first so a failure cannot leave a live release with nothing beside -// it; an uninstall removes the release first for the same reason. Dropping the -// ConfigMap up front and then failing to uninstall would turn a release swiss -// deployed into an `untracked` row -- the one thing that is supposed to mean -// somebody installed by hand. +// handleUninstall removes a release. A helm plan is marked uninstalling +// before helm runs and stays until that release's pods are gone. An LLMService +// delete returns once the API accepts it; the operator finishes the teardown. func (s *Server) handleUninstall(w http.ResponseWriter, r *http.Request) { ctx, cancel := contextWithTimeout(r, 15*time.Minute) defer cancel() ns, release := r.PathValue("namespace"), r.PathValue("release") if !s.tryApplyLock(ns, release) { + // The lock may be held by the uninstall itself, still running helm, + // or by another call while an LLMService is already being deleted. + // Asking again is not a conflict: it is the same answer as a repeat + // DELETE that does get the lock. + if s.uninstallInflight(ns, release) || s.llmUninstalling(ns, release) || s.planPhase(ctx, ns, release) == phaseUninstalling { + writeJSON(w, http.StatusOK, map[string]any{ + "release": release, + "namespace": ns, + "output": uninstallInProgress, + "planError": "", + }) + return + } writeStatus(w, &applyBusyError{ns: ns, release: release}, http.StatusConflict) return } @@ -717,9 +732,10 @@ func (s *Server) handleUninstall(w http.ResponseWriter, r *http.Request) { } const ( - phaseApplying = "applying" - phaseApplied = "applied" - phaseFailed = "failed" + phaseApplying = "applying" + phaseApplied = "applied" + phaseFailed = "failed" + phaseUninstalling = "uninstalling" ) // applyBudget bounds the cluster-changing half of an apply, which runs detached @@ -822,6 +838,26 @@ func (s *Server) pruneArchives(ctx context.Context, namespace, release string, n return nil } +// deleteArchives removes every archived plan for the release. Best effort, the +// same as the uninstall that first deletes them: a list error is left for the +// next attempt, and a delete error is logged. DeleteSecret succeeds when the +// Secret is already gone. The plan ConfigMap is a different object and stays +// until the pods are gone. +func (s *Server) deleteArchives(ctx context.Context, ns, release string) { + if s.writer == nil { + return + } + revs, err := s.archivedRevisions(ctx, ns, release) + if err != nil { + return + } + for _, r := range revs { + if err := s.writer.DeleteSecret(ctx, archiveRef(ns, release, r)); err != nil { + s.log.WarnContext(ctx, "archived plan not removed", "release", release, "revision", r, "err", err) + } + } +} + // archivedRevisions is every revision with a plan beside it, newest first. func (s *Server) archivedRevisions(ctx context.Context, namespace, release string) ([]int, error) { names, err := s.probe.SecretNames(ctx, namespace, archiveSelector(release)) @@ -857,7 +893,17 @@ func (s *Server) writePlan(ctx context.Context, p *plan.Plan, st planStatus) err return err } files["status.yaml"] = string(status) - return s.writer.PutConfigMap(ctx, planRef(p.Release.Namespace, p.Release.Name), files) + return s.writer.PutConfigMap(ctx, planRef(p.Release.Namespace, p.Release.Name), files, phaseLabel(st.Phase)) +} + +// phaseLabel is the metadata marker for a phase the deployments list has to +// see without reading status.yaml. Any other phase clears it: PutConfigMap +// replaces labels. +func phaseLabel(phase string) map[string]string { + if phase == phaseUninstalling { + return map[string]string{cluster.LabelPhase: phaseUninstalling} + } + return nil } func (s *Server) planFromRequest(ctx context.Context, r *http.Request) (*plan.Plan, error) { diff --git a/internal/server/deploy_test.go b/internal/server/deploy_test.go index 3c2a2bb..62488ac 100644 --- a/internal/server/deploy_test.go +++ b/internal/server/deploy_test.go @@ -65,7 +65,7 @@ func (f *fakeWriter) EnsureNamespace(_ context.Context, name string) error { return nil } -func (f *fakeWriter) PutConfigMap(_ context.Context, ref string, data map[string]string) error { +func (f *fakeWriter) PutConfigMap(_ context.Context, ref string, data, _ map[string]string) error { if err := f.missingNS(ref); err != nil { return err } @@ -488,7 +488,7 @@ func (failingWriter) EnsureNamespace(context.Context, string) error { return errors.New("forbidden") } -func (failingWriter) PutConfigMap(context.Context, string, map[string]string) error { +func (failingWriter) PutConfigMap(context.Context, string, map[string]string, map[string]string) error { return errors.New("forbidden") } diff --git a/internal/server/llmsvc_test.go b/internal/server/llmsvc_test.go index 28aa7e1..d261aaa 100644 --- a/internal/server/llmsvc_test.go +++ b/internal/server/llmsvc_test.go @@ -910,31 +910,6 @@ func TestLLMRollback(t *testing.T) { } } -func TestLLMUninstall(t *testing.T) { - rig := startRig(t, llmProbe(), true, true, backendLLMSVC) - hash := rig.storePlan(t, map[string]any{"release": llmRelease}) - if code, out := post(t, rig.srv, "/api/install", map[string]any{"planHash": hash}); code != 200 { - t.Fatalf("install %d %v", code, out) - } - code, out := del(t, rig.srv, "/api/releases/models/"+llmRelease) - if code != 200 { - t.Fatalf("uninstall %d %v", code, out) - } - if output, _ := out["output"].(string); output != "deleted LLMService models/"+llmRelease { - t.Fatalf("output %q", output) - } - if _, err := rig.c.Get(t.Context(), "models", llmRelease); !apierrors.IsNotFound(err) { - t.Fatalf("object still present: %v", err) - } - if len(rig.w.deleted) != 0 { - t.Fatalf("llmsvc uninstall must leave archives and ConfigMaps alone: %v", rig.w.deleted) - } - runs := rig.runs(t) - if len(runs) < 1 || runs[0].Action != "uninstall" || runs[0].Revision != 0 || runs[0].Error != "" { - t.Fatalf("audit %+v", runs) - } -} - func TestLLMRevisions(t *testing.T) { rig := startRig(t, llmProbe(), true, true, backendLLMSVC) hash := rig.storePlan(t, map[string]any{"release": llmRelease}) diff --git a/internal/server/rollback.go b/internal/server/rollback.go index ef648d1..c3419fa 100644 --- a/internal/server/rollback.go +++ b/internal/server/rollback.go @@ -214,6 +214,10 @@ func (s *Server) handleRollback(w http.ResponseWriter, r *http.Request) { } ns, release := r.PathValue("namespace"), r.PathValue("release") + if err := s.refuseUninstalling(ctx, ns, release); err != nil { + writeStatus(w, err, http.StatusConflict) + return + } p, err := s.archivedPlan(ctx, ns, release, req.ToRevision) if err != nil { writeError(w, http.StatusNotFound, err.Error()) diff --git a/internal/server/server.go b/internal/server/server.go index 57e9093..f205fcd 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -85,6 +85,19 @@ type Server struct { // decisionHTTP is the client for decision-gen. Nil uses a 5s timeout. decisionHTTP *http.Client + + // now is the clock stuck-terminating pods are judged against. New installs + // time.Now; tests replace it. + now func() time.Time + + // uninst is namespace/release for a release swiss is uninstalling. A restart + // leaves it empty; labeled plan ConfigMaps and LLMServices with a + // deletionTimestamp fill it again. llm means the operator owns teardown + // and reap waits for the object to disappear. inflight is set while helm + // has not returned, so a list cannot drop the plan out from under that call. + uninstMu sync.Mutex + uninst map[string]*uninstState + uninstReady bool } type openCatalog struct { @@ -102,6 +115,7 @@ func New(cfg *config.Config, probe cluster.Probe, log *slog.Logger, version stri catCache: catalog.NewCache(cfg.Server.CatalogCacheDir()), cats: map[string]openCatalog{}, charts: map[string]chartListing{}, + now: time.Now, } } @@ -459,7 +473,11 @@ func (s *Server) Handler() http.Handler { // Run serves until ctx is cancelled, then drains. func (s *Server) Run(ctx context.Context) error { - go s.runAutoscaler(ctx) + loopCtx, stopLoop := context.WithCancel(ctx) + defer stopLoop() + go s.runAutoscaler(loopCtx) + go s.uninstallLoop(loopCtx) + srv := &http.Server{ Addr: s.cfg.Server.Addr, Handler: s.Handler(), diff --git a/internal/server/status.go b/internal/server/status.go index 20c7595..00f4f7e 100644 --- a/internal/server/status.go +++ b/internal/server/status.go @@ -17,6 +17,8 @@ import ( "github.com/modelsphere/swiss/internal/plan" "github.com/modelsphere/swiss/internal/site" "github.com/modelsphere/swiss/internal/values" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) type releaseStatus struct { @@ -56,11 +58,37 @@ type releaseStatus struct { } // llmStatusView is the operator's status, in place of status.yaml. +// Phase is swiss's word for the row (uninstalling while the object is being +// deleted). OperatorPhase is status.phase as the operator wrote it. type llmStatusView struct { - Phase string `json:"phase,omitempty"` - Message string `json:"message,omitempty"` - Revision int64 `json:"revision,omitempty"` - Conditions []llmCondition `json:"conditions,omitempty"` + Phase string `json:"phase,omitempty"` + OperatorPhase string `json:"operatorPhase,omitempty"` + Message string `json:"message,omitempty"` + Revision int64 `json:"revision,omitempty"` + Conditions []llmCondition `json:"conditions,omitempty"` + Deletion *llmDeletionView `json:"deletion,omitempty"` +} + +// llmDeletionView is status.deletion. Swiss passes through what the operator +// reports and does not include the operator's internal timestamps. +type llmDeletionView struct { + Stage string `json:"stage,omitempty"` + Outcome string `json:"outcome,omitempty"` + PodsRemaining int `json:"podsRemaining,omitempty"` + Deadline metav1.Time `json:"deadline,omitempty,omitzero"` + StartedAt metav1.Time `json:"startedAt,omitempty,omitzero"` + Message string `json:"message,omitempty"` + Pods []llmDeletionPod `json:"pods,omitempty"` + ForceDeleted []string `json:"forceDeleted,omitempty"` +} + +type llmDeletionPod struct { + Name string `json:"name,omitempty"` + Phase string `json:"phase,omitempty"` + Node string `json:"node,omitempty"` + DeletionTimestamp metav1.Time `json:"deletionTimestamp,omitempty,omitzero"` + Grace int `json:"grace,omitempty"` + Stuck bool `json:"stuck,omitempty"` } type llmCondition struct { @@ -105,22 +133,27 @@ func (s *Server) handleStatus(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadGateway, err.Error()) return } - if obj, err := llmsvc.FromUnstructured(u); err == nil && obj.Status != nil { + var obj *llmsvc.LLMService + if decoded, err := llmsvc.FromUnstructured(u); err == nil { + obj = decoded out.Exists = true - out.Service = &llmStatusView{Phase: obj.Status.Phase, Message: obj.Status.Message} - for _, c := range obj.Status.Conditions { - out.Service.Conditions = append(out.Service.Conditions, llmCondition{ - Type: c.Type, Status: string(c.Status), Reason: c.Reason, Message: c.Message, - }) - } - if obj.Status.Helm != nil { - out.Revision = int(obj.Status.Helm.Revision) - out.HelmStatus = obj.Status.Helm.Status - out.Service.Revision = obj.Status.Helm.Revision + if obj.Status != nil { + out.Service = llmStatusFrom(obj.Status) + if obj.Status.Helm != nil { + out.Revision = int(obj.Status.Helm.Revision) + out.HelmStatus = obj.Status.Helm.Status + } } } else { out.Exists = true } + if llmDeleting(u, obj) { + out.Exists = true + if out.Service == nil { + out.Service = &llmStatusView{} + } + out.Service.Phase = phaseUninstalling + } if decoded, err := llmsvc.ToPlan(u); err == nil { p = decoded } @@ -165,9 +198,73 @@ func (s *Server) handleStatus(w http.ResponseWriter, r *http.Request) { out.Ready++ } } + if (out.Plan != nil && out.Plan.Phase == phaseUninstalling) || s.llmUninstalling(ns, release) { + s.reapRelease(ctx, ns, release) + } writeJSON(w, http.StatusOK, out) } +func llmStatusFrom(st *llmsvc.Status) *llmStatusView { + v := &llmStatusView{ + Phase: st.Phase, OperatorPhase: st.Phase, Message: st.Message, + Deletion: deletionView(st.Deletion), + } + if st.Helm != nil { + v.Revision = st.Helm.Revision + } + for _, c := range st.Conditions { + v.Conditions = append(v.Conditions, llmCondition{ + Type: c.Type, Status: string(c.Status), Reason: c.Reason, Message: c.Message, + }) + } + return v +} + +// llmDeleting is an LLMService the operator has been asked to remove, either +// because the delete is already stamped or because status.phase says so. +func llmDeleting(u *unstructured.Unstructured, obj *llmsvc.LLMService) bool { + if u != nil && !u.GetDeletionTimestamp().IsZero() { + return true + } + return obj != nil && obj.Status != nil && obj.Status.Phase == llmsvc.PhaseDeleting +} + +func uninstallDrift(stage string) string { + if stage == "" { + return "uninstall in progress" + } + return "uninstall in progress: " + stage +} + +func deletionStage(v *llmDeletionView) string { + if v == nil { + return "" + } + return v.Stage +} + +func deletionView(d *llmsvc.DeletionStatus) *llmDeletionView { + if d == nil { + return nil + } + v := &llmDeletionView{ + Stage: d.Stage, Outcome: d.Outcome, PodsRemaining: d.PodsRemaining, + Deadline: d.Deadline, StartedAt: d.StartedAt, Message: d.Message, + ForceDeleted: d.ForceDeleted, + } + if len(d.Pods) == 0 { + return v + } + v.Pods = make([]llmDeletionPod, len(d.Pods)) + for i, p := range d.Pods { + v.Pods[i] = llmDeletionPod{ + Name: p.Name, Phase: p.Phase, Node: p.Node, + DeletionTimestamp: p.DeletionTimestamp, Grace: p.Grace, Stuck: p.Stuck, + } + } + return v +} + // releaseDigestDrift compares the digest this release pinned with the digest // its catalog now publishes for that same version and variant. A catalog that // cannot be read leaves the warning empty: status is polled, and a dead diff --git a/internal/server/testdata/autoscale-api/uninstall-llmsvc.yaml b/internal/server/testdata/autoscale-api/uninstall-llmsvc.yaml index a0a1ccd..1fe934d 100644 --- a/internal/server/testdata/autoscale-api/uninstall-llmsvc.yaml +++ b/internal/server/testdata/autoscale-api/uninstall-llmsvc.yaml @@ -1,5 +1,5 @@ name: uninstall-llmsvc -description: llmsvc uninstall deletes the autoscale ConfigMap when it exists +description: llmsvc uninstall deletes the autoscale ConfigMap once the LLMService is gone backend: llmsvc uninstall: true release: qwen-new diff --git a/internal/server/testdata/autoscale-loop/helm-uninstalling.yaml b/internal/server/testdata/autoscale-loop/helm-uninstalling.yaml new file mode 100644 index 0000000..5c793d5 --- /dev/null +++ b/internal/server/testdata/autoscale-loop/helm-uninstalling.yaml @@ -0,0 +1,11 @@ +name: helm-uninstalling +description: a helm release whose plan is marked uninstalling holds and is not applied +backend: helm +decision: 3 +slices: 1 +min: 1 +max: 4 +revision: 2 +planPhase: uninstalling +wantApplies: 0 +wantHold: uninstalling diff --git a/internal/server/testdata/autoscale-loop/llm-deleting.yaml b/internal/server/testdata/autoscale-loop/llm-deleting.yaml new file mode 100644 index 0000000..aeb01cc --- /dev/null +++ b/internal/server/testdata/autoscale-loop/llm-deleting.yaml @@ -0,0 +1,11 @@ +name: llm-deleting +description: an LLMService with a deletionTimestamp holds and is not updated +backend: llmsvc +decision: 3 +slices: 1 +min: 1 +max: 4 +revision: 2 +deleting: true +wantApplies: 0 +wantHold: uninstalling diff --git a/internal/server/testdata/autoscale-loop/llm-phase-deleting.yaml b/internal/server/testdata/autoscale-loop/llm-phase-deleting.yaml new file mode 100644 index 0000000..d1a2003 --- /dev/null +++ b/internal/server/testdata/autoscale-loop/llm-phase-deleting.yaml @@ -0,0 +1,11 @@ +name: llm-phase-deleting +description: status.phase Deleting holds and is not updated +backend: llmsvc +decision: 3 +slices: 1 +min: 1 +max: 4 +revision: 2 +llmPhase: Deleting +wantApplies: 0 +wantHold: uninstalling diff --git a/internal/server/testdata/uninstalling/apply-during-uninstall-is-refused.yaml b/internal/server/testdata/uninstalling/apply-during-uninstall-is-refused.yaml new file mode 100644 index 0000000..324b0a1 --- /dev/null +++ b/internal/server/testdata/uninstalling/apply-during-uninstall-is-refused.yaml @@ -0,0 +1,37 @@ +# While helm uninstall runs it holds the release's lock: install, upgrade and +# rollback are 409 and none of them writes its applying phase over the marker. +# Once helm has returned, the lock is free and the marker is what refuses them. +kind: helm +release: glm-53 +namespace: models +chart: vllm +helmBlocks: true +pods: + - name: engine-0 + phase: Running + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: uninstallAsync + - do: install + status: 409 + - do: upgrade + status: 409 + - do: rollback + status: 409 + - do: list + status: 200 + present: true + phase: uninstalling + - do: releaseHelm + status: 200 + outputContains: helm uninstall + - do: upgrade + status: 409 + errorContains: uninstalling + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true diff --git a/internal/server/testdata/uninstalling/archive-cleanup-repeat-delete.yaml b/internal/server/testdata/uninstalling/archive-cleanup-repeat-delete.yaml new file mode 100644 index 0000000..6a0bec4 --- /dev/null +++ b/internal/server/testdata/uninstalling/archive-cleanup-repeat-delete.yaml @@ -0,0 +1,29 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +seedArchive: true +pods: + - name: engine-0 + phase: Running + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: uninstall + status: 200 + planError: false + archives: 0 + - do: seedArchives + archives: 1 + - do: dropHelm + - do: uninstall + status: 200 + outputContains: uninstall already in progress + planError: false + archives: 0 + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true diff --git a/internal/server/testdata/uninstalling/archive-cleanup-restart.yaml b/internal/server/testdata/uninstalling/archive-cleanup-restart.yaml new file mode 100644 index 0000000..7048c6b --- /dev/null +++ b/internal/server/testdata/uninstalling/archive-cleanup-restart.yaml @@ -0,0 +1,26 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +seedArchive: true +pods: + - name: engine-0 + phase: Running + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: uninstall + status: 200 + planError: false + archives: 0 + - do: seedArchives + archives: 1 + - do: dropHelm + - do: restart + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true + archives: 0 diff --git a/internal/server/testdata/uninstalling/archives-kept-when-uninstall-fails.yaml b/internal/server/testdata/uninstalling/archives-kept-when-uninstall-fails.yaml new file mode 100644 index 0000000..06538c4 --- /dev/null +++ b/internal/server/testdata/uninstalling/archives-kept-when-uninstall-fails.yaml @@ -0,0 +1,24 @@ +# A helm uninstall that fails leaves the release, so its revision history +# stays too. +kind: helm +release: glm-53 +namespace: models +chart: vllm +helmExit: 1 +seedArchive: true +pods: + - name: engine-0 + phase: Running + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: uninstall + status: 500 + errorContains: helm uninstall + archives: 1 + - do: list + status: 200 + present: true + phase: failed + archives: 1 diff --git a/internal/server/testdata/uninstalling/archives-kept-while-helm-holds.yaml b/internal/server/testdata/uninstalling/archives-kept-while-helm-holds.yaml new file mode 100644 index 0000000..d2f1c93 --- /dev/null +++ b/internal/server/testdata/uninstalling/archives-kept-while-helm-holds.yaml @@ -0,0 +1,29 @@ +# A marked release that helm still records (an uninstall that did not finish +# before a restart) keeps its archives. They go once helm has dropped it. +kind: helm +release: glm-53 +namespace: models +chart: vllm +pods: + - name: engine-0 + phase: Running + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: uninstall + status: 200 + - do: seedArchives + archives: 1 + - do: restart + - do: list + status: 200 + present: true + phase: uninstalling + archives: 1 + - do: dropHelm + - do: list + status: 200 + present: true + phase: uninstalling + archives: 0 diff --git a/internal/server/testdata/uninstalling/autoscale-configmap-removed-on-reap.yaml b/internal/server/testdata/uninstalling/autoscale-configmap-removed-on-reap.yaml new file mode 100644 index 0000000..0fbf1f6 --- /dev/null +++ b/internal/server/testdata/uninstalling/autoscale-configmap-removed-on-reap.yaml @@ -0,0 +1,30 @@ +# The autoscale ConfigMap stays while the release is uninstalling and goes +# with the plan, once helm has dropped the release and no pods remain. +kind: helm +release: glm-53 +namespace: models +chart: vllm +seedAutoscale: true +pods: + - name: engine-0 + phase: Running + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: uninstall + status: 200 + planError: false + autoscale: true + - do: dropHelm + - do: list + status: 200 + present: true + phase: uninstalling + autoscale: true + - do: setPods + - do: list + status: 200 + present: false + configMap: false + autoscale: false diff --git a/internal/server/testdata/uninstalling/configmap-delete-retries.yaml b/internal/server/testdata/uninstalling/configmap-delete-retries.yaml new file mode 100644 index 0000000..85f6d7b --- /dev/null +++ b/internal/server/testdata/uninstalling/configmap-delete-retries.yaml @@ -0,0 +1,25 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +deleteConfigMapFails: 2 +steps: + - do: uninstall + status: 200 + planError: false + configMap: true + - do: dropHelm + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true + - do: list + status: 200 + present: false + configMap: false diff --git a/internal/server/testdata/uninstalling/destroy-fails.yaml b/internal/server/testdata/uninstalling/destroy-fails.yaml new file mode 100644 index 0000000..ae6081d --- /dev/null +++ b/internal/server/testdata/uninstalling/destroy-fails.yaml @@ -0,0 +1,25 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +helmExit: 1 +pods: + - name: engine-0 + phase: Running + labels: + app: glm-53-vllm +steps: + - do: uninstall + status: 500 + errorContains: helm uninstall + - do: list + status: 200 + present: true + phase: failed + configMap: true + - do: setPods + - do: list + status: 200 + present: true + phase: failed + configMap: true diff --git a/internal/server/testdata/uninstalling/engine-pod-only.yaml b/internal/server/testdata/uninstalling/engine-pod-only.yaml new file mode 100644 index 0000000..574b5ad --- /dev/null +++ b/internal/server/testdata/uninstalling/engine-pod-only.yaml @@ -0,0 +1,21 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +pods: + - name: engine-0 + phase: Running + labels: + app: glm-53-vllm +steps: + - do: uninstall + status: 200 + - do: status + status: 200 + planPhase: uninstalling + minPods: 1 + configMap: true + - do: list + status: 200 + present: true + phase: uninstalling diff --git a/internal/server/testdata/uninstalling/finished-pods-ignored.yaml b/internal/server/testdata/uninstalling/finished-pods-ignored.yaml new file mode 100644 index 0000000..3a41042 --- /dev/null +++ b/internal/server/testdata/uninstalling/finished-pods-ignored.yaml @@ -0,0 +1,43 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +pods: + - name: engine-0 + phase: Running + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: uninstall + status: 200 + planError: false + configMap: true + - do: list + status: 200 + present: true + phase: uninstalling + - do: dropHelm + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true + - do: setPods + pods: + - name: glm-53-test + phase: Succeeded + labels: + app.kubernetes.io/instance: glm-53 + - name: glm-53-pre-delete + phase: Failed + labels: + app.kubernetes.io/instance: glm-53 + - name: glm-53-vllm-hook + phase: Failed + labels: + app: glm-53-vllm + - do: list + status: 200 + present: false + configMap: false diff --git a/internal/server/testdata/uninstalling/install-upgrade-rollback.yaml b/internal/server/testdata/uninstalling/install-upgrade-rollback.yaml new file mode 100644 index 0000000..2f0d90d --- /dev/null +++ b/internal/server/testdata/uninstalling/install-upgrade-rollback.yaml @@ -0,0 +1,25 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +pods: + - name: engine-0 + phase: Running + labels: + app: glm-53-vllm +steps: + - do: uninstall + status: 200 + - do: install + status: 409 + errorContains: uninstalling + - do: upgrade + status: 409 + errorContains: uninstalling + - do: rollback + status: 409 + errorContains: uninstalling + - do: list + status: 200 + present: true + phase: uninstalling diff --git a/internal/server/testdata/uninstalling/instance-pod-only.yaml b/internal/server/testdata/uninstalling/instance-pod-only.yaml new file mode 100644 index 0000000..ff2acf1 --- /dev/null +++ b/internal/server/testdata/uninstalling/instance-pod-only.yaml @@ -0,0 +1,21 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +pods: + - name: engine-0 + phase: Running + labels: + app.kubernetes.io/instance: glm-53 +steps: + - do: uninstall + status: 200 + - do: status + status: 200 + planPhase: uninstalling + totalPods: 0 + configMap: true + - do: list + status: 200 + present: true + phase: uninstalling diff --git a/internal/server/testdata/uninstalling/live-pods-keep-uninstalling.yaml b/internal/server/testdata/uninstalling/live-pods-keep-uninstalling.yaml new file mode 100644 index 0000000..16f91a0 --- /dev/null +++ b/internal/server/testdata/uninstalling/live-pods-keep-uninstalling.yaml @@ -0,0 +1,39 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +pods: + - name: engine-0 + phase: Running + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: uninstall + status: 200 + planError: false + - do: list + status: 200 + present: true + phase: uninstalling + - do: dropHelm + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true + - do: setPods + pods: + - name: glm-53-test + phase: Succeeded + labels: + app.kubernetes.io/instance: glm-53 + - name: engine-pending + phase: Pending + labels: + app: glm-53-vllm + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true diff --git a/internal/server/testdata/uninstalling/llmsvc-delete-in-progress.yaml b/internal/server/testdata/uninstalling/llmsvc-delete-in-progress.yaml new file mode 100644 index 0000000..40a3dce --- /dev/null +++ b/internal/server/testdata/uninstalling/llmsvc-delete-in-progress.yaml @@ -0,0 +1,16 @@ +kind: llmsvc +release: qwen +namespace: models +deleting: false +finalizer: true +steps: + - do: uninstall + status: 200 + outputContains: uninstall already in progress + planError: false + llmService: true + - do: list + status: 200 + present: true + phase: uninstalling + backend: llmsvc diff --git a/internal/server/testdata/uninstalling/llmsvc-delete-not-found-restores-plan.yaml b/internal/server/testdata/uninstalling/llmsvc-delete-not-found-restores-plan.yaml new file mode 100644 index 0000000..8eda231 --- /dev/null +++ b/internal/server/testdata/uninstalling/llmsvc-delete-not-found-restores-plan.yaml @@ -0,0 +1,14 @@ +kind: llmsvc +release: qwen +namespace: models +deleting: false +deleteNotFound: true +seedPlanConfigMap: true +planPhase: applied +steps: + - do: uninstall + status: 404 + errorContains: no release + configMap: true + storedPhase: applied + audits: 0 diff --git a/internal/server/testdata/uninstalling/llmsvc-delete-not-found.yaml b/internal/server/testdata/uninstalling/llmsvc-delete-not-found.yaml new file mode 100644 index 0000000..79e6de7 --- /dev/null +++ b/internal/server/testdata/uninstalling/llmsvc-delete-not-found.yaml @@ -0,0 +1,9 @@ +kind: llmsvc +release: qwen +namespace: models +missing: true +steps: + - do: uninstall + status: 404 + errorContains: no release + audits: 0 diff --git a/internal/server/testdata/uninstalling/llmsvc-deleting.yaml b/internal/server/testdata/uninstalling/llmsvc-deleting.yaml new file mode 100644 index 0000000..c68b253 --- /dev/null +++ b/internal/server/testdata/uninstalling/llmsvc-deleting.yaml @@ -0,0 +1,22 @@ +kind: llmsvc +release: qwen +namespace: models +operatorPhase: Applied +steps: + - do: list + status: 200 + present: true + phase: uninstalling + backend: llmsvc + - do: status + status: 200 + servicePhase: uninstalling + - do: apply + status: 409 + errorContains: uninstalling + - do: install + status: 409 + errorContains: uninstalling + - do: rollback + status: 409 + errorContains: uninstalling diff --git a/internal/server/testdata/uninstalling/llmsvc-reap-cleanup.yaml b/internal/server/testdata/uninstalling/llmsvc-reap-cleanup.yaml new file mode 100644 index 0000000..cf8b6c0 --- /dev/null +++ b/internal/server/testdata/uninstalling/llmsvc-reap-cleanup.yaml @@ -0,0 +1,29 @@ +kind: llmsvc +release: qwen +namespace: models +deleting: false +seedArchive: true +seedAutoscale: true +seedPlanConfigMap: true +planPhase: applied +steps: + - do: uninstall + status: 200 + outputContains: uninstall already in progress + llmService: false + configMap: true + autoscale: true + archives: 1 + audits: 1 + storedPhase: uninstalling + - do: reap + configMap: false + autoscale: false + archives: 0 + audits: 2 + llmService: false + - do: list + status: 200 + present: false + - do: reap + audits: 2 diff --git a/internal/server/testdata/uninstalling/llmsvc-reap-waits.yaml b/internal/server/testdata/uninstalling/llmsvc-reap-waits.yaml new file mode 100644 index 0000000..7ca5233 --- /dev/null +++ b/internal/server/testdata/uninstalling/llmsvc-reap-waits.yaml @@ -0,0 +1,47 @@ +kind: llmsvc +release: qwen +namespace: models +deleting: false +finalizer: true +operatorPhase: Deleting +seedArchive: true +seedAutoscale: true +deletion: + stage: PodsTerminating + podsRemaining: 2 + message: waiting for pods +steps: + - do: uninstall + status: 200 + outputContains: uninstall already in progress + llmService: true + audits: 1 + - do: reap + llmService: true + autoscale: true + archives: 1 + audits: 1 + - do: list + status: 200 + present: true + phase: uninstalling + backend: llmsvc + deletionStage: PodsTerminating + podsRemaining: 2 + driftContains: "uninstall in progress: PodsTerminating" + - do: status + status: 200 + servicePhase: uninstalling + operatorPhase: Deleting + deletionStage: PodsTerminating + podsRemaining: 2 + deletionMessage: waiting for pods + - do: operatorRelease + - do: reap + llmService: false + autoscale: false + archives: 0 + audits: 2 + - do: list + status: 200 + present: false diff --git a/internal/server/testdata/uninstalling/llmsvc-recreated.yaml b/internal/server/testdata/uninstalling/llmsvc-recreated.yaml new file mode 100644 index 0000000..dd2a7fe --- /dev/null +++ b/internal/server/testdata/uninstalling/llmsvc-recreated.yaml @@ -0,0 +1,27 @@ +kind: llmsvc +release: qwen +namespace: models +operatorPhase: Applied +seedArchive: true +seedAutoscale: true +seedPlanConfigMap: true +planPhase: uninstalling +steps: + - do: reap + archives: 1 + configMap: true + autoscale: true + audits: 0 + llmService: true + - do: clearDeletion + - do: reap + archives: 1 + configMap: true + autoscale: true + audits: 0 + llmService: true + - do: list + status: 200 + present: true + phase: Applied + backend: llmsvc diff --git a/internal/server/testdata/uninstalling/llmsvc-repeat-delete.yaml b/internal/server/testdata/uninstalling/llmsvc-repeat-delete.yaml new file mode 100644 index 0000000..48e7d0c --- /dev/null +++ b/internal/server/testdata/uninstalling/llmsvc-repeat-delete.yaml @@ -0,0 +1,21 @@ +kind: llmsvc +release: qwen +namespace: models +deleting: false +finalizer: true +steps: + - do: uninstall + status: 200 + outputContains: uninstall already in progress + audits: 1 + llmService: true + - do: uninstall + status: 200 + outputContains: uninstall already in progress + audits: 1 + llmService: true + - do: list + status: 200 + present: true + phase: uninstalling + backend: llmsvc diff --git a/internal/server/testdata/uninstalling/llmsvc-restart.yaml b/internal/server/testdata/uninstalling/llmsvc-restart.yaml new file mode 100644 index 0000000..4599633 --- /dev/null +++ b/internal/server/testdata/uninstalling/llmsvc-restart.yaml @@ -0,0 +1,42 @@ +kind: llmsvc +release: qwen +namespace: models +operatorPhase: Deleting +deletion: + stage: Uninstalling + podsRemaining: 2 +seedArchive: true +seedAutoscale: true +seedPlanConfigMap: true +planPhase: uninstalling +steps: + - do: restart + - do: reap + - do: list + status: 200 + present: true + phase: uninstalling + backend: llmsvc + deletionStage: Uninstalling + podsRemaining: 2 + configMap: true + autoscale: true + archives: 1 + audits: 0 + - do: status + status: 200 + servicePhase: uninstalling + operatorPhase: Deleting + deletionStage: Uninstalling + - do: operatorRelease + - do: reap + configMap: false + autoscale: false + archives: 0 + audits: 1 + llmService: false + - do: list + status: 200 + present: false + - do: reap + audits: 1 diff --git a/internal/server/testdata/uninstalling/llmsvc-status-deleting.yaml b/internal/server/testdata/uninstalling/llmsvc-status-deleting.yaml new file mode 100644 index 0000000..9faabdd --- /dev/null +++ b/internal/server/testdata/uninstalling/llmsvc-status-deleting.yaml @@ -0,0 +1,40 @@ +kind: llmsvc +release: qwen +namespace: models +deleting: false +operatorPhase: Deleting +deletion: + stage: RouteRemoving + outcome: NoRoute + podsRemaining: 2 + message: removing the route + startedAt: "2026-10-23T00:00:00Z" + deadline: "2026-10-23T00:05:00Z" + forceDeleted: + - qwen-old + pods: + - name: qwen-0 + phase: Running + node: gpu-1 + grace: 30 + stuck: true +steps: + - do: status + status: 200 + servicePhase: uninstalling + operatorPhase: Deleting + deletionStage: RouteRemoving + deletionOutcome: NoRoute + deletionMessage: removing the route + deletionDeadline: "2026-10-23T00:05:00Z" + podsRemaining: 2 + deletionPods: 1 + forceDeleted: 1 + - do: list + status: 200 + present: true + phase: uninstalling + backend: llmsvc + deletionStage: RouteRemoving + podsRemaining: 2 + driftContains: "uninstall in progress: RouteRemoving" diff --git a/internal/server/testdata/uninstalling/pod-list-error.yaml b/internal/server/testdata/uninstalling/pod-list-error.yaml new file mode 100644 index 0000000..960bb5d --- /dev/null +++ b/internal/server/testdata/uninstalling/pod-list-error.yaml @@ -0,0 +1,20 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +podListError: true +steps: + - do: uninstall + status: 200 + planError: false + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true + - do: setPods + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true diff --git a/internal/server/testdata/uninstalling/pods-gone.yaml b/internal/server/testdata/uninstalling/pods-gone.yaml new file mode 100644 index 0000000..029185f --- /dev/null +++ b/internal/server/testdata/uninstalling/pods-gone.yaml @@ -0,0 +1,30 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +pods: + - name: engine-0 + phase: Running + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: uninstall + status: 200 + planError: false + configMap: true + - do: list + status: 200 + present: true + phase: uninstalling + - do: dropHelm + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true + - do: setPods + - do: list + status: 200 + present: false + configMap: false diff --git a/internal/server/testdata/uninstalling/pods-remain.yaml b/internal/server/testdata/uninstalling/pods-remain.yaml new file mode 100644 index 0000000..54e644b --- /dev/null +++ b/internal/server/testdata/uninstalling/pods-remain.yaml @@ -0,0 +1,29 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +seedArchive: true +pods: + - name: engine-0 + phase: Running + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: uninstall + status: 200 + planError: false + archives: 0 + - do: list + status: 200 + present: true + phase: uninstalling + backend: helm + - do: status + status: 200 + planPhase: uninstalling + minPods: 1 + - do: list + status: 200 + present: true + configMap: true diff --git a/internal/server/testdata/uninstalling/repeat-delete-while-helm-runs.yaml b/internal/server/testdata/uninstalling/repeat-delete-while-helm-runs.yaml new file mode 100644 index 0000000..22948ca --- /dev/null +++ b/internal/server/testdata/uninstalling/repeat-delete-while-helm-runs.yaml @@ -0,0 +1,31 @@ +# A second DELETE while the first is still running helm finds the lock held by +# that uninstall. It is 200, not 409, and does not run helm again. +kind: helm +release: glm-53 +namespace: models +chart: vllm +helmBlocks: true +pods: + - name: engine-0 + phase: Running + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: uninstallAsync + - do: uninstall + status: 200 + outputContains: uninstall already in progress + planError: false + - do: releaseHelm + status: 200 + outputContains: helm uninstall + - do: dropHelm + - do: uninstall + status: 200 + outputContains: uninstall already in progress + - do: setPods + - do: list + status: 200 + present: false + configMap: false diff --git a/internal/server/testdata/uninstalling/restart-helm-remains.yaml b/internal/server/testdata/uninstalling/restart-helm-remains.yaml new file mode 100644 index 0000000..0bb1589 --- /dev/null +++ b/internal/server/testdata/uninstalling/restart-helm-remains.yaml @@ -0,0 +1,19 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +steps: + - do: uninstall + status: 200 + configMap: true + - do: restart + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true + - do: dropHelm + - do: list + status: 200 + present: false + configMap: false diff --git a/internal/server/testdata/uninstalling/restart.yaml b/internal/server/testdata/uninstalling/restart.yaml new file mode 100644 index 0000000..894466e --- /dev/null +++ b/internal/server/testdata/uninstalling/restart.yaml @@ -0,0 +1,34 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +pods: + - name: engine-0 + phase: Running + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: uninstall + status: 200 + - do: list + status: 200 + present: true + phase: uninstalling + - do: restart + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true + - do: dropHelm + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true + - do: setPods + - do: list + status: 200 + present: false + configMap: false diff --git a/internal/server/testdata/uninstalling/retry-while-helm-remains.yaml b/internal/server/testdata/uninstalling/retry-while-helm-remains.yaml new file mode 100644 index 0000000..34ca338 --- /dev/null +++ b/internal/server/testdata/uninstalling/retry-while-helm-remains.yaml @@ -0,0 +1,21 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +pods: + - name: engine-0 + phase: Running + labels: + app: glm-53-vllm +steps: + - do: uninstall + status: 200 + outputContains: helm uninstall + - do: uninstall + status: 200 + outputContains: helm uninstall + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true diff --git a/internal/server/testdata/uninstalling/revision-omitted.yaml b/internal/server/testdata/uninstalling/revision-omitted.yaml new file mode 100644 index 0000000..85adaf2 --- /dev/null +++ b/internal/server/testdata/uninstalling/revision-omitted.yaml @@ -0,0 +1,25 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +pods: + - name: engine-0 + phase: Running + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: uninstall + status: 200 + - do: list + status: 200 + present: true + phase: uninstalling + hasRevision: true + - do: dropHelm + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true + hasRevision: false diff --git a/internal/server/testdata/uninstalling/second-uninstall.yaml b/internal/server/testdata/uninstalling/second-uninstall.yaml new file mode 100644 index 0000000..7a39d6b --- /dev/null +++ b/internal/server/testdata/uninstalling/second-uninstall.yaml @@ -0,0 +1,23 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +pods: + - name: engine-0 + phase: Running + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: uninstall + status: 200 + - do: dropHelm + - do: uninstall + status: 200 + outputContains: uninstall already in progress + planError: false + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true diff --git a/internal/server/testdata/uninstalling/terminating-on-missing-node-ignored.yaml b/internal/server/testdata/uninstalling/terminating-on-missing-node-ignored.yaml new file mode 100644 index 0000000..fdafdb3 --- /dev/null +++ b/internal/server/testdata/uninstalling/terminating-on-missing-node-ignored.yaml @@ -0,0 +1,26 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +nodes: + - name: gpu-1 + ready: true +pods: + - name: engine-0 + phase: Running + node: gpu-gone + deletionTimestamp: 2026-10-22T12:00:00Z + deletionGracePeriodSeconds: 600 + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: setNow + now: 2026-10-22T12:10:30Z + - do: uninstall + status: 200 + - do: dropHelm + - do: list + status: 200 + present: false + configMap: false diff --git a/internal/server/testdata/uninstalling/terminating-on-missing-node-within-grace-keeps-uninstalling.yaml b/internal/server/testdata/uninstalling/terminating-on-missing-node-within-grace-keeps-uninstalling.yaml new file mode 100644 index 0000000..096c599 --- /dev/null +++ b/internal/server/testdata/uninstalling/terminating-on-missing-node-within-grace-keeps-uninstalling.yaml @@ -0,0 +1,37 @@ +# A missing node does not end the uninstall while the pod is still inside its +# grace period: a node that flaps NotReady may still be running it. Once the +# grace is over the node is enough, without the extra five minutes. +kind: helm +release: glm-53 +namespace: models +chart: vllm +nodes: + - name: gpu-1 + ready: true +pods: + - name: engine-0 + phase: Running + node: gpu-gone + deletionTimestamp: 2026-10-22T12:00:00Z + deletionGracePeriodSeconds: 600 + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: setNow + now: 2026-10-22T12:09:59Z + - do: uninstall + status: 200 + configMap: true + - do: dropHelm + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true + - do: setNow + now: 2026-10-22T12:10:01Z + - do: list + status: 200 + present: false + configMap: false diff --git a/internal/server/testdata/uninstalling/terminating-on-notready-node-ignored.yaml b/internal/server/testdata/uninstalling/terminating-on-notready-node-ignored.yaml new file mode 100644 index 0000000..af6e387 --- /dev/null +++ b/internal/server/testdata/uninstalling/terminating-on-notready-node-ignored.yaml @@ -0,0 +1,26 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +nodes: + - name: gpu-1 + ready: false +pods: + - name: engine-0 + phase: Running + node: gpu-1 + deletionTimestamp: 2026-10-22T12:00:00Z + deletionGracePeriodSeconds: 600 + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: setNow + now: 2026-10-22T12:10:30Z + - do: uninstall + status: 200 + - do: dropHelm + - do: list + status: 200 + present: false + configMap: false diff --git a/internal/server/testdata/uninstalling/terminating-on-notready-node-within-grace-keeps-uninstalling.yaml b/internal/server/testdata/uninstalling/terminating-on-notready-node-within-grace-keeps-uninstalling.yaml new file mode 100644 index 0000000..5926f2b --- /dev/null +++ b/internal/server/testdata/uninstalling/terminating-on-notready-node-within-grace-keeps-uninstalling.yaml @@ -0,0 +1,37 @@ +# A notready node does not end the uninstall while the pod is still inside its +# grace period: a node that flaps NotReady may still be running it. Once the +# grace is over the node is enough, without the extra five minutes. +kind: helm +release: glm-53 +namespace: models +chart: vllm +nodes: + - name: gpu-1 + ready: false +pods: + - name: engine-0 + phase: Running + node: gpu-1 + deletionTimestamp: 2026-10-22T12:00:00Z + deletionGracePeriodSeconds: 600 + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: setNow + now: 2026-10-22T12:09:59Z + - do: uninstall + status: 200 + configMap: true + - do: dropHelm + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true + - do: setNow + now: 2026-10-22T12:10:01Z + - do: list + status: 200 + present: false + configMap: false diff --git a/internal/server/testdata/uninstalling/terminating-past-grace-ignored.yaml b/internal/server/testdata/uninstalling/terminating-past-grace-ignored.yaml new file mode 100644 index 0000000..e6cf707 --- /dev/null +++ b/internal/server/testdata/uninstalling/terminating-past-grace-ignored.yaml @@ -0,0 +1,27 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +nodes: + - name: gpu-1 + ready: true +pods: + - name: engine-0 + phase: Running + node: gpu-1 + deletionTimestamp: 2026-10-22T12:00:00Z + terminationGracePeriodSeconds: 30 + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: setNow + now: 2026-10-22T12:06:00Z + - do: uninstall + status: 200 + planError: false + - do: dropHelm + - do: list + status: 200 + present: false + configMap: false diff --git a/internal/server/testdata/uninstalling/terminating-within-grace-keeps-uninstalling.yaml b/internal/server/testdata/uninstalling/terminating-within-grace-keeps-uninstalling.yaml new file mode 100644 index 0000000..a69e514 --- /dev/null +++ b/internal/server/testdata/uninstalling/terminating-within-grace-keeps-uninstalling.yaml @@ -0,0 +1,29 @@ +kind: helm +release: glm-53 +namespace: models +chart: vllm +nodes: + - name: gpu-1 + ready: true +pods: + - name: engine-0 + phase: Running + node: gpu-1 + deletionTimestamp: 2026-10-22T12:00:00Z + deletionGracePeriodSeconds: 30 + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: setNow + now: 2026-10-22T12:00:30Z + - do: uninstall + status: 200 + planError: false + configMap: true + - do: dropHelm + - do: list + status: 200 + present: true + phase: uninstalling + configMap: true diff --git a/internal/server/testdata/uninstalling/uninstall-while-apply-holds-lock.yaml b/internal/server/testdata/uninstalling/uninstall-while-apply-holds-lock.yaml new file mode 100644 index 0000000..c2a0ef1 --- /dev/null +++ b/internal/server/testdata/uninstalling/uninstall-while-apply-holds-lock.yaml @@ -0,0 +1,29 @@ +# An uninstall cannot start in the middle of an apply. The apply holds the +# release's lock; the DELETE is 409 and the plan is not marked. +kind: helm +release: glm-53 +namespace: models +chart: vllm +pods: + - name: engine-0 + phase: Running + labels: + app: glm-53-vllm + app.kubernetes.io/instance: glm-53 +steps: + - do: holdLock + - do: uninstall + status: 409 + errorContains: apply in progress + configMap: true + - do: list + status: 200 + present: true + phase: applied + - do: releaseLock + - do: uninstall + status: 200 + - do: list + status: 200 + present: true + phase: uninstalling diff --git a/internal/server/uninstalling.go b/internal/server/uninstalling.go new file mode 100644 index 0000000..938926f --- /dev/null +++ b/internal/server/uninstalling.go @@ -0,0 +1,705 @@ +package server + +import ( + "context" + "errors" + "fmt" + "maps" + "net/http" + "sort" + "strings" + "time" + + "gopkg.in/yaml.v3" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + + "github.com/modelsphere/swiss/internal/cluster" + "github.com/modelsphere/swiss/internal/exec" + "github.com/modelsphere/swiss/internal/llmsvc" + "github.com/modelsphere/swiss/internal/plan" +) + +// uninstallInProgress is the DELETE body when the release is already +// uninstalling and this call does not ask again. +const uninstallInProgress = "uninstall already in progress" + +// uninstallPoll is how often a running swissd looks for pods that have +// finished terminating. The deployments list and a release's status do the +// same check when they are read, so a poll is not the only way a row drops. +const uninstallPoll = 5 * time.Second + +// stuckTerminatingExtra is how long past a pod's grace period a deleting pod +// still counts as remaining. Past that it is treated as gone. +const stuckTerminatingExtra = 5 * time.Minute + +// defaultTerminationGrace is what Kubernetes uses when a pod sets neither +// deletionGracePeriodSeconds nor terminationGracePeriodSeconds. +const defaultTerminationGrace = 30 * time.Second + +// statusFile is the key writePlan stores beside the plan files. +const statusFile = "status.yaml" + +type uninstState struct { + inflight bool + // llm means the operator owns teardown. Reap waits for the LLMService to + // disappear and does not scan pods or helm. + llm bool + // started is the request time, or the deletionTimestamp after a restart. + // The final audit row uses it, so a repeat DELETE must not overwrite it. + started time.Time + planHash string + // finalAudit is set once cleanup has succeeded, so a delete error retries + // the row and a second reap does not write it again. + finalAudit bool +} + +func (s *Server) uninstallLoop(ctx context.Context) { + s.reapUninstalling(ctx) + ticker := time.NewTicker(uninstallPoll) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + s.reapUninstalling(ctx) + } + } +} + +func (s *Server) trackUninstalling(ns, release string) { + s.uninstMu.Lock() + defer s.uninstMu.Unlock() + s.trackLocked(ns + "/" + release) +} + +func (s *Server) trackLocked(key string) { + if s.uninst == nil { + s.uninst = map[string]*uninstState{} + } + if s.uninst[key] == nil { + s.uninst[key] = &uninstState{} + } +} + +func (s *Server) beginUninstall(ns, release string) bool { + s.uninstMu.Lock() + defer s.uninstMu.Unlock() + key := ns + "/" + release + if st := s.uninst[key]; st != nil && st.inflight { + return false + } + s.trackLocked(key) + s.uninst[key].inflight = true + return true +} + +func (s *Server) endUninstall(ns, release string) { + s.uninstMu.Lock() + defer s.uninstMu.Unlock() + if st := s.uninst[ns+"/"+release]; st != nil { + st.inflight = false + } +} + +func (s *Server) forgetUninstalling(ns, release string) { + s.uninstMu.Lock() + defer s.uninstMu.Unlock() + delete(s.uninst, ns+"/"+release) +} + +// trackLLMUninstall marks a release whose LLMService the operator is deleting. +// planHash and started stick to whatever was recorded first: a repeat DELETE +// and a restart must not move the final audit's clock. +func (s *Server) trackLLMUninstall(ns, release, planHash string, started time.Time) { + s.uninstMu.Lock() + defer s.uninstMu.Unlock() + key := ns + "/" + release + s.trackLocked(key) + st := s.uninst[key] + st.llm = true + if st.planHash == "" { + st.planHash = planHash + } + if st.started.IsZero() && !started.IsZero() { + st.started = started + } +} + +func (s *Server) llmUninstalling(ns, release string) bool { + s.uninstMu.Lock() + defer s.uninstMu.Unlock() + st := s.uninst[ns+"/"+release] + return st != nil && st.llm +} + +// claimLLMAudit returns the stored start and hash once, after cleanup has +// succeeded. A zero start means the process restarted without a timestamp. +func (s *Server) claimLLMAudit(ns, release string) (time.Time, string, bool) { + s.uninstMu.Lock() + defer s.uninstMu.Unlock() + st := s.uninst[ns+"/"+release] + if st == nil || st.finalAudit { + return time.Time{}, "", false + } + st.finalAudit = true + started := st.started + if started.IsZero() { + started = s.clock() + } + return started, st.planHash, true +} + +func (s *Server) uninstallInflight(ns, release string) bool { + s.uninstMu.Lock() + defer s.uninstMu.Unlock() + st := s.uninst[ns+"/"+release] + return st != nil && st.inflight +} + +func (s *Server) uninstallingKeys() []string { + s.uninstMu.Lock() + defer s.uninstMu.Unlock() + out := make([]string, 0, len(s.uninst)) + for key, st := range s.uninst { + if st != nil && st.inflight { + continue + } + out = append(out, key) + } + sort.Strings(out) + return out +} + +// planPhase is the phase recorded in the plan ConfigMap, or "" when there is +// no plan. A read error is "" as well: the caller that cares about the +// difference is the one writing the phase. +func (s *Server) planPhase(ctx context.Context, ns, release string) string { + data, err := s.probe.ConfigMap(ctx, planRef(ns, release)) + if err != nil || len(data) == 0 { + return "" + } + return parseStatus([]byte(data[statusFile])).Phase +} + +// markUninstalling writes the phase and the metadata label before the release +// is removed. hadPlan is false when there is nothing beside the release to keep listed. +func (s *Server) markUninstalling(ctx context.Context, ns, release string) (hadPlan bool, err error) { + ref := planRef(ns, release) + data, err := s.probe.ConfigMap(ctx, ref) + if err != nil { + if apierrors.IsNotFound(err) { + return false, nil + } + return false, err + } + if len(data) == 0 { + return false, nil + } + if s.writer == nil { + return false, fmt.Errorf("no cluster writer") + } + prev := parseStatus([]byte(data[statusFile])) + next := planStatus{ + Phase: phaseUninstalling, + Action: "uninstall", + Revision: prev.Revision, + StartedAt: stamp(time.Now()), + Note: prev.Note, + } + files := maps.Clone(data) + raw, err := yaml.Marshal(next) + if err != nil { + return false, err + } + files[statusFile] = string(raw) + if err := s.writer.PutConfigMap(ctx, ref, files, phaseLabel(next.Phase)); err != nil { + return false, err + } + return true, nil +} + +// finishUninstallFailed puts the plan back to failed so a destroy error is +// not stuck on uninstalling, and drops the metadata label with it. +func (s *Server) finishUninstallFailed(ctx context.Context, ns, release string, cause error) error { + if s.writer == nil { + return fmt.Errorf("no cluster writer") + } + ref := planRef(ns, release) + data, err := s.probe.ConfigMap(ctx, ref) + if err != nil { + return err + } + if len(data) == 0 { + return fmt.Errorf("plan configmap %s is empty", ref) + } + prev := parseStatus([]byte(data[statusFile])) + next := planStatus{ + Phase: phaseFailed, + Action: "uninstall", + Revision: prev.Revision, + StartedAt: prev.StartedAt, + UpdatedAt: stamp(time.Now()), + Error: cause.Error(), + Note: prev.Note, + } + files := maps.Clone(data) + raw, err := yaml.Marshal(next) + if err != nil { + return err + } + files[statusFile] = string(raw) + return s.writer.PutConfigMap(ctx, ref, files, nil) +} + +// snapshotPlan copies the plan ConfigMap so a delete that finds nothing can +// put it back. A missing map is an empty snapshot, not an error. +func (s *Server) snapshotPlan(ctx context.Context, ns, release string) (map[string]string, error) { + data, err := s.probe.ConfigMap(ctx, planRef(ns, release)) + if err != nil { + if apierrors.IsNotFound(err) { + return nil, nil + } + return nil, err + } + if len(data) == 0 { + return nil, nil + } + return maps.Clone(data), nil +} + +// restorePlan writes a snapshot back, including the phase label it had. An +// empty snapshot is a release that had no plan. +func (s *Server) restorePlan(ctx context.Context, ns, release string, data map[string]string) error { + if len(data) == 0 { + return nil + } + if s.writer == nil { + return fmt.Errorf("no cluster writer") + } + phase := parseStatus([]byte(data[statusFile])).Phase + return s.writer.PutConfigMap(ctx, planRef(ns, release), data, phaseLabel(phase)) +} + +// errUninstalling is what refuseUninstalling wraps, so the autoscaler can tell +// it from a failed apply. +var errUninstalling = errors.New("release is uninstalling") + +func uninstallingFailure(ns, release string) error { + return &httpFailure{code: http.StatusConflict, msg: fmt.Sprintf("release %s/%s is uninstalling", ns, release), err: errUninstalling} +} + +// refuseUninstalling is the 409 for install, upgrade, rollback and the +// autoscaler. applyLocked calls it under the release's lock. +func (s *Server) refuseUninstalling(ctx context.Context, ns, release string) error { + if s.planPhase(ctx, ns, release) == phaseUninstalling { + return uninstallingFailure(ns, release) + } + // A cached Forbidden hides the CRD the same way an unserved one does. Calling + // through it would turn every helm apply into another refused get. + if s.llms == nil || !s.llmsvcAccess(ctx).visible() { + return nil + } + u, err := s.llms.Get(ctx, ns, release) + if apierrors.IsForbidden(err) { + s.noteLLMSVCForbidden() + return nil + } + if err != nil || u.GetDeletionTimestamp().IsZero() { + return nil + } + return uninstallingFailure(ns, release) +} + +func (s *Server) reapUninstalling(ctx context.Context) { + s.ensureUninstalling(ctx) + for _, key := range s.uninstallingKeys() { + ns, name, ok := strings.Cut(key, "/") + if !ok { + continue + } + s.reapRelease(ctx, ns, name) + } +} + +// ensureUninstalling fills the in-memory set from plan ConfigMaps once. +// Later marks are tracked as they are written; a restarted process has an +// empty set and learns it here, from the label. +func (s *Server) ensureUninstalling(ctx context.Context) { + s.uninstMu.Lock() + ready := s.uninstReady + s.uninstMu.Unlock() + if ready { + return + } + if err := s.rebuildUninstalling(ctx); err != nil { + return + } + s.uninstMu.Lock() + s.uninstReady = true + s.uninstMu.Unlock() +} + +func (s *Server) rebuildUninstalling(ctx context.Context) error { + refs, err := s.probe.ManagedRefs(ctx) + if err != nil { + s.log.WarnContext(ctx, "uninstalling set not rebuilt", "err", err) + return err + } + seen := map[string]bool{} + for _, ref := range refs { + if ref.Labels[cluster.LabelPhase] != phaseUninstalling { + continue + } + key := ref.Namespace + "/" + ref.Name + seen[key] = true + s.trackUninstalling(ref.Namespace, ref.Name) + } + if err := s.trackDeletingLLMServices(ctx, seen); err != nil { + return err + } + s.uninstMu.Lock() + defer s.uninstMu.Unlock() + for key, st := range s.uninst { + // An llm entry stays even when the object is already gone: the reap + // still has to drop the plan, and a rebuild must not turn that into + // a helm pod scan. + if st != nil && (st.inflight || st.llm) || seen[key] { + continue + } + delete(s.uninst, key) + } + return nil +} + +// trackDeletingLLMServices records LLMServices the operator is still deleting. +// A labeled plan whose service still exists is the same kind of uninstall: the +// operator owns the teardown, so the helm pod scan must not run. A list error +// leaves the set unready so the next poll tries again. Forbidden is a grant +// problem, not a reason to treat every uninstalling plan as an LLMService. +func (s *Server) trackDeletingLLMServices(ctx context.Context, seen map[string]bool) error { + if s.llms == nil || !s.llmsvcAccess(ctx).visible() { + return nil + } + list, err := s.llms.List(ctx, s.releaseNamespaces()) + if apierrors.IsForbidden(err) { + s.noteLLMSVCForbidden() + return nil + } + if err != nil { + s.log.WarnContext(ctx, "llmservices not listed, uninstalling set not rebuilt", "err", err) + return err + } + found := map[string]struct { + hash string + started time.Time + }{} + for _, u := range list { + if u == nil { + continue + } + key := u.GetNamespace() + "/" + u.GetName() + info := struct { + hash string + started time.Time + }{hash: u.GetAnnotations()[llmsvc.AnnPlanHash]} + if ts := u.GetDeletionTimestamp(); ts != nil && !ts.IsZero() { + info.started = ts.Time + seen[key] = true + s.trackLLMUninstall(u.GetNamespace(), u.GetName(), info.hash, info.started) + } + found[key] = info + } + for key := range seen { + info, ok := found[key] + if !ok { + continue + } + ns, name, ok := strings.Cut(key, "/") + if !ok { + continue + } + s.trackLLMUninstall(ns, name, info.hash, info.started) + } + return nil +} + +// reapRelease drops the plan once helm no longer has the release and no pod +// matches either selector. The autoscale ConfigMap goes first, at the same +// point: the autoscaler holds while the release is uninstalling, and its +// config outlives the release only if that delete fails, which keeps the +// release listed until a later check removes it. Once helm has dropped the +// release, archive Secrets are removed even while pods remain, so a restart +// between the helm uninstall and that delete does not leave them behind. A +// lookup or list error leaves the release listed; a delete error is retried +// on the next check. The returned string is the delete error, for the +// uninstall response. +func (s *Server) reapRelease(ctx context.Context, ns, release string) string { + if s.uninstallInflight(ns, release) { + return "" + } + if s.llmUninstalling(ns, release) { + return s.reapLLM(ctx, ns, release) + } + ref := planRef(ns, release) + data, err := s.probe.ConfigMap(ctx, ref) + if apierrors.IsNotFound(err) || err == nil && len(data) == 0 { + s.forgetUninstalling(ns, release) + return "" + } + if err != nil { + s.log.WarnContext(ctx, "plan not read, release stays listed", "namespace", ns, "release", release, "err", err) + s.trackUninstalling(ns, release) + return "" + } + if parseStatus([]byte(data[statusFile])).Phase != phaseUninstalling { + s.forgetUninstalling(ns, release) + return "" + } + remain, err := s.podsRemain(ctx, ns, release, data) + if err != nil { + s.log.WarnContext(ctx, "pods not listed, release stays listed", "namespace", ns, "release", release, "err", err) + s.trackUninstalling(ns, release) + return "" + } + // Helm still recording the release means uninstall did not finish. Dropping + // the plan here would make a swiss release look hand-installed. A lookup + // that fails is the same as a pod list that fails: keep the row. The lookup + // happens even when pods remain, because the archive Secrets go as soon as + // helm has dropped the release and do not wait for those pods. + st, err := exec.Lookup(ctx, s.probe, ns, release) + if err != nil { + s.log.WarnContext(ctx, "helm release not read, release stays listed", "namespace", ns, "release", release, "err", err) + s.trackUninstalling(ns, release) + return "" + } + if !st.Exists { + s.deleteArchives(ctx, ns, release) + } + if remain || st.Exists { + s.trackUninstalling(ns, release) + return "" + } + if s.writer == nil { + s.trackUninstalling(ns, release) + return "" + } + if err := s.removeAutoscale(ctx, ns, release); err != nil { + s.log.ErrorContext(ctx, "autoscale configmap not removed", "release", release, "err", err) + s.trackUninstalling(ns, release) + return err.Error() + } + if err := s.writer.DeleteConfigMap(ctx, ref); err != nil { + s.log.ErrorContext(ctx, "plan configmap not removed", "release", release, "err", err) + s.trackUninstalling(ns, release) + return err.Error() + } + s.forgetUninstalling(ns, release) + return "" +} + +// reapLLM finishes an llmsvc uninstall once the operator has dropped the +// object. While the object exists, the operator is still working and a pod +// scan would race it. A recreated object (no deletionTimestamp) is someone +// else's release again, so tracking stops and nothing is deleted. +func (s *Server) reapLLM(ctx context.Context, ns, release string) string { + if s.llms == nil || !s.llmsvcAccess(ctx).visible() { + return "" + } + u, err := s.llms.Get(ctx, ns, release) + if apierrors.IsNotFound(err) { + return s.finishLLMGone(ctx, ns, release) + } + if apierrors.IsForbidden(err) { + s.noteLLMSVCForbidden() + return "" + } + if err != nil { + s.log.WarnContext(ctx, "llmservice not read, uninstall stays tracked", "namespace", ns, "release", release, "err", err) + return "" + } + if u.GetDeletionTimestamp().IsZero() { + s.log.InfoContext(ctx, "llmservice recreated, uninstall tracking dropped", "namespace", ns, "release", release) + s.forgetUninstalling(ns, release) + return "" + } + s.trackLLMUninstall(ns, release, u.GetAnnotations()[llmsvc.AnnPlanHash], u.GetDeletionTimestamp().Time) + return "" +} + +// finishLLMGone drops what swiss wrote beside the release, then records one +// audit row. A cleanup error leaves the release tracked so the next check +// retries, including the audit. +func (s *Server) finishLLMGone(ctx context.Context, ns, release string) string { + if s.writer == nil { + return "" + } + s.deleteArchives(ctx, ns, release) + if err := s.removeAutoscale(ctx, ns, release); err != nil { + s.log.ErrorContext(ctx, "autoscale configmap not removed", "release", release, "err", err) + return err.Error() + } + ref := planRef(ns, release) + data, err := s.probe.ConfigMap(ctx, ref) + if err != nil && !apierrors.IsNotFound(err) { + s.log.WarnContext(ctx, "plan not read, release stays listed", "namespace", ns, "release", release, "err", err) + return "" + } + if err == nil && len(data) > 0 && parseStatus([]byte(data[statusFile])).Phase == phaseUninstalling { + if err := s.writer.DeleteConfigMap(ctx, ref); err != nil { + s.log.ErrorContext(ctx, "plan configmap not removed", "release", release, "err", err) + return err.Error() + } + } + if started, hash, ok := s.claimLLMAudit(ns, release); ok { + s.recordRelease(ctx, "uninstall", ns, release, hash, exec.Result{Output: llmGone(ns, release)}, nil, started, "", 0) + } + s.forgetUninstalling(ns, release) + return "" +} + +func llmDeleteRequested(ns, release string) string { + return fmt.Sprintf("LLMService %s/%s deletion requested", ns, release) +} + +func llmGone(ns, release string) string { + return fmt.Sprintf("LLMService %s/%s is gone", ns, release) +} + +// podsRemain is true when any pod matches the chart label or the helm instance +// label. Finished pods (Succeeded or Failed) are ignored: helm test and hook +// Jobs leave them behind after the workload is gone. A pod that is being +// deleted still counts during its grace period. It is ignored only once it is +// stuck: now is past deletionTimestamp plus the grace period plus five +// minutes, or the grace period is over and its node is NotReady or not in the +// node list. A node that is NotReady for a moment does not end the uninstall +// while its pods may still be shutting down. Grace is +// metadata.deletionGracePeriodSeconds, else spec.terminationGracePeriodSeconds, +// else 30s. An unscheduled pod has no node, so only the time rule applies. +// Node readiness is listed only when a matched pod is terminating; if that +// list fails, a terminating pod stays counted unless the time rule already +// says it is stuck. Engine pods carry app=- and need not carry +// the instance label, so either selector still having a pod keeps the release. +func (s *Server) podsRemain(ctx context.Context, ns, release string, files map[string]string) (bool, error) { + sels, err := podSelectors(release, files) + if err != nil { + return true, err + } + var matched []cluster.Pod + terminating := false + for _, sel := range sels { + pods, err := s.probe.Pods(ctx, ns, sel) + if err != nil { + return true, err + } + for _, pod := range pods { + if pod.Phase == string(corev1.PodSucceeded) || pod.Phase == string(corev1.PodFailed) { + continue + } + matched = append(matched, pod) + if podTerminating(pod) { + terminating = true + } + } + } + if len(matched) == 0 { + return false, nil + } + // A node list is cluster-wide. Skip it unless a terminating pod needs it. + var nodeReady func(string) (ready, found bool) + if terminating { + nodeReady = s.nodeReady(ctx, ns, release) + } + now := s.clock() + for _, pod := range matched { + if podStuckTerminating(pod, now, nodeReady) { + continue + } + return true, nil + } + return false, nil +} + +func (s *Server) clock() time.Time { + if s.now != nil { + return s.now() + } + return time.Now() +} + +// nodeReady is nil when the node list cannot be read. Callers then apply only +// the time rule, which keeps a terminating pod that is still inside its window. +func (s *Server) nodeReady(ctx context.Context, ns, release string) func(string) (ready, found bool) { + nodes, err := s.probe.Nodes(ctx) + if err != nil { + s.log.WarnContext(ctx, "nodes not listed, only the grace rule can ignore a terminating pod", + "namespace", ns, "release", release, "err", err) + return nil + } + ready := make(map[string]bool, len(nodes)) + for _, n := range nodes { + ready[n.Name] = n.Ready + } + return func(name string) (bool, bool) { + r, ok := ready[name] + return r, ok + } +} + +func podTerminating(p cluster.Pod) bool { + return p.DeletionTimestamp != nil && !p.DeletionTimestamp.IsZero() +} + +// podStuckTerminating reports whether a deleting pod should be treated as gone. +// Inside its grace period it never is. After that, a NotReady or missing node +// is enough; otherwise it takes the five extra minutes. nodeReady is nil when +// the node list could not be read: only the time rule applies, so a pod still +// inside its window counts as remaining. An empty node name is unscheduled and +// is not treated as a missing node. +func podStuckTerminating(p cluster.Pod, now time.Time, nodeReady func(string) (ready, found bool)) bool { + if !podTerminating(p) { + return false + } + grace := podGrace(p) + if now.After(p.DeletionTimestamp.Add(grace + stuckTerminatingExtra)) { + return true + } + if !now.After(p.DeletionTimestamp.Add(grace)) { + return false + } + if p.Node == "" || nodeReady == nil { + return false + } + ready, found := nodeReady(p.Node) + return !found || !ready +} + +func podGrace(p cluster.Pod) time.Duration { + if p.DeletionGracePeriodSeconds != nil { + return time.Duration(*p.DeletionGracePeriodSeconds) * time.Second + } + if p.TerminationGracePeriodSeconds != nil { + return time.Duration(*p.TerminationGracePeriodSeconds) * time.Second + } + return defaultTerminationGrace +} + +func podSelectors(release string, files map[string]string) ([]string, error) { + instance := "app.kubernetes.io/instance=" + release + if len(files) == 0 { + return []string{instance}, nil + } + p, err := plan.FromFiles(files) + if err != nil { + return nil, err + } + if p.Chart.Name == "" { + return []string{instance}, nil + } + appSel := fmt.Sprintf("app=%s-%s", release, p.Chart.Name) + if appSel == instance { + return []string{instance}, nil + } + return []string{instance, appSel}, nil +} diff --git a/internal/server/uninstalling_cases_test.go b/internal/server/uninstalling_cases_test.go new file mode 100644 index 0000000..30ea27e --- /dev/null +++ b/internal/server/uninstalling_cases_test.go @@ -0,0 +1,25 @@ +package server + +import ( + "path/filepath" + "sort" + "strings" + "testing" +) + +func TestUninstallingCases(t *testing.T) { + paths, err := filepath.Glob("testdata/uninstalling/*.yaml") + if err != nil { + t.Fatal(err) + } + if len(paths) == 0 { + t.Fatal("no uninstalling cases under testdata/uninstalling") + } + sort.Strings(paths) + for _, path := range paths { + name := strings.TrimSuffix(filepath.Base(path), ".yaml") + t.Run(name, func(t *testing.T) { + runUninstallingCase(t, path) + }) + } +} diff --git a/internal/server/uninstalling_stuck_test.go b/internal/server/uninstalling_stuck_test.go new file mode 100644 index 0000000..18d2b0b --- /dev/null +++ b/internal/server/uninstalling_stuck_test.go @@ -0,0 +1,143 @@ +package server + +import ( + "testing" + "time" + + "github.com/modelsphere/swiss/internal/cluster" +) + +func TestPodStuckTerminating(t *testing.T) { + del := time.Date(2026, 10, 22, 12, 0, 0, 0, time.UTC) + ready := func(name string) (bool, bool) { + switch name { + case "gpu-1": + return true, true + case "gpu-down": + return false, true + default: + return false, false + } + } + window := 30*time.Second + 5*time.Minute + cases := []struct { + name string + pod cluster.Pod + now time.Time + nodes func(string) (bool, bool) + want bool + }{ + { + name: "not terminating", + pod: cluster.Pod{Phase: "Running", Node: "gpu-down"}, + now: del, + nodes: ready, + }, + { + name: "within deletion grace on a ready node", + pod: cluster.Pod{DeletionTimestamp: &del, DeletionGracePeriodSeconds: secs(30), TerminationGracePeriodSeconds: secs(90), Node: "gpu-1"}, + now: del.Add(window), + nodes: ready, + }, + { + name: "past deletion grace", + pod: cluster.Pod{DeletionTimestamp: &del, DeletionGracePeriodSeconds: secs(30), TerminationGracePeriodSeconds: secs(90), Node: "gpu-1"}, + now: del.Add(window + time.Nanosecond), + nodes: ready, + want: true, + }, + { + name: "termination grace when deletion grace is unset", + pod: cluster.Pod{DeletionTimestamp: &del, TerminationGracePeriodSeconds: secs(10), Node: "gpu-1"}, + now: del.Add(10*time.Second + 5*time.Minute + time.Second), + nodes: ready, + want: true, + }, + { + name: "default 30s is not yet stuck", + pod: cluster.Pod{DeletionTimestamp: &del, Node: "gpu-1"}, + now: del.Add(30*time.Second + 5*time.Minute), + nodes: ready, + }, + { + name: "default 30s just past", + pod: cluster.Pod{DeletionTimestamp: &del, Node: "gpu-1"}, + now: del.Add(30*time.Second + 5*time.Minute + time.Nanosecond), + nodes: ready, + want: true, + }, + { + name: "zero deletion grace is set and beats termination grace", + pod: cluster.Pod{DeletionTimestamp: &del, DeletionGracePeriodSeconds: secs(0), TerminationGracePeriodSeconds: secs(90), Node: "gpu-1"}, + now: del.Add(5*time.Minute + time.Nanosecond), + nodes: ready, + want: true, + }, + { + name: "notready node inside the grace period", + pod: cluster.Pod{DeletionTimestamp: &del, DeletionGracePeriodSeconds: secs(600), Node: "gpu-down"}, + now: del.Add(600 * time.Second), + nodes: ready, + }, + { + name: "notready node past the grace period", + pod: cluster.Pod{DeletionTimestamp: &del, DeletionGracePeriodSeconds: secs(600), Node: "gpu-down"}, + now: del.Add(600*time.Second + time.Nanosecond), + nodes: ready, + want: true, + }, + { + name: "node missing from the list inside the grace period", + pod: cluster.Pod{DeletionTimestamp: &del, DeletionGracePeriodSeconds: secs(600), Node: "gpu-gone"}, + now: del, + nodes: ready, + }, + { + name: "node missing from the list past the grace period", + pod: cluster.Pod{DeletionTimestamp: &del, DeletionGracePeriodSeconds: secs(600), Node: "gpu-gone"}, + now: del.Add(600*time.Second + time.Nanosecond), + nodes: ready, + want: true, + }, + { + name: "notready node with the default grace", + pod: cluster.Pod{DeletionTimestamp: &del, Node: "gpu-down"}, + now: del.Add(31 * time.Second), + nodes: ready, + want: true, + }, + { + name: "unscheduled pod inside the window", + pod: cluster.Pod{DeletionTimestamp: &del, DeletionGracePeriodSeconds: secs(30)}, + now: del, + nodes: ready, + }, + { + name: "unscheduled pod past the window", + pod: cluster.Pod{DeletionTimestamp: &del, DeletionGracePeriodSeconds: secs(30)}, + now: del.Add(window + time.Second), + nodes: ready, + want: true, + }, + { + name: "nodes unread keeps a pod inside the window", + pod: cluster.Pod{DeletionTimestamp: &del, DeletionGracePeriodSeconds: secs(30), Node: "gpu-down"}, + now: del, + }, + { + name: "nodes unread still honors the time rule", + pod: cluster.Pod{DeletionTimestamp: &del, DeletionGracePeriodSeconds: secs(30), Node: "gpu-down"}, + now: del.Add(window + time.Second), + want: true, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := podStuckTerminating(tc.pod, tc.now, tc.nodes); got != tc.want { + t.Fatalf("stuck=%v, want %v", got, tc.want) + } + }) + } +} + +func secs(n int64) *int64 { return &n } diff --git a/internal/server/uninstalling_test.go b/internal/server/uninstalling_test.go new file mode 100644 index 0000000..820348c --- /dev/null +++ b/internal/server/uninstalling_test.go @@ -0,0 +1,1278 @@ +package server + +import ( + "context" + "encoding/json" + "fmt" + "maps" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "sort" + "strconv" + "strings" + "sync" + "testing" + "time" + + "gopkg.in/yaml.v3" + apierrors "k8s.io/apimachinery/pkg/api/errors" + 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/runtime/schema" + k8stesting "k8s.io/client-go/testing" + + "github.com/modelsphere/swiss/internal/cluster" + "github.com/modelsphere/swiss/internal/llmsvc" + "github.com/modelsphere/swiss/internal/plan" + "github.com/modelsphere/swiss/internal/store" +) + +type uninstallCase struct { + Kind string `yaml:"kind"` + Release string `yaml:"release"` + Namespace string `yaml:"namespace"` + Chart string `yaml:"chart"` + HelmExit int `yaml:"helmExit"` + PodListError bool `yaml:"podListError"` + DeleteConfigMapFails int `yaml:"deleteConfigMapFails"` + SeedArchive bool `yaml:"seedArchive"` + SeedAutoscale bool `yaml:"seedAutoscale"` + // HelmBlocks makes helm uninstall wait for a releaseHelm step, so a case + // can act while an uninstall holds the release's lock. + HelmBlocks bool `yaml:"helmBlocks"` + OperatorPhase string `yaml:"operatorPhase"` + // Deleting nil means the LLMService is created with a deletionTimestamp, + // which is what the older cases describe. false creates it without one. + Deleting *bool `yaml:"deleting"` + Finalizer bool `yaml:"finalizer"` + DeleteNotFound bool `yaml:"deleteNotFound"` + Missing bool `yaml:"missing"` + SeedPlanConfigMap bool `yaml:"seedPlanConfigMap"` + PlanPhase string `yaml:"planPhase"` + Deletion map[string]any `yaml:"deletion"` + Nodes []nodeSpec `yaml:"nodes"` + Pods []podSpec `yaml:"pods"` + Steps []unstep `yaml:"steps"` +} + +type nodeSpec struct { + Name string `yaml:"name"` + Ready bool `yaml:"ready"` +} + +type podSpec struct { + Name string `yaml:"name"` + Phase string `yaml:"phase"` + Namespace string `yaml:"namespace"` + Node string `yaml:"node"` + DeletionTimestamp time.Time `yaml:"deletionTimestamp"` + DeletionGracePeriodSeconds *int64 `yaml:"deletionGracePeriodSeconds"` + TerminationGracePeriodSeconds *int64 `yaml:"terminationGracePeriodSeconds"` + Labels map[string]string `yaml:"labels"` +} + +type unstep struct { + Do string `yaml:"do"` + Status int `yaml:"status"` + Present *bool `yaml:"present"` + Phase string `yaml:"phase"` + PlanPhase string `yaml:"planPhase"` + ServicePhase string `yaml:"servicePhase"` + ConfigMap *bool `yaml:"configMap"` + PlanError *bool `yaml:"planError"` + ErrorContains string `yaml:"errorContains"` + OutputContains string `yaml:"outputContains"` + MinPods *int `yaml:"minPods"` + TotalPods *int `yaml:"totalPods"` + Backend string `yaml:"backend"` + Pods []podSpec `yaml:"pods"` + Archives *int `yaml:"archives"` + // Autoscale is whether swiss-autoscale- exists after the step. + Autoscale *bool `yaml:"autoscale"` + Now time.Time `yaml:"now"` + HasRevision *bool `yaml:"hasRevision"` + OperatorPhase string `yaml:"operatorPhase"` + DeletionStage string `yaml:"deletionStage"` + DeletionOutcome string `yaml:"deletionOutcome"` + DeletionMessage string `yaml:"deletionMessage"` + DeletionDeadline string `yaml:"deletionDeadline"` + PodsRemaining *int `yaml:"podsRemaining"` + DeletionPods *int `yaml:"deletionPods"` + ForceDeleted *int `yaml:"forceDeleted"` + DriftContains string `yaml:"driftContains"` + Audits *int `yaml:"audits"` + LLMService *bool `yaml:"llmService"` + StoredPhase string `yaml:"storedPhase"` +} + +func runUninstallingCase(t *testing.T, path string) { + t.Helper() + raw, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + var cs uninstallCase + if err := yaml.Unmarshal(raw, &cs); err != nil { + t.Fatal(err) + } + if cs.Kind == "" { + cs.Kind = "helm" + } + if cs.Namespace == "" { + cs.Namespace = "models" + } + if cs.Release == "" { + t.Fatal("release is required") + } + switch cs.Kind { + case "helm": + runHelmUninstallCase(t, &cs) + case "llmsvc": + runLLMUninstallCase(t, &cs) + default: + t.Fatalf("kind %q", cs.Kind) + } +} + +func runHelmUninstallCase(t *testing.T, cs *uninstallCase) { + t.Helper() + if cs.Chart == "" { + cs.Chart = "vllm" + } + c := newUninstallCluster(t, cs) + h := &helmHarness{cs: cs, c: c} + t.Cleanup(func() { + if h.srv != nil { + h.srv.Close() + } + }) + h.open(t) + for i, step := range cs.Steps { + t.Logf("step %d %s", i, step.Do) + code, body := h.do(t, step) + assertUnstep(t, cs, c, step, code, body, h.s, nil) + } +} + +type helmHarness struct { + cs *uninstallCase + c *uninstallCluster + srv *httptest.Server + s *Server + now time.Time + // gate is the file a blocked helm uninstall waits for. + gate string + // pending is the DELETE started by uninstallAsync. + pending chan asyncResult + locked bool +} + +// openGate lets a blocked helm return and waits for the DELETE it was part of. +func (h *helmHarness) openGate() (asyncResult, bool) { + if h.gate != "" { + _ = os.WriteFile(h.gate, nil, 0o644) + } + if h.pending == nil { + return asyncResult{}, true + } + select { + case r := <-h.pending: + h.pending = nil + return r, true + case <-time.After(10 * time.Second): + return asyncResult{}, false + } +} + +type asyncResult struct { + code int + body map[string]any +} + +func (h *helmHarness) open(t *testing.T) { + t.Helper() + if h.srv != nil { + h.srv.Close() + h.srv = nil + } + cfg := testConfig("prod-b300") + cfg.Server.AllowDeploy = true + cfg.Server.HelmBin = stubHelm(t, h.cs.HelmExit) + if h.cs.HelmBlocks { + if h.gate == "" { + h.gate = filepath.Join(t.TempDir(), "gate") + // Runs before the server closes and before the temp dir goes: a + // failed step must not leave helm, and with it the DELETE + // handler, waiting forever. + t.Cleanup(func() { h.openGate() }) + } + cfg.Server.HelmBin = stubHelmGated(t, h.cs.HelmExit, h.gate) + } + s := New(cfg, h.c, discardLogger(), "test") + db, err := store.Open(filepath.Join(t.TempDir(), "swiss.db")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { db.Close() }) + s.SetStore(db) + s.SetWriter(h.c) + h.s = s + if !h.now.IsZero() { + frozen := h.now + s.now = func() time.Time { return frozen } + } + h.srv = httptest.NewServer(s.Handler()) +} + +func (h *helmHarness) do(t *testing.T, step unstep) (int, map[string]any) { + t.Helper() + ns, rel := h.cs.Namespace, h.cs.Release + switch step.Do { + case "uninstall": + return del(t, h.srv, "/api/releases/"+ns+"/"+rel) + case "uninstallAsync": + // Returns once helm is running: the lock is held, the plan is marked. + if !h.cs.HelmBlocks { + t.Fatal("uninstallAsync needs helmBlocks") + } + h.pending = make(chan asyncResult, 1) + srv := h.srv + go func() { + code, body := delNoFatal(srv, "/api/releases/"+ns+"/"+rel) + h.pending <- asyncResult{code, body} + }() + deadline := time.Now().Add(10 * time.Second) + for !h.s.uninstallInflight(ns, rel) { + if time.Now().After(deadline) { + t.Fatal("uninstall did not start") + } + time.Sleep(5 * time.Millisecond) + } + return 0, nil + case "releaseHelm": + // Lets the blocked helm return; the step's status is the first DELETE's. + if h.pending == nil { + t.Fatal("releaseHelm without uninstallAsync") + } + r, ok := h.openGate() + if !ok { + t.Fatal("uninstall did not finish") + } + return r.code, r.body + case "holdLock": + // An apply in progress on this release, as far as the lock knows. + if !h.s.tryApplyLock(ns, rel) { + t.Fatal("lock already held") + } + h.locked = true + t.Cleanup(func() { + if h.locked { + h.s.unlockApply(ns, rel) + } + }) + return 0, nil + case "releaseLock": + if !h.locked { + t.Fatal("releaseLock without holdLock") + } + h.locked = false + h.s.unlockApply(ns, rel) + return 0, nil + case "list": + return get(t, h.srv, "/api/deployments?perPage=100") + case "status": + return get(t, h.srv, "/api/releases/"+ns+"/"+rel+"/status") + case "setPods": + h.c.setPods(ns, step.Pods) + return 0, nil + case "setNow": + if step.Now.IsZero() { + t.Fatal("setNow requires now") + } + h.now = step.Now + if h.s != nil { + frozen := h.now + h.s.now = func() time.Time { return frozen } + } + return 0, nil + case "seedArchives": + if err := h.c.seedArchive(ns, rel); err != nil { + t.Fatal(err) + } + return 0, nil + case "dropHelm": + h.c.dropHelm(ns, rel) + return 0, nil + case "restart": + h.open(t) + return 0, nil + case "install": + return h.mutate(t, "/api/install") + case "upgrade", "apply": + return h.mutate(t, "/api/apply") + case "rollback": + return post(t, h.srv, "/api/releases/"+ns+"/"+rel+"/rollback", map[string]any{ + "toRevision": 1, "expectRevision": 4, + }) + default: + t.Fatalf("unknown step %q", step.Do) + return 0, nil + } +} + +func (h *helmHarness) mutate(t *testing.T, path string) (int, map[string]any) { + t.Helper() + code, body := post(t, h.srv, "/api/plans", map[string]any{ + "model": "qwen3.6-35b-a3b", "release": h.cs.Release, + "namespace": h.cs.Namespace, "serviceId": h.cs.Release, + }) + if code != http.StatusOK { + return code, body + } + hash, _ := body["hash"].(string) + return post(t, h.srv, path, map[string]any{"planHash": hash}) +} + +func runLLMUninstallCase(t *testing.T, cs *uninstallCase) { + t.Helper() + if cs.OperatorPhase == "" { + cs.OperatorPhase = llmsvc.PhaseApplied + } + probe := llmProbe() + if probe.MapLabels == nil { + probe.MapLabels = map[string]map[string]string{} + } + if cs.SeedPlanConfigMap { + seedLLMPlan(t, &probe, cs) + } + if cs.SeedArchive { + seedLLMArchive(&probe, cs) + } + if cs.SeedAutoscale { + seedLLMAutoscale(&probe, cs) + } + // The rig operator would rewrite status, including status.deletion. + rig := startRig(t, probe, true, false, "") + base := rig.s.probe.(*cluster.Fake) + rig.s.probe = llmPlanProbe{Fake: base} + rig.s.SetWriter(&llmMirrorWriter{fakeWriter: rig.w, probe: base}) + h := &llmUninstallHarness{cs: cs, rig: rig, base: base, srv: rig.srv} + h.arm(t) + if !cs.Missing { + h.create(t) + } + ns, rel := cs.Namespace, cs.Release + for i, step := range cs.Steps { + t.Logf("step %d %s", i, step.Do) + var code int + var body map[string]any + switch step.Do { + case "list": + code, body = get(t, h.srv, "/api/deployments?perPage=100") + case "status": + code, body = get(t, h.srv, "/api/releases/"+ns+"/"+rel+"/status") + case "uninstall": + code, body = del(t, h.srv, "/api/releases/"+ns+"/"+rel) + case "reap": + h.rig.s.reapUninstalling(context.Background()) + case "operatorRelease": + if err := h.rig.dyn.Tracker().Delete(llmsvc.GVR, ns, rel); err != nil && !apierrors.IsNotFound(err) { + t.Fatal(err) + } + case "clearDeletion": + h.clearDeletion(t) + case "restart": + h.restart(t) + case "install": + code, body = llmMutate(t, h.rig, cs, "/api/install") + case "upgrade", "apply": + code, body = llmMutate(t, h.rig, cs, "/api/apply") + case "rollback": + code, body = post(t, h.srv, "/api/releases/"+ns+"/"+rel+"/rollback", map[string]any{ + "toRevision": 1, "expectRevision": 4, + }) + default: + t.Fatalf("unknown step %q", step.Do) + } + assertUnstep(t, cs, h.base, step, code, body, h.rig.s, h.rig.c) + } +} + +func llmMutate(t *testing.T, rig *llmRig, cs *uninstallCase, path string) (int, map[string]any) { + t.Helper() + hash := rig.storePlan(t, map[string]any{ + "release": cs.Release, "namespace": cs.Namespace, + }) + return post(t, rig.srv, path, map[string]any{"planHash": hash}) +} + +func assertUnstep(t *testing.T, cs *uninstallCase, p cluster.Probe, step unstep, code int, body map[string]any, srv *Server, llms *llmsvc.Client) { + t.Helper() + if step.Status != 0 && code != step.Status { + t.Fatalf("%s: status %d, body %v", step.Do, code, body) + } + if step.ErrorContains != "" && !strings.Contains(fmt.Sprint(body["error"]), step.ErrorContains) { + t.Fatalf("%s: error %q, want it to contain %q", step.Do, body["error"], step.ErrorContains) + } + if step.OutputContains != "" && !strings.Contains(fmt.Sprint(body["output"]), step.OutputContains) { + t.Fatalf("%s: output %q, want it to contain %q", step.Do, body["output"], step.OutputContains) + } + if step.PlanError != nil { + pe, _ := body["planError"].(string) + if *step.PlanError && pe == "" { + t.Fatalf("%s: expected planError, body %v", step.Do, body) + } + if !*step.PlanError && pe != "" { + t.Fatalf("%s: unexpected planError %q", step.Do, pe) + } + } + if step.Present != nil || step.Phase != "" || step.Backend != "" || step.HasRevision != nil { + if step.Do != "list" { + t.Fatalf("%s sets present, phase, backend, or hasRevision; those are read from a list step", step.Do) + } + row, found := deploymentRowIn(body, cs.Namespace, cs.Release) + if step.Present != nil && *step.Present != found { + t.Fatalf("present=%v, want %v, rows %v", found, *step.Present, body["deployments"]) + } + if step.Phase != "" { + if !found { + t.Fatalf("no row for %s/%s in %v", cs.Namespace, cs.Release, body["deployments"]) + } + if row["phase"] != step.Phase { + t.Fatalf("phase %v, want %s (%v)", row["phase"], step.Phase, row) + } + } + if step.Backend != "" && (!found || row["backend"] != step.Backend) { + t.Fatalf("backend %v, want %s", row, step.Backend) + } + if step.HasRevision != nil { + if !found { + t.Fatalf("no row for %s/%s in %v", cs.Namespace, cs.Release, body["deployments"]) + } + _, ok := row["revision"] + if ok != *step.HasRevision { + t.Fatalf("revision key present=%v, want %v, row %v", ok, *step.HasRevision, row) + } + } + } + if step.PlanPhase != "" || step.ServicePhase != "" || step.MinPods != nil || step.TotalPods != nil { + if step.Do != "status" { + t.Fatalf("%s sets a status field; those are read from a status step", step.Do) + } + if step.PlanPhase != "" { + ps, _ := body["planStatus"].(map[string]any) + if ps["phase"] != step.PlanPhase { + t.Fatalf("planStatus.phase %v, want %s (%v)", ps["phase"], step.PlanPhase, body["planStatus"]) + } + } + if step.ServicePhase != "" { + svc, _ := body["llmservice"].(map[string]any) + if svc["phase"] != step.ServicePhase { + t.Fatalf("llmservice.phase %v, want %s (%v)", svc["phase"], step.ServicePhase, body["llmservice"]) + } + } + total := asNum(body["total"]) + if step.MinPods != nil && total < *step.MinPods { + t.Fatalf("pods total %d, want at least %d (%v)", total, *step.MinPods, body["pods"]) + } + if step.TotalPods != nil && total != *step.TotalPods { + t.Fatalf("pods total %d, want %d (%v)", total, *step.TotalPods, body["pods"]) + } + } + if step.ConfigMap != nil || step.Autoscale != nil || step.Archives != nil || step.StoredPhase != "" { + if p == nil { + t.Fatal("resource assertions need a probe") + } + } + if step.ConfigMap != nil { + got := probeHasConfigMap(p, planRef(cs.Namespace, cs.Release)) + if got != *step.ConfigMap { + t.Fatalf("configMap present=%v, want %v", got, *step.ConfigMap) + } + } + if step.Autoscale != nil { + got := probeHasConfigMap(p, autoscaleRef(cs.Namespace, cs.Release)) + if got != *step.Autoscale { + t.Fatalf("autoscale configmap present=%v, want %v", got, *step.Autoscale) + } + } + if step.Archives != nil { + names, err := p.SecretNames(context.Background(), cs.Namespace, archiveSelector(cs.Release)) + if err != nil { + t.Fatal(err) + } + if len(names) != *step.Archives { + t.Fatalf("archives %v, want %d", names, *step.Archives) + } + } + if step.StoredPhase != "" { + data, err := p.ConfigMap(context.Background(), planRef(cs.Namespace, cs.Release)) + if err != nil { + t.Fatal(err) + } + if got := parseStatus([]byte(data[statusFile])).Phase; got != step.StoredPhase { + t.Fatalf("stored phase %q, want %q", got, step.StoredPhase) + } + } + if step.OperatorPhase != "" { + if step.Do != "status" { + t.Fatalf("%s sets operatorPhase; that is read from a status step", step.Do) + } + svc, _ := body["llmservice"].(map[string]any) + if svc["operatorPhase"] != step.OperatorPhase { + t.Fatalf("operatorPhase %v, want %s (%v)", svc["operatorPhase"], step.OperatorPhase, svc) + } + } + if step.DeletionStage != "" || step.DeletionOutcome != "" || step.DeletionMessage != "" || step.DeletionDeadline != "" || step.PodsRemaining != nil || step.DeletionPods != nil || step.ForceDeleted != nil { + del := deletionBlock(t, cs, step, body) + if step.DeletionStage != "" && del["stage"] != step.DeletionStage { + t.Fatalf("deletion.stage %v, want %s (%v)", del["stage"], step.DeletionStage, del) + } + if step.DeletionOutcome != "" && del["outcome"] != step.DeletionOutcome { + t.Fatalf("deletion.outcome %v, want %s (%v)", del["outcome"], step.DeletionOutcome, del) + } + if step.DeletionMessage != "" && del["message"] != step.DeletionMessage { + t.Fatalf("deletion.message %v, want %s", del["message"], step.DeletionMessage) + } + if step.DeletionDeadline != "" && del["deadline"] != step.DeletionDeadline { + t.Fatalf("deletion.deadline %v, want %s", del["deadline"], step.DeletionDeadline) + } + if step.PodsRemaining != nil && asNum(del["podsRemaining"]) != *step.PodsRemaining { + t.Fatalf("podsRemaining %v, want %d (%v)", del["podsRemaining"], *step.PodsRemaining, del) + } + if step.DeletionPods != nil { + pods, _ := del["pods"].([]any) + if len(pods) != *step.DeletionPods { + t.Fatalf("deletion pods %d, want %d (%v)", len(pods), *step.DeletionPods, del["pods"]) + } + } + if step.ForceDeleted != nil { + names, _ := del["forceDeleted"].([]any) + if len(names) != *step.ForceDeleted { + t.Fatalf("forceDeleted %d, want %d (%v)", len(names), *step.ForceDeleted, del["forceDeleted"]) + } + } + } + if step.DriftContains != "" { + if step.Do != "list" { + t.Fatalf("%s sets driftContains; that is read from a list step", step.Do) + } + row, found := deploymentRowIn(body, cs.Namespace, cs.Release) + if !found || !strings.Contains(fmt.Sprint(row["drift"]), step.DriftContains) { + t.Fatalf("drift %v, want it to contain %q", row["drift"], step.DriftContains) + } + } + if step.Audits != nil { + if srv == nil || srv.store == nil { + t.Fatal("audits need a store") + } + runs, err := srv.store.RunsFiltered(context.Background(), store.RunFilter{ + Namespace: cs.Namespace, Release: cs.Release, Action: "uninstall", Limit: 100, + }) + if err != nil { + t.Fatal(err) + } + if len(runs) != *step.Audits { + t.Fatalf("uninstall audits %d, want %d", len(runs), *step.Audits) + } + } + if step.LLMService != nil { + if llms == nil { + t.Fatal("llmService needs a client") + } + _, err := llms.Get(context.Background(), cs.Namespace, cs.Release) + exists := err == nil + if err != nil && !apierrors.IsNotFound(err) { + t.Fatal(err) + } + if exists != *step.LLMService { + t.Fatalf("llmservice exists=%v, want %v", exists, *step.LLMService) + } + } +} + +func probeHasConfigMap(p cluster.Probe, ref string) bool { + data, err := p.ConfigMap(context.Background(), ref) + if err != nil { + return false + } + return len(data) > 0 +} + +func deletionBlock(t *testing.T, cs *uninstallCase, step unstep, body map[string]any) map[string]any { + t.Helper() + switch step.Do { + case "status": + svc, _ := body["llmservice"].(map[string]any) + del, _ := svc["deletion"].(map[string]any) + if del == nil { + t.Fatalf("no llmservice.deletion in %v", svc) + } + return del + case "list": + row, found := deploymentRowIn(body, cs.Namespace, cs.Release) + if !found { + t.Fatalf("no row for %s/%s", cs.Namespace, cs.Release) + } + del, _ := row["deletion"].(map[string]any) + if del == nil { + t.Fatalf("no deletion on %v", row) + } + return del + default: + t.Fatalf("%s sets a deletion field; those are read from a list or status step", step.Do) + return nil + } +} + +func deploymentRowIn(body map[string]any, ns, release string) (map[string]any, bool) { + rows, _ := body["deployments"].([]any) + for _, raw := range rows { + row, _ := raw.(map[string]any) + if row["namespace"] == ns && row["release"] == release { + return row, true + } + } + return nil, false +} + +// uninstallCluster is one object behind both Probe and Writer, so a write the +// server makes is what the next read sees. Fake splits those, and its Pods +// ignores the selector, which is the difference these cases exist to pin. +type uninstallCluster struct { + cluster.Fake + mu sync.Mutex + cms map[string]*cmRec + pods []podFix + helm map[string]*helmRec + secs map[string]*secretRec + podErr error + deleteCMFails int +} + +type cmRec struct { + data map[string]string + labels map[string]string +} + +type podFix struct { + name, phase, namespace, node string + deletion time.Time + deletionGrace *int64 + terminationGrace *int64 + labels map[string]string +} + +type helmRec struct { + revision int + status string + chart string +} + +type secretRec struct { + data map[string]string + labels map[string]string +} + +func newUninstallCluster(t *testing.T, cs *uninstallCase) *uninstallCluster { + t.Helper() + c := &uninstallCluster{Fake: fakeProbe()} + c.Nod = []cluster.Node{ + {Name: "gpu-1", GPUProduct: "NVIDIA-B300-SXM6-AC", GPUs: 8, Schedulable: true, Ready: true}, + } + if len(cs.Nodes) > 0 { + c.Nod = nil + for _, n := range cs.Nodes { + c.Nod = append(c.Nod, cluster.Node{Name: n.Name, Ready: n.Ready, Schedulable: true}) + } + } + c.cms = map[string]*cmRec{} + c.helm = map[string]*helmRec{} + c.secs = map[string]*secretRec{} + if cs.PodListError { + c.podErr = fmt.Errorf("pods list forbidden") + } + c.deleteCMFails = cs.DeleteConfigMapFails + + p := &plan.Plan{ + APIVersion: plan.APIVersion, + Release: plan.Release{Name: cs.Release, Namespace: cs.Namespace}, + Source: plan.SourceRef{Model: "qwen3.6-35b-a3b", Variant: "sglang-tp2-h100"}, + Chart: plan.ChartRef{Name: cs.Chart, Version: "0.7.1"}, + } + if err := p.ComputeHash(); err != nil { + t.Fatal(err) + } + files, err := p.Files("") + if err != nil { + t.Fatal(err) + } + raw, err := yaml.Marshal(planStatus{ + Phase: phaseApplied, Action: "install", Revision: 4, StartedAt: stamp(metav1.Now().Time), + }) + if err != nil { + t.Fatal(err) + } + files[statusFile] = string(raw) + ctx := context.Background() + if err := c.PutConfigMap(ctx, planRef(cs.Namespace, cs.Release), files, nil); err != nil { + t.Fatal(err) + } + c.helm[cs.Namespace+"/"+cs.Release] = &helmRec{revision: 4, status: "deployed", chart: cs.Chart + "-0.7.1"} + c.setPods(cs.Namespace, cs.Pods) + if cs.SeedArchive { + if err := c.seedArchive(cs.Namespace, cs.Release); err != nil { + t.Fatal(err) + } + } + if cs.SeedAutoscale { + ref := autoscaleRef(cs.Namespace, cs.Release) + data := map[string]string{autoscaleConfigKey: "enabled: true\nminSlices: 1\nmaxSlices: 4\n"} + if err := c.PutLabeledConfigMap(ctx, ref, data, autoscaleLabels(cs.Release)); err != nil { + t.Fatal(err) + } + } + return c +} + +// stubHelmGated is stubHelm whose uninstall waits until gate exists. +func stubHelmGated(t *testing.T, exitCode int, gate string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "helm") + script := "#!/bin/sh\n" + + "if [ \"$1\" = uninstall ]; then\n" + + " while [ ! -f " + gate + " ]; do sleep 0.02; done\n" + + "fi\n" + + "echo \"helm $*\"\n" + + "exit " + strconv.Itoa(exitCode) + "\n" + if err := os.WriteFile(path, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + return path +} + +// delNoFatal is del for a goroutine, where t.Fatal is not allowed. +func delNoFatal(srv *httptest.Server, path string) (int, map[string]any) { + req, err := http.NewRequest(http.MethodDelete, srv.URL+path, nil) + if err != nil { + return 0, map[string]any{"error": err.Error()} + } + resp, err := http.DefaultClient.Do(req) + if err != nil { + return 0, map[string]any{"error": err.Error()} + } + defer resp.Body.Close() + var out map[string]any + _ = json.NewDecoder(resp.Body).Decode(&out) + return resp.StatusCode, out +} + +func (c *uninstallCluster) seedArchive(ns, release string) error { + ctx := context.Background() + data, err := c.ConfigMap(ctx, planRef(ns, release)) + if err != nil { + return err + } + return c.PutSecret(ctx, archiveRef(ns, release, 3), data, archiveLabels(release, 3)) +} + +func (c *uninstallCluster) setPods(ns string, pods []podSpec) { + c.mu.Lock() + defer c.mu.Unlock() + c.pods = nil + for _, p := range pods { + n := p.Namespace + if n == "" { + n = ns + } + phase := p.Phase + if phase == "" { + phase = "Running" + } + c.pods = append(c.pods, podFix{ + name: p.Name, phase: phase, namespace: n, node: p.Node, + deletion: p.DeletionTimestamp, deletionGrace: cloneInt64(p.DeletionGracePeriodSeconds), + terminationGrace: cloneInt64(p.TerminationGracePeriodSeconds), labels: maps.Clone(p.Labels), + }) + } +} + +func cloneInt64(p *int64) *int64 { + if p == nil { + return nil + } + v := *p + return &v +} + +func (c *uninstallCluster) dropHelm(ns, release string) { + c.mu.Lock() + defer c.mu.Unlock() + delete(c.helm, ns+"/"+release) +} + +func (c *uninstallCluster) hasConfigMap(ref string) bool { + c.mu.Lock() + defer c.mu.Unlock() + _, ok := c.cms[ref] + return ok +} + +func (c *uninstallCluster) ConfigMap(_ context.Context, ref string) (map[string]string, error) { + c.mu.Lock() + defer c.mu.Unlock() + if cm, ok := c.cms[ref]; ok { + return maps.Clone(cm.data), nil + } + if data, ok := c.Maps[ref]; ok && len(data) > 0 { + return maps.Clone(data), nil + } + _, name, err := cluster.SplitRef(ref) + if err != nil { + name = ref + } + return nil, apierrors.NewNotFound(schema.GroupResource{Resource: "configmaps"}, name) +} + +func (c *uninstallCluster) PutConfigMap(_ context.Context, ref string, data, labels map[string]string) error { + c.mu.Lock() + defer c.mu.Unlock() + labs := map[string]string{"app.kubernetes.io/managed-by": "swiss"} + for k, v := range labels { + if v != "" { + labs[k] = v + } + } + if c.cms == nil { + c.cms = map[string]*cmRec{} + } + c.cms[ref] = &cmRec{data: maps.Clone(data), labels: labs} + return nil +} + +func (c *uninstallCluster) PutLabeledConfigMap(ctx context.Context, ref string, data, labels map[string]string) error { + return c.PutConfigMap(ctx, ref, data, labels) +} + +func (c *uninstallCluster) UpdateConfigMapKey(_ context.Context, ref, key, value, _ string) error { + c.mu.Lock() + defer c.mu.Unlock() + cm, ok := c.cms[ref] + if !ok { + return apierrors.NewNotFound(schema.GroupResource{Resource: "configmaps"}, ref) + } + if cm.data == nil { + cm.data = map[string]string{} + } + cm.data[key] = value + return nil +} + +func (c *uninstallCluster) DeleteConfigMap(_ context.Context, ref string) error { + c.mu.Lock() + defer c.mu.Unlock() + if c.deleteCMFails > 0 { + c.deleteCMFails-- + return fmt.Errorf("configmap delete forbidden") + } + delete(c.cms, ref) + return nil +} + +func (c *uninstallCluster) ManagedRefs(context.Context) ([]cluster.ManagedRef, error) { + c.mu.Lock() + defer c.mu.Unlock() + var out []cluster.ManagedRef + for ref, cm := range c.cms { + ns, name, err := cluster.SplitRef(ref) + if err != nil { + continue + } + rel, ok := strings.CutPrefix(name, cluster.PlanConfigMapPrefix) + if !ok || rel == "" { + continue + } + out = append(out, cluster.ManagedRef{ + Namespace: ns, Name: rel, Labels: maps.Clone(cm.labels), + }) + } + sort.Slice(out, func(i, j int) bool { + if out[i].Namespace != out[j].Namespace { + return out[i].Namespace < out[j].Namespace + } + return out[i].Name < out[j].Name + }) + return out, nil +} + +func (c *uninstallCluster) Release(_ context.Context, namespace, name string) (*cluster.Release, error) { + c.mu.Lock() + defer c.mu.Unlock() + var files map[string]string + var status []byte + if cm, ok := c.cms[namespace+"/"+cluster.PlanConfigMapPrefix+name]; ok { + files = maps.Clone(cm.data) + status = []byte(cm.data[statusFile]) + } + h, ok := c.helm[namespace+"/"+name] + if !ok { + if files == nil { + return nil, nil + } + return &cluster.Release{Namespace: namespace, Name: name, SwissFiles: files, SwissStatus: status}, nil + } + return &cluster.Release{ + Name: name, Namespace: namespace, + Chart: h.chart, Status: h.status, Revision: h.revision, + SwissFiles: files, SwissStatus: status, + }, nil +} + +func (c *uninstallCluster) Pods(_ context.Context, namespace, selector string) ([]cluster.Pod, error) { + c.mu.Lock() + defer c.mu.Unlock() + if c.podErr != nil { + return nil, c.podErr + } + var out []cluster.Pod + for _, p := range c.pods { + if p.namespace != namespace || !selectorMatches(p.labels, selector) { + continue + } + pod := cluster.Pod{Name: p.name, Phase: p.phase, Node: p.node} + if !p.deletion.IsZero() { + ts := p.deletion + pod.DeletionTimestamp = &ts + } + pod.DeletionGracePeriodSeconds = p.deletionGrace + pod.TerminationGracePeriodSeconds = p.terminationGrace + out = append(out, pod) + } + return out, nil +} + +func (c *uninstallCluster) PutSecret(_ context.Context, ref string, data, labels map[string]string) error { + c.mu.Lock() + defer c.mu.Unlock() + if c.secs == nil { + c.secs = map[string]*secretRec{} + } + c.secs[ref] = &secretRec{data: maps.Clone(data), labels: maps.Clone(labels)} + return nil +} + +func (c *uninstallCluster) DeleteSecret(_ context.Context, ref string) error { + c.mu.Lock() + defer c.mu.Unlock() + delete(c.secs, ref) + return nil +} + +func (c *uninstallCluster) Secret(_ context.Context, ref string) (map[string]string, error) { + c.mu.Lock() + defer c.mu.Unlock() + s, ok := c.secs[ref] + if !ok { + _, name, err := cluster.SplitRef(ref) + if err != nil { + name = ref + } + return nil, apierrors.NewNotFound(schema.GroupResource{Resource: "secrets"}, name) + } + return maps.Clone(s.data), nil +} + +func (c *uninstallCluster) SecretNames(_ context.Context, namespace, selector string) ([]string, error) { + c.mu.Lock() + defer c.mu.Unlock() + var out []string + for ref, s := range c.secs { + ns, name, err := cluster.SplitRef(ref) + if err != nil || ns != namespace || !selectorMatches(s.labels, selector) { + continue + } + out = append(out, name) + } + sort.Strings(out) + return out, nil +} + +func (c *uninstallCluster) EnsureNamespace(context.Context, string) error { return nil } + +func selectorMatches(labels map[string]string, selector string) bool { + if selector == "" { + return true + } + for _, term := range strings.Split(selector, ",") { + k, v, ok := strings.Cut(strings.TrimSpace(term), "=") + if !ok || labels[k] != v { + return false + } + } + return true +} + +var ( + _ cluster.Probe = (*uninstallCluster)(nil) + _ cluster.Writer = (*uninstallCluster)(nil) +) + +// llmPlanProbe is the fake plus plan ConfigMaps labeled uninstalling. Fake's +// managed set comes from helm releases, which an llmsvc test does not have. +type llmPlanProbe struct { + *cluster.Fake +} + +func (p llmPlanProbe) ManagedRefs(ctx context.Context) ([]cluster.ManagedRef, error) { + out, err := p.Fake.ManagedRefs(ctx) + if err != nil { + return nil, err + } + seen := map[string]bool{} + for _, ref := range out { + seen[ref.Namespace+"/"+ref.Name] = true + } + for ref, labels := range p.MapLabels { + if labels[cluster.LabelPhase] != phaseUninstalling { + continue + } + ns, name, err := cluster.SplitRef(ref) + if err != nil { + continue + } + rel, ok := strings.CutPrefix(name, cluster.PlanConfigMapPrefix) + if !ok || rel == "" { + continue + } + key := ns + "/" + rel + if seen[key] { + continue + } + out = append(out, cluster.ManagedRef{Namespace: ns, Name: rel, Labels: maps.Clone(labels)}) + seen[key] = true + } + sort.Slice(out, func(i, j int) bool { + if out[i].Namespace != out[j].Namespace { + return out[i].Namespace < out[j].Namespace + } + return out[i].Name < out[j].Name + }) + return out, nil +} + +// llmMirrorWriter copies writes onto the probe the server reads. The stock +// fake writer only remembers them for the test. +type llmMirrorWriter struct { + *fakeWriter + probe *cluster.Fake +} + +func (m *llmMirrorWriter) PutConfigMap(ctx context.Context, ref string, data, labels map[string]string) error { + if err := m.fakeWriter.PutConfigMap(ctx, ref, data, labels); err != nil { + return err + } + if m.probe.Maps == nil { + m.probe.Maps = map[string]map[string]string{} + } + m.probe.Maps[ref] = cloneData(data) + if m.probe.MapLabels == nil { + m.probe.MapLabels = map[string]map[string]string{} + } + labs := map[string]string{"app.kubernetes.io/managed-by": "swiss"} + for k, v := range labels { + if v != "" { + labs[k] = v + } + } + m.probe.MapLabels[ref] = labs + return nil +} + +func (m *llmMirrorWriter) DeleteConfigMap(ctx context.Context, ref string) error { + if err := m.fakeWriter.DeleteConfigMap(ctx, ref); err != nil { + return err + } + delete(m.probe.Maps, ref) + delete(m.probe.MapLabels, ref) + return nil +} + +func (m *llmMirrorWriter) PutSecret(ctx context.Context, ref string, data, labels map[string]string) error { + if err := m.fakeWriter.PutSecret(ctx, ref, data, labels); err != nil { + return err + } + if m.probe.Secrets == nil { + m.probe.Secrets = map[string]map[string]string{} + } + if m.probe.SecretLabels == nil { + m.probe.SecretLabels = map[string]map[string]string{} + } + m.probe.Secrets[ref] = cloneData(data) + m.probe.SecretLabels[ref] = cloneData(labels) + return nil +} + +func (m *llmMirrorWriter) DeleteSecret(ctx context.Context, ref string) error { + if err := m.fakeWriter.DeleteSecret(ctx, ref); err != nil { + return err + } + delete(m.probe.Secrets, ref) + delete(m.probe.SecretLabels, ref) + return nil +} + +type llmUninstallHarness struct { + cs *uninstallCase + rig *llmRig + base *cluster.Fake + srv *httptest.Server +} + +func (h *llmUninstallHarness) arm(t *testing.T) { + t.Helper() + if h.cs.Finalizer { + h.rig.dyn.PrependReactor("delete", "llmservices", func(action k8stesting.Action) (bool, runtime.Object, error) { + del := action.(k8stesting.DeleteAction) + obj, err := h.rig.dyn.Tracker().Get(llmsvc.GVR, del.GetNamespace(), del.GetName()) + if err != nil { + return true, nil, err + } + u := obj.(*unstructured.Unstructured).DeepCopy() + if !u.GetDeletionTimestamp().IsZero() { + return true, nil, nil + } + ts := metav1.Now() + u.SetDeletionTimestamp(&ts) + if len(u.GetFinalizers()) == 0 { + u.SetFinalizers([]string{llmsvc.Group + "/cleanup"}) + } + if err := h.rig.dyn.Tracker().Update(llmsvc.GVR, u, del.GetNamespace()); err != nil { + return true, nil, err + } + return true, nil, nil + }) + } + // Prepended last so it runs first. A NotFound delete still removes the + // object, which is what makes the plan-restore path observable. + if h.cs.DeleteNotFound { + h.rig.dyn.PrependReactor("delete", "llmservices", func(action k8stesting.Action) (bool, runtime.Object, error) { + del := action.(k8stesting.DeleteAction) + _ = h.rig.dyn.Tracker().Delete(llmsvc.GVR, del.GetNamespace(), del.GetName()) + gr := schema.GroupResource{Group: llmsvc.Group, Resource: llmsvc.Resource} + return true, nil, apierrors.NewNotFound(gr, del.GetName()) + }) + } +} + +func (h *llmUninstallHarness) create(t *testing.T) { + t.Helper() + u := llmObject(h.cs.Namespace, h.cs.Release, map[string]string{llmsvc.AnnProfile: "prod"}) + st := &llmsvc.Status{ + Phase: h.cs.OperatorPhase, + Helm: &llmsvc.HelmStatus{Revision: 4, Status: "deployed"}, + } + if len(h.cs.Deletion) > 0 { + raw, err := json.Marshal(h.cs.Deletion) + if err != nil { + t.Fatal(err) + } + var del llmsvc.DeletionStatus + if err := json.Unmarshal(raw, &del); err != nil { + t.Fatal(err) + } + st.Deletion = &del + } + m, err := runtime.DefaultUnstructuredConverter.ToUnstructured(st) + if err != nil { + t.Fatal(err) + } + u.Object["status"] = m + if h.cs.Deleting == nil || *h.cs.Deleting { + ts := metav1.Now() + u.SetDeletionTimestamp(&ts) + } + if _, err := h.rig.c.Create(t.Context(), u); err != nil { + t.Fatal(err) + } +} + +func (h *llmUninstallHarness) clearDeletion(t *testing.T) { + t.Helper() + obj, err := h.rig.dyn.Tracker().Get(llmsvc.GVR, h.cs.Namespace, h.cs.Release) + if err != nil { + t.Fatal(err) + } + u := obj.(*unstructured.Unstructured).DeepCopy() + u.SetDeletionTimestamp(nil) + u.SetFinalizers(nil) + if err := h.rig.dyn.Tracker().Update(llmsvc.GVR, u, h.cs.Namespace); err != nil { + t.Fatal(err) + } +} + +func (h *llmUninstallHarness) restart(t *testing.T) { + t.Helper() + s := New(h.rig.s.cfg, h.rig.s.probe, discardLogger(), "test") + s.SetStore(h.rig.s.store) + s.SetWriter(h.rig.s.writer) + s.SetLLMServices(h.rig.s.llms) + h.rig.s = s + if h.srv != nil { + h.srv.Close() + } + h.srv = httptest.NewServer(s.Handler()) + h.rig.srv = h.srv + t.Cleanup(h.srv.Close) +} + +func seedLLMPlan(t *testing.T, probe *cluster.Fake, cs *uninstallCase) { + t.Helper() + phase := cs.PlanPhase + if phase == "" { + phase = phaseApplied + } + raw, err := yaml.Marshal(planStatus{Phase: phase, Action: "install", Revision: 4, StartedAt: stamp(time.Now())}) + if err != nil { + t.Fatal(err) + } + ref := planRef(cs.Namespace, cs.Release) + if probe.Maps == nil { + probe.Maps = map[string]map[string]string{} + } + probe.Maps[ref] = map[string]string{ + "helmfile.yaml": "releases: []\n", + statusFile: string(raw), + } + if phase == phaseUninstalling { + if probe.MapLabels == nil { + probe.MapLabels = map[string]map[string]string{} + } + probe.MapLabels[ref] = map[string]string{ + "app.kubernetes.io/managed-by": "swiss", + cluster.LabelPhase: phaseUninstalling, + } + } +} + +func seedLLMArchive(probe *cluster.Fake, cs *uninstallCase) { + ref := archiveRef(cs.Namespace, cs.Release, 1) + if probe.Secrets == nil { + probe.Secrets = map[string]map[string]string{} + } + if probe.SecretLabels == nil { + probe.SecretLabels = map[string]map[string]string{} + } + probe.Secrets[ref] = map[string]string{"helmfile.yaml": "releases: []\n"} + probe.SecretLabels[ref] = archiveLabels(cs.Release, 1) +} + +func seedLLMAutoscale(probe *cluster.Fake, cs *uninstallCase) { + ref := autoscaleRef(cs.Namespace, cs.Release) + if probe.Maps == nil { + probe.Maps = map[string]map[string]string{} + } + probe.Maps[ref] = map[string]string{autoscaleConfigKey: "enabled: true\nminSlices: 1\nmaxSlices: 4\n"} +} diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 0da68a5..316ce9d 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -35,6 +35,7 @@ const ( apiTimeout = 60 * time.Second installTimeout = 16 * time.Minute goneTimeout = 180 * time.Second + llmGoneTimeout = 6 * time.Minute readyPoll = 5 * time.Second gonePoll = 3 * time.Second ) @@ -177,6 +178,59 @@ func deployCycle(backend string) { _, _ = fmt.Fprintf(GinkgoWriter, "uninstall release=%s planError=%s\n", removed.Release, removed.PlanError) Expect(removed.Release).To(Equal(rel)) + // Helm returns before pods finish terminating. If they are already gone on + // the first poll, there is nothing left to list — that is success, not a miss. + if backend == "helm" { + By("[" + backend + "] staying listed while pods terminate") + Eventually(func(g Gomega) { + pods, err := vllmPods(rel) + g.Expect(err).NotTo(HaveOccurred()) + if len(pods) == 0 { + return + } + var listed deploymentList + g.Expect(api.call(http.MethodGet, "/api/deployments?perPage=100", nil, &listed, apiTimeout)).To(Succeed()) + phase, found := deploymentPhase(listed, rel) + if !found { + pods, err = vllmPods(rel) + g.Expect(err).NotTo(HaveOccurred()) + if len(pods) == 0 { + return + } + g.Expect(found).To(BeTrue(), "pods still exist but %s is not listed", rel) + } + g.Expect(phase).To(Equal("uninstalling")) + var during releaseStatusBody + g.Expect(api.call(http.MethodGet, releasePath(rel)+"/status", nil, &during, apiTimeout)).To(Succeed()) + g.Expect(during.Plan).NotTo(BeNil()) + g.Expect(during.Plan.Phase).To(Equal("uninstalling")) + }).WithTimeout(time.Minute).WithPolling(gonePoll).Should(Succeed()) + } + + // An llmsvc delete returns as soon as the API accepts it. An old operator + // drops the object before the first poll; a staged one keeps it, phase + // Deleting, through route settle and pod termination. + if backend == "llmsvc" { + By("[" + backend + "] staying listed while the operator deletes") + Eventually(func(g Gomega) { + exists, err := llmServiceExists(rel) + g.Expect(err).NotTo(HaveOccurred()) + if !exists { + return + } + var listed deploymentList + g.Expect(api.call(http.MethodGet, "/api/deployments?perPage=100", nil, &listed, apiTimeout)).To(Succeed()) + phase, found := deploymentPhase(listed, rel) + g.Expect(found).To(BeTrue(), "%s still exists but is not listed", rel) + g.Expect(phase).To(Equal("uninstalling")) + g.Expect(api.call(http.MethodDelete, releasePath(rel), nil, &uninstallResult{}, apiTimeout)).To(Succeed()) + }).WithTimeout(time.Minute).WithPolling(gonePoll).Should(Succeed()) + } + + timeout := goneTimeout + if backend == "llmsvc" { + timeout = llmGoneTimeout + } By(fmt.Sprintf("[%s] waiting for %s to be gone", backend, rel)) Eventually(func(g Gomega) { pods, err := vllmPods(rel) @@ -186,7 +240,20 @@ func deployCycle(backend string) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(exists).To(BeFalse()) g.Expect(helmReleasePresent(cfg.modelNS, rel)).To(BeFalse()) - }).WithTimeout(goneTimeout).WithPolling(gonePoll).Should(Succeed()) + var listed deploymentList + g.Expect(api.call(http.MethodGet, "/api/deployments?perPage=100", nil, &listed, apiTimeout)).To(Succeed()) + _, found := deploymentPhase(listed, rel) + g.Expect(found).To(BeFalse(), "%s is still listed after its pods are gone", rel) + }).WithTimeout(timeout).WithPolling(gonePoll).Should(Succeed()) +} + +func deploymentPhase(list deploymentList, rel string) (string, bool) { + for _, d := range list.Deployments { + if d.Release == rel && d.Namespace == cfg.modelNS { + return d.Phase, true + } + } + return "", false } func engineAnswers(rel string) { @@ -428,6 +495,9 @@ type releaseStatusBody struct { Revision int `json:"revision"` HelmStatus string `json:"helmStatus"` Model string `json:"model"` + Plan *struct { + Phase string `json:"phase"` + } `json:"planStatus"` } type objectBody struct { @@ -441,6 +511,7 @@ type deploymentList struct { Release string `json:"release"` Namespace string `json:"namespace"` Backend string `json:"backend"` + Phase string `json:"phase"` } `json:"deployments"` }