Skip to content

Commit 1ad3321

Browse files
authored
chore(vmop): improve error logging (#1928)
- Wrap handler errors for better traceability. - Move TypeSignalSent condition initialization before phase setting for improved logical flow. Signed-off-by: Roman Sysoev <roman.sysoev@flant.com>
1 parent 40261e2 commit 1ad3321

File tree

2 files changed

+14
-10
lines changed

2 files changed

+14
-10
lines changed

images/virtualization-artifact/pkg/controller/vmop/migration/internal/handler/deletion.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package handler
1818

1919
import (
2020
"context"
21+
"fmt"
2122

2223
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
2324
"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -57,13 +58,13 @@ func (h DeletionHandler) Handle(ctx context.Context, vmop *v1alpha2.VirtualMachi
5758
// Delete migration if exists.
5859
err := h.migration.DeleteMigration(ctx, vmop)
5960
if err != nil {
60-
return reconcile.Result{}, err
61+
return reconcile.Result{}, fmt.Errorf("failed to delete migration: %w", err)
6162
}
6263

6364
// Check if migration removed.
6465
mig, err := h.migration.GetMigration(ctx, vmop)
6566
if err != nil {
66-
return reconcile.Result{}, err
67+
return reconcile.Result{}, fmt.Errorf("failed to get migration after deletion': %w", err)
6768
}
6869

6970
if mig != nil {

images/virtualization-artifact/pkg/controller/vmop/migration/internal/handler/lifecycle.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,6 @@ func (h LifecycleHandler) Handle(ctx context.Context, vmop *v1alpha2.VirtualMach
7777
}
7878

7979
// 1.Initialize new VMOP resource: set phase to Pending and all conditions to Unknown.
80-
h.base.Init(vmop)
81-
8280
if vmop.Status.Phase == "" {
8381
conditions.SetCondition(
8482
conditions.NewConditionBuilder(vmopcondition.TypeSignalSent).
@@ -90,12 +88,14 @@ func (h LifecycleHandler) Handle(ctx context.Context, vmop *v1alpha2.VirtualMach
9088
)
9189
}
9290

91+
h.base.Init(vmop)
92+
9393
completedCond := conditions.NewConditionBuilder(vmopcondition.TypeCompleted).Generation(vmop.GetGeneration())
9494

9595
// Pending if quota exceeded.
9696
isQuotaExceededDuringMigration, err := h.isKubeVirtMigrationRejectedDueToQuota(ctx, vmop)
9797
if err != nil {
98-
return reconcile.Result{}, err
98+
return reconcile.Result{}, fmt.Errorf("failed to check if migration was rejected due to quota for VMOP: %w", err)
9999
}
100100
if isQuotaExceededDuringMigration {
101101
h.recorder.Event(vmop, corev1.EventTypeWarning, v1alpha2.ReasonErrVMOPPending, "Project quota exceeded")
@@ -111,7 +111,7 @@ func (h LifecycleHandler) Handle(ctx context.Context, vmop *v1alpha2.VirtualMach
111111
// 2. Get VirtualMachine for validation vmop.
112112
vm, err := h.base.FetchVirtualMachineOrSetFailedPhase(ctx, vmop)
113113
if vm == nil || err != nil {
114-
return reconcile.Result{}, err
114+
return reconcile.Result{}, fmt.Errorf("failed to fetch VirtualMachine for VMOP: %w", err)
115115
}
116116

117117
log, ctx := logger.GetHandlerContext(ctx, lifecycleHandlerName)
@@ -127,7 +127,7 @@ func (h LifecycleHandler) Handle(ctx context.Context, vmop *v1alpha2.VirtualMach
127127
// Do it now.
128128
mig, err := h.migration.GetMigration(ctx, vmop)
129129
if err != nil {
130-
return reconcile.Result{}, err
130+
return reconcile.Result{}, fmt.Errorf("failed to get migration for VMOP: %w", err)
131131
}
132132
if mig != nil {
133133
conditions.SetCondition(
@@ -143,7 +143,7 @@ func (h LifecycleHandler) Handle(ctx context.Context, vmop *v1alpha2.VirtualMach
143143
// All operations must be performed in course, check it and set phase if operation cannot be executed now.
144144
should, err := h.base.ShouldExecuteOrSetFailedPhase(ctx, vmop)
145145
if err != nil {
146-
return reconcile.Result{}, err
146+
return reconcile.Result{}, fmt.Errorf("failed to determine if VMOP should execute: %w", err)
147147
}
148148
if !should {
149149
return reconcile.Result{}, nil
@@ -174,7 +174,7 @@ func (h LifecycleHandler) Handle(ctx context.Context, vmop *v1alpha2.VirtualMach
174174
// 6.2 Fail if there is at least one other migration in progress.
175175
found, err := h.otherMigrationsAreInProgress(ctx, vmop)
176176
if err != nil {
177-
return reconcile.Result{}, err
177+
return reconcile.Result{}, fmt.Errorf("failed to check other migrations in progress for VMOP: %w", err)
178178
}
179179
if found {
180180
vmop.Status.Phase = v1alpha2.VMOPPhaseFailed
@@ -194,8 +194,11 @@ func (h LifecycleHandler) Handle(ctx context.Context, vmop *v1alpha2.VirtualMach
194194
}
195195
// 7.1 The Operation is valid, and can be executed.
196196
err = h.execute(ctx, vmop, vm)
197+
if err != nil {
198+
return reconcile.Result{}, fmt.Errorf("failed to execute VMOP: %w", err)
199+
}
197200

198-
return reconcile.Result{}, err
201+
return reconcile.Result{}, nil
199202
}
200203

201204
func (h LifecycleHandler) Name() string {

0 commit comments

Comments
 (0)