diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go index e6296ea4680..ba956273725 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go @@ -670,13 +670,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) @@ -686,6 +686,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 index 1be6823c61f..01dfb49dd2e 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go @@ -76,7 +76,7 @@ type testSpec struct { const customCAPIGroup = "custom.x-k8s.io" const fifteenSecondDuration = time.Second * 15 -func mustCreateTestController(t *testing.T, testConfigs ...*testConfig) (*machineController, testControllerShutdownFunc) { +func mustCreateTestController(t testing.TB, testConfigs ...*testConfig) (*machineController, testControllerShutdownFunc) { t.Helper() nodeObjects := make([]runtime.Object, 0) @@ -514,7 +514,7 @@ func makeLinkedNodeAndMachine(i int, namespace, clusterName string, owner metav1 return node, machine } -func addTestConfigs(t *testing.T, controller *machineController, testConfigs ...*testConfig) error { +func addTestConfigs(t testing.TB, controller *machineController, testConfigs ...*testConfig) error { t.Helper() for _, config := range testConfigs { @@ -2136,7 +2136,7 @@ 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() @@ -2144,7 +2144,7 @@ func Test_machineController_nodeGroups(t *testing.T) { 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 index e11ea053e72..cd7e94d7162 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go @@ -249,7 +249,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) @@ -336,7 +336,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) @@ -415,7 +415,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 { @@ -584,7 +584,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) @@ -664,7 +664,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) @@ -877,7 +877,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) @@ -949,7 +949,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() @@ -1054,7 +1054,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) @@ -1120,7 +1120,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 index cd8555fdd04..e3f5123ee20 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go @@ -58,17 +58,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) { diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider_test.go index 5c6fbb44bbb..47e58b6bfcd 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider_test.go @@ -101,3 +101,29 @@ func TestProviderConstructorProperties(t *testing.T) { t.Fatalf("expected 0 GPU types, got %d", got) } } +func BenchmarkNodeGroups(b *testing.B) { + resourceLimits := cloudprovider.ResourceLimiter{} + annotations := map[string]string{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "2", + } + + controller, stop := mustCreateTestController(b) + defer stop() + machineSetConfigs := createMachineSetTestConfigs("namespace", "", RandomString(6), 100, 1, annotations, nil) + if err := addTestConfigs(b, controller, machineSetConfigs...); err != nil { + b.Fatalf("unexpected error: %v", err) + } + + provider := newProvider(cloudprovider.ClusterAPIProviderName, &resourceLimits, controller) + if actual := provider.Name(); actual != cloudprovider.ClusterAPIProviderName { + b.Errorf("expected %q, got %q", cloudprovider.ClusterAPIProviderName, actual) + } + + b.ResetTimer() + b.Run("NodeGroups", func(b *testing.B) { + for i := 0; i < b.N; i++ { + provider.NodeGroups() + } + }) +}