Skip to content

Commit 8261698

Browse files
afritzlerafritzler
authored
Refactor usage of GetBMCClientForServer (#354)
* Refactor usage of `GetBMCClientForServer` Reduce the usage of the BMC client creation + subsequent logout calls. * Refactor requeue handling in BIOS version controller
1 parent c4ed0b9 commit 8261698

File tree

6 files changed

+162
-223
lines changed

6 files changed

+162
-223
lines changed

‎config/crd/bases/metal.ironcore.dev_biosversions.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
33
kind: CustomResourceDefinition
44
metadata:
55
annotations:
6-
controller-gen.kubebuilder.io/version: v0.17.1
6+
controller-gen.kubebuilder.io/version: v0.18.0
77
name: biosversions.metal.ironcore.dev
88
spec:
99
group: metal.ironcore.dev

‎dist/chart/templates/crd/metal.ironcore.dev_biosversions.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ metadata:
99
{{- if .Values.crd.keep }}
1010
"helm.sh/resource-policy": keep
1111
{{- end }}
12-
controller-gen.kubebuilder.io/version: v0.17.1
12+
controller-gen.kubebuilder.io/version: v0.18.0
1313
name: biosversions.metal.ironcore.dev
1414
spec:
1515
group: metal.ironcore.dev

‎docs/api-reference/api.md‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ BIOSSettingUpdateState
327327
<td>
328328
<code>metadata</code><br/>
329329
<em>
330-
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.32/#objectmeta-v1-meta">
330+
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.33/#objectmeta-v1-meta">
331331
Kubernetes meta/v1.ObjectMeta
332332
</a>
333333
</em>
@@ -391,7 +391,7 @@ ImageSpec
391391
<td>
392392
<code>serverRef</code><br/>
393393
<em>
394-
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.32/#localobjectreference-v1-core">
394+
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.33/#localobjectreference-v1-core">
395395
Kubernetes core/v1.LocalObjectReference
396396
</a>
397397
</em>
@@ -417,7 +417,7 @@ ServerMaintenancePolicy
417417
<td>
418418
<code>serverMaintenanceRef</code><br/>
419419
<em>
420-
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.32/#objectreference-v1-core">
420+
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.33/#objectreference-v1-core">
421421
Kubernetes core/v1.ObjectReference
422422
</a>
423423
</em>
@@ -500,7 +500,7 @@ ImageSpec
500500
<td>
501501
<code>serverRef</code><br/>
502502
<em>
503-
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.32/#localobjectreference-v1-core">
503+
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.33/#localobjectreference-v1-core">
504504
Kubernetes core/v1.LocalObjectReference
505505
</a>
506506
</em>
@@ -526,7 +526,7 @@ ServerMaintenancePolicy
526526
<td>
527527
<code>serverMaintenanceRef</code><br/>
528528
<em>
529-
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.32/#objectreference-v1-core">
529+
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.33/#objectreference-v1-core">
530530
Kubernetes core/v1.ObjectReference
531531
</a>
532532
</em>
@@ -611,7 +611,7 @@ TaskStatus
611611
<td>
612612
<code>conditions</code><br/>
613613
<em>
614-
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.32/#condition-v1-meta">
614+
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.33/#condition-v1-meta">
615615
[]Kubernetes meta/v1.Condition
616616
</a>
617617
</em>
@@ -1517,7 +1517,7 @@ net/netip.Prefix
15171517
<td>
15181518
<code>secretRef</code><br/>
15191519
<em>
1520-
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.32/#localobjectreference-v1-core">
1520+
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.33/#localobjectreference-v1-core">
15211521
Kubernetes core/v1.LocalObjectReference
15221522
</a>
15231523
</em>

‎internal/controller/biossettings_controller.go‎

Lines changed: 28 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -177,11 +177,7 @@ func (r *BiosSettingsReconciler) cleanupReferences(
177177
return err
178178
}
179179

180-
func (r *BiosSettingsReconciler) reconcile(
181-
ctx context.Context,
182-
log logr.Logger,
183-
biosSettings *metalv1alpha1.BIOSSettings,
184-
) (ctrl.Result, error) {
180+
func (r *BiosSettingsReconciler) reconcile(ctx context.Context, log logr.Logger, biosSettings *metalv1alpha1.BIOSSettings) (ctrl.Result, error) {
185181
if shouldIgnoreReconciliation(biosSettings) {
186182
log.V(1).Info("Skipped BIOS Setting reconciliation")
187183
return ctrl.Result{}, nil
@@ -224,18 +220,25 @@ func (r *BiosSettingsReconciler) reconcile(
224220
return ctrl.Result{}, err
225221
}
226222

227-
return r.ensureBIOSSettingsStateTransition(ctx, log, biosSettings, server)
223+
bmcClient, err := bmcutils.GetBMCClientForServer(ctx, r.Client, server, r.Insecure, r.BMCOptions)
224+
if err != nil {
225+
return ctrl.Result{}, fmt.Errorf("failed to get BMC client for server: %w", err)
226+
}
227+
defer bmcClient.Logout()
228+
229+
return r.ensureBIOSSettingsStateTransition(ctx, log, bmcClient, biosSettings, server)
228230
}
229231

230232
func (r *BiosSettingsReconciler) ensureBIOSSettingsStateTransition(
231233
ctx context.Context,
232234
log logr.Logger,
235+
bmcClient bmc.BMC,
233236
biosSettings *metalv1alpha1.BIOSSettings,
234237
server *metalv1alpha1.Server,
235238
) (ctrl.Result, error) {
236239
switch biosSettings.Status.State {
237240
case "", metalv1alpha1.BIOSSettingsStatePending:
238-
pendingSettings, err := r.getPendingSettingsOnBIOS(ctx, log, server)
241+
pendingSettings, err := r.getPendingSettingsOnBIOS(ctx, log, bmcClient, server)
239242
if err != nil {
240243
return ctrl.Result{}, fmt.Errorf("failed to get pending settings on bios: %w", err)
241244
}
@@ -247,9 +250,9 @@ func (r *BiosSettingsReconciler) ensureBIOSSettingsStateTransition(
247250
err = r.updateBiosSettingsStatus(ctx, log, biosSettings, metalv1alpha1.BIOSSettingsStateInProgress)
248251
return ctrl.Result{}, err
249252
case metalv1alpha1.BIOSSettingsStateInProgress:
250-
return r.handleSettingInProgressState(ctx, log, biosSettings, server)
253+
return r.handleSettingInProgressState(ctx, log, bmcClient, biosSettings, server)
251254
case metalv1alpha1.BIOSSettingsStateApplied:
252-
return r.handleSettingAppliedState(ctx, log, biosSettings, server)
255+
return r.handleSettingAppliedState(ctx, log, bmcClient, biosSettings, server)
253256
case metalv1alpha1.BIOSSettingsStateFailed:
254257
return r.handleFailedState(ctx, log, biosSettings, server)
255258
}
@@ -260,10 +263,11 @@ func (r *BiosSettingsReconciler) ensureBIOSSettingsStateTransition(
260263
func (r *BiosSettingsReconciler) handleSettingInProgressState(
261264
ctx context.Context,
262265
log logr.Logger,
266+
bmcClient bmc.BMC,
263267
biosSettings *metalv1alpha1.BIOSSettings,
264268
server *metalv1alpha1.Server,
265269
) (ctrl.Result, error) {
266-
currentBiosVersion, settingsDiff, err := r.getBIOSVersionAndSettingDifference(ctx, log, biosSettings, server)
270+
currentBiosVersion, settingsDiff, err := r.getBIOSVersionAndSettingDifference(ctx, log, bmcClient, biosSettings, server)
267271
if err != nil {
268272
return ctrl.Result{}, fmt.Errorf("failed to get BIOS settings: %w", err)
269273
}
@@ -280,7 +284,7 @@ func (r *BiosSettingsReconciler) handleSettingInProgressState(
280284
return ctrl.Result{}, nil
281285
}
282286

283-
if req, err := r.checkAndRequestMaintenance(ctx, log, biosSettings, server, settingsDiff); err != nil || req {
287+
if req, err := r.checkAndRequestMaintenance(ctx, log, bmcClient, biosSettings, server, settingsDiff); err != nil || req {
284288
return ctrl.Result{}, err
285289
}
286290

@@ -290,25 +294,20 @@ func (r *BiosSettingsReconciler) handleSettingInProgressState(
290294
return ctrl.Result{}, nil
291295
}
292296

293-
return r.applySettingUpdateStateTransition(ctx, log, biosSettings, server, settingsDiff)
297+
return r.applySettingUpdateStateTransition(ctx, log, bmcClient, biosSettings, server, settingsDiff)
294298
}
295299

296300
func (r *BiosSettingsReconciler) checkAndRequestMaintenance(
297301
ctx context.Context,
298302
log logr.Logger,
303+
bmcClient bmc.BMC,
299304
biosSettings *metalv1alpha1.BIOSSettings,
300305
server *metalv1alpha1.Server,
301306
settingsDiff redfish.SettingsAttributes,
302307
) (bool, error) {
303308
// check if we need to request maintenance if we dont have it already
304309
// note: having this check will reduce the call made to BMC.
305310
if biosSettings.Spec.ServerMaintenanceRef == nil {
306-
bmcClient, err := bmcutils.GetBMCClientForServer(ctx, r.Client, server, r.Insecure, r.BMCOptions)
307-
if err != nil {
308-
return false, err
309-
}
310-
defer bmcClient.Logout()
311-
312311
resetReq, err := bmcClient.CheckBiosAttributes(settingsDiff)
313312
if resetReq {
314313
// request maintenance if needed, even if err was reported.
@@ -325,6 +324,7 @@ func (r *BiosSettingsReconciler) checkAndRequestMaintenance(
325324
func (r *BiosSettingsReconciler) applySettingUpdateStateTransition(
326325
ctx context.Context,
327326
log logr.Logger,
327+
bmcClient bmc.BMC,
328328
biosSettings *metalv1alpha1.BIOSSettings,
329329
server *metalv1alpha1.Server,
330330
settingsDiff redfish.SettingsAttributes,
@@ -350,14 +350,8 @@ func (r *BiosSettingsReconciler) applySettingUpdateStateTransition(
350350
log.V(1).Info("Reconciled biosSettings at Pending state")
351351
return ctrl.Result{}, err
352352
case metalv1alpha1.BIOSSettingUpdateStateIssue:
353-
bmcClient, err := bmcutils.GetBMCClientForServer(ctx, r.Client, server, r.Insecure, r.BMCOptions)
354-
if err != nil {
355-
return ctrl.Result{}, err
356-
}
357-
defer bmcClient.Logout()
358-
359353
// check if the pending tasks not present on the bios settings
360-
pendingSettings, err := r.getPendingSettingsOnBIOS(ctx, log, server)
354+
pendingSettings, err := r.getPendingSettingsOnBIOS(ctx, log, bmcClient, server)
361355
if err != nil {
362356
return ctrl.Result{}, fmt.Errorf("failed to get pending BIOS settings: %w", err)
363357
}
@@ -369,13 +363,13 @@ func (r *BiosSettingsReconciler) applySettingUpdateStateTransition(
369363
}
370364
}
371365

372-
// get latest pending settings, and expect it to be zero different from the required settings.
373-
pendingSettings, err = r.getPendingSettingsOnBIOS(ctx, log, server)
366+
// Get the latest pending settings and expect it to be zero different from the required settings.
367+
pendingSettings, err = r.getPendingSettingsOnBIOS(ctx, log, bmcClient, server)
374368
if err != nil {
375369
return ctrl.Result{}, fmt.Errorf("failed to get pending BIOS settings: %w", err)
376370
}
377371

378-
// at this point the bios setting update needs to be already issued.
372+
// At this point the BIOS setting update needs to be already issued.
379373
if len(pendingSettings) == 0 {
380374
// todo: fail after X amount of time
381375
log.V(1).Info("bios Setting update issued to bmc not accepted. retrying....")
@@ -430,7 +424,7 @@ func (r *BiosSettingsReconciler) applySettingUpdateStateTransition(
430424
return ctrl.Result{}, nil
431425
case metalv1alpha1.BIOSSettingUpdateStateVerification:
432426
// make sure the setting has actually applied.
433-
_, settingsDiff, err := r.getBIOSVersionAndSettingDifference(ctx, log, biosSettings, server)
427+
_, settingsDiff, err := r.getBIOSVersionAndSettingDifference(ctx, log, bmcClient, biosSettings, server)
434428

435429
if err != nil {
436430
return ctrl.Result{}, fmt.Errorf("failed to get BIOS settings: %w", err)
@@ -453,6 +447,7 @@ func (r *BiosSettingsReconciler) applySettingUpdateStateTransition(
453447
func (r *BiosSettingsReconciler) handleSettingAppliedState(
454448
ctx context.Context,
455449
log logr.Logger,
450+
bmcClient bmc.BMC,
456451
biosSettings *metalv1alpha1.BIOSSettings,
457452
server *metalv1alpha1.Server,
458453
) (ctrl.Result, error) {
@@ -461,7 +456,7 @@ func (r *BiosSettingsReconciler) handleSettingAppliedState(
461456
return ctrl.Result{}, err
462457
}
463458

464-
_, settingsDiff, err := r.getBIOSVersionAndSettingDifference(ctx, log, biosSettings, server)
459+
_, settingsDiff, err := r.getBIOSVersionAndSettingDifference(ctx, log, bmcClient, biosSettings, server)
465460

466461
if err != nil {
467462
log.V(1).Error(err, "unable to fetch and check BIOSSettings")
@@ -507,15 +502,10 @@ func (r *BiosSettingsReconciler) checkPendingSettingsDiff(
507502
func (r *BiosSettingsReconciler) getPendingSettingsOnBIOS(
508503
ctx context.Context,
509504
log logr.Logger,
505+
bmcClient bmc.BMC,
510506
server *metalv1alpha1.Server,
511507
) (pendingSettings redfish.SettingsAttributes, err error) {
512-
bmcClient, err := bmcutils.GetBMCClientForServer(ctx, r.Client, server, r.Insecure, r.BMCOptions)
513-
if err != nil {
514-
return pendingSettings, fmt.Errorf("failed to create BMC client: %w", err)
515-
}
516-
defer bmcClient.Logout()
517-
518-
log.V(1).Info("fetching the pending settings on bios")
508+
log.V(1).Info("Fetching the pending settings on bios")
519509

520510
pendingSettings, err = bmcClient.GetBiosPendingAttributeValues(ctx, server.Spec.SystemUUID)
521511
if err != nil {
@@ -528,15 +518,10 @@ func (r *BiosSettingsReconciler) getPendingSettingsOnBIOS(
528518
func (r *BiosSettingsReconciler) getBIOSVersionAndSettingDifference(
529519
ctx context.Context,
530520
log logr.Logger,
521+
bmcClient bmc.BMC,
531522
biosSettings *metalv1alpha1.BIOSSettings,
532523
server *metalv1alpha1.Server,
533524
) (currentbiosVersion string, diff redfish.SettingsAttributes, err error) {
534-
bmcClient, err := bmcutils.GetBMCClientForServer(ctx, r.Client, server, r.Insecure, r.BMCOptions)
535-
if err != nil {
536-
return "", diff, fmt.Errorf("failed to create BMC client: %w", err)
537-
}
538-
defer bmcClient.Logout()
539-
540525
keys := slices.Collect(maps.Keys(biosSettings.Spec.SettingsMap))
541526

542527
currentSettings, err := bmcClient.GetBiosAttributeValues(ctx, server.Spec.SystemUUID, keys)

0 commit comments

Comments
 (0)