Skip to content

Commit 265c3d1

Browse files
EmilienMEmilienM
committed
WIP - use logger from ctx instead of scope
1 parent f128d15 commit 265c3d1

File tree

9 files changed

+121
-85
lines changed

9 files changed

+121
-85
lines changed

‎controllers/openstackcluster_controller.go

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,12 @@ func (r *OpenStackClusterReconciler) Reconcile(ctx context.Context, req ctrl.Req
135135
}
136136

137137
// Handle non-deleted clusters
138-
return reconcileNormal(scope, cluster, openStackCluster)
138+
return reconcileNormal(ctx, scope, cluster, openStackCluster)
139139
}
140140

141141
func (r *OpenStackClusterReconciler) reconcileDelete(ctx context.Context, scope scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) (ctrl.Result, error) {
142-
scope.Logger().Info("Reconciling Cluster delete")
142+
log := ctrl.LoggerFrom(ctx)
143+
log.Info("Reconciling OpenStackCluster delete")
143144

144145
// Wait for machines to be deleted before removing the finalizer as they
145146
// depend on this resource to deprovision. Additionally it appears that
@@ -151,23 +152,23 @@ func (r *OpenStackClusterReconciler) reconcileDelete(ctx context.Context, scope
151152
}
152153

153154
if len(machines) != 0 {
154-
scope.Logger().Info("Waiting for machines to be deleted", "remaining", len(machines))
155+
log.Info("Waiting for machines to be deleted", "remaining", len(machines))
155156
return ctrl.Result{RequeueAfter: 5 * time.Second}, nil
156157
}
157158

158-
if err := deleteBastion(scope, cluster, openStackCluster); err != nil {
159+
if err := deleteBastion(ctx, scope, cluster, openStackCluster); err != nil {
159160
return reconcile.Result{}, err
160161
}
161162

162-
networkingService, err := networking.NewService(scope)
163+
networkingService, err := networking.NewService(scope, ctx)
163164
if err != nil {
164165
return reconcile.Result{}, err
165166
}
166167

167168
clusterName := fmt.Sprintf("%s-%s", cluster.Namespace, cluster.Name)
168169

169170
if openStackCluster.Spec.APIServerLoadBalancer.Enabled {
170-
loadBalancerService, err := loadbalancer.NewService(scope)
171+
loadBalancerService, err := loadbalancer.NewService(scope, ctx)
171172
if err != nil {
172173
return reconcile.Result{}, err
173174
}
@@ -203,7 +204,7 @@ func (r *OpenStackClusterReconciler) reconcileDelete(ctx context.Context, scope
203204

204205
// Cluster is deleted so remove the finalizer.
205206
controllerutil.RemoveFinalizer(openStackCluster, infrav1.ClusterFinalizer)
206-
scope.Logger().Info("Reconciled Cluster deleted successfully")
207+
log.Info("Reconciled Cluster deleted successfully")
207208
return ctrl.Result{}, nil
208209
}
209210

@@ -216,12 +217,12 @@ func contains(arr []string, target string) bool {
216217
return false
217218
}
218219

219-
func deleteBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) error {
220+
func deleteBastion(ctx context.Context, scope scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) error {
220221
computeService, err := compute.NewService(scope)
221222
if err != nil {
222223
return err
223224
}
224-
networkingService, err := networking.NewService(scope)
225+
networkingService, err := networking.NewService(scope, ctx)
225226
if err != nil {
226227
return err
227228
}
@@ -283,8 +284,9 @@ func deleteBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackClust
283284
return nil
284285
}
285286

286-
func reconcileNormal(scope scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) (ctrl.Result, error) { //nolint:unparam
287-
scope.Logger().Info("Reconciling Cluster")
287+
func reconcileNormal(ctx context.Context, scope scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) (ctrl.Result, error) { //nolint:unparam
288+
log := ctrl.LoggerFrom(ctx)
289+
log.Info("Reconciling Cluster")
288290

289291
// If the OpenStackCluster doesn't have our finalizer, add it.
290292
if controllerutil.AddFinalizer(openStackCluster, infrav1.ClusterFinalizer) {
@@ -297,12 +299,12 @@ func reconcileNormal(scope scope.Scope, cluster *clusterv1.Cluster, openStackClu
297299
return reconcile.Result{}, err
298300
}
299301

300-
err = reconcileNetworkComponents(scope, cluster, openStackCluster)
302+
err = reconcileNetworkComponents(ctx, scope, cluster, openStackCluster)
301303
if err != nil {
302304
return reconcile.Result{}, err
303305
}
304306

305-
result, err := reconcileBastion(scope, cluster, openStackCluster)
307+
result, err := reconcileBastion(ctx, scope, cluster, openStackCluster)
306308
if err != nil || !reflect.DeepEqual(result, reconcile.Result{}) {
307309
return result, err
308310
}
@@ -330,15 +332,16 @@ func reconcileNormal(scope scope.Scope, cluster *clusterv1.Cluster, openStackClu
330332
openStackCluster.Status.Ready = true
331333
openStackCluster.Status.FailureMessage = nil
332334
openStackCluster.Status.FailureReason = nil
333-
scope.Logger().Info("Reconciled Cluster created successfully")
335+
log.Info("Reconciled Cluster created successfully")
334336
return reconcile.Result{}, nil
335337
}
336338

337-
func reconcileBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) (ctrl.Result, error) {
338-
scope.Logger().Info("Reconciling Bastion")
339+
func reconcileBastion(ctx context.Context, scope scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) (ctrl.Result, error) {
340+
log := ctrl.LoggerFrom(ctx)
341+
log.Info("Reconciling Bastion")
339342

340343
if openStackCluster.Spec.Bastion == nil || !openStackCluster.Spec.Bastion.Enabled {
341-
return reconcile.Result{}, deleteBastion(scope, cluster, openStackCluster)
344+
return reconcile.Result{}, deleteBastion(ctx, scope, cluster, openStackCluster)
342345
}
343346

344347
computeService, err := compute.NewService(scope)
@@ -352,7 +355,7 @@ func reconcileBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackCl
352355
return reconcile.Result{}, fmt.Errorf("failed computing bastion hash from instance spec: %w", err)
353356
}
354357
if bastionHashHasChanged(bastionHash, openStackCluster.ObjectMeta.Annotations) {
355-
if err := deleteBastion(scope, cluster, openStackCluster); err != nil {
358+
if err := deleteBastion(ctx, scope, cluster, openStackCluster); err != nil {
356359
return ctrl.Result{}, err
357360
}
358361
}
@@ -385,15 +388,15 @@ func reconcileBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackCl
385388
case infrav1.InstanceStateError:
386389
return ctrl.Result{}, fmt.Errorf("failed to reconcile bastion, instance state is ERROR")
387390
case infrav1.InstanceStateBuild, infrav1.InstanceStateUndefined:
388-
scope.Logger().Info("Waiting for bastion instance to become ACTIVE", "id", instanceStatus.ID(), "status", instanceStatus.State())
391+
log.Info("Waiting for bastion instance to become ACTIVE", "id", instanceStatus.ID(), "status", instanceStatus.State())
389392
return ctrl.Result{RequeueAfter: waitForBuildingInstanceToReconcile}, nil
390393
case infrav1.InstanceStateDeleted:
391394
// This should normally be handled by deleteBastion
392395
openStackCluster.Status.Bastion = nil
393396
return ctrl.Result{}, nil
394397
}
395398

396-
networkingService, err := networking.NewService(scope)
399+
networkingService, err := networking.NewService(scope, ctx)
397400
if err != nil {
398401
return ctrl.Result{}, err
399402
}
@@ -474,15 +477,17 @@ func bastionHashHasChanged(computeHash string, clusterAnnotations map[string]str
474477
return latestHash != computeHash
475478
}
476479

477-
func reconcileNetworkComponents(scope scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) error {
480+
func reconcileNetworkComponents(ctx context.Context, scope scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) error {
481+
log := ctrl.LoggerFrom(ctx)
482+
478483
clusterName := fmt.Sprintf("%s-%s", cluster.Namespace, cluster.Name)
479484

480-
networkingService, err := networking.NewService(scope)
485+
networkingService, err := networking.NewService(scope, ctx)
481486
if err != nil {
482487
return err
483488
}
484489

485-
scope.Logger().Info("Reconciling network components")
490+
log.Info("Reconciling network components")
486491

487492
err = networkingService.ReconcileExternalNetwork(openStackCluster)
488493
if err != nil {
@@ -491,7 +496,7 @@ func reconcileNetworkComponents(scope scope.Scope, cluster *clusterv1.Cluster, o
491496
}
492497

493498
if openStackCluster.Spec.NodeCIDR == "" {
494-
scope.Logger().V(4).Info("No need to reconcile network, searching network and subnet instead")
499+
log.V(4).Info("No need to reconcile network, searching network and subnet instead")
495500

496501
netOpts := openStackCluster.Spec.Network.ToListOpt()
497502
networkList, err := networkingService.GetNetworksByFilter(&netOpts)
@@ -559,7 +564,7 @@ func reconcileNetworkComponents(scope scope.Scope, cluster *clusterv1.Cluster, o
559564
}
560565

561566
if openStackCluster.Spec.APIServerLoadBalancer.Enabled {
562-
loadBalancerService, err := loadbalancer.NewService(scope)
567+
loadBalancerService, err := loadbalancer.NewService(scope, ctx)
563568
if err != nil {
564569
return err
565570
}

‎controllers/openstackcluster_controller_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ var _ = Describe("OpenStackCluster controller", func() {
217217
networkClientRecorder := mockScopeFactory.NetworkClient.EXPECT()
218218
networkClientRecorder.ListSecGroup(gomock.Any()).Return([]groups.SecGroup{}, nil)
219219

220-
err = deleteBastion(scope, capiCluster, testCluster)
220+
err = deleteBastion(ctx, scope, capiCluster, testCluster)
221221
Expect(testCluster.Status.Bastion).To(BeNil())
222222
Expect(err).To(BeNil())
223223
})
@@ -263,7 +263,7 @@ var _ = Describe("OpenStackCluster controller", func() {
263263
networkClientRecorder.ListPort(gomock.Any()).Return([]ports.Port{{ID: "portID"}}, nil)
264264
networkClientRecorder.ListFloatingIP(floatingips.ListOpts{PortID: "portID"}).Return(make([]floatingips.FloatingIP, 1), nil)
265265

266-
res, err := reconcileBastion(scope, capiCluster, testCluster)
266+
res, err := reconcileBastion(ctx, scope, capiCluster, testCluster)
267267
Expect(testCluster.Status.Bastion).To(Equal(&infrav1.BastionStatus{
268268
ID: "adopted-bastion-uuid",
269269
State: "ACTIVE",
@@ -315,7 +315,7 @@ var _ = Describe("OpenStackCluster controller", func() {
315315
networkClientRecorder.ListPort(gomock.Any()).Return([]ports.Port{{ID: "portID"}}, nil)
316316
networkClientRecorder.ListFloatingIP(floatingips.ListOpts{PortID: "portID"}).Return([]floatingips.FloatingIP{{FloatingIP: "1.2.3.4"}}, nil)
317317

318-
res, err := reconcileBastion(scope, capiCluster, testCluster)
318+
res, err := reconcileBastion(ctx, scope, capiCluster, testCluster)
319319
Expect(testCluster.Status.Bastion).To(Equal(&infrav1.BastionStatus{
320320
ID: "adopted-fip-bastion-uuid",
321321
FloatingIP: "1.2.3.4",
@@ -364,7 +364,7 @@ var _ = Describe("OpenStackCluster controller", func() {
364364
computeClientRecorder := mockScopeFactory.ComputeClient.EXPECT()
365365
computeClientRecorder.GetServer("requeue-bastion-uuid").Return(&server, nil)
366366

367-
res, err := reconcileBastion(scope, capiCluster, testCluster)
367+
res, err := reconcileBastion(ctx, scope, capiCluster, testCluster)
368368
Expect(testCluster.Status.Bastion).To(Equal(&infrav1.BastionStatus{
369369
ID: "requeue-bastion-uuid",
370370
State: "BUILD",
@@ -412,7 +412,7 @@ var _ = Describe("OpenStackCluster controller", func() {
412412
networkClientRecorder.ListExtensions().Return([]extensions.Extension{}, nil)
413413
networkClientRecorder.ListSecGroup(gomock.Any()).Return([]groups.SecGroup{}, nil)
414414

415-
err = deleteBastion(scope, capiCluster, testCluster)
415+
err = deleteBastion(ctx, scope, capiCluster, testCluster)
416416
Expect(err).To(BeNil())
417417
})
418418
It("should implicitly filter cluster subnets by cluster network", func() {
@@ -473,7 +473,7 @@ var _ = Describe("OpenStackCluster controller", func() {
473473
},
474474
}, nil)
475475

476-
err = reconcileNetworkComponents(scope, capiCluster, testCluster)
476+
err = reconcileNetworkComponents(ctx, scope, capiCluster, testCluster)
477477
Expect(err).To(BeNil())
478478
})
479479

@@ -540,7 +540,7 @@ var _ = Describe("OpenStackCluster controller", func() {
540540
CIDR: "2001:db8:2222:5555::/64",
541541
}, nil)
542542

543-
err = reconcileNetworkComponents(scope, capiCluster, testCluster)
543+
err = reconcileNetworkComponents(ctx, scope, capiCluster, testCluster)
544544
Expect(err).To(BeNil())
545545
Expect(len(testCluster.Status.Network.Subnets)).To(Equal(2))
546546
})

‎controllers/openstackfloatingippool_controller.go

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ func (r *OpenStackFloatingIPPoolReconciler) Reconcile(ctx context.Context, req c
114114
}
115115
}()
116116

117-
if err := r.reconcileFloatingIPNetwork(scope, pool); err != nil {
117+
if err := r.reconcileFloatingIPNetwork(ctx, scope, pool); err != nil {
118118
return ctrl.Result{}, err
119119
}
120120

@@ -181,7 +181,7 @@ func (r *OpenStackFloatingIPPoolReconciler) Reconcile(ctx context.Context, req c
181181
})
182182
if err != nil {
183183
// If we failed to create the IPAddress, there might be an IP leak in OpenStack if we also failed to tag the IP after creation
184-
scope.Logger().Error(err, "Failed to create IPAddress", "ip", ip)
184+
log.Error(err, "Failed to create IPAddress", "ip", ip)
185185
return ctrl.Result{}, err
186186
}
187187
}
@@ -190,7 +190,7 @@ func (r *OpenStackFloatingIPPoolReconciler) Reconcile(ctx context.Context, req c
190190
log.Error(err, "Failed to update IPAddressClaim status", "claim", claim.Name, "ipaddress", ipAddress.Name)
191191
return ctrl.Result{}, err
192192
}
193-
scope.Logger().Info("Claimed IP", "ip", ipAddress.Spec.Address)
193+
log.Info("Claimed IP", "ip", ipAddress.Spec.Address)
194194
}
195195
}
196196
return ctrl.Result{}, r.Client.Status().Update(ctx, pool)
@@ -210,7 +210,7 @@ func (r *OpenStackFloatingIPPoolReconciler) reconcileDelete(ctx context.Context,
210210
return errors.New("waiting for IPAddress to be deleted, until we can delete the OpenStackFloatingIPPool")
211211
}
212212

213-
networkingService, err := networking.NewService(scope)
213+
networkingService, err := networking.NewService(scope, ctx)
214214
if err != nil {
215215
return err
216216
}
@@ -266,7 +266,7 @@ func (r *OpenStackFloatingIPPoolReconciler) reconcileIPAddresses(ctx context.Con
266266
return err
267267
}
268268

269-
networkingService, err := networking.NewService(scope)
269+
networkingService, err := networking.NewService(scope, ctx)
270270
if err != nil {
271271
return err
272272
}
@@ -303,12 +303,14 @@ func (r *OpenStackFloatingIPPoolReconciler) reconcileIPAddresses(ctx context.Con
303303
}
304304

305305
func (r *OpenStackFloatingIPPoolReconciler) getIP(ctx context.Context, scope scope.Scope, pool *infrav1alpha1.OpenStackFloatingIPPool) (string, error) {
306+
log := ctrl.LoggerFrom(ctx)
307+
306308
// There's a potential leak of IPs here, if the reconcile loop fails after we claim an IP but before we create the IPAddress object.
307309
var ip string
308310

309-
networkingService, err := networking.NewService(scope)
311+
networkingService, err := networking.NewService(scope, ctx)
310312
if err != nil {
311-
scope.Logger().Error(err, "Failed to create networking service")
313+
log.Error(err, "Failed to create networking service")
312314
return "", err
313315
}
314316

@@ -317,14 +319,14 @@ func (r *OpenStackFloatingIPPoolReconciler) getIP(ctx context.Context, scope sco
317319
if len(pool.Status.AvailableIPs) == 0 {
318320
taggedIPs, err := networkingService.GetFloatingIPsByTag(pool.GetFloatingIPTag())
319321
if err != nil {
320-
scope.Logger().Error(err, "Failed to get floating IPs by tag", "pool", pool.Name)
322+
log.Error(err, "Failed to get floating IPs by tag", "pool", pool.Name)
321323
return "", err
322324
}
323325
for _, taggedIP := range taggedIPs {
324326
if contains(pool.Status.AvailableIPs, taggedIP.FloatingIP) || contains(pool.Status.ClaimedIPs, taggedIP.FloatingIP) {
325327
continue
326328
}
327-
scope.Logger().Info("Tagged floating IP found that was not known to the pool, adding it to the pool", "ip", taggedIP.FloatingIP)
329+
log.Info("Tagged floating IP found that was not known to the pool, adding it to the pool", "ip", taggedIP.FloatingIP)
328330
pool.Status.AvailableIPs = append(pool.Status.AvailableIPs, taggedIP.FloatingIP)
329331
}
330332
}
@@ -346,7 +348,7 @@ func (r *OpenStackFloatingIPPoolReconciler) getIP(ctx context.Context, scope sco
346348

347349
fp, err := networkingService.CreateFloatingIPForPool(pool)
348350
if err != nil {
349-
scope.Logger().Error(err, "Failed to create floating IP", "pool", pool.Name)
351+
log.Error(err, "Failed to create floating IP", "pool", pool.Name)
350352
conditions.MarkFalse(pool, infrav1alpha1.OpenstackFloatingIPPoolReadyCondition, infrav1.OpenStackErrorReason, clusterv1.ConditionSeverityError, "Failed to create floating IP: %v", err)
351353
if ip != "" {
352354
pool.Status.FailedIPs = append(pool.Status.FailedIPs, ip)
@@ -358,13 +360,13 @@ func (r *OpenStackFloatingIPPoolReconciler) getIP(ctx context.Context, scope sco
358360

359361
err := wait.ExponentialBackoffWithContext(ctx, backoff, func(ctx context.Context) (bool, error) {
360362
if err := networkingService.TagFloatingIP(fp.FloatingIP, tag); err != nil {
361-
scope.Logger().Error(err, "Failed to tag floating IP, retrying", "ip", fp.FloatingIP, "tag", tag)
363+
log.Error(err, "Failed to tag floating IP, retrying", "ip", fp.FloatingIP, "tag", tag)
362364
return false, err
363365
}
364366
return true, nil
365367
})
366368
if err != nil {
367-
scope.Logger().Error(err, "Failed to tag floating IP", "ip", fp.FloatingIP, "tag", tag)
369+
log.Error(err, "Failed to tag floating IP", "ip", fp.FloatingIP, "tag", tag)
368370
}
369371
}()
370372

@@ -375,13 +377,13 @@ func (r *OpenStackFloatingIPPoolReconciler) getIP(ctx context.Context, scope sco
375377
return ip, nil
376378
}
377379

378-
func (r *OpenStackFloatingIPPoolReconciler) reconcileFloatingIPNetwork(scope scope.Scope, pool *infrav1alpha1.OpenStackFloatingIPPool) error {
380+
func (r *OpenStackFloatingIPPoolReconciler) reconcileFloatingIPNetwork(ctx context.Context, scope scope.Scope, pool *infrav1alpha1.OpenStackFloatingIPPool) error {
379381
// If the pool already has a network, we don't need to do anything
380382
if pool.Status.FloatingIPNetwork != nil {
381383
return nil
382384
}
383385

384-
networkingService, err := networking.NewService(scope)
386+
networkingService, err := networking.NewService(scope, ctx)
385387
if err != nil {
386388
return err
387389
}

0 commit comments

Comments
 (0)