Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(vm): add support for pool update #505

Merged
merged 3 commits into from
Aug 19, 2023
Merged

feat(vm): add support for pool update #505

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2c7a531
feat(vm): pool update support
tseeker Aug 18, 2023
e76be5d
Merge branch 'main' into eb2308-vm-pool-update
bpg Aug 19, 2023
d7a8c22
fix: use `types.CustomCommaSeparatedList` in `PoolUpdateRequestBody` …
bpg Aug 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions proxmox/pools/pool_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

package pools

import "github.com/bpg/terraform-provider-proxmox/internal/types"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh... I didn't realize this mistake that I've made before -- the proxmox package (API client) is now dependent on the internal package types 😞
You're also adding new API types and more dependencies between API and FWK types in #498.

This all won't work long term, as I think it will make sense to extract the API client into a separate repo (#423)

But I'll leave it as is for now, the "separation" refactoring can be done later.


// PoolCreateRequestBody contains the data for a pool create request.
type PoolCreateRequestBody struct {
Comment *string `json:"comment,omitempty" url:"comment,omitempty"`
Expand Down Expand Up @@ -45,5 +47,12 @@ type PoolListResponseData struct {

// PoolUpdateRequestBody contains the data for an pool update request.
type PoolUpdateRequestBody struct {
// The pool's comment
Comment *string `json:"comment,omitempty" url:"comment,omitempty"`
// If this is set to 1, VMs and datastores will be removed from the pool instead of added.
Delete *types.CustomBool `json:"delete,omitempty" url:"delete,omitempty,int"`
// The list of virtual machines to add or delete.
VMs *[]string `json:"vms,omitempty" url:"vms,omitempty,comma"`
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd need a custom JSON serialization to a comma-separated list, though it works now as there is only a single VM ID in the use case.

// The list of datastores to add or delete.
Storage *[]string `json:"storage,omitempty" url:"storage,omitempty,comma"`
}
50 changes: 49 additions & 1 deletion proxmoxtf/resource/vm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"time"
"unicode"

"github.com/google/go-cmp/cmp"
"github.com/google/uuid"
"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
Expand All @@ -27,6 +28,7 @@ import (
"github.com/bpg/terraform-provider-proxmox/internal/types"
"github.com/bpg/terraform-provider-proxmox/proxmox/cluster"
"github.com/bpg/terraform-provider-proxmox/proxmox/nodes/vms"
"github.com/bpg/terraform-provider-proxmox/proxmox/pools"
"github.com/bpg/terraform-provider-proxmox/proxmoxtf"
"github.com/bpg/terraform-provider-proxmox/proxmoxtf/resource/validator"
"github.com/bpg/terraform-provider-proxmox/proxmoxtf/structure"
Expand Down Expand Up @@ -1212,7 +1214,6 @@ func VM() *schema.Resource {
Type: schema.TypeString,
Description: "The ID of the pool to assign the virtual machine to",
Optional: true,
ForceNew: true,
Default: dvResourceVirtualEnvironmentVMPoolID,
},
mkResourceVirtualEnvironmentVMSerialDevice: {
Expand Down Expand Up @@ -4782,6 +4783,49 @@ func vmReadPrimitiveValues(
return diags
}

// vmUpdatePool moves the VM to the pool it is supposed to be in if the pool ID changed.
func vmUpdatePool(
ctx context.Context,
d *schema.ResourceData,
api *pools.Client,
vmID int,
) error {
oldPoolValue, newPoolValue := d.GetChange(mkResourceVirtualEnvironmentVMPoolID)
if cmp.Equal(newPoolValue, oldPoolValue) {
return nil
}

oldPool := oldPoolValue.(string)
newPool := newPoolValue.(string)
vmList := []string{strconv.Itoa(vmID)}

tflog.Debug(ctx, fmt.Sprintf("Moving VM %d from pool '%s' to pool '%s'", vmID, oldPool, newPool))

if oldPool != "" {
trueValue := types.CustomBool(true)
poolUpdate := &pools.PoolUpdateRequestBody{
VMs: &vmList,
Delete: &trueValue,
}

err := api.UpdatePool(ctx, oldPool, poolUpdate)
if err != nil {
return fmt.Errorf("while removing VM %d from pool %s: %w", vmID, oldPool, err)
}
}

if newPool != "" {
poolUpdate := &pools.PoolUpdateRequestBody{VMs: &vmList}

err := api.UpdatePool(ctx, newPool, poolUpdate)
if err != nil {
return fmt.Errorf("while adding VM %d to pool %s: %w", vmID, newPool, err)
}
}

return nil
}

func vmUpdate(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
config := m.(proxmoxtf.ProviderConfiguration)

Expand All @@ -4798,6 +4842,10 @@ func vmUpdate(ctx context.Context, d *schema.ResourceData, m interface{}) diag.D
return diag.FromErr(e)
}

if vmUpdatePool(ctx, d, api.Pool(), vmID) != nil {
return diag.FromErr(e)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this swallows the returned error value and uses error e from the previous statement

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woops, sorry. Thanks for the fix.

}

vmAPI := api.Node(nodeName).VM(vmID)

updateBody := &vms.UpdateRequestBody{
Expand Down