Skip to content

Commit 642184e

Browse files
Gerrit91Gerrit91
committed
Refactor.
1 parent 469acd2 commit 642184e

File tree

1 file changed

+78
-58
lines changed

1 file changed

+78
-58
lines changed

‎internal/controller/metalstackmachine_controller.go‎

Lines changed: 78 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,16 @@ type MetalStackMachineReconciler struct {
5858
Scheme *runtime.Scheme
5959
}
6060

61+
type machineReconciler struct {
62+
metalClient metalgo.Client
63+
client client.Client
64+
ctx context.Context
65+
log logr.Logger
66+
infraCluster *v1alpha1.MetalStackCluster
67+
clusterMachine *clusterv1.Machine
68+
infraMachine *v1alpha1.MetalStackMachine
69+
}
70+
6171
// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machines,verbs=get;list;watch
6272
// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=metalstackmachines,verbs=get;list;watch;create;update;patch;delete
6373
// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=metalstackmachines/status,verbs=get;update;patch
@@ -119,8 +129,18 @@ func (r *MetalStackMachineReconciler) Reconcile(ctx context.Context, req ctrl.Re
119129
return ctrl.Result{}, err
120130
}
121131

132+
reconciler := &machineReconciler{
133+
metalClient: r.MetalClient,
134+
client: r.Client,
135+
ctx: ctx,
136+
log: log,
137+
infraCluster: infraCluster,
138+
clusterMachine: machine,
139+
infraMachine: infraMachine,
140+
}
141+
122142
defer func() {
123-
statusErr := r.status(ctx, infraCluster, machine, infraMachine)
143+
statusErr := reconciler.status()
124144
if statusErr != nil {
125145
err = errors.Join(err, fmt.Errorf("unable to update status: %w", statusErr))
126146
}
@@ -132,7 +152,7 @@ func (r *MetalStackMachineReconciler) Reconcile(ctx context.Context, req ctrl.Re
132152
}
133153

134154
log.Info("reconciling resource deletion flow")
135-
err := r.delete(ctx, log, infraCluster, infraMachine, machine)
155+
err := reconciler.delete()
136156
if err != nil {
137157
return ctrl.Result{}, err
138158
}
@@ -168,7 +188,7 @@ func (r *MetalStackMachineReconciler) Reconcile(ctx context.Context, req ctrl.Re
168188
return ctrl.Result{}, errors.New("waiting until control plane ip was set to cluster spec")
169189
}
170190

171-
err = r.reconcile(ctx, log, infraCluster, machine, infraMachine)
191+
err = reconciler.reconcile()
172192

173193
return ctrl.Result{}, err // remember to return err here and not nil because the defer func can influence this
174194
}
@@ -181,14 +201,14 @@ func (r *MetalStackMachineReconciler) SetupWithManager(mgr ctrl.Manager) error {
181201
Complete(r)
182202
}
183203

184-
func (r *MetalStackMachineReconciler) reconcile(ctx context.Context, log logr.Logger, infraCluster *v1alpha1.MetalStackCluster, clusterMachine *clusterv1.Machine, infraMachine *v1alpha1.MetalStackMachine) error {
185-
m, err := r.findProviderMachine(ctx, infraCluster, infraMachine, util.IsControlPlaneMachine(clusterMachine))
204+
func (r *machineReconciler) reconcile() error {
205+
m, err := r.findProviderMachine()
186206
if err != nil && !errors.Is(err, errProviderMachineNotFound) {
187207
return err
188208
}
189209

190210
if errors.Is(err, errProviderMachineNotFound) {
191-
m, err = r.create(ctx, infraCluster, infraMachine, util.IsControlPlaneMachine(clusterMachine))
211+
m, err = r.create()
192212
if err != nil {
193213
return fmt.Errorf("unable to create machine at provider: %w", err)
194214
}
@@ -198,25 +218,25 @@ func (r *MetalStackMachineReconciler) reconcile(ctx context.Context, log logr.Lo
198218
return errors.New("machine allocated but got no provider ID")
199219
}
200220

201-
log.Info("setting provider id into machine resource")
221+
r.log.Info("setting provider id into machine resource")
202222

203-
helper, err := patch.NewHelper(infraMachine, r.Client)
223+
helper, err := patch.NewHelper(r.infraMachine, r.client)
204224
if err != nil {
205225
return err
206226
}
207227

208-
infraMachine.Spec.ProviderID = *m.ID
228+
r.infraMachine.Spec.ProviderID = *m.ID
209229

210-
err = helper.Patch(ctx, infraMachine) // TODO:check whether patch is not executed when no changes occur
230+
err = helper.Patch(r.ctx, r.infraMachine) // TODO:check whether patch is not executed when no changes occur
211231
if err != nil {
212-
return fmt.Errorf("failed to update infra machine provider ID %q: %w", infraMachine.Spec.ProviderID, err)
232+
return fmt.Errorf("failed to update infra machine provider ID %q: %w", r.infraMachine.Spec.ProviderID, err)
213233
}
214234

215235
return nil
216236
}
217237

218-
func (r *MetalStackMachineReconciler) delete(ctx context.Context, log logr.Logger, infraCluster *v1alpha1.MetalStackCluster, infraMachine *v1alpha1.MetalStackMachine, clusterMachine *clusterv1.Machine) error {
219-
m, err := r.findProviderMachine(ctx, infraCluster, infraMachine, util.IsControlPlaneMachine(clusterMachine))
238+
func (r *machineReconciler) delete() error {
239+
m, err := r.findProviderMachine()
220240
if errors.Is(err, errProviderMachineNotFound) {
221241
// metal-stack machine already freed
222242
return nil
@@ -225,31 +245,31 @@ func (r *MetalStackMachineReconciler) delete(ctx context.Context, log logr.Logge
225245
return fmt.Errorf("failed to find provider machine: %w", err)
226246
}
227247

228-
_, err = r.MetalClient.Machine().FreeMachine(metalmachine.NewFreeMachineParamsWithContext(ctx).WithID(*m.ID), nil)
248+
_, err = r.metalClient.Machine().FreeMachine(metalmachine.NewFreeMachineParamsWithContext(r.ctx).WithID(*m.ID), nil)
229249
if err != nil {
230250
return fmt.Errorf("failed to delete provider machine: %w", err)
231251
}
232252

233-
log.Info("freed provider machine")
253+
r.log.Info("freed provider machine")
234254

235255
return nil
236256
}
237257

238-
func (r *MetalStackMachineReconciler) create(ctx context.Context, infraCluster *v1alpha1.MetalStackCluster, infraMachine *v1alpha1.MetalStackMachine, isControlPlaneMachine bool) (*models.V1MachineResponse, error) {
258+
func (r *machineReconciler) create() (*models.V1MachineResponse, error) {
239259
var (
240260
ips []string
241261
nws = []*models.V1MachineAllocationNetwork{
242262
{
243263
Autoacquire: ptr.To(true),
244-
Networkid: infraCluster.Status.NodeNetworkID,
264+
Networkid: r.infraCluster.Status.NodeNetworkID,
245265
},
246266
}
247267
)
248268

249-
if isControlPlaneMachine {
250-
ips = append(ips, infraCluster.Spec.ControlPlaneEndpoint.Host)
269+
if util.IsControlPlaneMachine(r.clusterMachine) {
270+
ips = append(ips, r.infraCluster.Spec.ControlPlaneEndpoint.Host)
251271

252-
resp, err := r.MetalClient.IP().FindIP(ipmodels.NewFindIPParams().WithID(infraCluster.Spec.ControlPlaneEndpoint.Host).WithContext(ctx), nil)
272+
resp, err := r.metalClient.IP().FindIP(ipmodels.NewFindIPParams().WithID(r.infraCluster.Spec.ControlPlaneEndpoint.Host).WithContext(r.ctx), nil)
253273
if err != nil {
254274
return nil, fmt.Errorf("unable to lookup control plane ip: %w", err)
255275
}
@@ -260,16 +280,16 @@ func (r *MetalStackMachineReconciler) create(ctx context.Context, infraCluster *
260280
})
261281
}
262282

263-
resp, err := r.MetalClient.Machine().AllocateMachine(metalmachine.NewAllocateMachineParamsWithContext(ctx).WithBody(&models.V1MachineAllocateRequest{
264-
Partitionid: &infraCluster.Spec.Partition,
265-
Projectid: &infraCluster.Spec.ProjectID,
266-
PlacementTags: []string{tag.New(tag.ClusterID, string(infraCluster.GetUID()))},
267-
Tags: machineTags(infraCluster, infraMachine, isControlPlaneMachine),
268-
Name: infraMachine.Name,
269-
Hostname: infraMachine.Name,
270-
Sizeid: &infraMachine.Spec.Size,
271-
Imageid: &infraMachine.Spec.Image,
272-
Description: fmt.Sprintf("%s/%s for cluster %s/%s", infraMachine.Namespace, infraMachine.Name, infraCluster.Namespace, infraCluster.Name),
283+
resp, err := r.metalClient.Machine().AllocateMachine(metalmachine.NewAllocateMachineParamsWithContext(r.ctx).WithBody(&models.V1MachineAllocateRequest{
284+
Partitionid: &r.infraCluster.Spec.Partition,
285+
Projectid: &r.infraCluster.Spec.ProjectID,
286+
PlacementTags: []string{tag.New(tag.ClusterID, string(r.infraCluster.GetUID()))},
287+
Tags: r.machineTags(),
288+
Name: r.infraMachine.Name,
289+
Hostname: r.infraMachine.Name,
290+
Sizeid: &r.infraMachine.Spec.Size,
291+
Imageid: &r.infraMachine.Spec.Image,
292+
Description: fmt.Sprintf("%s/%s for cluster %s/%s", r.infraMachine.Namespace, r.infraMachine.Name, r.infraCluster.Namespace, r.infraCluster.Name),
273293
Networks: nws,
274294
Ips: ips,
275295
// TODO: UserData, SSHPubKeys, ...
@@ -281,14 +301,14 @@ func (r *MetalStackMachineReconciler) create(ctx context.Context, infraCluster *
281301
return resp.Payload, nil
282302
}
283303

284-
func (r *MetalStackMachineReconciler) status(ctx context.Context, infraCluster *v1alpha1.MetalStackCluster, clusterMachine *clusterv1.Machine, infraMachine *v1alpha1.MetalStackMachine) error {
304+
func (r *machineReconciler) status() error {
285305
var (
286-
g, _ = errgroup.WithContext(ctx)
306+
g, _ = errgroup.WithContext(r.ctx)
287307
conditionUpdates = make(chan func())
288308

289309
// TODO: probably there is a helper for this available somewhere?
290310
allConditionsTrue = func() bool {
291-
for _, c := range infraMachine.Status.Conditions {
311+
for _, c := range r.infraMachine.Status.Conditions {
292312
if c.Status != corev1.ConditionTrue {
293313
return false
294314
}
@@ -303,21 +323,21 @@ func (r *MetalStackMachineReconciler) status(ctx context.Context, infraCluster *
303323
}()
304324

305325
g.Go(func() error {
306-
m, err := r.findProviderMachine(ctx, infraCluster, infraMachine, util.IsControlPlaneMachine(clusterMachine))
326+
m, err := r.findProviderMachine()
307327

308328
conditionUpdates <- func() {
309329
switch {
310330
case err != nil && !errors.Is(err, errProviderMachineNotFound):
311-
conditions.MarkFalse(infraMachine, v1alpha1.ProviderMachineCreated, "InternalError", clusterv1.ConditionSeverityError, "%s", err.Error())
312-
conditions.MarkFalse(infraMachine, v1alpha1.ProviderMachineHealthy, "NotHealthy", clusterv1.ConditionSeverityWarning, "machine not created")
331+
conditions.MarkFalse(r.infraMachine, v1alpha1.ProviderMachineCreated, "InternalError", clusterv1.ConditionSeverityError, "%s", err.Error())
332+
conditions.MarkFalse(r.infraMachine, v1alpha1.ProviderMachineHealthy, "NotHealthy", clusterv1.ConditionSeverityWarning, "machine not created")
313333
case err != nil && errors.Is(err, errProviderMachineNotFound):
314-
conditions.MarkFalse(infraMachine, v1alpha1.ProviderMachineCreated, "NotCreated", clusterv1.ConditionSeverityError, "%s", err.Error())
315-
conditions.MarkFalse(infraMachine, v1alpha1.ProviderMachineHealthy, "NotHealthy", clusterv1.ConditionSeverityWarning, "machine not created")
334+
conditions.MarkFalse(r.infraMachine, v1alpha1.ProviderMachineCreated, "NotCreated", clusterv1.ConditionSeverityError, "%s", err.Error())
335+
conditions.MarkFalse(r.infraMachine, v1alpha1.ProviderMachineHealthy, "NotHealthy", clusterv1.ConditionSeverityWarning, "machine not created")
316336
default:
317-
if infraMachine.Spec.ProviderID == *m.ID {
318-
conditions.MarkTrue(infraMachine, v1alpha1.ProviderMachineCreated)
337+
if r.infraMachine.Spec.ProviderID == *m.ID {
338+
conditions.MarkTrue(r.infraMachine, v1alpha1.ProviderMachineCreated)
319339
} else {
320-
conditions.MarkFalse(infraMachine, v1alpha1.ProviderMachineCreated, "NotSet", clusterv1.ConditionSeverityWarning, "provider id was not yet patched into the machine's spec")
340+
conditions.MarkFalse(r.infraMachine, v1alpha1.ProviderMachineCreated, "NotSet", clusterv1.ConditionSeverityWarning, "provider id was not yet patched into the machine's spec")
321341
}
322342

323343
var errs []error
@@ -338,15 +358,15 @@ func (r *MetalStackMachineReconciler) status(ctx context.Context, infraCluster *
338358
}
339359

340360
if len(errs) == 0 {
341-
conditions.MarkTrue(infraMachine, v1alpha1.ProviderMachineHealthy)
361+
conditions.MarkTrue(r.infraMachine, v1alpha1.ProviderMachineHealthy)
342362
} else {
343-
conditions.MarkFalse(infraMachine, v1alpha1.ProviderMachineHealthy, "NotHealthy", clusterv1.ConditionSeverityWarning, "%s", errors.Join(errs...).Error())
363+
conditions.MarkFalse(r.infraMachine, v1alpha1.ProviderMachineHealthy, "NotHealthy", clusterv1.ConditionSeverityWarning, "%s", errors.Join(errs...).Error())
344364
}
345365

346-
infraMachine.Status.Addresses = nil
366+
r.infraMachine.Status.Addresses = nil
347367

348368
if m.Allocation.Hostname != nil {
349-
infraMachine.Status.Addresses = append(infraMachine.Status.Addresses, clusterv1.MachineAddress{
369+
r.infraMachine.Status.Addresses = append(r.infraMachine.Status.Addresses, clusterv1.MachineAddress{
350370
Type: clusterv1.MachineHostName,
351371
Address: *m.Allocation.Hostname,
352372
})
@@ -356,14 +376,14 @@ func (r *MetalStackMachineReconciler) status(ctx context.Context, infraCluster *
356376
switch ptr.Deref(nw.Networktype, "") {
357377
case "privateprimaryunshared":
358378
for _, ip := range nw.Ips {
359-
infraMachine.Status.Addresses = append(infraMachine.Status.Addresses, clusterv1.MachineAddress{
379+
r.infraMachine.Status.Addresses = append(r.infraMachine.Status.Addresses, clusterv1.MachineAddress{
360380
Type: clusterv1.MachineInternalIP,
361381
Address: ip,
362382
})
363383
}
364384
case "external":
365385
for _, ip := range nw.Ips {
366-
infraMachine.Status.Addresses = append(infraMachine.Status.Addresses, clusterv1.MachineAddress{
386+
r.infraMachine.Status.Addresses = append(r.infraMachine.Status.Addresses, clusterv1.MachineAddress{
367387
Type: clusterv1.MachineExternalIP,
368388
Address: ip,
369389
})
@@ -384,22 +404,22 @@ func (r *MetalStackMachineReconciler) status(ctx context.Context, infraCluster *
384404

385405
groupErr := g.Wait()
386406
if groupErr == nil && allConditionsTrue() {
387-
infraMachine.Status.Ready = true
407+
r.infraMachine.Status.Ready = true
388408
}
389409

390-
err := r.Client.Status().Update(ctx, infraMachine)
410+
err := r.client.Status().Update(r.ctx, r.infraMachine)
391411

392412
return errors.Join(groupErr, err)
393413
}
394414

395-
func (r *MetalStackMachineReconciler) findProviderMachine(ctx context.Context, infraCluster *v1alpha1.MetalStackCluster, infraMachine *v1alpha1.MetalStackMachine, isControlPlaneMachine bool) (*models.V1MachineResponse, error) {
415+
func (r *machineReconciler) findProviderMachine() (*models.V1MachineResponse, error) {
396416
mfr := &models.V1MachineFindRequest{
397-
ID: infraMachine.Spec.ProviderID,
398-
AllocationProject: infraCluster.Spec.ProjectID,
399-
Tags: machineTags(infraCluster, infraMachine, isControlPlaneMachine),
417+
ID: r.infraMachine.Spec.ProviderID,
418+
AllocationProject: r.infraCluster.Spec.ProjectID,
419+
Tags: r.machineTags(),
400420
}
401421

402-
resp, err := r.MetalClient.Machine().FindMachines(metalmachine.NewFindMachinesParamsWithContext(ctx).WithBody(mfr), nil)
422+
resp, err := r.metalClient.Machine().FindMachines(metalmachine.NewFindMachinesParamsWithContext(r.ctx).WithBody(mfr), nil)
403423
if err != nil {
404424
return nil, err
405425
}
@@ -415,13 +435,13 @@ func (r *MetalStackMachineReconciler) findProviderMachine(ctx context.Context, i
415435
}
416436
}
417437

418-
func machineTags(infraCluster *v1alpha1.MetalStackCluster, infraMachine *v1alpha1.MetalStackMachine, isControlPlaneMachine bool) []string {
438+
func (r *machineReconciler) machineTags() []string {
419439
tags := []string{
420-
tag.New(tag.ClusterID, string(infraCluster.GetUID())),
421-
tag.New(v1alpha1.TagInfraMachineID, string(infraMachine.GetUID())),
440+
tag.New(tag.ClusterID, string(r.infraCluster.GetUID())),
441+
tag.New(v1alpha1.TagInfraMachineID, string(r.infraMachine.GetUID())),
422442
}
423443

424-
if isControlPlaneMachine {
444+
if util.IsControlPlaneMachine(r.clusterMachine) {
425445
tags = append(tags, v1alpha1.TagControlPlanePurpose)
426446
}
427447

0 commit comments

Comments
 (0)