Add unit test case for update Egress Status

pkg/agent/controller/egress/egress_controller.go

@@ -543,14 +543,13 @@ func (c *EgressController) updateEgressStatus(egress *crdv1a2.Egress, isLocal bo
 		_, updateErr = c.crdClient.CrdV1alpha2().Egresses().UpdateStatus(context.TODO(), toUpdate, metav1.UpdateOptions{})
 		if updateErr != nil && errors.IsConflict(updateErr) {
 			if toUpdate, getErr = c.crdClient.CrdV1alpha2().Egresses().Get(context.TODO(), egress.Name, metav1.GetOptions{}); getErr != nil {
-				// If the GET fails we can't trust status.Replicas anymore. This error is bound to be more interesting than the update failure.
-				return fmt.Errorf("get Egress %s error while retrying update: %v", egress.Name, getErr)
+				return getErr
 			}
 		}
 		// Return the error from UPDATE.
 		return updateErr
 	}); err != nil {
-		return fmt.Errorf("updating Egress %s status error: %v", egress.Name, err)
+		return err
 	}
 	klog.V(2).InfoS("Updated Egress status", "Egress", egress.Name)
 	metrics.AntreaEgressStatusUpdates.Inc()
@@ -629,7 +628,7 @@ func (c *EgressController) syncEgress(egressName string) error {
 	}
 
 	if err := c.updateEgressStatus(egress, c.localIPDetector.IsLocalIP(egress.Spec.EgressIP)); err != nil {
-		return err
+		return fmt.Errorf("update Egress %s status error: %v", egressName, err)
 	}
 
 	// Copy the previous ofPorts and Pods. They will be used to identify stale ofPorts and Pods.

pkg/agent/controller/egress/egress_controller_test.go

@@ -16,6 +16,7 @@ package egress
 
 import (
 	"context"
+	"fmt"
 	"net"
 	"reflect"
 	"testing"
@@ -24,10 +25,12 @@ import (
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
+	testing2 "k8s.io/client-go/testing"
 	"k8s.io/client-go/util/workqueue"
 
 	ipassignertest "antrea.io/antrea/pkg/agent/controller/egress/ipassigner/testing"
@@ -636,3 +639,90 @@ func addPodInterface(ifaceStore interfacestore.InterfaceStore, podNamespace, pod
 		OVSPortConfig:            &interfacestore.OVSPortConfig{OFPort: ofPort},
 	})
 }
+
+func TestUpdateEgressStatus(t *testing.T) {
+	getError := fmt.Errorf("fake get Egress error")
+	updateConflictError := &errors.StatusError{ErrStatus: metav1.Status{Reason: metav1.StatusReasonConflict, Message: "update Egress conflict"}}
+	updateError := fmt.Errorf("update Egress error")
+	egress := crdv1a2.Egress{
+		ObjectMeta: metav1.ObjectMeta{Name: "egressA", UID: "uidA", ResourceVersion: "fake-ResourceVersion"},
+		Spec:       crdv1a2.EgressSpec{EgressIP: fakeLocalEgressIP1},
+	}
+	tests := []struct {
+		name                 string
+		updateErrorNum       int
+		getErrorNum          int
+		expectedUpdateCalled int
+		expectedGetCalled    int
+		expectedError        error
+		updateFailure        error
+	}{
+		{
+			name:                 "succeed immediately",
+			updateErrorNum:       0,
+			getErrorNum:          0,
+			expectedUpdateCalled: 1,
+			expectedGetCalled:    0,
+			expectedError:        nil,
+			updateFailure:        updateConflictError,
+		},
+		{
+			name:                 "succeed after one update conflict failure",
+			updateErrorNum:       1,
+			getErrorNum:          0,
+			expectedUpdateCalled: 2,
+			expectedGetCalled:    1,
+			expectedError:        nil,
+			updateFailure:        updateConflictError,
+		},
+		{
+			name:                 "fail after one update failures",
+			updateErrorNum:       1,
+			getErrorNum:          0,
+			expectedUpdateCalled: 1,
+			expectedGetCalled:    0,
+			expectedError:        updateError,
+			updateFailure:        updateError,
+		},
+		{
+			name:                 "fail after one update conflict failure and one get failure",
+			updateErrorNum:       1,
+			getErrorNum:          1,
+			expectedUpdateCalled: 1,
+			expectedGetCalled:    1,
+			expectedError:        getError,
+			updateFailure:        updateConflictError,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			fakeClient := &fakeversioned.Clientset{}
+			getCalled := 0
+			fakeClient.AddReactor("get", "egresses", func(action testing2.Action) (bool, runtime.Object, error) {
+				getCalled += 1
+				if getCalled <= tt.getErrorNum {
+					return true, nil, getError
+				}
+				return true, &egress, nil
+			})
+			updateCalled := 0
+			fakeClient.AddReactor("update", "*", func(action testing2.Action) (bool, runtime.Object, error) {
+				updateCalled += 1
+				if updateCalled <= tt.updateErrorNum {
+					return true, nil, tt.updateFailure
+				}
+				return true, &egress, nil
+			})
+
+			c := &EgressController{crdClient: fakeClient}
+			_, err := c.crdClient.CrdV1alpha2().Egresses().Create(context.TODO(), &egress, metav1.CreateOptions{})
+			assert.NoError(t, err)
+			err = c.updateEgressStatus(&egress, true)
+			if err != tt.expectedError {
+				t.Errorf("Update Egress error not match, got: %v, expected: %v", err, tt.expectedError)
+			}
+			assert.Equal(t, tt.expectedGetCalled, getCalled, "Get called num not match")
+			assert.Equal(t, tt.expectedUpdateCalled, updateCalled, "Update called num not match")
+		})
+	}
+}