feat: support VPC network type and existing network attachment for instance creation - #47
feat: support VPC network type and existing network attachment for instance creation#47cokerrd wants to merge 4 commits into
Conversation
|
Warning Review limit reached
Next review available in: 40 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 Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesInstance network creation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant InstanceCreateCommand
participant instance.Service.Create
participant API server
User->>InstanceCreateCommand: Provide network options
InstanceCreateCommand->>InstanceCreateCommand: Validate options by network type
InstanceCreateCommand->>instance.Service.Create: Send CreateRequest
instance.Service.Create->>API server: POST /virtual-machines
API server-->>instance.Service.Create: Return created VM
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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
pkg/api/instance/instance_test.go (1)
49-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the new network request fields.
TestCreateonly verifiesname. A missing or incorrectvr_plan,default_network, ornetworksfield will still pass this test. Populate these fields inreqand assert their JSON values ingotBody.🤖 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_test.go` around lines 49 - 64, Update TestCreate to populate req with representative values for vr_plan, default_network, and networks, then assert the corresponding JSON fields in gotBody alongside the existing name check. Preserve the current Create success and VM response assertions.
🤖 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.
Inline comments:
In `@internal/commands/instance.go`:
- Around line 546-555: Update the flag descriptions in the command setup around
the networkType, networkPlan, and vrPlan variables to reflect validation: remove
“required” from --network-type, and state that --network-plan and --vr-plan are
only required when creating their applicable network types unless --networks is
provided.
- Around line 461-462: Update the network validation around the existing
len(networks) check in the instance command to require a non-empty
defaultNetwork only when networks are provided, and verify that its value
matches one of the entries in networks. Reject values that are absent from
networks, including when defaultNetwork is supplied without networks, before
reaching the API.
---
Nitpick comments:
In `@pkg/api/instance/instance_test.go`:
- Around line 49-64: Update TestCreate to populate req with representative
values for vr_plan, default_network, and networks, then assert the corresponding
JSON fields in gotBody alongside the existing name check. Preserve the current
Create success and VM response assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 12e1a1f2-0490-453d-98b9-0ec693259c30
📒 Files selected for processing (3)
internal/commands/instance.gopkg/api/instance/instance.gopkg/api/instance/instance_test.go
| cmd.Flags().StringVar(&plan, "plan", "", "Plan slug (e.g. ca2sxs- see: zcp plan vm (required)") | ||
| cmd.Flags().StringVar(&billingCycle, "billing-cycle", "", "Billing cycle slug: hourly, monthly, etc. (required)") | ||
| cmd.Flags().StringVar(&networkType, "network-type", "Isolated", "Network type (default: Isolated)") | ||
| cmd.Flags().StringVar(&networkType, "network-type", "Isolated", "Network type: Isolated, L2 or Vpc (required)") | ||
| cmd.Flags().StringVar(&sshKey, "ssh-key", "", "Name of an existing SSH key to attach for login (optional; see 'zcp ssh-key list')") | ||
| cmd.Flags().StringVar(&hostname, "hostname", "", "Hostname (defaults to --name)") | ||
| cmd.Flags().StringVar(&storageCategory, "storage-category", "", "Storage category (required, e.g. premium-ssd - see: zcp plan storage)") | ||
| cmd.Flags().StringVar(&computeCategory, "compute-category", "", "Compute category slug (optional)") | ||
| cmd.Flags().StringVar(&blockstoragePlan, "blockstorage-plan", "", "Block storage plan slug (optional, e.g. b2g1 — see: zcp plan storage)") | ||
| cmd.Flags().StringVar(&networkPlan, "network-plan", "", "Network plan slug (required, e.g. pnet-yow, pnet-yul — see: zcp plan network)") | ||
| cmd.Flags().StringVar(&networkPlan, "network-plan", "", "Network plan slug (optional; required when creating an Isolated or L2 network type— see: zcp plan network)") | ||
| cmd.Flags().StringVar(&vrPlan, "vr-plan", "", "Virtual router plan slug (optional; required when creating a VPC — see: zcp plan router)") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the conditional flag help text.
--network-type has a default value, so it is not required. --network-plan and --vr-plan are optional when --networks is set. Update these descriptions to match the validation rules.
🤖 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 `@internal/commands/instance.go` around lines 546 - 555, Update the flag
descriptions in the command setup around the networkType, networkPlan, and
vrPlan variables to reflect validation: remove “required” from --network-type,
and state that --network-plan and --vr-plan are only required when creating
their applicable network types unless --networks is provided.
1 participant
This adds support for creating an instance of Vpc network type, and also attaching an existing network or networks to the creation of an instance instead of creating a new network
Changes
--vr-planflag to specify the router for the VPC creation--networksflag to specify the network(s) to attach to the instance--default-networkflag to specify the default network if multiple networks are specifiedScreen.Recording.2026-08-02.at.23.23.53.mov
Summary by CodeRabbit
New Features
Bug Fixes
Tests