Skip to content

Commit

Permalink
if an error is non-nil not need to set Result.Requeue true
Browse files Browse the repository at this point in the history
Signed-off-by: huangyanfeng <huangyanfeng1992@gmail.com>
  • Loading branch information
yanfeng1992 committed Apr 3, 2024
1 parent a0c0a98 commit 7c2b3e1
Show file tree
Hide file tree
Showing 25 changed files with 88 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (c *CRBApplicationFailoverController) Reconcile(ctx context.Context, req co
c.workloadUnhealthyMap.delete(req.NamespacedName)
return controllerruntime.Result{}, nil
}
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

if !c.clusterResourceBindingFilter(binding) {
Expand All @@ -81,7 +81,7 @@ func (c *CRBApplicationFailoverController) Reconcile(ctx context.Context, req co

retryDuration, err := c.syncBinding(binding)
if err != nil {
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}
if retryDuration > 0 {
klog.V(4).Infof("Retry to check health status of the workload after %v minutes.", retryDuration.Minutes())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (c *RBApplicationFailoverController) Reconcile(ctx context.Context, req con
c.workloadUnhealthyMap.delete(req.NamespacedName)
return controllerruntime.Result{}, nil
}
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

if !c.bindingFilter(binding) {
Expand All @@ -81,7 +81,7 @@ func (c *RBApplicationFailoverController) Reconcile(ctx context.Context, req con

retryDuration, err := c.syncBinding(binding)
if err != nil {
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}
if retryDuration > 0 {
klog.V(4).Infof("Retry to check health status of the workload after %v minutes.", retryDuration.Minutes())
Expand Down
12 changes: 6 additions & 6 deletions pkg/controllers/binding/binding_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,14 @@ func (c *ResourceBindingController) Reconcile(ctx context.Context, req controlle
return controllerruntime.Result{}, nil
}

return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

if !binding.DeletionTimestamp.IsZero() {
klog.V(4).Infof("Begin to delete works owned by binding(%s).", req.NamespacedName.String())
if err := helper.DeleteWorkByRBNamespaceAndName(c.Client, req.Namespace, req.Name); err != nil {
klog.Errorf("Failed to delete works related to %s/%s: %v", binding.GetNamespace(), binding.GetName(), err)
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}
return c.removeFinalizer(binding)
}
Expand All @@ -101,15 +101,15 @@ func (c *ResourceBindingController) removeFinalizer(rb *workv1alpha2.ResourceBin
controllerutil.RemoveFinalizer(rb, util.BindingControllerFinalizer)
err := c.Client.Update(context.TODO(), rb)
if err != nil {
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}
return controllerruntime.Result{}, nil
}

// syncBinding will sync resourceBinding to Works.
func (c *ResourceBindingController) syncBinding(binding *workv1alpha2.ResourceBinding) (controllerruntime.Result, error) {
if err := c.removeOrphanWorks(binding); err != nil {
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

workload, err := helper.FetchResourceTemplate(c.DynamicClient, c.InformerManager, c.RESTMapper, binding.Spec.Resource)
Expand All @@ -122,7 +122,7 @@ func (c *ResourceBindingController) syncBinding(binding *workv1alpha2.ResourceBi
}
klog.Errorf("Failed to fetch workload for resourceBinding(%s/%s). Error: %v.",
binding.GetNamespace(), binding.GetName(), err)
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}
start := time.Now()
err = ensureWork(c.Client, c.ResourceInterpreter, workload, c.OverrideManager, binding, apiextensionsv1.NamespaceScoped)
Expand All @@ -132,7 +132,7 @@ func (c *ResourceBindingController) syncBinding(binding *workv1alpha2.ResourceBi
binding.GetNamespace(), binding.GetName(), err)
c.EventRecorder.Event(binding, corev1.EventTypeWarning, events.EventReasonSyncWorkFailed, err.Error())
c.EventRecorder.Event(workload, corev1.EventTypeWarning, events.EventReasonSyncWorkFailed, err.Error())
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

msg := fmt.Sprintf("Sync work of resourceBinding(%s/%s) successful.", binding.Namespace, binding.Name)
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/binding/binding_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func TestResourceBindingController_Reconcile(t *testing.T) {
},
{
name: "RB found without deleting",
want: controllerruntime.Result{Requeue: true},
want: controllerruntime.Result{},
wantErr: true,
rb: &workv1alpha2.ResourceBinding{
ObjectMeta: metav1.ObjectMeta{
Expand Down
12 changes: 6 additions & 6 deletions pkg/controllers/binding/cluster_resource_binding_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,14 @@ func (c *ClusterResourceBindingController) Reconcile(ctx context.Context, req co
return controllerruntime.Result{}, nil
}

return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

if !clusterResourceBinding.DeletionTimestamp.IsZero() {
klog.V(4).Infof("Begin to delete works owned by binding(%s).", req.NamespacedName.String())
if err := helper.DeleteWorkByCRBName(c.Client, req.Name); err != nil {
klog.Errorf("Failed to delete works related to %s: %v", clusterResourceBinding.GetName(), err)
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}
return c.removeFinalizer(clusterResourceBinding)
}
Expand All @@ -101,15 +101,15 @@ func (c *ClusterResourceBindingController) removeFinalizer(crb *workv1alpha2.Clu
controllerutil.RemoveFinalizer(crb, util.ClusterResourceBindingControllerFinalizer)
err := c.Client.Update(context.TODO(), crb)
if err != nil {
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}
return controllerruntime.Result{}, nil
}

// syncBinding will sync clusterResourceBinding to Works.
func (c *ClusterResourceBindingController) syncBinding(binding *workv1alpha2.ClusterResourceBinding) (controllerruntime.Result, error) {
if err := c.removeOrphanWorks(binding); err != nil {
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

workload, err := helper.FetchResourceTemplate(c.DynamicClient, c.InformerManager, c.RESTMapper, binding.Spec.Resource)
Expand All @@ -121,7 +121,7 @@ func (c *ClusterResourceBindingController) syncBinding(binding *workv1alpha2.Clu
return controllerruntime.Result{}, nil
}
klog.Errorf("Failed to fetch workload for clusterResourceBinding(%s). Error: %v.", binding.GetName(), err)
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

start := time.Now()
Expand All @@ -131,7 +131,7 @@ func (c *ClusterResourceBindingController) syncBinding(binding *workv1alpha2.Clu
klog.Errorf("Failed to transform clusterResourceBinding(%s) to works. Error: %v.", binding.GetName(), err)
c.EventRecorder.Event(binding, corev1.EventTypeWarning, events.EventReasonSyncWorkFailed, err.Error())
c.EventRecorder.Event(workload, corev1.EventTypeWarning, events.EventReasonSyncWorkFailed, err.Error())
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

msg := fmt.Sprintf("Sync work of clusterResourceBinding(%s) successful.", binding.GetName())
Expand Down
8 changes: 4 additions & 4 deletions pkg/controllers/certificate/cert_rotation_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func (c *CertRotationController) Reconcile(ctx context.Context, req controllerru
return controllerruntime.Result{}, nil
}

return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

if !cluster.DeletionTimestamp.IsZero() {
Expand All @@ -109,18 +109,18 @@ func (c *CertRotationController) Reconcile(ctx context.Context, req controllerru
c.ClusterClient, err = c.ClusterClientSetFunc(cluster.Name, c.Client, c.ClusterClientOption)
if err != nil {
klog.Errorf("Failed to create a ClusterClient for the given member cluster: %s, err is: %v", cluster.Name, err)
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

secret, err := c.ClusterClient.KubeClient.CoreV1().Secrets(c.KarmadaKubeconfigNamespace).Get(ctx, KarmadaKubeconfigName, metav1.GetOptions{})
if err != nil {
klog.Errorf("failed to get karmada kubeconfig secret: %v", err)
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

if err = c.syncCertRotation(secret); err != nil {
klog.Errorf("Failed to rotate the certificate of karmada-agent for the given member cluster: %s, err is: %v", cluster.Name, err)
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

return controllerruntime.Result{RequeueAfter: c.CertRotationCheckingInterval}, nil
Expand Down
18 changes: 9 additions & 9 deletions pkg/controllers/cluster/cluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func (c *Controller) Reconcile(ctx context.Context, req controllerruntime.Reques
return controllerruntime.Result{}, nil
}

return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

if !cluster.DeletionTimestamp.IsZero() {
Expand Down Expand Up @@ -225,13 +225,13 @@ func (c *Controller) syncCluster(ctx context.Context, cluster *clusterv1alpha1.C
err := c.createExecutionSpace(cluster)
if err != nil {
c.EventRecorder.Event(cluster, corev1.EventTypeWarning, events.EventReasonCreateExecutionSpaceFailed, err.Error())
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

// taint cluster by condition
err = c.taintClusterByCondition(ctx, cluster)
if err != nil {
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

// ensure finalizer
Expand All @@ -242,20 +242,20 @@ func (c *Controller) removeCluster(ctx context.Context, cluster *clusterv1alpha1
// add terminating taint before cluster is deleted
if err := c.updateClusterTaints(ctx, []*corev1.Taint{TerminatingTaintTemplate}, nil, cluster); err != nil {
klog.ErrorS(err, "Failed to update terminating taint", "cluster", cluster.Name)
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

if err := c.removeExecutionSpace(cluster); err != nil {
klog.Errorf("Failed to remove execution space %s: %v", cluster.Name, err)
c.EventRecorder.Event(cluster, corev1.EventTypeWarning, events.EventReasonRemoveExecutionSpaceFailed, err.Error())
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}
msg := fmt.Sprintf("Removed execution space for cluster(%s).", cluster.Name)
c.EventRecorder.Event(cluster, corev1.EventTypeNormal, events.EventReasonRemoveExecutionSpaceSucceed, msg)

if exist, err := c.ExecutionSpaceExistForCluster(cluster.Name); err != nil {
klog.Errorf("Failed to check weather the execution space exist in the given member cluster or not, error is: %v", err)
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
} else if exist {
klog.Infof("Requeuing operation until the cluster(%s) execution space deleted", cluster.Name)
return controllerruntime.Result{RequeueAfter: c.ExecutionSpaceRetryFrequency}, nil
Expand All @@ -268,7 +268,7 @@ func (c *Controller) removeCluster(ctx context.Context, cluster *clusterv1alpha1
if c.EnableTaintManager {
if done, err := c.isTargetClusterRemoved(ctx, cluster); err != nil {
klog.ErrorS(err, "Failed to check whether target cluster is removed from bindings", "cluster", cluster.Name)
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
} else if !done {
klog.InfoS("Terminating taint eviction process has not finished yet, will try again later", "cluster", cluster.Name)
return controllerruntime.Result{RequeueAfter: c.ClusterTaintEvictionRetryFrequency}, nil
Expand Down Expand Up @@ -386,7 +386,7 @@ func (c *Controller) removeFinalizer(cluster *clusterv1alpha1.Cluster) (controll
controllerutil.RemoveFinalizer(cluster, util.ClusterControllerFinalizer)
err := c.Client.Update(context.TODO(), cluster)
if err != nil {
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

return controllerruntime.Result{}, nil
Expand All @@ -400,7 +400,7 @@ func (c *Controller) ensureFinalizer(cluster *clusterv1alpha1.Cluster) (controll
controllerutil.AddFinalizer(cluster, util.ClusterControllerFinalizer)
err := c.Client.Update(context.TODO(), cluster)
if err != nil {
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

return controllerruntime.Result{}, nil
Expand Down
6 changes: 3 additions & 3 deletions pkg/controllers/cluster/taint_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func (tc *NoExecuteTaintManager) Reconcile(ctx context.Context, req reconcile.Re
if apierrors.IsNotFound(err) {
return controllerruntime.Result{}, nil
}
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

// Check whether the target cluster has no execute taints.
Expand All @@ -85,7 +85,7 @@ func (tc *NoExecuteTaintManager) syncCluster(ctx context.Context, cluster *clust
Selector: fields.OneTermEqualSelector(rbClusterKeyIndex, cluster.Name),
}); err != nil {
klog.ErrorS(err, "Failed to list ResourceBindings", "cluster", cluster.Name)
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}
for i := range rbList.Items {
key, err := keys.FederatedKeyFunc(cluster.Name, &rbList.Items[i])
Expand All @@ -102,7 +102,7 @@ func (tc *NoExecuteTaintManager) syncCluster(ctx context.Context, cluster *clust
Selector: fields.OneTermEqualSelector(crbClusterKeyIndex, cluster.Name),
}); err != nil {
klog.ErrorS(err, "Failed to list ClusterResourceBindings", "cluster", cluster.Name)
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}
for i := range crbList.Items {
key, err := keys.FederatedKeyFunc(cluster.Name, &crbList.Items[i])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (c *CronFHPAController) Reconcile(ctx context.Context, req controllerruntim
}

klog.Errorf("Fail to get CronFederatedHPA(%s):%v", req.NamespacedName, err)
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

// If this CronFederatedHPA is deleting, stop all related cron executors
Expand Down Expand Up @@ -92,7 +92,7 @@ func (c *CronFHPAController) Reconcile(ctx context.Context, req controllerruntim
newRuleSets := sets.New[string]()
for _, rule := range cronFHPA.Spec.Rules {
if err = c.processCronRule(cronFHPA, rule); err != nil {
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}
newRuleSets.Insert(rule.Name)
}
Expand All @@ -104,7 +104,7 @@ func (c *CronFHPAController) Reconcile(ctx context.Context, req controllerruntim
}
c.CronHandler.StopRuleExecutor(req.NamespacedName.String(), name)
if err = c.removeCronFHPAHistory(cronFHPA, name); err != nil {
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}
}

Expand Down
16 changes: 8 additions & 8 deletions pkg/controllers/execution/execution_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,19 @@ func (c *Controller) Reconcile(ctx context.Context, req controllerruntime.Reques
return controllerruntime.Result{}, nil
}

return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

clusterName, err := names.GetClusterName(work.Namespace)
if err != nil {
klog.Errorf("Failed to get member cluster name for work %s/%s", work.Namespace, work.Name)
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

cluster, err := util.GetCluster(c.Client, clusterName)
if err != nil {
klog.Errorf("Failed to get the given member cluster %s", clusterName)
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

if !work.DeletionTimestamp.IsZero() {
Expand All @@ -100,18 +100,18 @@ func (c *Controller) Reconcile(ctx context.Context, req controllerruntime.Reques
err := c.tryDeleteWorkload(clusterName, work)
if err != nil {
klog.Errorf("Failed to delete work %v, namespace is %v, err is %v", work.Name, work.Namespace, err)
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}
} else if cluster.DeletionTimestamp.IsZero() { // cluster is unready, but not terminating
return controllerruntime.Result{Requeue: true}, fmt.Errorf("cluster(%s) not ready", cluster.Name)
return controllerruntime.Result{}, fmt.Errorf("cluster(%s) not ready", cluster.Name)
}

return c.removeFinalizer(work)
}

if !util.IsClusterReady(&cluster.Status) {
klog.Errorf("Stop sync work(%s/%s) for cluster(%s) as cluster not ready.", work.Namespace, work.Name, cluster.Name)
return controllerruntime.Result{Requeue: true}, fmt.Errorf("cluster(%s) not ready", cluster.Name)
return controllerruntime.Result{}, fmt.Errorf("cluster(%s) not ready", cluster.Name)
}

return c.syncWork(clusterName, work)
Expand All @@ -136,7 +136,7 @@ func (c *Controller) syncWork(clusterName string, work *workv1alpha1.Work) (cont
msg := fmt.Sprintf("Failed to sync work(%s) to cluster(%s): %v", work.Name, clusterName, err)
klog.Errorf(msg)
c.EventRecorder.Event(work, corev1.EventTypeWarning, events.EventReasonSyncWorkloadFailed, msg)
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}
msg := fmt.Sprintf("Sync work (%s) to cluster(%s) successful.", work.Name, clusterName)
klog.V(4).Infof(msg)
Expand Down Expand Up @@ -194,7 +194,7 @@ func (c *Controller) removeFinalizer(work *workv1alpha1.Work) (controllerruntime
controllerutil.RemoveFinalizer(work, util.ExecutionControllerFinalizer)
err := c.Client.Update(context.TODO(), work)
if err != nil {
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}
return controllerruntime.Result{}, nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (c *StatusController) Reconcile(ctx context.Context, req controllerruntime.
if apierrors.IsNotFound(err) {
return controllerruntime.Result{}, nil
}
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}

if !quota.DeletionTimestamp.IsZero() {
Expand All @@ -80,7 +80,7 @@ func (c *StatusController) Reconcile(ctx context.Context, req controllerruntime.
if err := c.collectQuotaStatus(quota); err != nil {
klog.Errorf("Failed to collect status from works to federatedResourceQuota(%s), error: %v", req.NamespacedName.String(), err)
c.EventRecorder.Eventf(quota, corev1.EventTypeWarning, events.EventReasonCollectFederatedResourceQuotaStatusFailed, err.Error())
return controllerruntime.Result{Requeue: true}, err
return controllerruntime.Result{}, err
}
c.EventRecorder.Eventf(quota, corev1.EventTypeNormal, events.EventReasonCollectFederatedResourceQuotaStatusSucceed, "Collect status of FederatedResourceQuota(%s) succeed.", req.NamespacedName.String())
return controllerruntime.Result{}, nil
Expand Down
Loading

0 comments on commit 7c2b3e1

Please sign in to comment.