Skip to content

[Bug]: slinky-drainer caches every Pod in the cluster instead of only the Slinky namespace #1822

Description

@XRFXLP

Prerequisites

  • I searched existing issues
  • I can reproduce this issue

Code of Conduct

  • I agree to follow NVSentinel's Code of Conduct

Bug Description

slinky-drainer builds its controller-runtime manager with no cache configuration, so the Pod informer is cluster-wide. The controller only ever needs Pods in the namespace given by --slinky-namespace (default slinky), but it lists and watches every Pod in every namespace and holds them all in memory.

getSlinkyPods looks correctly scoped, but client.InNamespace and client.MatchingFields filter the cache — they do not narrow what the cache subscribes to. The spec.nodeName field index registered in SetupWithManager is built over all Pods cluster-wide for the same reason. ClusterRole slinky-drainer-role grants pods: get,list,watch cluster-wide, so nothing constrains it.

Node objects are also cached in full with no transform. The Node watch itself is needed for annotation cleanup, but the retained status, image list and managed fields are not.

Expected: the Pod cache is restricted to the Slinky namespace, and cached Pod and Node objects are stripped to the fields the reconciler reads — the pattern already used by janitor, slurm-drain-monitor, nvcre-certification-monitor, labeler and preflight.

Impact. On a 10k-node fleet at ~30 pods per node this is roughly 300k cached Pod objects against a shipped limit of memory: 256Mi in plugins/slinky-drainer/config/manager/deployment.yaml, versus roughly one relevant Pod per node. Two consequences:

  1. A full Pod LIST against the API server at informer start, repeated on resync — avoidable load on large clusters.
  2. The Pod informer starts lazily, on the first Pod List, which is the first DrainRequest reconcile. So the memory growth does not appear at startup during validation; it appears the first time a node actually needs draining. If the container is OOMKilled there, drains stall exactly when remediation is required.

Component

Plugins

Steps to Reproduce

  1. Deploy slinky-drainer with kubectl apply -k plugins/slinky-drainer/config/default (defaults: --slinky-namespace=slinky, memory: 256Mi).
  2. Note steady-state memory with kubectl top pod -n nvsentinel -l app=slinky-drainer — no Pod informer exists yet.
  3. Create several thousand Pods in a namespace unrelated to slinky.
  4. Create a DrainRequest for any node, which triggers the first Pod List and starts the informer.
  5. Re-check kubectl top pod. Resident memory tracks total cluster Pod count, not Slinky Pod count; with enough unrelated Pods the container is OOMKilled and the drain does not progress.

Environment

  • NVSentinel version: main; chart distros/kubernetes/nvsentinel version 0.1.0, appVersion 1.0.0
  • Kubernetes version: any (not version-dependent)
  • Deployment method: plugins/slinky-drainer/config/default kustomize overlay (the plugin is not part of the Helm chart)

Logs/Output

The relevant code:

// plugins/slinky-drainer/main.go:67 — no Cache options, so the cache is cluster-wide per kind
mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
	Scheme:                 scheme,
	Metrics:                metricsserver.Options{BindAddress: metricsAddr},
	HealthProbeBindAddress: probeAddr,
})

// plugins/slinky-drainer/pkg/controller/drainrequest_controller.go:285 — filters the cache, does not scope it
opts := []client.ListOption{
	client.InNamespace(r.SlinkyNamespace),
	client.MatchingFields{"spec.nodeName": nodeName},
}

Suggested fix, mirroring janitor/pkg/cacheconfig/cache.go. slinkyNamespace is already parsed before NewManager, so it can be passed straight in:

Cache: cache.Options{
	ByObject: map[client.Object]cache.ByObject{
		&corev1.Pod{}: {
			Namespaces: map[string]cache.Config{slinkyNamespace: {}},
			Transform:  transformPodForCache,  // name, spec.nodeName, status.conditions
		},
		&corev1.Node{}: {Transform: transformNodeForCache}, // labels + cordon annotation
	},
},

The Pod rule in ClusterRole slinky-drainer-role could then narrow to a namespaced Role, as a separate follow-up.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions