Skip to content

refactor(operator): drop CheckpointJob naming from the snapshot protocol - #134

Open
oleg-kushniriov wants to merge 1 commit into
mainfrom
RUN-39806-protocol-cleanup
Open

refactor(operator): drop CheckpointJob naming from the snapshot protocol#134
oleg-kushniriov wants to merge 1 commit into
mainfrom
RUN-39806-protocol-cleanup

Conversation

@oleg-kushniriov

@oleg-kushniriov oleg-kushniriov commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Jira: RUN-39806

What

Final cleanup PR of the SnapshotJob series. SnapshotJob creates a plain batch/v1 Job whose pod is the capture source — the CheckpointJob identifiers in the snapshot protocol are Dynamo-era naming. This renames them to the source-Job vocabulary:

  • NewCheckpointJobNewSourceJob
  • CheckpointJobOptionsSourceJobOptions
  • DisableCheckpointJobSidecarInjectionDisableSidecarInjection

protocol/checkpoint.go and its tests move to source_job*.go, and "checkpoint job" prose in errors and comments now says "source job" where it names the Job. The word "checkpoint" for the CRIU capture itself is unchanged.

What this PR does not do

  • The dead-code half of the originally planned cleanup (GetCheckpointJobName, DefaultCheckpointJobTTLSeconds, ApplyCheckpointStorageMetadata, DiscoverAndResolveStorage, PrepareRestorePodSpecForCheckpoint, CheckpointStatus*) was already removed by earlier PRs, so this is renames only.
  • No behavior change: the snapshotctl CLI surface (checkpoint subcommand, flags, checkpoint_job= output) is deliberately untouched — that is user-facing behavior, not internal naming.
  • No api/v1alpha1 surface change: the one touched file there is a single doc-comment line; no types, constants, labels, or CRD schema change, so nothing regenerates.

Verification

  • go build ./... and go test ./... pass in the operator and api modules; gofmt clean.
  • GOOS=linux go build + go vet pass as well (covers the linux-only snapshotctl/checkpoint_test.go).
  • Repo-wide grep confirms no CheckpointJob identifier remains.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Improvements

    • Clarified snapshot workflow terminology by consistently referring to source jobs.
    • Improved validation and error reporting for source-job creation, including target-container checks and restore configuration safeguards.
    • Preserved existing pod behavior while ensuring service-mesh sidecar injection can be disabled when needed.
    • Improved reliability when passing checkpoint job details to CUDA workloads, including clearer handling of missing job information.
  • Tests

    • Expanded coverage for source-job creation, validation, and CUDA job-file handling.

SnapshotJob creates a plain batch/v1 Job whose pod is the capture
source; the CheckpointJob identifiers are Dynamo-era naming left in the
snapshot protocol. Rename them to match the source-Job vocabulary:

- NewCheckpointJob -> NewSourceJob
- CheckpointJobOptions -> SourceJobOptions
- DisableCheckpointJobSidecarInjection -> DisableSidecarInjection

protocol/checkpoint.go and its tests move to source_job*.go, and the
"checkpoint job" prose in errors and comments now says "source job"
where it names the Job (the CRIU capture itself is still a checkpoint).

The dead-code half of the planned cleanup (GetCheckpointJobName,
DefaultCheckpointJobTTLSeconds, ApplyCheckpointStorageMetadata,
DiscoverAndResolveStorage, PrepareRestorePodSpecForCheckpoint,
CheckpointStatus*) was already removed by earlier PRs, so this is
renames only. No behavior change: the snapshotctl CLI surface
(checkpoint subcommand, flags, output keys) is untouched, and no
api/v1alpha1 type, constant, or CRD schema changes.

Signed-off-by: Oleg Kushniriov <okushniriov@nvidia.com>
@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: fca3c933-4365-4410-a776-1a3bb2193282

📥 Commits

Reviewing files that changed from the base of the PR and between c1206a4 and 4c7f217.

📒 Files selected for processing (8)
  • api/v1alpha1/constants.go
  • operator/cmd/snapshotctl/checkpoint.go
  • operator/internal/controller/snapshotjob_job.go
  • operator/internal/controller/snapshotjob_job_test.go
  • operator/internal/protocol/control_volume.go
  • operator/internal/protocol/source_job.go
  • operator/internal/protocol/source_job_identity_test.go
  • operator/internal/protocol/source_job_test.go

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Walkthrough

The protocol renames checkpoint-job APIs to source-job APIs. The command and controller adopt the new constructors. Documentation and tests update the terminology and validate source-job construction, wrapping, validation, and CUDA wrapper file handling.

Changes

Source Job API Rename

Layer / File(s) Summary
Source job protocol API
operator/internal/protocol/source_job.go
Renames CheckpointJobOptions, NewCheckpointJob, and DisableCheckpointJobSidecarInjection. Validation errors and documentation now use source-job terminology.
Source job callers and contracts
operator/cmd/snapshotctl/checkpoint.go, operator/internal/controller/snapshotjob_job.go, api/v1alpha1/constants.go, operator/internal/protocol/control_volume.go
The command and controller construct source jobs. Related comments identify source-job behavior and contracts.
Source job validation tests
operator/internal/protocol/source_job_test.go, operator/internal/protocol/source_job_identity_test.go, operator/internal/controller/snapshotjob_job_test.go
Tests use the renamed APIs and cover construction, target wrapping, sidecar handling, validation, no-wrap behavior, and CUDA checkpoint job-file handling.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 4c7f2

This PR only updates internal snapshot source-job naming while preserving the CLI and API behavior; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 6 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 8 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title uses the valid conventional-commit prefix refactor, clearly describes the source-job naming cleanup, and is exactly 72 characters.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Breaking Api Changes ✅ Passed PASS: The PR changes only one line in api/v1alpha1/constants.go, and the change is a comment from “checkpoint job” to “source job.” No API fields, Go types, JSON tags, optional markers, defaults, or X…
Rbac Least Privilege ✅ Passed No RBAC wildcard grant was introduced. The PR diff changes no kubebuilder RBAC marker or Helm RBAC manifest, and the added-line scan found no wildcard in RBAC syntax. Current markers and Helm RBAC tem…
Full details: Breaking Api Changes

Explanation

PASS: The PR changes only one line in api/v1alpha1/constants.go, and the change is a comment from “checkpoint job” to “source job.” No API fields, Go types, JSON tags, optional markers, defaults, or XValidation immutability markers changed. PodSnapshotSpec and PodSnapshotContentSpec are identical to the parent revision.

Full details: Rbac Least Privilege

Explanation

No RBAC wildcard grant was introduced. The PR diff changes no kubebuilder RBAC marker or Helm RBAC manifest, and the added-line scan found no wildcard in RBAC syntax. Current markers and Helm RBAC templates use explicit verbs and resources; repository-wide scans found no wildcard verbs or resources entries.

  • Fix all pre-merge checks with AI

Warning

Some tools did not complete. Review the errors below.

🔧 golangci-lint (2.12.2)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions


Comment @coderabbitai help to get the list of available commands.

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.

1 participant