Skip to content

Commit bd1b51e

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #492 from fmount/cleanup_updates_log
Improve logging and remove placeholder reconcile functions
2 parents 47e8a3c + 5d6aed5 commit bd1b51e

File tree

3 files changed

+25
-86
lines changed

3 files changed

+25
-86
lines changed

‎controllers/glance_controller.go‎

Lines changed: 9 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package controllers
1919
import (
2020
"context"
2121
"fmt"
22-
"time"
2322

2423
"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
2524
rbacv1 "k8s.io/api/rbac/v1"
@@ -109,6 +108,7 @@ func (r *GlanceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
109108
return ctrl.Result{}, nil
110109
}
111110
// Error reading the object - requeue the request.
111+
r.Log.Error(err, fmt.Sprintf("could not fetch Glance instance %s", instance.Name))
112112
return ctrl.Result{}, err
113113
}
114114

@@ -120,6 +120,7 @@ func (r *GlanceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
120120
r.Log,
121121
)
122122
if err != nil {
123+
r.Log.Error(err, fmt.Sprintf("could not instantiate helper for instance %s", instance.Name))
123124
return ctrl.Result{}, err
124125
}
125126

@@ -401,7 +402,7 @@ func (r *GlanceReconciler) reconcileInit(
401402
_, _, err := oko_secret.GetSecret(ctx, helper, instance.Spec.Secret, instance.Namespace)
402403
if err != nil {
403404
if k8s_errors.IsNotFound(err) {
404-
return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, fmt.Errorf("OpenStack secret %s not found", instance.Spec.Secret)
405+
return glance.ResultRequeue, fmt.Errorf("OpenStack secret %s not found", instance.Spec.Secret)
405406
}
406407
return ctrl.Result{}, err
407408
}
@@ -416,7 +417,7 @@ func (r *GlanceReconciler) reconcileInit(
416417
PasswordSelector: instance.Spec.PasswordSelectors.Service,
417418
}
418419

419-
ksSvc := keystonev1.NewKeystoneService(ksSvcSpec, instance.Namespace, serviceLabels, time.Duration(10)*time.Second)
420+
ksSvc := keystonev1.NewKeystoneService(ksSvcSpec, instance.Namespace, serviceLabels, glance.NormalDuration)
420421
ctrlResult, err := ksSvc.CreateOrPatch(ctx, helper)
421422
if err != nil {
422423
return ctrlResult, err
@@ -448,7 +449,7 @@ func (r *GlanceReconciler) reconcileInit(
448449
jobDef,
449450
glancev1.DbSyncHash,
450451
instance.Spec.PreserveJobs,
451-
time.Duration(5)*time.Second,
452+
glance.ShortDuration,
452453
dbSyncHash,
453454
)
454455
ctrlResult, err = dbSyncjob.DoJob(
@@ -486,26 +487,6 @@ func (r *GlanceReconciler) reconcileInit(
486487
return ctrl.Result{}, nil
487488
}
488489

489-
func (r *GlanceReconciler) reconcileUpdate(ctx context.Context, instance *glancev1.Glance, helper *helper.Helper) (ctrl.Result, error) {
490-
r.Log.Info(fmt.Sprintf("Reconciling Service '%s' update", instance.Name))
491-
492-
// TODO: should have minor update tasks if required
493-
// - delete dbsync hash from status to rerun it?
494-
495-
r.Log.Info(fmt.Sprintf("Reconciled Service '%s' update successfully", instance.Name))
496-
return ctrl.Result{}, nil
497-
}
498-
499-
func (r *GlanceReconciler) reconcileUpgrade(ctx context.Context, instance *glancev1.Glance, helper *helper.Helper) (ctrl.Result, error) {
500-
r.Log.Info(fmt.Sprintf("Reconciling Service '%s' upgrade", instance.Name))
501-
502-
// TODO: should have major version upgrade tasks
503-
// -delete dbsync hash from status to rerun it?
504-
505-
r.Log.Info(fmt.Sprintf("Reconciled Service '%s' upgrade successfully", instance.Name))
506-
return ctrl.Result{}, nil
507-
}
508-
509490
func (r *GlanceReconciler) reconcileNormal(ctx context.Context, instance *glancev1.Glance, helper *helper.Helper) (ctrl.Result, error) {
510491
r.Log.Info(fmt.Sprintf("Reconciling Service '%s'", instance.Name))
511492

@@ -549,7 +530,7 @@ func (r *GlanceReconciler) reconcileNormal(ctx context.Context, instance *glance
549530
condition.RequestedReason,
550531
condition.SeverityInfo,
551532
condition.MemcachedReadyWaitingMessage))
552-
return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, fmt.Errorf("memcached %s not found", instance.Spec.MemcachedInstance)
533+
return glance.ResultRequeue, fmt.Errorf("memcached %s not found", instance.Spec.MemcachedInstance)
553534
}
554535
instance.Status.Conditions.Set(condition.FalseCondition(
555536
condition.MemcachedReadyCondition,
@@ -566,7 +547,7 @@ func (r *GlanceReconciler) reconcileNormal(ctx context.Context, instance *glance
566547
condition.RequestedReason,
567548
condition.SeverityInfo,
568549
condition.MemcachedReadyWaitingMessage))
569-
return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, fmt.Errorf("memcached %s is not ready", memcached.Name)
550+
return glance.ResultRequeue, fmt.Errorf("memcached %s is not ready", memcached.Name)
570551
}
571552
// Mark the Memcached Service as Ready if we get to this point with no errors
572553
instance.Status.Conditions.MarkTrue(
@@ -584,7 +565,7 @@ func (r *GlanceReconciler) reconcileNormal(ctx context.Context, instance *glance
584565
condition.RequestedReason,
585566
condition.SeverityInfo,
586567
condition.InputReadyWaitingMessage))
587-
return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, fmt.Errorf("OpenStack secret %s not found", instance.Spec.Secret)
568+
return glance.ResultRequeue, fmt.Errorf("OpenStack secret %s not found", instance.Spec.Secret)
588569
}
589570
instance.Status.Conditions.Set(condition.FalseCondition(
590571
condition.InputReadyCondition,
@@ -643,22 +624,6 @@ func (r *GlanceReconciler) reconcileNormal(ctx context.Context, instance *glance
643624
return ctrlResult, nil
644625
}
645626

646-
// Handle service update
647-
ctrlResult, err = r.reconcileUpdate(ctx, instance, helper)
648-
if err != nil {
649-
return ctrlResult, err
650-
} else if (ctrlResult != ctrl.Result{}) {
651-
return ctrlResult, nil
652-
}
653-
654-
// Handle service upgrade
655-
ctrlResult, err = r.reconcileUpgrade(ctx, instance, helper)
656-
if err != nil {
657-
return ctrlResult, err
658-
} else if (ctrlResult != ctrl.Result{}) {
659-
return ctrlResult, nil
660-
}
661-
662627
//
663628
// Reconcile the GlanceAPI deployment
664629
//
@@ -1052,7 +1017,7 @@ func (r *GlanceReconciler) ensureDBPurgeJob(
10521017

10531018
dbPurgeCronJob := cronjob.NewCronJob(
10541019
cronjobDef,
1055-
5*time.Second,
1020+
glance.ShortDuration,
10561021
)
10571022
ctrlResult, err := dbPurgeCronJob.CreateOrPatch(ctx, h)
10581023
if err != nil {

‎controllers/glanceapi_controller.go‎

Lines changed: 7 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"context"
2121
"fmt"
2222
"strings"
23-
"time"
2423

2524
batchv1 "k8s.io/api/batch/v1"
2625
"k8s.io/apimachinery/pkg/api/resource"
@@ -104,6 +103,7 @@ func (r *GlanceAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
104103
return ctrl.Result{}, nil
105104
}
106105
// Error reading the object - requeue the request.
106+
r.Log.Error(err, fmt.Sprintf("could not fetch GlanceAPI instance %s", instance.Name))
107107
return ctrl.Result{}, err
108108
}
109109

@@ -115,6 +115,7 @@ func (r *GlanceAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
115115
r.Log,
116116
)
117117
if err != nil {
118+
r.Log.Error(err, fmt.Sprintf("could not instantiate helper for instance %s", instance.Name))
118119
return ctrl.Result{}, err
119120
}
120121

@@ -525,26 +526,6 @@ func (r *GlanceAPIReconciler) reconcileInit(
525526
return ctrl.Result{}, nil
526527
}
527528

528-
func (r *GlanceAPIReconciler) reconcileUpdate(ctx context.Context, instance *glancev1.GlanceAPI, helper *helper.Helper) (ctrl.Result, error) {
529-
r.Log.Info(fmt.Sprintf("Reconciling Service '%s' update", instance.Name))
530-
531-
// TODO: should have minor update tasks if required
532-
// - delete dbsync hash from status to rerun it?
533-
534-
r.Log.Info(fmt.Sprintf("Reconciled Service '%s' update successfully", instance.Name))
535-
return ctrl.Result{}, nil
536-
}
537-
538-
func (r *GlanceAPIReconciler) reconcileUpgrade(ctx context.Context, instance *glancev1.GlanceAPI, helper *helper.Helper) (ctrl.Result, error) {
539-
r.Log.Info(fmt.Sprintf("Reconciling Service '%s' upgrade", instance.Name))
540-
541-
// TODO: should have major version upgrade tasks
542-
// -delete dbsync hash from status to rerun it?
543-
544-
r.Log.Info(fmt.Sprintf("Reconciled Service '%s' upgrade successfully", instance.Name))
545-
return ctrl.Result{}, nil
546-
}
547-
548529
func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *glancev1.GlanceAPI, helper *helper.Helper, req ctrl.Request) (ctrl.Result, error) {
549530
r.Log.Info(fmt.Sprintf("Reconciling Service '%s'", instance.Name))
550531

@@ -563,7 +544,7 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla
563544
condition.RequestedReason,
564545
condition.SeverityInfo,
565546
condition.InputReadyWaitingMessage))
566-
return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, fmt.Errorf("OpenStack secret %s not found", instance.Spec.Secret)
547+
return glance.ResultRequeue, fmt.Errorf("OpenStack secret %s not found", instance.Spec.Secret)
567548
}
568549
instance.Status.Conditions.Set(condition.FalseCondition(
569550
condition.InputReadyCondition,
@@ -619,7 +600,7 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla
619600
condition.SeverityInfo,
620601
glancev1.CinderInitMessage),
621602
)
622-
return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil
603+
return glance.ResultRequeue, nil
623604
}
624605
// Cinder CR is found, we can unblock glance deployment because
625606
// it represents a valid backend.
@@ -750,22 +731,6 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla
750731
return ctrlResult, nil
751732
}
752733

753-
// Handle service update
754-
ctrlResult, err = r.reconcileUpdate(ctx, instance, helper)
755-
if err != nil {
756-
return ctrlResult, err
757-
} else if (ctrlResult != ctrl.Result{}) {
758-
return ctrlResult, nil
759-
}
760-
761-
// Handle service upgrade
762-
ctrlResult, err = r.reconcileUpgrade(ctx, instance, helper)
763-
if err != nil {
764-
return ctrlResult, err
765-
} else if (ctrlResult != ctrl.Result{}) {
766-
return ctrlResult, nil
767-
}
768-
769734
//
770735
// normal reconcile tasks
771736
//
@@ -783,7 +748,7 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla
783748
}
784749
depl := statefulset.NewStatefulSet(
785750
deplDef,
786-
time.Duration(5)*time.Second,
751+
glance.ShortDuration,
787752
)
788753

789754
ctrlResult, err = depl.CreateOrPatch(ctx, helper)
@@ -1116,7 +1081,7 @@ func (r *GlanceAPIReconciler) ensureKeystoneEndpoints(
11161081
instance.Namespace,
11171082
ksEndpointSpec,
11181083
serviceLabels,
1119-
time.Duration(10)*time.Second,
1084+
glance.NormalDuration,
11201085
)
11211086
ctrlResult, err = ksSvc.CreateOrPatch(ctx, helper)
11221087
if err != nil {
@@ -1201,7 +1166,7 @@ func (r *GlanceAPIReconciler) ensureImageCacheJob(
12011166
)
12021167
imageCacheCronJob := cronjob.NewCronJob(
12031168
cronjobDef,
1204-
5*time.Second,
1169+
glance.ShortDuration,
12051170
)
12061171
ctrlResult, err := imageCacheCronJob.CreateOrPatch(ctx, h)
12071172
if err != nil {

‎pkg/glance/const.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ package glance
1717

1818
import (
1919
"github.com/openstack-k8s-operators/lib-common/modules/storage"
20+
ctrl "sigs.k8s.io/controller-runtime"
21+
"time"
2022
)
2123

2224
// CronJobType -
@@ -101,6 +103,10 @@ const (
101103
GlanceCacheCleaner = "/usr/bin/glance-cache-cleaner"
102104
// GlanceCachePruner -
103105
GlanceCachePruner = "/usr/bin/glance-cache-pruner"
106+
// ShortDuration -
107+
ShortDuration = time.Duration(5) * time.Second
108+
// NormalDuration -
109+
NormalDuration = time.Duration(10) * time.Second
104110
)
105111

106112
// DbsyncPropagation keeps track of the DBSync Service Propagation Type
@@ -110,3 +116,6 @@ var DbsyncPropagation = []storage.PropagationType{storage.DBSync}
110116
// It allows the GlanceAPI pod to mount volumes destined to Glance related
111117
// ServiceTypes
112118
var GlanceAPIPropagation = []storage.PropagationType{Glance, GlanceAPI}
119+
120+
// ResultRequeue - Used to requeue a request
121+
var ResultRequeue = ctrl.Result{RequeueAfter: NormalDuration}

0 commit comments

Comments
 (0)