Use common variable for durations and requeue

We have a number of places where we are using 5 seconds durations, 10 seconds duration, and requeue after 10 seconds. These are all "magic numbers" spread all over the operator code. This patch creates 2 constants and 1 variable (because golang doesn't allow it to be defined as a constant) for these so that they can be used throughout the code having defined the "magic numbers" in a single location.
openstack-k8s-operators · Sep 3, 2024 · 7f0c780 · 7f0c780
1 parent 534075b
commit 7f0c780
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 49 deletions.
diff --git a/controllers/cinder_controller.go b/controllers/cinder_controller.go
@@ -396,7 +396,7 @@ func (r *CinderReconciler) reconcileInit(
 		jobDef,
 		cinderv1beta1.DbSyncHash,
 		instance.Spec.PreserveJobs,
-		time.Duration(5)*time.Second,
+		cinder.ShortDuration,
 		dbSyncHash,
 	)
 	ctrlResult, err := dbSyncjob.DoJob(
@@ -492,7 +492,7 @@ func (r *CinderReconciler) reconcileNormal(ctx context.Context, instance *cinder
 			condition.RequestedReason,
 			condition.SeverityInfo,
 			condition.RabbitMqTransportURLReadyRunningMessage))
-		return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil
+		return cinder.ResultRequeue, nil
 	}
 
 	instance.Status.Conditions.MarkTrue(condition.RabbitMqTransportURLReadyCondition, condition.RabbitMqTransportURLReadyMessage)
@@ -504,14 +504,15 @@ func (r *CinderReconciler) reconcileNormal(ctx context.Context, instance *cinder
 	//
 	memcached, err := memcachedv1.GetMemcachedByName(ctx, helper, instance.Spec.MemcachedInstance, instance.Namespace)
 	if err != nil {
+		Log.Info(fmt.Sprintf("%s... requeueing", condition.MemcachedReadyWaitingMessage))
 		if k8s_errors.IsNotFound(err) {
 			Log.Info(fmt.Sprintf("memcached %s not found", instance.Spec.MemcachedInstance))
 			instance.Status.Conditions.Set(condition.FalseCondition(
 				condition.MemcachedReadyCondition,
 				condition.RequestedReason,
 				condition.SeverityInfo,
 				condition.MemcachedReadyWaitingMessage))
-			return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil
+			return cinder.ResultRequeue, nil
 		}
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			condition.MemcachedReadyCondition,
@@ -523,13 +524,13 @@ func (r *CinderReconciler) reconcileNormal(ctx context.Context, instance *cinder
 	}
 
 	if !memcached.IsReady() {
-		Log.Info(fmt.Sprintf("memcached %s is not ready", memcached.Name))
+		Log.Info(fmt.Sprintf("%s... requeueing", condition.MemcachedReadyWaitingMessage))
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			condition.MemcachedReadyCondition,
 			condition.RequestedReason,
 			condition.SeverityInfo,
 			condition.MemcachedReadyWaitingMessage))
-		return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil
+		return cinder.ResultRequeue, nil
 	}
 	// Mark the Memcached Service as Ready if we get to this point with no errors
 	instance.Status.Conditions.MarkTrue(
@@ -548,7 +549,7 @@ func (r *CinderReconciler) reconcileNormal(ctx context.Context, instance *cinder
 				condition.RequestedReason,
 				condition.SeverityInfo,
 				condition.InputReadyWaitingMessage))
-			return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil
+			return cinder.ResultRequeue, fmt.Errorf("OpenStack secret %s not found", instance.Spec.Secret)
 		}
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			condition.InputReadyCondition,
@@ -629,7 +630,7 @@ func (r *CinderReconciler) reconcileNormal(ctx context.Context, instance *cinder
 					condition.SeverityInfo,
 					condition.NetworkAttachmentsReadyWaitingMessage,
 					netAtt))
-				return ctrl.Result{RequeueAfter: time.Second * 10}, nil
+				return cinder.ResultRequeue, fmt.Errorf(condition.NetworkAttachmentsReadyWaitingMessage, netAtt)
 			}
 			instance.Status.Conditions.Set(condition.FalseCondition(
 				condition.NetworkAttachmentsReadyCondition,

diff --git a/controllers/cinderapi_controller.go b/controllers/cinderapi_controller.go
@@ -19,7 +19,6 @@ package controllers
 import (
 	"context"
 	"fmt"
-	"time"
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -569,7 +568,7 @@ func (r *CinderAPIReconciler) reconcileInit(
 			PasswordSelector:   instance.Spec.PasswordSelectors.Service,
 		}
 
-		ksSvcObj := keystonev1.NewKeystoneService(ksSvcSpec, instance.Namespace, serviceLabels, time.Duration(10)*time.Second)
+		ksSvcObj := keystonev1.NewKeystoneService(ksSvcSpec, instance.Namespace, serviceLabels, cinder.NormalDuration)
 		ctrlResult, err := ksSvcObj.CreateOrPatch(ctx, helper)
 		if err != nil {
 			instance.Status.Conditions.MarkFalse(
@@ -604,7 +603,7 @@ func (r *CinderAPIReconciler) reconcileInit(
 			instance.Namespace,
 			ksEndptSpec,
 			serviceLabels,
-			time.Duration(10)*time.Second)
+			cinder.NormalDuration)
 		ctrlResult, err = ksEndptObj.CreateOrPatch(ctx, helper)
 		if err != nil {
 			instance.Status.Conditions.MarkFalse(
@@ -781,7 +780,7 @@ func (r *CinderAPIReconciler) reconcileNormal(ctx context.Context, instance *cin
 					condition.SeverityInfo,
 					condition.NetworkAttachmentsReadyWaitingMessage,
 					netAtt))
-				return ctrl.Result{RequeueAfter: time.Second * 10}, nil
+				return cinder.ResultRequeue, fmt.Errorf("network-attachment-definition %s not found", netAtt)
 			}
 			instance.Status.Conditions.Set(condition.FalseCondition(
 				condition.NetworkAttachmentsReadyCondition,
@@ -869,10 +868,7 @@ func (r *CinderAPIReconciler) reconcileNormal(ctx context.Context, instance *cin
 			err.Error()))
 		return ctrl.Result{}, err
 	}
-	ss := statefulset.NewStatefulSet(
-		ssDef,
-		time.Duration(5)*time.Second,
-	)
+	ss := statefulset.NewStatefulSet(ssDef, cinder.ShortDuration)
 
 	var ssData appsv1.StatefulSet
 	ctrlResult, err = ss.CreateOrPatch(ctx, helper)
@@ -889,9 +885,8 @@ func (r *CinderAPIReconciler) reconcileNormal(ctx context.Context, instance *cin
 		// Wait until the data in the StatefulSet is for the current generation
 		ssData = ss.GetStatefulSet()
 		if ssData.Generation != ssData.Status.ObservedGeneration {
-			ctrlResult = ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}
-			Log.Info(fmt.Sprintf("waiting for Statefulset %s to start reconciling", ssData.Name))
-			err = nil
+			ctrlResult = cinder.ResultRequeue
+			err = fmt.Errorf("waiting for Statefulset %s to start reconciling", ssData.Name)
 		}
 	}
 
@@ -1021,7 +1016,7 @@ func (r *CinderAPIReconciler) getSecret(
 				condition.RequestedReason,
 				condition.SeverityInfo,
 				condition.InputReadyWaitingMessage))
-			return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil
+			return cinder.ResultRequeue, fmt.Errorf("Secret %s not found", secretName)
 		}
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			condition.InputReadyCondition,

diff --git a/controllers/cinderbackup_controller.go b/controllers/cinderbackup_controller.go
@@ -19,7 +19,6 @@ package controllers
 import (
 	"context"
 	"fmt"
-	"time"
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -485,7 +484,7 @@ func (r *CinderBackupReconciler) reconcileNormal(ctx context.Context, instance *
 					condition.SeverityInfo,
 					condition.NetworkAttachmentsReadyWaitingMessage,
 					netAtt))
-				return ctrl.Result{RequeueAfter: time.Second * 10}, nil
+				return cinder.ResultRequeue, fmt.Errorf("network-attachment-definition %s not found", netAtt)
 			}
 			instance.Status.Conditions.Set(condition.FalseCondition(
 				condition.NetworkAttachmentsReadyCondition,
@@ -539,10 +538,7 @@ func (r *CinderBackupReconciler) reconcileNormal(ctx context.Context, instance *
 
 	// Deploy a statefulset
 	ssDef := cinderbackup.StatefulSet(instance, inputHash, serviceLabels, serviceAnnotations)
-	ss := statefulset.NewStatefulSet(
-		ssDef,
-		time.Duration(5)*time.Second,
-	)
+	ss := statefulset.NewStatefulSet(ssDef, cinder.ShortDuration)
 
 	var ssData appsv1.StatefulSet
 	ctrlResult, err = ss.CreateOrPatch(ctx, helper)
@@ -559,9 +555,8 @@ func (r *CinderBackupReconciler) reconcileNormal(ctx context.Context, instance *
 		// Wait until the data in the StatefulSet is for the current generation
 		ssData = ss.GetStatefulSet()
 		if ssData.Generation != ssData.Status.ObservedGeneration {
-			ctrlResult = ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}
-			Log.Info(fmt.Sprintf("waiting for Statefulset %s to start reconciling", ssData.Name))
-			err = nil
+			ctrlResult = cinder.ResultRequeue
+			err = fmt.Errorf("waiting for Statefulset %s to start reconciling", ssData.Name)
 		}
 	}
 
@@ -690,7 +685,7 @@ func (r *CinderBackupReconciler) getSecret(
 				condition.RequestedReason,
 				condition.SeverityInfo,
 				condition.InputReadyWaitingMessage))
-			return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil
+			return cinder.ResultRequeue, fmt.Errorf("Secret %s not found", secretName)
 		}
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			condition.InputReadyCondition,

diff --git a/controllers/cinderscheduler_controller.go b/controllers/cinderscheduler_controller.go
@@ -19,7 +19,6 @@ package controllers
 import (
 	"context"
 	"fmt"
-	"time"
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -484,7 +483,7 @@ func (r *CinderSchedulerReconciler) reconcileNormal(ctx context.Context, instanc
 					condition.SeverityInfo,
 					condition.NetworkAttachmentsReadyWaitingMessage,
 					netAtt))
-				return ctrl.Result{RequeueAfter: time.Second * 10}, nil
+				return cinder.ResultRequeue, fmt.Errorf("network-attachment-definition %s not found", netAtt)
 			}
 			instance.Status.Conditions.Set(condition.FalseCondition(
 				condition.NetworkAttachmentsReadyCondition,
@@ -538,10 +537,7 @@ func (r *CinderSchedulerReconciler) reconcileNormal(ctx context.Context, instanc
 
 	// Deploy a statefulset
 	ssDef := cinderscheduler.StatefulSet(instance, inputHash, serviceLabels, serviceAnnotations)
-	ss := statefulset.NewStatefulSet(
-		ssDef,
-		time.Duration(5)*time.Second,
-	)
+	ss := statefulset.NewStatefulSet(ssDef, cinder.ShortDuration)
 
 	var ssData appsv1.StatefulSet
 	ctrlResult, err = ss.CreateOrPatch(ctx, helper)
@@ -558,9 +554,8 @@ func (r *CinderSchedulerReconciler) reconcileNormal(ctx context.Context, instanc
 		// Wait until the data in the StatefulSet is for the current generation
 		ssData = ss.GetStatefulSet()
 		if ssData.Generation != ssData.Status.ObservedGeneration {
-			ctrlResult = ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}
-			Log.Info(fmt.Sprintf("waiting for Statefulset %s to start reconciling", ssData.Name))
-			err = nil
+			ctrlResult = cinder.ResultRequeue
+			err = fmt.Errorf("waiting for Statefulset %s to start reconciling", ssData.Name)
 		}
 	}
 
@@ -689,7 +684,7 @@ func (r *CinderSchedulerReconciler) getSecret(
 				condition.RequestedReason,
 				condition.SeverityInfo,
 				condition.InputReadyWaitingMessage))
-			return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil
+			return cinder.ResultRequeue, fmt.Errorf("Secret %s not found", secretName)
 		}
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			condition.InputReadyCondition,

diff --git a/controllers/cindervolume_controller.go b/controllers/cindervolume_controller.go
@@ -20,7 +20,6 @@ import (
 	"context"
 	"fmt"
 	"strings"
-	"time"
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -487,7 +486,7 @@ func (r *CinderVolumeReconciler) reconcileNormal(ctx context.Context, instance *
 					condition.SeverityInfo,
 					condition.NetworkAttachmentsReadyWaitingMessage,
 					netAtt))
-				return ctrl.Result{RequeueAfter: time.Second * 10}, nil
+				return cinder.ResultRequeue, fmt.Errorf("network-attachment-definition %s not found", netAtt)
 			}
 			instance.Status.Conditions.Set(condition.FalseCondition(
 				condition.NetworkAttachmentsReadyCondition,
@@ -541,10 +540,7 @@ func (r *CinderVolumeReconciler) reconcileNormal(ctx context.Context, instance *
 
 	// Deploy a statefulset
 	ssDef := cindervolume.StatefulSet(instance, inputHash, serviceLabels, serviceAnnotations, usesLVM)
-	ss := statefulset.NewStatefulSet(
-		ssDef,
-		time.Duration(5)*time.Second,
-	)
+	ss := statefulset.NewStatefulSet(ssDef, cinder.ShortDuration)
 
 	var ssData appsv1.StatefulSet
 	ctrlResult, err = ss.CreateOrPatch(ctx, helper)
@@ -561,9 +557,8 @@ func (r *CinderVolumeReconciler) reconcileNormal(ctx context.Context, instance *
 		// Wait until the data in the StatefulSet is for the current generation
 		ssData = ss.GetStatefulSet()
 		if ssData.Generation != ssData.Status.ObservedGeneration {
-			ctrlResult = ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}
-			Log.Info(fmt.Sprintf("waiting for Statefulset %s to start reconciling", ssData.Name))
-			err = nil
+			ctrlResult = cinder.ResultRequeue
+			err = fmt.Errorf("waiting for Statefulset %s to start reconciling", ssData.Name)
 		}
 	}
 
@@ -692,7 +687,7 @@ func (r *CinderVolumeReconciler) getSecret(
 				condition.RequestedReason,
 				condition.SeverityInfo,
 				condition.InputReadyWaitingMessage))
-			return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil
+			return cinder.ResultRequeue, fmt.Errorf("Secret %s not found", secretName)
 		}
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			condition.InputReadyCondition,

diff --git a/pkg/cinder/const.go b/pkg/cinder/const.go
@@ -16,6 +16,10 @@ limitations under the License.
 package cinder
 
 import (
+	"time"
+
+	ctrl "sigs.k8s.io/controller-runtime"
+
 	"github.com/openstack-k8s-operators/lib-common/modules/storage"
 )
 
@@ -66,8 +70,13 @@ const (
 	// Cinder is the global ServiceType that refers to all the components deployed
 	// by the cinder operator
 	Cinder storage.PropagationType = "Cinder"
+
+	ShortDuration  = time.Duration(5) * time.Second
+	NormalDuration = time.Duration(10) * time.Second
 )
 
+var ResultRequeue = ctrl.Result{RequeueAfter: NormalDuration}
+
 // DbsyncPropagation keeps track of the DBSync Service Propagation Type
 var DbsyncPropagation = []storage.PropagationType{storage.DBSync}