Merge pull request #7854 from AppliedIntuition/master

Fix 2 bugs in the OCI integration

Author: k8s-ci-robot

cluster-autoscaler/cloudprovider/oci/common/oci_shape.go

@@ -105,23 +105,31 @@ func (osf *shapeGetterImpl) GetNodePoolShape(np *oke.NodePool, ephemeralStorage
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
-	resp, err := osf.shapeClient.ListShapes(ctx, core.ListShapesRequest{
+	request := core.ListShapesRequest{
 		CompartmentId: np.CompartmentId,
-		Limit:         common.Int(500),
-	})
-	if err != nil {
-		return nil, errors.Wrap(err, "unable to ListShapes")
 	}
 
-	// Update the cache based on latest results
-	for _, s := range resp.Items {
-		osf.cache[*s.Shape] = &Shape{
-			Name:                    shapeName,
-			CPU:                     getFloat32(s.Ocpus) * 2, // convert ocpu to vcpu
-			GPU:                     getInt(s.Gpus),
-			MemoryInBytes:           getFloat32(s.MemoryInGBs) * 1024 * 1024 * 1024,
-			EphemeralStorageInBytes: float32(ephemeralStorage),
+	for {
+		resp, err := osf.shapeClient.ListShapes(ctx, request)
+		if err != nil {
+			return nil, errors.Wrap(err, "unable to ListShapes")
+		}
+
+		// Update the cache based on latest results
+		for _, s := range resp.Items {
+			osf.cache[*s.Shape] = &Shape{
+				Name:                    *s.Shape,
+				CPU:                     getFloat32(s.Ocpus) * 2, // convert ocpu to vcpu
+				GPU:                     getInt(s.Gpus),
+				MemoryInBytes:           getFloat32(s.MemoryInGBs) * 1024 * 1024 * 1024,
+				EphemeralStorageInBytes: float32(ephemeralStorage),
+			}
+		}
+
+		if resp.OpcNextPage == nil {
+			break
 		}
+		request.Page = resp.OpcNextPage
 	}
 
 	// fetch value from updated cache... if it exists.

cluster-autoscaler/cloudprovider/oci/common/oci_shape_test.go

@@ -6,23 +6,26 @@ package common
 
 import (
 	"context"
-	oke "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/oci/vendor-internal/github.com/oracle/oci-go-sdk/v65/containerengine"
 	"reflect"
 	"strings"
 	"testing"
 
+	oke "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/oci/vendor-internal/github.com/oracle/oci-go-sdk/v65/containerengine"
+
 	"k8s.io/autoscaler/cluster-autoscaler/cloudprovider/oci/vendor-internal/github.com/oracle/oci-go-sdk/v65/common"
 	"k8s.io/autoscaler/cluster-autoscaler/cloudprovider/oci/vendor-internal/github.com/oracle/oci-go-sdk/v65/core"
 )
 
 type mockShapeClient struct {
 	err                   error
-	listShapeResp         core.ListShapesResponse
+	listShapeResponses    []core.ListShapesResponse
 	getInstanceConfigResp core.GetInstanceConfigurationResponse
+	requestCount          int
 }
 
 func (m *mockShapeClient) ListShapes(_ context.Context, _ core.ListShapesRequest) (core.ListShapesResponse, error) {
-	return m.listShapeResp, m.err
+	m.requestCount++
+	return m.listShapeResponses[m.requestCount-1], m.err
 }
 
 func (m *mockShapeClient) GetInstanceConfiguration(context.Context, core.GetInstanceConfigurationRequest) (core.GetInstanceConfigurationResponse, error) {
@@ -46,12 +49,14 @@ var instanceDetails = core.ComputeInstanceDetails{
 
 var shapeClient = &mockShapeClient{
 	err: nil,
-	listShapeResp: core.ListShapesResponse{
-		Items: []core.Shape{
-			{
-				Shape:       common.String("VM.Standard2.8"),
-				Ocpus:       common.Float32(8),
-				MemoryInGBs: common.Float32(120),
+	listShapeResponses: []core.ListShapesResponse{
+		{
+			Items: []core.Shape{
+				{
+					Shape:       common.String("VM.Standard2.8"),
+					Ocpus:       common.Float32(8),
+					MemoryInGBs: common.Float32(120),
+				},
 			},
 		},
 	},
@@ -76,12 +81,24 @@ func TestNodePoolGetShape(t *testing.T) {
 
 	shapeClient := &mockShapeClient{
 		err: nil,
-		listShapeResp: core.ListShapesResponse{
-			Items: []core.Shape{
-				{
-					Shape:       common.String("VM.Standard1.2"),
-					Ocpus:       common.Float32(2),
-					MemoryInGBs: common.Float32(16),
+		listShapeResponses: []core.ListShapesResponse{
+			{
+				Items: []core.Shape{
+					{
+						Shape:       common.String("VM.Standard1.1"),
+						Ocpus:       common.Float32(2),
+						MemoryInGBs: common.Float32(16),
+					},
+				},
+				OpcNextPage: common.String("nextPage"),
+			},
+			{
+				Items: []core.Shape{
+					{
+						Shape:       common.String("VM.Standard1.2"),
+						Ocpus:       common.Float32(2),
+						MemoryInGBs: common.Float32(16),
+					},
 				},
 			},
 		},

cluster-autoscaler/cloudprovider/oci/nodepools/oci_manager.go

@@ -461,6 +461,12 @@ func (m *ociManagerImpl) GetExistingNodePoolSizeViaCompute(np NodePool) (int, er
 			if !strings.HasPrefix(*item.DisplayName, displayNamePrefix) {
 				continue
 			}
+			// A node pool can fail to scale up if there's no capacity in the region. In that case, the node pool will be
+			// returned by the API, but it will not actually exist or have an ID, so we don't want to tell the autoscaler about it.
+			if *item.Id == "" {
+				klog.V(4).Infof("skipping node as it doesn't have a scaled-up instance")
+				continue
+			}
 			switch item.LifecycleState {
 			case core.InstanceLifecycleStateStopped, core.InstanceLifecycleStateTerminated:
 				klog.V(4).Infof("skipping instance is in stopped/terminated state: %q", *item.Id)
@@ -525,6 +531,13 @@ func (m *ociManagerImpl) GetNodePoolNodes(np NodePool) ([]cloudprovider.Instance
 	var instances []cloudprovider.Instance
 	for _, node := range nodePool.Nodes {
 
+		// A node pool can fail to scale up if there's no capacity in the region. In that case, the node pool will be
+		// returned by the API, but it will not actually exist or have an ID, so we don't want to tell the autoscaler about it.
+		if *node.Id == "" {
+			klog.V(4).Infof("skipping node as it doesn't have a scaled-up instance")
+			continue
+		}
+
 		if node.NodeError != nil {
 
 			errorClass := cloudprovider.OtherErrorClass

cluster-autoscaler/cloudprovider/oci/nodepools/oci_manager_test.go

@@ -6,11 +6,12 @@ package nodepools
 
 import (
 	"context"
-	"k8s.io/autoscaler/cluster-autoscaler/cloudprovider/oci/nodepools/consts"
 	"net/http"
 	"reflect"
 	"testing"
 
+	"k8s.io/autoscaler/cluster-autoscaler/cloudprovider/oci/nodepools/consts"
+
 	apiv1 "k8s.io/api/core/v1"
 	"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
 	"k8s.io/autoscaler/cluster-autoscaler/cloudprovider/oci/vendor-internal/github.com/oracle/oci-go-sdk/v65/common"
@@ -123,6 +124,12 @@ func TestGetNodePoolNodes(t *testing.T) {
 					Message: common.String("blah blah quota exceeded blah blah"),
 				},
 			},
+			{
+				// This case happens if a node fails to scale up due to lack of capacity in the region.
+				// It's not a real node, so we shouldn't return it in the list of nodes.
+				Id:             common.String(""),
+				LifecycleState: oke.NodeLifecycleStateCreating,
+			},
 		},
 	}