From 5d6aed5a5c92ec816b81e61931e891cd9480fd7b Mon Sep 17 00:00:00 2001 From: Francesco Pantano Date: Wed, 27 Mar 2024 23:41:17 +0100 Subject: [PATCH] Improve logging and remove placeholder reconcile functions Signed-off-by: Francesco Pantano --- controllers/glance_controller.go | 53 +++++------------------------ controllers/glanceapi_controller.go | 49 ++++---------------------- pkg/glance/const.go | 9 +++++ 3 files changed, 25 insertions(+), 86 deletions(-) diff --git a/controllers/glance_controller.go b/controllers/glance_controller.go index ec254df8..a37109aa 100644 --- a/controllers/glance_controller.go +++ b/controllers/glance_controller.go @@ -19,7 +19,6 @@ package controllers import ( "context" "fmt" - "time" "github.com/openstack-k8s-operators/lib-common/modules/common/tls" rbacv1 "k8s.io/api/rbac/v1" @@ -109,6 +108,7 @@ func (r *GlanceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res return ctrl.Result{}, nil } // Error reading the object - requeue the request. + r.Log.Error(err, fmt.Sprintf("could not fetch Glance instance %s", instance.Name)) return ctrl.Result{}, err } @@ -120,6 +120,7 @@ func (r *GlanceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res r.Log, ) if err != nil { + r.Log.Error(err, fmt.Sprintf("could not instantiate helper for instance %s", instance.Name)) return ctrl.Result{}, err } @@ -408,7 +409,7 @@ func (r *GlanceReconciler) reconcileInit( _, _, err := oko_secret.GetSecret(ctx, helper, instance.Spec.Secret, instance.Namespace) if err != nil { if k8s_errors.IsNotFound(err) { - return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, fmt.Errorf("OpenStack secret %s not found", instance.Spec.Secret) + return glance.ResultRequeue, fmt.Errorf("OpenStack secret %s not found", instance.Spec.Secret) } return ctrl.Result{}, err } @@ -423,7 +424,7 @@ func (r *GlanceReconciler) reconcileInit( PasswordSelector: instance.Spec.PasswordSelectors.Service, } - ksSvc := keystonev1.NewKeystoneService(ksSvcSpec, instance.Namespace, serviceLabels, time.Duration(10)*time.Second) + ksSvc := keystonev1.NewKeystoneService(ksSvcSpec, instance.Namespace, serviceLabels, glance.NormalDuration) ctrlResult, err := ksSvc.CreateOrPatch(ctx, helper) if err != nil { return ctrlResult, err @@ -455,7 +456,7 @@ func (r *GlanceReconciler) reconcileInit( jobDef, glancev1.DbSyncHash, instance.Spec.PreserveJobs, - time.Duration(5)*time.Second, + glance.ShortDuration, dbSyncHash, ) ctrlResult, err = dbSyncjob.DoJob( @@ -489,26 +490,6 @@ func (r *GlanceReconciler) reconcileInit( return ctrl.Result{}, nil } -func (r *GlanceReconciler) reconcileUpdate(ctx context.Context, instance *glancev1.Glance, helper *helper.Helper) (ctrl.Result, error) { - r.Log.Info(fmt.Sprintf("Reconciling Service '%s' update", instance.Name)) - - // TODO: should have minor update tasks if required - // - delete dbsync hash from status to rerun it? - - r.Log.Info(fmt.Sprintf("Reconciled Service '%s' update successfully", instance.Name)) - return ctrl.Result{}, nil -} - -func (r *GlanceReconciler) reconcileUpgrade(ctx context.Context, instance *glancev1.Glance, helper *helper.Helper) (ctrl.Result, error) { - r.Log.Info(fmt.Sprintf("Reconciling Service '%s' upgrade", instance.Name)) - - // TODO: should have major version upgrade tasks - // -delete dbsync hash from status to rerun it? - - r.Log.Info(fmt.Sprintf("Reconciled Service '%s' upgrade successfully", instance.Name)) - return ctrl.Result{}, nil -} - func (r *GlanceReconciler) reconcileNormal(ctx context.Context, instance *glancev1.Glance, helper *helper.Helper) (ctrl.Result, error) { r.Log.Info(fmt.Sprintf("Reconciling Service '%s'", instance.Name)) @@ -552,7 +533,7 @@ func (r *GlanceReconciler) reconcileNormal(ctx context.Context, instance *glance condition.RequestedReason, condition.SeverityInfo, condition.MemcachedReadyWaitingMessage)) - return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, fmt.Errorf("memcached %s not found", instance.Spec.MemcachedInstance) + return glance.ResultRequeue, fmt.Errorf("memcached %s not found", instance.Spec.MemcachedInstance) } instance.Status.Conditions.Set(condition.FalseCondition( condition.MemcachedReadyCondition, @@ -569,7 +550,7 @@ func (r *GlanceReconciler) reconcileNormal(ctx context.Context, instance *glance condition.RequestedReason, condition.SeverityInfo, condition.MemcachedReadyWaitingMessage)) - return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, fmt.Errorf("memcached %s is not ready", memcached.Name) + return glance.ResultRequeue, fmt.Errorf("memcached %s is not ready", memcached.Name) } // Mark the Memcached Service as Ready if we get to this point with no errors instance.Status.Conditions.MarkTrue( @@ -587,7 +568,7 @@ func (r *GlanceReconciler) reconcileNormal(ctx context.Context, instance *glance condition.RequestedReason, condition.SeverityInfo, condition.InputReadyWaitingMessage)) - return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, fmt.Errorf("OpenStack secret %s not found", instance.Spec.Secret) + return glance.ResultRequeue, fmt.Errorf("OpenStack secret %s not found", instance.Spec.Secret) } instance.Status.Conditions.Set(condition.FalseCondition( condition.InputReadyCondition, @@ -646,22 +627,6 @@ func (r *GlanceReconciler) reconcileNormal(ctx context.Context, instance *glance return ctrlResult, nil } - // Handle service update - ctrlResult, err = r.reconcileUpdate(ctx, instance, helper) - if err != nil { - return ctrlResult, err - } else if (ctrlResult != ctrl.Result{}) { - return ctrlResult, nil - } - - // Handle service upgrade - ctrlResult, err = r.reconcileUpgrade(ctx, instance, helper) - if err != nil { - return ctrlResult, err - } else if (ctrlResult != ctrl.Result{}) { - return ctrlResult, nil - } - // // Reconcile the GlanceAPI deployment // @@ -1054,7 +1019,7 @@ func (r *GlanceReconciler) ensureDBPurgeJob( dbPurgeCronJob := cronjob.NewCronJob( cronjobDef, - 5*time.Second, + glance.ShortDuration, ) ctrlResult, err := dbPurgeCronJob.CreateOrPatch(ctx, h) if err != nil { diff --git a/controllers/glanceapi_controller.go b/controllers/glanceapi_controller.go index 044f234a..d8c595b5 100644 --- a/controllers/glanceapi_controller.go +++ b/controllers/glanceapi_controller.go @@ -20,7 +20,6 @@ import ( "context" "fmt" "strings" - "time" batchv1 "k8s.io/api/batch/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -104,6 +103,7 @@ func (r *GlanceAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, nil } // Error reading the object - requeue the request. + r.Log.Error(err, fmt.Sprintf("could not fetch GlanceAPI instance %s", instance.Name)) return ctrl.Result{}, err } @@ -115,6 +115,7 @@ func (r *GlanceAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( r.Log, ) if err != nil { + r.Log.Error(err, fmt.Sprintf("could not instantiate helper for instance %s", instance.Name)) return ctrl.Result{}, err } @@ -525,26 +526,6 @@ func (r *GlanceAPIReconciler) reconcileInit( return ctrl.Result{}, nil } -func (r *GlanceAPIReconciler) reconcileUpdate(ctx context.Context, instance *glancev1.GlanceAPI, helper *helper.Helper) (ctrl.Result, error) { - r.Log.Info(fmt.Sprintf("Reconciling Service '%s' update", instance.Name)) - - // TODO: should have minor update tasks if required - // - delete dbsync hash from status to rerun it? - - r.Log.Info(fmt.Sprintf("Reconciled Service '%s' update successfully", instance.Name)) - return ctrl.Result{}, nil -} - -func (r *GlanceAPIReconciler) reconcileUpgrade(ctx context.Context, instance *glancev1.GlanceAPI, helper *helper.Helper) (ctrl.Result, error) { - r.Log.Info(fmt.Sprintf("Reconciling Service '%s' upgrade", instance.Name)) - - // TODO: should have major version upgrade tasks - // -delete dbsync hash from status to rerun it? - - r.Log.Info(fmt.Sprintf("Reconciled Service '%s' upgrade successfully", instance.Name)) - return ctrl.Result{}, nil -} - func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *glancev1.GlanceAPI, helper *helper.Helper, req ctrl.Request) (ctrl.Result, error) { r.Log.Info(fmt.Sprintf("Reconciling Service '%s'", instance.Name)) @@ -563,7 +544,7 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla condition.RequestedReason, condition.SeverityInfo, condition.InputReadyWaitingMessage)) - return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, fmt.Errorf("OpenStack secret %s not found", instance.Spec.Secret) + return glance.ResultRequeue, fmt.Errorf("OpenStack secret %s not found", instance.Spec.Secret) } instance.Status.Conditions.Set(condition.FalseCondition( condition.InputReadyCondition, @@ -612,7 +593,7 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla condition.SeverityInfo, glancev1.CinderInitMessage), ) - return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil + return glance.ResultRequeue, nil } // Cinder CR is found, we can unblock glance deployment because // it represents a valid backend. @@ -743,22 +724,6 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla return ctrlResult, nil } - // Handle service update - ctrlResult, err = r.reconcileUpdate(ctx, instance, helper) - if err != nil { - return ctrlResult, err - } else if (ctrlResult != ctrl.Result{}) { - return ctrlResult, nil - } - - // Handle service upgrade - ctrlResult, err = r.reconcileUpgrade(ctx, instance, helper) - if err != nil { - return ctrlResult, err - } else if (ctrlResult != ctrl.Result{}) { - return ctrlResult, nil - } - // // normal reconcile tasks // @@ -776,7 +741,7 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla } depl := statefulset.NewStatefulSet( deplDef, - time.Duration(5)*time.Second, + glance.ShortDuration, ) ctrlResult, err = depl.CreateOrPatch(ctx, helper) @@ -1109,7 +1074,7 @@ func (r *GlanceAPIReconciler) ensureKeystoneEndpoints( instance.Namespace, ksEndpointSpec, serviceLabels, - time.Duration(10)*time.Second, + glance.NormalDuration, ) ctrlResult, err = ksSvc.CreateOrPatch(ctx, helper) if err != nil { @@ -1194,7 +1159,7 @@ func (r *GlanceAPIReconciler) ensureImageCacheJob( ) imageCacheCronJob := cronjob.NewCronJob( cronjobDef, - 5*time.Second, + glance.ShortDuration, ) ctrlResult, err := imageCacheCronJob.CreateOrPatch(ctx, h) if err != nil { diff --git a/pkg/glance/const.go b/pkg/glance/const.go index bec044f1..2022b648 100644 --- a/pkg/glance/const.go +++ b/pkg/glance/const.go @@ -17,6 +17,8 @@ package glance import ( "github.com/openstack-k8s-operators/lib-common/modules/storage" + ctrl "sigs.k8s.io/controller-runtime" + "time" ) // CronJobType - @@ -101,6 +103,10 @@ const ( GlanceCacheCleaner = "/usr/bin/glance-cache-cleaner" // GlanceCachePruner - GlanceCachePruner = "/usr/bin/glance-cache-pruner" + // ShortDuration - + ShortDuration = time.Duration(5) * time.Second + // NormalDuration - + NormalDuration = time.Duration(10) * time.Second ) // DbsyncPropagation keeps track of the DBSync Service Propagation Type @@ -110,3 +116,6 @@ var DbsyncPropagation = []storage.PropagationType{storage.DBSync} // It allows the GlanceAPI pod to mount volumes destined to Glance related // ServiceTypes var GlanceAPIPropagation = []storage.PropagationType{Glance, GlanceAPI} + +// ResultRequeue - Used to requeue a request +var ResultRequeue = ctrl.Result{RequeueAfter: NormalDuration}