Skip to content

Commit d56c058

Browse files
committed
Fix potential panic
It's possible the cloudkitty enabled field will be nil. Especially after an upgrade. Check the field before dereferencing and use false if it's nil. Use the same check for Autoscaling and Ceilometer enabled field just in case. We might be able to simplify to what's described in the TODO comment, but as noted there, the code wouldn't be 100% equivalent. And I'm not sure if there was a reason to have the code like this. I also don't want to risk destroying a working solution because of an edgecase I didn't think of.
1 parent accf975 commit d56c058

1 file changed

Lines changed: 20 additions & 3 deletions

File tree

pkg/openstack/telemetry.go

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -381,9 +381,26 @@ func ReconcileTelemetry(ctx context.Context, instance *corev1beta1.OpenStackCont
381381
instance.Spec.Telemetry.Template.CloudKitty.CloudKittyAPI.DeepCopyInto(&telemetry.Spec.CloudKitty.CloudKittyAPI.CloudKittyAPITemplateCore)
382382
instance.Spec.Telemetry.Template.CloudKitty.CloudKittyProc.DeepCopyInto(&telemetry.Spec.CloudKitty.CloudKittyProc.CloudKittyProcTemplateCore)
383383

384-
telemetry.Spec.Ceilometer.Enabled = ptr.To(*instance.Spec.Telemetry.Template.Ceilometer.Enabled)
385-
telemetry.Spec.Autoscaling.Enabled = ptr.To(*instance.Spec.Telemetry.Template.Autoscaling.Enabled)
386-
telemetry.Spec.CloudKitty.Enabled = ptr.To(*instance.Spec.Telemetry.Template.CloudKitty.Enabled)
384+
// TODO: investigate if the following could be simplified to
385+
// telemetry.Spec.<service>.Enabled = instance.Spec.Telemetry.Template.<service>.Enabled
386+
// With current implementation we essentially create a copy of the bools and point to that, so the
387+
// resulting pointers in telemetry and instance objects are different (different addresses)
388+
// but once dereferenced, they point to the same true / false value. Do we need to do that?
389+
if instance.Spec.Telemetry.Template.Ceilometer.Enabled == nil {
390+
telemetry.Spec.Ceilometer.Enabled = ptr.To(false)
391+
} else {
392+
telemetry.Spec.Ceilometer.Enabled = ptr.To(*instance.Spec.Telemetry.Template.Ceilometer.Enabled)
393+
}
394+
if instance.Spec.Telemetry.Template.Autoscaling.Enabled == nil {
395+
telemetry.Spec.Autoscaling.Enabled = ptr.To(false)
396+
} else {
397+
telemetry.Spec.Autoscaling.Enabled = ptr.To(*instance.Spec.Telemetry.Template.Autoscaling.Enabled)
398+
}
399+
if instance.Spec.Telemetry.Template.CloudKitty.Enabled == nil {
400+
telemetry.Spec.CloudKitty.Enabled = ptr.To(false)
401+
} else {
402+
telemetry.Spec.CloudKitty.Enabled = ptr.To(*instance.Spec.Telemetry.Template.CloudKitty.Enabled)
403+
}
387404

388405
telemetry.Spec.Ceilometer.CentralImage = *version.Status.ContainerImages.CeilometerCentralImage
389406
telemetry.Spec.Ceilometer.ComputeImage = *version.Status.ContainerImages.CeilometerComputeImage

0 commit comments

Comments
 (0)