Skip to content

Commit 2fc05c6

Browse files
committed
Remove unused base parameter from eviction updateStatus and handleNotFound
After the updateStatus refactor to re-fetch resources on retry, the base parameter is no longer used. Remove it along with the now-unnecessary DeepCopy calls at each call site. Also remove unused gosec nolint directive in utils.go.
1 parent 179d588 commit 2fc05c6

2 files changed

Lines changed: 17 additions & 24 deletions

File tree

internal/controller/eviction_controller.go

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -118,15 +118,14 @@ func (r *EvictionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
118118

119119
statusCondition := meta.FindStatusCondition(eviction.Status.Conditions, kvmv1.ConditionTypeEvicting)
120120
if statusCondition == nil {
121-
base := eviction.DeepCopy()
122121
meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{
123122
Type: kvmv1.ConditionTypeEvicting,
124123
Status: metav1.ConditionTrue,
125124
Message: "Running",
126125
Reason: kvmv1.ConditionReasonRunning,
127126
})
128127

129-
return ctrl.Result{}, r.updateStatus(ctx, eviction, base)
128+
return ctrl.Result{}, r.updateStatus(ctx, eviction)
130129
}
131130

132131
switch statusCondition.Status {
@@ -158,7 +157,6 @@ func (r *EvictionReconciler) handleRunning(ctx context.Context, eviction *kvmv1.
158157
return r.evictNext(ctx, eviction)
159158
}
160159

161-
base := eviction.DeepCopy()
162160
meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{
163161
Type: kvmv1.ConditionTypeEvicting,
164162
Status: metav1.ConditionFalse,
@@ -168,10 +166,10 @@ func (r *EvictionReconciler) handleRunning(ctx context.Context, eviction *kvmv1.
168166

169167
eviction.Status.OutstandingRamMb = 0
170168
logger.FromContext(ctx).Info("succeeded")
171-
return ctrl.Result{}, r.updateStatus(ctx, eviction, base)
169+
return ctrl.Result{}, r.updateStatus(ctx, eviction)
172170
}
173171

174-
func (r *EvictionReconciler) updateStatus(ctx context.Context, eviction, base *kvmv1.Eviction) error {
172+
func (r *EvictionReconciler) updateStatus(ctx context.Context, eviction *kvmv1.Eviction) error {
175173
// Capture the desired status to re-apply on each retry attempt
176174
desiredStatus := eviction.Status.DeepCopy()
177175
return retry.RetryOnConflict(utils.StatusPatchBackoff, func() error {
@@ -188,7 +186,6 @@ func (r *EvictionReconciler) updateStatus(ctx context.Context, eviction, base *k
188186
}
189187

190188
func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv1.Eviction, hv *kvmv1.Hypervisor) (ctrl.Result, error) {
191-
base := eviction.DeepCopy()
192189
expectHypervisor := hv.Status.HypervisorID != "" && hv.Status.ServiceID != "" // The hypervisor has been registered
193190

194191
// If the hypervisor should exist, then we need to ensure it is disabled before we start evicting
@@ -200,7 +197,7 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv
200197
Message: "hypervisor not disabled",
201198
Reason: kvmv1.ConditionReasonFailed,
202199
}) {
203-
return ctrl.Result{}, r.updateStatus(ctx, eviction, base)
200+
return ctrl.Result{}, r.updateStatus(ctx, eviction)
204201
}
205202
return ctrl.Result{RequeueAfter: defaultPollTime}, nil // Wait for hypervisor to be disabled
206203
}
@@ -221,7 +218,7 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv
221218
Message: fmt.Sprintf("failed to get hypervisor %v", err),
222219
Reason: kvmv1.ConditionReasonFailed,
223220
})
224-
return ctrl.Result{}, r.updateStatus(ctx, eviction, base)
221+
return ctrl.Result{}, r.updateStatus(ctx, eviction)
225222
} else {
226223
// That is (likely) an eviction for a node that never registered
227224
// so we are good to go
@@ -234,7 +231,7 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv
234231
})
235232
eviction.Status.OutstandingRamMb = 0
236233
logger.FromContext(ctx).Info(msg)
237-
return ctrl.Result{}, r.updateStatus(ctx, eviction, base)
234+
return ctrl.Result{}, r.updateStatus(ctx, eviction)
238235
}
239236
}
240237

@@ -255,18 +252,15 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv
255252
Message: "Preflight checks passed, hypervisor is disabled and ready for eviction",
256253
Reason: kvmv1.ConditionReasonSucceeded,
257254
})
258-
return ctrl.Result{}, r.updateStatus(ctx, eviction, base)
255+
return ctrl.Result{}, r.updateStatus(ctx, eviction)
259256
}
260257

261258
// Tries to handle the NotFound-error by updating the status
262-
func (r *EvictionReconciler) handleNotFound(ctx context.Context, eviction, base *kvmv1.Eviction, err error) error {
259+
func (r *EvictionReconciler) handleNotFound(ctx context.Context, eviction *kvmv1.Eviction, err error) error {
263260
if !gophercloud.ResponseCodeIs(err, http.StatusNotFound) {
264261
return err
265262
}
266263
logger.FromContext(ctx).Info("Instance is gone")
267-
if base == nil {
268-
base = eviction.DeepCopy()
269-
}
270264
var uuid string
271265
eviction.Status.OutstandingInstances, uuid = popInstance(eviction.Status.OutstandingInstances)
272266
if uuid == "" {
@@ -278,11 +272,10 @@ func (r *EvictionReconciler) handleNotFound(ctx context.Context, eviction, base
278272
Message: fmt.Sprintf("Instance %s is gone", uuid),
279273
Reason: kvmv1.ConditionReasonSucceeded,
280274
})
281-
return r.updateStatus(ctx, eviction, base)
275+
return r.updateStatus(ctx, eviction)
282276
}
283277

284278
func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Eviction) (ctrl.Result, error) {
285-
base := eviction.DeepCopy()
286279
uuid := peekInstance(eviction.Status.OutstandingInstances)
287280
if uuid == "" {
288281
return ctrl.Result{}, nil
@@ -294,7 +287,7 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic
294287
vm, err := res.Extract()
295288

296289
if err != nil {
297-
if err2 := r.handleNotFound(ctx, eviction, base, err); err2 != nil {
290+
if err2 := r.handleNotFound(ctx, eviction, err); err2 != nil {
298291
return ctrl.Result{}, err2
299292
} else {
300293
return ctrl.Result{RequeueAfter: shortRetryTime}, nil
@@ -322,7 +315,7 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic
322315
})
323316

324317
return ctrl.Result{}, errors.Join(fmt.Errorf("error migrating instance %v", uuid),
325-
r.updateStatus(ctx, eviction, base))
318+
r.updateStatus(ctx, eviction))
326319
}
327320

328321
currentHypervisor, _, _ := strings.Cut(vm.HypervisorHostname, ".")
@@ -339,7 +332,7 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic
339332
// So, it is already off this one, do we need to verify it?
340333
if vm.Status == "VERIFY_RESIZE" {
341334
err := servers.ConfirmResize(ctx, r.computeClient, vm.ID).ExtractErr()
342-
if err2 := r.handleNotFound(ctx, eviction, base, err); err2 != nil {
335+
if err2 := r.handleNotFound(ctx, eviction, err); err2 != nil {
343336
// Retry confirm in next reconciliation
344337
return ctrl.Result{}, err2
345338
} else {
@@ -350,7 +343,7 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic
350343

351344
// All done
352345
eviction.Status.OutstandingInstances, _ = popInstance(eviction.Status.OutstandingInstances)
353-
return ctrl.Result{}, r.updateStatus(ctx, eviction, base)
346+
return ctrl.Result{}, r.updateStatus(ctx, eviction)
354347
}
355348

356349
if vm.TaskState == "deleting" { //nolint:gocritic
@@ -363,22 +356,22 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic
363356
Message: fmt.Sprintf("Live migration of terminating instance %s skipped", vm.ID),
364357
Reason: kvmv1.ConditionReasonFailed,
365358
})
366-
if err := r.updateStatus(ctx, eviction, base); err != nil {
359+
if err := r.updateStatus(ctx, eviction); err != nil {
367360
return ctrl.Result{}, fmt.Errorf("could not update status due to %w", err)
368361
}
369362
return ctrl.Result{RequeueAfter: defaultPollTime}, nil
370363
} else if vm.Status == "ACTIVE" || vm.PowerState == 1 {
371364
log.Info("trigger live-migration")
372365
if err := r.liveMigrate(ctx, vm.ID, eviction); err != nil {
373-
if err2 := r.handleNotFound(ctx, eviction, base, err); err2 != nil {
366+
if err2 := r.handleNotFound(ctx, eviction, err); err2 != nil {
374367
return ctrl.Result{}, err2
375368
}
376369
return ctrl.Result{RequeueAfter: shortRetryTime}, nil
377370
}
378371
} else {
379372
log.Info("trigger cold-migration")
380373
if err := r.coldMigrate(ctx, vm.ID, eviction); err != nil {
381-
if err2 := r.handleNotFound(ctx, eviction, base, err); err2 != nil {
374+
if err2 := r.handleNotFound(ctx, eviction, err); err2 != nil {
382375
return ctrl.Result{}, err2
383376
}
384377
return ctrl.Result{RequeueAfter: shortRetryTime}, nil

internal/controller/utils.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func updateInstanceHA(hypervisor *kvmv1.Hypervisor, data string, acceptedCodes [
6363
url := InstanceHaUrl(region, zone, hostname)
6464
client := &http.Client{Timeout: 30 * time.Second}
6565
// G107: Potential HTTP request made with variable url
66-
resp, err := client.Post(url, "application/json", bytes.NewBuffer([]byte(data))) //nolint:gosec,bodyclose
66+
resp, err := client.Post(url, "application/json", bytes.NewBuffer([]byte(data))) //nolint:bodyclose
6767
if err != nil {
6868
return fmt.Errorf("failed to send request to ha service due to %w", err)
6969
}

0 commit comments

Comments
 (0)