From 8b7e69e4575e3a1f33f69ddaa2bd1b0396ac5875 Mon Sep 17 00:00:00 2001
From: Carina Antunes <carinadeoliveiraantunes@gmail.com>
Date: Fri, 21 Oct 2022 11:20:49 +0200
Subject: [PATCH 1/2] remove redudant conditions. fix return from
ensurespecfinalizer
---
controllers/drupalsite_controller.go | 2 +-
controllers/drupalsite_controller_utils.go | 20 +++++++++++---------
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/controllers/drupalsite_controller.go b/controllers/drupalsite_controller.go
index b97ffea5..77f9dabd 100644
--- a/controllers/drupalsite_controller.go
+++ b/controllers/drupalsite_controller.go
@@ -259,7 +259,7 @@ func (r *DrupalSiteReconciler) Reconcile(ctx context.Context, req ctrl.Request)
// Set Current version
if drupalSite.Status.ReleaseID.Current != releaseID(drupalSite) {
drupalSite.Status.ReleaseID.Current = releaseID(drupalSite)
- update = true || update
+ update = true
}
// Check if the drupal site is ready to serve requests
diff --git a/controllers/drupalsite_controller_utils.go b/controllers/drupalsite_controller_utils.go
index 94118d4f..9482d552 100644
--- a/controllers/drupalsite_controller_utils.go
+++ b/controllers/drupalsite_controller_utils.go
@@ -178,26 +178,27 @@ func (r *DrupalSiteReconciler) ensureSpecFinalizer(ctx context.Context, drp *web
if !controllerutil.ContainsFinalizer(drp, finalizerStr) {
log.V(3).Info("Adding finalizer")
controllerutil.AddFinalizer(drp, finalizerStr)
- update = true || update
+ update = true
}
if drp.Spec.Configuration.WebDAVPassword == "" {
drp.Spec.Configuration.WebDAVPassword = generateRandomPassword()
- update = true || update
+ update = true
}
// Set default value for DiskSize to 2000Mi
- if drp.Spec.Configuration.CloneFrom == "" && drp.Spec.Configuration.DiskSize == "" {
+ if drp.Spec.Configuration.DiskSize == "" {
drp.Spec.Configuration.DiskSize = "2000Mi"
- update = true || update
+ update = true
}
+
// Validate that CloneFrom is an existing DrupalSite
if drp.Spec.Configuration.CloneFrom != "" {
sourceSite := webservicesv1a1.DrupalSite{}
err := r.Get(ctx, types.NamespacedName{Name: string(drp.Spec.Configuration.CloneFrom), Namespace: drp.Namespace}, &sourceSite)
switch {
case k8sapierrors.IsNotFound(err):
- return update, newApplicationError(fmt.Errorf("CloneFrom DrupalSite doesn't exist"), ErrInvalidSpec)
+ return false, newApplicationError(fmt.Errorf("CloneFrom DrupalSite doesn't exist"), ErrInvalidSpec)
case err != nil:
- return update, newApplicationError(err, ErrClientK8s)
+ return false, newApplicationError(err, ErrClientK8s)
}
// The destination disk size must be at least as large as the source
if drp.Spec.Configuration.DiskSize < sourceSite.Spec.Configuration.DiskSize {
@@ -213,8 +214,9 @@ func (r *DrupalSiteReconciler) ensureSpecFinalizer(ctx context.Context, drp *web
drp.Spec.Configuration.ExtraConfigurationRepository.Branch = sourceSite.Spec.Configuration.ExtraConfigurationRepository.Branch
drp.Spec.Configuration.ExtraConfigurationRepository.RepositoryUrl = sourceSite.Spec.Configuration.ExtraConfigurationRepository.RepositoryUrl
}
- update = true || update
+ // if it's a clone it should not change update, to allow continuing with the deployment
}
+
// Initialize 'spec.version.releaseSpec' if empty
if len(drp.Spec.Version.ReleaseSpec) == 0 {
// Fetch the SupportedDrupalVersions instance
@@ -237,14 +239,14 @@ func (r *DrupalSiteReconciler) ensureSpecFinalizer(ctx context.Context, drp *web
for _, v := range supportedDrupalVersions.Status.AvailableVersions {
if drp.Spec.Name == v.Name {
drp.Spec.Version.ReleaseSpec = v.LatestReleaseSpec
- update = true || update
+ update = true
break
}
}
// If no available ReleaseSpec, we log and don't update
if drp.Spec.Version.ReleaseSpec == "" {
log.V(3).Info(fmt.Sprintf("Failed to get a ReleaseSpec for version %s", drp.Spec.Version.Name))
- return update, nil
+ return false, nil
}
}
return update, nil
--
GitLab
From dced581da30d4b5b052801035543530fd39e51af Mon Sep 17 00:00:00 2001
From: Rajula Vineet Reddy <rajula.vineet.reddy@cern.ch>
Date: Fri, 21 Oct 2022 11:50:39 +0200
Subject: [PATCH 2/2] Ensure update is true when modiyfing CR in
ensureSpecFinalizer
---
controllers/drupalsite_controller_utils.go | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/controllers/drupalsite_controller_utils.go b/controllers/drupalsite_controller_utils.go
index 9482d552..bcd107b4 100644
--- a/controllers/drupalsite_controller_utils.go
+++ b/controllers/drupalsite_controller_utils.go
@@ -175,6 +175,7 @@ func (r *DrupalSiteReconciler) cleanupDrupalSite(ctx context.Context, log logr.L
// ensureSpecFinalizer ensures that the spec is valid, adding extra info if necessary, and that the finalizer is there,
// then returns if it needs to be updated.
func (r *DrupalSiteReconciler) ensureSpecFinalizer(ctx context.Context, drp *webservicesv1a1.DrupalSite, log logr.Logger) (update bool, err reconcileError) {
+ // We want the update variable to be true, only when we make changes to the CR & want it to be updated
if !controllerutil.ContainsFinalizer(drp, finalizerStr) {
log.V(3).Info("Adding finalizer")
controllerutil.AddFinalizer(drp, finalizerStr)
@@ -203,18 +204,20 @@ func (r *DrupalSiteReconciler) ensureSpecFinalizer(ctx context.Context, drp *web
// The destination disk size must be at least as large as the source
if drp.Spec.Configuration.DiskSize < sourceSite.Spec.Configuration.DiskSize {
drp.Spec.Configuration.DiskSize = sourceSite.Spec.Configuration.DiskSize
+ update = true
}
// The extraConfigurationRepo should be set in the clone site if defined in the source
// TODO: Remove logic for ExtraConfigurationRepo once we deprecate the field
if sourceSite.Spec.Configuration.ExtraConfigurationRepo != "" && drp.Spec.Configuration.ExtraConfigurationRepo == "" {
drp.Spec.Configuration.ExtraConfigurationRepo = sourceSite.Spec.Configuration.ExtraConfigurationRepo
+ update = true
}
// The extraConfigurationRepository should be set in the clone site if defined in the source
if sourceSite.Spec.Configuration.ExtraConfigurationRepository.Branch != "" && sourceSite.Spec.Configuration.ExtraConfigurationRepository.RepositoryUrl != "" && drp.Spec.Configuration.ExtraConfigurationRepository.Branch == "" && drp.Spec.Configuration.ExtraConfigurationRepository.RepositoryUrl != "" {
drp.Spec.Configuration.ExtraConfigurationRepository.Branch = sourceSite.Spec.Configuration.ExtraConfigurationRepository.Branch
drp.Spec.Configuration.ExtraConfigurationRepository.RepositoryUrl = sourceSite.Spec.Configuration.ExtraConfigurationRepository.RepositoryUrl
+ update = true
}
- // if it's a clone it should not change update, to allow continuing with the deployment
}
// Initialize 'spec.version.releaseSpec' if empty
@@ -233,7 +236,7 @@ func (r *DrupalSiteReconciler) ensureSpecFinalizer(ctx context.Context, drp *web
}
// Error reading the object - fail the request.
log.Error(err, fmt.Sprintf("Failed to get SupportedDrupalVersions '%s'", SupportedDrupalVersionName))
- return update, newApplicationError(err, ErrClientK8s)
+ return false, newApplicationError(err, ErrClientK8s)
}
// Iterate over available versions to find if the one requested has a ReleaseSpec
for _, v := range supportedDrupalVersions.Status.AvailableVersions {
--
GitLab