Просмотр исходного кода

Fix the issue where the prices for most Alibaba Cloud instance types are incorrect when the instance family generation number is greater than or equal to 7 (#2858)

* Fix the issue where the prices for most Alibaba Cloud instance types are incorrect when the instance family generation number is greater than or equal to 7.

Update provider.go

remove debug info

Signed-off-by: qixiangyang <qixiangyangrm@foxmail.com>

* 1. Revise 'alibabaDefaultToCloudEssd' exclude 6+ generation instance types.
2. Revise comments when system disk information is not available
3. Add error logging and optimize the return value for the 'getInstanceFamilyGenerationFromType' function.
4. Add test cases for generating node requests for various instance types.

Signed-off-by: xiangyang.qi <xiangyang.qi@advancegroup.com>

1. Revise 'alibabaDefaultToCloudEssd' exclude 6+ generation instance types.
2. Revise comments when system disk information is not available
3. Add error logging and optimize the return value for the 'getInstanceFamilyGenerationFromType' function.
4. Add test cases for generating node requests for various instance types.

Signed-off-by: qixiangyang <qixiangyangrm@foxmail.com>

* Update pkg/cloud/alibaba/provider.go

Co-authored-by: Alex Meijer <ameijer@users.noreply.github.com>
Signed-off-by: qixiangyang <qixiangyangrm@foxmail.com>

* delete useless const

Signed-off-by: qixiangyang <qixiangyangrm@foxmail.com>

---------

Signed-off-by: qixiangyang <qixiangyangrm@foxmail.com>
Co-authored-by: xiangyang.qi <xiangyang.qi@advancegroup.com>
Co-authored-by: Alex Meijer <ameijer@users.noreply.github.com>
qixiangyang 1 год назад
Родитель
Сommit
c2de805f66
2 измененных файлов с 143 добавлено и 4 удалено
  1. 24 4
      pkg/cloud/alibaba/provider.go
  2. 119 0
      pkg/cloud/alibaba/provider_test.go

+ 24 - 4
pkg/cloud/alibaba/provider.go

@@ -67,7 +67,7 @@ var (
 )
 
 // Variable to keep track of instance families that fail in DescribePrice API due improper defaulting of systemDisk if the information is not available
-var alibabaDefaultToCloudEssd = []string{"g6e", "r6e", "r7", "g7", "g7a", "r7a"}
+var alibabaDefaultToCloudEssd = []string{"g6e", "r6e"}
 
 var alibabaRegions = []string{
 	"cn-qingdao",
@@ -980,9 +980,9 @@ func createDescribePriceACSRequest(i interface{}) (*requests.CommonRequest, erro
 				request.QueryParams["SystemDisk.PerformanceLevel"] = node.SystemDisk.PerformanceLevel
 			}
 		} else {
-			// When System Disk information is not available for instance family g6e, r7 and r6e the defaults in
-			// DescribePrice dont default rightly to cloud_essd for these instances.
-			if slices.Contains(alibabaDefaultToCloudEssd, node.InstanceTypeFamily) {
+			// When the system disk information is not available, and the instance family is g6e or r6e,
+			// or the instance generation is 6 or above, the default disk category in DescribePrice should be cloud_essd.
+			if slices.Contains(alibabaDefaultToCloudEssd, node.InstanceTypeFamily) || getInstanceFamilyGenerationFromType(node.InstanceType) > 6 {
 				request.QueryParams["SystemDisk.Category"] = ALIBABA_DISK_CLOUD_ESSD_CATEGORY
 			}
 		}
@@ -1148,6 +1148,26 @@ func getInstanceFamilyFromType(instanceType string) string {
 	return splitinstanceType[1]
 }
 
+// This function is used to obtain the generation of the instance family from the InstanceType,
+// because when the generation is higher than or equal to 7, the instance disk type will not support cloud_efficiency.
+// In such cases, when calling the DescribePrice interface, the system disk type will default to cloud_essd.
+func getInstanceFamilyGenerationFromType(instanceType string) int {
+	// FamilyName format: g7ne or g7 or r7 or r6e,
+	familyName := getInstanceFamilyFromType(instanceType)
+	re := regexp.MustCompile(`(\d+)`)
+	match := re.FindString(familyName)
+	if match != "" {
+		generation, err := strconv.Atoi(match)
+		if err != nil {
+			log.Errorf("unable to convert the generation of the instance type %s to integer", instanceType)
+		} else {
+			return generation
+		}
+	}
+	log.Warnf("unable to find the generation of the instance type %s,", instanceType)
+	return -1
+}
+
 // getInstanceIDFromProviderID returns the instance ID associated with the Node. A *v1.Node providerID in Alibaba cloud
 // is of <REGION-ID>.<INSTANCE-ID>. This function returns the Instance ID for the given ProviderID. if its unable to interpret
 // it defaults to empty string.

+ 119 - 0
pkg/cloud/alibaba/provider_test.go

@@ -112,6 +112,20 @@ func TestProcessDescribePriceAndCreateAlibabaPricing(t *testing.T) {
 			},
 			expectedError: nil,
 		},
+		{
+			name: "test General Purpose Type g8a instance family",
+			teststruct: &SlimK8sNode{
+				InstanceType:       "ecs.g8a.8xlarge",
+				RegionID:           "cn-hangzhou",
+				PriceUnit:          "Hour",
+				MemorySizeInKiB:    "33554432KiB",
+				IsIoOptimized:      true,
+				OSType:             "Linux",
+				ProviderID:         "cn-hangzhou.i-test-01c",
+				InstanceTypeFamily: "g8a",
+			},
+			expectedError: nil,
+		},
 		{
 			name: "test Enhanced General Purpose Type g6e instance family",
 			teststruct: &SlimK8sNode{
@@ -856,3 +870,108 @@ func TestDeterminePVRegion(t *testing.T) {
 	}
 
 }
+
+func TestGetInstanceFamilyGenerationFromType(t *testing.T) {
+	cases := []struct {
+		name                             string
+		instanceType                     string
+		expectedInstanceFamilyGeneration int
+	}{
+		{
+			name:                             "test if ecs.[instance-family].[different-type] work",
+			instanceType:                     "ecs.sn2ne.2xlarge",
+			expectedInstanceFamilyGeneration: 2,
+		},
+		{
+			name:                             "test if ecs.[instance-family].[different-type] work",
+			instanceType:                     "ecs.g7.large",
+			expectedInstanceFamilyGeneration: 7,
+		},
+		{
+			name:                             "test if random word gives you ALIBABA_UNKNOWN_INSTANCE_FAMILY_TYPE value ",
+			instanceType:                     "random.value",
+			expectedInstanceFamilyGeneration: -1,
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			returnValue := getInstanceFamilyGenerationFromType(c.instanceType)
+			if returnValue != c.expectedInstanceFamilyGeneration {
+				t.Fatalf("Case name %s: expected instance family generation of type %d but got %d", c.name, c.expectedInstanceFamilyGeneration, returnValue)
+			}
+		})
+	}
+}
+
+func TestCreateDescribeNodePriceACSRequest(t *testing.T) {
+
+	cases := []struct {
+		name                 string
+		testStruct           interface{}
+		expectedError        error
+		expectedDiskCategory string
+	}{
+		{
+			// Test case for instance type ecs.g6.large
+			name: "test request parma when instance type is ecs.g6.large",
+			testStruct: &SlimK8sNode{
+				InstanceType:       "ecs.g6.large",
+				RegionID:           "cn-hangzhou",
+				PriceUnit:          "Hour",
+				MemorySizeInKiB:    "16KiB",
+				IsIoOptimized:      true,
+				OSType:             "Linux",
+				ProviderID:         "Ali-XXX-node-01",
+				InstanceTypeFamily: "g6",
+			},
+			expectedError:        nil,
+			expectedDiskCategory: "",
+		},
+		{
+			// Test case for instance type ecs.g7.large
+			name: "test request parma when instance type is ecs.g7.large",
+			testStruct: &SlimK8sNode{
+				InstanceType:       "ecs.g7.large",
+				RegionID:           "cn-hangzhou",
+				PriceUnit:          "Hour",
+				MemorySizeInKiB:    "16KiB",
+				IsIoOptimized:      true,
+				OSType:             "Linux",
+				ProviderID:         "Ali-XXX-node-02",
+				InstanceTypeFamily: "g7",
+			},
+			expectedError:        nil,
+			expectedDiskCategory: ALIBABA_DISK_CLOUD_ESSD_CATEGORY,
+		},
+		{
+			// Test case for instance type ecs.g7.large, this instance type is in 'alibabaDefaultToCloudEssd'
+			name: "test request parma when instance type is ecs.g6e.large",
+			testStruct: &SlimK8sNode{
+				InstanceType:       "ecs.g6e.large",
+				RegionID:           "cn-hangzhou",
+				PriceUnit:          "Hour",
+				MemorySizeInKiB:    "16KiB",
+				IsIoOptimized:      true,
+				OSType:             "Linux",
+				ProviderID:         "Ali-XXX-node-03",
+				InstanceTypeFamily: "g6e",
+			},
+			expectedError:        nil,
+			expectedDiskCategory: ALIBABA_DISK_CLOUD_ESSD_CATEGORY,
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			req, err := createDescribePriceACSRequest(c.testStruct)
+			t.Logf("Request Params SystemDisk.Category: %v", req.QueryParams["SystemDisk.Category"])
+			if err != nil && c.expectedError != nil {
+				t.Fatalf("Case name %s: Error converting to Alibaba cloud request", c.name)
+			}
+			if c.expectedDiskCategory != req.QueryParams["SystemDisk.Category"] {
+				t.Fatalf("Case name %s: Disk Category is not set correctly", c.name)
+			}
+		})
+	}
+}