Break out unmarshal from GenerateEC2InstanceTypes

Refactor to allow for optimisation

cluster-autoscaler/cloudprovider/aws/aws_util.go

@@ -20,17 +20,20 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"github.com/aws/aws-sdk-go/aws"
+	"io"
-	"github.com/aws/aws-sdk-go/aws/ec2metadata"
-	"github.com/aws/aws-sdk-go/aws/endpoints"
-	"github.com/aws/aws-sdk-go/aws/session"
 	"io/ioutil"
-	klog "k8s.io/klog/v2"
 	"net/http"
 	"os"
 	"regexp"
 	"strconv"
 	"strings"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/aws/ec2metadata"
+	"github.com/aws/aws-sdk-go/aws/endpoints"
+	"github.com/aws/aws-sdk-go/aws/session"
+
+	klog "k8s.io/klog/v2"
 )
 
 var (
@@ -87,16 +90,9 @@ func GenerateEC2InstanceTypes(region string) (map[string]*InstanceType, error) {
 
 			defer res.Body.Close()
 
-			body, err := ioutil.ReadAll(res.Body)
+			unmarshalled, err := unmarshalProductsResponse(res.Body)
 			if err != nil {
-				klog.Warningf("Error parsing %s skipping...\n", url)
+				klog.Warningf("Error parsing %s skipping...\n%s\n", url, err)
-				continue
-			}
-
-			var unmarshalled = response{}
-			err = json.Unmarshal(body, &unmarshalled)
-			if err != nil {
-				klog.Warningf("Error unmarshalling %s, skip...\n", url)
 				continue
 			}
 
@@ -135,6 +131,21 @@ func GetStaticEC2InstanceTypes() (map[string]*InstanceType, string) {
 	return InstanceTypes, staticListLastUpdateTime
 }
 
+func unmarshalProductsResponse(r io.Reader) (*response, error) {
+	body, err := ioutil.ReadAll(r)
+	if err != nil {
+		return nil, err
+	}
+
+	var unmarshalled = response{}
+	err = json.Unmarshal(body, &unmarshalled)
+	if err != nil {
+		return nil, err
+	}
+
+	return &unmarshalled, nil
+}
+
 func parseMemory(memory string) int64 {
 	reg, err := regexp.Compile("[^0-9\\.]+")
 	if err != nil {

cluster-autoscaler/cloudprovider/aws/aws_util_test.go

@@ -17,12 +17,14 @@ limitations under the License.
 package aws
 
 import (
-	"github.com/stretchr/testify/assert"
 	"net/http"
 	"net/http/httptest"
 	"os"
 	"strconv"
+	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestGetStaticEC2InstanceTypes(t *testing.T) {
@@ -136,3 +138,118 @@ func TestGetCurrentAwsRegionWithRegionEnv(t *testing.T) {
 	assert.Nil(t, err)
 	assert.Equal(t, region, result)
 }
+
+func TestUnmarshalProductsResponse(t *testing.T) {
+	body := `
+{
+  "products": {
+	"VVD8BG8WWFD3DAZN" : {
+      "sku" : "VVD8BG8WWFD3DAZN",
+      "productFamily" : "Compute Instance",
+      "attributes" : {
+        "servicecode" : "AmazonEC2",
+        "location" : "US East (N. Virginia)",
+        "locationType" : "AWS Region",
+        "instanceType" : "r5b.4xlarge",
+        "currentGeneration" : "Yes",
+        "instanceFamily" : "Memory optimized",
+        "vcpu" : "16",
+        "physicalProcessor" : "Intel Xeon Platinum 8259 (Cascade Lake)",
+        "clockSpeed" : "3.1 GHz",
+        "memory" : "128 GiB",
+        "storage" : "EBS only",
+        "networkPerformance" : "Up to 10 Gigabit",
+        "processorArchitecture" : "64-bit",
+        "tenancy" : "Shared",
+        "operatingSystem" : "Linux",
+        "licenseModel" : "No License required",
+        "usagetype" : "UnusedBox:r5b.4xlarge",
+        "operation" : "RunInstances:0004",
+        "availabilityzone" : "NA",
+        "capacitystatus" : "UnusedCapacityReservation",
+        "classicnetworkingsupport" : "false",
+        "dedicatedEbsThroughput" : "10 Gbps",
+        "ecu" : "NA",
+        "enhancedNetworkingSupported" : "Yes",
+        "instancesku" : "G4NFAXD9TGJM3RY8",
+        "intelAvxAvailable" : "Yes",
+        "intelAvx2Available" : "No",
+        "intelTurboAvailable" : "No",
+        "marketoption" : "OnDemand",
+        "normalizationSizeFactor" : "32",
+        "preInstalledSw" : "SQL Std",
+        "servicename" : "Amazon Elastic Compute Cloud",
+        "vpcnetworkingsupport" : "true"
+      }
+    },
+    "C36QEQQQJ8ZR7N32" : {
+      "sku" : "C36QEQQQJ8ZR7N32",
+      "productFamily" : "Compute Instance",
+      "attributes" : {
+        "servicecode" : "AmazonEC2",
+        "location" : "US East (N. Virginia)",
+        "locationType" : "AWS Region",
+        "instanceType" : "d3en.8xlarge",
+        "currentGeneration" : "Yes",
+        "instanceFamily" : "Storage optimized",
+        "vcpu" : "32",
+        "physicalProcessor" : "Intel Xeon Platinum 8259 (Cascade Lake)",
+        "clockSpeed" : "3.1 GHz",
+        "memory" : "128 GiB",
+        "storage" : "16 x 14000 HDD",
+        "networkPerformance" : "50 Gigabit",
+        "processorArchitecture" : "64-bit",
+        "tenancy" : "Dedicated",
+        "operatingSystem" : "SUSE",
+        "licenseModel" : "No License required",
+        "usagetype" : "DedicatedRes:d3en.8xlarge",
+        "operation" : "RunInstances:000g",
+        "availabilityzone" : "NA",
+        "capacitystatus" : "AllocatedCapacityReservation",
+        "classicnetworkingsupport" : "false",
+        "dedicatedEbsThroughput" : "5000 Mbps",
+        "ecu" : "NA",
+        "enhancedNetworkingSupported" : "Yes",
+        "instancesku" : "2XW3BCEZ83WMGFJY",
+        "intelAvxAvailable" : "Yes",
+        "intelAvx2Available" : "Yes",
+        "intelTurboAvailable" : "Yes",
+        "marketoption" : "OnDemand",
+        "normalizationSizeFactor" : "64",
+        "preInstalledSw" : "NA",
+        "processorFeatures" : "AVX; AVX2; Intel AVX; Intel AVX2; Intel AVX512; Intel Turbo",
+        "servicename" : "Amazon Elastic Compute Cloud",
+        "vpcnetworkingsupport" : "true"
+      }
+    }
+  }
+}
+`
+	r := strings.NewReader(body)
+	resp, err := unmarshalProductsResponse(r)
+	assert.Nil(t, err)
+	assert.Len(t, resp.Products, 2)
+	assert.NotNil(t, resp.Products["VVD8BG8WWFD3DAZN"])
+	assert.NotNil(t, resp.Products["C36QEQQQJ8ZR7N32"])
+	assert.Equal(t, resp.Products["VVD8BG8WWFD3DAZN"].Attributes.InstanceType, "r5b.4xlarge")
+	assert.Equal(t, resp.Products["C36QEQQQJ8ZR7N32"].Attributes.InstanceType, "d3en.8xlarge")
+
+	invalidJsonTests := map[string]string{
+		"[":                     "[",
+		"]":                     "]",
+		"}":                     "}",
+		"{":                     "{",
+		"Plain text":            "invalid",
+		"List":                  "[]",
+		"Invalid products ([])": `{"products":[]}`,
+		"Invalid product ([])":  `{"products":{"zz":[]}}`,
+	}
+	for name, body := range invalidJsonTests {
+		t.Run(name, func(t *testing.T) {
+			r := strings.NewReader(body)
+			resp, err := unmarshalProductsResponse(r)
+			assert.NotNil(t, err)
+			assert.Nil(t, resp)
+		})
+	}
+}