From da0664728ccbf89a128397537b080de6dcddadd9 Mon Sep 17 00:00:00 2001 From: changzhen Date: Sat, 25 Feb 2023 18:16:11 +0800 Subject: [PATCH] the condition is updated only after scheduling Signed-off-by: changzhen --- pkg/scheduler/scheduler.go | 72 +++++++++++++++++------------------ pkg/util/helper/workstatus.go | 36 ++++++++++-------- 2 files changed, 57 insertions(+), 51 deletions(-) diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go index a19d7c52f..100576d62 100644 --- a/pkg/scheduler/scheduler.go +++ b/pkg/scheduler/scheduler.go @@ -342,24 +342,6 @@ func (s *Scheduler) doScheduleBinding(namespace, name string) (err error) { return err } - // Update "Scheduled" condition according to schedule result. - defer func() { - s.recordScheduleResultEventForResourceBinding(rb, err) - var condition metav1.Condition - if err == nil { - condition = util.NewCondition(workv1alpha2.Scheduled, scheduleSuccessReason, scheduleSuccessMessage, metav1.ConditionTrue) - } else { - condition = util.NewCondition(workv1alpha2.Scheduled, scheduleFailedReason, err.Error(), metav1.ConditionFalse) - } - if updateErr := s.patchBindingScheduleStatus(rb, condition); updateErr != nil { - klog.Errorf("Failed to patch schedule status to ResourceBinding(%s/%s): %v", rb.Namespace, rb.Name, err) - if err == nil { - // schedule succeed but update status failed, return err in order to retry in next loop. - err = updateErr - } - } - }() - start := time.Now() policyPlacement, policyPlacementStr, err := s.getPlacement(rb) if err != nil { @@ -404,24 +386,6 @@ func (s *Scheduler) doScheduleClusterBinding(name string) (err error) { return err } - // Update "Scheduled" condition according to schedule result. - defer func() { - s.recordScheduleResultEventForClusterResourceBinding(crb, err) - var condition metav1.Condition - if err == nil { - condition = util.NewCondition(workv1alpha2.Scheduled, scheduleSuccessReason, scheduleSuccessMessage, metav1.ConditionTrue) - } else { - condition = util.NewCondition(workv1alpha2.Scheduled, scheduleFailedReason, err.Error(), metav1.ConditionFalse) - } - if updateErr := s.patchClusterBindingScheduleStatus(crb, condition); updateErr != nil { - klog.Errorf("Failed to patch schedule status to ClusterResourceBinding(%s): %v", crb.Name, err) - if err == nil { - // schedule succeed but update status failed, return err in order to retry in next loop. - err = updateErr - } - } - }() - start := time.Now() policyPlacement, policyPlacementStr, err := s.getClusterPlacement(crb) if err != nil { @@ -460,6 +424,24 @@ func (s *Scheduler) scheduleResourceBinding(resourceBinding *workv1alpha2.Resour klog.V(4).InfoS("Begin scheduling resource binding", "resourceBinding", klog.KObj(resourceBinding)) defer klog.V(4).InfoS("End scheduling resource binding", "resourceBinding", klog.KObj(resourceBinding)) + // Update "Scheduled" condition according to schedule result. + defer func() { + s.recordScheduleResultEventForResourceBinding(resourceBinding, err) + var condition metav1.Condition + if err == nil { + condition = util.NewCondition(workv1alpha2.Scheduled, scheduleSuccessReason, scheduleSuccessMessage, metav1.ConditionTrue) + } else { + condition = util.NewCondition(workv1alpha2.Scheduled, scheduleFailedReason, err.Error(), metav1.ConditionFalse) + } + if updateErr := s.patchBindingScheduleStatus(resourceBinding, condition); updateErr != nil { + klog.Errorf("Failed to patch schedule status to ResourceBinding(%s/%s): %v", resourceBinding.Namespace, resourceBinding.Name, err) + if err == nil { + // schedule succeed but update status failed, return err in order to retry in next loop. + err = updateErr + } + } + }() + scheduleResult, err := s.Algorithm.Schedule(context.TODO(), &resourceBinding.Spec, &resourceBinding.Status, &core.ScheduleAlgorithmOption{EnableEmptyWorkloadPropagation: s.enableEmptyWorkloadPropagation}) var noClusterFit *framework.FitError @@ -506,6 +488,24 @@ func (s *Scheduler) scheduleClusterResourceBinding(clusterResourceBinding *workv klog.V(4).InfoS("Begin scheduling cluster resource binding", "clusterResourceBinding", klog.KObj(clusterResourceBinding)) defer klog.V(4).InfoS("End scheduling cluster resource binding", "clusterResourceBinding", klog.KObj(clusterResourceBinding)) + // Update "Scheduled" condition according to schedule result. + defer func() { + s.recordScheduleResultEventForClusterResourceBinding(clusterResourceBinding, err) + var condition metav1.Condition + if err == nil { + condition = util.NewCondition(workv1alpha2.Scheduled, scheduleSuccessReason, scheduleSuccessMessage, metav1.ConditionTrue) + } else { + condition = util.NewCondition(workv1alpha2.Scheduled, scheduleFailedReason, err.Error(), metav1.ConditionFalse) + } + if updateErr := s.patchClusterBindingScheduleStatus(clusterResourceBinding, condition); updateErr != nil { + klog.Errorf("Failed to patch schedule status to ClusterResourceBinding(%s): %v", clusterResourceBinding.Name, err) + if err == nil { + // schedule succeed but update status failed, return err in order to retry in next loop. + err = updateErr + } + } + }() + scheduleResult, err := s.Algorithm.Schedule(context.TODO(), &clusterResourceBinding.Spec, &clusterResourceBinding.Status, &core.ScheduleAlgorithmOption{EnableEmptyWorkloadPropagation: s.enableEmptyWorkloadPropagation}) var noClusterFit *framework.FitError diff --git a/pkg/util/helper/workstatus.go b/pkg/util/helper/workstatus.go index 74059a81e..b0d7fa83e 100644 --- a/pkg/util/helper/workstatus.go +++ b/pkg/util/helper/workstatus.go @@ -50,17 +50,20 @@ func AggregateResourceBindingWorkStatus(c client.Client, binding *workv1alpha2.R return err } + fullyAppliedCondition := generateFullyAppliedCondition(binding.Spec, aggregatedStatuses) + currentBindingStatus := binding.Status.DeepCopy() currentBindingStatus.AggregatedStatus = aggregatedStatuses - meta.SetStatusCondition(¤tBindingStatus.Conditions, generateFullyAppliedCondition(binding.Spec, aggregatedStatuses)) - - if reflect.DeepEqual(binding.Status, currentBindingStatus) { - klog.V(4).Infof("New aggregatedStatuses are equal with old resourceBinding(%s/%s) AggregatedStatus, no update required.", - binding.Namespace, binding.Name) - return nil - } - err = retry.RetryOnConflict(retry.DefaultRetry, func() (err error) { + // set binding status with the newest condition + currentBindingStatus.Conditions = binding.Status.Conditions + meta.SetStatusCondition(¤tBindingStatus.Conditions, fullyAppliedCondition) + if reflect.DeepEqual(binding.Status, currentBindingStatus) { + klog.V(4).Infof("New aggregatedStatuses are equal with old resourceBinding(%s/%s) AggregatedStatus, no update required.", + binding.Namespace, binding.Name) + return nil + } + binding.Status = *currentBindingStatus updateErr := c.Status().Update(context.TODO(), binding) if updateErr == nil { @@ -102,16 +105,19 @@ func AggregateClusterResourceBindingWorkStatus(c client.Client, binding *workv1a return err } + fullyAppliedCondition := generateFullyAppliedCondition(binding.Spec, aggregatedStatuses) + currentBindingStatus := binding.Status.DeepCopy() currentBindingStatus.AggregatedStatus = aggregatedStatuses - meta.SetStatusCondition(¤tBindingStatus.Conditions, generateFullyAppliedCondition(binding.Spec, aggregatedStatuses)) - - if reflect.DeepEqual(binding.Status, currentBindingStatus) { - klog.Infof("New aggregatedStatuses are equal with old clusterResourceBinding(%s) AggregatedStatus, no update required.", binding.Name) - return nil - } - err = retry.RetryOnConflict(retry.DefaultRetry, func() (err error) { + // set binding status with the newest condition + currentBindingStatus.Conditions = binding.Status.Conditions + meta.SetStatusCondition(¤tBindingStatus.Conditions, fullyAppliedCondition) + if reflect.DeepEqual(binding.Status, currentBindingStatus) { + klog.Infof("New aggregatedStatuses are equal with old clusterResourceBinding(%s) AggregatedStatus, no update required.", binding.Name) + return nil + } + binding.Status = *currentBindingStatus updateErr := c.Status().Update(context.TODO(), binding) if updateErr == nil {