From 0821eed7b782faad7ea2861995fdda26ee03f820 Mon Sep 17 00:00:00 2001 From: Alan Bishop Date: Wed, 31 May 2023 10:24:01 -0400 Subject: [PATCH] Make deploying cinder-backup optional Many OpenStack deployments do not use the cinder-backup service. With this in mind, the service should not be deployed unless the number of replicas is greater than zero. Conversely, the deployment should be cleaned up (deleted) if the service is scaled down to zero replicas. --- .../backends/bases/openstack/openstack.yaml | 1 - controllers/cinder_controller.go | 89 ++++++++++++++----- 2 files changed, 67 insertions(+), 23 deletions(-) diff --git a/config/samples/backends/bases/openstack/openstack.yaml b/config/samples/backends/bases/openstack/openstack.yaml index 98c66a9e..7aed427a 100644 --- a/config/samples/backends/bases/openstack/openstack.yaml +++ b/config/samples/backends/bases/openstack/openstack.yaml @@ -38,7 +38,6 @@ spec: replicas: 1 cinderScheduler: replicas: 1 - # Can omit the cinderBackup section. Making it explicit for reference. cinderBackup: replicas: 0 manila: diff --git a/controllers/cinder_controller.go b/controllers/cinder_controller.go index 6cc4ba86..f12ead49 100644 --- a/controllers/cinder_controller.go +++ b/controllers/cinder_controller.go @@ -640,7 +640,6 @@ func (r *CinderReconciler) reconcileNormal(ctx context.Context, instance *cinder instance.Status.Conditions.Set(c) } - // TODO: These will not work without rabbit yet // deploy cinder-scheduler cinderScheduler, op, err := r.schedulerDeploymentCreateOrUpdate(ctx, instance) if err != nil { @@ -665,31 +664,50 @@ func (r *CinderReconciler) reconcileNormal(ctx context.Context, instance *cinder instance.Status.Conditions.Set(c) } - // deploy cinder-backup - cinderBackup, op, err := r.backupDeploymentCreateOrUpdate(ctx, instance) - if err != nil { - instance.Status.Conditions.Set(condition.FalseCondition( - cinderv1beta1.CinderBackupReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - cinderv1beta1.CinderBackupReadyErrorMessage, - err.Error())) - return ctrl.Result{}, err - } - if op != controllerutil.OperationResultNone { - r.Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", instance.Name, string(op))) - } + // deploy cinder-backup, but only if necessary + // + // Many OpenStack deployments don't use the cinder-backup service (it's optional), + // so there's no need to deploy it unless it's required. + var backupCondition *condition.Condition + if instance.Spec.CinderBackup.Replicas > 0 { + cinderBackup, op, err := r.backupDeploymentCreateOrUpdate(ctx, instance) + if err != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + cinderv1beta1.CinderBackupReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + cinderv1beta1.CinderBackupReadyErrorMessage, + err.Error())) + return ctrl.Result{}, err + } + if op != controllerutil.OperationResultNone { + r.Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", instance.Name, string(op))) + } - // Mirror CinderBackup status' ReadyCount to this parent CR - instance.Status.CinderBackupReadyCount = cinderBackup.Status.ReadyCount + // Mirror CinderBackup status' ReadyCount to this parent CR + instance.Status.CinderBackupReadyCount = cinderBackup.Status.ReadyCount - // Mirror CinderBackup's condition status - c = cinderBackup.Status.Conditions.Mirror(cinderv1beta1.CinderBackupReadyCondition) - if c != nil { - instance.Status.Conditions.Set(c) + // Mirror CinderBackup's condition status + backupCondition = cinderBackup.Status.Conditions.Mirror(cinderv1beta1.CinderBackupReadyCondition) + + } else { + // Clean up cinder-backup if there are no replicas + err = r.backupCleanupDeployment(ctx, instance) + if err != nil { + return ctrl.Result{}, err + } + } + + if backupCondition != nil { + // If there's a backupCondition then set that as the CinderBackupReadyCondition + instance.Status.Conditions.Set(backupCondition) + } else { + // The CinderBackup is ready, even if the service wasn't deployed. + // Using "condition.DeploymentReadyMessage" here because that is what gets mirrored + // as the message for the other Cinder children when they are successfully-deployed + instance.Status.Conditions.MarkTrue(cinderv1beta1.CinderBackupReadyCondition, condition.DeploymentReadyMessage) } - // TODO: This requires some sort of backend and rabbit, and will not work without them // deploy cinder-volumes var volumeCondition *condition.Condition for name, volume := range instance.Spec.CinderVolumes { @@ -983,6 +1001,33 @@ func (r *CinderReconciler) backupDeploymentCreateOrUpdate(ctx context.Context, i return deployment, op, err } +func (r *CinderReconciler) backupCleanupDeployment(ctx context.Context, instance *cinderv1beta1.Cinder) error { + deployment := &cinderv1beta1.CinderBackup{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("%s-backup", instance.Name), + Namespace: instance.Namespace, + }, + } + key := client.ObjectKeyFromObject(deployment) + err := r.Client.Get(ctx, key, deployment) + + if k8s_errors.IsNotFound(err) { + // Nothing to clean up + return nil + } + + if err != nil { + return fmt.Errorf("Error looking for '%s' deployment in '%s' namespace: %w", deployment.Name, instance.Namespace, err) + } + + err = r.Client.Delete(ctx, deployment) + if err != nil && !k8s_errors.IsNotFound(err) { + return fmt.Errorf("Error cleaning up %s: %w", deployment.Name, err) + } + + return nil +} + func (r *CinderReconciler) volumeDeploymentCreateOrUpdate(ctx context.Context, instance *cinderv1beta1.Cinder, name string, volTemplate cinderv1beta1.CinderVolumeTemplate) (*cinderv1beta1.CinderVolume, controllerutil.OperationResult, error) { cinderVolumeSpec := cinderv1beta1.CinderVolumeSpec{