Repository: antrea-io/antrea
Pull request #4202 (2022-09-06 02:24:26)

Automated cherry pick of #4176: Bugfix: Remove multicast group from cache when group is
Cherry pick of #4176 on release-1.8.

#4176: Bugfix: Remove multicast group from cache when group is

For details on the cherry pick process, see the [cherry pick requests](https://github.com/antrea-io/antrea/blob/main/docs/contributors/cherry-picks.md) page.

codecov[bot]: # [Codecov](https://codecov.io/gh/antrea-io/antrea/pull/4202?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io) Report
> Merging [#4202](https://codecov.io/gh/antrea-io/antrea/pull/4202?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io) (07b8728) into [release-1.8](https://codecov.io/gh/antrea-io/antrea/commit/33e1b53eb6399437f858d2c0f62f2c06e965eaa3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io) (33e1b53) will **decrease** coverage by `0.00%`.
> The diff coverage is `60.00%`.

[![Impacted file tree graph](https://codecov.io/gh/antrea-io/antrea/pull/4202/graphs/tree.svg?width=650&height=150&src=pr&token=TjWaLL2GoC&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io)](https://codecov.io/gh/antrea-io/antrea/pull/4202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io)

```diff
@@               Coverage Diff               @@
##           release-1.8    #4202      +/-   ##
===============================================
- Coverage        65.91%   65.90%   -0.01%     
===============================================
  Files              304      304              
  Lines            46593    46785     +192     
===============================================
+ Hits             30712    30835     +123     
- Misses           13465    13532      +67     
- Partials          2416     2418       +2     
```

| Flag | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| integration-tests | `34.95% <0.00%> (ø)` | | Carriedforward from [33e1b53](https://codecov.io/gh/antrea-io/antrea/commit/33e1b53eb6399437f858d2c0f62f2c06e965eaa3?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io) |
| kind-e2e-tests | `48.85% <0.00%> (ø)` | | Carriedforward from [33e1b53](https://codecov.io/gh/antrea-io/antrea/commit/33e1b53eb6399437f858d2c0f62f2c06e965eaa3?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io) |
| unit-tests | `44.99% <69.23%> (+0.28%)` | :arrow_up: | |

*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io) to find out more.

| [Impacted Files](https://codecov.io/gh/antrea-io/antrea/pull/4202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io) | Coverage Δ | |
|---|---|---|
| [pkg/agent/multicast/mcast\_controller.go](https://codecov.io/gh/antrea-io/antrea/pull/4202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io#diff-cGtnL2FnZW50L211bHRpY2FzdC9tY2FzdF9jb250cm9sbGVyLmdv) | `53.47% <0.00%> (ø)` | |
| [pkg/agent/proxy/proxier.go](https://codecov.io/gh/antrea-io/antrea/pull/4202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io#diff-cGtnL2FnZW50L3Byb3h5L3Byb3hpZXIuZ28=) | `65.51% <0.00%> (ø)` | |
| [pkg/agent/openflow/client.go](https://codecov.io/gh/antrea-io/antrea/pull/4202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io#diff-cGtnL2FnZW50L29wZW5mbG93L2NsaWVudC5nbw==) | `63.66% <81.81%> (-6.06%)` | :arrow_down: |
| [pkg/controller/ipam/antrea\_ipam\_controller.go](https://codecov.io/gh/antrea-io/antrea/pull/4202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io#diff-cGtnL2NvbnRyb2xsZXIvaXBhbS9hbnRyZWFfaXBhbV9jb250cm9sbGVyLmdv) | `76.25% <0.00%> (-2.01%)` | :arrow_down: |
| [...r/ipseccertificate/ipsec\_certificate\_controller.go](https://codecov.io/gh/antrea-io/antrea/pull/4202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io#diff-cGtnL2FnZW50L2NvbnRyb2xsZXIvaXBzZWNjZXJ0aWZpY2F0ZS9pcHNlY19jZXJ0aWZpY2F0ZV9jb250cm9sbGVyLmdv) | `62.54% <0.00%> (-0.98%)` | :arrow_down: |
| [pkg/agent/controller/trafficcontrol/controller.go](https://codecov.io/gh/antrea-io/antrea/pull/4202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io#diff-cGtnL2FnZW50L2NvbnRyb2xsZXIvdHJhZmZpY2NvbnRyb2wvY29udHJvbGxlci5nbw==) | `82.78% <0.00%> (ø)` | |
| [pkg/agent/cniserver/ipam/antrea\_ipam.go](https://codecov.io/gh/antrea-io/antrea/pull/4202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io#diff-cGtnL2FnZW50L2NuaXNlcnZlci9pcGFtL2FudHJlYV9pcGFtLmdv) | `75.75% <0.00%> (+1.73%)` | :arrow_up: |
| [...gent/controller/noderoute/node\_route\_controller.go](https://codecov.io/gh/antrea-io/antrea/pull/4202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io#diff-cGtnL2FnZW50L2NvbnRyb2xsZXIvbm9kZXJvdXRlL25vZGVfcm91dGVfY29udHJvbGxlci5nbw==) | `59.40% <0.00%> (+1.86%)` | :arrow_up: |
| [pkg/apiserver/certificate/certificate.go](https://codecov.io/gh/antrea-io/antrea/pull/4202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io#diff-cGtnL2FwaXNlcnZlci9jZXJ0aWZpY2F0ZS9jZXJ0aWZpY2F0ZS5nbw==) | `76.85% <0.00%> (+6.48%)` | :arrow_up: |
| ... and [2 more](https://codecov.io/gh/antrea-io/antrea/pull/4202/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io) | |

wenyingd: /test-all

Patch:
diff --git a/hack/update-codegen-dockerized.sh b/hack/update-codegen-dockerized.sh
index 59187fb43..8b21d5d28 100755
--- a/hack/update-codegen-dockerized.sh
+++ b/hack/update-codegen-dockerized.sh
@@ -55,7 +55,7 @@ function generate_mocks {
     "pkg/controller/networkpolicy EndpointQuerier testing"
     "pkg/controller/querier ControllerQuerier testing"
     "pkg/ipfix IPFIXExportingProcess,IPFIXRegistry,IPFIXCollectingProcess,IPFIXAggregationProcess testing"
-    "pkg/ovs/openflow Bridge,Table,Flow,Action,CTAction,FlowBuilder testing"
+    "pkg/ovs/openflow Bridge,Table,Flow,Action,CTAction,FlowBuilder,Group,BucketBuilder testing"
     "pkg/ovs/ovsconfig OVSBridgeClient testing"
     "pkg/ovs/ovsctl OVSCtlClient testing"
     "pkg/querier AgentNetworkPolicyInfoQuerier,AgentMulticastInfoQuerier testing"
diff --git a/pkg/agent/multicast/mcast_controller.go b/pkg/agent/multicast/mcast_controller.go
index f557041a2..e04b5dd64 100644
--- a/pkg/agent/multicast/mcast_controller.go
+++ b/pkg/agent/multicast/mcast_controller.go
@@ -520,7 +520,7 @@ func (c *Controller) syncGroup(groupKey string) error {
 					return err
 				}
 				// Remove the multicast flow entry if no local Pod is in the group.
-				if err := c.ofClient.UninstallGroup(status.ofGroupID); err != nil {
+				if err := c.ofClient.UninstallMulticastGroup(status.ofGroupID); err != nil {
 					klog.ErrorS(err, "Failed to uninstall multicast group", "group", groupKey)
 					return err
 				}
diff --git a/pkg/agent/openflow/client.go b/pkg/agent/openflow/client.go
index 18003d03a..007f6d447 100644
--- a/pkg/agent/openflow/client.go
+++ b/pkg/agent/openflow/client.go
@@ -84,9 +84,9 @@ type Client interface {
 	// InstallServiceGroup installs a group for Service LB. Each endpoint
 	// is a bucket of the group. For now, each bucket has the same weight.
 	InstallServiceGroup(groupID binding.GroupIDType, withSessionAffinity bool, endpoints []proxy.Endpoint) error
-	// UninstallGroup removes the group and its buckets that are
-	// installed by InstallServiceGroup or InstallMulticastGroup.
-	UninstallGroup(groupID binding.GroupIDType) error
+	// UninstallServiceGroup removes the group and its buckets that are
+	// installed by InstallServiceGroup.
+	UninstallServiceGroup(groupID binding.GroupIDType) error
 
 	// InstallEndpointFlows installs flows for accessing Endpoints.
 	// If an Endpoint is on the current Node, then flows for hairpin and endpoint
@@ -314,6 +314,9 @@ type Client interface {
 	UninstallTrafficControlReturnPortFlow(returnOFPort uint32) error
 
 	InstallMulticastGroup(ofGroupID binding.GroupIDType, localReceivers []uint32, remoteNodeReceivers []net.IP) error
+	// UninstallMulticastGroup removes the group and its buckets that are
+	// installed by InstallMulticastGroup.
+	UninstallMulticastGroup(groupID binding.GroupIDType) error
 
 	// SendIGMPRemoteReportPacketOut sends the IGMP report packet as a packet-out to remote Nodes via the tunnel port.
 	SendIGMPRemoteReportPacketOut(
@@ -627,7 +630,7 @@ func (c *client) InstallServiceGroup(groupID binding.GroupIDType, withSessionAff
 	return nil
 }
 
-func (c *client) UninstallGroup(groupID binding.GroupIDType) error {
+func (c *client) UninstallServiceGroup(groupID binding.GroupIDType) error {
 	c.replayMutex.RLock()
 	defer c.replayMutex.RUnlock()
 	if !c.bridge.DeleteGroup(groupID) {
@@ -1322,6 +1325,16 @@ func (c *client) InstallMulticastGroup(groupID binding.GroupIDType, localReceive
 	return nil
 }
 
+func (c *client) UninstallMulticastGroup(groupID binding.GroupIDType) error {
+	c.replayMutex.RLock()
+	defer c.replayMutex.RUnlock()
+	if !c.bridge.DeleteGroup(groupID) {
+		return fmt.Errorf("group %d delete failed", groupID)
+	}
+	c.featureMulticast.groupCache.Delete(groupID)
+	return nil
+}
+
 // InstallMulticlusterNodeFlows installs flows to handle cross-cluster packets between a regular
 // Node and a local Gateway.
 func (c *client) InstallMulticlusterNodeFlows(clusterID string,
diff --git a/pkg/agent/proxy/proxier.go b/pkg/agent/proxy/proxier.go
index 2c22baeb7..690b8a559 100644
--- a/pkg/agent/proxy/proxier.go
+++ b/pkg/agent/proxy/proxier.go
@@ -178,7 +178,7 @@ func (p *proxier) removeStaleServices() {
 		// Remove Service group whose Endpoints are local.
 		if svcInfo.NodeLocalExternal() {
 			if groupIDLocal, exist := p.groupCounter.Get(svcPortName, true); exist {
-				if err := p.ofClient.UninstallGroup(groupIDLocal); err != nil {
+				if err := p.ofClient.UninstallServiceGroup(groupIDLocal); err != nil {
 					klog.ErrorS(err, "Failed to remove Group of local Endpoints for Service", "Service", svcPortName)
 					continue
 				}
@@ -187,7 +187,7 @@ func (p *proxier) removeStaleServices() {
 		}
 		// Remove Service group which has all Endpoints.
 		if groupID, exist := p.groupCounter.Get(svcPortName, false); exist {
-			if err := p.ofClient.UninstallGroup(groupID); err != nil {
+			if err := p.ofClient.UninstallServiceGroup(groupID); err != nil {
 				klog.ErrorS(err, "Failed to remove Group of all Endpoints for Service", "Service", svcPortName)
 				continue
 			}
@@ -563,7 +563,7 @@ func (p *proxier) installServices() {
 					continue
 				}
 				if groupID, exist := p.groupCounter.Get(svcPortName, !nodeLocalVal); exist {
-					if err := p.ofClient.UninstallGroup(groupID); err != nil {
+					if err := p.ofClient.UninstallServiceGroup(groupID); err != nil {
 						klog.ErrorS(err, "Failed to uninstall Group of all Endpoints for Service", "Service", svcPortName)
 						continue
 					}


Test patch:
diff --git a/pkg/agent/multicast/mcast_controller_test.go b/pkg/agent/multicast/mcast_controller_test.go
index af1ec5931..06fedf573 100644
--- a/pkg/agent/multicast/mcast_controller_test.go
+++ b/pkg/agent/multicast/mcast_controller_test.go
@@ -157,12 +157,12 @@ func TestCheckLastMember(t *testing.T) {
 		if ev != nil {
 			status.group = ev.group
 			if ev.eType == groupLeave {
-				mockOFClient.EXPECT().UninstallGroup(gomock.Any())
+				mockOFClient.EXPECT().UninstallMulticastGroup(gomock.Any())
 				mockOFClient.EXPECT().UninstallMulticastFlows(status.group)
 			}
 		} else {
 			status.group = mgroup
-			mockOFClient.EXPECT().UninstallGroup(gomock.Any())
+			mockOFClient.EXPECT().UninstallMulticastGroup(gomock.Any())
 			mockOFClient.EXPECT().UninstallMulticastFlows(status.group)
 		}
 		_ = mctrl.groupCache.Add(status)
@@ -268,7 +268,7 @@ func TestClearStaleGroups(t *testing.T) {
 			fakePort++
 		}
 	}
-	mockOFClient.EXPECT().UninstallGroup(gomock.Any()).Times(len(staleGroups))
+	mockOFClient.EXPECT().UninstallMulticastGroup(gomock.Any()).Times(len(staleGroups))
 	mockOFClient.EXPECT().UninstallMulticastFlows(gomock.Any()).Times(len(staleGroups))
 	mockMulticastSocket.EXPECT().MulticastInterfaceLeaveMgroup(gomock.Any(), gomock.Any(), gomock.Any()).Times(len(staleGroups))
 	mctrl.clearStaleGroups()
@@ -511,7 +511,7 @@ func TestEncapLocalReportAndNotifyRemote(t *testing.T) {
 				}
 			}
 			if tc.groupChanged {
-				mockOFClient.EXPECT().UninstallGroup(gomock.Any())
+				mockOFClient.EXPECT().UninstallMulticastGroup(gomock.Any())
 				mockOFClient.EXPECT().UninstallMulticastFlows(tc.e.group)
 				mockMulticastSocket.EXPECT().MulticastInterfaceLeaveMgroup(gomock.Any(), gomock.Any(), gomock.Any()).Times(1)
 				mockOFClient.EXPECT().SendIGMPRemoteReportPacketOut(igmpReportDstMac, types.IGMPv3Router, gomock.Any())
diff --git a/pkg/agent/openflow/client_test.go b/pkg/agent/openflow/client_test.go
index 4193a9ebd..c19d698c4 100644
--- a/pkg/agent/openflow/client_test.go
+++ b/pkg/agent/openflow/client_test.go
@@ -34,6 +34,7 @@ import (
 	ovsoftest "antrea.io/antrea/pkg/ovs/openflow/testing"
 	"antrea.io/antrea/pkg/ovs/ovsconfig"
 	utilip "antrea.io/antrea/pkg/util/ip"
+	k8sproxy "antrea.io/antrea/third_party/proxy"
 )
 
 const bridgeName = "dummy-br"
@@ -583,3 +584,128 @@ func TestMulticlusterFlowsInstallation(t *testing.T) {
 	_, ok = client.featureMulticluster.cachedFlows.Load(cacheKey)
 	require.False(t, ok)
 }
+
+func TestServiceGroupInstallAndUninstall(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	mockOFBridge := ovsoftest.NewMockBridge(ctrl)
+	m := oftest.NewMockOFEntryOperations(ctrl)
+	ofClient := NewClient(bridgeName, bridgeMgmtAddr, true, false, false, false, true, false, false, false, false)
+	client := ofClient.(*client)
+	client.bridge = mockOFBridge
+	client.cookieAllocator = cookie.NewAllocator(0)
+	client.ofEntryOperations = m
+	client.nodeConfig = nodeConfig
+	client.networkConfig = networkConfig
+	client.serviceConfig = serviceConfig
+	client.ipProtocols = []binding.Protocol{binding.ProtocolIP}
+	client.generatePipelines()
+	endpoints := []k8sproxy.Endpoint{
+		&k8sproxy.BaseEndpointInfo{
+			Endpoint: net.JoinHostPort("192.168.1.2", "8081"),
+			IsLocal:  true,
+		},
+		&k8sproxy.BaseEndpointInfo{
+			Endpoint: net.JoinHostPort("10.20.1.11", "8081"),
+			IsLocal:  false,
+		},
+	}
+	groupID1 := binding.GroupIDType(1)
+	groupID2 := binding.GroupIDType(2)
+	for _, tc := range []struct {
+		groupID         binding.GroupIDType
+		sessionAffinity bool
+		deleteSucceeded bool
+	}{
+		{groupID: groupID1, deleteSucceeded: false, sessionAffinity: true},
+		{groupID: groupID2, deleteSucceeded: true, sessionAffinity: false},
+	} {
+		mockGroup := ovsoftest.NewMockGroup(ctrl)
+		mockBucketBuilder := ovsoftest.NewMockBucketBuilder(ctrl)
+		mockOFBridge.EXPECT().CreateGroup(tc.groupID).Return(mockGroup).Times(1)
+		mockGroup.EXPECT().ResetBuckets().Return(mockGroup).Times(1)
+		mockGroup.EXPECT().Bucket().Return(mockBucketBuilder).Times(len(endpoints))
+		mockBucketBuilder.EXPECT().Weight(gomock.Any()).Return(mockBucketBuilder).Times(len(endpoints))
+		mockBucketBuilder.EXPECT().LoadToRegField(gomock.Any(), gomock.Any()).Return(mockBucketBuilder).AnyTimes()
+		if tc.sessionAffinity {
+			mockBucketBuilder.EXPECT().ResubmitToTable(ServiceLBTable.GetID()).Return(mockBucketBuilder).Times(len(endpoints))
+		} else {
+			mockBucketBuilder.EXPECT().ResubmitToTable(EndpointDNATTable.GetID()).Return(mockBucketBuilder).Times(len(endpoints))
+		}
+		mockBucketBuilder.EXPECT().Done().Return(mockGroup).Times(len(endpoints))
+		mockGroup.EXPECT().Add().Return(nil)
+		err := client.InstallServiceGroup(tc.groupID, tc.sessionAffinity, endpoints)
+		require.NoError(t, err)
+		_, exists := client.featureService.groupCache.Load(tc.groupID)
+		assert.True(t, exists)
+		mockOFBridge.EXPECT().DeleteGroup(tc.groupID).Return(tc.deleteSucceeded)
+		err = client.UninstallServiceGroup(tc.groupID)
+		_, exists = client.featureService.groupCache.Load(tc.groupID)
+		if tc.deleteSucceeded {
+			assert.NoError(t, err)
+			assert.False(t, exists)
+		} else {
+			assert.Error(t, err)
+			assert.True(t, exists)
+		}
+	}
+}
+
+func TestMulticastGroupInstallAndUninstall(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	mockOFBridge := ovsoftest.NewMockBridge(ctrl)
+	m := oftest.NewMockOFEntryOperations(ctrl)
+	ofClient := NewClient(bridgeName, bridgeMgmtAddr, false, true, false, false, false, false, true, false, false)
+	client := ofClient.(*client)
+	client.bridge = mockOFBridge
+	client.cookieAllocator = cookie.NewAllocator(0)
+	client.ofEntryOperations = m
+	client.nodeConfig = nodeConfig
+	client.networkConfig = networkConfig
+	client.serviceConfig = serviceConfig
+	client.ipProtocols = []binding.Protocol{binding.ProtocolIP}
+	client.generatePipelines()
+	localReceivers := []uint32{101, 102}
+	remoteIPs := []net.IP{
+		net.ParseIP("1.1.1.2"),
+		net.ParseIP("1.1.1.3"),
+		net.ParseIP("1.1.1.4"),
+	}
+	groupID1 := binding.GroupIDType(1)
+	groupID2 := binding.GroupIDType(2)
+	for _, tc := range []struct {
+		groupID         binding.GroupIDType
+		deleteSucceeded bool
+	}{
+		{groupID: groupID1, deleteSucceeded: false},
+		{groupID: groupID2, deleteSucceeded: true},
+	} {
+		mockGroup := ovsoftest.NewMockGroup(ctrl)
+		mockBucketBuilder := ovsoftest.NewMockBucketBuilder(ctrl)
+		mockOFBridge.EXPECT().CreateGroupTypeAll(tc.groupID).Return(mockGroup).Times(1)
+		mockGroup.EXPECT().ResetBuckets().Return(mockGroup).Times(1)
+		mockGroup.EXPECT().Bucket().Return(mockBucketBuilder).Times(len(localReceivers) + len(remoteIPs))
+		mockBucketBuilder.EXPECT().LoadToRegField(gomock.Any(), gomock.Any()).Return(mockBucketBuilder).AnyTimes()
+		mockBucketBuilder.EXPECT().ResubmitToTable(MulticastIngressRuleTable.GetID()).Return(mockBucketBuilder).Times(len(localReceivers))
+		mockBucketBuilder.EXPECT().ResubmitToTable(MulticastOutputTable.GetID()).Return(mockBucketBuilder).Times(len(remoteIPs))
+		mockBucketBuilder.EXPECT().SetTunnelDst(gomock.Any()).Return(mockBucketBuilder).Times(len(remoteIPs))
+		mockBucketBuilder.EXPECT().Done().Return(mockGroup).Times(len(localReceivers) + len(remoteIPs))
+		mockGroup.EXPECT().Add().Return(nil)
+		err := client.InstallMulticastGroup(tc.groupID, localReceivers, remoteIPs)
+		require.NoError(t, err)
+		_, exists := client.featureMulticast.groupCache.Load(tc.groupID)
+		assert.True(t, exists)
+		mockOFBridge.EXPECT().DeleteGroup(tc.groupID).Return(tc.deleteSucceeded)
+		err = client.UninstallMulticastGroup(tc.groupID)
+		if tc.deleteSucceeded {
+			assert.NoError(t, err)
+			_, exists = client.featureMulticast.groupCache.Load(tc.groupID)
+			assert.False(t, exists)
+		} else {
+			assert.Error(t, err)
+			_, exists = client.featureMulticast.groupCache.Load(tc.groupID)
+			assert.True(t, exists)
+		}
+	}
+}
diff --git a/pkg/agent/openflow/testing/mock_openflow.go b/pkg/agent/openflow/testing/mock_openflow.go
index b118ee1b3..be27e495a 100644
--- a/pkg/agent/openflow/testing/mock_openflow.go
+++ b/pkg/agent/openflow/testing/mock_openflow.go
@@ -808,32 +808,32 @@ func (mr *MockClientMockRecorder) UninstallEndpointFlows(arg0, arg1 interface{})
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UninstallEndpointFlows", reflect.TypeOf((*MockClient)(nil).UninstallEndpointFlows), arg0, arg1)
 }
 
-// UninstallGroup mocks base method
-func (m *MockClient) UninstallGroup(arg0 openflow.GroupIDType) error {
+// UninstallMulticastFlows mocks base method
+func (m *MockClient) UninstallMulticastFlows(arg0 net.IP) error {
 	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "UninstallGroup", arg0)
+	ret := m.ctrl.Call(m, "UninstallMulticastFlows", arg0)
 	ret0, _ := ret[0].(error)
 	return ret0
 }
 
-// UninstallGroup indicates an expected call of UninstallGroup
-func (mr *MockClientMockRecorder) UninstallGroup(arg0 interface{}) *gomock.Call {
+// UninstallMulticastFlows indicates an expected call of UninstallMulticastFlows
+func (mr *MockClientMockRecorder) UninstallMulticastFlows(arg0 interface{}) *gomock.Call {
 	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UninstallGroup", reflect.TypeOf((*MockClient)(nil).UninstallGroup), arg0)
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UninstallMulticastFlows", reflect.TypeOf((*MockClient)(nil).UninstallMulticastFlows), arg0)
 }
 
-// UninstallMulticastFlows mocks base method
-func (m *MockClient) UninstallMulticastFlows(arg0 net.IP) error {
+// UninstallMulticastGroup mocks base method
+func (m *MockClient) UninstallMulticastGroup(arg0 openflow.GroupIDType) error {
 	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "UninstallMulticastFlows", arg0)
+	ret := m.ctrl.Call(m, "UninstallMulticastGroup", arg0)
 	ret0, _ := ret[0].(error)
 	return ret0
 }
 
-// UninstallMulticastFlows indicates an expected call of UninstallMulticastFlows
-func (mr *MockClientMockRecorder) UninstallMulticastFlows(arg0 interface{}) *gomock.Call {
+// UninstallMulticastGroup indicates an expected call of UninstallMulticastGroup
+func (mr *MockClientMockRecorder) UninstallMulticastGroup(arg0 interface{}) *gomock.Call {
 	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UninstallMulticastFlows", reflect.TypeOf((*MockClient)(nil).UninstallMulticastFlows), arg0)
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UninstallMulticastGroup", reflect.TypeOf((*MockClient)(nil).UninstallMulticastGroup), arg0)
 }
 
 // UninstallMulticlusterFlows mocks base method
@@ -935,6 +935,20 @@ func (mr *MockClientMockRecorder) UninstallServiceFlows(arg0, arg1, arg2 interfa
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UninstallServiceFlows", reflect.TypeOf((*MockClient)(nil).UninstallServiceFlows), arg0, arg1, arg2)
 }
 
+// UninstallServiceGroup mocks base method
+func (m *MockClient) UninstallServiceGroup(arg0 openflow.GroupIDType) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "UninstallServiceGroup", arg0)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// UninstallServiceGroup indicates an expected call of UninstallServiceGroup
+func (mr *MockClientMockRecorder) UninstallServiceGroup(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UninstallServiceGroup", reflect.TypeOf((*MockClient)(nil).UninstallServiceGroup), arg0)
+}
+
 // UninstallTraceflowFlows mocks base method
 func (m *MockClient) UninstallTraceflowFlows(arg0 byte) error {
 	m.ctrl.T.Helper()
diff --git a/pkg/agent/proxy/proxier_test.go b/pkg/agent/proxy/proxier_test.go
index 78f17b93f..f55b4ed29 100644
--- a/pkg/agent/proxy/proxier_test.go
+++ b/pkg/agent/proxy/proxier_test.go
@@ -347,7 +347,7 @@ func testLoadBalancer(t *testing.T, nodePortAddresses []net.IP, svcIP, ep1IP, ep
 			mockOFClient.EXPECT().InstallServiceFlows(groupID, loadBalancerIP, uint16(svcPort), bindingProtocol, uint16(0), nodeLocalExternal, corev1.ServiceTypeLoadBalancer).Times(1)
 		}
 		groupID = fp.groupCounter.AllocateIfNotExist(svcPortName, !nodeLocalVal)
-		mockOFClient.EXPECT().UninstallGroup(groupID).Times(1)
+		mockOFClient.EXPECT().UninstallServiceGroup(groupID).Times(1)
 	}
 	mockRouteClient.EXPECT().AddClusterIPRoute(svcIP).Times(1)
 	if proxyLoadBalancerIPs {
@@ -466,7 +466,7 @@ func testNodePort(t *testing.T, nodePortAddresses []net.IP, svcIP, ep1IP, ep2IP
 		mockOFClient.EXPECT().InstallServiceFlows(groupID, gomock.Any(), uint16(svcNodePort), bindingProtocol, uint16(0), nodeLocalExternal, corev1.ServiceTypeNodePort).Times(1)
 
 		groupID = fp.groupCounter.AllocateIfNotExist(svcPortName, !nodeLocalVal)
-		mockOFClient.EXPECT().UninstallGroup(groupID).Times(1)
+		mockOFClient.EXPECT().UninstallServiceGroup(groupID).Times(1)
 	}
 	mockRouteClient.EXPECT().AddClusterIPRoute(svcIP).Times(1)
 	mockRouteClient.EXPECT().AddNodePort(gomock.Any(), uint16(svcNodePort), bindingProtocol).Times(1)
@@ -748,7 +748,7 @@ func testClusterIPRemoval(t *testing.T, svcIP net.IP, epIP net.IP, isIPv6 bool)
 	mockRouteClient.EXPECT().AddClusterIPRoute(svcIP).Times(1)
 	mockOFClient.EXPECT().UninstallServiceFlows(svcIP, uint16(svcPort), bindingProtocol).Times(1)
 	mockOFClient.EXPECT().UninstallEndpointFlows(bindingProtocol, gomock.Any()).Times(1)
-	mockOFClient.EXPECT().UninstallGroup(gomock.Any()).Times(1)
+	mockOFClient.EXPECT().UninstallServiceGroup(gomock.Any()).Times(1)
 	fp.syncProxyRules()
 
 	fp.serviceChanges.OnServiceUpdate(service, nil)
@@ -1231,8 +1231,8 @@ func TestServicesWithSameEndpoints(t *testing.T) {
 	mockOFClient.EXPECT().InstallServiceFlows(groupID2, svcIP2, uint16(svcPort), bindingProtocol, uint16(0), false, corev1.ServiceTypeClusterIP).Times(1)
 	mockOFClient.EXPECT().UninstallServiceFlows(svcIP1, uint16(svcPort), bindingProtocol).Times(1)
 	mockOFClient.EXPECT().UninstallServiceFlows(svcIP2, uint16(svcPort), bindingProtocol).Times(1)
-	mockOFClient.EXPECT().UninstallGroup(groupID1).Times(1)
-	mockOFClient.EXPECT().UninstallGroup(groupID2).Times(1)
+	mockOFClient.EXPECT().UninstallServiceGroup(groupID1).Times(1)
+	mockOFClient.EXPECT().UninstallServiceGroup(groupID2).Times(1)
 	// Since these two Services reference to the same Endpoint, there should only be one operation.
 	mockOFClient.EXPECT().UninstallEndpointFlows(bindingProtocol, gomock.Any()).Times(1)
 
diff --git a/pkg/ovs/openflow/testing/mock_openflow.go b/pkg/ovs/openflow/testing/mock_openflow.go
index 6f1b6ef6e..ee337edb3 100644
--- a/pkg/ovs/openflow/testing/mock_openflow.go
+++ b/pkg/ovs/openflow/testing/mock_openflow.go
@@ -14,7 +14,7 @@
 //
 
 // Code generated by MockGen. DO NOT EDIT.
-// Source: antrea.io/antrea/pkg/ovs/openflow (interfaces: Bridge,Table,Flow,Action,CTAction,FlowBuilder)
+// Source: antrea.io/antrea/pkg/ovs/openflow (interfaces: Bridge,Table,Flow,Action,CTAction,FlowBuilder,Group,BucketBuilder)
 
 // Package testing is a generated GoMock package.
 package testing
@@ -2168,3 +2168,286 @@ func (mr *MockFlowBuilderMockRecorder) SetIdleTimeout(arg0 interface{}) *gomock.
 	mr.mock.ctrl.T.Helper()
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetIdleTimeout", reflect.TypeOf((*MockFlowBuilder)(nil).SetIdleTimeout), arg0)
 }
+
+// MockGroup is a mock of Group interface
+type MockGroup struct {
+	ctrl     *gomock.Controller
+	recorder *MockGroupMockRecorder
+}
+
+// MockGroupMockRecorder is the mock recorder for MockGroup
+type MockGroupMockRecorder struct {
+	mock *MockGroup
+}
+
+// NewMockGroup creates a new mock instance
+func NewMockGroup(ctrl *gomock.Controller) *MockGroup {
+	mock := &MockGroup{ctrl: ctrl}
+	mock.recorder = &MockGroupMockRecorder{mock}
+	return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use
+func (m *MockGroup) EXPECT() *MockGroupMockRecorder {
+	return m.recorder
+}
+
+// Add mocks base method
+func (m *MockGroup) Add() error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Add")
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// Add indicates an expected call of Add
+func (mr *MockGroupMockRecorder) Add() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Add", reflect.TypeOf((*MockGroup)(nil).Add))
+}
+
+// Bucket mocks base method
+func (m *MockGroup) Bucket() openflow.BucketBuilder {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Bucket")
+	ret0, _ := ret[0].(openflow.BucketBuilder)
+	return ret0
+}
+
+// Bucket indicates an expected call of Bucket
+func (mr *MockGroupMockRecorder) Bucket() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Bucket", reflect.TypeOf((*MockGroup)(nil).Bucket))
+}
+
+// Delete mocks base method
+func (m *MockGroup) Delete() error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Delete")
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// Delete indicates an expected call of Delete
+func (mr *MockGroupMockRecorder) Delete() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockGroup)(nil).Delete))
+}
+
+// GetBundleMessage mocks base method
+func (m *MockGroup) GetBundleMessage(arg0 openflow.OFOperation) (ofctrl.OpenFlowModMessage, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "GetBundleMessage", arg0)
+	ret0, _ := ret[0].(ofctrl.OpenFlowModMessage)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// GetBundleMessage indicates an expected call of GetBundleMessage
+func (mr *MockGroupMockRecorder) GetBundleMessage(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetBundleMessage", reflect.TypeOf((*MockGroup)(nil).GetBundleMessage), arg0)
+}
+
+// KeyString mocks base method
+func (m *MockGroup) KeyString() string {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "KeyString")
+	ret0, _ := ret[0].(string)
+	return ret0
+}
+
+// KeyString indicates an expected call of KeyString
+func (mr *MockGroupMockRecorder) KeyString() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "KeyString", reflect.TypeOf((*MockGroup)(nil).KeyString))
+}
+
+// Modify mocks base method
+func (m *MockGroup) Modify() error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Modify")
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// Modify indicates an expected call of Modify
+func (mr *MockGroupMockRecorder) Modify() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Modify", reflect.TypeOf((*MockGroup)(nil).Modify))
+}
+
+// Reset mocks base method
+func (m *MockGroup) Reset() {
+	m.ctrl.T.Helper()
+	m.ctrl.Call(m, "Reset")
+}
+
+// Reset indicates an expected call of Reset
+func (mr *MockGroupMockRecorder) Reset() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Reset", reflect.TypeOf((*MockGroup)(nil).Reset))
+}
+
+// ResetBuckets mocks base method
+func (m *MockGroup) ResetBuckets() openflow.Group {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "ResetBuckets")
+	ret0, _ := ret[0].(openflow.Group)
+	return ret0
+}
+
+// ResetBuckets indicates an expected call of ResetBuckets
+func (mr *MockGroupMockRecorder) ResetBuckets() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResetBuckets", reflect.TypeOf((*MockGroup)(nil).ResetBuckets))
+}
+
+// Type mocks base method
+func (m *MockGroup) Type() openflow.EntryType {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Type")
+	ret0, _ := ret[0].(openflow.EntryType)
+	return ret0
+}
+
+// Type indicates an expected call of Type
+func (mr *MockGroupMockRecorder) Type() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Type", reflect.TypeOf((*MockGroup)(nil).Type))
+}
+
+// MockBucketBuilder is a mock of BucketBuilder interface
+type MockBucketBuilder struct {
+	ctrl     *gomock.Controller
+	recorder *MockBucketBuilderMockRecorder
+}
+
+// MockBucketBuilderMockRecorder is the mock recorder for MockBucketBuilder
+type MockBucketBuilderMockRecorder struct {
+	mock *MockBucketBuilder
+}
+
+// NewMockBucketBuilder creates a new mock instance
+func NewMockBucketBuilder(ctrl *gomock.Controller) *MockBucketBuilder {
+	mock := &MockBucketBuilder{ctrl: ctrl}
+	mock.recorder = &MockBucketBuilderMockRecorder{mock}
+	return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use
+func (m *MockBucketBuilder) EXPECT() *MockBucketBuilderMockRecorder {
+	return m.recorder
+}
+
+// Done mocks base method
+func (m *MockBucketBuilder) Done() openflow.Group {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Done")
+	ret0, _ := ret[0].(openflow.Group)
+	return ret0
+}
+
+// Done indicates an expected call of Done
+func (mr *MockBucketBuilderMockRecorder) Done() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Done", reflect.TypeOf((*MockBucketBuilder)(nil).Done))
+}
+
+// LoadReg mocks base method
+func (m *MockBucketBuilder) LoadReg(arg0 int, arg1 uint32) openflow.BucketBuilder {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "LoadReg", arg0, arg1)
+	ret0, _ := ret[0].(openflow.BucketBuilder)
+	return ret0
+}
+
+// LoadReg indicates an expected call of LoadReg
+func (mr *MockBucketBuilderMockRecorder) LoadReg(arg0, arg1 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LoadReg", reflect.TypeOf((*MockBucketBuilder)(nil).LoadReg), arg0, arg1)
+}
+
+// LoadRegRange mocks base method
+func (m *MockBucketBuilder) LoadRegRange(arg0 int, arg1 uint32, arg2 *openflow.Range) openflow.BucketBuilder {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "LoadRegRange", arg0, arg1, arg2)
+	ret0, _ := ret[0].(openflow.BucketBuilder)
+	return ret0
+}
+
+// LoadRegRange indicates an expected call of LoadRegRange
+func (mr *MockBucketBuilderMockRecorder) LoadRegRange(arg0, arg1, arg2 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LoadRegRange", reflect.TypeOf((*MockBucketBuilder)(nil).LoadRegRange), arg0, arg1, arg2)
+}
+
+// LoadToRegField mocks base method
+func (m *MockBucketBuilder) LoadToRegField(arg0 *openflow.RegField, arg1 uint32) openflow.BucketBuilder {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "LoadToRegField", arg0, arg1)
+	ret0, _ := ret[0].(openflow.BucketBuilder)
+	return ret0
+}
+
+// LoadToRegField indicates an expected call of LoadToRegField
+func (mr *MockBucketBuilderMockRecorder) LoadToRegField(arg0, arg1 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LoadToRegField", reflect.TypeOf((*MockBucketBuilder)(nil).LoadToRegField), arg0, arg1)
+}
+
+// LoadXXReg mocks base method
+func (m *MockBucketBuilder) LoadXXReg(arg0 int, arg1 []byte) openflow.BucketBuilder {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "LoadXXReg", arg0, arg1)
+	ret0, _ := ret[0].(openflow.BucketBuilder)
+	return ret0
+}
+
+// LoadXXReg indicates an expected call of LoadXXReg
+func (mr *MockBucketBuilderMockRecorder) LoadXXReg(arg0, arg1 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LoadXXReg", reflect.TypeOf((*MockBucketBuilder)(nil).LoadXXReg), arg0, arg1)
+}
+
+// ResubmitToTable mocks base method
+func (m *MockBucketBuilder) ResubmitToTable(arg0 byte) openflow.BucketBuilder {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "ResubmitToTable", arg0)
+	ret0, _ := ret[0].(openflow.BucketBuilder)
+	return ret0
+}
+
+// ResubmitToTable indicates an expected call of ResubmitToTable
+func (mr *MockBucketBuilderMockRecorder) ResubmitToTable(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResubmitToTable", reflect.TypeOf((*MockBucketBuilder)(nil).ResubmitToTable), arg0)
+}
+
+// SetTunnelDst mocks base method
+func (m *MockBucketBuilder) SetTunnelDst(arg0 net.IP) openflow.BucketBuilder {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "SetTunnelDst", arg0)
+	ret0, _ := ret[0].(openflow.BucketBuilder)
+	return ret0
+}
+
+// SetTunnelDst indicates an expected call of SetTunnelDst
+func (mr *MockBucketBuilderMockRecorder) SetTunnelDst(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetTunnelDst", reflect.TypeOf((*MockBucketBuilder)(nil).SetTunnelDst), arg0)
+}
+
+// Weight mocks base method
+func (m *MockBucketBuilder) Weight(arg0 uint16) openflow.BucketBuilder {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Weight", arg0)
+	ret0, _ := ret[0].(openflow.BucketBuilder)
+	return ret0
+}
+
+// Weight indicates an expected call of Weight
+func (mr *MockBucketBuilderMockRecorder) Weight(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Weight", reflect.TypeOf((*MockBucketBuilder)(nil).Weight), arg0)
+}
diff --git a/test/integration/agent/openflow_test.go b/test/integration/agent/openflow_test.go
index 851853f4d..7a59344e3 100644
--- a/test/integration/agent/openflow_test.go
+++ b/test/integration/agent/openflow_test.go
@@ -708,7 +708,7 @@ func uninstallServiceFlowsFunc(t *testing.T, gid uint32, svc svcConfig, endpoint
 	groupID := ofconfig.GroupIDType(gid)
 	err := c.UninstallServiceFlows(svc.ip, svc.port, svc.protocol)
 	assert.Nil(t, err)
-	err = c.UninstallGroup(groupID)
+	err = c.UninstallServiceGroup(groupID)
 	assert.Nil(t, err)
 	for _, ep := range endpointList {
 		err := c.UninstallEndpointFlows(svc.protocol, ep)