fix: Address review comments

Signed-off-by: Tsubasa Watanabe <w.tsubasa@fujitsu.com>

Author: ttsuuubasa

api/nvidia.com/resource/v1beta1/computedomain.go

@@ -28,8 +28,8 @@ const (
 	ComputeDomainChannelAllocationModeSingle = "Single"
 	ComputeDomainChannelAllocationModeAll    = "All"
 
-	ComputeDomainBindingConditions        = "IMEXDaemonSettingsDone"
-	ComputeDomainBindingFailureConditions = "IMEXDaemonSettingsFailed"
+	ComputeDomainBindingConditions        = "ComputeDomainReady"
+	ComputeDomainBindingFailureConditions = "ComputeDomainNotReady"
 )
 
 // +genclient

cmd/compute-domain-controller/computedomain.go

@@ -28,6 +28,7 @@ import (
 	"k8s.io/klog/v2"
 
 	nvapi "github.com/NVIDIA/k8s-dra-driver-gpu/api/nvidia.com/resource/v1beta1"
+	"github.com/NVIDIA/k8s-dra-driver-gpu/pkg/featuregates"
 	nvinformers "github.com/NVIDIA/k8s-dra-driver-gpu/pkg/nvidia.com/informers/externalversions"
 	nvlisters "github.com/NVIDIA/k8s-dra-driver-gpu/pkg/nvidia.com/listers/resource/v1beta1"
 )
@@ -160,8 +161,10 @@ func (m *ComputeDomainManager) Start(ctx context.Context) (rerr error) {
 		return fmt.Errorf("error starting Node manager: %w", err)
 	}
 
-	if err := m.resourceClaimManager.Start(ctx); err != nil {
-		return fmt.Errorf("error starting ResourceClaim manager: %w", err)
+	if featuregates.Enabled(featuregates.ComputeDomainBindingConditions) {
+		if err := m.resourceClaimManager.Start(ctx); err != nil {
+			return fmt.Errorf("error starting ResourceClaim manager: %w", err)
+		}
 	}
 
 	return nil
@@ -177,8 +180,10 @@ func (m *ComputeDomainManager) Stop() error {
 	if err := m.nodeManager.Stop(); err != nil {
 		return fmt.Errorf("error stopping Node manager: %w", err)
 	}
-	if err := m.resourceClaimManager.Stop(); err != nil {
-		return fmt.Errorf("error stopping ResourceClaim manager: %w", err)
+	if featuregates.Enabled(featuregates.ComputeDomainBindingConditions) {
+		if err := m.resourceClaimManager.Stop(); err != nil {
+			return fmt.Errorf("error stopping ResourceClaim manager: %w", err)
+		}
 	}
 	if m.cancelContext != nil {
 		m.cancelContext()

cmd/compute-domain-controller/daemonsetpods.go

@@ -85,18 +85,10 @@ func (m *DaemonSetPodManager) Start(ctx context.Context) (rerr error) {
 		}
 	}()
 
-	if err := addComputeDomainLabelIndexer[*corev1.Pod](m.informer); err != nil {
-		return fmt.Errorf("error adding indexer for MultiNodeEnvironment label: %w", err)
+	if err := addComputeDomainNodePodInexer(m.informer); err != nil {
+		return fmt.Errorf("error adding ComputeDomain Node indexer: %w", err)
 	}
 
-	m.mutationCache = cache.NewIntegerResourceVersionMutationCache(
-		klog.Background(),
-		m.informer.GetStore(),
-		m.informer.GetIndexer(),
-		mutationCacheTTL,
-		true,
-	)
-
 	m.waitGroup.Add(1)
 	go func() {
 		defer m.waitGroup.Done()
@@ -124,16 +116,12 @@ func (m *DaemonSetPodManager) List() ([]*corev1.Pod, error) {
 }
 
 func (m *DaemonSetPodManager) Get(ctx context.Context, cdUID string, nodeName string) (*corev1.Pod, error) {
-	pods, err := getByComputeDomainUID[*corev1.Pod](ctx, m.mutationCache, cdUID)
+	pod, err := getByComputeDomainUIDAndNode(ctx, m.informer.GetIndexer(), cdUID, nodeName)
 	if err != nil {
 		return nil, fmt.Errorf("error retrieving DaemonSetPods: %w", err)
 	}
-
-	for _, p := range pods {
-		if p.Spec.NodeName == nodeName {
-			return p, nil
-		}
+	if pod == nil {
+		return nil, fmt.Errorf("no DeamonSetPod with name %s matching DomainID %s found", nodeName, cdUID)
 	}
-
-	return nil, fmt.Errorf("no DeamonSetPod with name %s matching DomainID %s found", nodeName, cdUID)
+	return pod, nil
 }

cmd/compute-domain-controller/indexers.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/tools/cache"
 )
@@ -73,3 +74,41 @@ func getByComputeDomainUID[T1 *T2, T2 any](ctx context.Context, indexer Indexer,
 
 	return ds, nil
 }
+
+func addComputeDomainNodePodInexer(informer cache.SharedIndexInformer) error {
+	return informer.AddIndexers(cache.Indexers{
+		"computeDomainNode": func(obj any) ([]string, error) {
+			pod, ok := obj.(*corev1.Pod)
+			if !ok {
+				return nil, fmt.Errorf("expected a *corev1.Pod but got %T", obj)
+			}
+			labels := pod.GetObjectMeta().GetLabels()
+			if value, exists := labels[computeDomainLabelKey]; exists {
+				nodeName := pod.Spec.NodeName
+				if len(nodeName) != 0 {
+					return []string{fmt.Sprintf("%s/%s", value, nodeName)}, nil
+				}
+			}
+			return nil, nil
+		},
+	})
+}
+
+func getByComputeDomainUIDAndNode(ctx context.Context, indexer Indexer, cdUID string, nodeName string) (*corev1.Pod, error) {
+	key := fmt.Sprintf("%s/%s", cdUID, nodeName)
+	objs, err := indexer.ByIndex("computeDomainNode", key)
+	if err != nil {
+		return nil, fmt.Errorf("error getting Pod via ComputeDomain+Node index: %w", err)
+	}
+	if len(objs) == 0 {
+		return nil, nil
+	}
+	if len(objs) > 1 {
+		return nil, fmt.Errorf("multiple DaemonSetPods found for cdUID %s and nodeName %s", cdUID, nodeName)
+	}
+	pod, ok := objs[0].(*corev1.Pod)
+	if !ok {
+		return nil, fmt.Errorf("failed to cast to *corev1.Pod")
+	}
+	return pod, nil
+}

cmd/compute-domain-controller/mnsdaemonset.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 
 	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
 
 	nvapi "github.com/NVIDIA/k8s-dra-driver-gpu/api/nvidia.com/resource/v1beta1"
 )
@@ -124,3 +125,7 @@ func (m *MultiNamespaceDaemonSetManager) AssertRemoved(ctx context.Context, cdUI
 	}
 	return nil
 }
+
+func (m *MultiNamespaceDaemonSetManager) GetDaemonSetPod(ctx context.Context, namspace, cdUID, nodeName string) (*corev1.Pod, error) {
+	return nil, nil
+}

cmd/compute-domain-controller/resourceclaim.go

@@ -159,7 +159,7 @@ func (m *ResourceClaimManager) Stop() error {
 
 func (m *ResourceClaimManager) addOrUpdate(ctx context.Context, obj any) error {
 	rc, ok := obj.(*resourcev1.ResourceClaim)
-	if !ok || rc == nil {
+	if !ok {
 		return fmt.Errorf("failed to cast object to ResourceClaim")
 	}
 
@@ -168,13 +168,22 @@ func (m *ResourceClaimManager) addOrUpdate(ctx context.Context, obj any) error {
 	}
 
 	// Check the allocationResult of ResourceClaim to determine whether it should be monitored.
-	isEligible, domainID := m.checkClaimEligibleForComputeDomainLabeling(rc)
-	if !isEligible {
+	req, err := m.getComputeDomainChannelRequestConfig(rc)
+	if err != nil {
+		return fmt.Errorf("error getting config for ComputeDomainChannel request from ResourceClaim %s/%s: %w", rc.Namespace, rc.Name, err)
+	}
+
+	if req == nil {
 		return nil
 	}
 
-	if domainID == "" {
-		return fmt.Errorf("matching ResourceClaim %s/%s has no domainID in allocation config", rc.Namespace, rc.Name)
+	// Get domain id
+	domainID := req.DomainID
+
+	//Check namespace
+	err = m.AssertComputeDomainNamespace(rc.Namespace, domainID)
+	if err != nil {
+		return fmt.Errorf("failed to assert Namespace for computeDomain with domainID %s and ResourceClaim %s/%s: %w", domainID, rc.Namespace, rc.Name, err)
 	}
 
 	currentAllocationTimestamp := ""
@@ -198,12 +207,6 @@ func (m *ResourceClaimManager) addOrUpdate(ctx context.Context, obj any) error {
 	// Cancel a monitor that has already timed out.
 	m.cancelTimeoutedMonitor(ctx, rc, rcMonitors, currentAllocationTimestamp)
 
-	//Check namespace
-	err = m.AssertComputeDomainNamespace(rc.Namespace, domainID)
-	if err != nil {
-		return fmt.Errorf("failed to assert Namespace for computeDomain with domainID %s and ResourceClaim %s/%s: %w", domainID, rc.Namespace, rc.Name, err)
-	}
-
 	//Check if monitoring already launched and return if yes
 	if _, loaded := rcMonitors.Load(currentAllocationTimestamp); loaded {
 		return nil
@@ -300,17 +303,24 @@ func (m *ResourceClaimManager) addOrUpdate(ctx context.Context, obj any) error {
 	return nil
 }
 
-// Checks if ResourceClaim is Eligible for Compute Domain Labeling and returns domainID
-func (m *ResourceClaimManager) checkClaimEligibleForComputeDomainLabeling(rc *resourcev1.ResourceClaim) (bool, string) {
-	var domainID string
+// getComputeDomainChannelRequestConfig determines if a ResourceClaim is a monitoring target by filtering
+// the allocationResult and returns the config information associated with the target allocationResult.
+//
+// The processing target must meet the following conditions:
+// - The driver is "compute-domain.nvidia.com"
+// - The device is a channel device (determined by whether its corresponding config is ComputeDomainChannelConfig)
+// - The device has BindingConditions
+// - The device is not set any BindingConditions/BindingFailureConditions
+func (m *ResourceClaimManager) getComputeDomainChannelRequestConfig(rc *resourcev1.ResourceClaim) (*nvapi.ComputeDomainChannelConfig, error) {
+	var config *nvapi.ComputeDomainChannelConfig
 
 	configs, err := GetOpaqueDeviceConfigs(
 		nvapi.StrictDecoder,
 		DriverName,
 		rc.Status.Allocation.Devices.Config,
 	)
 	if err != nil {
-		return false, ""
+		return nil, err
 	}
 
 	var configResults []*resourcev1.DeviceRequestAllocationResult
@@ -324,8 +334,8 @@ func (m *ResourceClaimManager) checkClaimEligibleForComputeDomainLabeling(rc *re
 			if slices.Contains(c.Requests, result.Request) {
 				if _, ok := c.Config.(*nvapi.ComputeDomainChannelConfig); ok {
 					configResults = append(configResults, &result)
-					if domainID == "" {
-						domainID = c.Config.(*nvapi.ComputeDomainChannelConfig).DomainID
+					if config == nil {
+						config = c.Config.(*nvapi.ComputeDomainChannelConfig)
 					}
 					break
 				}
@@ -341,19 +351,19 @@ func (m *ResourceClaimManager) checkClaimEligibleForComputeDomainLabeling(rc *re
 					for _, cond := range deviceStatus.Conditions {
 						// Check the device is not set BindingConditions
 						if cond.Type == nvapi.ComputeDomainBindingConditions && cond.Status == metav1.ConditionTrue {
-							return false, domainID
+							return nil, nil
 						}
 						// Check the device is not set BindingFailureConditions
 						if cond.Type == nvapi.ComputeDomainBindingFailureConditions && cond.Status == metav1.ConditionTrue {
-							return false, domainID
+							return nil, nil
 						}
 					}
 				}
 			}
-			return true, domainID
+			return config, nil
 		}
 	}
-	return false, domainID
+	return nil, nil
 
 }
 
@@ -418,8 +428,8 @@ func (m *ResourceClaimManager) setBindingConditions(ctx context.Context, rc *res
 		for _, allocationDevice := range rcToUpdate.Status.Allocation.Devices.Results {
 			device := &resourcev1.AllocatedDeviceStatus{
 				Driver: allocationDevice.Driver,
-				Device: allocationDevice.Device,
 				Pool:   allocationDevice.Pool,
+				Device: allocationDevice.Device,
 			}
 			rcToUpdate.Status.Devices = append(rcToUpdate.Status.Devices, *device)
 		}

cmd/compute-domain-kubelet-plugin/computedomain.go

@@ -293,6 +293,50 @@ func (m *ComputeDomainManager) isCurrentNodeReadyInClique(ctx context.Context, c
 	return false
 }
 
+func (m *ComputeDomainManager) AssertComputeDomainNamespace(ctx context.Context, claimNamespace, cdUID string) error {
+	cd, err := m.GetComputeDomain(ctx, cdUID)
+	if err != nil {
+		return fmt.Errorf("error getting ComputeDomain: %w", err)
+	}
+	if cd == nil {
+		return fmt.Errorf("ComputeDomain not found: %s", cdUID)
+	}
+
+	if cd.Namespace != claimNamespace {
+		return fmt.Errorf("the ResourceClaim's namespace is different than the ComputeDomain's namespace")
+	}
+
+	return nil
+}
+
+func (m *ComputeDomainManager) AddNodeLabel(ctx context.Context, cdUID string) error {
+	node, err := m.config.clientsets.Core.CoreV1().Nodes().Get(ctx, m.config.flags.nodeName, metav1.GetOptions{})
+	if err != nil {
+		return fmt.Errorf("error retrieving Node: %w", err)
+	}
+
+	currentValue, exists := node.Labels[computeDomainLabelKey]
+	if exists && currentValue != cdUID {
+		return fmt.Errorf("label already exists for a different ComputeDomain")
+	}
+
+	if exists && currentValue == cdUID {
+		return nil
+	}
+
+	newNode := node.DeepCopy()
+	if newNode.Labels == nil {
+		newNode.Labels = make(map[string]string)
+	}
+	newNode.Labels[computeDomainLabelKey] = cdUID
+
+	if _, err = m.config.clientsets.Core.CoreV1().Nodes().Update(ctx, newNode, metav1.UpdateOptions{}); err != nil {
+		return fmt.Errorf("error updating Node with label: %w", err)
+	}
+
+	return nil
+}
+
 // RemoveNodeLabel() attempts removal and returns no error if the label was
 // removed or didn't exist in the first place.
 func (m *ComputeDomainManager) RemoveNodeLabel(ctx context.Context, cdUID string) error {

cmd/compute-domain-kubelet-plugin/device_state.go

@@ -487,6 +487,21 @@ func (s *DeviceState) applyComputeDomainChannelConfig(ctx context.Context, confi
 		return nil, fmt.Errorf("allocation failed: %w", err)
 	}
 
+	if featuregates.Enabled(featuregates.ComputeDomainBindingConditions) {
+		goto skipAssertNamespaceAddNodeLabel
+	}
+
+	// Create any necessary ComputeDomain channels and gather their CDI container edits.
+	if err := s.computeDomainManager.AssertComputeDomainNamespace(ctx, claim.Namespace, config.DomainID); err != nil {
+		return nil, permanentError{fmt.Errorf("error asserting ComputeDomain's namespace: %w", err)}
+	}
+
+	if err := s.computeDomainManager.AddNodeLabel(ctx, config.DomainID); err != nil {
+		return nil, fmt.Errorf("error adding Node label for ComputeDomain: %w", err)
+	}
+
+skipAssertNamespaceAddNodeLabel:
+
 	if err := s.computeDomainManager.AssertComputeDomainReady(ctx, config.DomainID); err != nil {
 		return nil, fmt.Errorf("error asserting ComputeDomain Ready: %w", err)
 	}

cmd/compute-domain-kubelet-plugin/deviceinfo.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 
 	nvapi "github.com/NVIDIA/k8s-dra-driver-gpu/api/nvidia.com/resource/v1beta1"
+	"github.com/NVIDIA/k8s-dra-driver-gpu/pkg/featuregates"
 	resourceapi "k8s.io/api/resource/v1"
 	"k8s.io/utils/ptr"
 )
@@ -59,8 +60,10 @@ func (d *ComputeDomainChannelInfo) GetDevice() resourceapi.Device {
 				IntValue: ptr.To(int64(d.ID)),
 			},
 		},
-		BindingConditions:        []string{nvapi.ComputeDomainBindingConditions},
-		BindingFailureConditions: []string{nvapi.ComputeDomainBindingFailureConditions},
+	}
+	if featuregates.Enabled(featuregates.ComputeDomainBindingConditions) {
+		device.BindingConditions = []string{nvapi.ComputeDomainBindingConditions}
+		device.BindingFailureConditions = []string{nvapi.ComputeDomainBindingFailureConditions}
 	}
 	return device
 }

pkg/featuregates/featuregates.go

@@ -55,6 +55,10 @@ const (
 	// CrashOnNVLinkFabricErrors causes the kubelet plugin to crash instead of
 	// falling back to non-fabric mode when NVLink fabric errors are detected.
 	CrashOnNVLinkFabricErrors featuregate.Feature = "CrashOnNVLinkFabricErrors"
+
+	// ComputeDomainBindingConditions enables scheduling of workload pods with channel devices
+	// to be delayed by DRADeviceBindingConditions until the IMEX Daemon Pods complete their processing.
+	ComputeDomainBindingConditions featuregate.Feature = "ComputeDomainBindingConditions"
 )
 
 // defaultFeatureGates contains the default settings for all project-specific feature gates.
@@ -116,6 +120,13 @@ var defaultFeatureGates = map[featuregate.Feature]featuregate.VersionedSpecs{
 			Version:    version.MajorMinor(25, 12),
 		},
 	},
+	ComputeDomainBindingConditions: {
+		{
+			Default:    false,
+			PreRelease: featuregate.Alpha,
+			Version:    version.MajorMinor(25, 12),
+		},
+	},
 }
 
 var (