Use format "namespace/name" as the key for ExternalNode span calculat…

…ion (#4401)

1. Use the new key format in ANP span calculation
2. Use the new key format in SupportBundleCollection span
3. Antrea Agent uses the new format as the filter when watching internal resources.

Signed-off-by: wenyingd <wenyingd@vmware.com>

cmd/antrea-agent/agent.go

@@ -362,11 +362,15 @@ func run(o *Options) error {
 		tunPort = nodeConfig.TunnelOFPort
 	}
 
+	nodeKey := nodeConfig.Name
+	if o.nodeType == config.ExternalNode {
+		nodeKey = k8s.NamespacedName(o.config.ExternalNode.ExternalNodeNamespace, nodeKey)
+	}
 	networkPolicyController, err := networkpolicy.NewNetworkPolicyController(
 		antreaClientProvider,
 		ofClient,
 		ifaceStore,
-		nodeConfig.Name,
+		nodeKey,
 		podUpdateChannel,
 		externalEntityUpdateChannel,
 		groupCounters,

pkg/controller/externalnode/controller.go

@@ -362,7 +362,7 @@ func (c *ExternalNodeController) deleteExternalEntity(namespace string, name str
 func genExternalEntity(eeName string, en *v1alpha1.ExternalNode) (*v1alpha2.ExternalEntity, error) {
 	ownerRef := &metav1.OwnerReference{
 		APIVersion: "crd.antrea.io/v1alpha1",
-		Kind:       "ExternalNode",
+		Kind:       externalnode.EntityOwnerKind,
 		Name:       en.GetName(),
 		UID:        en.GetUID(),
 	}

pkg/controller/networkpolicy/networkpolicy_controller.go

@@ -59,6 +59,7 @@ import (
 	"antrea.io/antrea/pkg/controller/networkpolicy/store"
 	antreatypes "antrea.io/antrea/pkg/controller/types"
 	"antrea.io/antrea/pkg/features"
+	"antrea.io/antrea/pkg/util/externalnode"
 	"antrea.io/antrea/pkg/util/k8s"
 	utilsets "antrea.io/antrea/pkg/util/sets"
 )
@@ -1283,17 +1284,18 @@ func (n *NetworkPolicyController) syncAppliedToGroup(key string) error {
 				appGroupNodeNames.Insert(pod.Spec.NodeName)
 			}
 			for _, extEntity := range externalEntities {
-				if extEntity.Spec.ExternalNode == "" {
+				entityNodeKey := externalnode.GenerateEntityNodeKey(extEntity)
+				if entityNodeKey == "" {
 					continue
 				}
 				scheduledExtEntityNum++
-				entitySet := memberSetByNode[extEntity.Spec.ExternalNode]
+				entitySet := memberSetByNode[entityNodeKey]
 				if entitySet == nil {
 					entitySet = controlplane.GroupMemberSet{}
 				}
 				entitySet.Insert(externalEntityToGroupMember(extEntity, false))
-				memberSetByNode[extEntity.Spec.ExternalNode] = entitySet
-				appGroupNodeNames.Insert(extEntity.Spec.ExternalNode)
+				memberSetByNode[entityNodeKey] = entitySet
+				appGroupNodeNames.Insert(entityNodeKey)
 			}
 			updatedAppliedToGroup = &antreatypes.AppliedToGroup{
 				UID:               appliedToGroup.UID,

pkg/controller/networkpolicy/networkpolicy_controller_test.go

@@ -31,6 +31,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/apimachinery/pkg/util/rand"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -54,6 +55,7 @@ import (
 	"antrea.io/antrea/pkg/controller/labelidentity"
 	"antrea.io/antrea/pkg/controller/networkpolicy/store"
 	antreatypes "antrea.io/antrea/pkg/controller/types"
+	"antrea.io/antrea/pkg/util/externalnode"
 )
 
 var alwaysReady = func() bool { return true }
@@ -3393,6 +3395,107 @@ func TestSyncInternalNetworkPolicyWithGroups(t *testing.T) {
 	}
 }
 
+func TestSyncAppliedToGroupWithExternalEntity(t *testing.T) {
+	selectorSpec := metav1.LabelSelector{
+		MatchLabels: map[string]string{"group": "appliedTo"},
+	}
+	tests := []struct {
+		name                string
+		addedExternalEntity *v1alpha2.ExternalEntity
+		entityNodeKey       string
+		addedInSpan         bool
+	}{
+		{
+			name: "match-external-entity-created-by-external-node",
+			addedExternalEntity: &v1alpha2.ExternalEntity{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "entityA",
+					Namespace: "nsA",
+					Labels:    map[string]string{"group": "appliedTo"},
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							APIVersion: "crd.antrea.io/v1alpha1",
+							Kind:       externalnode.EntityOwnerKind,
+							Name:       "nodeA",
+						},
+					},
+				},
+				Spec: v1alpha2.ExternalEntitySpec{
+					ExternalNode: "nodeA",
+				},
+			},
+			entityNodeKey: "nsA/nodeA",
+			addedInSpan:   true,
+		},
+		{
+			name: "match-external-entity-created-by-other-modules",
+			addedExternalEntity: &v1alpha2.ExternalEntity{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "entityB",
+					Namespace: "nsA",
+					Labels:    map[string]string{"group": "appliedTo"},
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							APIVersion: "crd.antrea.io/v1alpha1",
+							Kind:       "external-modules",
+							Name:       "nodeB",
+						},
+					},
+				},
+				Spec: v1alpha2.ExternalEntitySpec{
+					ExternalNode: "nodeB",
+				},
+			},
+			entityNodeKey: "nodeB",
+			addedInSpan:   true,
+		},
+		{
+			name: "match-external-entity-not-set-external-node",
+			addedExternalEntity: &v1alpha2.ExternalEntity{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "entityA",
+					Namespace: "nsA",
+					Labels:    map[string]string{"group": "appliedTo"},
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							APIVersion: "crd.antrea.io/v1alpha1",
+							Kind:       "external-modules",
+							Name:       "nodeB",
+						},
+					},
+				},
+				Spec: v1alpha2.ExternalEntitySpec{},
+			},
+			entityNodeKey: "nodeB",
+			addedInSpan:   false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, npc := newController()
+			npc.groupingInterface.AddExternalEntity(tt.addedExternalEntity)
+			groupSelector := antreatypes.NewGroupSelector("nsA", nil, nil, &selectorSpec, nil)
+			appGroupID := getNormalizedUID(groupSelector.NormalizedName)
+			appliedToGroup := &antreatypes.AppliedToGroup{
+				Name:     appGroupID,
+				UID:      types.UID(appGroupID),
+				Selector: groupSelector,
+			}
+			npc.appliedToGroupStore.Create(appliedToGroup)
+			npc.groupingInterface.AddGroup(appliedToGroupType, appliedToGroup.Name, appliedToGroup.Selector)
+			npc.syncAppliedToGroup(appGroupID)
+			appGroupObj, _, _ := npc.appliedToGroupStore.Get(appGroupID)
+			appGroup := appGroupObj.(*antreatypes.AppliedToGroup)
+			entitiesAdded := appGroup.GroupMemberByNode[tt.entityNodeKey]
+			if tt.addedInSpan {
+				assert.Equal(t, 1, len(entitiesAdded), "expected Entity Node to add into AppliedToGroup span")
+			} else {
+				assert.Equal(t, 0, len(entitiesAdded), "expected Entity Node not to add into AppliedToGroup span")
+			}
+		})
+	}
+}
+
 func checkQueueItemExistence(t *testing.T, queue workqueue.RateLimitingInterface, items ...string) {
 	require.Equal(t, len(items), queue.Len())
 	expectedItems := sets.NewString(items...)

pkg/controller/supportbundlecollection/controller.go

@@ -45,6 +45,7 @@ import (
 	crdinformers "antrea.io/antrea/pkg/client/informers/externalversions/crd/v1alpha1"
 	crdlisters "antrea.io/antrea/pkg/client/listers/crd/v1alpha1"
 	"antrea.io/antrea/pkg/controller/types"
+	"antrea.io/antrea/pkg/util/k8s"
 )
 
 const (
@@ -461,7 +462,8 @@ func (c *Controller) getBundleExternalNodes(en *v1alpha1.BundleExternalNodes) (s
 		}
 		enNames := sets.NewString()
 		for _, n := range allExternalNodes {
-			enNames.Insert(n.Name)
+			enNameKey := k8s.NamespacedName(n.Namespace, n.Name)
+			enNames.Insert(enNameKey)
 		}
 		return enNames, nil
 	}
@@ -475,7 +477,7 @@ func (c *Controller) getBundleExternalNodes(en *v1alpha1.BundleExternalNodes) (s
 			}
 			return nil, fmt.Errorf("unable to get existing ExternalNode %s/%s: %v", en.Namespace, name, err)
 		}
-		enNames.Insert(name)
+		enNames.Insert(k8s.NamespacedName(en.Namespace, name))
 	}
 	if en.NodeSelector != nil {
 		nodeSelector, _ := metav1.LabelSelectorAsSelector(en.NodeSelector)
@@ -484,7 +486,7 @@ func (c *Controller) getBundleExternalNodes(en *v1alpha1.BundleExternalNodes) (s
 			return nil, fmt.Errorf("failed to list ExternalNodes with labels %s in namespace %s: %v", nodeSelector.String(), en.Namespace, err)
 		}
 		for _, n := range selectedNodes {
-			enNames.Insert(n.Name)
+			enNames.Insert(k8s.NamespacedName(n.Namespace, n.Name))
 		}
 	}
 	return enNames, nil
@@ -920,8 +922,9 @@ func mergeConditions(oldConditions, newConditions []v1alpha1.SupportBundleCollec
 }
 
 func getNodeKey(status *controlplane.SupportBundleCollectionNodeStatus) string {
-	// TODO: use NodeNamespace/NodeName for ExternalNode after the Namespace is added in the connection key between
-	// antrea-agent and antrea-controller.
+	if status.NodeType == controlplane.SupportBundleCollectionNodeTypeExternalNode {
+		return k8s.NamespacedName(status.NodeNamespace, status.NodeName)
+	}
 	return status.NodeName
 }
 

pkg/controller/supportbundlecollection/controller_test.go

@@ -43,6 +43,7 @@ import (
 	crdinformers "antrea.io/antrea/pkg/client/informers/externalversions"
 	bundlecollectionstore "antrea.io/antrea/pkg/controller/supportbundlecollection/store"
 	"antrea.io/antrea/pkg/controller/types"
+	"antrea.io/antrea/pkg/util/k8s"
 )
 
 const (
@@ -621,7 +622,7 @@ func TestGetBundleExternalNodes(t *testing.T) {
 				Namespace: "ns1",
 				NodeNames: []string{"n1", "n2"},
 			},
-			expectedNodes: sets.NewString("n1", "n2"),
+			expectedNodes: sets.NewString("ns1/n1", "ns1/n2"),
 		}, {
 			bundleNodes: &v1alpha1.BundleExternalNodes{
 				Namespace: "ns1",
@@ -630,12 +631,12 @@ func TestGetBundleExternalNodes(t *testing.T) {
 					MatchLabels: map[string]string{"test": "selected"},
 				},
 			},
-			expectedNodes: sets.NewString("n1", "n3"),
+			expectedNodes: sets.NewString("ns1/n1", "ns1/n3"),
 		}, {
 			bundleNodes: &v1alpha1.BundleExternalNodes{
 				Namespace: "ns1",
 			},
-			expectedNodes: sets.NewString("n1", "n2", "n3", "n4"),
+			expectedNodes: sets.NewString("ns1/n1", "ns1/n2", "ns1/n3", "ns1/n4"),
 		}, {
 			bundleNodes:   nil,
 			expectedNodes: sets.NewString(),
@@ -806,7 +807,7 @@ func TestCreateAndDeleteInternalSupportBundleCollection(t *testing.T) {
 				},
 				authType: v1alpha1.APIKey,
 			},
-			expectedNodes: sets.NewString("en1", "en2", "en3"),
+			expectedNodes: sets.NewString("ns1/en1", "ns1/en2", "ns1/en3"),
 			expectedAuth: controlplane.BundleServerAuthConfiguration{
 				APIKey: testKeyString,
 			},
@@ -822,7 +823,7 @@ func TestCreateAndDeleteInternalSupportBundleCollection(t *testing.T) {
 				},
 				authType: v1alpha1.APIKey,
 			},
-			expectedNodes: sets.NewString("n1", "n2", "en5"),
+			expectedNodes: sets.NewString("n1", "n2", "ns2/en5"),
 			expectedAuth: controlplane.BundleServerAuthConfiguration{
 				APIKey: testKeyString,
 			},
@@ -1603,10 +1604,15 @@ func TestUpdateSupportBundleCollectionStatus(t *testing.T) {
 
 func TestUpdateStatus(t *testing.T) {
 	var controller *Controller
+	namespace := "ns1"
 	getSpan := func(nodesCount int) []string {
 		nodes := make([]string, nodesCount)
 		for i := 0; i < nodesCount; i++ {
-			nodes[i] = fmt.Sprintf("n%d", i)
+			nodeKey := fmt.Sprintf("n%d", i)
+			if i%2 == 0 {
+				nodeKey = k8s.NamespacedName(namespace, nodeKey)
+			}
+			nodes[i] = nodeKey
 		}
 		return nodes
 	}
@@ -1662,7 +1668,6 @@ func TestUpdateStatus(t *testing.T) {
 		assert.NoError(t, err)
 	}
 
-	namespace := "ns1"
 	agentReportStatus := func(nodesCount, failedNodes int, collectionName string) {
 		for i := 0; i < nodesCount; i++ {
 			nodeName := fmt.Sprintf("n%d", i)
@@ -1740,7 +1745,7 @@ func TestUpdateStatus(t *testing.T) {
 			Type:    v1alpha1.CollectionFailure,
 			Status:  metav1.ConditionTrue,
 			Reason:  string(metav1.StatusReasonInternalError),
-			Message: fmt.Sprintf(`Failed Agent count: 2, "unknown error":[n0, n1]`),
+			Message: fmt.Sprintf(`Failed Agent count: 2, "unknown error":[n1, ns1/n0]`),
 		}
 		assert.True(t, conditionExistsIgnoreLastTransitionTime(bundleCollection.Status.Conditions, failureStatus))
 		// Test merging failure message.
@@ -1752,7 +1757,7 @@ func TestUpdateStatus(t *testing.T) {
 			Type:    v1alpha1.CollectionFailure,
 			Status:  metav1.ConditionTrue,
 			Reason:  string(metav1.StatusReasonInternalError),
-			Message: fmt.Sprintf(`Failed Agent count: 3, "agent internal error":[n5], "unknown error":[n0, n1]`),
+			Message: fmt.Sprintf(`Failed Agent count: 3, "agent internal error":[n5], "unknown error":[n1, ns1/n0]`),
 		}))
 		assert.False(t, conditionExistsIgnoreLastTransitionTime(bundleCollection.Status.Conditions, failureStatus))
 		assert.True(t, conditionExistsIgnoreLastTransitionTime(bundleCollection.Status.Conditions, v1alpha1.SupportBundleCollectionCondition{

pkg/util/externalnode/externalnode.go

@@ -21,9 +21,15 @@ import (
 	"io"
 
 	"antrea.io/antrea/pkg/apis/crd/v1alpha1"
+	"antrea.io/antrea/pkg/apis/crd/v1alpha2"
+	"antrea.io/antrea/pkg/util/k8s"
 )
 
-const interfaceNameLength = 5
+const (
+	EntityOwnerKind = "ExternalNode"
+
+	interfaceNameLength = 5
+)
 
 func GenExternalEntityName(externalNode *v1alpha1.ExternalNode) (string, error) {
 	if len(externalNode.Spec.Interfaces) == 0 {
@@ -42,3 +48,18 @@ func GenExternalEntityName(externalNode *v1alpha1.ExternalNode) (string, error)
 		return externalNode.Name + "-" + hashedIfName[:interfaceNameLength], nil
 	}
 }
+
+func GenerateEntityNodeKey(externalEntity *v1alpha2.ExternalEntity) string {
+	if externalEntity.Spec.ExternalNode == "" {
+		return ""
+	}
+	entityNodeKey := externalEntity.Spec.ExternalNode
+	if len(externalEntity.OwnerReferences) == 0 {
+		return entityNodeKey
+	}
+	ownerRef := externalEntity.OwnerReferences[0]
+	if ownerRef.Kind == EntityOwnerKind {
+		entityNodeKey = k8s.NamespacedName(externalEntity.Namespace, entityNodeKey)
+	}
+	return entityNodeKey
+}