fix: change swap_size to pointer type and update related logic#418
fix: change swap_size to pointer type and update related logic#418zliang-akamai wants to merge 1 commit intodevfrom
Conversation
zliang-akamai
requested review from
dawiddzhafarov,
jriddle-linode and
psnoch-akamai
and removed request for
a team
|
Isn't that a breaking change? |
There was a problem hiding this comment.
Pull request overview
Updates the Linode builder config so swap_size can be omitted from the Linode API request payload when not configured, aligning request behavior with optional configuration semantics.
Changes:
- Changed
Config.SwapSizefromintto*intto distinguish “unset” vs “set to 0”. - Updated instance creation to pass
SwapSizethrough as a pointer (allowing omission when nil). - Added unit tests and updated generated docs to reflect the new optional/pointer behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| builder/linode/step_create_linode.go | Passes swap size as a pointer so it can be omitted when unset. |
| builder/linode/config.go | Changes swap_size to *int and updates validation logic to handle nil safely. |
| builder/linode/builder_test.go | Adds tests for swap_size omitted vs explicitly set to 0. |
| .web-docs/components/builder/linode/README.md | Updates generated docs type display for swap_size. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| if c.SwapSize > 0 { | ||
| if c.SwapSize != nil && *c.SwapSize > 0 { |
There was a problem hiding this comment.
With SwapSize now a pointer, swap_size can be detected as explicitly set even when its value is 0. The current check only errors when *c.SwapSize > 0, but both the error message and the docs state swap_size must not be specified when using custom disks. Consider treating any non-nil SwapSize as incompatible in custom-disk mode (or adjust the messaging/docs to match the intended behavior).
| if c.SwapSize != nil && *c.SwapSize > 0 { | |
| if c.SwapSize != nil { |
| t.Run("explicit zero remains non-nil", func(t *testing.T) { | ||
| var b Builder | ||
| config := testConfig() | ||
| config["swap_size"] = 0 | ||
|
|
||
| _, warnings, err := b.Prepare(config) | ||
| if len(warnings) > 0 { | ||
| t.Fatalf("bad: %#v", warnings) | ||
| } | ||
| if err != nil { | ||
| t.Fatalf("should not have error: %s", err) | ||
| } | ||
|
|
||
| if b.config.SwapSize == nil || *b.config.SwapSize != 0 { | ||
| t.Fatalf("swap_size = %v, want pointer to 0", b.config.SwapSize) | ||
| } | ||
| }) |
There was a problem hiding this comment.
The new SwapSize pointer behavior is tested for omitted vs explicitly set to 0, but there’s no coverage for the custom-disk incompatibility case when swap_size is explicitly set to 0. If the intent is that swap_size must not be specified with custom disks, add a test case for swap_size = 0 alongside the existing incompatible-swap-size test to prevent regressions.
📝 Description
When swap_size is not configured, we should not include it in the request payload. Update it to be a pointer type to achieve it.
✔️ How to Test