From 0af84956b1bc83fee12360bbe355579cd6318d8d Mon Sep 17 00:00:00 2001 From: Pulkit Jain Date: Wed, 21 Feb 2024 11:19:33 +0530 Subject: [PATCH] Record event when EgressIP remains unassigned Modified the code to record an event for case, when an egressIP is unassigned from a node and is not assigned to any other node. Signed-off-by: Pulkit Jain --- .../controller/egress/egress_controller.go | 12 +- .../egress/egress_controller_test.go | 244 +++++++++++++++--- test/e2e/egress_test.go | 13 + 3 files changed, 223 insertions(+), 46 deletions(-) diff --git a/pkg/agent/controller/egress/egress_controller.go b/pkg/agent/controller/egress/egress_controller.go index 382cf28fc4b..a6213897d40 100644 --- a/pkg/agent/controller/egress/egress_controller.go +++ b/pkg/agent/controller/egress/egress_controller.go @@ -1002,7 +1002,7 @@ func (c *EgressController) syncEgress(egressName string) error { if !exist { return nil } - if err := c.uninstallEgress(egressName, eState); err != nil { + if err := c.uninstallEgress(egressName, eState, egress); err != nil { return err } return nil @@ -1030,7 +1030,7 @@ func (c *EgressController) syncEgress(egressName string) error { eState, exist := c.getEgressState(egressName) // If the EgressIP changes, uninstalls this Egress first. if exist && eState.egressIP != desiredEgressIP { - if err := c.uninstallEgress(egressName, eState); err != nil { + if err := c.uninstallEgress(egressName, eState, egress); err != nil { return err } exist = false @@ -1153,7 +1153,7 @@ func (c *EgressController) syncEgress(egressName string) error { return nil } -func (c *EgressController) uninstallEgress(egressName string, eState *egressState) error { +func (c *EgressController) uninstallEgress(egressName string, eState *egressState, egress *crdv1b1.Egress) error { // Uninstall all of its Pod flows. if err := c.uninstallPodFlows(egressName, eState, eState.ofPorts, eState.pods); err != nil { return err @@ -1169,9 +1169,13 @@ func (c *EgressController) uninstallEgress(egressName string, eState *egressStat } } // Unassign the Egress IP from the local Node if it was assigned by the agent. - if _, err := c.ipAssigner.UnassignIP(eState.egressIP); err != nil { + unassigned, err := c.ipAssigner.UnassignIP(eState.egressIP) + if err != nil { return err } + if unassigned && egress != nil { + c.record.Eventf(egress, corev1.EventTypeNormal, "IPUnassigned", "Unassigned Egress %s with IP %s from Node %s", egressName, eState.egressIP, c.nodeName) + } // Remove the Egress's state. c.deleteEgressState(egressName) return nil diff --git a/pkg/agent/controller/egress/egress_controller_test.go b/pkg/agent/controller/egress/egress_controller_test.go index 7e91d2e53ca..f35d8e738d6 100644 --- a/pkg/agent/controller/egress/egress_controller_test.go +++ b/pkg/agent/controller/egress/egress_controller_test.go @@ -32,6 +32,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" + k8sv1 "k8s.io/client-go/kubernetes/typed/core/v1" k8stesting "k8s.io/client-go/testing" "k8s.io/client-go/util/workqueue" @@ -48,6 +49,7 @@ import ( crdv1b1 "antrea.io/antrea/pkg/apis/crd/v1beta1" "antrea.io/antrea/pkg/client/clientset/versioned" fakeversioned "antrea.io/antrea/pkg/client/clientset/versioned/fake" + "antrea.io/antrea/pkg/client/clientset/versioned/scheme" crdinformers "antrea.io/antrea/pkg/client/informers/externalversions" "antrea.io/antrea/pkg/util/channel" "antrea.io/antrea/pkg/util/ip" @@ -76,6 +78,11 @@ var ( } ) +type expectedEvents struct { + expectedMessage string + messageCount int32 +} + type fakeLocalIPDetector struct { localIPs sets.Set[string] } @@ -231,6 +238,7 @@ func TestSyncEgress(t *testing.T) { newLocalIPs sets.Set[string] expectedEgresses []*crdv1b1.Egress expectedCalls func(mockOFClient *openflowtest.MockClient, mockRouteClient *routetest.MockInterface, mockIPAssigner *ipassignertest.MockIPAssigner) + egressEvents []expectedEvents }{ { name: "Local IP becomes non local", @@ -269,18 +277,24 @@ func TestSyncEgress(t *testing.T) { mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(2), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP1), uint32(1)) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Return(true, nil) mockOFClient.EXPECT().UninstallSNATMarkFlows(uint32(1)) mockRouteClient.EXPECT().DeleteSNATRule(uint32(1)) mockOFClient.EXPECT().UninstallPodSNATFlows(uint32(1)) mockOFClient.EXPECT().UninstallPodSNATFlows(uint32(2)) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Return(true, nil) mockOFClient.EXPECT().UninstallEgressQoS(uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP1), uint32(0)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(3), net.ParseIP(fakeLocalEgressIP1), uint32(0)) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Return(true, nil) + }, + egressEvents: []expectedEvents{ + { + expectedMessage: "Unassigned Egress egressA with IP 1.1.1.1 from Node node1", + messageCount: 3, + }, }, }, { @@ -318,18 +332,24 @@ func TestSyncEgress(t *testing.T) { expectedCalls: func(mockOFClient *openflowtest.MockClient, mockRouteClient *routetest.MockInterface, mockIPAssigner *ipassignertest.MockIPAssigner) { mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeRemoteEgressIP1), uint32(0)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(2), net.ParseIP(fakeRemoteEgressIP1), uint32(0)) - mockIPAssigner.EXPECT().UnassignIP(fakeRemoteEgressIP1) + mockIPAssigner.EXPECT().UnassignIP(fakeRemoteEgressIP1).Return(true, nil) mockOFClient.EXPECT().UninstallPodSNATFlows(uint32(1)) mockOFClient.EXPECT().UninstallPodSNATFlows(uint32(2)) - mockIPAssigner.EXPECT().UnassignIP(fakeRemoteEgressIP1) + mockIPAssigner.EXPECT().UnassignIP(fakeRemoteEgressIP1).Return(true, nil) mockOFClient.EXPECT().InstallEgressQoS(uint32(1), uint32(500), uint32(500)) mockOFClient.EXPECT().InstallSNATMarkFlows(net.ParseIP(fakeRemoteEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeRemoteEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(3), net.ParseIP(fakeRemoteEgressIP1), uint32(1)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeRemoteEgressIP1), uint32(1)) - mockIPAssigner.EXPECT().UnassignIP(fakeRemoteEgressIP1) + mockIPAssigner.EXPECT().UnassignIP(fakeRemoteEgressIP1).Return(true, nil) + }, + egressEvents: []expectedEvents{ + { + expectedMessage: "Unassigned Egress egressA with IP 1.1.1.3 from Node node1", + messageCount: 3, + }, }, }, { @@ -369,22 +389,32 @@ func TestSyncEgress(t *testing.T) { mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(2), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP1), uint32(1)) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Return(true, nil) mockOFClient.EXPECT().UninstallEgressQoS(uint32(1)) mockOFClient.EXPECT().UninstallSNATMarkFlows(uint32(1)) mockRouteClient.EXPECT().DeleteSNATRule(uint32(1)) mockOFClient.EXPECT().UninstallPodSNATFlows(uint32(1)) mockOFClient.EXPECT().UninstallPodSNATFlows(uint32(2)) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP2) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Return(true, nil) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP2).Return(true, nil) mockOFClient.EXPECT().InstallEgressQoS(uint32(1), uint32(500), uint32(500)) mockOFClient.EXPECT().InstallSNATMarkFlows(net.ParseIP(fakeLocalEgressIP2), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP2), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(3), net.ParseIP(fakeLocalEgressIP2), uint32(1)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP2), uint32(1)) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP2) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP2).Return(true, nil) + }, + egressEvents: []expectedEvents{ + { + expectedMessage: "Unassigned Egress egressA with IP 1.1.1.1 from Node node1", + messageCount: 2, + }, + { + expectedMessage: "Unassigned Egress egressA with IP 1.1.1.2 from Node node1", + messageCount: 2, + }, }, }, { @@ -423,19 +453,29 @@ func TestSyncEgress(t *testing.T) { mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(2), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP1), uint32(1)) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Return(true, nil) mockOFClient.EXPECT().UninstallSNATMarkFlows(uint32(1)) mockRouteClient.EXPECT().DeleteSNATRule(uint32(1)) mockOFClient.EXPECT().UninstallPodSNATFlows(uint32(1)) mockOFClient.EXPECT().UninstallPodSNATFlows(uint32(2)) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1) - mockIPAssigner.EXPECT().UnassignIP(fakeRemoteEgressIP1) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Return(true, nil) + mockIPAssigner.EXPECT().UnassignIP(fakeRemoteEgressIP1).Return(true, nil) mockOFClient.EXPECT().UninstallEgressQoS(uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeRemoteEgressIP1), uint32(0)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(3), net.ParseIP(fakeRemoteEgressIP1), uint32(0)) - mockIPAssigner.EXPECT().UnassignIP(fakeRemoteEgressIP1) + mockIPAssigner.EXPECT().UnassignIP(fakeRemoteEgressIP1).Return(true, nil) + }, + egressEvents: []expectedEvents{ + { + expectedMessage: "Unassigned Egress egressA with IP 1.1.1.1 from Node node1", + messageCount: 2, + }, + { + expectedMessage: "Unassigned Egress egressA with IP 1.1.1.3 from Node node1", + messageCount: 2, + }, }, }, { @@ -472,19 +512,29 @@ func TestSyncEgress(t *testing.T) { expectedCalls: func(mockOFClient *openflowtest.MockClient, mockRouteClient *routetest.MockInterface, mockIPAssigner *ipassignertest.MockIPAssigner) { mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeRemoteEgressIP1), uint32(0)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(2), net.ParseIP(fakeRemoteEgressIP1), uint32(0)) - mockIPAssigner.EXPECT().UnassignIP(fakeRemoteEgressIP1) + mockIPAssigner.EXPECT().UnassignIP(fakeRemoteEgressIP1).Return(true, nil) mockOFClient.EXPECT().UninstallPodSNATFlows(uint32(1)) mockOFClient.EXPECT().UninstallPodSNATFlows(uint32(2)) - mockIPAssigner.EXPECT().UnassignIP(fakeRemoteEgressIP1) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1) + mockIPAssigner.EXPECT().UnassignIP(fakeRemoteEgressIP1).Return(true, nil) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Return(true, nil) mockOFClient.EXPECT().InstallEgressQoS(uint32(1), uint32(500), uint32(500)) mockOFClient.EXPECT().InstallSNATMarkFlows(net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(3), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP1), uint32(1)) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Return(true, nil) + }, + egressEvents: []expectedEvents{ + { + expectedMessage: "Unassigned Egress egressA with IP 1.1.1.3 from Node node1", + messageCount: 2, + }, + { + expectedMessage: "Unassigned Egress egressA with IP 1.1.1.1 from Node node1", + messageCount: 2, + }, }, }, { @@ -528,14 +578,24 @@ func TestSyncEgress(t *testing.T) { mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(2), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP1), uint32(1)) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Return(true, nil) mockOFClient.EXPECT().InstallSNATMarkFlows(net.ParseIP(fakeLocalEgressIP2), uint32(2)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(3), net.ParseIP(fakeLocalEgressIP2), uint32(2)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP2), uint32(2)) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP2) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP2).Return(true, nil) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP2) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP2).Return(true, nil) + }, + egressEvents: []expectedEvents{ + { + expectedMessage: "Unassigned Egress egressA with IP 1.1.1.1 from Node node1", + messageCount: 1, + }, + { + expectedMessage: "Unassigned Egress egressB with IP 1.1.1.2 from Node node1", + messageCount: 2, + }, }, }, { @@ -580,7 +640,17 @@ func TestSyncEgress(t *testing.T) { mockOFClient.EXPECT().InstallPodSNATFlows(uint32(2), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(3), net.ParseIP(fakeLocalEgressIP1), uint32(1)) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Times(3) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Return(true, nil).Times(3) + }, + egressEvents: []expectedEvents{ + { + expectedMessage: "Unassigned Egress egressA with IP 1.1.1.1 from Node node1", + messageCount: 1, + }, + { + expectedMessage: "Unassigned Egress egressB with IP 1.1.1.1 from Node node1", + messageCount: 2, + }, }, }, { @@ -623,18 +693,28 @@ func TestSyncEgress(t *testing.T) { }, }, expectedCalls: func(mockOFClient *openflowtest.MockClient, mockRouteClient *routetest.MockInterface, mockIPAssigner *ipassignertest.MockIPAssigner) { - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Return(true, nil) mockOFClient.EXPECT().InstallSNATMarkFlows(net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(2), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP1), uint32(1)) - mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP2, nil, true) + mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP2, nil, true).Return(true, nil) // forceAdvertise depends on how fast the Egress status update is reflected in the informer cache, which doesn't really matter. - mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP2, nil, gomock.Any()) + mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP2, nil, gomock.Any()).Return(true, nil) mockOFClient.EXPECT().InstallSNATMarkFlows(net.ParseIP(fakeLocalEgressIP2), uint32(2)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(3), net.ParseIP(fakeLocalEgressIP2), uint32(2)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP2), uint32(2)) }, + egressEvents: []expectedEvents{ + { + expectedMessage: "Unassigned Egress egressA with IP 1.1.1.1 from Node node1", + messageCount: 1, + }, + { + expectedMessage: "Assigned Egress egressB with IP 1.1.1.2 on Node node1", + messageCount: 2, + }, + }, }, { name: "Exceed maxEgressIPsPerNode", @@ -677,12 +757,18 @@ func TestSyncEgress(t *testing.T) { }, }, expectedCalls: func(mockOFClient *openflowtest.MockClient, mockRouteClient *routetest.MockInterface, mockIPAssigner *ipassignertest.MockIPAssigner) { - mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, nil, true) + mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, nil, true).Return(true, nil) mockOFClient.EXPECT().InstallSNATMarkFlows(net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(2), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP1), uint32(1)) }, + egressEvents: []expectedEvents{ + { + expectedMessage: "Assigned Egress egressA with IP 1.1.1.1 on Node node1", + messageCount: 1, + }, + }, }, { name: "Remove Egress IP", @@ -717,18 +803,28 @@ func TestSyncEgress(t *testing.T) { }, }, expectedCalls: func(mockOFClient *openflowtest.MockClient, mockRouteClient *routetest.MockInterface, mockIPAssigner *ipassignertest.MockIPAssigner) { - mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, nil, true) + mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, nil, true).Return(true, nil) mockOFClient.EXPECT().InstallSNATMarkFlows(net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(2), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP1), uint32(1)) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Return(true, nil) mockOFClient.EXPECT().UninstallSNATMarkFlows(uint32(1)) mockOFClient.EXPECT().UninstallPodSNATFlows(uint32(1)) mockOFClient.EXPECT().UninstallPodSNATFlows(uint32(2)) mockRouteClient.EXPECT().DeleteSNATRule(uint32(1)) }, + egressEvents: []expectedEvents{ + { + expectedMessage: "Assigned Egress egressA with IP 1.1.1.1 on Node node1", + messageCount: 1, + }, + { + expectedMessage: "Unassigned Egress egressA with IP 1.1.1.1 from Node node1", + messageCount: 1, + }, + }, }, { name: "Update Egress from non-rate-limited to rate-limited", @@ -763,10 +859,16 @@ func TestSyncEgress(t *testing.T) { mockOFClient.EXPECT().InstallSNATMarkFlows(net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP1), uint32(1)) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Times(3) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Return(true, nil).Times(3) mockOFClient.EXPECT().InstallEgressQoS(uint32(1), uint32(500), uint32(500)) }, + egressEvents: []expectedEvents{ + { + expectedMessage: "Unassigned Egress egressA with IP 1.1.1.1 from Node node1", + messageCount: 3, + }, + }, }, { name: "Update Egress from rate-limited to non-rate-limited", @@ -802,10 +904,16 @@ func TestSyncEgress(t *testing.T) { mockOFClient.EXPECT().InstallSNATMarkFlows(net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP1), uint32(1)) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Times(3) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Return(true, nil).Times(3) mockOFClient.EXPECT().UninstallEgressQoS(uint32(1)) }, + egressEvents: []expectedEvents{ + { + expectedMessage: "Unassigned Egress egressA with IP 1.1.1.1 from Node node1", + messageCount: 3, + }, + }, }, { name: "Update Egress rate-limited config", @@ -841,9 +949,15 @@ func TestSyncEgress(t *testing.T) { mockOFClient.EXPECT().InstallSNATMarkFlows(net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP1), uint32(1)) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Times(3) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Return(true, nil).Times(3) mockOFClient.EXPECT().InstallEgressQoS(uint32(1), uint32(10000), uint32(20000)) }, + egressEvents: []expectedEvents{ + { + expectedMessage: "Unassigned Egress egressA with IP 1.1.1.1 from Node node1", + messageCount: 3, + }, + }, }, { name: "Add SubnetInfo to ExternalIPPool", @@ -885,18 +999,24 @@ func TestSyncEgress(t *testing.T) { }, }, expectedCalls: func(mockOFClient *openflowtest.MockClient, mockRouteClient *routetest.MockInterface, mockIPAssigner *ipassignertest.MockIPAssigner) { - mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, nil, true) + mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, nil, true).Return(true, nil) mockOFClient.EXPECT().InstallSNATMarkFlows(net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP1), uint32(1)) - mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, &crdv1b1.SubnetInfo{Gateway: fakeGatewayIP, PrefixLength: 16, VLAN: 10}, true) + mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, &crdv1b1.SubnetInfo{Gateway: fakeGatewayIP, PrefixLength: 16, VLAN: 10}, true).Return(true, nil) mockIPAssigner.EXPECT().GetInterfaceID(&crdv1b1.SubnetInfo{Gateway: fakeGatewayIP, PrefixLength: 16, VLAN: 10}).Return(20, true) mockRouteClient.EXPECT().AddEgressRoutes(uint32(101), 20, net.ParseIP(fakeGatewayIP), 16) mockRouteClient.EXPECT().AddEgressRule(uint32(101), uint32(1)) // forceAdvertise depends on how fast the Egress status update is reflected in the informer cache, which doesn't really matter. - mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, &crdv1b1.SubnetInfo{Gateway: fakeGatewayIP, PrefixLength: 16, VLAN: 10}, gomock.Any()) + mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, &crdv1b1.SubnetInfo{Gateway: fakeGatewayIP, PrefixLength: 16, VLAN: 10}, gomock.Any()).Return(true, nil) + }, + egressEvents: []expectedEvents{ + { + expectedMessage: "Assigned Egress egressA with IP 1.1.1.1 on Node node1", + messageCount: 3, + }, }, }, { @@ -940,7 +1060,7 @@ func TestSyncEgress(t *testing.T) { }, }, expectedCalls: func(mockOFClient *openflowtest.MockClient, mockRouteClient *routetest.MockInterface, mockIPAssigner *ipassignertest.MockIPAssigner) { - mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, &crdv1b1.SubnetInfo{Gateway: fakeGatewayIP, PrefixLength: 16, VLAN: 10}, true) + mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, &crdv1b1.SubnetInfo{Gateway: fakeGatewayIP, PrefixLength: 16, VLAN: 10}, true).Return(true, nil) mockOFClient.EXPECT().InstallSNATMarkFlows(net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP1), uint32(1)) @@ -948,7 +1068,7 @@ func TestSyncEgress(t *testing.T) { mockRouteClient.EXPECT().AddEgressRoutes(uint32(101), 20, net.ParseIP(fakeGatewayIP), 16) mockRouteClient.EXPECT().AddEgressRule(uint32(101), uint32(1)) - mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, &crdv1b1.SubnetInfo{Gateway: fakeGatewayIP2, PrefixLength: 16}, true) + mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, &crdv1b1.SubnetInfo{Gateway: fakeGatewayIP2, PrefixLength: 16}, true).Return(true, nil) mockRouteClient.EXPECT().DeleteEgressRule(uint32(101), uint32(1)) mockRouteClient.EXPECT().DeleteEgressRoutes(uint32(101)) mockIPAssigner.EXPECT().GetInterfaceID(&crdv1b1.SubnetInfo{Gateway: fakeGatewayIP2, PrefixLength: 16}).Return(30, true) @@ -956,7 +1076,13 @@ func TestSyncEgress(t *testing.T) { mockRouteClient.EXPECT().AddEgressRule(uint32(101), uint32(1)) // forceAdvertise depends on how fast the Egress status update is reflected in the informer cache, which doesn't really matter. - mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, &crdv1b1.SubnetInfo{Gateway: fakeGatewayIP2, PrefixLength: 16}, gomock.Any()) + mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, &crdv1b1.SubnetInfo{Gateway: fakeGatewayIP2, PrefixLength: 16}, gomock.Any()).Return(true, nil) + }, + egressEvents: []expectedEvents{ + { + expectedMessage: "Assigned Egress egressA with IP 1.1.1.1 on Node node1", + messageCount: 3, + }, }, }, { @@ -1006,7 +1132,7 @@ func TestSyncEgress(t *testing.T) { }, }, expectedCalls: func(mockOFClient *openflowtest.MockClient, mockRouteClient *routetest.MockInterface, mockIPAssigner *ipassignertest.MockIPAssigner) { - mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, &crdv1b1.SubnetInfo{Gateway: fakeGatewayIP, PrefixLength: 16, VLAN: 10}, true) + mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, &crdv1b1.SubnetInfo{Gateway: fakeGatewayIP, PrefixLength: 16, VLAN: 10}, true).Return(true, nil) mockOFClient.EXPECT().InstallSNATMarkFlows(net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP1), uint32(1)) @@ -1014,14 +1140,24 @@ func TestSyncEgress(t *testing.T) { mockRouteClient.EXPECT().AddEgressRoutes(uint32(101), 20, net.ParseIP(fakeGatewayIP), 16) mockRouteClient.EXPECT().AddEgressRule(uint32(101), uint32(1)) - mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP2, &crdv1b1.SubnetInfo{Gateway: fakeGatewayIP, PrefixLength: 16, VLAN: 10}, true) + mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP2, &crdv1b1.SubnetInfo{Gateway: fakeGatewayIP, PrefixLength: 16, VLAN: 10}, true).Return(true, nil) mockOFClient.EXPECT().InstallSNATMarkFlows(net.ParseIP(fakeLocalEgressIP2), uint32(2)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(2), net.ParseIP(fakeLocalEgressIP2), uint32(2)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP2), uint32(2)) mockRouteClient.EXPECT().AddEgressRule(uint32(101), uint32(2)) // forceAdvertise depends on how fast the Egress status update is reflected in the informer cache, which doesn't really matter. - mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP2, &crdv1b1.SubnetInfo{Gateway: fakeGatewayIP, PrefixLength: 16, VLAN: 10}, gomock.Any()) + mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP2, &crdv1b1.SubnetInfo{Gateway: fakeGatewayIP, PrefixLength: 16, VLAN: 10}, gomock.Any()).Return(true, nil) + }, + egressEvents: []expectedEvents{ + { + expectedMessage: "Assigned Egress egressA with IP 1.1.1.1 on Node node1", + messageCount: 1, + }, + { + expectedMessage: "Assigned Egress egressB with IP 1.1.1.2 on Node node1", + messageCount: 2, + }, }, }, { @@ -1055,7 +1191,7 @@ func TestSyncEgress(t *testing.T) { }, }, expectedCalls: func(mockOFClient *openflowtest.MockClient, mockRouteClient *routetest.MockInterface, mockIPAssigner *ipassignertest.MockIPAssigner) { - mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, &crdv1b1.SubnetInfo{Gateway: fakeGatewayIP, PrefixLength: 16, VLAN: 10}, true) + mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, &crdv1b1.SubnetInfo{Gateway: fakeGatewayIP, PrefixLength: 16, VLAN: 10}, true).Return(true, nil) mockOFClient.EXPECT().InstallSNATMarkFlows(net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP1), uint32(1)) @@ -1063,13 +1199,23 @@ func TestSyncEgress(t *testing.T) { mockRouteClient.EXPECT().AddEgressRoutes(uint32(101), 20, net.ParseIP(fakeGatewayIP), 16) mockRouteClient.EXPECT().AddEgressRule(uint32(101), uint32(1)) - mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1) + mockIPAssigner.EXPECT().UnassignIP(fakeLocalEgressIP1).Return(true, nil) mockRouteClient.EXPECT().DeleteEgressRule(uint32(101), uint32(1)) mockRouteClient.EXPECT().DeleteEgressRoutes(uint32(101)) mockOFClient.EXPECT().UninstallSNATMarkFlows(uint32(1)) mockOFClient.EXPECT().UninstallPodSNATFlows(uint32(1)) mockRouteClient.EXPECT().DeleteSNATRule(uint32(1)) }, + egressEvents: []expectedEvents{ + { + expectedMessage: "Assigned Egress egressA with IP 1.1.1.1 on Node node1", + messageCount: 1, + }, + { + expectedMessage: "Unassigned Egress egressA with IP 1.1.1.1 from Node node1", + messageCount: 1, + }, + }, }, } for _, tt := range tests { @@ -1084,6 +1230,10 @@ func TestSyncEgress(t *testing.T) { if tt.maxEgressIPsPerNode > 0 { c.egressIPScheduler.maxEgressIPsPerNode = tt.maxEgressIPsPerNode } + c.eventBroadcaster.StartStructuredLogging(0) + c.eventBroadcaster.StartRecordingToSink(&k8sv1.EventSinkImpl{ + Interface: c.k8sClient.CoreV1().Events(""), + }) stopCh := make(chan struct{}) defer close(stopCh) @@ -1139,6 +1289,16 @@ func TestSyncEgress(t *testing.T) { require.NoError(t, err) assert.True(t, k8s.SemanticIgnoringTime.DeepEqual(expectedEgress, gotEgress)) } + + assert.EventuallyWithT(t, func(a *assert.CollectT) { + events, err := c.k8sClient.CoreV1().Events("").Search(scheme.Scheme, tt.existingEgress) + if assert.NoError(a, err) && assert.Len(a, events.Items, len(tt.egressEvents)) { + for ind, items := range events.Items { + assert.Contains(a, items.Message, tt.egressEvents[ind].expectedMessage) + assert.Equal(a, items.Count, tt.egressEvents[ind].messageCount) + } + } + }, 2*time.Second, 200*time.Millisecond) }) } } diff --git a/test/e2e/egress_test.go b/test/e2e/egress_test.go index d128598f778..63c723b95ae 100644 --- a/test/e2e/egress_test.go +++ b/test/e2e/egress_test.go @@ -517,6 +517,7 @@ func testEgressCRUD(t *testing.T, data *TestData) { err = data.crdClient.CrdV1beta1().Egresses().Delete(context.TODO(), egress.Name, metav1.DeleteOptions{}) require.NoError(t, err, "Failed to delete Egress") + if egress.Status.EgressNode != "" { err := wait.PollUntilContextTimeout(context.Background(), 200*time.Millisecond, timeout, true, func(ctx context.Context) (done bool, err error) { @@ -600,6 +601,18 @@ func testEgressUpdateEgressIP(t *testing.T, data *TestData) { return err }) require.NoError(t, err, "Failed to update Egress") + // Testing the events recorded during deletion of an Egress resource. + expectedMessage := fmt.Sprintf("Unassigned Egress %s with IP %s from Node %v", egress.Name, tt.originalEgressIP, egress.Status.EgressNode) + assert.EventuallyWithT(t, func(c *assert.CollectT) { + events, err := data.clientset.CoreV1().Events("").Search(scheme.Scheme, egress) + assert.NoError(c, err) + recordedMessages := []string{} + for _, items := range events.Items { + recordedMessages = append(recordedMessages, items.Message) + } + + assert.Contains(c, recordedMessages, expectedMessage) + }, 2*time.Second, 200*time.Millisecond) _, err = data.checkEgressState(egress.Name, tt.newEgressIP, tt.newNode, "", time.Second) require.NoError(t, err)