From 229937d843fccfe9a350dbcf873fb9e54afc7fb0 Mon Sep 17 00:00:00 2001 From: Francesco Pantano Date: Wed, 27 Mar 2024 23:32:23 +0100 Subject: [PATCH] Improve logging and remove placeholder functions This patch adds more logging where necessary and remove the reconcileUpdate and reconcileUgrade placeholder functions. Signed-off-by: Francesco Pantano --- controllers/manila_controller.go | 53 ++++------------------- controllers/manilaapi_controller.go | 49 +++------------------ controllers/manilascheduler_controller.go | 46 +++----------------- controllers/manilashare_controller.go | 45 +++---------------- pkg/manila/const.go | 9 ++++ 5 files changed, 35 insertions(+), 167 deletions(-) diff --git a/controllers/manila_controller.go b/controllers/manila_controller.go index c1597ddc..9c94b5f2 100644 --- a/controllers/manila_controller.go +++ b/controllers/manila_controller.go @@ -19,7 +19,6 @@ package controllers import ( "context" "fmt" - "time" "github.com/go-logr/logr" memcachedv1 "github.com/openstack-k8s-operators/infra-operator/apis/memcached/v1beta1" @@ -128,6 +127,7 @@ func (r *ManilaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res if k8s_errors.IsNotFound(err) { return ctrl.Result{}, nil } + r.Log.Error(err, fmt.Sprintf("could not fetch Manila instance %s", instance.Name)) return ctrl.Result{}, err } @@ -139,6 +139,7 @@ func (r *ManilaReconciler) 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 } @@ -364,7 +365,7 @@ func (r *ManilaReconciler) reconcileInit( jobDef, manilav1beta1.DbSyncHash, instance.Spec.PreserveJobs, - time.Duration(5)*time.Second, + manila.ShortDuration, dbSyncHash, ) ctrlResult, err := dbSyncjob.DoJob( @@ -455,7 +456,7 @@ func (r *ManilaReconciler) reconcileNormal(ctx context.Context, instance *manila condition.RequestedReason, condition.SeverityInfo, condition.RabbitMqTransportURLReadyRunningMessage)) - return ctrl.Result{RequeueAfter: time.Second * 10}, nil + return manila.ResultRequeue, nil } instance.Status.Conditions.MarkTrue(condition.RabbitMqTransportURLReadyCondition, condition.RabbitMqTransportURLReadyMessage) @@ -473,7 +474,7 @@ func (r *ManilaReconciler) reconcileNormal(ctx context.Context, instance *manila 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 manila.ResultRequeue, fmt.Errorf("memcached %s not found", instance.Spec.MemcachedInstance) } instance.Status.Conditions.Set(condition.FalseCondition( condition.MemcachedReadyCondition, @@ -490,7 +491,7 @@ func (r *ManilaReconciler) reconcileNormal(ctx context.Context, instance *manila 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 manila.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( @@ -508,7 +509,7 @@ func (r *ManilaReconciler) reconcileNormal(ctx context.Context, instance *manila 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 manila.ResultRequeue, fmt.Errorf("OpenStack secret %s not found", instance.Spec.Secret) } instance.Status.Conditions.Set(condition.FalseCondition( condition.InputReadyCondition, @@ -601,7 +602,7 @@ func (r *ManilaReconciler) reconcileNormal(ctx context.Context, instance *manila condition.SeverityInfo, condition.NetworkAttachmentsReadyWaitingMessage, netAtt)) - return ctrl.Result{RequeueAfter: time.Second * 10}, fmt.Errorf("network-attachment-definition %s not found", netAtt) + return manila.ResultRequeue, fmt.Errorf("network-attachment-definition %s not found", netAtt) } instance.Status.Conditions.Set(condition.FalseCondition( condition.NetworkAttachmentsReadyCondition, @@ -635,22 +636,6 @@ func (r *ManilaReconciler) reconcileNormal(ctx context.Context, instance *manila 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 // @@ -761,7 +746,7 @@ func (r *ManilaReconciler) reconcileNormal(ctx context.Context, instance *manila cronjobDef := manila.CronJob(instance, serviceLabels, serviceAnnotations) cronjob := cronjob.NewCronJob( cronjobDef, - 5*time.Second, + manila.ShortDuration, ) ctrlResult, err = cronjob.CreateOrPatch(ctx, helper) @@ -787,26 +772,6 @@ func (r *ManilaReconciler) reconcileNormal(ctx context.Context, instance *manila return ctrl.Result{}, nil } -func (r *ManilaReconciler) reconcileUpdate(ctx context.Context, instance *manilav1beta1.Manila, 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 *ManilaReconciler) reconcileUpgrade(ctx context.Context, instance *manilav1beta1.Manila, 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 -} - // generateServiceConfigMaps - create create configmaps which hold scripts and service configuration func (r *ManilaReconciler) generateServiceConfig( ctx context.Context, diff --git a/controllers/manilaapi_controller.go b/controllers/manilaapi_controller.go index 7b6c20ba..b0009648 100644 --- a/controllers/manilaapi_controller.go +++ b/controllers/manilaapi_controller.go @@ -19,7 +19,6 @@ package controllers import ( "context" "fmt" - "time" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/runtime" @@ -127,6 +126,7 @@ func (r *ManilaAPIReconciler) 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 ManilaAPI instance %s", instance.Name)) return ctrl.Result{}, err } @@ -138,6 +138,7 @@ func (r *ManilaAPIReconciler) 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 } @@ -562,7 +563,7 @@ func (r *ManilaAPIReconciler) 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, manila.NormalDuration) ctrlResult, err := ksSvcObj.CreateOrPatch(ctx, helper) if err != nil { instance.Status.Conditions.MarkFalse( @@ -595,7 +596,7 @@ func (r *ManilaAPIReconciler) reconcileInit( instance.Namespace, ksEndptSpec, serviceLabels, - time.Duration(10)*time.Second) + manila.NormalDuration) ctrlResult, err = ksEndptObj.CreateOrPatch(ctx, helper) if err != nil { instance.Status.Conditions.MarkFalse( @@ -781,7 +782,7 @@ func (r *ManilaAPIReconciler) reconcileNormal(ctx context.Context, instance *man condition.SeverityInfo, condition.NetworkAttachmentsReadyWaitingMessage, netAtt)) - return ctrl.Result{RequeueAfter: time.Second * 10}, fmt.Errorf("network-attachment-definition %s not found", netAtt) + return manila.ResultRequeue, fmt.Errorf("network-attachment-definition %s not found", netAtt) } instance.Status.Conditions.Set(condition.FalseCondition( condition.NetworkAttachmentsReadyCondition, @@ -813,22 +814,6 @@ func (r *ManilaAPIReconciler) reconcileNormal(ctx context.Context, instance *man 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 // @@ -845,7 +830,7 @@ func (r *ManilaAPIReconciler) reconcileNormal(ctx context.Context, instance *man return ctrl.Result{}, err } - ss := statefulset.NewStatefulSet(ssDef, time.Duration(5)*time.Second) + ss := statefulset.NewStatefulSet(ssDef, manila.ShortDuration) ctrlResult, err = ss.CreateOrPatch(ctx, helper) if err != nil { @@ -933,26 +918,6 @@ func (r *ManilaAPIReconciler) reconcileNormal(ctx context.Context, instance *man return ctrl.Result{}, nil } -func (r *ManilaAPIReconciler) reconcileUpdate(ctx context.Context, instance *manilav1beta1.ManilaAPI, 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 *ManilaAPIReconciler) reconcileUpgrade(ctx context.Context, instance *manilav1beta1.ManilaAPI, 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 -} - // getSecret - get the specified secret, and add its hash to envVars func (r *ManilaAPIReconciler) getSecret( ctx context.Context, @@ -969,7 +934,7 @@ func (r *ManilaAPIReconciler) getSecret( condition.RequestedReason, condition.SeverityInfo, condition.InputReadyWaitingMessage)) - return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, fmt.Errorf("Secret %s not found", secretName) + return manila.ResultRequeue, fmt.Errorf("Secret %s not found", secretName) } instance.Status.Conditions.Set(condition.FalseCondition( condition.InputReadyCondition, diff --git a/controllers/manilascheduler_controller.go b/controllers/manilascheduler_controller.go index d6357111..157496c7 100644 --- a/controllers/manilascheduler_controller.go +++ b/controllers/manilascheduler_controller.go @@ -19,7 +19,6 @@ package controllers import ( "context" "fmt" - "time" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -105,6 +104,7 @@ func (r *ManilaSchedulerReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, nil } // Error reading the object - requeue the request. + r.Log.Error(err, fmt.Sprintf("could not fetch ManilaScheduler instance %s", instance.Name)) return ctrl.Result{}, err } @@ -116,6 +116,7 @@ func (r *ManilaSchedulerReconciler) Reconcile(ctx context.Context, req ctrl.Requ r.Log, ) if err != nil { + r.Log.Error(err, fmt.Sprintf("could not instantiate helper for instance %s", instance.Name)) return ctrl.Result{}, err } @@ -465,7 +466,7 @@ func (r *ManilaSchedulerReconciler) reconcileNormal(ctx context.Context, instanc condition.SeverityInfo, condition.NetworkAttachmentsReadyWaitingMessage, netAtt)) - return ctrl.Result{RequeueAfter: time.Second * 10}, fmt.Errorf("network-attachment-definition %s not found", netAtt) + return manila.ResultRequeue, fmt.Errorf("network-attachment-definition %s not found", netAtt) } instance.Status.Conditions.Set(condition.FalseCondition( condition.NetworkAttachmentsReadyCondition, @@ -497,22 +498,6 @@ func (r *ManilaSchedulerReconciler) reconcileNormal(ctx context.Context, instanc 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 // @@ -520,7 +505,7 @@ func (r *ManilaSchedulerReconciler) reconcileNormal(ctx context.Context, instanc // Deploy a statefulset ss := statefulset.NewStatefulSet( manilascheduler.StatefulSet(instance, inputHash, serviceLabels, serviceAnnotations), - time.Duration(5)*time.Second, + manila.ShortDuration, ) ctrlResult, err = ss.CreateOrPatch(ctx, helper) @@ -608,27 +593,6 @@ func (r *ManilaSchedulerReconciler) reconcileNormal(ctx context.Context, instanc return ctrl.Result{}, nil } -func (r *ManilaSchedulerReconciler) reconcileUpdate(ctx context.Context, instance *manilav1beta1.ManilaScheduler, 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)) - // update the overall status condition if service is ready - return ctrl.Result{}, nil -} - -func (r *ManilaSchedulerReconciler) reconcileUpgrade(ctx context.Context, instance *manilav1beta1.ManilaScheduler, 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 -} - // getSecret - get the specified secret, and add its hash to envVars func (r *ManilaSchedulerReconciler) getSecret( ctx context.Context, @@ -645,7 +609,7 @@ func (r *ManilaSchedulerReconciler) getSecret( condition.RequestedReason, condition.SeverityInfo, condition.InputReadyWaitingMessage)) - return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, fmt.Errorf("Secret %s not found", secretName) + return manila.ResultRequeue, fmt.Errorf("Secret %s not found", secretName) } instance.Status.Conditions.Set(condition.FalseCondition( condition.InputReadyCondition, diff --git a/controllers/manilashare_controller.go b/controllers/manilashare_controller.go index d2b9ecae..2cdeac0a 100644 --- a/controllers/manilashare_controller.go +++ b/controllers/manilashare_controller.go @@ -19,7 +19,6 @@ package controllers import ( "context" "fmt" - "time" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -106,6 +105,7 @@ func (r *ManilaShareReconciler) 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 ManilaShare instance %s", instance.Name)) return ctrl.Result{}, err } @@ -117,6 +117,7 @@ func (r *ManilaShareReconciler) 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 } @@ -461,7 +462,7 @@ func (r *ManilaShareReconciler) reconcileNormal(ctx context.Context, instance *m condition.SeverityInfo, condition.NetworkAttachmentsReadyWaitingMessage, netAtt)) - return ctrl.Result{RequeueAfter: time.Second * 10}, fmt.Errorf("network-attachment-definition %s not found", netAtt) + return manila.ResultRequeue, fmt.Errorf("network-attachment-definition %s not found", netAtt) } instance.Status.Conditions.Set(condition.FalseCondition( condition.NetworkAttachmentsReadyCondition, @@ -493,22 +494,6 @@ func (r *ManilaShareReconciler) reconcileNormal(ctx context.Context, instance *m 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 // @@ -516,7 +501,7 @@ func (r *ManilaShareReconciler) reconcileNormal(ctx context.Context, instance *m // Deploy a statefulset ss := statefulset.NewStatefulSet( manilashare.StatefulSet(instance, inputHash, serviceLabels, serviceAnnotations), - time.Duration(5)*time.Second, + manila.ShortDuration, ) ctrlResult, err = ss.CreateOrPatch(ctx, helper) @@ -605,26 +590,6 @@ func (r *ManilaShareReconciler) reconcileNormal(ctx context.Context, instance *m return ctrl.Result{}, nil } -func (r *ManilaShareReconciler) reconcileUpdate(ctx context.Context, instance *manilav1beta1.ManilaShare, 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 *ManilaShareReconciler) reconcileUpgrade(ctx context.Context, instance *manilav1beta1.ManilaShare, 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 -} - // getSecret - get the specified secret, and add its hash to envVars func (r *ManilaShareReconciler) getSecret( ctx context.Context, @@ -641,7 +606,7 @@ func (r *ManilaShareReconciler) getSecret( condition.RequestedReason, condition.SeverityInfo, condition.InputReadyWaitingMessage)) - return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, fmt.Errorf("Secret %s not found", secretName) + return manila.ResultRequeue, fmt.Errorf("Secret %s not found", secretName) } instance.Status.Conditions.Set(condition.FalseCondition( condition.InputReadyCondition, diff --git a/pkg/manila/const.go b/pkg/manila/const.go index cc9ba9d7..b9399a62 100644 --- a/pkg/manila/const.go +++ b/pkg/manila/const.go @@ -14,6 +14,8 @@ package manila import ( "github.com/openstack-k8s-operators/lib-common/modules/storage" + ctrl "sigs.k8s.io/controller-runtime" + "time" ) const ( @@ -65,6 +67,10 @@ const ( // CustomServiceConfigSecretsFileName - Snippet generated by Secrets passed // to the sub CR CustomServiceConfigSecretsFileName = "04-config.conf" + // ShortDuration - + ShortDuration = time.Duration(5) * time.Second + // NormalDuration - + NormalDuration = time.Duration(10) * time.Second ) // DbsyncPropagation keeps track of the DBSync Service Propagation Type @@ -84,3 +90,6 @@ var ManilaAPIPropagation = []storage.PropagationType{Manila, ManilaAPI} // It allows the ManilaShare pods to mount volumes destined to Manila and ManilaShare // ServiceTypes var ManilaSharePropagation = []storage.PropagationType{Manila, ManilaShare} + +// ResultRequeue - Used to requeue a request +var ResultRequeue = ctrl.Result{RequeueAfter: NormalDuration}