Skip to content

Commit 2d67091

Browse files
NiccoloFeiarmru
andauthored
fix: avoid panic while reconciling the Pooler reconciler when its Cluster is deleted (cloudnative-pg#10667)
Fixing a `nil pointer` that would happen when reconciling a Pooler while its Cluster has been deleted. Closes cloudnative-pg#10665 Signed-off-by: Niccolò Fei <niccolo.fei@enterprisedb.com> Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com> Co-authored-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
1 parent c9ea3aa commit 2d67091

3 files changed

Lines changed: 45 additions & 17 deletions

File tree

internal/controller/pooler_controller.go

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,21 @@ func (r *PoolerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
9494
return ctrl.Result{}, nil
9595
}
9696

97+
// Resolve the referenced Cluster up front: it gates the rest of the
98+
// reconciliation and is required by getManagedResources.
99+
cluster, err := getClusterOrNil(
100+
ctx, r.Client, client.ObjectKey{Name: pooler.Spec.Cluster.Name, Namespace: pooler.Namespace})
101+
if err != nil {
102+
return ctrl.Result{}, fmt.Errorf("while getting referenced cluster: %w", err)
103+
}
104+
if cluster == nil {
105+
contextLogger.Info("Cluster not found, will retry in 30 seconds",
106+
"cluster", pooler.Spec.Cluster.Name)
107+
return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
108+
}
109+
97110
// Get the set of resources we directly manage and their status
98-
resources, err := r.getManagedResources(ctx, &pooler)
111+
resources, err := r.getManagedResources(ctx, &pooler, cluster)
99112
if err != nil {
100113
return ctrl.Result{}, fmt.Errorf("while getting managed resources: %w", err)
101114
}
@@ -220,12 +233,6 @@ func (r *PoolerReconciler) waitForPrerequisites(
220233
contextLogger := log.FromContext(ctx)
221234
waitResult := &ctrl.Result{RequeueAfter: 30 * time.Second}
222235

223-
if resources.Cluster == nil {
224-
contextLogger.Info("Cluster not found, will retry in 30 seconds",
225-
"cluster", pooler.Spec.Cluster.Name)
226-
return waitResult
227-
}
228-
229236
// For automated integration, we need AuthUserSecret
230237
if pooler.IsAutomatedIntegration() && resources.AuthUserSecret == nil {
231238
contextLogger.Info("AuthUserSecret not found, waiting 30 seconds",

internal/controller/pooler_controller_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ package controller
2121

2222
import (
2323
"context"
24+
"time"
2425

2526
corev1 "k8s.io/api/core/v1"
2627
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -193,6 +194,31 @@ var _ = Describe("pooler_controller unit tests", func() {
193194
})
194195
})
195196

197+
It("should requeue without panicking when the referenced cluster is missing", func() {
198+
ctx := context.Background()
199+
namespace := newFakeNamespace(env.client)
200+
201+
// cluster is intentionally not persisted: the Pooler references
202+
// a Cluster that does not exist (e.g., it has been deleted while
203+
// the Pooler still existed).
204+
cluster := &apiv1.Cluster{
205+
ObjectMeta: metav1.ObjectMeta{
206+
Name: "missing-cluster",
207+
Namespace: namespace,
208+
},
209+
}
210+
pooler := newFakePooler(env.client, cluster)
211+
212+
result, err := env.poolerReconciler.Reconcile(ctx, ctrl.Request{
213+
NamespacedName: types.NamespacedName{
214+
Name: pooler.Name,
215+
Namespace: pooler.Namespace,
216+
},
217+
})
218+
Expect(err).ToNot(HaveOccurred())
219+
Expect(result.RequeueAfter).To(Equal(30 * time.Second))
220+
})
221+
196222
It("should make sure that isOwnedByPoolerKind works correctly", func() {
197223
namespace := newFakeNamespace(env.client)
198224
cluster := newFakeCNPGCluster(env.client, namespace)

internal/controller/pooler_resources.go

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -68,20 +68,15 @@ type poolerManagedResources struct {
6868
Role *rbacv1.Role
6969
}
7070

71-
// getManagedResources detects the list of the resources created and manager
72-
// by this pooler
71+
// getManagedResources detects the list of the resources created and managed
72+
// by this pooler. The caller is responsible for resolving the referenced
73+
// Cluster and passing it in: this function requires a non-nil Cluster.
7374
func (r *PoolerReconciler) getManagedResources(
7475
ctx context.Context,
7576
pooler *apiv1.Pooler,
77+
cluster *apiv1.Cluster,
7678
) (result *poolerManagedResources, err error) {
77-
result = &poolerManagedResources{}
78-
79-
// Get the referenced cluster
80-
result.Cluster, err = getClusterOrNil(
81-
ctx, r.Client, client.ObjectKey{Name: pooler.Spec.Cluster.Name, Namespace: pooler.Namespace})
82-
if err != nil {
83-
return nil, err
84-
}
79+
result = &poolerManagedResources{Cluster: cluster}
8580

8681
// Get the auth query secret if any
8782
result.AuthUserSecret, err = getSecretOrNil(

0 commit comments

Comments
 (0)