Merge pull request #8358 from k8s-infra-cherrypick-robot/cherry-pick-8315-to-cluster-autoscaler-release-1.31

[cluster-autoscaler-release-1.31] Handle Out of host capacity scenario in OCI nodepools

Author: k8s-ci-robot

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

@@ -519,6 +519,7 @@ func (m *ociManagerImpl) GetNodePoolNodes(np NodePool) ([]cloudprovider.Instance
 
 	nodePool, err := m.nodePoolCache.get(np.Id())
 	if err != nil {
+		klog.Error(err, "error while performing GetNodePoolNodes call")
 		return nil, err
 	}
 
@@ -527,10 +528,14 @@ func (m *ociManagerImpl) GetNodePoolNodes(np NodePool) ([]cloudprovider.Instance
 
 		if node.NodeError != nil {
 
+			// We should move away from the approach of determining a node error as a Out of host capacity
+			// through string comparison. An error code specifically for Out of host capacity must be set
+			// and returned in the API response.
 			errorClass := cloudprovider.OtherErrorClass
 			if *node.NodeError.Code == "LimitExceeded" ||
-				(*node.NodeError.Code == "InternalServerError" &&
-					strings.Contains(*node.NodeError.Message, "quota")) {
+				*node.NodeError.Code == "QuotaExceeded" ||
+				(*node.NodeError.Code == "InternalError" &&
+					strings.Contains(*node.NodeError.Message, "Out of host capacity")) {
 				errorClass = cloudprovider.OutOfResourcesErrorClass
 			}
 

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

@@ -119,8 +119,8 @@ func TestGetNodePoolNodes(t *testing.T) {
 			{
 				Id: common.String("node8"),
 				NodeError: &oke.NodeError{
-					Code:    common.String("InternalServerError"),
-					Message: common.String("blah blah quota exceeded blah blah"),
+					Code:    common.String("InternalError"),
+					Message: common.String("blah blah Out of host capacity blah blah"),
 				},
 			},
 		},
@@ -179,8 +179,8 @@ func TestGetNodePoolNodes(t *testing.T) {
 				State: cloudprovider.InstanceCreating,
 				ErrorInfo: &cloudprovider.InstanceErrorInfo{
 					ErrorClass:   cloudprovider.OutOfResourcesErrorClass,
-					ErrorCode:    "InternalServerError",
-					ErrorMessage: "blah blah quota exceeded blah blah",
+					ErrorCode:    "InternalError",
+					ErrorMessage: "blah blah Out of host capacity blah blah",
 				},
 			},
 		},

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

@@ -209,6 +209,27 @@ func (np *nodePool) DecreaseTargetSize(delta int) error {
 		}
 	}
 	klog.V(4).Infof("DECREASE_TARGET_CHECK_VIA_COMPUTE: %v", decreaseTargetCheckViaComputeBool)
+	np.manager.InvalidateAndRefreshCache()
+	nodes, err := np.manager.GetNodePoolNodes(np)
+	if err != nil {
+		klog.V(4).Error(err, "error while performing GetNodePoolNodes call")
+		return err
+	}
+	// We do not have an OCI API that allows us to delete a node with a compute instance. So we rely on
+	// the below approach to determine the number running instance in a nodepool from the compute API and
+	//update the size of the nodepool accordingly. We should move away from this approach once we have an API
+	// to delete a specific node without a compute instance.
+	if !decreaseTargetCheckViaComputeBool {
+		for _, node := range nodes {
+			if node.Status != nil && node.Status.ErrorInfo != nil {
+				if node.Status.ErrorInfo.ErrorClass == cloudprovider.OutOfResourcesErrorClass {
+					klog.Infof("Using Compute to calculate nodepool size as nodepool may contain nodes without a compute instance.")
+					decreaseTargetCheckViaComputeBool = true
+					break
+				}
+			}
+		}
+	}
 	var nodesLen int
 	if decreaseTargetCheckViaComputeBool {
 		nodesLen, err = np.manager.GetExistingNodePoolSizeViaCompute(np)
@@ -217,12 +238,6 @@ func (np *nodePool) DecreaseTargetSize(delta int) error {
 			return err
 		}
 	} else {
-		np.manager.InvalidateAndRefreshCache()
-		nodes, err := np.manager.GetNodePoolNodes(np)
-		if err != nil {
-			klog.V(4).Error(err, "error while performing GetNodePoolNodes call")
-			return err
-		}
 		nodesLen = len(nodes)
 	}