Fix a few corner cases when updating Egress conditions

1. Avoid generating a transient IPAssigned failure by differentiating
scheduling failure from unprocessed case.
2. Fix duplicate IPAllocated conditions.
3. Add/update unit tests and e2e tests.

Signed-off-by: Quan Tian <qtian@vmware.com>

pkg/agent/controller/egress/egress_controller.go

@@ -614,7 +614,7 @@ func (c *EgressController) unbindPodEgress(pod, egress string) (string, bool) {
 	return "", false
 }
 
-func (c *EgressController) updateEgressStatus(egress *crdv1b1.Egress, egressIP string) error {
+func (c *EgressController) updateEgressStatus(egress *crdv1b1.Egress, egressIP string, scheduleErr error) error {
 	isLocal := false
 	if egressIP != "" {
 		isLocal = c.localIPDetector.IsLocalIP(egressIP)
@@ -647,14 +647,16 @@ func (c *EgressController) updateEgressStatus(egress *crdv1b1.Egress, egressIP s
 		}
 		desiredStatus.EgressNode = ""
 		desiredStatus.EgressIP = ""
-		if isEgressSchedulable(egress) {
+		// If the error is nil, it means the Egress hasn't been processed yet. Therefore, we only set IPAssigned
+		// condition to false when there is an error.
+		if scheduleErr != nil {
 			desiredStatus.Conditions = []crdv1b1.EgressCondition{
 				{
 					Type:               crdv1b1.IPAssigned,
 					Status:             v1.ConditionFalse,
 					LastTransitionTime: metav1.Now(),
-					Reason:             "NoAvailableNode",
-					Message:            "No available Node can be elected as EgressNode",
+					Reason:             "AssignmentError",
+					Message:            fmt.Sprintf("Failed to assign the IP to EgressNode: %v", scheduleErr),
 				},
 			}
 		}
@@ -668,12 +670,14 @@ func (c *EgressController) updateEgressStatus(egress *crdv1b1.Egress, egressIP s
 		if compareEgressStatus(toUpdate.Status, *desiredStatus) {
 			return nil
 		}
+		statusToUpdate := desiredStatus.DeepCopy()
+		// Copy conditions other than crdv1b1.IPAssigned to statusToUpdate.
 		for _, c := range toUpdate.Status.Conditions {
 			if c.Type != crdv1b1.IPAssigned {
-				desiredStatus.Conditions = append(desiredStatus.Conditions, c)
+				statusToUpdate.Conditions = append(statusToUpdate.Conditions, c)
 			}
 		}
-		toUpdate.Status = *desiredStatus
+		toUpdate.Status = *statusToUpdate
 
 		klog.V(2).InfoS("Updating Egress status", "Egress", egress.Name, "oldNode", egress.Status.EgressNode, "newNode", toUpdate.Status.EgressNode)
 		_, updateErr = c.crdClient.CrdV1beta1().Egresses().UpdateStatus(context.TODO(), toUpdate, metav1.UpdateOptions{})
@@ -717,13 +721,16 @@ func (c *EgressController) syncEgress(egressName string) error {
 
 	var desiredEgressIP string
 	var desiredNode string
+	var scheduleErr error
 	// Only check whether the Egress IP should be assigned to this Node when the Egress is schedulable.
 	// Otherwise, users are responsible for assigning the Egress IP to Nodes.
 	if isEgressSchedulable(egress) {
-		egressIP, egressNode, scheduled := c.egressIPScheduler.GetEgressIPAndNode(egressName)
+		egressIP, egressNode, err, scheduled := c.egressIPScheduler.GetEgressIPAndNode(egressName)
 		if scheduled {
 			desiredEgressIP = egressIP
 			desiredNode = egressNode
+		} else {
+			scheduleErr = err
 		}
 	} else {
 		desiredEgressIP = egress.Spec.EgressIP
@@ -739,7 +746,7 @@ func (c *EgressController) syncEgress(egressName string) error {
 	}
 	// Do not proceed if EgressIP is empty.
 	if desiredEgressIP == "" {
-		if err := c.updateEgressStatus(egress, ""); err != nil {
+		if err := c.updateEgressStatus(egress, "", scheduleErr); err != nil {
 			return fmt.Errorf("update Egress %s status error: %v", egressName, err)
 		}
 		return nil
@@ -778,7 +785,7 @@ func (c *EgressController) syncEgress(egressName string) error {
 		eState.mark = mark
 	}
 
-	if err := c.updateEgressStatus(egress, desiredEgressIP); err != nil {
+	if err := c.updateEgressStatus(egress, desiredEgressIP, nil); err != nil {
 		return fmt.Errorf("update Egress %s status error: %v", egressName, err)
 	}
 
@@ -1071,16 +1078,8 @@ func compareEgressStatus(currentStatus, desiredStatus crdv1b1.EgressStatus) bool
 	if currentStatus.EgressIP != desiredStatus.EgressIP || currentStatus.EgressNode != desiredStatus.EgressNode {
 		return false
 	}
-	getIPAssignedCondition := func(conditions []crdv1b1.EgressCondition) *crdv1b1.EgressCondition {
-		for _, c := range conditions {
-			if c.Type == crdv1b1.IPAssigned {
-				return &c
-			}
-		}
-		return nil
-	}
-	currentIPAssignedCondition := getIPAssignedCondition(currentStatus.Conditions)
-	desiredIPAssignedCondition := getIPAssignedCondition(desiredStatus.Conditions)
+	currentIPAssignedCondition := crdv1b1.GetEgressCondition(currentStatus.Conditions, crdv1b1.IPAssigned)
+	desiredIPAssignedCondition := crdv1b1.GetEgressCondition(desiredStatus.Conditions, crdv1b1.IPAssigned)
 	if currentIPAssignedCondition == nil && desiredIPAssignedCondition == nil {
 		return true
 	}