From ad98d85f8607f7a736199a6677ce15c4f048fee6 Mon Sep 17 00:00:00 2001 From: guofei Date: Mon, 19 Sep 2022 23:08:06 -0700 Subject: [PATCH 1/4] fix: Sink IsConflict error when updating IMC/MC status --- .../internalmembercluster/member_controller.go | 8 ++++++-- pkg/controllers/membercluster/membercluster_controller.go | 4 +++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/pkg/controllers/internalmembercluster/member_controller.go b/pkg/controllers/internalmembercluster/member_controller.go index 2f73a6684..cd106f918 100644 --- a/pkg/controllers/internalmembercluster/member_controller.go +++ b/pkg/controllers/internalmembercluster/member_controller.go @@ -77,7 +77,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu updateHealthErr := r.updateHealth(ctx, &imc) r.markInternalMemberClusterJoined(&imc) if err := r.updateInternalMemberClusterWithRetry(ctx, &imc); err != nil { - klog.ErrorS(err, "failed to update status for %s", klog.KObj(&imc)) + if !apierrors.IsConflict(err) { + klog.ErrorS(err, "failed to update status for %s", klog.KObj(&imc)) + } return ctrl.Result{}, client.IgnoreNotFound(err) } if updateHealthErr != nil { @@ -92,7 +94,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } r.markInternalMemberClusterLeft(&imc) if err := r.updateInternalMemberClusterWithRetry(ctx, &imc); err != nil { - klog.ErrorS(err, "failed to update status for %s", klog.KObj(&imc)) + if !apierrors.IsConflict(err) { + klog.ErrorS(err, "failed to update status for %s", klog.KObj(&imc)) + } return ctrl.Result{}, client.IgnoreNotFound(err) } return ctrl.Result{}, nil diff --git a/pkg/controllers/membercluster/membercluster_controller.go b/pkg/controllers/membercluster/membercluster_controller.go index f0aa5cce8..8da1cb03a 100644 --- a/pkg/controllers/membercluster/membercluster_controller.go +++ b/pkg/controllers/membercluster/membercluster_controller.go @@ -117,7 +117,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu // Copy status from InternalMemberCluster to MemberCluster. r.syncInternalMemberClusterStatus(currentImc, mc) if err := r.updateMemberClusterStatus(ctx, mc); err != nil { - klog.ErrorS(err, "failed to update status for", klog.KObj(mc)) + if !apierrors.IsConflict(err) { + klog.ErrorS(err, "failed to update status for", klog.KObj(mc)) + } return ctrl.Result{}, client.IgnoreNotFound(err) } From da5febafd18e1fe352abbee7eff372a13739e80f Mon Sep 17 00:00:00 2001 From: guofei Date: Tue, 20 Sep 2022 16:28:55 -0700 Subject: [PATCH 2/4] Add log for update failures due to conflicts --- .../internalmembercluster/member_controller.go | 8 ++++++-- pkg/controllers/membercluster/membercluster_controller.go | 4 +++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/pkg/controllers/internalmembercluster/member_controller.go b/pkg/controllers/internalmembercluster/member_controller.go index cd106f918..0a973c666 100644 --- a/pkg/controllers/internalmembercluster/member_controller.go +++ b/pkg/controllers/internalmembercluster/member_controller.go @@ -77,7 +77,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu updateHealthErr := r.updateHealth(ctx, &imc) r.markInternalMemberClusterJoined(&imc) if err := r.updateInternalMemberClusterWithRetry(ctx, &imc); err != nil { - if !apierrors.IsConflict(err) { + if apierrors.IsConflict(err) { + klog.V(2).InfoS("failed to update status due to conflicts", "name", imc.Name) + } else { klog.ErrorS(err, "failed to update status for %s", klog.KObj(&imc)) } return ctrl.Result{}, client.IgnoreNotFound(err) @@ -94,7 +96,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } r.markInternalMemberClusterLeft(&imc) if err := r.updateInternalMemberClusterWithRetry(ctx, &imc); err != nil { - if !apierrors.IsConflict(err) { + if apierrors.IsConflict(err) { + klog.V(2).InfoS("failed to update status due to conflicts", "name", imc.Name) + } else { klog.ErrorS(err, "failed to update status for %s", klog.KObj(&imc)) } return ctrl.Result{}, client.IgnoreNotFound(err) diff --git a/pkg/controllers/membercluster/membercluster_controller.go b/pkg/controllers/membercluster/membercluster_controller.go index 8da1cb03a..a7b8435dc 100644 --- a/pkg/controllers/membercluster/membercluster_controller.go +++ b/pkg/controllers/membercluster/membercluster_controller.go @@ -117,7 +117,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu // Copy status from InternalMemberCluster to MemberCluster. r.syncInternalMemberClusterStatus(currentImc, mc) if err := r.updateMemberClusterStatus(ctx, mc); err != nil { - if !apierrors.IsConflict(err) { + if apierrors.IsConflict(err) { + klog.V(2).InfoS("failed to update status due to conflicts", "name", mc.Name) + } else { klog.ErrorS(err, "failed to update status for", klog.KObj(mc)) } return ctrl.Result{}, client.IgnoreNotFound(err) From 1760406c7cae367d6f5d091681bb2e4d0aa904be Mon Sep 17 00:00:00 2001 From: Ryan Zhang Date: Wed, 21 Sep 2022 11:13:13 -0700 Subject: [PATCH 3/4] fix error format --- .../internalmembercluster/member_controller.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/controllers/internalmembercluster/member_controller.go b/pkg/controllers/internalmembercluster/member_controller.go index 0a973c666..87d8a0d91 100644 --- a/pkg/controllers/internalmembercluster/member_controller.go +++ b/pkg/controllers/internalmembercluster/member_controller.go @@ -78,14 +78,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu r.markInternalMemberClusterJoined(&imc) if err := r.updateInternalMemberClusterWithRetry(ctx, &imc); err != nil { if apierrors.IsConflict(err) { - klog.V(2).InfoS("failed to update status due to conflicts", "name", imc.Name) + klog.V(2).InfoS("failed to update status due to conflicts", "imc", klog.KObj(&imc)) } else { - klog.ErrorS(err, "failed to update status for %s", klog.KObj(&imc)) + klog.ErrorS(err, "failed to update status", "imc", klog.KObj(&imc)) } return ctrl.Result{}, client.IgnoreNotFound(err) } if updateHealthErr != nil { - klog.ErrorS(updateHealthErr, "failed to update health for %s", klog.KObj(&imc)) + klog.ErrorS(updateHealthErr, "failed to update health", "imc", klog.KObj(&imc)) return ctrl.Result{}, updateHealthErr } return ctrl.Result{RequeueAfter: time.Second * time.Duration(imc.Spec.HeartbeatPeriodSeconds)}, nil @@ -97,9 +97,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu r.markInternalMemberClusterLeft(&imc) if err := r.updateInternalMemberClusterWithRetry(ctx, &imc); err != nil { if apierrors.IsConflict(err) { - klog.V(2).InfoS("failed to update status due to conflicts", "name", imc.Name) + klog.V(2).InfoS("failed to update status due to conflicts", "imc", klog.KObj(&imc)) } else { - klog.ErrorS(err, "failed to update status for %s", klog.KObj(&imc)) + klog.ErrorS(err, "failed to update status", "imc", klog.KObj(&imc)) } return ctrl.Result{}, client.IgnoreNotFound(err) } From 21a04487c42310a96409cd779a2044d9c7351b2d Mon Sep 17 00:00:00 2001 From: Ryan Zhang Date: Wed, 21 Sep 2022 11:14:16 -0700 Subject: [PATCH 4/4] fix mc log format --- pkg/controllers/membercluster/membercluster_controller.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/controllers/membercluster/membercluster_controller.go b/pkg/controllers/membercluster/membercluster_controller.go index a7b8435dc..4704e4602 100644 --- a/pkg/controllers/membercluster/membercluster_controller.go +++ b/pkg/controllers/membercluster/membercluster_controller.go @@ -118,9 +118,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu r.syncInternalMemberClusterStatus(currentImc, mc) if err := r.updateMemberClusterStatus(ctx, mc); err != nil { if apierrors.IsConflict(err) { - klog.V(2).InfoS("failed to update status due to conflicts", "name", mc.Name) + klog.V(2).InfoS("failed to update status due to conflicts", "memberCluster", mcObjRef) } else { - klog.ErrorS(err, "failed to update status for", klog.KObj(mc)) + klog.ErrorS(err, "failed to update status", "memberCluster", mcObjRef) } return ctrl.Result{}, client.IgnoreNotFound(err) }