From 62d3c5b7b345409540ade22745e1dfe4be9247c9 Mon Sep 17 00:00:00 2001
From: Daniel Gutowski <danielgutowski@google.com>
Date: Mon, 24 Apr 2023 05:47:38 -0700
Subject: [PATCH] Add a dedicated struct for the MaxResourceLimitReached
 Reasons

This allows us to get the list of missing resources from the
skipped reaons object, without computing it one more time.
---
 .../core/scaleup/orchestrator/orchestrator.go |  4 +--
 .../scaleup/orchestrator/skippedreasons.go    | 25 ++++++++++++++++---
 .../orchestrator/skippedreasons_test.go       |  2 +-
 3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/cluster-autoscaler/core/scaleup/orchestrator/orchestrator.go b/cluster-autoscaler/core/scaleup/orchestrator/orchestrator.go
index ed25bdb152..9b4c263cc9 100644
--- a/cluster-autoscaler/core/scaleup/orchestrator/orchestrator.go
+++ b/cluster-autoscaler/core/scaleup/orchestrator/orchestrator.go
@@ -513,7 +513,7 @@ func (o *ScaleUpOrchestrator) IsNodeGroupReadyToScaleUp(nodeGroup cloudprovider.
 }
 
 // IsNodeGroupResourceExceeded returns nil if node group resource limits are not exceeded, otherwise a reason is provided.
-func (o *ScaleUpOrchestrator) IsNodeGroupResourceExceeded(resourcesLeft resource.Limits, nodeGroup cloudprovider.NodeGroup, nodeInfo *schedulerframework.NodeInfo) *SkippedReasons {
+func (o *ScaleUpOrchestrator) IsNodeGroupResourceExceeded(resourcesLeft resource.Limits, nodeGroup cloudprovider.NodeGroup, nodeInfo *schedulerframework.NodeInfo) status.Reasons {
 	resourcesDelta, err := o.resourceManager.DeltaForNode(o.autoscalingContext, nodeInfo, nodeGroup)
 	if err != nil {
 		klog.Errorf("Skipping node group %s; error getting node group resources: %v", nodeGroup.Id(), err)
@@ -533,7 +533,7 @@ func (o *ScaleUpOrchestrator) IsNodeGroupResourceExceeded(resourcesLeft resource
 				continue
 			}
 		}
-		return MaxResourceLimitReached(checkResult.ExceededResources)
+		return NewMaxResourceLimitReached(checkResult.ExceededResources)
 	}
 	return nil
 }
diff --git a/cluster-autoscaler/core/scaleup/orchestrator/skippedreasons.go b/cluster-autoscaler/core/scaleup/orchestrator/skippedreasons.go
index ba85316492..85537cedc6 100644
--- a/cluster-autoscaler/core/scaleup/orchestrator/skippedreasons.go
+++ b/cluster-autoscaler/core/scaleup/orchestrator/skippedreasons.go
@@ -45,7 +45,26 @@ var (
 	NotReadyReason = NewSkippedReasons("not ready for scale-up")
 )
 
-// MaxResourceLimitReached returns a reason describing which cluster wide resource limits were reached.
-func MaxResourceLimitReached(resources []string) *SkippedReasons {
-	return NewSkippedReasons(fmt.Sprintf("max cluster %s limit reached", strings.Join(resources, ", ")))
+// MaxResourceLimitReached contains information why given node group was skipped.
+type MaxResourceLimitReached struct {
+	messages  []string
+	resources []string
+}
+
+// Reasons returns a slice of reasons why the node group was not considered for scale up.
+func (sr *MaxResourceLimitReached) Reasons() []string {
+	return sr.messages
+}
+
+// Resources returns a slice of resources which were missing in the node group.
+func (sr *MaxResourceLimitReached) Resources() []string {
+	return sr.resources
+}
+
+// NewMaxResourceLimitReached returns a reason describing which cluster wide resource limits were reached.
+func NewMaxResourceLimitReached(resources []string) *MaxResourceLimitReached {
+	return &MaxResourceLimitReached{
+		messages:  []string{fmt.Sprintf("max cluster %s limit reached", strings.Join(resources, ", "))},
+		resources: resources,
+	}
 }
diff --git a/cluster-autoscaler/core/scaleup/orchestrator/skippedreasons_test.go b/cluster-autoscaler/core/scaleup/orchestrator/skippedreasons_test.go
index 8f888d653b..0c768e523b 100644
--- a/cluster-autoscaler/core/scaleup/orchestrator/skippedreasons_test.go
+++ b/cluster-autoscaler/core/scaleup/orchestrator/skippedreasons_test.go
@@ -44,7 +44,7 @@ func TestMaxResourceLimitReached(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := MaxResourceLimitReached(tt.resources); !reflect.DeepEqual(got.Reasons(), tt.wantReasons) {
+			if got := NewMaxResourceLimitReached(tt.resources); !reflect.DeepEqual(got.Reasons(), tt.wantReasons) {
 				t.Errorf("MaxResourceLimitReached(%v) = %v, want %v", tt.resources, got.Reasons(), tt.wantReasons)
 			}
 		})