Skip to content

Commit 2bcc921

Browse files
AshleyDumaineAshleyDumaine
committed
address prealloc complaints and switch from record to events for EventRecorder, stop abusing events
1 parent 4ecd250 commit 2bcc921

25 files changed

Lines changed: 302 additions & 219 deletions

‎api/v1alpha2/linodeobjectstoragebucket_types.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,12 @@ type LinodeObjectStorageBucketStatus struct {
8888
// +optional
8989
FailureMessage *string `json:"failureMessage,omitempty"`
9090

91+
// failureReason will be set in the event that there is a terminal problem
92+
// reconciling the Object Storage Bucket and will contain a succinct value suitable
93+
// for machine interpretation.
94+
// +optional
95+
FailureReason *string `json:"failureReason,omitempty"`
96+
9197
// hostname is the address assigned to the bucket.
9298
// +optional
9399
Hostname *string `json:"hostname,omitempty"`

‎api/v1alpha2/linodeobjectstoragekey_types.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,12 @@ type LinodeObjectStorageKeyStatus struct {
129129
// +optional
130130
FailureMessage *string `json:"failureMessage,omitempty"`
131131

132+
// failureReason will be set in the event that there is a terminal problem
133+
// reconciling the Object Storage Key and will contain a succinct value suitable
134+
// for machine interpretation.
135+
// +optional
136+
FailureReason *string `json:"failureReason,omitempty"`
137+
132138
// creationTime specifies the creation timestamp for the secret.
133139
// +optional
134140
CreationTime *metav1.Time `json:"creationTime,omitempty"`

‎api/v1alpha2/zz_generated.deepcopy.go‎

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎cloud/services/loadbalancers.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -364,13 +364,13 @@ func processAndCreateNodeBalancerNodes(ctx context.Context, ipAddress string, cl
364364
apiserverLBPort := DetermineAPIServerLBPort(clusterScope)
365365

366366
// Set the port number and NB config ID for standard ports
367-
portsToBeAdded := make([]map[string]int, 0)
367+
portsToBeAdded := make([]map[string]int, 1+len(clusterScope.LinodeCluster.Spec.Network.AdditionalPorts))
368368
standardPort := map[string]int{"configID": *clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID, "port": apiserverLBPort}
369-
portsToBeAdded = append(portsToBeAdded, standardPort)
369+
portsToBeAdded[0] = standardPort
370370

371371
// Set the port number and NB config ID for any additional ports
372-
for _, portConfig := range clusterScope.LinodeCluster.Spec.Network.AdditionalPorts {
373-
portsToBeAdded = append(portsToBeAdded, map[string]int{"configID": *portConfig.NodeBalancerConfigID, "port": portConfig.Port})
372+
for i, portConfig := range clusterScope.LinodeCluster.Spec.Network.AdditionalPorts {
373+
portsToBeAdded[i+1] = map[string]int{"configID": *portConfig.NodeBalancerConfigID, "port": portConfig.Port}
374374
}
375375

376376
// Cycle through all ports to be added

‎cmd/main.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ func setupControllers(mgr manager.Manager, flags flagVars, linodeClientConfig, d
281281
// LinodeCluster Controller
282282
if err := (&controller.LinodeClusterReconciler{
283283
Client: mgr.GetClient(),
284-
Recorder: mgr.GetEventRecorderFor("LinodeClusterReconciler"),
284+
Recorder: mgr.GetEventRecorder("LinodeClusterReconciler"),
285285
WatchFilterValue: flags.clusterWatchFilter,
286286
LinodeClientConfig: linodeClientConfig,
287287
DnsClientConfig: dnsConfig,
@@ -298,7 +298,7 @@ func setupControllers(mgr manager.Manager, flags flagVars, linodeClientConfig, d
298298
// LinodeMachine Controller
299299
if err := (&controller.LinodeMachineReconciler{
300300
Client: mgr.GetClient(),
301-
Recorder: mgr.GetEventRecorderFor("LinodeMachineReconciler"),
301+
Recorder: mgr.GetEventRecorder("LinodeMachineReconciler"),
302302
WatchFilterValue: flags.machineWatchFilter,
303303
LinodeClientConfig: linodeClientConfig,
304304
GzipCompressionEnabled: useGzip,
@@ -310,7 +310,7 @@ func setupControllers(mgr manager.Manager, flags flagVars, linodeClientConfig, d
310310
// LinodeVPC Controller
311311
if err := (&controller.LinodeVPCReconciler{
312312
Client: mgr.GetClient(),
313-
Recorder: mgr.GetEventRecorderFor("LinodeVPCReconciler"),
313+
Recorder: mgr.GetEventRecorder("LinodeVPCReconciler"),
314314
LinodeClientConfig: linodeClientConfig,
315315
WatchFilterValue: flags.clusterWatchFilter,
316316
}).SetupWithManager(mgr, crcontroller.Options{MaxConcurrentReconciles: flags.linodeVPCConcurrency}); err != nil {
@@ -322,7 +322,7 @@ func setupControllers(mgr manager.Manager, flags flagVars, linodeClientConfig, d
322322
if err := (&controller.LinodeObjectStorageBucketReconciler{
323323
Client: mgr.GetClient(),
324324
Logger: ctrl.Log.WithName("LinodeObjectStorageBucketReconciler"),
325-
Recorder: mgr.GetEventRecorderFor("LinodeObjectStorageBucketReconciler"),
325+
Recorder: mgr.GetEventRecorder("LinodeObjectStorageBucketReconciler"),
326326
WatchFilterValue: flags.objectStorageBucketWatchFilter,
327327
LinodeClientConfig: linodeClientConfig,
328328
}).SetupWithManager(mgr, crcontroller.Options{MaxConcurrentReconciles: flags.linodeObjectStorageBucketConcurrency}); err != nil {
@@ -333,7 +333,7 @@ func setupControllers(mgr manager.Manager, flags flagVars, linodeClientConfig, d
333333
// LinodePlacementGroup Controller
334334
if err := (&controller.LinodePlacementGroupReconciler{
335335
Client: mgr.GetClient(),
336-
Recorder: mgr.GetEventRecorderFor("LinodePlacementGroupReconciler"),
336+
Recorder: mgr.GetEventRecorder("LinodePlacementGroupReconciler"),
337337
WatchFilterValue: flags.clusterWatchFilter,
338338
LinodeClientConfig: linodeClientConfig,
339339
}).SetupWithManager(mgr, crcontroller.Options{MaxConcurrentReconciles: flags.linodePlacementGroupConcurrency}); err != nil {
@@ -345,7 +345,7 @@ func setupControllers(mgr manager.Manager, flags flagVars, linodeClientConfig, d
345345
if err := (&controller.LinodeObjectStorageKeyReconciler{
346346
Client: mgr.GetClient(),
347347
Logger: ctrl.Log.WithName("LinodeObjectStorageKeyReconciler"),
348-
Recorder: mgr.GetEventRecorderFor("LinodeObjectStorageKeyReconciler"),
348+
Recorder: mgr.GetEventRecorder("LinodeObjectStorageKeyReconciler"),
349349
WatchFilterValue: flags.objectStorageKeyWatchFilter,
350350
LinodeClientConfig: linodeClientConfig,
351351
}).SetupWithManager(mgr, crcontroller.Options{MaxConcurrentReconciles: flags.linodeObjectStorageBucketConcurrency}); err != nil {
@@ -356,7 +356,7 @@ func setupControllers(mgr manager.Manager, flags flagVars, linodeClientConfig, d
356356
// LinodeFirewall Controller
357357
if err := (&controller.LinodeFirewallReconciler{
358358
Client: mgr.GetClient(),
359-
Recorder: mgr.GetEventRecorderFor("LinodeFirewallReconciler"),
359+
Recorder: mgr.GetEventRecorder("LinodeFirewallReconciler"),
360360
WatchFilterValue: flags.clusterWatchFilter,
361361
LinodeClientConfig: linodeClientConfig,
362362
}).SetupWithManager(mgr, crcontroller.Options{MaxConcurrentReconciles: flags.linodeFirewallConcurrency}); err != nil {

‎config/crd/bases/infrastructure.cluster.x-k8s.io_linodeobjectstoragebuckets.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,12 @@ spec:
222222
reconciling the Object Storage Bucket and will contain a verbose string
223223
suitable for logging and human consumption.
224224
type: string
225+
failureReason:
226+
description: |-
227+
failureReason will be set in the event that there is a terminal problem
228+
reconciling the Object Storage Bucket and will contain a succinct value suitable
229+
for machine interpretation.
230+
type: string
225231
hostname:
226232
description: hostname is the address assigned to the bucket.
227233
type: string

‎config/crd/bases/infrastructure.cluster.x-k8s.io_linodeobjectstoragekeys.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,12 @@ spec:
238238
reconciling the Object Storage Key and will contain a verbose string
239239
suitable for logging and human consumption.
240240
type: string
241+
failureReason:
242+
description: |-
243+
failureReason will be set in the event that there is a terminal problem
244+
reconciling the Object Storage Key and will contain a succinct value suitable
245+
for machine interpretation.
246+
type: string
241247
lastKeyGeneration:
242248
description: lastKeyGeneration tracks the last known value of .spec.keyGeneration.
243249
type: integer

‎docs/src/reference/out.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -919,6 +919,7 @@ _Appears in:_
919919
| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.32/#condition-v1-meta) array_ | conditions define the current service state of the LinodeObjectStorageBucket. | | |
920920
| `ready` _boolean_ | ready denotes that the bucket has been provisioned along with access keys. | false | |
921921
| `failureMessage` _string_ | failureMessage will be set in the event that there is a terminal problem<br />reconciling the Object Storage Bucket and will contain a verbose string<br />suitable for logging and human consumption. | | |
922+
| `failureReason` _string_ | failureReason will be set in the event that there is a terminal problem<br />reconciling the Object Storage Bucket and will contain a succinct value suitable<br />for machine interpretation. | | |
922923
| `hostname` _string_ | hostname is the address assigned to the bucket. | | |
923924
| `creationTime` _[Time](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.32/#time-v1-meta)_ | creationTime specifies the creation timestamp for the bucket. | | |
924925

@@ -1002,6 +1003,7 @@ _Appears in:_
10021003
| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.32/#condition-v1-meta) array_ | conditions define the current service state of the LinodeObjectStorageKey. | | |
10031004
| `ready` _boolean_ | ready denotes that the key has been provisioned. | false | |
10041005
| `failureMessage` _string_ | failureMessage will be set in the event that there is a terminal problem<br />reconciling the Object Storage Key and will contain a verbose string<br />suitable for logging and human consumption. | | |
1006+
| `failureReason` _string_ | failureReason will be set in the event that there is a terminal problem<br />reconciling the Object Storage Key and will contain a succinct value suitable<br />for machine interpretation. | | |
10051007
| `creationTime` _[Time](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.32/#time-v1-meta)_ | creationTime specifies the creation timestamp for the secret. | | |
10061008
| `lastKeyGeneration` _integer_ | lastKeyGeneration tracks the last known value of .spec.keyGeneration. | | |
10071009
| `accessKeyRef` _integer_ | accessKeyRef stores the ID for Object Storage key provisioned. | | |

‎internal/controller/linodecluster_controller.go‎

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,15 @@ import (
2020
"context"
2121
"errors"
2222
"fmt"
23+
corev1 "k8s.io/api/core/v1"
2324
"net/http"
2425
"time"
2526

2627
"github.com/go-logr/logr"
27-
corev1 "k8s.io/api/core/v1"
2828
apierrors "k8s.io/apimachinery/pkg/api/errors"
2929
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3030
utilerrors "k8s.io/apimachinery/pkg/util/errors"
31-
"k8s.io/client-go/tools/record"
31+
"k8s.io/client-go/tools/events"
3232
clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
3333
kutil "sigs.k8s.io/cluster-api/util"
3434
"sigs.k8s.io/cluster-api/util/predicates"
@@ -58,7 +58,7 @@ const (
5858
// LinodeClusterReconciler reconciles a LinodeCluster object
5959
type LinodeClusterReconciler struct {
6060
client.Client
61-
Recorder record.EventRecorder
61+
Recorder events.EventRecorder
6262
LinodeClientConfig scope.ClientConfig
6363
DnsClientConfig scope.ClientConfig
6464
WatchFilterValue string
@@ -182,19 +182,18 @@ func (r *LinodeClusterReconciler) reconcile(
182182
// Create
183183
if clusterScope.LinodeCluster.Spec.ControlPlaneEndpoint.Host == "" {
184184
if err := r.reconcileCreate(ctx, logger, clusterScope); err != nil {
185-
if !reconciler.HasStaleCondition(clusterScope.LinodeCluster.GetCondition(string(clusterv1.ReadyCondition)),
185+
if !reconciler.HasStaleCondition(clusterScope.LinodeCluster.GetCondition(clusterv1.ReadyCondition),
186186
reconciler.DefaultTimeout(r.ReconcileTimeout, reconciler.DefaultClusterControllerReconcileTimeout)) {
187187
logger.Info("re-queuing cluster/load-balancer creation")
188188
return ctrl.Result{RequeueAfter: reconciler.DefaultClusterControllerReconcileDelay}, nil
189189
}
190190
return res, err
191191
}
192-
r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "Load balancer is ready")
193192
}
194193

195194
clusterScope.LinodeCluster.Status.Ready = true
196195
clusterScope.LinodeCluster.SetCondition(metav1.Condition{
197-
Type: string(clusterv1.ReadyCondition),
196+
Type: clusterv1.ReadyCondition,
198197
Status: metav1.ConditionTrue,
199198
Reason: "LoadBalancerReady", // We have to set the reason to not fail object patching
200199
})
@@ -311,8 +310,6 @@ func (r *LinodeClusterReconciler) reconcilePreflightLinodeFirewallCheck(ctx cont
311310
return ctrl.Result{RequeueAfter: reconciler.DefaultClusterControllerReconcileDelay}, nil
312311
}
313312

314-
r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "Linode firewall is now available")
315-
316313
// Only set to true if there was no error
317314
clusterScope.LinodeCluster.SetCondition(metav1.Condition{
318315
Type: ConditionPreflightLinodeNBFirewallReady,
@@ -350,7 +347,6 @@ func (r *LinodeClusterReconciler) reconcilePreflightLinodeVPCCheck(ctx context.C
350347
})
351348
return ctrl.Result{RequeueAfter: reconciler.DefaultClusterControllerReconcileDelay}, nil
352349
}
353-
r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), fmt.Sprintf("VPC with ID %d is available", vpcID))
354350

355351
// Only set to true if there was no error
356352
clusterScope.LinodeCluster.SetCondition(metav1.Condition{
@@ -401,7 +397,6 @@ func (r *LinodeClusterReconciler) reconcilePreflightLinodeVPCCheck(ctx context.C
401397
})
402398
return ctrl.Result{RequeueAfter: reconciler.DefaultClusterControllerReconcileDelay}, nil
403399
}
404-
r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "LinodeVPC is now available")
405400

406401
// Only set to true if there was no error
407402
clusterScope.LinodeCluster.SetCondition(metav1.Condition{
@@ -412,24 +407,22 @@ func (r *LinodeClusterReconciler) reconcilePreflightLinodeVPCCheck(ctx context.C
412407
return ctrl.Result{}, nil
413408
}
414409

415-
func setFailureReason(clusterScope *scope.ClusterScope, failureReason string, err error, lcr *LinodeClusterReconciler) {
410+
func (r *LinodeClusterReconciler) setFailureReason(clusterScope *scope.ClusterScope, failureReason, failureMessage string) {
416411
clusterScope.LinodeCluster.Status.FailureReason = util.Pointer(failureReason)
417-
clusterScope.LinodeCluster.Status.FailureMessage = util.Pointer(err.Error())
412+
clusterScope.LinodeCluster.Status.FailureMessage = util.Pointer(failureMessage)
418413

419414
clusterScope.LinodeCluster.SetCondition(metav1.Condition{
420-
Type: string(clusterv1.ReadyCondition),
415+
Type: clusterv1.ReadyCondition,
421416
Status: metav1.ConditionFalse,
422417
Reason: failureReason,
423-
Message: err.Error(),
418+
Message: failureMessage,
424419
})
425-
426-
lcr.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeWarning, failureReason, err.Error())
427420
}
428421

429422
func (r *LinodeClusterReconciler) reconcileCreate(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error {
430423
if err := clusterScope.AddCredentialsRefFinalizer(ctx); err != nil {
431424
logger.Error(err, "failed to update credentials finalizer")
432-
setFailureReason(clusterScope, util.CreateError, err, r)
425+
r.setFailureReason(clusterScope, util.CreateError, err.Error())
433426
return err
434427
}
435428

@@ -456,7 +449,6 @@ func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger lo
456449
Reason: clusterv1.DeletionCompletedReason,
457450
Message: "Deletion in progress",
458451
})
459-
r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeWarning, "LoadBalancing managed externally", "LoadBalancing managed externally, nothing to do.")
460452

461453
case clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == lbTypeDNS:
462454
if err := removeMachineFromDNS(ctx, logger, clusterScope); err != nil {
@@ -468,11 +460,9 @@ func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger lo
468460
Reason: clusterv1.DeletionCompletedReason,
469461
Message: "Load balancing for Type DNS deleted",
470462
})
471-
r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, clusterv1.DeletionCompletedReason, "Load balancing for Type DNS deleted")
472463

473464
case clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == lbTypeNB && clusterScope.LinodeCluster.Spec.Network.NodeBalancerID == nil:
474465
logger.Info("NodeBalancer ID is missing for Type NodeBalancer, nothing to do")
475-
r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeWarning, "NodeBalancerIDMissing", "NodeBalancer already removed, nothing to do")
476466

477467
case clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == lbTypeNB && clusterScope.LinodeCluster.Spec.Network.NodeBalancerID != nil:
478468
if err := removeMachineFromNB(ctx, logger, clusterScope); err != nil {
@@ -482,7 +472,15 @@ func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger lo
482472
err := clusterScope.LinodeClient.DeleteNodeBalancer(ctx, *clusterScope.LinodeCluster.Spec.Network.NodeBalancerID)
483473
if util.IgnoreLinodeAPIError(err, http.StatusNotFound) != nil {
484474
logger.Error(err, "failed to delete NodeBalancer")
485-
setFailureReason(clusterScope, util.DeleteError, err, r)
475+
r.setFailureReason(clusterScope, util.DeleteError, err.Error())
476+
r.Recorder.Eventf(
477+
clusterScope.LinodeCluster,
478+
nil,
479+
corev1.EventTypeWarning,
480+
util.DeleteError,
481+
"DeleteNodeBalancer",
482+
err.Error(),
483+
)
486484
return err
487485
}
488486

@@ -492,7 +490,6 @@ func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger lo
492490
Reason: clusterv1.DeletionCompletedReason,
493491
Message: "Load balancer for Type NodeBalancer deleted",
494492
})
495-
r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, clusterv1.DeletionCompletedReason, "Load balancer for Type NodeBalancer deleted")
496493

497494
clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = nil
498495
clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID = nil
@@ -506,7 +503,7 @@ func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger lo
506503

507504
if err := clusterScope.RemoveCredentialsRefFinalizer(ctx); err != nil {
508505
logger.Error(err, "failed to remove credentials finalizer")
509-
setFailureReason(clusterScope, util.DeleteError, err, r)
506+
r.setFailureReason(clusterScope, util.DeleteError, err.Error())
510507
return err
511508
}
512509
controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1alpha2.ClusterFinalizer)

‎internal/controller/linodecluster_controller_test.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,6 @@ var _ = Describe("cluster-delete", Ordered, Label("cluster", "cluster-delete"),
557557
reconciler.Client = mck.K8sClient
558558
err := reconciler.reconcileDelete(ctx, logr.Logger{}, cScope)
559559
Expect(err).NotTo(HaveOccurred())
560-
Expect(mck.Events()).To(ContainSubstring("Warning NodeBalancerIDMissing NodeBalancer already removed, nothing to do"))
561560
}),
562561
),
563562
Path(

0 commit comments

Comments
 (0)