the condition is updated only after scheduling

Signed-off-by: changzhen <changzhen5@huawei.com>

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

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(&currentBindingStatus.Conditions, generateFullyAppliedCondition(binding.Spec, aggregatedStatuses))
+	err = retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
-
+		// set binding status with the newest condition
+		currentBindingStatus.Conditions = binding.Status.Conditions
+		meta.SetStatusCondition(&currentBindingStatus.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
 		}
 
-	err = retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
 		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(&currentBindingStatus.Conditions, generateFullyAppliedCondition(binding.Spec, aggregatedStatuses))
+	err = retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
-
+		// set binding status with the newest condition
+		currentBindingStatus.Conditions = binding.Status.Conditions
+		meta.SetStatusCondition(&currentBindingStatus.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
 		}
 
-	err = retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
 		binding.Status = *currentBindingStatus
 		updateErr := c.Status().Update(context.TODO(), binding)
 		if updateErr == nil {