Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions src/clis/nvcf-cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1756,14 +1756,19 @@ the desired state.

### How kill works

`kill-function` and `kill-all` delete the matching `ICMSRequest` CRs; the NVCA
reconciler detects the deletion and evicts the workloads. Deleting a CR only
accepts the deletion; the object stays `Terminating` behind its finalizer
until NVCA finishes evicting the workload and removes it. The command polls
for the CR to actually disappear before reporting success: a request removed
within `--timeout` (default 60s) is reported `deleted`, and one still present
when the timeout elapses is reported `terminating` instead, with a non-zero
exit code. `--force` additionally strips finalizers so a request stuck
`kill-function` and `kill-all` terminate the matching `ICMSRequest`'s
instances directly (deleting the Pod for a container function, or the
`MiniService` object for a Helm function), mark them terminated on the CR,
then delete the CR. Deleting the CR alone never evicts the workload: NVCA's
reconciler only clears the CR's finalizer once its own `status.instances`
shows every instance gone and reported terminated, and nothing else in NVCA
ever produces that for a CLI-initiated kill. Performing the eviction and
status update directly satisfies that precondition, so NVCA's own reconcile
clears the finalizer on its next pass. The command polls for the CR to
actually disappear before reporting success: a request removed within
`--timeout` (default 60s) is reported `deleted`, and one still present when
the timeout elapses is reported `terminating` instead, with a non-zero exit
code. `--force` additionally strips finalizers so a request stuck
`Terminating` is removed even when NVCA is not running to process its
finalizer.
Comment thread
rohithb-hub marked this conversation as resolved.

Expand All @@ -1786,8 +1791,10 @@ and `--json` for automation.

These commands need write access to the target cluster: list/update on the
`NVCFBackend` CR for drain (plus read access to the `agent-config` ConfigMap
and the `nvca` Deployment, to wait for the NVCA operator's rollout), and
list/delete (and update, with `--force`) on `ICMSRequest` CRs for kill.
and the `nvca` Deployment, to wait for the NVCA operator's rollout), and for
kill, list/delete (and update, with `--force`) on `ICMSRequest` CRs, update on
the `ICMSRequest` status subresource, delete on Pods in the requests
namespace, and delete on `MiniService` CRs (cluster-scoped).

### Examples

Expand Down
4 changes: 4 additions & 0 deletions src/clis/nvcf-cli/internal/clusteragent/k8s_inspector.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ var (
icmsRequestGVR = schema.GroupVersionResource{
Group: "nvca.nvcf.nvidia.io", Version: "v2beta1", Resource: "icmsrequests",
}
// miniServiceGVR is cluster-scoped, unlike NVCFBackend/ICMSRequest.
miniServiceGVR = schema.GroupVersionResource{
Group: "nvca.nvcf.nvidia.io", Version: "v1alpha1", Resource: "miniservices",
}
)

// k8sInspector reads NVCA state from a compute-plane cluster's Kubernetes API
Expand Down
117 changes: 117 additions & 0 deletions src/clis/nvcf-cli/internal/clusteragent/k8s_maintainer.go
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,20 @@ func (m *k8sMaintainer) killMatching(ctx context.Context, target *ClusterTarget,
FunctionVersionID: vid,
}
if !opts.DryRun {
// Deleting the ICMSRequest CR alone never evicts the workload
// (see evictInstances doc comment), so drive the real eviction
// ourselves first. A failure here must stop this item rather
// than fall through to delete: with --force in particular,
// proceeding would strip the finalizer and report success while
// the workload (pod or MiniService) may still be running.
if err := m.evictInstances(ctx, killed.Namespace, &items[i]); err != nil {
killed.Error = fmt.Sprintf("evicting instances: %v", err)
result.FailedCount++
failures = append(failures, fmt.Errorf("%s/%s (function=%s version=%s cluster=%s): evicting instances: %w",
killed.Namespace, killed.Name, killed.FunctionID, killed.FunctionVersionID, clusterLabel(target), err))
result.Affected = append(result.Affected, killed)
continue
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
terminating, err := m.deleteICMSRequest(ctx, killed.Namespace, killed.Name, opts.Force, timeout)
Comment thread
apartha-nv marked this conversation as resolved.
switch {
case err != nil:
Expand All @@ -534,6 +548,109 @@ func (m *k8sMaintainer) killMatching(ctx context.Context, target *ClusterTarget,
return result, failures, nil
}

// evictInstances deletes the Pod or MiniService backing each instance an
// ICMSRequest tracks in status.instances, and marks each one
// lastReportedStatus: "terminated" on that same CR.
//
// NVCA's reconciler only removes the CR's finalizer once
// AllInstancesTerminatedAndReported is true for that CR's own
// status.instances; nothing else in NVCA ever makes that true for a
// CLI-initiated kill, so deleting the CR alone leaves it (and the workload)
// stuck behind the finalizer forever. Doing both steps here satisfies that
// precondition, so NVCA's existing reconcile clears the finalizer itself.
//
// MiniService deletion needs no further cleanup step: its own controller
// (internal/miniservice/reconcile.go) tears down the chart on delete.
func (m *k8sMaintainer) evictInstances(ctx context.Context, namespace string, obj *unstructured.Unstructured) error {
instances, found, err := unstructured.NestedMap(obj.Object, "status", "instances")
if err != nil {
return fmt.Errorf("reading status.instances: %w", err)
}
if !found || len(instances) == 0 {
return nil
}

var errs []error
terminated := map[string]string{}
for id, raw := range instances {
inst, ok := raw.(map[string]interface{})
if !ok {
errs = append(errs, fmt.Errorf("instance %s: status.instances record is not an object", id))
continue
}
// instanceType is the current field; type is a legacy alias some
// older records still carry (see extractInstances in
// k8s_inspector.go, which reads both for the same reason).
instanceType, _, _ := unstructured.NestedString(inst, "instanceType")
if instanceType == "" {
instanceType, _, _ = unstructured.NestedString(inst, "type")
}
var delErr error
switch instanceType {
case "", "Pod":
delErr = m.cs.CoreV1().Pods(namespace).Delete(ctx, id, metav1.DeleteOptions{})
case "MiniService":
// Cluster-scoped: no .Namespace(...).
delErr = m.dc.Resource(miniServiceGVR).Delete(ctx, id, metav1.DeleteOptions{})
default:
// Unrecognized instance type: do not guess which resource kind
// to delete. Reported as a failure (rather than silently
// skipped) so the caller does not proceed to delete the CR
// while this instance's workload was never touched.
errs = append(errs, fmt.Errorf("instance %s: unsupported instance type %q", id, instanceType))
continue
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
if delErr != nil && !apierrors.IsNotFound(delErr) {
errs = append(errs, fmt.Errorf("deleting %s instance %s: %w", firstNonEmpty(instanceType, "Pod"), id, delErr))
continue
}
terminated[id] = "terminated"
}
if len(terminated) == 0 {
return errors.Join(errs...)
}

err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
latest, err := m.dc.Resource(icmsRequestGVR).Namespace(namespace).Get(ctx, obj.GetName(), metav1.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
return nil
}
return err
}
existing, _, _ := unstructured.NestedMap(latest.Object, "status", "instances")
if existing == nil {
existing = map[string]interface{}{}
}
// Merge lastReportedStatus into whatever is currently on the
// server, rather than overwriting the whole instance record with
// the pre-eviction snapshot: NVCA may have concurrently updated
// other instance fields (attributes, timestamps) since obj was read.
for id, status := range terminated {
cur, ok := existing[id].(map[string]interface{})
if !ok {
cur = map[string]interface{}{"id": id}
}
cur["lastReportedStatus"] = status
existing[id] = cur
}
if err := unstructured.SetNestedMap(latest.Object, existing, "status", "instances"); err != nil {
return err
}
latest.SetGroupVersionKind(schema.GroupVersionKind{
Group: icmsRequestGVR.Group,
Version: icmsRequestGVR.Version,
Kind: "ICMSRequest",
})
_, err = m.dc.Resource(icmsRequestGVR).Namespace(namespace).UpdateStatus(ctx, latest, metav1.UpdateOptions{})
return err
})
if err != nil {
errs = append(errs, fmt.Errorf("patching instance status: %w", err))
}
return errors.Join(errs...)
}

// deleteICMSRequest deletes one ICMSRequest and waits up to timeout for it to
// actually disappear. When force is set, it first strips finalizers so a CR
// stuck Terminating is removed even if NVCA is not running.
Expand Down
Loading
Loading