Skip to content

Commit 1e0a30f

Browse files
komer3komer3
authored
Refactor reconciliation logic in Linode controllers to handle deletion cases gracefully (#709)
* Refactor reconciliation logic in Linode controllers to handle deletion cases gracefully Updated the Reconcile methods in LinodeFirewall, LinodePlacementGroup, and LinodeVPC controllers to allow for successful reconciliation when the associated cluster is not found during deletion. Added logging to indicate when a cluster is not found but the resource is being deleted, ensuring that the deletion process continues smoothly. Additionally, modified the pause logic to only check for paused conditions if the resource is not being deleted or if the cluster still exists. * Fix nil pointer dereference in buildInstanceAddrs function by adding nil check for VPC IP addresses
1 parent 2522ab5 commit 1e0a30f

File tree

4 files changed

+54
-30
lines changed

4 files changed

+54
-30
lines changed

‎internal/controller/linodefirewall_controller.go‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,13 @@ func (r *LinodeFirewallReconciler) Reconcile(ctx context.Context, req ctrl.Reque
8484
if _, ok := linodeFirewall.ObjectMeta.Labels[clusterv1.ClusterNameLabel]; ok {
8585
cluster, err = kutil.GetClusterFromMetadata(ctx, r.TracedClient(), linodeFirewall.ObjectMeta)
8686
if err != nil {
87-
log.Error(err, "failed to fetch cluster from metadata")
88-
return ctrl.Result{}, err
87+
// If we're deleting and cluster isn't found, that's okay
88+
if !linodeFirewall.DeletionTimestamp.IsZero() && apierrors.IsNotFound(err) {
89+
log.Info("Cluster not found but LinodeFirewall is being deleted, continuing with deletion")
90+
} else {
91+
log.Error(err, "failed to fetch cluster from metadata")
92+
return ctrl.Result{}, err
93+
}
8994
}
9095
}
9196
// Create the firewall scope.
@@ -103,13 +108,16 @@ func (r *LinodeFirewallReconciler) Reconcile(ctx context.Context, req ctrl.Reque
103108
return ctrl.Result{}, fmt.Errorf("failed to create cluster scope: %w", err)
104109
}
105110

106-
isPaused, _, err := paused.EnsurePausedCondition(ctx, fwScope.Client, fwScope.Cluster, fwScope.LinodeFirewall)
107-
if err != nil {
108-
return ctrl.Result{}, err
109-
}
110-
if isPaused {
111-
log.Info("linodefirewall or linked cluster is paused, skipping reconciliation")
112-
return ctrl.Result{}, nil
111+
// Only check pause if not deleting or if cluster still exists
112+
if linodeFirewall.DeletionTimestamp.IsZero() || cluster != nil {
113+
isPaused, _, err := paused.EnsurePausedCondition(ctx, fwScope.Client, fwScope.Cluster, fwScope.LinodeFirewall)
114+
if err != nil {
115+
return ctrl.Result{}, err
116+
}
117+
if isPaused {
118+
log.Info("linodefirewall or linked cluster is paused, skipping reconciliation")
119+
return ctrl.Result{}, nil
120+
}
113121
}
114122
return r.reconcile(ctx, log, fwScope)
115123
}

‎internal/controller/linodemachine_controller_helpers.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ func buildInstanceAddrs(ctx context.Context, machineScope *scope.MachineScope, i
230230

231231
// check if a node has vpc specific ip and store it
232232
for _, vpcIP := range addresses.IPv4.VPC {
233-
if *vpcIP.Address != "" {
233+
if vpcIP.Address != nil && *vpcIP.Address != "" {
234234
ips = append(ips, clusterv1.MachineAddress{
235235
Address: *vpcIP.Address,
236236
Type: clusterv1.MachineInternalIP,

‎internal/controller/linodeplacementgroup_controller.go‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import (
2828
corev1 "k8s.io/api/core/v1"
2929
apierrors "k8s.io/apimachinery/pkg/api/errors"
3030
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
31-
"k8s.io/apimachinery/pkg/runtime"
3231
utilerrors "k8s.io/apimachinery/pkg/util/errors"
3332
"k8s.io/client-go/tools/record"
3433
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -59,7 +58,6 @@ type LinodePlacementGroupReconciler struct {
5958
Recorder record.EventRecorder
6059
LinodeClientConfig scope.ClientConfig
6160
WatchFilterValue string
62-
Scheme *runtime.Scheme
6361
ReconcileTimeout time.Duration
6462
}
6563

@@ -92,8 +90,13 @@ func (r *LinodePlacementGroupReconciler) Reconcile(ctx context.Context, req ctrl
9290
if _, ok := linodeplacementgroup.ObjectMeta.Labels[clusterv1.ClusterNameLabel]; ok {
9391
cluster, err = kutil.GetClusterFromMetadata(ctx, r.TracedClient(), linodeplacementgroup.ObjectMeta)
9492
if err != nil {
95-
log.Error(err, "failed to fetch cluster from metadata")
96-
return ctrl.Result{}, err
93+
// If we're deleting and cluster isn't found, that's okay
94+
if !linodeplacementgroup.DeletionTimestamp.IsZero() && apierrors.IsNotFound(err) {
95+
log.Info("Cluster not found but LinodePlacementGroup is being deleted, continuing with deletion")
96+
} else {
97+
log.Error(err, "failed to fetch cluster from metadata")
98+
return ctrl.Result{}, err
99+
}
97100
}
98101
}
99102

@@ -111,13 +114,17 @@ func (r *LinodePlacementGroupReconciler) Reconcile(ctx context.Context, req ctrl
111114

112115
return ctrl.Result{}, fmt.Errorf("failed to create Placement Group scope: %w", err)
113116
}
114-
isPaused, _, err := paused.EnsurePausedCondition(ctx, pgScope.Client, pgScope.Cluster, pgScope.LinodePlacementGroup)
115-
if err != nil {
116-
return ctrl.Result{}, err
117-
}
118-
if isPaused {
119-
log.Info("linodeplacementgroup or linked cluster is paused, skipping reconciliation")
120-
return ctrl.Result{}, nil
117+
118+
// Only check pause if not deleting or if cluster still exists
119+
if linodeplacementgroup.DeletionTimestamp.IsZero() || cluster != nil {
120+
isPaused, _, err := paused.EnsurePausedCondition(ctx, pgScope.Client, pgScope.Cluster, pgScope.LinodePlacementGroup)
121+
if err != nil {
122+
return ctrl.Result{}, err
123+
}
124+
if isPaused {
125+
log.Info("linodeplacementgroup or linked cluster is paused, skipping reconciliation")
126+
return ctrl.Result{}, nil
127+
}
121128
}
122129

123130
return r.reconcile(ctx, log, pgScope)

‎internal/controller/linodevpc_controller.go‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,13 @@ func (r *LinodeVPCReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
9797
if _, ok := linodeVPC.ObjectMeta.Labels[clusterv1.ClusterNameLabel]; ok {
9898
cluster, err = kutil.GetClusterFromMetadata(ctx, r.TracedClient(), linodeVPC.ObjectMeta)
9999
if err != nil {
100-
log.Error(err, "failed to fetch cluster from metadata")
101-
return ctrl.Result{}, err
100+
// If we're deleting and cluster isn't found, that's okay
101+
if !linodeVPC.DeletionTimestamp.IsZero() && apierrors.IsNotFound(err) {
102+
log.Info("Cluster not found but LinodeVPC is being deleted, continuing with deletion")
103+
} else {
104+
log.Error(err, "failed to fetch cluster from metadata")
105+
return ctrl.Result{}, err
106+
}
102107
}
103108
}
104109
vpcScope, err := scope.NewVPCScope(
@@ -115,13 +120,17 @@ func (r *LinodeVPCReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
115120

116121
return ctrl.Result{}, fmt.Errorf("failed to create VPC scope: %w", err)
117122
}
118-
isPaused, _, err := paused.EnsurePausedCondition(ctx, vpcScope.Client, vpcScope.Cluster, vpcScope.LinodeVPC)
119-
if err != nil {
120-
return ctrl.Result{}, err
121-
}
122-
if isPaused {
123-
log.Info("linodeVPC or linked cluster is paused, skipping reconciliation")
124-
return ctrl.Result{}, nil
123+
124+
// Only check pause if not deleting or if cluster still exists
125+
if linodeVPC.DeletionTimestamp.IsZero() || cluster != nil {
126+
isPaused, _, err := paused.EnsurePausedCondition(ctx, vpcScope.Client, vpcScope.Cluster, vpcScope.LinodeVPC)
127+
if err != nil {
128+
return ctrl.Result{}, err
129+
}
130+
if isPaused {
131+
log.Info("linodeVPC or linked cluster is paused, skipping reconciliation")
132+
return ctrl.Result{}, nil
133+
}
125134
}
126135

127136
return r.reconcile(ctx, log, vpcScope)

0 commit comments

Comments
 (0)