From 801d225f95ab83c7c134fbb228ce4b3f6e6f656d Mon Sep 17 00:00:00 2001 From: Christian Schwede Date: Fri, 1 Mar 2024 09:07:16 +0100 Subject: [PATCH 1/2] Fix requeueing if error is non-nil Returning both a non-zero result and a non-nil error is always ignored. Fixing this by logging the error and then returning without an error and RequeAfter. https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/reconcile#Reconciler --- controllers/swift_controller.go | 6 ++++-- controllers/swiftproxy_controller.go | 6 ++++-- controllers/swiftstorage_controller.go | 3 ++- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/controllers/swift_controller.go b/controllers/swift_controller.go index d02e2087..6bfa6dce 100644 --- a/controllers/swift_controller.go +++ b/controllers/swift_controller.go @@ -238,7 +238,8 @@ func (r *SwiftReconciler) reconcileNormal(ctx context.Context, instance *swiftv1 condition.RequestedReason, condition.SeverityInfo, condition.MemcachedReadyWaitingMessage)) - return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, fmt.Errorf("memcached %s not found", instance.Spec.MemcachedInstance) + r.Log.Error(err, "memcached not found") + return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil } instance.Status.Conditions.Set(condition.FalseCondition( condition.MemcachedReadyCondition, @@ -255,7 +256,8 @@ func (r *SwiftReconciler) reconcileNormal(ctx context.Context, instance *swiftv1 condition.RequestedReason, condition.SeverityInfo, condition.MemcachedReadyWaitingMessage)) - return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, fmt.Errorf("memcached %s is not ready", memcached.Name) + r.Log.Error(err, "memcached is not ready") + return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil } // Mark the Memcached Service as Ready if we get to this point with no errors instance.Status.Conditions.MarkTrue( diff --git a/controllers/swiftproxy_controller.go b/controllers/swiftproxy_controller.go index 17b75e08..dfb401d7 100644 --- a/controllers/swiftproxy_controller.go +++ b/controllers/swiftproxy_controller.go @@ -417,7 +417,8 @@ func (r *SwiftProxyReconciler) Reconcile(ctx context.Context, req ctrl.Request) condition.RequestedReason, condition.SeverityInfo, condition.InputReadyWaitingMessage)) - return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, fmt.Errorf("Secret %s not found", instance.Spec.Secret) + r.Log.Error(err, "Secret not found") + return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil } instance.Status.Conditions.Set(condition.FalseCondition( condition.InputReadyCondition, @@ -499,7 +500,8 @@ func (r *SwiftProxyReconciler) Reconcile(ctx context.Context, req ctrl.Request) condition.SeverityInfo, condition.NetworkAttachmentsReadyWaitingMessage, netAtt)) - return ctrl.Result{RequeueAfter: time.Second * 10}, fmt.Errorf("network-attachment-definition %s not found", netAtt) + r.Log.Error(err, "network-attachment-definition not found") + return ctrl.Result{RequeueAfter: time.Second * 10}, nil } instance.Status.Conditions.Set(condition.FalseCondition( condition.NetworkAttachmentsReadyCondition, diff --git a/controllers/swiftstorage_controller.go b/controllers/swiftstorage_controller.go index c7ce04be..2d034e26 100644 --- a/controllers/swiftstorage_controller.go +++ b/controllers/swiftstorage_controller.go @@ -171,7 +171,8 @@ func (r *SwiftStorageReconciler) Reconcile(ctx context.Context, req ctrl.Request condition.SeverityInfo, condition.NetworkAttachmentsReadyWaitingMessage, netAtt)) - return ctrl.Result{RequeueAfter: time.Second * 10}, fmt.Errorf("network-attachment-definition %s not found", netAtt) + r.Log.Error(err, "network-attachment-definition not found") + return ctrl.Result{RequeueAfter: time.Second * 10}, nil } instance.Status.Conditions.Set(condition.FalseCondition( condition.NetworkAttachmentsReadyCondition, From 0976a97b583ba06fed062027988e8ade575a5f34 Mon Sep 17 00:00:00 2001 From: Christian Schwede Date: Fri, 1 Mar 2024 09:19:45 +0100 Subject: [PATCH 2/2] Fix using wrong return values The used return values were from earlier calls, fixing these by returning a default response. --- controllers/swiftproxy_controller.go | 6 +++--- controllers/swiftstorage_controller.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/controllers/swiftproxy_controller.go b/controllers/swiftproxy_controller.go index dfb401d7..cb719ada 100644 --- a/controllers/swiftproxy_controller.go +++ b/controllers/swiftproxy_controller.go @@ -383,15 +383,15 @@ func (r *SwiftProxyReconciler) Reconcile(ctx context.Context, req ctrl.Request) // Get the Keystone endpoint URLs keystoneAPI, err := keystonev1.GetKeystoneAPI(ctx, helper, instance.Namespace, map[string]string{}) if err != nil { - return ctrlResult, err + return ctrl.Result{}, err } keystonePublicURL, err := keystoneAPI.GetEndpoint(endpoint.EndpointPublic) if err != nil { - return ctrlResult, err + return ctrl.Result{}, err } keystoneInternalURL, err := keystoneAPI.GetEndpoint(endpoint.EndpointInternal) if err != nil { - return ctrlResult, err + return ctrl.Result{}, err } // Create OpenStack roles for Swift RBAC diff --git a/controllers/swiftstorage_controller.go b/controllers/swiftstorage_controller.go index 2d034e26..f7415792 100644 --- a/controllers/swiftstorage_controller.go +++ b/controllers/swiftstorage_controller.go @@ -187,7 +187,7 @@ func (r *SwiftStorageReconciler) Reconcile(ctx context.Context, req ctrl.Request // NetworkPolicy for the storage pods config := Netconfig{} if err = json.Unmarshal([]byte(nad.Spec.Config), &config); err != nil { - return ctrlResult, err + return ctrl.Result{}, err } if config.Name == "storage" { storageNetworkRange = config.Ipam.Range