Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
9ad79ea
feat(core): add TaskResult type for task warnings
bpg-dev Apr 3, 2026
0f29930
feat(core): WaitForTask returns *TaskResult instead of error
bpg-dev Apr 3, 2026
238e189
refactor(core): update all WaitForTask callers to use TaskResult
bpg-dev Apr 3, 2026
2c8ab07
refactor(vm): update StartVM callers to use TaskResult
bpg-dev Apr 3, 2026
7e8cc89
feat(core): propagate task warnings through DoTask callers
bpg-dev Apr 3, 2026
ba75587
fix(core): review fixes for TaskResult implementation
bpg-dev Apr 3, 2026
a580e11
refactor(core): address review comments
bpg-dev Apr 3, 2026
6899a3f
feat(vm): propagate task warnings from all VM operations
bpg-dev Apr 3, 2026
356521e
cleanup
bpg-dev Apr 3, 2026
99e9629
simplification
bpg-dev Apr 3, 2026
d15dd1c
ignore warnings now by default
bpg-dev Apr 4, 2026
c53cb57
address gemini feedback
bpg-dev Apr 4, 2026
22c05d3
fix warning handling in stop / shutdown
bpg-dev Apr 4, 2026
8c6a21b
moar cleanups
bpg-dev Apr 4, 2026
236c1ee
diages handling improvements
bpg-dev Apr 4, 2026
e9bea6b
Merge branch 'main' into feat/2597-include-proxmox-task-results-durin…
bpg-dev Apr 4, 2026
169e22e
review fixes
bpg-dev Apr 4, 2026
e041412
fix propagating a some vm warnings
bpg-dev Apr 4, 2026
564d2c7
relocate common package
bpg-dev Apr 4, 2026
50dffa4
fix result re-wrap
bpg-dev Apr 4, 2026
84ec285
fix doc ````
bpg-dev Apr 4, 2026
f4f5364
more poliching and fixes
bpg-dev Apr 4, 2026
d1258f7
few more tests
bpg-dev Apr 4, 2026
7bb6a63
hope this is the last one
bpg-dev Apr 4, 2026
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
28 changes: 23 additions & 5 deletions fwprovider/nodes/clonedvm/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,18 @@ func (r *Resource) Create(ctx context.Context, req resource.CreateRequest, resp

retries := int(plan.Clone.Retries.ValueInt64())

err := sourceVM.CloneVM(ctx, retries, cloneBody)
if err != nil {
resp.Diagnostics.AddError("Failed to clone VM", err.Error())
cloneResult := sourceVM.CloneVM(ctx, retries, cloneBody)
if cloneResult.Err() != nil {
resp.Diagnostics.AddError("Failed to clone VM", cloneResult.Err().Error())
return
}

if cloneResult.HasWarnings() {
for _, w := range cloneResult.Warnings() {
resp.Diagnostics.AddWarning("VM clone warning", w)
}
}

vmAPI := r.client.Node(targetNode).VM(int(plan.ID.ValueInt64()))

// Read current VM config to get existing disk file paths before updating
Expand Down Expand Up @@ -193,12 +199,18 @@ func (r *Resource) Create(ctx context.Context, req resource.CreateRequest, resp
if plan.Started.ValueBool() {
tflog.Debug(ctx, "Starting VM after clone")

_, err = vmAPI.StartVM(ctx, int(timeout.Seconds()))
startResult, err := vmAPI.StartVM(ctx, int(timeout.Seconds()))
if err != nil {
resp.Diagnostics.AddError("Failed to start VM", err.Error())
return
}

if startResult.HasWarnings() {
for _, w := range startResult.Warnings() {
resp.Diagnostics.AddWarning("VM start warning", w)
}
}

err = vmAPI.WaitForVMStatus(ctx, "running")
if err != nil {
resp.Diagnostics.AddError("Failed waiting for VM to start", err.Error())
Expand Down Expand Up @@ -303,12 +315,18 @@ func (r *Resource) Update(ctx context.Context, req resource.UpdateRequest, resp
if plan.Started.ValueBool() {
tflog.Debug(ctx, "Starting VM")

_, err = vmAPI.StartVM(ctx, int(timeout.Seconds()))
startResult, err := vmAPI.StartVM(ctx, int(timeout.Seconds()))
if err != nil {
resp.Diagnostics.AddError("Failed to start VM", err.Error())
return
}

if startResult.HasWarnings() {
for _, w := range startResult.Warnings() {
resp.Diagnostics.AddWarning("VM start warning", w)
}
}

err = vmAPI.WaitForVMStatus(ctx, "running")
if err != nil {
resp.Diagnostics.AddError("Failed waiting for VM to start", err.Error())
Expand Down
4 changes: 2 additions & 2 deletions fwprovider/nodes/resource_acme_certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ func (r *acmeCertificateResource) Create(

// Wait for the task to complete
if taskID != nil && *taskID != "" {
err = nodeClient.Tasks().WaitForTask(ctx, *taskID)
err = nodeClient.Tasks().WaitForTask(ctx, *taskID).Err()
if err != nil {
resp.Diagnostics.AddError(
"Certificate order task failed",
Expand Down Expand Up @@ -417,7 +417,7 @@ func (r *acmeCertificateResource) Update(

// Wait for the task to complete
if taskID != nil && *taskID != "" {
err = nodeClient.Tasks().WaitForTask(ctx, *taskID)
err = nodeClient.Tasks().WaitForTask(ctx, *taskID).Err()
if err != nil {
resp.Diagnostics.AddError(
"Certificate renewal task failed",
Expand Down
6 changes: 5 additions & 1 deletion fwprovider/nodes/vm/concurrency_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,11 @@ func TestBatchCreate(t *testing.T) {

id := 999900 + i
if err == nil {
err = te.NodeClient().VM(sourceID).CloneVM(ctx, 5, &vms.CloneRequestBody{VMIDNew: id})
cloneResult := te.NodeClient().VM(sourceID).CloneVM(ctx, 5, &vms.CloneRequestBody{VMIDNew: id})
if cloneResult.Err() != nil {
err = cloneResult.Err()
}

ids[i] = id
}

Expand Down
6 changes: 3 additions & 3 deletions proxmox/cluster/acme/account/acme_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func (c *Client) Create(ctx context.Context, data *ACMEAccountCreateRequestBody)
return api.ErrNoDataObjectInResponse
}

err = c.Tasks().WaitForTask(ctx, *resBody.Data)
err = c.Tasks().WaitForTask(ctx, *resBody.Data).Err()
if err != nil {
return fmt.Errorf(
"error updating ACME account: failed waiting for task: %w",
Expand All @@ -89,7 +89,7 @@ func (c *Client) Update(ctx context.Context, accountName string, data *ACMEAccou
return api.ErrNoDataObjectInResponse
}

err = c.Tasks().WaitForTask(ctx, *resBody.Data)
err = c.Tasks().WaitForTask(ctx, *resBody.Data).Err()
if err != nil {
return fmt.Errorf(
"error updating ACME account: failed waiting for task: %w",
Expand All @@ -113,7 +113,7 @@ func (c *Client) Delete(ctx context.Context, accountName string) error {
return api.ErrNoDataObjectInResponse
}

err = c.Tasks().WaitForTask(ctx, *resBody.Data)
err = c.Tasks().WaitForTask(ctx, *resBody.Data).Err()
if err != nil {
return fmt.Errorf(
"error deleting ACME account: failed waiting for task: %w",
Expand Down
2 changes: 1 addition & 1 deletion proxmox/cluster/sdn/applier/applier.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (c *Client) ApplyConfig(ctx context.Context) error {
return fmt.Errorf("SDN apply did not return a task UPID")
}

err = c.Tasks().WaitForTask(ctx, *resBody.Data)
err = c.Tasks().WaitForTask(ctx, *resBody.Data).Err()
if err != nil {
return fmt.Errorf("error waiting for SDN apply: %w", err)
}
Expand Down
72 changes: 55 additions & 17 deletions proxmox/nodes/containers/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,32 @@ import (
var errContainerAlreadyRunning = errors.New("container is already running")

// CloneContainer clones a container.
func (c *Client) CloneContainer(ctx context.Context, d *CloneRequestBody) error {
// The returned TaskResult carries any warnings from the task log.
func (c *Client) CloneContainer(ctx context.Context, d *CloneRequestBody) *tasks.TaskResult {
var taskResult *tasks.TaskResult

op := retry.NewTaskOperation("container clone",
retry.WithBaseDelay(10*time.Second),
retry.WithRetryIf(retry.IsTransientAPIError),
retry.WithAlreadyDoneCheck(retry.ErrorContains("already exists")),
)

return op.DoTask(ctx,
err := op.DoTask(ctx,
func() (*string, error) { return c.CloneContainerAsync(ctx, d) },
func(ctx context.Context, taskID string) error {
return c.Tasks().WaitForTask(ctx, taskID, tasks.WithIgnoreWarnings())
taskResult = c.Tasks().WaitForTask(ctx, taskID, tasks.WithIgnoreWarnings())
return taskResult.Err()
},
)
if err != nil {
return tasks.TaskFailed(err)
}

if taskResult != nil {
return taskResult
}

return tasks.TaskOK()
}

// CloneContainerAsync clones a container asynchronously.
Expand All @@ -55,18 +68,31 @@ func (c *Client) CloneContainerAsync(ctx context.Context, d *CloneRequestBody) (
}

// CreateContainer creates a container.
func (c *Client) CreateContainer(ctx context.Context, d *CreateRequestBody) error {
// The returned TaskResult carries any warnings from the task log.
func (c *Client) CreateContainer(ctx context.Context, d *CreateRequestBody) *tasks.TaskResult {
var taskResult *tasks.TaskResult

op := retry.NewTaskOperation("container create",
retry.WithRetryIf(retry.IsTransientAPIError),
retry.WithAlreadyDoneCheck(retry.ErrorContains("already exists")),
)

return op.DoTask(ctx,
err := op.DoTask(ctx,
func() (*string, error) { return c.CreateContainerAsync(ctx, d) },
func(ctx context.Context, taskID string) error {
return c.Tasks().WaitForTask(ctx, taskID, tasks.WithIgnoreWarnings())
taskResult = c.Tasks().WaitForTask(ctx, taskID, tasks.WithIgnoreWarnings())
return taskResult.Err()
},
)
if err != nil {
return tasks.TaskFailed(err)
}

if taskResult != nil {
return taskResult
}

return tasks.TaskOK()
}

// CreateContainerAsync creates a container asynchronously.
Expand Down Expand Up @@ -95,7 +121,7 @@ func (c *Client) DeleteContainer(ctx context.Context) error {

return op.DoTask(ctx,
func() (*string, error) { return c.DeleteContainerAsync(ctx) },
func(ctx context.Context, taskID string) error { return c.Tasks().WaitForTask(ctx, taskID) },
func(ctx context.Context, taskID string) error { return c.Tasks().WaitForTask(ctx, taskID).Err() },
)
}

Expand Down Expand Up @@ -284,7 +310,7 @@ func (c *Client) RebootContainer(ctx context.Context, d *RebootRequestBody) erro
return err
}

err = c.Tasks().WaitForTask(ctx, *taskID)
err = c.Tasks().WaitForTask(ctx, *taskID).Err()
if err != nil {
return fmt.Errorf("error waiting for container reboot: %w", err)
}
Expand Down Expand Up @@ -315,7 +341,7 @@ func (c *Client) ShutdownContainer(ctx context.Context, d *ShutdownRequestBody)
return err
}

err = c.Tasks().WaitForTask(ctx, *taskID)
err = c.Tasks().WaitForTask(ctx, *taskID).Err()
if err != nil {
return fmt.Errorf("error waiting for container shut down: %w", err)
}
Expand All @@ -340,34 +366,46 @@ func (c *Client) ShutdownContainerAsync(ctx context.Context, d *ShutdownRequestB
}

// StartContainer starts a container if is not already running.
func (c *Client) StartContainer(ctx context.Context) error {
// The returned TaskResult carries any warnings from the task log.
func (c *Client) StartContainer(ctx context.Context) *tasks.TaskResult {
status, err := c.GetContainerStatus(ctx)
if err != nil {
return fmt.Errorf("error retrieving container status: %w", err)
return tasks.TaskFailed(fmt.Errorf("error retrieving container status: %w", err))
}

if status.Status == "running" {
return nil
return tasks.TaskOK()
}

var taskResult *tasks.TaskResult

op := retry.NewTaskOperation("container start",
retry.WithRetryIf(retry.ErrorContains("got no worker upid")),
)

if err := op.DoTask(ctx,
func() (*string, error) { return c.StartContainerAsync(ctx) },
func(ctx context.Context, taskID string) error {
return c.Tasks().WaitForTask(ctx, taskID, tasks.WithIgnoreWarnings())
taskResult = c.Tasks().WaitForTask(ctx, taskID, tasks.WithIgnoreWarnings())
return taskResult.Err()
},
); err != nil {
if errors.Is(err, errContainerAlreadyRunning) {
return nil
return tasks.TaskOK()
}

return err
return tasks.TaskFailed(err)
}

if err := c.WaitForContainerStatus(ctx, "running"); err != nil {
return tasks.TaskFailed(err)
}

if taskResult != nil {
return taskResult
}

return c.WaitForContainerStatus(ctx, "running")
return tasks.TaskOK()
}

// StartContainerAsync starts a container asynchronously.
Expand Down Expand Up @@ -489,7 +527,7 @@ func (c *Client) ResizeContainerDisk(ctx context.Context, d *ResizeRequestBody)

return op.DoTask(ctx,
func() (*string, error) { return c.ResizeContainerDiskAsync(ctx, d) },
func(ctx context.Context, taskID string) error { return c.Tasks().WaitForTask(ctx, taskID) },
func(ctx context.Context, taskID string) error { return c.Tasks().WaitForTask(ctx, taskID).Err() },
)
}

Expand Down
40 changes: 20 additions & 20 deletions proxmox/nodes/containers/containers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ func TestCreateContainerSucceedsWithWarnings(t *testing.T) {

client := newTestClient(t, server.URL)

err := client.CreateContainer(t.Context(), &CreateRequestBody{})
require.NoError(t, err, "CreateContainer should succeed when task completes with warnings")
result := client.CreateContainer(t.Context(), &CreateRequestBody{})
require.NoError(t, result.Err(), "CreateContainer should succeed when task completes with warnings")
}

// TestCloneContainerSucceedsWithWarnings verifies that CloneContainer succeeds
Expand All @@ -157,8 +157,8 @@ func TestCloneContainerSucceedsWithWarnings(t *testing.T) {

client := newTestClient(t, server.URL)

err := client.CloneContainer(t.Context(), &CloneRequestBody{})
require.NoError(t, err, "CloneContainer should succeed when task completes with warnings")
result := client.CloneContainer(t.Context(), &CloneRequestBody{})
require.NoError(t, result.Err(), "CloneContainer should succeed when task completes with warnings")
}

func TestDeleteContainerWaitsForTask(t *testing.T) {
Expand Down Expand Up @@ -242,8 +242,8 @@ func TestCreateContainerRetries(t *testing.T) {

client := newTestClient(t, server.URL)

err := client.CreateContainer(t.Context(), &CreateRequestBody{})
require.NoError(t, err)
result := client.CreateContainer(t.Context(), &CreateRequestBody{})
require.NoError(t, result.Err())

assert.Equal(t, 2, captures.countPOST("/lxc"),
"expected exactly 2 POST calls (1 failure + 1 success), proving retry occurred")
Expand Down Expand Up @@ -272,8 +272,8 @@ func TestCreateContainerNoRetryOn400(t *testing.T) {

client := newTestClient(t, server.URL)

err := client.CreateContainer(t.Context(), &CreateRequestBody{})
require.Error(t, err)
result := client.CreateContainer(t.Context(), &CreateRequestBody{})
require.Error(t, result.Err())

assert.Equal(t, 1, captures.countPOST("/lxc"),
"expected exactly 1 POST call (no retry on 400)")
Expand Down Expand Up @@ -311,8 +311,8 @@ func TestCloneContainerRetries(t *testing.T) {

client := newTestClient(t, server.URL)

err := client.CloneContainer(t.Context(), &CloneRequestBody{})
require.NoError(t, err)
result := client.CloneContainer(t.Context(), &CloneRequestBody{})
require.NoError(t, result.Err())

assert.Equal(t, 2, captures.countPOST("/clone"),
"expected exactly 2 POST calls (1 failure + 1 success), proving retry occurred")
Expand Down Expand Up @@ -376,8 +376,8 @@ func TestStartContainerAlreadyRunningOnFirstAttempt(t *testing.T) {

client := newTestClient(t, server.URL)

err := client.StartContainer(t.Context())
require.NoError(t, err, "StartContainer should succeed when API returns 'already running'")
result := client.StartContainer(t.Context())
require.NoError(t, result.Err(), "StartContainer should succeed when API returns 'already running'")

assert.Equal(t, 1, captures.countPOST("/status/start"),
"expected exactly 1 start attempt")
Expand All @@ -404,8 +404,8 @@ func TestStartContainerAlreadyRunningDetectedByPreCheck(t *testing.T) {

client := newTestClient(t, server.URL)

err := client.StartContainer(t.Context())
require.NoError(t, err)
result := client.StartContainer(t.Context())
require.NoError(t, result.Err())

assert.Equal(t, 0, captures.countPOST("/status/start"),
"expected no start calls when container is already running")
Expand Down Expand Up @@ -460,8 +460,8 @@ func TestStartContainerRetriesOnNoWorkerUpid(t *testing.T) {

client := newTestClient(t, server.URL)

err := client.StartContainer(t.Context())
require.NoError(t, err)
result := client.StartContainer(t.Context())
require.NoError(t, result.Err())

assert.Equal(t, 2, captures.countPOST("/status/start"),
"expected exactly 2 start calls (1 failure + 1 success), proving retry occurred")
Expand Down Expand Up @@ -509,8 +509,8 @@ func TestStartContainerAlreadyRunningOnRetry(t *testing.T) {

client := newTestClient(t, server.URL)

err := client.StartContainer(t.Context())
require.NoError(t, err, "should succeed when retry finds container already running")
result := client.StartContainer(t.Context())
require.NoError(t, result.Err(), "should succeed when retry finds container already running")

assert.Equal(t, 2, captures.countPOST("/status/start"),
"expected exactly 2 start calls (1 transient failure + 1 already running)")
Expand Down Expand Up @@ -538,8 +538,8 @@ func TestCloneContainerNoRetryOn400(t *testing.T) {

client := newTestClient(t, server.URL)

err := client.CloneContainer(t.Context(), &CloneRequestBody{})
require.Error(t, err)
result := client.CloneContainer(t.Context(), &CloneRequestBody{})
require.Error(t, result.Err())

assert.Equal(t, 1, captures.countPOST("/clone"),
"expected exactly 1 POST call (no retry on 400)")
Expand Down
Loading
Loading