-
Notifications
You must be signed in to change notification settings - Fork 29
api: report the actual fork mode on Instance #419
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
dc161ad
47bae27
8024927
01d649b
8127e47
a1dab35
a603396
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -94,6 +94,9 @@ func TestForkInstance_VZStoppedSourceSupported(t *testing.T) { | |
| assert.Equal(t, StateStopped, forked.State) | ||
| assert.Equal(t, hypervisor.TypeVZ, forked.HypervisorType) | ||
| assert.NotEqual(t, sourceID, forked.Id) | ||
| // A VZ stopped-source fork copies memory rather than sharing it, and the | ||
| // instance reports that measured mode (barista-046 §3.4 consumes this). | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: please remove the consumer-specific issue reference from this comment; the assertion is self-explanatory, and code comments should not include external tracking context. |
||
| assert.Equal(t, ForkModeCopied, forked.ForkMode) | ||
| } | ||
|
|
||
| func TestResolveForkTargetState_DefaultsToSourceState(t *testing.T) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -428,6 +428,24 @@ func (m *manager) forkSnapshot(ctx context.Context, snapshotID string, req ForkS | |
| forkMeta := cloneStoredMetadataWithoutPendingStandbyCompression(rec.StoredMetadata) | ||
| forkMeta.Id = forkID | ||
| forkMeta.Name = req.Name | ||
| // Caller-supplied tags override the cloned source tags (request wins per | ||
| // key), so a fork can be re-identified rather than inherit the source's | ||
| // identity labels. Unrelated source tags are preserved. | ||
| if len(req.Tags) > 0 { | ||
| if forkMeta.Tags == nil { | ||
| forkMeta.Tags = make(map[string]string, len(req.Tags)) | ||
| } | ||
| for k, v := range req.Tags { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The request tags are validated before this merge, but the resulting map can exceed |
||
| forkMeta.Tags[k] = v | ||
| } | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| // Record the measured fork mode, mirroring the instance-fork path: a shared | ||
| // mem-file is copy-on-write, anything else is a full copy. | ||
| if shareMemFile { | ||
| forkMeta.ForkMode = ForkModeShared | ||
| } else { | ||
| forkMeta.ForkMode = ForkModeCopied | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| forkMeta.CreatedAt = now | ||
| forkMeta.ExpiresAt = nil | ||
| forkMeta.StartedAt = nil | ||
|
|
@@ -596,6 +614,12 @@ func validateForkSnapshotRequest(req ForkSnapshotRequest) error { | |
| if req.TargetState != "" && req.TargetState != StateStopped && req.TargetState != StateStandby && req.TargetState != StateRunning { | ||
| return fmt.Errorf("%w: invalid target_state %q", ErrInvalidRequest, req.TargetState) | ||
| } | ||
| // Caller-supplied tags are written into instance metadata below, so they must | ||
| // clear the same validation as create-instance and create-snapshot — the | ||
| // OpenAPI layer does not guard this direct manager path. | ||
| if err := tags.Validate(req.Tags); err != nil { | ||
| return fmt.Errorf("%w: %v", ErrInvalidRequest, err) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This records the initial
shareMemFiledecision, not whether the memory file was actually shared.cloneGuestDirectoryForForkdisables sharing when the raw mem-file is absent, andlinkForkFirecrackerMemFilefalls back to a copy whenos.Linkfails; neither outcome reaches this metadata. The snapshot-fork path has the same issue atlib/instances/snapshot.go:444. Could the copy helper return the actual outcome and have both paths persist it?