Skip to content

Conversation

@amdove
Copy link

@amdove amdove commented Jan 29, 2026

Adds toleration for prepull daemonset to run on tainted nodes. This is needed specifically to make sure it's running when Karpenter nodes are configured with session taints. Omitting this isn't breaking functionality completely, but is preventing prepull from doing what it should be to pre-pull images to nodes that will run sessions.

@claude
Copy link

claude bot commented Jan 29, 2026

Claude finished @amdove's task —— View job


Review Complete

  • Read review guidelines
  • Analyze code changes
  • Create pending review
  • Submit review with inline comments

Reviewed the toleration change. The universal toleration approach is correct for a prepull daemonset. Left one suggestion about removing the now-redundant workbench tolerations block.

Copy link

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

The change correctly adds a universal toleration (Operator: TolerationOpExists) to ensure the prepull daemonset runs on all nodes regardless of taints. This is appropriate for a prepull daemonset since images should be cached on every node.

One consideration:

The universal toleration makes the workbench-specific tolerations block (lines 114-121) redundant since TolerationOpExists already tolerates everything. You could:

  1. Remove the workbench tolerations block entirely, or
  2. Keep it with a comment explaining why (e.g., for documentation/visibility)

Otherwise, the implementation follows existing patterns and the reconciliation remains idempotent.

@amdove amdove marked this pull request as ready for review January 29, 2026 19:54
@amdove amdove marked this pull request as draft January 29, 2026 19:59
@amdove amdove closed this Jan 30, 2026
@amdove amdove deleted the fix-tolerations branch January 30, 2026 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants