Skip to content

Adding helm charts for the snapshot-agent - #19

Merged
Edwinhr716 merged 2 commits into
llm-d-incubation:mainfrom
Edwinhr716:helm-charts-daemonset
Jun 8, 2026
Merged

Adding helm charts for the snapshot-agent#19
Edwinhr716 merged 2 commits into
llm-d-incubation:mainfrom
Edwinhr716:helm-charts-daemonset

Conversation

@Edwinhr716

Copy link
Copy Markdown
Collaborator

What does this PR do?

Why is this change needed?

Adds the helm charts needed to deploy the snapshot daemon.

Sample command:

helm upgrade --install snapshot-agent ./deploy/snapshot-agent   --set image.repository=<image>   --set image.tag=<tag>   --set "tolerations[0].key=nvidia.com/gpu"   --set "tolerations[0].operator=Equal"   --set "tolerations[0].value=present"   --set "tolerations[0].effect=NoSchedule"   --set serviceAccount.create=true   --set serviceAccount.name=snapshot-agent-new-sa

How was this tested?

  • Unit tests added/updated
  • Integration/e2e tests added/updated
  • Manual testing performed

Checklist

  • Commits are signed off (git commit -s) per DCO
  • Code follows project contributing guidelines
  • Tests pass locally (make test)
  • Linters pass (make lint)
  • Documentation updated (if applicable)

Related Issues

@Edwinhr716
Edwinhr716 requested a review from aishukamal as a code owner June 4, 2026 00:22

@aishukamal aishukamal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

/approve

initContainers:
- name: install-cuda-checkpoint
image: alpine:latest
command: ["sh", "-c", "apk add --no-cache wget && wget -qO /opt/bin/cuda-checkpoint https://raw.githubusercontent.com/NVIDIA/cuda-checkpoint/main/bin/x86_64_Linux/cuda-checkpoint && chmod +x /opt/bin/cuda-checkpoint && echo 'cuda-checkpoint installed'"]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not a blocker for this PR. I wonder if it would be better to fork cuda-checkpoint into the time-slicing repo and build it into the snapshot-agent image at build time rather than pulling the main branch at run time? It would help avoid instabilities/incompatibilities caused by upstream changes to cuda-checkpoint.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not sure what the usual pattern is, but don't think a fork into this repo is the best move. Can we pin it to a commit install that version instead?

port: 9001

securityContext:
privileged: true

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not a blocker for this PR. We should maybe file a follow-up issue in our backlog to scope the permissions down to exactly what we need rather than broad full host access - for e.g. SYS_PTRACE, SYS_ADMIN, CHECKPOINT_RESTORE capabilities might suffice. This requires some testing, so let's not block this PR on this.

Comment thread deploy/snapshot-agent/templates/daemonset.yaml
@aishukamal

Copy link
Copy Markdown
Collaborator

/approve

github-actions[bot]
github-actions Bot previously approved these changes Jun 5, 2026
@aishukamal

Copy link
Copy Markdown
Collaborator

/approve

@Edwinhr716
Edwinhr716 merged commit 6d2bbff into llm-d-incubation:main Jun 8, 2026
7 checks passed
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