Fix/instance wait live meta v0.0.23#35
Conversation
instance create/start/stop --wait polled the cached list/show endpoint
(svc.Get), which can report Starting for many minutes after a VM is
actually Running because the CMP's background state reconciliation is
unreliable. --wait could hang until timeout on an already-running VM.
Poll GET /virtual-machines/{slug}/meta instead: it performs a live
reconcile against the platform (CloudStack/APC) and returns the
authoritative state. Adds SDK method instance.Service.Meta. Verified
live: create --wait returned Running via /meta while instance list still
showed Starting.
The flag advertised releasing the auto-assigned public IP on delete, but that never worked against the live API: DELETE ignores it and the IP-releasing PUT .../destroy endpoint rejects API-token auth (a CMP bug, under fix). The help, confirmation prompt, and examples now state the IP is not auto-released and point to 'zcp ip release <ip-slug>' as the manual workaround. Messaging only; no behavior change.
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a ChangesInstance meta polling and delete UX correction
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as instance --wait
participant Service as instance.Service
participant API as CMP API
CLI->>Service: WaitForState(ctx, slug, targetStates)
loop poll ticks
Service->>API: GET /virtual-machines/{slug}/meta
API-->>Service: VMMeta{State}
alt meta.State matches target
Service->>API: Get(ctx, slug)
API-->>Service: VirtualMachine
Service->>Service: vm.State = meta.State
Service-->>CLI: VirtualMachine
end
end
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/api/instance/instance.go (2)
630-633: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winTransient
Metaerrors abort the entire polling loop.
WaitForStatereturns immediately on anyMetaerror. For a function designed to poll for minutes, a single transient failure (network timeout, 502, etc.) will abort the wait and force the caller to retry from scratch. Consider logging the error and continuing to the next tick, only returning on context cancellation.♻️ Suggested refactor: continue polling on transient errors
case <-ticker.C: meta, err := s.Meta(ctx, slug) if err != nil { - return nil, err + // Log and keep polling; only context cancellation stops the wait. + continue }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/api/instance/instance.go` around lines 630 - 633, WaitForState currently exits immediately when s.Meta(ctx, slug) returns an error, which makes transient failures break the whole poll loop. Update WaitForState so Meta errors are logged and the loop continues to the next tick instead of returning, and only exit early on context cancellation or a terminal state condition. Use the existing WaitForState and Meta calls to locate the polling logic.
584-641: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd tests for
MetaandWaitForState.pkg/api/instance/instance_test.gocovers List/Get/Start/Stop/Delete, but not the new/metapath or the updated polling/error paths inWaitForState.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/api/instance/instance.go` around lines 584 - 641, The new `Meta` and `WaitForState` behavior in `Service` is untested, so add coverage in `instance_test.go` for the `/virtual-machines/{slug}/meta` path and the polling flow. Verify `Meta` returns a decoded `VMMeta` on success and propagates HTTP/status and unmarshal errors, and that `WaitForState` uses `Meta` to detect target states, returns the `VirtualMachine` with the authoritative state, and surfaces context/lookup errors correctly. Use the existing `Service`, `Meta`, and `WaitForState` helpers in the test suite to keep the new tests aligned with the current API patterns.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/api/instance/instance.go`:
- Around line 630-633: WaitForState currently exits immediately when s.Meta(ctx,
slug) returns an error, which makes transient failures break the whole poll
loop. Update WaitForState so Meta errors are logged and the loop continues to
the next tick instead of returning, and only exit early on context cancellation
or a terminal state condition. Use the existing WaitForState and Meta calls to
locate the polling logic.
- Around line 584-641: The new `Meta` and `WaitForState` behavior in `Service`
is untested, so add coverage in `instance_test.go` for the
`/virtual-machines/{slug}/meta` path and the polling flow. Verify `Meta` returns
a decoded `VMMeta` on success and propagates HTTP/status and unmarshal errors,
and that `WaitForState` uses `Meta` to detect target states, returns the
`VirtualMachine` with the authoritative state, and surfaces context/lookup
errors correctly. Use the existing `Service`, `Meta`, and `WaitForState` helpers
in the test suite to keep the new tests aligned with the current API patterns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 39af3dc1-613b-44f0-91a3-0f028a7cfb25
📒 Files selected for processing (4)
CHANGELOG.mdRELEASE_NOTES.mdinternal/commands/instance.gopkg/api/instance/instance.go
govulncheck flagged GO-2026-5856 (Encrypted Client Hello privacy leak in the stdlib crypto/tls), fixed in go1.26.5. Bump the go.mod toolchain and the CI go-version pins (build.yml x3, smoke.yml) from 1.26.4 to 1.26.5 so the security scan builds against the patched standard library. Verified locally: build + tests pass on go1.26.5 and govulncheck reports 0 called vulnerabilities.
1 participant
Summary by CodeRabbit
instance --waitnow uses live VM state, so create/start/stop commands complete more reliably when updates take longer to reconcile.instance delete --delete-public-ipmessaging now correctly warns that public IPs are not released automatically and must be freed manually after deletion.