feat!: Add instance branching + checkpointing by jedevc · Pull Request #403 · unikraft-cloud/cli

jedevc · 2026-07-07T09:26:25Z

TODO:

Quick design once-over

Signed-off-by: Justin Chadwell <justin@unikraft.com>

Signed-off-by: ayn <ayn.khatib@gmail.com>

Signed-off-by: ayn <ayn.khatib@gmail.com> Signed-off-by: ayn <ayn.khatib@gmail.com> Signed-off-by: ayn <ayn.khatib@gmail.com>

Signed-off-by: ayn <ayn.khatib@gmail.com>

--branch and --checkpoint parsed a metro/name ref but silently dropped the metro part before sending the request to a single metro client, unlike the sibling template/volumes/service handling in the same function. A cross-metro ref could silently target a same-named but wrong resource, or fail with a misleading error. Signed-off-by: ayn <ayn.khatib@gmail.com>

InstanceCheckpoint.Get() matched API response items to requested refs by array position, unlike every sibling Get() which uses matchRef. If the platform reorders or omits entries, a checkpoint's tracked identity (Key()) could get swapped with another's. Signed-off-by: ayn <ayn.khatib@gmail.com>

Create() discarded successfully-created checkpoint keys whenever any sibling instance in the same batch failed, because the outer loop skipped the append on error. This left orphaned, untracked checkpoints on the platform while the CLI reported total failure. Signed-off-by: ayn <ayn.khatib@gmail.com>

getInstanceHistory() skipped only entries with an explicit non-success status, so a nil Status fell through and was treated as successful. Every other status check in this file (and codebase) treats a missing status as failure, not success. Signed-off-by: ayn <ayn.khatib@gmail.com>

--image, --template, --branch, and --checkpoint are mutually exclusive per the SDK docs, but only the roms field enforced a similar pattern. Reject the request when more than one source is set instead of silently letting one win. Signed-off-by: ayn <ayn.khatib@gmail.com>

Signed-off-by: ayn <ayn.khatib@gmail.com> Signed-off-by: ayn <ayn.khatib@gmail.com>

Signed-off-by: ayn <ayn.khatib@gmail.com>

InstanceCheckpoint.Create() never set the SDK's TimeoutS field, so there was no way to wait for a checkpoint to reach its terminal state before restoring from it. Wire a wait-timeout field through, matching the existing pattern on instance create, and use it in the create-from-checkpoint and checkpoint-state integration tests to close a race against checkpoint readiness. Signed-off-by: ayn <ayn.khatib@gmail.com>

HTTPPost retried the non-idempotent POST /increment call on any transport or read error, including cases where the server had already applied the increment before the response was lost. Send a stable Idempotency-Key header across retries and have the test counter server dedupe by key so repeated deliveries return the cached result instead of re-applying the delta. Signed-off-by: ayn <ayn.khatib@gmail.com>

Every existing checkpoint create case passed a single instance target, so the multi-ref creation loop and Get() ref-matching logic never ran against 2+ items. Signed-off-by: ayn <ayn.khatib@gmail.com>

Signed-off-by: ayn <ayn.khatib@gmail.com>

The identical delete-lock field on instances already has a dedicated test; checkpoints had none. Signed-off-by: ayn <ayn.khatib@gmail.com>

create-from-checkpoint only checked the restored instance's name, not that image/memory/vcpus were inherited from the checkpoint. Signed-off-by: ayn <ayn.khatib@gmail.com>

The SDK documents --branch as supporting running, stopped, or template sources, but only the running case was tested. Signed-off-by: ayn <ayn.khatib@gmail.com>

restart-follow needs more than 5s to catch the second boot's log line since restart has extra stop/start overhead over a plain start. branch-stopped and branch-template are skipped: verified live against staging that the backend does not preserve in-guest memory state across a stop, and branch_from cannot resolve template-state sources at all. Signed-off-by: ayn <ayn.khatib@gmail.com>

khatibomar · 2026-08-04T17:40:13Z

+	// instance is stopped rather than running, and that the branched
+	// instance carries over the state the source had when it was stopped.
+	t.Run("branch-stopped", func(t *testing.T) {
+		// NOTE: Remove once branching from a stopped source carries over its


This one is weird, is that the expected behavior currently?

Yeah, this is weird. We should check that branching from a stopped state works, but the application isn't running.

Instead, we would need to check something from disk, not from memory.

Error Trace: /home/runner/_work/cli/cli/cmd/unikraft/integration/instance_test.go:1264 Error: "{\"count\": 0}" does not contain "\"count\": 5" Test: TestInstances/branch-stopped

It seems it doesn't copy the fs either.

I added a skip also for the disk test for now

Hm, worth checking actually. This works for not branched instances right? Where a start/stop, keeps the disk?

If it does, and branching isn't copying the disk contents, then something feels off to me.

Based on tests that I tried the root disk get wiped on reboot, regardless if it's branched or not. Which means reboot/stop instance itself wipes root disk. Only attached volumes persist.

Oh okay, welp. Branching should copy the volumes - so we can test that then.

Okay, added

Am I missing it? I don't see it pushed.

(also needs rebasing).

91ce09f

this commit

jedevc · 2026-08-05T10:20:53Z

+		case "instances":
+			instances = field.Create.Set.([]string)


Hm, this is weird, but it is like templates. I wonder if it's worth making this a breaking change, and making both checkpoint and template only accept a single instance. It feels weird to accept multiple.

Returning multiple resources from create was only really intended for handling --replicas.

WDYT?

I think we have some usage metrics ( which I believe can be disabled by users )
So we can kinda predict if this breaking change is a big deal or not.

I think we safe to do that

jedevc · 2026-08-05T10:22:41Z

+			// Create checkpoints one at a time since the platform API only accepts single operations
+			for _, ref := range refs {
+				refStr := cmp.Or(ref.Name, ref.UUID)
+				log.G(ctx).Trace().Str("ref", refStr).Msg("creating instance checkpoint")
+				req := platform.CreateCheckpointInstancesRequestItem{
+					From:     ref.NameOrUUID(),
+					TimeoutS: timeoutS,
+				}
+				resp, err := c.CreateCheckpointInstances(ctx, []platform.CreateCheckpointInstancesRequestItem{req})


So if we do decide to support bulk requests, you should be able to attach multiple reqs. Similar for templates.

instance_checkpoints.go:

cli/internal/cmd/instance_checkpoints.go

Lines 379 to 387 in a9d3708

var errs []error

// Create checkpoints one at a time since the platform API only accepts single operations

for _, ref := range refs {

refStr := cmp.Or(ref.Name, ref.UUID)

log.G(ctx).Trace().Str("ref", refStr).Msg("creating instance checkpoint")

req := platform.CreateCheckpointInstancesRequestItem{

From: ref.NameOrUUID(),

TimeoutS: timeoutS,

}

instance_templates.go:

cli/internal/cmd/instance_templates.go

Lines 370 to 385 in a9d3708

// Create templates one at a time since the platform API only accepts single operations

for _, ref := range refs {

refStr := cmp.Or(ref.Name, ref.UUID)

var reqItem platform.CreateTemplateInstancesRequestItem

if ref.Name != "" {

reqItem.Name = new(ref.Name)

} else {

reqItem.Uuid = new(ref.UUID)

}

log.G(ctx).Trace().Str("ref", refStr).Msg("creating instance template")

resp, err := c.CreateTemplateInstances(

ctx,

[]platform.CreateTemplateInstancesRequestItem{

reqItem,

},

)

volume_templates.go:

cli/internal/cmd/volume_templates.go

Lines 353 to 357 in a9d3708

// Create templates one at a time since the platform API only accepts single operations

for _, ref := range refs {

refStr := cmp.Or(ref.Name, ref.UUID)

log.G(ctx).Trace().Str("ref", refStr).Msg("creating volume template")

resp, err := c.CreateTemplateVolume(ctx, []platform.NameOrUUID{ref.NameOrUUID()})

Ham templates following the same pattern

The checkpoint subcommand's help group key was never registered with kong.ExplicitGroups, so it rendered as the raw key "cmd-checkpoints" instead of "Checkpoints:" like the other resource groups. Signed-off-by: ayn <ayn.khatib@gmail.com>

The table printer skipped a field whenever its Go zero value was reflect-zero, even when the field renders meaningful text at that value (e.g. a zero RelativeTime renders as "never"). This left the CREATED column blank under a populated header instead of showing "never", inconsistent with the kv-all view. Signed-off-by: ayn <ayn.khatib@gmail.com>

Replaces the manual Name/UUID field-by-field construction of platform.NameOrUUID with the existing group.Ref.NameOrUUID() helper. Signed-off-by: ayn <ayn.khatib@gmail.com>

Checkpoints could only be polled via repeated inspect calls, so the checkpoint test tolerated a transient "starting" state instead of waiting for it to settle. Wire up the generic WaitableResourceCmd and have the test wait for state==checkpoint instead. Signed-off-by: ayn <ayn.khatib@gmail.com>

Branching from a stopped source can't carry over in-guest memory (stopping doesn't take a memory snapshot), so the test was skipped entirely rather than verifying anything meaningful. Persist the counter to disk in the test app and read it back on boot, so the test can verify what branching from a stopped instance actually should carry over: the source's disk state. Signed-off-by: ayn <ayn.khatib@gmail.com>

Verified live against staging: branching from a stopped instance always boots the branch with a fresh, empty disk (count resets to 0), so the assertion added in a9d3708 fails consistently rather than flaking. Same backend gap already documented for branch-template. Signed-off-by: ayn <ayn.khatib@gmail.com> Signed-off-by: ayn <ayn.khatib@gmail.com>

Accepting multiple instances per invocation was inconsistent with the platform API, which only ever creates one checkpoint/template at a time anyway. Narrow both commands to a single instance argument. Signed-off-by: ayn <ayn.khatib@gmail.com>

Signed-off-by: ayn <ayn.khatib@gmail.com>

Branching copies attached volumes, so use one to verify state survives a branch of a stopped instance, instead of the root disk which never survives a stop. Verified against staging that the branch gets an independent copy of the volume. Signed-off-by: ayn <ayn.khatib@gmail.com>

jedevc · 2026-08-07T09:48:28Z

@khatibomar can you resolve the code comments that are resolved?

khatibomar · 2026-08-07T09:55:21Z

I did close the ones that I don't think they need follow up, the opened ones, need some input or just an ACK

Signed-off-by: ayn <ayn.khatib@gmail.com>

feat: Add instance branching

a96fe64

Signed-off-by: Justin Chadwell <justin@unikraft.com>

khatibomar reviewed Jul 31, 2026

View reviewed changes

Comment thread cmd/unikraft/integration/instance_test.go Outdated

jedevc assigned khatibomar Jul 31, 2026

jedevc and others added 3 commits July 31, 2026 14:48

feat: Add checkpoints

805e312

Signed-off-by: Justin Chadwell <justin@unikraft.com>

chore: Merge with prod-staging

d17d216

Signed-off-by: ayn <ayn.khatib@gmail.com>

refactor: Move test counter server into .py file

8078334

Signed-off-by: ayn <ayn.khatib@gmail.com> Signed-off-by: ayn <ayn.khatib@gmail.com> Signed-off-by: ayn <ayn.khatib@gmail.com>

khatibomar force-pushed the jedevc/instance-branching branch 2 times, most recently from 384f0e7 to f78fa9b Compare July 31, 2026 17:40

fix: Replace go-sdk with go-sdk-internal

a2224e5

Signed-off-by: ayn <ayn.khatib@gmail.com>

khatibomar force-pushed the jedevc/instance-branching branch from f78fa9b to a2224e5 Compare July 31, 2026 17:46

khatibomar added 6 commits July 31, 2026 22:21

fix: Remove go-sdk-internal replace and use public go-sdk again

08d8f90

Signed-off-by: ayn <ayn.khatib@gmail.com> Signed-off-by: ayn <ayn.khatib@gmail.com>

khatibomar force-pushed the jedevc/instance-branching branch from 3ec0c39 to 08d8f90 Compare July 31, 2026 19:40

khatibomar added 9 commits July 31, 2026 22:41

chore: Go mod tidy

45c12fd

Signed-off-by: ayn <ayn.khatib@gmail.com>

test: Add coverage for bulk instance checkpoint create

7d2c0f0

Every existing checkpoint create case passed a single instance target, so the multi-ref creation loop and Get() ref-matching logic never ran against 2+ items. Signed-off-by: ayn <ayn.khatib@gmail.com>

test: Verify checkpoint outlives its source instance

622b7e3

Signed-off-by: ayn <ayn.khatib@gmail.com>

chore: Update gold test

7a540e6

Signed-off-by: ayn <ayn.khatib@gmail.com>

test: Add coverage for checkpoint delete-lock

7623704

The identical delete-lock field on instances already has a dedicated test; checkpoints had none. Signed-off-by: ayn <ayn.khatib@gmail.com>

test: Assert config fidelity on checkpoint-restored instance

5cafa79

create-from-checkpoint only checked the restored instance's name, not that image/memory/vcpus were inherited from the checkpoint. Signed-off-by: ayn <ayn.khatib@gmail.com>

test: Add coverage for branching from stopped and template instances

1cebe2c

The SDK documents --branch as supporting running, stopped, or template sources, but only the running case was tested. Signed-off-by: ayn <ayn.khatib@gmail.com>

khatibomar marked this pull request as ready for review August 4, 2026 16:06

khatibomar reviewed Aug 4, 2026

View reviewed changes

jedevc commented Aug 5, 2026

View reviewed changes

Comment thread internal/cmd/instances.go Outdated

jedevc commented Aug 5, 2026

View reviewed changes

khatibomar added 6 commits August 5, 2026 19:17

fix: Register cmd-checkpoints help group title

433ec8a

The checkpoint subcommand's help group key was never registered with kong.ExplicitGroups, so it rendered as the raw key "cmd-checkpoints" instead of "Checkpoints:" like the other resource groups. Signed-off-by: ayn <ayn.khatib@gmail.com>

refactor: Use Ref().NameOrUUID() for branch and checkpoint create fields

70a5f5b

Replaces the manual Name/UUID field-by-field construction of platform.NameOrUUID with the existing group.Ref.NameOrUUID() helper. Signed-off-by: ayn <ayn.khatib@gmail.com>

khatibomar changed the title ~~feat: Add instance branching + checkpointing~~ feat!: Add instance branching + checkpointing Aug 6, 2026

khatibomar added 3 commits August 6, 2026 16:36

refactor: Test comments

bd85612

Signed-off-by: ayn <ayn.khatib@gmail.com>

chore: Merge prod-staging into instance-branching

fda2109

Signed-off-by: ayn <ayn.khatib@gmail.com>

khatibomar force-pushed the jedevc/instance-branching branch from bde63fb to fda2109 Compare August 7, 2026 09:55

	var errs []error
	// Create checkpoints one at a time since the platform API only accepts single operations
	for _, ref := range refs {
	refStr := cmp.Or(ref.Name, ref.UUID)
	log.G(ctx).Trace().Str("ref", refStr).Msg("creating instance checkpoint")
	req := platform.CreateCheckpointInstancesRequestItem{
	From: ref.NameOrUUID(),
	TimeoutS: timeoutS,
	}

	// Create templates one at a time since the platform API only accepts single operations
	for _, ref := range refs {
	refStr := cmp.Or(ref.Name, ref.UUID)
	var reqItem platform.CreateTemplateInstancesRequestItem
	if ref.Name != "" {
	reqItem.Name = new(ref.Name)
	} else {
	reqItem.Uuid = new(ref.UUID)
	}
	log.G(ctx).Trace().Str("ref", refStr).Msg("creating instance template")
	resp, err := c.CreateTemplateInstances(
	ctx,
	[]platform.CreateTemplateInstancesRequestItem{
	reqItem,
	},
	)

Uh oh!

Conversation

jedevc commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jedevc commented Aug 7, 2026

Uh oh!

khatibomar commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jedevc commented Jul 7, 2026 •

edited

Loading

khatibomar commented Aug 7, 2026 •

edited

Loading