From b5a62e20a1f8f3cc4c6b9352f88cba4fefe0274e Mon Sep 17 00:00:00 2001 From: free6om Date: Mon, 23 Oct 2023 11:49:06 +0800 Subject: [PATCH] fixed --- controllers/apps/cluster_controller.go | 4 ++ controllers/apps/cluster_plan_builder.go | 38 ++++++------------- .../replicatedstatemachine_controller.go | 4 ++ pkg/controller/rsm/plan_builder.go | 1 - 4 files changed, 19 insertions(+), 28 deletions(-) diff --git a/controllers/apps/cluster_controller.go b/controllers/apps/cluster_controller.go index a41a3ededc2..1bc93ca6177 100644 --- a/controllers/apps/cluster_controller.go +++ b/controllers/apps/cluster_controller.go @@ -28,6 +28,7 @@ import ( corev1 "k8s.io/api/core/v1" policyv1 "k8s.io/api/policy/v1" rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" @@ -139,6 +140,9 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct if re, ok := err.(intctrlutil.RequeueError); ok { return intctrlutil.RequeueAfter(re.RequeueAfter(), reqCtx.Log, re.Reason()) } + if apierrors.IsConflict(err) { + return intctrlutil.Requeue(reqCtx.Log, err.Error()) + } return intctrlutil.RequeueWithError(err, reqCtx.Log, "") } diff --git a/controllers/apps/cluster_plan_builder.go b/controllers/apps/cluster_plan_builder.go index bb04e036abd..25dbbb71daa 100644 --- a/controllers/apps/cluster_plan_builder.go +++ b/controllers/apps/cluster_plan_builder.go @@ -56,8 +56,6 @@ const ( clusterWeight ) -// TODO: cluster plan builder can be abstracted as a common flow - // clusterTransformContext a graph.TransformContext implementation for Cluster reconciliation type clusterTransformContext struct { context.Context @@ -233,21 +231,21 @@ func NewClusterPlanBuilder(ctx intctrlutil.RequestCtx, cli client.Client, req ct func (c *clusterPlanBuilder) defaultWalkFuncWithLogging(vertex graph.Vertex) error { node, ok := vertex.(*model.ObjectVertex) err := c.defaultWalkFunc(vertex) - if err != nil { - if !ok { - c.transCtx.Logger.Error(err, "") - } else { - if node.Action == nil { - c.transCtx.Logger.Error(err, fmt.Sprintf("%T", node)) - } else { - c.transCtx.Logger.Error(err, fmt.Sprintf("%s %T error", *node.Action, node.Obj)) - } - } + switch { + case err == nil: + return err + case !ok: + c.transCtx.Logger.Error(err, "") + case node.Action == nil: + c.transCtx.Logger.Error(err, fmt.Sprintf("%T", node)) + case apierrors.IsConflict(err): + return err + default: + c.transCtx.Logger.Error(err, fmt.Sprintf("%s %T error", *node.Action, node.Obj)) } return err } -// TODO: retry strategy on error func (c *clusterPlanBuilder) defaultWalkFunc(vertex graph.Vertex) error { node, ok := vertex.(*model.ObjectVertex) if !ok { @@ -281,7 +279,6 @@ func (c *clusterPlanBuilder) reconcileObject(node *model.ObjectVertex) error { case model.PATCH: patch := client.MergeFrom(node.OriObj) if err := c.cli.Patch(c.transCtx.Context, node.Obj, patch); err != nil && !apierrors.IsNotFound(err) { - c.transCtx.Logger.Error(err, fmt.Sprintf("patch %T error", node.OriObj)) return err } case model.DELETE: @@ -294,7 +291,6 @@ func (c *clusterPlanBuilder) reconcileObject(node *model.ObjectVertex) error { // delete secondary objects if _, ok := node.Obj.(*appsv1alpha1.Cluster); !ok { err := intctrlutil.BackgroundDeleteObject(c.cli, c.transCtx.Context, node.Obj) - // err := c.cli.Delete(c.transCtx.Context, node.obj) if err != nil && !apierrors.IsNotFound(err) { return err } @@ -323,20 +319,8 @@ func (c *clusterPlanBuilder) reconcileCluster(node *model.ObjectVertex) error { // cluster.meta and cluster.spec might change case model.STATUS: if !reflect.DeepEqual(cluster.ObjectMeta, origCluster.ObjectMeta) || !reflect.DeepEqual(cluster.Spec, origCluster.Spec) { - // TODO: we should Update instead of Patch cluster object, - // TODO: but Update failure happens too frequently as other controllers are updating cluster object too. - // TODO: use Patch here, revert to Update after refactoring done - // if err := c.cli.Update(c.ctx.Ctx, cluster); err != nil { - // tmpCluster := &appsv1alpha1.Cluster{} - // err = c.cli.Get(c.ctx.Ctx,client.ObjectKeyFromObject(origCluster), tmpCluster) - // c.ctx.Log.Error(err, fmt.Sprintf("update %T error, orig: %v, curr: %v, api-server: %v", origCluster, origCluster, cluster, tmpCluster)) - // return err - // } patch := client.MergeFrom(origCluster.DeepCopy()) if err := c.cli.Patch(c.transCtx.Context, cluster, patch); err != nil { - // log for debug - // TODO:(free6om) make error message smaller when refactor done. - c.transCtx.Logger.Error(err, fmt.Sprintf("patch %T error, orig: %v, curr: %v", origCluster, origCluster, cluster)) return err } } diff --git a/controllers/workloads/replicatedstatemachine_controller.go b/controllers/workloads/replicatedstatemachine_controller.go index 4c66f486748..dafccaa6483 100644 --- a/controllers/workloads/replicatedstatemachine_controller.go +++ b/controllers/workloads/replicatedstatemachine_controller.go @@ -25,6 +25,7 @@ import ( appsv1 "k8s.io/api/apps/v1" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" @@ -74,6 +75,9 @@ func (r *ReplicatedStateMachineReconciler) Reconcile(ctx context.Context, req ct if re, ok := err.(model.RequeueError); ok { return intctrlutil.RequeueAfter(re.RequeueAfter(), reqCtx.Log, re.Reason()) } + if apierrors.IsConflict(err) { + return intctrlutil.Requeue(reqCtx.Log, err.Error()) + } return intctrlutil.CheckedRequeueWithError(err, reqCtx.Log, "") } diff --git a/pkg/controller/rsm/plan_builder.go b/pkg/controller/rsm/plan_builder.go index c3c1e6d6196..eb028fadc33 100644 --- a/pkg/controller/rsm/plan_builder.go +++ b/pkg/controller/rsm/plan_builder.go @@ -120,7 +120,6 @@ func (b *PlanBuilder) rsmWalkFunc(v graph.Vertex) error { case model.UPDATE: err := b.cli.Update(b.transCtx.Context, vertex.Obj) if err != nil && !apierrors.IsNotFound(err) { - b.transCtx.Logger.Error(err, fmt.Sprintf("update %T error: %s", vertex.Obj, vertex.OriObj.GetName())) return err } case model.DELETE: