Skip to content

cli: prepare the restart config the same way as start - #1645

Open
somaz94 wants to merge 2 commits into
abiosoft:mainfrom
somaz94:fix/restart-prepare-config
Open

somaz94 wants to merge 2 commits into
abiosoft:mainfrom
somaz94:fix/restart-prepare-config

Conversation

@somaz94

@somaz94 somaz94 commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Refs #1561

Problem

With COLIMA_SAVE_CONFIG=0 and a partial, read-only colima.yaml (the Nix setup in #1561), colima start works but colima restart stops the VM and then fails, leaving it stopped. With this file:

cpu: 2
memory: 2
disk: 20
runtime: containerd
vmType: vz

restart ends with:

Resize instance colima-r1561's disk from 20GiB to 0B
disk shrinking is not supported

Cause

colima start runs prepareConfig first, which fills in the flag defaults, the template and the fixed settings. colima restart passed the file to app.Start exactly as it was read. With the default COLIMA_SAVE_CONFIG=1 that file is the full config saved by the last start, so nothing is missing. With a partial file, restart starts with rootDisk: 0, so Lima tries to shrink the root disk, and with binfmt unset, which skips the binfmt setup.

Fix

restart now prepares the config with the same prepareConfig as start and validates it before stopping the VM, so a config that fails validation no longer leaves the VM stopped. It still never writes the config file. When the file is missing or has no runtime, prepareConfig would ignore it and use plain defaults, which can grow the data disk (for example after COLIMA_SAVE_CONFIG=0 colima start --disk 60), so restart uses the config the instance last started with instead. An unreadable file is still an error, as before.

Testing

Test_restartConfig covers a partial file, a missing file and an unreadable file, and the first two fail on main. go test ./... passes on macOS and in a golang:1.26.1 Linux container, and golangci-lint v2.11.3 reports no issues for darwin and linux. I also reproduced the issue on macOS 14 (Apple Silicon, vz, containerd) with the file above: restart fails on main as shown and works with this change whether the VM is running or stopped. With the file moved away, it restarts with the instance's last config and the data disk stays at 20GiB.

The networks.yaml and colima list symptoms in #1561 are not covered here.

AI disclosure: all of the code and tests in this PR were written with Claude Code (Claude Opus 5.5), and the checks above were run through it. I reviewed the whole diff before opening this PR.

Signed-off-by: somaz <genius5711@gmail.com>
somaz94 added a commit to somaz94/somaz94 that referenced this pull request Oct 6, 2026
@phant0um

Copy link
Copy Markdown

I checked this PR against main with unit tests only, without a VM. On main, colima restart gives the partial file from #1561 to app.Start with rootDisk 0 and binfmt unset. The head fills both in. When restartConfig is changed back to main's behavior, the new test fails on the 2 cases it should.

Environment

  • main 49dfe87 (this PR's base) and head 141d529.
  • Go 1.26.4 on macOS (Apple Silicon), GOTOOLCHAIN=local.
  • COLIMA_HOME and LIMA_HOME pointed at temp dirs.
  • Each test command was run twice, with the same output both times.

Tests

The new test does not compile on main, because restartConfig does not exist there. So on main I used a probe test. It writes the file from #1561 (cpu: 2, memory: 2, disk: 20, runtime: containerd, vmType: vz) and calls configmanager.Load(), which is what cmd/restart.go:40 gives to app.Start on main. It prints:

rootDisk=0 binfmtNil=true disk=20

go test ./cmd -run Test_restartConfig -count=1 -v:

Tree Result
Head 3 of 3 subtests pass
Head with the body of restartConfig replaced by return configmanager.Load() partial_config_gets_start_defaults and missing_config_uses_the_instance_state fail, unreadable_config_is_an_error passes

Output of the failing run:

restart_test.go:57: restartConfig() RootDisk = 0, want 20
restart_test.go:60: restartConfig() Binfmt = <nil>, want true
restart_test.go:72: restartConfig() did not use the instance state: cpu=0 disk=0 runtime=""

On the head, go test ./... -count=1 exits 0, go vet ./cmd exits 0 and gofmt -l cmd prints nothing. golangci-lint v2.14.0 with the repo's .golangci.yml reports 0 issues. for ./cmd/.... go test ./cmd -count=1 passes twice, so the new TestMain does not break the other cmd tests.

Notes from reading the code

I did not test these points.

  1. The tests cover restartConfig() only. No test checks that ValidateConfig runs before app.Stop. If the 2 calls change order, all tests stay green.
  2. With the default COLIMA_SAVE_CONFIG=1, restart now goes through prepareConfig too. This gives the same result as start, with 2 small differences from main's restart:
    • ValidateConfig can now refuse a saved config that restart accepted before. This happens before app.Stop, so the VM keeps running.
    • prepareConfig does not copy diskImage from the file. The value comes from the --disk-image flag default, which is empty (cmd/start.go:196). On main, restart gave the saved diskImage to app.Start.
  3. The third subtest uses a YAML parse error (cpu: [), not a file that cannot be read because of permissions. Both go through LoadFrom.

Not tested

Signed-off-by: somaz <genius5711@gmail.com>
@somaz94

somaz94 commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for testing and catching the diskImage omission. I have pushed a fix in f0b8f31 to preserve any saved custom diskImage on restart, along with a test case covering it. Regarding validation ordering, ValidateConfig returns on error before app.Stop is called in RunE, so invalid configurations will not stop a running instance.

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