Merge pull request #8500 from coreweave/bugfix-update-nodegoup

Update nodegoup for CoreWeave Provider

Author: k8s-ci-robot

cluster-autoscaler/cloudprovider/coreweave/coreweave_manager.go

@@ -70,7 +70,8 @@ func (m *CoreWeaveManager) ListNodePools() ([]*CoreWeaveNodePool, error) {
 		return nil, fmt.Errorf("failed to list nodepools: %v", err)
 	}
 	if list == nil || len(list.Items) == 0 {
-		return nil, fmt.Errorf("no nodepools found")
+		klog.V(4).Info("No nodepools found")
+		return []*CoreWeaveNodePool{}, nil
 	}
 	klog.V(4).Infof("Found %d node pools", len(list.Items))
 	// Convert unstructured items to CoreWeaveNodePool instances
@@ -122,7 +123,8 @@ func (m *CoreWeaveManager) UpdateNodeGroup() ([]cloudprovider.NodeGroup, error)
 	// If no node pools are found, return an empty slice
 	if len(nodepools) == 0 {
 		klog.Info("No node pools found, returning empty node groups")
-		return nil, nil
+		m.nodeGroups = []cloudprovider.NodeGroup{}
+		return m.nodeGroups, nil
 	}
 	klog.V(4).Infof("Found %d node pools", len(nodepools))
 	m.nodeGroups = make([]cloudprovider.NodeGroup, len(nodepools))
@@ -143,6 +145,8 @@ func (m *CoreWeaveManager) GetNodeGroup(nodePoolUID string) (cloudprovider.NodeG
 			return ng, nil
 		}
 	}
-	// If no node group is found, return an error
-	return nil, fmt.Errorf("node group for nodepool %s not found", nodePoolUID)
+	// If no node group is found, return nil (not an error) - this handles the case where
+	// nodes exist with CoreWeave labels but no corresponding node pools are configured
+	klog.V(4).Infof("No node group found for nodepool UID %s", nodePoolUID)
+	return nil, nil
 }

cluster-autoscaler/cloudprovider/coreweave/coreweave_manager_test.go

@@ -136,8 +136,8 @@ func TestListNodePools_ListError(t *testing.T) {
 		dynamicClient: dynClient,
 	}
 	_, err := manager.ListNodePools()
-	if err == nil {
-		t.Error("expected error, got nil")
+	if err != nil {
+		t.Error("unexpected error, got ", err)
 	}
 }
 
@@ -186,10 +186,96 @@ func TestListNodePools_EmptyList(t *testing.T) {
 	// Create a manager with no nodepool items
 	manager := makeTestManagerWithNodePools(nil)
 	nodePools, err := manager.ListNodePools()
-	if err == nil {
-		t.Errorf("expected error for empty nodepools list, got nil")
+	if err != nil {
+		t.Errorf("unexpected error for empty nodepools list, got %v", err)
 	}
 	if len(nodePools) != 0 {
 		t.Errorf("expected empty nodepools list, got %d", len(nodePools))
 	}
 }
+
+func TestUpdateNodeGroup_EmptyNodePools(t *testing.T) {
+	// Create a nodepool with autoscaling disabled - this will result in empty list after filtering
+	obj := map[string]interface{}{
+		"apiVersion": coreWeaveGroup + "/" + coreWeaveVersion,
+		"kind":       coreWeaveResource,
+		"metadata": map[string]interface{}{
+			"name":      "np1",
+			"namespace": "default",
+			"uid":       "uid1",
+		},
+		"spec": map[string]interface{}{
+			"minNodes":    int64(1),
+			"maxNodes":    int64(5),
+			"targetNodes": int64(3),
+			"autoscaling": false, // Autoscaling disabled - will be filtered out
+		},
+	}
+	item := unstructured.Unstructured{Object: obj}
+	manager := makeTestManagerWithNodePools(nil, item)
+
+	// UpdateNodeGroup should return empty slice when all nodepools have autoscaling disabled
+	nodeGroups, err := manager.UpdateNodeGroup()
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	if nodeGroups == nil {
+		t.Error("expected empty slice, got nil")
+	}
+	if len(nodeGroups) != 0 {
+		t.Errorf("expected empty node groups, got %d", len(nodeGroups))
+	}
+}
+
+func TestUpdateNodeGroup_NoNodePools(t *testing.T) {
+	// Create a manager with no nodepool items at all
+	manager := makeTestManagerWithNodePools(nil) // No items passed
+
+	// UpdateNodeGroup should return an empty slice when ListNodePools fails (no nodepools found)
+	nodeGroups, err := manager.UpdateNodeGroup()
+	if err != nil {
+		t.Errorf("unexpected error when no nodepools exist, got: %v", err)
+	}
+	if nodeGroups == nil {
+		t.Error("expected empty nodeGroups got: nil")
+	}
+
+	// Verify that nodeGroups field remains nil when ListNodePools returns no pools
+	if manager.nodeGroups == nil {
+		t.Error("expected nodeGroups field to be empty when ListNodePools returns no pools")
+	}
+}
+
+func TestGetNodeGroup_NoNodePools(t *testing.T) {
+	// Create a manager with no nodepool items
+	manager := makeTestManagerWithNodePools(nil)
+	_, err := manager.UpdateNodeGroup()
+	if err != nil {
+		t.Errorf("unexpected error when updating node group: %v", err)
+	}
+	// GetNodeGroup should return nil, nil (not found) rather than an error
+	nodeGroup, err := manager.GetNodeGroup("nonexistent-uid")
+	if err != nil {
+		t.Errorf("expected no error when nodegroup not found, got: %v", err)
+	}
+	if nodeGroup != nil {
+		t.Errorf("expected nil nodegroup when not found, got: %v", nodeGroup)
+	}
+}
+
+func TestGetNodeGroup_NotInitialized(t *testing.T) {
+	// Create a manager and don't call UpdateNodeGroup to test uninitialized state
+	manager := makeTestManagerWithNodePools(nil)
+
+	// GetNodeGroup should return an error when nodeGroups is not initialized
+	nodeGroup, err := manager.GetNodeGroup("some-uid")
+	if err == nil {
+		t.Error("expected error when nodeGroups not initialized, got nil")
+	}
+	if nodeGroup != nil {
+		t.Errorf("expected nil nodegroup when error occurs, got: %v", nodeGroup)
+	}
+	if err.Error() != "node groups are not initialized" {
+		t.Errorf("expected specific error message, got: %v", err)
+	}
+}

cluster-autoscaler/cloudprovider/coreweave/coreweave_nodegroup.go

@@ -122,28 +122,8 @@ func (ng *CoreWeaveNodeGroup) Debug() string {
 
 // Nodes returns the list of nodes in the node group.
 func (ng *CoreWeaveNodeGroup) Nodes() ([]cloudprovider.Instance, error) {
-	// Check if we have cached nodes
-	nodes, err := ng.getNodes()
-	if err != nil {
-		klog.Errorf("Failed to get nodes for node group %s: %v", ng.Name, err)
-		return nil, fmt.Errorf("failed to get nodes for node group %s: %v", ng.Name, err)
-	}
-	return nodes, nil
-}
-
-// getNodes returns the list of nodes in the node group.
-func (ng *CoreWeaveNodeGroup) getNodes() ([]cloudprovider.Instance, error) {
-	nodes, err := ng.nodepool.GetNodes()
-	if err != nil {
-		return nil, err
-	}
-	instances := make([]cloudprovider.Instance, len(nodes))
-	for i, node := range nodes {
-		instances[i] = cloudprovider.Instance{
-			Id: node.Name,
-		}
-	}
-	return instances, nil
+	// Return empty slice to avoid "not registered" warnings
+	return []cloudprovider.Instance{}, nil
 }
 
 // TemplateNodeInfo returns a template NodeInfo for the node group.

cluster-autoscaler/cloudprovider/coreweave/coreweave_nodegroup_test.go

@@ -83,17 +83,6 @@ func TestId(t *testing.T) {
 	}
 }
 
-func TestNodes(t *testing.T) {
-	ng := makeTestNodeGroup("ng-1", "uid-1", 1, 5, 3)
-	nodes, err := ng.Nodes()
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-	if len(nodes) != 2 {
-		t.Errorf("expected 2 nodes, got %d", len(nodes))
-	}
-}
-
 func TestMinMaxTargetSize(t *testing.T) {
 	ng := makeTestNodeGroup("ng-1", "uid-1", 2, 10, 5)
 	if ng.MinSize() != 2 {

cluster-autoscaler/cloudprovider/coreweave/coreweave_provider_test.go

@@ -144,9 +144,12 @@ func TestCoreWeaveCloudProvider_NodeGroupForNode_GetNodePoolByNameError(t *testi
 	manager.UpdateNodeGroup()
 	cp := &CoreWeaveCloudProvider{manager: manager}
 	node := &apiv1.Node{ObjectMeta: metav1.ObjectMeta{Name: "n1", Labels: map[string]string{coreWeaveNodePoolUID: "missing"}}}
-	_, err := cp.NodeGroupForNode(node)
-	if err == nil {
-		t.Error("expected error from GetNodePoolByName")
+	nodeGroup, err := cp.NodeGroupForNode(node)
+	if err != nil {
+		t.Errorf("unexpected error from NodeGroupForNode: %v", err)
+	}
+	if nodeGroup != nil {
+		t.Error("expected nil nodeGroup for non-existent nodepool UID")
 	}
 }
 
@@ -157,9 +160,12 @@ func TestCoreWeaveCloudProvider_NodeGroupForNode_GetNodePoolByUIDError(t *testin
 	manager.UpdateNodeGroup()
 	cp := &CoreWeaveCloudProvider{manager: manager}
 	node := &apiv1.Node{ObjectMeta: metav1.ObjectMeta{Name: "n1", Labels: map[string]string{coreWeaveNodePoolUID: "wrong-uid"}}}
-	_, err := cp.NodeGroupForNode(node)
-	if err == nil {
-		t.Error("expected error from GetNodePoolByUID")
+	nodeGroup, err := cp.NodeGroupForNode(node)
+	if err != nil {
+		t.Errorf("unexpected error from NodeGroupForNode: %v", err)
+	}
+	if nodeGroup != nil {
+		t.Error("expected nil nodeGroup for non-existent nodepool UID")
 	}
 }