diff --git a/multicluster/test/e2e/antreapolicy_test.go b/multicluster/test/e2e/antreapolicy_test.go index 493983323fc..463b9b3a62a 100644 --- a/multicluster/test/e2e/antreapolicy_test.go +++ b/multicluster/test/e2e/antreapolicy_test.go @@ -153,7 +153,7 @@ func executeTestsOnAllMemberClusters(t *testing.T, testList []*antreae2e.TestCas // skip verification for the leader cluster continue } - if err := k8sUtils.WaitForACNPCreatiionAndRealization(t, acnpName, policyRealizedTimeout); err != nil { + if err := k8sUtils.WaitForACNPCreationAndRealization(t, acnpName, policyRealizedTimeout); err != nil { t.Errorf("Failed to get ACNP to be replicated in cluster %s", clusterName) failOnError(err, t) } diff --git a/pkg/controller/labelidentity/label_group_index.go b/pkg/controller/labelidentity/label_group_index.go index d38b6f04230..73598fa8c93 100644 --- a/pkg/controller/labelidentity/label_group_index.go +++ b/pkg/controller/labelidentity/label_group_index.go @@ -54,7 +54,7 @@ type Interface interface { // DeleteSelector deletes or updates a selectorItem when a selector is deleted from a policy. DeleteSelector(selectorKey string, policyKey string) // RemoveStalePolicySelectors cleans up any outdated selector <-> policy mapping based on the policy's latest selectors. - RemoveStalePolicySelectors(selectors []*types.GroupSelector, policyKey string) + RemoveStalePolicySelectors(selectorKeys sets.String, policyKey string) // DeletePolicySelectors removes any selectors from referring to the policy being deleted. DeletePolicySelectors(policyKey string) // AddLabelIdentity adds LabelIdentity-ID mapping to the index. @@ -298,17 +298,13 @@ func (i *LabelIdentityIndex) deleteSelector(selectorKey string, policyKey string } // RemoveStalePolicySelectors cleans up any outdated selector <-> policy mapping based on the policy's latest selectors. -func (i *LabelIdentityIndex) RemoveStalePolicySelectors(selectors []*types.GroupSelector, policyKey string) { +func (i *LabelIdentityIndex) RemoveStalePolicySelectors(selectorKeys sets.String, policyKey string) { originalSelectors := i.getPolicySelectors(policyKey) if originalSelectors == nil { return } - if len(selectors) > 0 { - newSelectors := map[string]*types.GroupSelector{} - for _, s := range selectors { - newSelectors[s.NormalizedName] = s - } - for selKey := range newSelectors { + if selectorKeys.Len() > 0 { + for _, selKey := range selectorKeys.UnsortedList() { if _, exists := originalSelectors[selKey]; exists { // Remove matched ClusterSet-scoped selectors of the policy before and after the update. // The selectors remaining in originalSelectors will need to be unbounded from the policy. diff --git a/pkg/controller/labelidentity/label_group_index_test.go b/pkg/controller/labelidentity/label_group_index_test.go index 296d922aa23..921221be13f 100644 --- a/pkg/controller/labelidentity/label_group_index_test.go +++ b/pkg/controller/labelidentity/label_group_index_test.go @@ -336,10 +336,12 @@ func TestSetPolicySelectors(t *testing.T) { } } var labelIDs []uint32 + selectorKeys := sets.NewString() for _, sel := range tt.selectors { labelIDs = append(labelIDs, i.AddSelector(sel, tt.policyKey)...) + selectorKeys.Insert(sel.NormalizedName) } - i.RemoveStalePolicySelectors(tt.selectors, tt.policyKey) + i.RemoveStalePolicySelectors(selectorKeys, tt.policyKey) assert.ElementsMatch(t, tt.expIDs, dedupLabelIdentites(labelIDs)) assert.Equalf(t, len(tt.expSelectorItems), len(i.selectorItems.List()), "Unexpected number of cached selectorItems") for selKey, expSelItem := range tt.expSelectorItems { diff --git a/pkg/controller/networkpolicy/antreanetworkpolicy.go b/pkg/controller/networkpolicy/antreanetworkpolicy.go index 5ed17164188..81379e71670 100644 --- a/pkg/controller/networkpolicy/antreanetworkpolicy.go +++ b/pkg/controller/networkpolicy/antreanetworkpolicy.go @@ -86,11 +86,11 @@ func (n *NetworkPolicyController) processAntreaNetworkPolicy(np *crdv1alpha1.Net appliedToGroups := map[string]*antreatypes.AppliedToGroup{} addressGroups := map[string]*antreatypes.AddressGroup{} rules := make([]controlplane.NetworkPolicyRule, 0, len(np.Spec.Ingress)+len(np.Spec.Egress)) - // clusterSetScopeSelectors keeps track of all the ClusterSet-scoped selectors of the policy. + // clusterSetScopeSelectorKeys keeps track of all the ClusterSet-scoped selector keys of the policy. // During policy peer processing, any ClusterSet-scoped selector will be registered with the - // labelIdentityInterface and added to this list. By the end of the function, this list will + // labelIdentityInterface and added to this set. By the end of the function, this set will // be used to remove any stale selector from the policy in the labelIdentityInterface. - var clusterSetScopeSelectors []*antreatypes.GroupSelector + var clusterSetScopeSelectorKeys sets.String // Create AppliedToGroup for each AppliedTo present in AntreaNetworkPolicy spec. atgs := n.processAppliedTo(np.Namespace, np.Spec.AppliedTo) appliedToGroups = mergeAppliedToGroups(appliedToGroups, atgs...) @@ -101,7 +101,8 @@ func (n *NetworkPolicyController) processAntreaNetworkPolicy(np *crdv1alpha1.Net // Create AppliedToGroup for each AppliedTo present in the ingress rule. atgs := n.processAppliedTo(np.Namespace, ingressRule.AppliedTo) appliedToGroups = mergeAppliedToGroups(appliedToGroups, atgs...) - peer, ags := n.toAntreaPeerForCRD(ingressRule.From, np, controlplane.DirectionIn, namedPortExists, &clusterSetScopeSelectors) + peer, ags, selKeys := n.toAntreaPeerForCRD(ingressRule.From, np, controlplane.DirectionIn, namedPortExists) + clusterSetScopeSelectorKeys = clusterSetScopeSelectorKeys.Union(selKeys) addressGroups = mergeAddressGroups(addressGroups, ags...) rules = append(rules, controlplane.NetworkPolicyRule{ Direction: controlplane.DirectionIn, @@ -127,8 +128,10 @@ func (n *NetworkPolicyController) processAntreaNetworkPolicy(np *crdv1alpha1.Net peer = n.svcRefToPeerForCRD(egressRule.ToServices, np.Namespace) } else { var ags []*antreatypes.AddressGroup - peer, ags = n.toAntreaPeerForCRD(egressRule.To, np, controlplane.DirectionOut, namedPortExists, &clusterSetScopeSelectors) + var selKeys sets.String + peer, ags, selKeys = n.toAntreaPeerForCRD(egressRule.To, np, controlplane.DirectionOut, namedPortExists) addressGroups = mergeAddressGroups(addressGroups, ags...) + clusterSetScopeSelectorKeys = clusterSetScopeSelectorKeys.Union(selKeys) } rules = append(rules, controlplane.NetworkPolicyRule{ Direction: controlplane.DirectionOut, @@ -160,7 +163,7 @@ func (n *NetworkPolicyController) processAntreaNetworkPolicy(np *crdv1alpha1.Net AppliedToPerRule: appliedToPerRule, } if n.stretchNPEnabled { - n.labelIdentityInterface.RemoveStalePolicySelectors(clusterSetScopeSelectors, internalNetworkPolicyKeyFunc(np)) + n.labelIdentityInterface.RemoveStalePolicySelectors(clusterSetScopeSelectorKeys, internalNetworkPolicyKeyFunc(np)) } return internalNetworkPolicy, appliedToGroups, addressGroups } diff --git a/pkg/controller/networkpolicy/clusternetworkpolicy.go b/pkg/controller/networkpolicy/clusternetworkpolicy.go index 772f76f2148..40bfa2200d4 100644 --- a/pkg/controller/networkpolicy/clusternetworkpolicy.go +++ b/pkg/controller/networkpolicy/clusternetworkpolicy.go @@ -339,11 +339,11 @@ func (n *NetworkPolicyController) processClusterNetworkPolicy(cnp *crdv1alpha1.C var clusterAppliedToAffectedNS []string // atgForNamespace is the appliedToGroups split by Namespaces. var atgForNamespace []*antreatypes.AppliedToGroup - // clusterSetScopeSelectors keeps track of all the ClusterSet-scoped selectors (including per-Namespace - // selectors) of the policy. During policy peer processing, any ClusterSet-scoped selector will be - // registered with the labelIdentityInterface and added to this list. By the end of the function, this - // list will be used to remove any stale selector from the policy in the labelIdentityInterface. - var clusterSetScopeSelectors []*antreatypes.GroupSelector + // clusterSetScopeSelectorKeys keeps track of all the ClusterSet-scoped selector keys of the policy. + // During policy peer processing, any ClusterSet-scoped selector will be registered with the + // labelIdentityInterface and added to this set. By the end of the function, this set will + // be used to remove any stale selector from the policy in the labelIdentityInterface. + var clusterSetScopeSelectorKeys sets.String if hasPerNamespaceRule && len(cnp.Spec.AppliedTo) > 0 { for _, at := range cnp.Spec.AppliedTo { if at.ServiceAccount != nil { @@ -401,7 +401,8 @@ func (n *NetworkPolicyController) processClusterNetworkPolicy(cnp *crdv1alpha1.C if cnpRule.ToServices != nil { addRule(n.svcRefToPeerForCRD(cnpRule.ToServices, ""), nil, direction, ruleATGs) } else { - peer, ags := n.toAntreaPeerForCRD(clusterPeers, cnp, direction, namedPortExists, &clusterSetScopeSelectors) + peer, ags, selKeys := n.toAntreaPeerForCRD(clusterPeers, cnp, direction, namedPortExists) + clusterSetScopeSelectorKeys = clusterSetScopeSelectorKeys.Union(selKeys) addRule(peer, ags, direction, ruleATGs) } } @@ -410,7 +411,8 @@ func (n *NetworkPolicyController) processClusterNetworkPolicy(cnp *crdv1alpha1.C // Create a rule for each affected Namespace of appliedTo at spec level for i := range clusterAppliedToAffectedNS { klog.V(4).Infof("Adding a new per-namespace rule with appliedTo %v for rule %d of %s", clusterAppliedToAffectedNS[i], idx, cnp.Name) - peer, ags := n.toNamespacedPeerForCRD(perNSPeers, cnp, clusterAppliedToAffectedNS[i], &clusterSetScopeSelectors) + peer, ags, selKeys := n.toNamespacedPeerForCRD(perNSPeers, cnp, clusterAppliedToAffectedNS[i]) + clusterSetScopeSelectorKeys = clusterSetScopeSelectorKeys.Union(selKeys) addRule(peer, ags, direction, []*antreatypes.AppliedToGroup{atgForNamespace[i]}) } } else { @@ -419,14 +421,16 @@ func (n *NetworkPolicyController) processClusterNetworkPolicy(cnp *crdv1alpha1.C if at.ServiceAccount != nil { atg := n.createAppliedToGroup(at.ServiceAccount.Namespace, serviceAccountNameToPodSelector(at.ServiceAccount.Name), nil, nil) klog.V(4).Infof("Adding a new per-namespace rule with appliedTo %v for rule %d of %s", atg, idx, cnp.Name) - peer, ags := n.toNamespacedPeerForCRD(perNSPeers, cnp, at.ServiceAccount.Namespace, &clusterSetScopeSelectors) + peer, ags, selKeys := n.toNamespacedPeerForCRD(perNSPeers, cnp, at.ServiceAccount.Namespace) + clusterSetScopeSelectorKeys = clusterSetScopeSelectorKeys.Union(selKeys) addRule(peer, ags, direction, []*antreatypes.AppliedToGroup{atg}) } else { affectedNS := n.getAffectedNamespacesForAppliedTo(at) for _, ns := range affectedNS { atg := n.createAppliedToGroup(ns, at.PodSelector, nil, at.ExternalEntitySelector) klog.V(4).Infof("Adding a new per-namespace rule with appliedTo %v for rule %d of %s", atg, idx, cnp.Name) - peer, ags := n.toNamespacedPeerForCRD(perNSPeers, cnp, ns, &clusterSetScopeSelectors) + peer, ags, selKeys := n.toNamespacedPeerForCRD(perNSPeers, cnp, ns) + clusterSetScopeSelectorKeys = clusterSetScopeSelectorKeys.Union(selKeys) addRule(peer, ags, direction, []*antreatypes.AppliedToGroup{atg}) } } @@ -460,7 +464,7 @@ func (n *NetworkPolicyController) processClusterNetworkPolicy(cnp *crdv1alpha1.C AppliedToPerRule: appliedToPerRule, } if n.stretchNPEnabled { - n.labelIdentityInterface.RemoveStalePolicySelectors(clusterSetScopeSelectors, internalNetworkPolicyKeyFunc(cnp)) + n.labelIdentityInterface.RemoveStalePolicySelectors(clusterSetScopeSelectorKeys, internalNetworkPolicyKeyFunc(cnp)) } return internalNetworkPolicy, appliedToGroups, addressGroups } diff --git a/pkg/controller/networkpolicy/crd_utils.go b/pkg/controller/networkpolicy/crd_utils.go index 48abc29b1f6..f4f335d2ebb 100644 --- a/pkg/controller/networkpolicy/crd_utils.go +++ b/pkg/controller/networkpolicy/crd_utils.go @@ -15,6 +15,7 @@ package networkpolicy import ( + "k8s.io/apimachinery/pkg/util/sets" "strings" "time" @@ -142,8 +143,7 @@ func toAntreaIPBlockForCRD(ipBlock *v1alpha1.IPBlock) (*controlplane.IPBlock, er // Any ClusterSet scoped selector in this peer will also be registered with the labelIdentityInterface // for the policy. func (n *NetworkPolicyController) toAntreaPeerForCRD(peers []v1alpha1.NetworkPolicyPeer, - np metav1.Object, dir controlplane.Direction, namedPortExists bool, - clusterSetScopeSelectors *[]*antreatypes.GroupSelector) (*controlplane.NetworkPolicyPeer, []*antreatypes.AddressGroup) { + np metav1.Object, dir controlplane.Direction, namedPortExists bool) (*controlplane.NetworkPolicyPeer, []*antreatypes.AddressGroup, sets.String) { var addressGroups []*antreatypes.AddressGroup // NetworkPolicyPeer is supposed to match all addresses when it is empty and no clusterGroup is present. // It's treated as an IPBlock "0.0.0.0/0". @@ -155,18 +155,19 @@ func (n *NetworkPolicyController) toAntreaPeerForCRD(peers []v1alpha1.NetworkPol // For other cases it uses the IPBlock "0.0.0.0/0" to avoid the overhead // of handling member updates of the AddressGroup. if dir == controlplane.DirectionIn || !namedPortExists { - return &matchAllPeer, nil + return &matchAllPeer, nil, sets.NewString() } allPodsGroup := n.createAddressGroup("", matchAllPodsPeerCrd.PodSelector, matchAllPodsPeerCrd.NamespaceSelector, nil, nil) addressGroups = append(addressGroups, allPodsGroup) podsPeer := matchAllPeer podsPeer.AddressGroups = append(podsPeer.AddressGroups, allPodsGroup.Name) - return &podsPeer, addressGroups + return &podsPeer, addressGroups, sets.NewString() } var ipBlocks []controlplane.IPBlock var fqdns []string var labelIdentities []uint32 uniqueLabelIDs := map[uint32]struct{}{} + clusterSetScopeSelectorKeys := sets.NewString() for _, peer := range peers { // A v1alpha1.NetworkPolicyPeer will have exactly one of the following fields set: // - podSelector and/or namespaceSelector (in-cluster scope or ClusterSet scope) @@ -200,7 +201,7 @@ func (n *NetworkPolicyController) toAntreaPeerForCRD(peers []v1alpha1.NetworkPol } if n.stretchNPEnabled && peer.Scope == v1alpha1.ScopeClusterSet { newClusterSetScopeSelector := antreatypes.NewGroupSelector(np.GetNamespace(), peer.PodSelector, peer.NamespaceSelector, nil, nil) - *clusterSetScopeSelectors = append(*clusterSetScopeSelectors, newClusterSetScopeSelector) + clusterSetScopeSelectorKeys.Insert(newClusterSetScopeSelector.NormalizedName) // In addition to getting the matched Label Identity IDs, AddSelector also registers the selector // with the labelIdentityInterface. matchedLabelIDs := n.labelIdentityInterface.AddSelector(newClusterSetScopeSelector, internalNetworkPolicyKeyFunc(np)) @@ -217,23 +218,24 @@ func (n *NetworkPolicyController) toAntreaPeerForCRD(peers []v1alpha1.NetworkPol IPBlocks: ipBlocks, FQDNs: fqdns, LabelIdentities: labelIdentities, - }, addressGroups + }, addressGroups, clusterSetScopeSelectorKeys } // toNamespacedPeerForCRD creates an Antrea controlplane NetworkPolicyPeer for crdv1alpha1 NetworkPolicyPeer // for a particular Namespace. It is used when a single crdv1alpha1 NetworkPolicyPeer maps to multiple // controlplane NetworkPolicyPeers because the appliedTo workloads reside in different Namespaces. func (n *NetworkPolicyController) toNamespacedPeerForCRD(peers []v1alpha1.NetworkPolicyPeer, - np metav1.Object, namespace string, clusterSetScopeSelectors *[]*antreatypes.GroupSelector) (*controlplane.NetworkPolicyPeer, []*antreatypes.AddressGroup) { + np metav1.Object, namespace string) (*controlplane.NetworkPolicyPeer, []*antreatypes.AddressGroup, sets.String) { var addressGroups []*antreatypes.AddressGroup var labelIdentities []uint32 uniqueLabelIDs := map[uint32]struct{}{} + clusterSetScopeSelectorKeys := sets.NewString() for _, peer := range peers { addressGroup := n.createAddressGroup(namespace, peer.PodSelector, nil, peer.ExternalEntitySelector, nil) addressGroups = append(addressGroups, addressGroup) if n.stretchNPEnabled && peer.Scope == v1alpha1.ScopeClusterSet { newClusterSetScopeSelector := antreatypes.NewGroupSelector(namespace, peer.PodSelector, nil, peer.ExternalEntitySelector, nil) - *clusterSetScopeSelectors = append(*clusterSetScopeSelectors, newClusterSetScopeSelector) + clusterSetScopeSelectorKeys.Insert(newClusterSetScopeSelector.NormalizedName) // In addition to getting the matched Label Identity IDs, AddSelector also registers the selector // with the labelIdentityInterface. matchedLabelIDs := n.labelIdentityInterface.AddSelector(newClusterSetScopeSelector, internalNetworkPolicyKeyFunc(np)) @@ -247,7 +249,7 @@ func (n *NetworkPolicyController) toNamespacedPeerForCRD(peers []v1alpha1.Networ } return &controlplane.NetworkPolicyPeer{ AddressGroups: getAddressGroupNames(addressGroups), LabelIdentities: labelIdentities, - }, addressGroups + }, addressGroups, clusterSetScopeSelectorKeys } // svcRefToPeerForCRD creates an Antrea controlplane NetworkPolicyPeer from ServiceReferences in ToServices diff --git a/pkg/controller/networkpolicy/crd_utils_test.go b/pkg/controller/networkpolicy/crd_utils_test.go index 29a274afea1..9391e899c69 100644 --- a/pkg/controller/networkpolicy/crd_utils_test.go +++ b/pkg/controller/networkpolicy/crd_utils_test.go @@ -475,8 +475,7 @@ func TestToAntreaPeerForCRD(t *testing.T) { npc.labelIdentityInterface.AddLabelIdentity(labelIdentityA, 1) npc.labelIdentityInterface.AddLabelIdentity(labelIdentityB, 2) } - var clusterSetScopeSelectors []*antreatypes.GroupSelector - actualPeer, _ := npc.toAntreaPeerForCRD(tt.inPeers, testCNPObj, tt.direction, tt.namedPortExists, &clusterSetScopeSelectors) + actualPeer, _, _ := npc.toAntreaPeerForCRD(tt.inPeers, testCNPObj, tt.direction, tt.namedPortExists) if !reflect.DeepEqual(tt.outPeer.AddressGroups, actualPeer.AddressGroups) { t.Errorf("Unexpected AddressGroups in Antrea Peer conversion. Expected %v, got %v", tt.outPeer.AddressGroups, actualPeer.AddressGroups) } diff --git a/test/e2e/k8s_util.go b/test/e2e/k8s_util.go index 5f8a089365d..90732dfcf28 100644 --- a/test/e2e/k8s_util.go +++ b/test/e2e/k8s_util.go @@ -980,7 +980,7 @@ func (data *TestData) CleanANPs(namespaces []string) error { return nil } -func (data *TestData) WaitForACNPCreatiionAndRealization(t *testing.T, name string, timeout time.Duration) error { +func (data *TestData) WaitForACNPCreationAndRealization(t *testing.T, name string, timeout time.Duration) error { t.Logf("Waiting for ACNP '%s' to be created and realized", name) if err := wait.Poll(100*time.Millisecond, timeout, func() (bool, error) { acnp, err := data.crdClient.CrdV1alpha1().ClusterNetworkPolicies().Get(context.TODO(), name, metav1.GetOptions{})