From d15d007da6b090d75df7e2d44e728e7b26717d28 Mon Sep 17 00:00:00 2001 From: qmhu Date: Wed, 28 Dec 2022 16:41:40 +0800 Subject: [PATCH 1/3] fix the update status error for ehpa --- .../ehpa/effective_hpa_controller.go | 35 ++++++++++++++----- pkg/controller/ehpa/substitute.go | 2 +- .../recommendation_controller.go | 2 +- .../recommendation_rule_controller.go | 4 +-- 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/pkg/controller/ehpa/effective_hpa_controller.go b/pkg/controller/ehpa/effective_hpa_controller.go index 70c6d6deb..6400d2edf 100644 --- a/pkg/controller/ehpa/effective_hpa_controller.go +++ b/pkg/controller/ehpa/effective_hpa_controller.go @@ -10,10 +10,12 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/version" "k8s.io/client-go/discovery" "k8s.io/client-go/scale" "k8s.io/client-go/tools/record" + "k8s.io/client-go/util/retry" "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" @@ -152,17 +154,34 @@ func (c *EffectiveHPAController) Reconcile(ctx context.Context, req ctrl.Request } func (c *EffectiveHPAController) UpdateStatus(ctx context.Context, ehpa *autoscalingapi.EffectiveHorizontalPodAutoscaler, newStatus *autoscalingapi.EffectiveHorizontalPodAutoscalerStatus) { - if !equality.Semantic.DeepEqual(&ehpa.Status, newStatus) { - ehpa.Status = *newStatus - err := c.Status().Update(ctx, ehpa) - if err != nil { - c.Recorder.Event(ehpa, v1.EventTypeWarning, "FailedUpdateStatus", err.Error()) - klog.Errorf("Failed to update status, ehpa %s error %v", klog.KObj(ehpa), err) - return + ehpaCopy := ehpa.DeepCopy() + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + if !equality.Semantic.DeepEqual(&ehpaCopy.Status, newStatus) { + ehpaCopy.Status = *newStatus + err := c.Status().Update(ctx, ehpaCopy) + if err == nil { + return nil + } + + updated := &autoscalingapi.EffectiveHorizontalPodAutoscaler{} + errGet := c.Get(context.TODO(), types.NamespacedName{Namespace: ehpaCopy.Namespace, Name: ehpaCopy.Name}, updated) + if errGet == nil { + ehpaCopy = updated + } + + return err } - klog.Infof("Update EffectiveHorizontalPodAutoscaler status successful, ehpa %s", klog.KObj(ehpa)) + return nil + }) + + if err != nil { + c.Recorder.Event(ehpa, v1.EventTypeWarning, "FailedUpdateStatus", err.Error()) + klog.Errorf("Failed to update status, EffectiveHorizontalPodAutoscaler %s error %v", klog.KObj(ehpa), err) + return } + + klog.V(2).Infof("Update EffectiveHorizontalPodAutoscaler %s status successful ", klog.KObj(ehpa)) } func (c *EffectiveHPAController) SetupWithManager(mgr ctrl.Manager) error { diff --git a/pkg/controller/ehpa/substitute.go b/pkg/controller/ehpa/substitute.go index 9f67586bf..90398e7c6 100644 --- a/pkg/controller/ehpa/substitute.go +++ b/pkg/controller/ehpa/substitute.go @@ -97,7 +97,7 @@ func (c *EffectiveHPAController) UpdateSubstituteIfNeed(ctx context.Context, ehp return nil, err } - klog.Infof("Update Substitute successful, Substitute %s", klog.KObj(substituteExist)) + klog.V(2).Infof("Update Substitute successful, Substitute %s", klog.KObj(substituteExist)) } return substituteExist, nil diff --git a/pkg/controller/recommendation/recommendation_controller.go b/pkg/controller/recommendation/recommendation_controller.go index ade8ca572..7e9e1ceca 100644 --- a/pkg/controller/recommendation/recommendation_controller.go +++ b/pkg/controller/recommendation/recommendation_controller.go @@ -87,7 +87,7 @@ func (c *RecommendationController) UpdateStatus(ctx context.Context, recommendat return err } - klog.Infof("Update Recommendation status successful, Recommendation %s", klog.KObj(recommendation)) + klog.V(2).Infof("Update Recommendation status successful, Recommendation %s", klog.KObj(recommendation)) } return nil diff --git a/pkg/controller/recommendation/recommendation_rule_controller.go b/pkg/controller/recommendation/recommendation_rule_controller.go index 79652c8ce..712d523c1 100644 --- a/pkg/controller/recommendation/recommendation_rule_controller.go +++ b/pkg/controller/recommendation/recommendation_rule_controller.go @@ -414,7 +414,7 @@ func (c *RecommendationRuleController) executeMission(ctx context.Context, wg *s } func (c *RecommendationRuleController) UpdateStatus(ctx context.Context, recommendationRule *analysisv1alph1.RecommendationRule, newStatus *analysisv1alph1.RecommendationRuleStatus) { - klog.Infof("Updating RecommendationRule %s status", klog.KObj(recommendationRule)) + klog.V(2).Infof("Updating RecommendationRule %s status", klog.KObj(recommendationRule)) recommendationRuleCopy := recommendationRule.DeepCopy() err := retry.RetryOnConflict(retry.DefaultRetry, func() error { if !equality.Semantic.DeepEqual(&recommendationRuleCopy.Status, newStatus) { @@ -442,7 +442,7 @@ func (c *RecommendationRuleController) UpdateStatus(ctx context.Context, recomme return } - klog.Infof("Update RecommendationRule status successful, RecommendationRule %s", klog.KObj(recommendationRule)) + klog.V(2).Infof("Update RecommendationRule status successful, RecommendationRule %s", klog.KObj(recommendationRule)) } type ObjectIdentity struct { From e977b5e0918c1b535df85a75476ef9da589e1a4f Mon Sep 17 00:00:00 2001 From: qmhu Date: Thu, 5 Jan 2023 19:22:43 +0800 Subject: [PATCH 2/3] improvement hpa and substitute --- .../ehpa/effective_hpa_controller.go | 23 +++++++++---------- pkg/controller/ehpa/hpa.go | 20 +++++++++++++++- pkg/controller/ehpa/substitute_controller.go | 21 +++++++++++++++-- 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/pkg/controller/ehpa/effective_hpa_controller.go b/pkg/controller/ehpa/effective_hpa_controller.go index 6400d2edf..2d8361c6b 100644 --- a/pkg/controller/ehpa/effective_hpa_controller.go +++ b/pkg/controller/ehpa/effective_hpa_controller.go @@ -154,9 +154,9 @@ func (c *EffectiveHPAController) Reconcile(ctx context.Context, req ctrl.Request } func (c *EffectiveHPAController) UpdateStatus(ctx context.Context, ehpa *autoscalingapi.EffectiveHorizontalPodAutoscaler, newStatus *autoscalingapi.EffectiveHorizontalPodAutoscalerStatus) { - ehpaCopy := ehpa.DeepCopy() - err := retry.RetryOnConflict(retry.DefaultRetry, func() error { - if !equality.Semantic.DeepEqual(&ehpaCopy.Status, newStatus) { + if !equality.Semantic.DeepEqual(&ehpa.Status, newStatus) { + ehpaCopy := ehpa.DeepCopy() + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { ehpaCopy.Status = *newStatus err := c.Status().Update(ctx, ehpaCopy) if err == nil { @@ -170,18 +170,17 @@ func (c *EffectiveHPAController) UpdateStatus(ctx context.Context, ehpa *autosca } return err - } - return nil - }) + }) - if err != nil { - c.Recorder.Event(ehpa, v1.EventTypeWarning, "FailedUpdateStatus", err.Error()) - klog.Errorf("Failed to update status, EffectiveHorizontalPodAutoscaler %s error %v", klog.KObj(ehpa), err) - return - } + if err != nil { + c.Recorder.Event(ehpa, v1.EventTypeWarning, "FailedUpdateStatus", err.Error()) + klog.Errorf("Failed to update status, EffectiveHorizontalPodAutoscaler %s error %v", klog.KObj(ehpa), err) + return + } - klog.V(2).Infof("Update EffectiveHorizontalPodAutoscaler %s status successful ", klog.KObj(ehpa)) + klog.V(2).Infof("Update EffectiveHorizontalPodAutoscaler %s status successful ", klog.KObj(ehpa)) + } } func (c *EffectiveHPAController) SetupWithManager(mgr ctrl.Manager) error { diff --git a/pkg/controller/ehpa/hpa.go b/pkg/controller/ehpa/hpa.go index 5abb2822b..b0e57d2b6 100644 --- a/pkg/controller/ehpa/hpa.go +++ b/pkg/controller/ehpa/hpa.go @@ -11,6 +11,8 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/retry" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -161,12 +163,28 @@ func (c *EffectiveHPAController) UpdateHPAIfNeed(ctx context.Context, ehpa *auto } if needUpdate { - err := c.Update(ctx, hpaExist) + hpaCopy := hpaExist.DeepCopy() + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + err := c.Update(ctx, hpaCopy) + if err == nil { + return nil + } + + updated := &autoscalingv2.HorizontalPodAutoscaler{} + errGet := c.Get(context.TODO(), types.NamespacedName{Namespace: hpaCopy.Namespace, Name: hpaCopy.Name}, updated) + if errGet == nil { + hpaCopy = updated + } + + return err + }) + if err != nil { c.Recorder.Event(ehpa, v1.EventTypeWarning, "FailedUpdateHPA", err.Error()) klog.Errorf("Failed to update HorizontalPodAutoscaler %s error %v", klog.KObj(hpaExist), err) return nil, err } + klog.Infof("Update HorizontalPodAutoscaler successful, HorizontalPodAutoscaler %s", klog.KObj(hpaExist)) } diff --git a/pkg/controller/ehpa/substitute_controller.go b/pkg/controller/ehpa/substitute_controller.go index ff09bb60d..1207281c8 100644 --- a/pkg/controller/ehpa/substitute_controller.go +++ b/pkg/controller/ehpa/substitute_controller.go @@ -8,8 +8,10 @@ import ( "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/scale" "k8s.io/client-go/tools/record" + "k8s.io/client-go/util/retry" "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" @@ -72,8 +74,23 @@ func (c *SubstituteController) Reconcile(ctx context.Context, req ctrl.Request) } if !equality.Semantic.DeepEqual(&substitute.Status, &newStatus) { - substitute.Status = newStatus - err := c.Status().Update(ctx, substitute) + substituteCopy := substitute.DeepCopy() + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + substituteCopy.Status = newStatus + err := c.Status().Update(ctx, substituteCopy) + if err == nil { + return nil + } + + updated := &autoscalingapi.Substitute{} + errGet := c.Get(context.TODO(), types.NamespacedName{Namespace: substituteCopy.Namespace, Name: substituteCopy.Name}, updated) + if errGet == nil { + substituteCopy = updated + } + + return err + }) + if err != nil { c.Recorder.Event(substitute, v1.EventTypeWarning, "FailedUpdateStatus", err.Error()) klog.Errorf("Failed to update status, Substitute %s error %v", klog.KObj(substitute), err) From 5103f25d7b910afc461dfa4c125b9db6ada5a232 Mon Sep 17 00:00:00 2001 From: qmhu Date: Wed, 11 Jan 2023 18:47:19 +0800 Subject: [PATCH 3/3] fix tsp update --- .../recommendation_rule_controller.go | 24 +++++++++---------- pkg/controller/timeseriesprediction/status.go | 21 ++++++++++++++-- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/pkg/controller/recommendation/recommendation_rule_controller.go b/pkg/controller/recommendation/recommendation_rule_controller.go index 712d523c1..7bfdbfbb7 100644 --- a/pkg/controller/recommendation/recommendation_rule_controller.go +++ b/pkg/controller/recommendation/recommendation_rule_controller.go @@ -414,10 +414,10 @@ func (c *RecommendationRuleController) executeMission(ctx context.Context, wg *s } func (c *RecommendationRuleController) UpdateStatus(ctx context.Context, recommendationRule *analysisv1alph1.RecommendationRule, newStatus *analysisv1alph1.RecommendationRuleStatus) { - klog.V(2).Infof("Updating RecommendationRule %s status", klog.KObj(recommendationRule)) - recommendationRuleCopy := recommendationRule.DeepCopy() - err := retry.RetryOnConflict(retry.DefaultRetry, func() error { - if !equality.Semantic.DeepEqual(&recommendationRuleCopy.Status, newStatus) { + if !equality.Semantic.DeepEqual(&recommendationRule.Status, newStatus) { + klog.V(2).Infof("Updating RecommendationRule %s status", klog.KObj(recommendationRule)) + recommendationRuleCopy := recommendationRule.DeepCopy() + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { recommendationRuleCopy.Status = *newStatus err := c.Update(ctx, recommendationRuleCopy) if err == nil { @@ -431,18 +431,16 @@ func (c *RecommendationRuleController) UpdateStatus(ctx context.Context, recomme } return err - } + }) - return nil - }) + if err != nil { + c.Recorder.Event(recommendationRule, corev1.EventTypeWarning, "FailedUpdateStatus", err.Error()) + klog.Errorf("Failed to update status, RecommendationRule %s error %v", klog.KObj(recommendationRule), err) + return + } - if err != nil { - c.Recorder.Event(recommendationRule, corev1.EventTypeWarning, "FailedUpdateStatus", err.Error()) - klog.Errorf("Failed to update status, RecommendationRule %s error %v", klog.KObj(recommendationRule), err) - return + klog.V(2).Infof("Update RecommendationRule status successful, RecommendationRule %s", klog.KObj(recommendationRule)) } - - klog.V(2).Infof("Update RecommendationRule status successful, RecommendationRule %s", klog.KObj(recommendationRule)) } type ObjectIdentity struct { diff --git a/pkg/controller/timeseriesprediction/status.go b/pkg/controller/timeseriesprediction/status.go index f0479014b..73d6e9086 100644 --- a/pkg/controller/timeseriesprediction/status.go +++ b/pkg/controller/timeseriesprediction/status.go @@ -11,7 +11,9 @@ import ( "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" utilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/client-go/util/retry" "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -173,8 +175,23 @@ func (tc *Controller) doPredict(tsPrediction *predictionapi.TimeSeriesPrediction func (tc *Controller) UpdateStatus(ctx context.Context, tsPrediction *predictionapi.TimeSeriesPrediction, newStatus *predictionapi.TimeSeriesPredictionStatus) error { if !equality.Semantic.DeepEqual(&tsPrediction.Status, newStatus) { - tsPrediction.Status = *newStatus - err := tc.Client.Status().Update(ctx, tsPrediction) + tsPredictionCopy := tsPrediction.DeepCopy() + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + tsPredictionCopy.Status = *newStatus + err := tc.Client.Status().Update(ctx, tsPredictionCopy) + if err == nil { + return nil + } + + updated := &predictionapi.TimeSeriesPrediction{} + errGet := tc.Get(context.TODO(), types.NamespacedName{Namespace: tsPredictionCopy.Namespace, Name: tsPredictionCopy.Name}, updated) + if errGet == nil { + tsPredictionCopy = updated + } + + return err + }) + if err != nil { tc.Recorder.Event(tsPrediction, v1.EventTypeWarning, "FailedUpdateStatus", err.Error()) klog.Errorf("Failed to update status for %v", klog.KObj(tsPrediction))