Avoid expesive pointer copy in capi nodegroup

enxebre · May 7, 2024 · 31fdc39 · 31fdc39
1 parent 3fd892a
commit 31fdc39
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 19 deletions.
diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go
@@ -672,13 +672,13 @@ func (c *machineController) findScalableResourceProviderIDs(scalableResource *un
 	return providerIDs, nil
 }
 
-func (c *machineController) nodeGroups() ([]*nodegroup, error) {
+func (c *machineController) nodeGroups() ([]cloudprovider.NodeGroup, error) {
 	scalableResources, err := c.listScalableResources()
 	if err != nil {
 		return nil, err
 	}
 
-	nodegroups := make([]*nodegroup, 0, len(scalableResources))
+	nodegroups := make([]cloudprovider.NodeGroup, 0, len(scalableResources))
 
 	for _, r := range scalableResources {
 		ng, err := newNodeGroupFromScalableResource(c, r)
@@ -688,6 +688,7 @@ func (c *machineController) nodeGroups() ([]*nodegroup, error) {
 
 		if ng != nil {
 			nodegroups = append(nodegroups, ng)
+			klog.V(4).Infof("discovered node group: %s", ng.Debug())
 		}
 	}
 	return nodegroups, nil

diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go
@@ -2113,15 +2113,15 @@ func Test_machineController_nodeGroups(t *testing.T) {
 
 			// Sort results as order is not guaranteed.
 			sort.Slice(got, func(i, j int) bool {
-				return got[i].scalableResource.Name() < got[j].scalableResource.Name()
+				return got[i].(*nodegroup).scalableResource.Name() < got[j].(*nodegroup).scalableResource.Name()
 			})
 			sort.Slice(tc.expectedScalableResources, func(i, j int) bool {
 				return tc.expectedScalableResources[i].GetName() < tc.expectedScalableResources[j].GetName()
 			})
 
 			if err == nil {
 				for i := range got {
-					if !reflect.DeepEqual(got[i].scalableResource.unstructured, tc.expectedScalableResources[i]) {
+					if !reflect.DeepEqual(got[i].(*nodegroup).scalableResource.unstructured, tc.expectedScalableResources[i]) {
 						t.Errorf("nodeGroups() got = %v, expected to consist of nodegroups for scalable resources: %v", got, tc.expectedScalableResources)
 					}
 				}

diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go
@@ -261,7 +261,7 @@ func TestNodeGroupIncreaseSizeErrors(t *testing.T) {
 			t.Fatalf("expected 1 nodegroup, got %d", l)
 		}
 
-		ng := nodegroups[0]
+		ng := nodegroups[0].(*nodegroup)
 		currReplicas, err := ng.TargetSize()
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
@@ -348,7 +348,7 @@ func TestNodeGroupIncreaseSize(t *testing.T) {
 			t.Fatalf("expected 1 nodegroup, got %d", l)
 		}
 
-		ng := nodegroups[0]
+		ng := nodegroups[0].(*nodegroup)
 		currReplicas, err := ng.TargetSize()
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
@@ -427,7 +427,7 @@ func TestNodeGroupDecreaseTargetSize(t *testing.T) {
 			t.Fatalf("expected 1 nodegroup, got %d", l)
 		}
 
-		ng := nodegroups[0]
+		ng := nodegroups[0].(*nodegroup)
 
 		gvr, err := ng.scalableResource.GroupVersionResource()
 		if err != nil {
@@ -596,7 +596,7 @@ func TestNodeGroupDecreaseSizeErrors(t *testing.T) {
 			t.Fatalf("expected 1 nodegroup, got %d", l)
 		}
 
-		ng := nodegroups[0]
+		ng := nodegroups[0].(*nodegroup)
 		currReplicas, err := ng.TargetSize()
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
@@ -676,7 +676,7 @@ func TestNodeGroupDeleteNodes(t *testing.T) {
 			t.Fatalf("expected 1 nodegroup, got %d", l)
 		}
 
-		ng := nodegroups[0]
+		ng := nodegroups[0].(*nodegroup)
 		nodeNames, err := ng.Nodes()
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
@@ -889,7 +889,7 @@ func TestNodeGroupDeleteNodesTwice(t *testing.T) {
 			t.Fatalf("expected 1 nodegroup, got %d", l)
 		}
 
-		ng := nodegroups[0]
+		ng := nodegroups[0].(*nodegroup)
 		nodeNames, err := ng.Nodes()
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
@@ -961,7 +961,7 @@ func TestNodeGroupDeleteNodesTwice(t *testing.T) {
 			t.Fatalf("unexpected error: %v", err)
 		}
 
-		ng = nodegroups[0]
+		ng = nodegroups[0].(*nodegroup)
 
 		// Check the nodegroup is at the expected size
 		actualSize, err := ng.TargetSize()
@@ -1066,7 +1066,7 @@ func TestNodeGroupDeleteNodesSequential(t *testing.T) {
 			t.Fatalf("expected 1 nodegroup, got %d", l)
 		}
 
-		ng := nodegroups[0]
+		ng := nodegroups[0].(*nodegroup)
 		nodeNames, err := ng.Nodes()
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
@@ -1132,7 +1132,7 @@ func TestNodeGroupDeleteNodesSequential(t *testing.T) {
 			t.Fatalf("unexpected error: %v", err)
 		}
 
-		ng = nodegroups[0]
+		ng = nodegroups[0].(*nodegroup)
 
 		// Check the nodegroup is at the expected size
 		actualSize, err := ng.scalableResource.Replicas()

diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go
@@ -59,17 +59,12 @@ func (p *provider) GetResourceLimiter() (*cloudprovider.ResourceLimiter, error)
 }
 
 func (p *provider) NodeGroups() []cloudprovider.NodeGroup {
-	var result []cloudprovider.NodeGroup
 	nodegroups, err := p.controller.nodeGroups()
 	if err != nil {
 		klog.Errorf("error getting node groups: %v", err)
 		return nil
 	}
-	for _, ng := range nodegroups {
-		klog.V(4).Infof("discovered node group: %s", ng.Debug())
-		result = append(result, ng)
-	}
-	return result
+	return nodegroups
 }
 
 func (p *provider) NodeGroupForNode(node *corev1.Node) (cloudprovider.NodeGroup, error) {