diff --git a/pkg/agent/controller/networkpolicy/cache_test.go b/pkg/agent/controller/networkpolicy/cache_test.go index 564a027eadf..e86970cfede 100644 --- a/pkg/agent/controller/networkpolicy/cache_test.go +++ b/pkg/agent/controller/networkpolicy/cache_test.go @@ -193,6 +193,18 @@ func newAddressGroupMember(ips ...string) *v1beta2.GroupMember { return &v1beta2.GroupMember{IPs: ipAddrs} } +func newAddressGroupPodMember(name, namespace string, ips ...string) *v1beta2.GroupMember { + ipAddrs := make([]v1beta2.IPAddress, len(ips)) + for idx, ip := range ips { + ipAddrs[idx] = v1beta2.IPAddress(net.ParseIP(ip)) + } + pod := &v1beta2.PodReference{ + Name: name, + Namespace: namespace, + } + return &v1beta2.GroupMember{Pod: pod, IPs: ipAddrs} +} + func TestRuleCacheAddAddressGroup(t *testing.T) { rule1 := &rule{ ID: "rule1", diff --git a/pkg/agent/controller/networkpolicy/reconciler.go b/pkg/agent/controller/networkpolicy/reconciler.go index c6b2c0eeca6..7b3dd3de6ba 100644 --- a/pkg/agent/controller/networkpolicy/reconciler.go +++ b/pkg/agent/controller/networkpolicy/reconciler.go @@ -631,8 +631,8 @@ func (r *reconciler) update(lastRealized *lastRealized, newRule *CompletedRule, } lastRealized.ofIDs[svcKey] = ofRule.FlowID } else { - addedTo := groupMembersToOFAddresses(members.Difference(prevMembersByServicesMap[svcKey])) - deletedTo := groupMembersToOFAddresses(prevMembersByServicesMap[svcKey].Difference(members)) + addedTo := ipsToOFAddresses(members.IPDifference(prevMembersByServicesMap[svcKey])) + deletedTo := ipsToOFAddresses(prevMembersByServicesMap[svcKey].IPDifference(members)) if err := r.updateOFRule(ofID, addedFrom, addedTo, deletedFrom, deletedTo, ofPriority); err != nil { return err } diff --git a/pkg/agent/controller/networkpolicy/reconciler_test.go b/pkg/agent/controller/networkpolicy/reconciler_test.go index 01abbe8e039..9a0fd557c0c 100644 --- a/pkg/agent/controller/networkpolicy/reconciler_test.go +++ b/pkg/agent/controller/networkpolicy/reconciler_test.go @@ -874,6 +874,37 @@ func TestReconcilerUpdate(t *testing.T) { false, false, }, + { + "updating-egress-rule-with-duplicate-ip", + &CompletedRule{ + rule: &rule{ID: "egress-rule", Direction: v1beta2.DirectionOut, SourceRef: &np1}, + ToAddresses: v1beta2.NewGroupMemberSet( + newAddressGroupPodMember("pod1", "ns1", "1.1.1.1"), + newAddressGroupPodMember("pod2", "ns1", "1.1.1.1"), + newAddressGroupPodMember("pod3", "ns1", "1.1.1.2"), + newAddressGroupPodMember("pod4", "ns1", "1.1.1.2"), + newAddressGroupPodMember("pod5", "ns1", "1.1.1.3"), + newAddressGroupPodMember("pod6", "ns1", "1.1.1.3"), + newAddressGroupPodMember("pod7", "ns1", "1.1.1.4"), + ), + TargetMembers: appliedToGroup1, + }, + &CompletedRule{ + rule: &rule{ID: "egress-rule", Direction: v1beta2.DirectionOut, SourceRef: &np1}, + ToAddresses: v1beta2.NewGroupMemberSet( + newAddressGroupPodMember("pod1", "ns1", "1.1.1.1"), + newAddressGroupPodMember("pod5", "ns1", "1.1.1.5"), + newAddressGroupPodMember("pod8", "ns1", "1.1.1.4"), + ), + TargetMembers: appliedToGroup2, + }, + ipsToOFAddresses(sets.NewString("3.3.3.3")), + ipsToOFAddresses(sets.NewString("1.1.1.5")), + ipsToOFAddresses(sets.NewString("2.2.2.2")), + ipsToOFAddresses(sets.NewString("1.1.1.2", "1.1.1.3")), + false, + false, + }, { "updating-egress-rule-deny-all", &CompletedRule{ @@ -948,16 +979,16 @@ func TestReconcilerUpdate(t *testing.T) { mockOFClient.EXPECT().UninstallPolicyRuleFlows(gomock.Any()) } if len(tt.expectedAddedFrom) > 0 { - mockOFClient.EXPECT().AddPolicyRuleAddress(gomock.Any(), types.SrcAddress, gomock.Eq(tt.expectedAddedFrom), priority) + mockOFClient.EXPECT().AddPolicyRuleAddress(gomock.Any(), types.SrcAddress, gomock.InAnyOrder(tt.expectedAddedFrom), priority) } if len(tt.expectedAddedTo) > 0 { - mockOFClient.EXPECT().AddPolicyRuleAddress(gomock.Any(), types.DstAddress, gomock.Eq(tt.expectedAddedTo), priority) + mockOFClient.EXPECT().AddPolicyRuleAddress(gomock.Any(), types.DstAddress, gomock.InAnyOrder(tt.expectedAddedTo), priority) } if len(tt.expectedDeletedFrom) > 0 { - mockOFClient.EXPECT().DeletePolicyRuleAddress(gomock.Any(), types.SrcAddress, gomock.Eq(tt.expectedDeletedFrom), priority) + mockOFClient.EXPECT().DeletePolicyRuleAddress(gomock.Any(), types.SrcAddress, gomock.InAnyOrder(tt.expectedDeletedFrom), priority) } if len(tt.expectedDeletedTo) > 0 { - mockOFClient.EXPECT().DeletePolicyRuleAddress(gomock.Any(), types.DstAddress, gomock.Eq(tt.expectedDeletedTo), priority) + mockOFClient.EXPECT().DeletePolicyRuleAddress(gomock.Any(), types.DstAddress, gomock.InAnyOrder(tt.expectedDeletedTo), priority) } r := newReconciler(mockOFClient, ifaceStore, testAsyncDeleteInterval) if err := r.Reconcile(tt.originalRule); (err != nil) != tt.wantErr {