Repository: antrea-io/antrea
Pull request #4203 (2022-09-06 03:06:26)

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

#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/4203?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io) Report
> Merging [#4203](https://codecov.io/gh/antrea-io/antrea/pull/4203?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io) (97b8af2) into [release-1.7](https://codecov.io/gh/antrea-io/antrea/commit/3b0338970fa46a0d7f4356a45b7b5d31a435ecf9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io) (3b03389) will **decrease** coverage by `18.53%`.
> The diff coverage is `7.69%`.

[![Impacted file tree graph](https://codecov.io/gh/antrea-io/antrea/pull/4203/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/4203?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.7    #4203       +/-   ##
================================================
- Coverage        50.28%   31.74%   -18.54%     
================================================
  Files              261      258        -3     
  Lines            39347    38651      -696     
================================================
- Hits             19785    12270     -7515     
- Misses           17609    25063     +7454     
+ Partials          1953     1318      -635     
```

| Flag | Coverage Δ | |
|---|---|---|
| kind-e2e-tests | `31.74% <7.69%> (-18.54%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [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#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/antrea-io/antrea/pull/4203?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/4203/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io#diff-cGtnL2FnZW50L211bHRpY2FzdC9tY2FzdF9jb250cm9sbGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/agent/proxy/proxier.go](https://codecov.io/gh/antrea-io/antrea/pull/4203/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io#diff-cGtnL2FnZW50L3Byb3h5L3Byb3hpZXIuZ28=) | `41.46% <0.00%> (-13.34%)` | :arrow_down: |
| [pkg/agent/openflow/client.go](https://codecov.io/gh/antrea-io/antrea/pull/4203/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io#diff-cGtnL2FnZW50L29wZW5mbG93L2NsaWVudC5nbw==) | `38.91% <11.11%> (-22.70%)` | :arrow_down: |
| [pkg/util/sets/int32.go](https://codecov.io/gh/antrea-io/antrea/pull/4203/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io#diff-cGtnL3V0aWwvc2V0cy9pbnQzMi5nbw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [pkg/apis/controlplane/v1beta2/helper.go](https://codecov.io/gh/antrea-io/antrea/pull/4203/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io#diff-cGtnL2FwaXMvY29udHJvbHBsYW5lL3YxYmV0YTIvaGVscGVyLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [pkg/agent/nodeportlocal/k8s/annotations.go](https://codecov.io/gh/antrea-io/antrea/pull/4203/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io#diff-cGtnL2FnZW50L25vZGVwb3J0bG9jYWwvazhzL2Fubm90YXRpb25zLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [pkg/ovs/openflow/ofctrl\_pipeline.go](https://codecov.io/gh/antrea-io/antrea/pull/4203/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io#diff-cGtnL292cy9vcGVuZmxvdy9vZmN0cmxfcGlwZWxpbmUuZ28=) | `0.00% <0.00%> (-95.84%)` | :arrow_down: |
| [pkg/ovs/openflow/logs.go](https://codecov.io/gh/antrea-io/antrea/pull/4203/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io#diff-cGtnL292cy9vcGVuZmxvdy9sb2dzLmdv) | `10.34% <0.00%> (-89.66%)` | :arrow_down: |
| [pkg/util/channel/channel.go](https://codecov.io/gh/antrea-io/antrea/pull/4203/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io#diff-cGtnL3V0aWwvY2hhbm5lbC9jaGFubmVsLmdv) | `0.00% <0.00%> (-80.65%)` | :arrow_down: |
| [...g/agent/controller/serviceexternalip/controller.go](https://codecov.io/gh/antrea-io/antrea/pull/4203/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io#diff-cGtnL2FnZW50L2NvbnRyb2xsZXIvc2VydmljZWV4dGVybmFsaXAvY29udHJvbGxlci5nbw==) | `0.00% <0.00%> (-80.55%)` | :arrow_down: |
| ... and [132 more](https://codecov.io/gh/antrea-io/antrea/pull/4203/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=antrea-io) | |

tnqn: /test-all

Patch:
diff --git a/hack/update-codegen-dockerized.sh b/hack/update-codegen-dockerized.sh
index 4f59a72e9..dc686ef12 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 f00d5991f..2669852d7 100644
--- a/pkg/agent/multicast/mcast_controller.go
+++ b/pkg/agent/multicast/mcast_controller.go
@@ -388,7 +388,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 5606158e1..30b2d163c 100644
--- a/pkg/agent/openflow/client.go
+++ b/pkg/agent/openflow/client.go
@@ -83,9 +83,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
@@ -305,6 +305,10 @@ type Client interface {
 
 	InstallMulticastGroup(ofGroupID binding.GroupIDType, localReceivers []uint32) error
 
+	// UninstallMulticastGroup removes the group and its buckets that are
+	// installed by InstallMulticastGroup.
+	UninstallMulticastGroup(groupID binding.GroupIDType) error
+
 	// InstallMulticlusterNodeFlows installs flows to handle cross-cluster packets between a regular
 	// Node and a local Gateway.
 	InstallMulticlusterNodeFlows(
@@ -599,7 +603,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) {
@@ -1253,6 +1257,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 d891f0357..3c8be7238 100644
--- a/pkg/agent/proxy/proxier.go
+++ b/pkg/agent/proxy/proxier.go
@@ -173,7 +173,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
 				}
@@ -182,7 +182,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
 			}
@@ -555,7 +555,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 e1bf4a3d2..4eeed0409 100644
--- a/pkg/agent/multicast/mcast_controller_test.go
+++ b/pkg/agent/multicast/mcast_controller_test.go
@@ -156,12 +156,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)
@@ -265,7 +265,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()
diff --git a/pkg/agent/openflow/client_test.go b/pkg/agent/openflow/client_test.go
index d698ab6fe..8f8e715f5 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"
@@ -582,3 +583,121 @@ 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}
+	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))
+		mockBucketBuilder.EXPECT().LoadToRegField(gomock.Any(), gomock.Any()).Return(mockBucketBuilder).AnyTimes()
+		mockBucketBuilder.EXPECT().ResubmitToTable(MulticastIngressRuleTable.GetID()).Return(mockBucketBuilder).Times(len(localReceivers))
+		mockBucketBuilder.EXPECT().Done().Return(mockGroup).Times(len(localReceivers))
+		mockGroup.EXPECT().Add().Return(nil)
+		err := client.InstallMulticastGroup(tc.groupID, localReceivers)
+		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 f59dc1900..2809b29e5 100644
--- a/pkg/agent/openflow/testing/mock_openflow.go
+++ b/pkg/agent/openflow/testing/mock_openflow.go
@@ -752,32 +752,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
@@ -879,6 +879,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 6c6ec07c2..fc809925f 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), nodeLocalVal, 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), nodeLocalVal, 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 45a1e3f47..125fec994 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
@@ -2154,3 +2154,272 @@ 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)
+}
+
+// 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 b3fdc9162..b7484b778 100644
--- a/test/integration/agent/openflow_test.go
+++ b/test/integration/agent/openflow_test.go
@@ -706,7 +706,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)