api: add PoolName and NodeGroupStatus

api/numaresourcesoperator/v1/helper/nodegroup/nodegroup.go

@@ -59,31 +59,42 @@ func (ttr Tree) Clone() Tree {
 // One of the key design assumptions in NROP is the 1:1 mapping between NodeGroups and MCPs.
 // This historical accident should be fixed in future versions.
 func FindTrees(mcps *mcov1.MachineConfigPoolList, nodeGroups []nropv1.NodeGroup) ([]Tree, error) {
+	// node groups are validated by the controller before getting to this phase, so for sure all node groups will be valid at this point.
+	// a valid node group has either PoolName OR MachineConfigPoolSelector, not both. Getting here means operator is deployed on Openshift thus processing MCPs
 	var result []Tree
 	for idx := range nodeGroups {
 		nodeGroup := &nodeGroups[idx] // shortcut
+		treeMCPs := []*mcov1.MachineConfigPool{}
 
-		if nodeGroup.MachineConfigPoolSelector == nil {
-			continue
-		}
-		selector, err := metav1.LabelSelectorAsSelector(nodeGroup.MachineConfigPoolSelector)
-		if err != nil {
-			klog.Errorf("bad node group machine config pool selector %q", nodeGroup.MachineConfigPoolSelector.String())
-			continue
+		if nodeGroup.PoolName != nil {
+			for i := range mcps.Items {
+				mcp := &mcps.Items[i]
+				if mcp.Name == *nodeGroup.PoolName {
+					treeMCPs = append(treeMCPs, mcp)
+					// MCO ensures there are no mcp name duplications
+					break
+				}
+			}
 		}
 
-		treeMCPs := []*mcov1.MachineConfigPool{}
-		for i := range mcps.Items {
-			mcp := &mcps.Items[i] // shortcut
-			mcpLabels := labels.Set(mcp.Labels)
-			if !selector.Matches(mcpLabels) {
+		if nodeGroup.MachineConfigPoolSelector != nil {
+			selector, err := metav1.LabelSelectorAsSelector(nodeGroup.MachineConfigPoolSelector)
+			if err != nil {
+				klog.Errorf("bad node group machine config pool selector %q", nodeGroup.MachineConfigPoolSelector.String())
 				continue
 			}
-			treeMCPs = append(treeMCPs, mcp)
-		}
 
+			for i := range mcps.Items {
+				mcp := &mcps.Items[i] // shortcut
+				mcpLabels := labels.Set(mcp.Labels)
+				if !selector.Matches(mcpLabels) {
+					continue
+				}
+				treeMCPs = append(treeMCPs, mcp)
+			}
+		}
 		if len(treeMCPs) == 0 {
-			return nil, fmt.Errorf("failed to find MachineConfigPool for the node group with the selector %q", nodeGroup.MachineConfigPoolSelector.String())
+			return nil, fmt.Errorf("failed to find MachineConfigPool for the node group %+v", nodeGroup)
 		}
 
 		result = append(result, Tree{

api/numaresourcesoperator/v1/helper/nodegroup/nodegroup_test.go

@@ -23,6 +23,7 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/klog/v2"
 
 	mcov1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
@@ -185,6 +186,47 @@ func TestFindTrees(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "node group with PoolName and MachineConfigPoolSelector in another node group",
+			mcps: &mcpList,
+			ngs: []nropv1.NodeGroup{
+				{
+					PoolName: &mcpList.Items[0].Name,
+				},
+				{
+					MachineConfigPoolSelector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"mcp-label-3": "test3",
+						},
+					},
+				},
+			},
+			expected: []Tree{
+				{
+					MachineConfigPools: []*mcov1.MachineConfigPool{
+						{
+							ObjectMeta: metav1.ObjectMeta{
+								Name: "mcp1",
+							},
+						},
+					},
+				},
+				{
+					MachineConfigPools: []*mcov1.MachineConfigPool{
+						{
+							ObjectMeta: metav1.ObjectMeta{
+								Name: "mcp3",
+							},
+						},
+						{
+							ObjectMeta: metav1.ObjectMeta{
+								Name: "mcp5",
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 	for _, tt := range testCases {
 		t.Run(tt.name, func(t *testing.T) {
@@ -203,9 +245,10 @@ func TestFindTrees(t *testing.T) {
 			if err != nil {
 				t.Errorf("unexpected error checking backward compat: %v", err)
 			}
-			compatNames := mcpNamesFromList(gotMcps)
-			if !reflect.DeepEqual(gotNames, compatNames) {
-				t.Errorf("Trees mismatch (non backward compatible): got=%v compat=%v", gotNames, compatNames)
+			compatibleNames := mcpNamesFromList(gotMcps)
+			gotSet := sets.New[string](gotNames...)
+			if !gotSet.HasAll(compatibleNames...) {
+				t.Errorf("Trees mismatch (non backward compatible): got=%v compat=%v", gotNames, compatibleNames)
 			}
 		})
 	}

api/numaresourcesoperator/v1/numaresourcesoperator_types.go

@@ -112,14 +112,37 @@ type NodeGroupConfig struct {
 }
 
 // NodeGroup defines group of nodes that will run resource topology exporter daemon set
-// You can choose the group of node by MachineConfigPoolSelector or by NodeSelector
+// You can choose the group of node by MachineConfigPoolSelector or by PoolName
 type NodeGroup struct {
 	// MachineConfigPoolSelector defines label selector for the machine config pool
 	// +optional
 	MachineConfigPoolSelector *metav1.LabelSelector `json:"machineConfigPoolSelector,omitempty"`
 	// Config defines the RTE behavior for this NodeGroup
 	// +optional
 	Config *NodeGroupConfig `json:"config,omitempty"`
+	// PoolName defines the pool name to which the nodes belong that the config of this node group will be applied to
+	// +optional
+	PoolName *string `json:"poolName,omitempty"`
+}
+
+// NodeGroupStatus reports the status of a NodeGroup once matches an actual set of nodes and it is correctly processed
+// by the system. In other words, is not possible to have a NodeGroupStatus which does not represent a valid NodeGroup
+// which in turn correctly references unambiguously a set of nodes in the cluster.
+// Hence, if a NodeGroupStatus is published, its `Name` must be present, because it refers back to a NodeGroup whose
+// config was correctly processed in the Spec. And its DaemonSet will be nonempty, because matches correctly a set
+// of nodes in the cluster. The Config is best-effort always represented, possibly reflecting the system defaults.
+// If the system cannot process a NodeGroup correctly from the Spec, it will report Degraded state in the top-level
+// condition, and will provide details using the aforementioned conditions.
+type NodeGroupStatus struct {
+	// DaemonSet of the configured RTEs, for this node group
+	//+operator-sdk:csv:customresourcedefinitions:type=status,displayName="RTE DaemonSets"
+	DaemonSet NamespacedName `json:"daemonsets"`
+	// NodeGroupConfig represents the latest available configuration applied to this NodeGroup
+	//+operator-sdk:csv:customresourcedefinitions:type=status,displayName="Optional configuration enforced on this NodeGroup"
+	Config NodeGroupConfig `json:"config"`
+	// PoolName represents the pool name to which the nodes belong that the config of this node group is be applied to
+	//+operator-sdk:csv:customresourcedefinitions:type=status,displayName="Pool name of nodes in this node group"
+	PoolName string `json:"selector"`
 }
 
 // NUMAResourcesOperatorStatus defines the observed state of NUMAResourcesOperator
@@ -130,6 +153,10 @@ type NUMAResourcesOperatorStatus struct {
 	// MachineConfigPools resolved from configured node groups
 	//+operator-sdk:csv:customresourcedefinitions:type=status,displayName="RTE MCPs from node groups"
 	MachineConfigPools []MachineConfigPool `json:"machineconfigpools,omitempty"`
+	// NodeGroups report the observed status of the configured NodeGroups, matching by their name
+	// +optional
+	//+operator-sdk:csv:customresourcedefinitions:type=status,displayName="Node groups observed status"
+	NodeGroups []NodeGroupStatus `json:"nodeGroups,omitempty"`
 	// Conditions show the current state of the NUMAResourcesOperator Operator
 	//+operator-sdk:csv:customresourcedefinitions:type=status,displayName="Condition reported"
 	Conditions []metav1.Condition `json:"conditions,omitempty"`

bundle/manifests/nodetopology.openshift.io_numaresourcesoperators.yaml

@@ -62,7 +62,7 @@ spec:
                 items:
                   description: |-
                     NodeGroup defines group of nodes that will run resource topology exporter daemon set
-                    You can choose the group of node by MachineConfigPoolSelector or by NodeSelector
+                    You can choose the group of node by MachineConfigPoolSelector or by PoolName
                   properties:
                     config:
                       description: Config defines the RTE behavior for this NodeGroup
@@ -182,6 +182,11 @@ spec:
                           type: object
                       type: object
                       x-kubernetes-map-type: atomic
+                    poolName:
+                      description: PoolName defines the pool name to which the nodes
+                        belong that the config of this node group will be applied
+                        to
+                      type: string
                   type: object
                 type: array
               podExcludes:
@@ -401,6 +406,114 @@ spec:
                   - name
                   type: object
                 type: array
+              nodeGroups:
+                description: NodeGroups report the observed status of the configured
+                  NodeGroups, matching by their name
+                items:
+                  description: |-
+                    NodeGroupStatus reports the status of a NodeGroup once matches an actual set of nodes and it is correctly processed
+                    by the system. In other words, is not possible to have a NodeGroupStatus which does not represent a valid NodeGroup
+                    which in turn correctly references unambiguously a set of nodes in the cluster.
+                    Hence, if a NodeGroupStatus is published, its `Name` must be present, because it refers back to a NodeGroup whose
+                    config was correctly processed in the Spec. And its DaemonSet will be nonempty, because matches correctly a set
+                    of nodes in the cluster. The Config is best-effort always represented, possibly reflecting the system defaults.
+                    If the system cannot process a NodeGroup correctly from the Spec, it will report Degraded state in the top-level
+                    condition, and will provide details using the aforementioned conditions.
+                  properties:
+                    config:
+                      description: NodeGroupConfig represents the latest available
+                        configuration applied to this NodeGroup
+                      properties:
+                        infoRefreshMode:
+                          description: InfoRefreshMode sets the mechanism which will
+                            be used to refresh the topology info.
+                          enum:
+                          - Periodic
+                          - Events
+                          - PeriodicAndEvents
+                          type: string
+                        infoRefreshPause:
+                          description: InfoRefreshPause defines if updates to NRTs
+                            are paused for the machines belonging to this group
+                          enum:
+                          - Disabled
+                          - Enabled
+                          type: string
+                        infoRefreshPeriod:
+                          description: InfoRefreshPeriod sets the topology info refresh
+                            period. Use explicit 0 to disable.
+                          type: string
+                        podsFingerprinting:
+                          description: PodsFingerprinting defines if pod fingerprint
+                            should be reported for the machines belonging to this
+                            group
+                          enum:
+                          - Disabled
+                          - Enabled
+                          - EnabledExclusiveResources
+                          type: string
+                        tolerations:
+                          description: |-
+                            Tolerations overrides tolerations to be set into RTE daemonsets for this NodeGroup. If not empty, the tolerations will be the one set here.
+                            Leave empty to make the system use the default tolerations.
+                          items:
+                            description: |-
+                              The pod this Toleration is attached to tolerates any taint that matches
+                              the triple <key,value,effect> using the matching operator <operator>.
+                            properties:
+                              effect:
+                                description: |-
+                                  Effect indicates the taint effect to match. Empty means match all taint effects.
+                                  When specified, allowed values are NoSchedule, PreferNoSchedule and NoExecute.
+                                type: string
+                              key:
+                                description: |-
+                                  Key is the taint key that the toleration applies to. Empty means match all taint keys.
+                                  If the key is empty, operator must be Exists; this combination means to match all values and all keys.
+                                type: string
+                              operator:
+                                description: |-
+                                  Operator represents a key's relationship to the value.
+                                  Valid operators are Exists and Equal. Defaults to Equal.
+                                  Exists is equivalent to wildcard for value, so that a pod can
+                                  tolerate all taints of a particular category.
+                                type: string
+                              tolerationSeconds:
+                                description: |-
+                                  TolerationSeconds represents the period of time the toleration (which must be
+                                  of effect NoExecute, otherwise this field is ignored) tolerates the taint. By default,
+                                  it is not set, which means tolerate the taint forever (do not evict). Zero and
+                                  negative values will be treated as 0 (evict immediately) by the system.
+                                format: int64
+                                type: integer
+                              value:
+                                description: |-
+                                  Value is the taint value the toleration matches to.
+                                  If the operator is Exists, the value should be empty, otherwise just a regular string.
+                                type: string
+                            type: object
+                          type: array
+                      type: object
+                    daemonsets:
+                      description: DaemonSet of the configured RTEs, for this node
+                        group
+                      properties:
+                        name:
+                          type: string
+                        namespace:
+                          type: string
+                      type: object
+                    selector:
+                      description: PoolName represents the pool name to which the
+                        nodes belong that the config of this node group is be applied
+                        to
+                      type: string
+                  required:
+                  - config
+                  - daemonsets
+                  - selector
+                  type: object
+                type: array
               relatedObjects:
                 description: RelatedObjects list of objects of interest for this operator
                 items:

bundle/manifests/numaresources-operator.clusterserviceversion.yaml

@@ -62,7 +62,7 @@ metadata:
         }
       ]
     capabilities: Basic Install
-    createdAt: "2024-10-14T13:06:40Z"
+    createdAt: "2024-10-16T11:10:10Z"
     olm.skipRange: '>=4.17.0 <4.18.0'
     operators.operatorframework.io/builder: operator-sdk-v1.36.1
     operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
@@ -149,6 +149,21 @@ spec:
           applied to this MachineConfigPool
         displayName: Optional configuration enforced on this NodeGroup
         path: machineconfigpools[0].config
+      - description: NodeGroups report the observed status of the configured NodeGroups,
+          matching by their name
+        displayName: Node groups observed status
+        path: nodeGroups
+      - description: NodeGroupConfig represents the latest available configuration
+          applied to this NodeGroup
+        displayName: Optional configuration enforced on this NodeGroup
+        path: nodeGroups[0].config
+      - description: DaemonSet of the configured RTEs, for this node group
+        displayName: RTE DaemonSets
+        path: nodeGroups[0].daemonsets
+      - description: PoolName represents the pool name to which the nodes belong that
+          the config of this node group is be applied to
+        displayName: Pool name of nodes in this node group
+        path: nodeGroups[0].selector
       - description: RelatedObjects list of objects of interest for this operator
         displayName: Related Objects
         path: relatedObjects