chore: Reason only in terms of NodePools in the scheduler (#1065)

kubernetes-sigs · Mar 6, 2024 · 2c3c7e6 · 2c3c7e6
1 parent 1489b70
commit 2c3c7e6
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 25 deletions.
diff --git a/pkg/controllers/provisioning/provisioner.go b/pkg/controllers/provisioning/provisioner.go
@@ -193,11 +193,6 @@ var ErrNodePoolsNotFound = errors.New("no nodepools found")
 
 //nolint:gocyclo
 func (p *Provisioner) NewScheduler(ctx context.Context, pods []*v1.Pod, stateNodes []*state.StateNode) (*scheduler.Scheduler, error) {
-	// Build node templates
-	var nodeClaimTemplates []*scheduler.NodeClaimTemplate
-	instanceTypes := map[string][]*cloudprovider.InstanceType{}
-	domains := map[string]sets.Set[string]{}
-
 	nodePoolList := &v1beta1.NodePoolList{}
 	err := p.kubeClient.List(ctx, nodePoolList)
 	if err != nil {
@@ -219,12 +214,11 @@ func (p *Provisioner) NewScheduler(ctx context.Context, pods []*v1.Pod, stateNod
 	// will always attempt to schedule on the first nodeTemplate
 	nodePoolList.OrderByWeight()
 
-	for i := range nodePoolList.Items {
-		nodePool := &nodePoolList.Items[i]
-		// Create node template
-		nodeClaimTemplates = append(nodeClaimTemplates, scheduler.NewNodeClaimTemplate(nodePool))
+	instanceTypes := map[string][]*cloudprovider.InstanceType{}
+	domains := map[string]sets.Set[string]{}
+	for _, nodePool := range nodePoolList.Items {
 		// Get instance type options
-		instanceTypeOptions, err := p.cloudProvider.GetInstanceTypes(ctx, nodePool)
+		instanceTypeOptions, err := p.cloudProvider.GetInstanceTypes(ctx, lo.ToPtr(nodePool))
 		if err != nil {
 			// we just log an error and skip the provisioner to prevent a single mis-configured provisioner from stopping
 			// all scheduling
@@ -273,7 +267,7 @@ func (p *Provisioner) NewScheduler(ctx context.Context, pods []*v1.Pod, stateNod
 	}
 
 	// inject topology constraints
-	pods = p.injectTopology(ctx, pods)
+	pods = p.injectVolumeTopologyRequirements(ctx, pods)
 
 	// Calculate cluster topology
 	topology, err := scheduler.NewTopology(ctx, p.kubeClient, p.cluster, domains, pods)
@@ -284,7 +278,7 @@ func (p *Provisioner) NewScheduler(ctx context.Context, pods []*v1.Pod, stateNod
 	if err != nil {
 		return nil, fmt.Errorf("getting daemon pods, %w", err)
 	}
-	return scheduler.NewScheduler(p.kubeClient, nodeClaimTemplates, nodePoolList.Items, p.cluster, stateNodes, topology, instanceTypes, daemonSetPods, p.recorder), nil
+	return scheduler.NewScheduler(ctx, p.kubeClient, lo.ToSlicePtr(nodePoolList.Items), p.cluster, stateNodes, topology, instanceTypes, daemonSetPods, p.recorder), nil
 }
 
 func (p *Provisioner) Schedule(ctx context.Context) (scheduler.Results, error) {
@@ -436,7 +430,7 @@ func validateKarpenterManagedLabelCanExist(p *v1.Pod) error {
 	return nil
 }
 
-func (p *Provisioner) injectTopology(ctx context.Context, pods []*v1.Pod) []*v1.Pod {
+func (p *Provisioner) injectVolumeTopologyRequirements(ctx context.Context, pods []*v1.Pod) []*v1.Pod {
 	var schedulablePods []*v1.Pod
 	for _, pod := range pods {
 		if err := p.volumeTopology.Inject(ctx, pod); err != nil {

diff --git a/pkg/controllers/provisioning/scheduling/scheduler.go b/pkg/controllers/provisioning/scheduling/scheduler.go
@@ -43,36 +43,34 @@ import (
 	"sigs.k8s.io/karpenter/pkg/utils/resources"
 )
 
-func NewScheduler(kubeClient client.Client, nodeClaimTemplates []*NodeClaimTemplate,
-	nodePools []v1beta1.NodePool, cluster *state.Cluster, stateNodes []*state.StateNode, topology *Topology,
+func NewScheduler(ctx context.Context, kubeClient client.Client, nodePools []*v1beta1.NodePool,
+	cluster *state.Cluster, stateNodes []*state.StateNode, topology *Topology,
 	instanceTypes map[string][]*cloudprovider.InstanceType, daemonSetPods []*v1.Pod,
 	recorder events.Recorder) *Scheduler {
 
 	// if any of the nodePools add a taint with a prefer no schedule effect, we add a toleration for the taint
 	// during preference relaxation
 	toleratePreferNoSchedule := false
-	for _, prov := range nodePools {
-		for _, taint := range prov.Spec.Template.Spec.Taints {
+	for _, np := range nodePools {
+		for _, taint := range np.Spec.Template.Spec.Taints {
 			if taint.Effect == v1.TaintEffectPreferNoSchedule {
 				toleratePreferNoSchedule = true
 			}
 		}
 	}
 
+	templates := lo.Map(nodePools, func(np *v1beta1.NodePool, _ int) *NodeClaimTemplate { return NewNodeClaimTemplate(np) })
 	s := &Scheduler{
 		id:                 uuid.NewUUID(),
 		kubeClient:         kubeClient,
-		nodeClaimTemplates: nodeClaimTemplates,
+		nodeClaimTemplates: templates,
 		topology:           topology,
 		cluster:            cluster,
 		instanceTypes:      instanceTypes,
-		daemonOverhead:     getDaemonOverhead(nodeClaimTemplates, daemonSetPods),
+		daemonOverhead:     getDaemonOverhead(templates, daemonSetPods),
 		recorder:           recorder,
 		preferences:        &Preferences{ToleratePreferNoSchedule: toleratePreferNoSchedule},
-		remainingResources: map[string]v1.ResourceList{},
-	}
-	for _, nodePool := range nodePools {
-		s.remainingResources[nodePool.Name] = v1.ResourceList(nodePool.Spec.Limits)
+		remainingResources: lo.SliceToMap(nodePools, func(np *v1beta1.NodePool) (string, v1.ResourceList) { return np.Name, v1.ResourceList(np.Spec.Limits) }),
 	}
 	s.calculateExistingNodeClaims(stateNodes, daemonSetPods)
 	return s

diff --git a/pkg/controllers/provisioning/scheduling/scheduling_benchmark_test.go b/pkg/controllers/provisioning/scheduling/scheduling_benchmark_test.go
@@ -174,8 +174,8 @@ func benchmarkScheduler(b *testing.B, instanceCount, podCount int) {
 		b.Fatalf("creating topology, %s", err)
 	}
 
-	scheduler := scheduling.NewScheduler(client, []*scheduling.NodeClaimTemplate{scheduling.NewNodeClaimTemplate(nodePool)},
-		nil, cluster, nil, topology,
+	scheduler := scheduling.NewScheduler(ctx, client, []*v1beta1.NodePool{nodePool},
+		cluster, nil, topology,
 		map[string][]*cloudprovider.InstanceType{nodePool.Name: instanceTypes}, nil,
 		events.NewRecorder(&record.FakeRecorder{}))