Skip to content

Commit 32f2614

Browse files
committed
Fix panic due to missing memcachedInstance
We currently assign to the subCR spec memcached instance under certain conditions. We observe a panic for the subcontrollers (api/scheduler/shares) on [1] when it tries to dereference a field that is a nil pointer, and this let the operator crash without any chance to recover. If you try to patch the manilaShare backend you can easily see that. Regardless of the memcached CR config (TLS or not), we always wait in the main controller for it to be ready, and only then we unblock the rest of the reconciliation loop. For this reason, we can always assign to the underlying components the MemcachedInstance defined at the top-level because: 1. we know it exists 2. we need to pass it down to the statefulset and we do not support a different memcached instance per component (we only support inheritance from the top-level) Based on the above, the subCR parameter is not independent, and needs to always be assigned from the top-level controller. The statefulSet produces the right configuration according to the memcached instance retrieved from the namespace. This patch fixes the panic by always assigning the MemcachedInstance to the subCR template. Signed-off-by: Francesco Pantano <fpantano@redhat.com>
1 parent 614f08a commit 32f2614

File tree

1 file changed

+8
-24
lines changed

1 file changed

+8
-24
lines changed

controllers/manila_controller.go

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -728,7 +728,7 @@ func (r *ManilaReconciler) reconcileNormal(ctx context.Context, instance *manila
728728
//
729729

730730
// deploy manila-api
731-
manilaAPI, op, err := r.apiDeploymentCreateOrUpdate(ctx, instance, memcached)
731+
manilaAPI, op, err := r.apiDeploymentCreateOrUpdate(ctx, instance)
732732
if err != nil {
733733
instance.Status.Conditions.Set(condition.FalseCondition(
734734
manilav1beta1.ManilaAPIReadyCondition,
@@ -776,7 +776,7 @@ func (r *ManilaReconciler) reconcileNormal(ctx context.Context, instance *manila
776776
}
777777

778778
// Deploy ManilaScheduler
779-
manilaScheduler, op, err := r.schedulerDeploymentCreateOrUpdate(ctx, instance, memcached)
779+
manilaScheduler, op, err := r.schedulerDeploymentCreateOrUpdate(ctx, instance)
780780
if err != nil {
781781
instance.Status.Conditions.Set(condition.FalseCondition(
782782
manilav1beta1.ManilaSchedulerReadyCondition,
@@ -819,7 +819,7 @@ func (r *ManilaReconciler) reconcileNormal(ctx context.Context, instance *manila
819819
// Deploy ManilaShare
820820
var shareCondition *condition.Condition
821821
for name, share := range instance.Spec.ManilaShares {
822-
manilaShare, op, err := r.shareDeploymentCreateOrUpdate(ctx, instance, name, share, serviceLabels, memcached)
822+
manilaShare, op, err := r.shareDeploymentCreateOrUpdate(ctx, instance, name, share, serviceLabels)
823823
if err != nil {
824824
instance.Status.Conditions.Set(condition.FalseCondition(
825825
manilav1beta1.ManilaShareReadyCondition,
@@ -1094,7 +1094,7 @@ func (r *ManilaReconciler) createHashOfInputHashes(
10941094
return hash, changed, nil
10951095
}
10961096

1097-
func (r *ManilaReconciler) apiDeploymentCreateOrUpdate(ctx context.Context, instance *manilav1beta1.Manila, memcached *memcachedv1.Memcached) (*manilav1beta1.ManilaAPI, controllerutil.OperationResult, error) {
1097+
func (r *ManilaReconciler) apiDeploymentCreateOrUpdate(ctx context.Context, instance *manilav1beta1.Manila) (*manilav1beta1.ManilaAPI, controllerutil.OperationResult, error) {
10981098
deployment := &manilav1beta1.ManilaAPI{
10991099
ObjectMeta: metav1.ObjectMeta{
11001100
Name: fmt.Sprintf("%s-api", instance.Name),
@@ -1109,6 +1109,7 @@ func (r *ManilaReconciler) apiDeploymentCreateOrUpdate(ctx context.Context, inst
11091109
DatabaseHostname: instance.Status.DatabaseHostname,
11101110
TransportURLSecret: instance.Status.TransportURLSecret,
11111111
ServiceAccount: instance.RbacResourceName(),
1112+
MemcachedInstance: &instance.Spec.MemcachedInstance,
11121113
}
11131114

11141115
if apiSpec.NodeSelector == nil {
@@ -1121,12 +1122,6 @@ func (r *ManilaReconciler) apiDeploymentCreateOrUpdate(ctx context.Context, inst
11211122
apiSpec.TopologyRef = instance.Spec.TopologyRef
11221123
}
11231124

1124-
// If memcached is not present in the underlying ManilaAPI Spec,
1125-
// inherit from the top-level CR (only when MTLS is in use)
1126-
if memcached.GetMemcachedMTLSSecret() != "" {
1127-
apiSpec.MemcachedInstance = &instance.Spec.MemcachedInstance
1128-
}
1129-
11301125
op, err := controllerutil.CreateOrUpdate(ctx, r.Client, deployment, func() error {
11311126
deployment.Spec = apiSpec
11321127

@@ -1145,7 +1140,7 @@ func (r *ManilaReconciler) apiDeploymentCreateOrUpdate(ctx context.Context, inst
11451140
return deployment, op, err
11461141
}
11471142

1148-
func (r *ManilaReconciler) schedulerDeploymentCreateOrUpdate(ctx context.Context, instance *manilav1beta1.Manila, memcached *memcachedv1.Memcached) (*manilav1beta1.ManilaScheduler, controllerutil.OperationResult, error) {
1143+
func (r *ManilaReconciler) schedulerDeploymentCreateOrUpdate(ctx context.Context, instance *manilav1beta1.Manila) (*manilav1beta1.ManilaScheduler, controllerutil.OperationResult, error) {
11491144
deployment := &manilav1beta1.ManilaScheduler{
11501145
ObjectMeta: metav1.ObjectMeta{
11511146
Name: fmt.Sprintf("%s-scheduler", instance.Name),
@@ -1161,6 +1156,7 @@ func (r *ManilaReconciler) schedulerDeploymentCreateOrUpdate(ctx context.Context
11611156
TransportURLSecret: instance.Status.TransportURLSecret,
11621157
ServiceAccount: instance.RbacResourceName(),
11631158
TLS: instance.Spec.ManilaAPI.TLS.Ca,
1159+
MemcachedInstance: &instance.Spec.MemcachedInstance,
11641160
}
11651161

11661162
if schedulerSpec.NodeSelector == nil {
@@ -1173,12 +1169,6 @@ func (r *ManilaReconciler) schedulerDeploymentCreateOrUpdate(ctx context.Context
11731169
schedulerSpec.TopologyRef = instance.Spec.TopologyRef
11741170
}
11751171

1176-
// If memcached is not present in the underlying ManilaScheduler Spec,
1177-
// inherit from the top-level CR (only when MTLS is in use)
1178-
if memcached.GetMemcachedMTLSSecret() != "" {
1179-
schedulerSpec.MemcachedInstance = &instance.Spec.MemcachedInstance
1180-
}
1181-
11821172
op, err := controllerutil.CreateOrUpdate(ctx, r.Client, deployment, func() error {
11831173
deployment.Spec = schedulerSpec
11841174

@@ -1203,7 +1193,6 @@ func (r *ManilaReconciler) shareDeploymentCreateOrUpdate(
12031193
name string,
12041194
share manilav1beta1.ManilaShareTemplate,
12051195
serviceLabels map[string]string,
1206-
memcached *memcachedv1.Memcached,
12071196
) (*manilav1beta1.ManilaShare, controllerutil.OperationResult, error) {
12081197

12091198
// Add the ShareName to the ManilaShare instance as a label
@@ -1224,6 +1213,7 @@ func (r *ManilaReconciler) shareDeploymentCreateOrUpdate(
12241213
TransportURLSecret: instance.Status.TransportURLSecret,
12251214
ServiceAccount: instance.RbacResourceName(),
12261215
TLS: instance.Spec.ManilaAPI.TLS.Ca,
1216+
MemcachedInstance: &instance.Spec.MemcachedInstance,
12271217
}
12281218

12291219
if shareSpec.NodeSelector == nil {
@@ -1236,12 +1226,6 @@ func (r *ManilaReconciler) shareDeploymentCreateOrUpdate(
12361226
shareSpec.TopologyRef = instance.Spec.TopologyRef
12371227
}
12381228

1239-
// If memcached is not present in the underlying ManilaSchare Spec,
1240-
// inherit from the top-level CR (only when MTLS is in use)
1241-
if memcached.GetMemcachedMTLSSecret() != "" {
1242-
shareSpec.MemcachedInstance = &instance.Spec.MemcachedInstance
1243-
}
1244-
12451229
op, err := controllerutil.CreateOrUpdate(ctx, r.Client, deployment, func() error {
12461230
deployment.Spec = shareSpec
12471231

0 commit comments

Comments
 (0)