Skip to content

Do not fail eviction on single instance migration failure, and some typo changes #133

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

Merged
fwiesel merged 2 commits into from
Oct 7, 2025
Merged

Do not fail eviction on single instance migration failure, and some typo changes #133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1aeb5d5
Fix spelling and match name of variable to content
fwiesel Oct 7, 2025
4ec8a12
Do not fail eviction on failed migration
fwiesel Oct 7, 2025
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
8 changes: 4 additions & 4 deletions api/v1/eviction_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,17 @@ const (
// ConditionTypeHypervisorDisabled is the type of condition for hypervisor disabled status
ConditionTypeHypervisorDisabled = "HypervisorDisabled"

// ConditionTypeEviction is the type of condition for eviction status
ConditionTypeEviction = "Evicting"
// ConditionTypeEvicting is the type of condition for eviction status
ConditionTypeEvicting = "Evicting"

// ConditionReasonRunning means the eviction is currently running
ConditionReasonRunning string = "Running"

// ConditionReasonFailed means the eviction has failed
ConditionReasonFailed string = "Failed"

// ConditionReasonSuceeded means the eviction has succeeded
ConditionReasonSuceeded string = "Succeeded"
// ConditionReasonSucceeded means the eviction has succeeded
ConditionReasonSucceeded string = "Succeeded"
)

// EvictionStatus defines the observed state of Eviction
Expand Down
48 changes: 29 additions & 19 deletions internal/controller/eviction_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func (r *EvictionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
return ctrl.Result{}, err
}

statusCondition := meta.FindStatusCondition(eviction.Status.Conditions, kvmv1.ConditionTypeEviction)
statusCondition := meta.FindStatusCondition(eviction.Status.Conditions, kvmv1.ConditionTypeEvicting)
if statusCondition == nil {
// No status condition, so we need to add it
if r.addCondition(ctx, eviction, metav1.ConditionTrue, "Running", kvmv1.ConditionReasonRunning) {
Expand Down Expand Up @@ -117,10 +117,10 @@ func (r *EvictionReconciler) handleRunning(ctx context.Context, eviction *kvmv1.
}

meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{
Type: kvmv1.ConditionTypeEviction,
Type: kvmv1.ConditionTypeEvicting,
Status: metav1.ConditionFalse,
Message: "eviction completed successfully",
Reason: kvmv1.ConditionReasonSuceeded,
Reason: kvmv1.ConditionReasonSucceeded,
})

eviction.Status.OutstandingRamMb = 0
Expand Down Expand Up @@ -148,7 +148,7 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv
// Abort eviction
err = fmt.Errorf("failed to get hypervisor %w", err)
meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{
Type: kvmv1.ConditionTypeEviction,
Type: kvmv1.ConditionTypeEvicting,
Status: metav1.ConditionFalse,
Message: err.Error(),
Reason: kvmv1.ConditionReasonFailed,
Expand All @@ -159,10 +159,10 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv
// so we are good to go
msg := "eviction completed successfully due to expected case of no hypervisor"
meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{
Type: kvmv1.ConditionTypeEviction,
Type: kvmv1.ConditionTypeEvicting,
Status: metav1.ConditionFalse,
Message: msg,
Reason: kvmv1.ConditionReasonSuceeded,
Reason: kvmv1.ConditionReasonSucceeded,
})
eviction.Status.OutstandingRamMb = 0
logger.FromContext(ctx).Info(msg)
Expand All @@ -176,7 +176,7 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv
err = fmt.Errorf("hypervisor name %q does not match spec %q", currentHypervisor, hypervisorName)
log.Error(err, "Hypervisor name mismatch")
meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{
Type: kvmv1.ConditionTypeEviction,
Type: kvmv1.ConditionTypeEvicting,
Status: metav1.ConditionFalse,
Message: err.Error(),
Reason: kvmv1.ConditionReasonFailed,
Expand Down Expand Up @@ -210,7 +210,7 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv
Type: kvmv1.ConditionTypePreflight,
Status: metav1.ConditionTrue,
Message: "Preflight checks passed, hypervisor is disabled and ready for eviction",
Reason: kvmv1.ConditionReasonSuceeded,
Reason: kvmv1.ConditionReasonSucceeded,
})
return ctrl.Result{}, r.Status().Update(ctx, eviction)
}
Expand All @@ -231,7 +231,7 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic
Type: kvmv1.ConditionTypeMigration,
Status: metav1.ConditionFalse,
Message: fmt.Sprintf("Instance %s is gone", uuid),
Reason: kvmv1.ConditionReasonSuceeded,
Reason: kvmv1.ConditionReasonSucceeded,
})
return ctrl.Result{}, r.Status().Update(ctx, eviction)
}
Expand Down Expand Up @@ -272,7 +272,7 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic
Type: kvmv1.ConditionTypeMigration,
Status: metav1.ConditionFalse,
Message: fmt.Sprintf("Migration of instance %s finished", vm.ID),
Reason: kvmv1.ConditionReasonSuceeded,
Reason: kvmv1.ConditionReasonSucceeded,
})

// So, it is already off this one, do we need to verify it?
Expand Down Expand Up @@ -381,7 +381,7 @@ func (r *EvictionReconciler) enableHypervisorService(ctx context.Context, evicti
Type: kvmv1.ConditionTypeHypervisorReEnabled,
Status: metav1.ConditionTrue,
Message: "Hypervisor is gone, no need to re-enable",
Reason: kvmv1.ConditionReasonSuceeded,
Reason: kvmv1.ConditionReasonSucceeded,
})
} else {
// update the condition to reflect the error, but do not fail the reconciliation
Expand All @@ -401,7 +401,7 @@ func (r *EvictionReconciler) enableHypervisorService(ctx context.Context, evicti
Status: metav1.ConditionTrue,
Message: fmt.Sprintf("Hypervisor already re-enabled for reason: %s",
hypervisor.Service.DisabledReason),
Reason: kvmv1.ConditionReasonSuceeded,
Reason: kvmv1.ConditionReasonSucceeded,
})
return r.Status().Update(ctx, eviction)
}
Expand All @@ -422,7 +422,7 @@ func (r *EvictionReconciler) enableHypervisorService(ctx context.Context, evicti
Type: kvmv1.ConditionTypeHypervisorReEnabled,
Status: metav1.ConditionTrue,
Message: "Hypervisor re-enabled successfully",
Reason: kvmv1.ConditionReasonSuceeded,
Reason: kvmv1.ConditionReasonSucceeded,
})
}
return r.Status().Update(ctx, eviction)
Expand All @@ -440,7 +440,7 @@ func (r *EvictionReconciler) disableHypervisor(ctx context.Context, hypervisor *
Type: kvmv1.ConditionTypeHypervisorDisabled,
Status: metav1.ConditionTrue,
Message: fmt.Sprintf("Hypervisor already disabled for reason %q", disabledReason),
Reason: kvmv1.ConditionReasonSuceeded,
Reason: kvmv1.ConditionReasonSucceeded,
})
return r.Status().Update(ctx, eviction)
}
Expand Down Expand Up @@ -470,7 +470,7 @@ func (r *EvictionReconciler) disableHypervisor(ctx context.Context, hypervisor *
Type: kvmv1.ConditionTypeHypervisorDisabled,
Status: metav1.ConditionTrue,
Message: "Hypervisor disabled successfully",
Reason: kvmv1.ConditionReasonSuceeded,
Reason: kvmv1.ConditionReasonSucceeded,
})
return r.Status().Update(ctx, eviction)
}
Expand All @@ -485,8 +485,13 @@ func (r *EvictionReconciler) liveMigrate(ctx context.Context, uuid string, evict
res := servers.LiveMigrate(ctx, r.computeClient, uuid, liveMigrateOpts)
if res.Err != nil {
err := fmt.Errorf("failed to evict VM %s due to %w", uuid, res.Err)
r.addCondition(ctx, eviction, metav1.ConditionFalse, err.Error(), kvmv1.ConditionReasonFailed)
return res.Err
meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{
Type: kvmv1.ConditionTypeMigration,
Status: metav1.ConditionFalse,
Message: err.Error(),
Reason: kvmv1.ConditionReasonFailed,
})
return err
}

log.Info("Live migrating server", "server", uuid, "source", eviction.Spec.Hypervisor, "X-Openstack-Request-Id", res.Header["X-Openstack-Request-Id"][0])
Expand All @@ -499,7 +504,12 @@ func (r *EvictionReconciler) coldMigrate(ctx context.Context, uuid string, evict
res := servers.Migrate(ctx, r.computeClient, uuid)
if res.Err != nil {
err := fmt.Errorf("failed to evict stopped server %s due to %w", uuid, res.Err)
r.addCondition(ctx, eviction, metav1.ConditionFalse, err.Error(), kvmv1.ConditionReasonFailed)
meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{
Type: kvmv1.ConditionTypeMigration,
Status: metav1.ConditionFalse,
Message: err.Error(),
Reason: kvmv1.ConditionReasonFailed,
})
return err
}

Expand All @@ -511,7 +521,7 @@ func (r *EvictionReconciler) coldMigrate(ctx context.Context, uuid string, evict
func (r *EvictionReconciler) addCondition(ctx context.Context, eviction *kvmv1.Eviction,
status metav1.ConditionStatus, message string, reason string) bool {
if !meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{
Type: kvmv1.ConditionTypeEviction,
Type: kvmv1.ConditionTypeEvicting,
Status: status,
Message: message,
Reason: reason,
Expand Down
18 changes: 9 additions & 9 deletions internal/controller/eviction_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ var _ = Describe("Eviction Controller", func() {
Expect(err).NotTo(HaveOccurred())

// expect eviction condition to be false due to missing hypervisor
reconcileStatus := meta.FindStatusCondition(resource.Status.Conditions, kvmv1.ConditionTypeEviction)
reconcileStatus := meta.FindStatusCondition(resource.Status.Conditions, kvmv1.ConditionTypeEvicting)
Expect(reconcileStatus).NotTo(BeNil())
Expect(reconcileStatus.Status).To(Equal(metav1.ConditionFalse))
Expect(reconcileStatus.Reason).To(Equal("Failed"))
Expand Down Expand Up @@ -269,7 +269,7 @@ var _ = Describe("Eviction Controller", func() {
})
It("should succeed the reconciliation", func() {
runningCond := &metav1.Condition{
Type: kvmv1.ConditionTypeEviction,
Type: kvmv1.ConditionTypeEvicting,
Status: metav1.ConditionTrue,
Reason: kvmv1.ConditionReasonRunning,
Message: "Running",
Expand All @@ -278,14 +278,14 @@ var _ = Describe("Eviction Controller", func() {
hypervisorDisabledCond := &metav1.Condition{
Type: kvmv1.ConditionTypeHypervisorDisabled,
Status: metav1.ConditionTrue,
Reason: kvmv1.ConditionReasonSuceeded,
Reason: kvmv1.ConditionReasonSucceeded,
Message: "Hypervisor disabled successfully",
}

preflightCond := &metav1.Condition{
Type: kvmv1.ConditionTypePreflight,
Status: metav1.ConditionTrue,
Reason: kvmv1.ConditionReasonSuceeded,
Reason: kvmv1.ConditionReasonSucceeded,
Message: "Preflight checks passed",
}

Expand Down Expand Up @@ -314,9 +314,9 @@ var _ = Describe("Eviction Controller", func() {
// 5. expect the eviction condition to be set to succeeded
{
conditions: []*metav1.Condition{{
Type: kvmv1.ConditionTypeEviction,
Type: kvmv1.ConditionTypeEvicting,
Status: metav1.ConditionFalse,
Reason: kvmv1.ConditionReasonSuceeded,
Reason: kvmv1.ConditionReasonSucceeded,
Message: "eviction completed successfully"}},
finalizers: []string{evictionFinalizerName}},
}
Expand Down Expand Up @@ -374,7 +374,7 @@ var _ = Describe("Eviction Controller", func() {
Expect(err).NotTo(HaveOccurred())

// expect eviction condition to be true
reconcileStatus := meta.FindStatusCondition(resource.Status.Conditions, kvmv1.ConditionTypeEviction)
reconcileStatus := meta.FindStatusCondition(resource.Status.Conditions, kvmv1.ConditionTypeEvicting)
Expect(reconcileStatus).NotTo(BeNil())
Expect(reconcileStatus.Status).To(Equal(metav1.ConditionTrue))
Expect(reconcileStatus.Reason).To(Equal(kvmv1.ConditionReasonRunning))
Expand All @@ -389,10 +389,10 @@ var _ = Describe("Eviction Controller", func() {
Expect(err).NotTo(HaveOccurred())

// expect reconciliation to be successfully finished
reconcileStatus = meta.FindStatusCondition(resource.Status.Conditions, kvmv1.ConditionTypeEviction)
reconcileStatus = meta.FindStatusCondition(resource.Status.Conditions, kvmv1.ConditionTypeEvicting)
Expect(reconcileStatus).NotTo(BeNil())
Expect(reconcileStatus.Status).To(Equal(metav1.ConditionFalse))
Expect(reconcileStatus.Reason).To(Equal(kvmv1.ConditionReasonSuceeded))
Expect(reconcileStatus.Reason).To(Equal(kvmv1.ConditionReasonSucceeded))

Expect(resource.GetFinalizers()).To(BeEmpty())
})
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/node_eviction_label_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func (r *NodeEvictionLabelReconciler) reconcileEviction(ctx context.Context, evi

// check if the eviction is already succeeded
var evictionState string
if status := meta.FindStatusCondition(eviction.Status.Conditions, kvmv1.ConditionTypeEviction); status != nil {
if status := meta.FindStatusCondition(eviction.Status.Conditions, kvmv1.ConditionTypeEvicting); status != nil {
evictionState = status.Reason
}
switch evictionState {
Expand Down
Loading