Automated cherry pick of #70353: Improve Azure instance metadata handling

pkg/cloudprovider/providers/azure/azure.go

@@ -160,7 +160,7 @@ type Cloud struct {
 	DisksClient             DisksClient
 	FileClient              FileClient
 	resourceRequestBackoff  wait.Backoff
-	metadata                *InstanceMetadata
+	metadata                *InstanceMetadataService
 	vmSet                   VMSet
 
 	// Lock for access to node caches, includes nodeZones, nodeResourceGroups, and unmanagedNodes.
@@ -329,7 +329,10 @@ func NewCloud(configReader io.Reader) (cloudprovider.Interface, error) {
 			az.CloudProviderBackoffJitter)
 	}
 
-	az.metadata = NewInstanceMetadata()
+	az.metadata, err = NewInstanceMetadataService(metadataURL)
+	if err != nil {
+		return nil, err
+	}
 
 	if az.MaximumLoadBalancerRuleCount == 0 {
 		az.MaximumLoadBalancerRuleCount = maximumLoadBalancerRuleCount

pkg/cloudprovider/providers/azure/azure_instance_metadata.go

@@ -18,11 +18,17 @@ package azure
 
 import (
 	"encoding/json"
+	"fmt"
 	"io/ioutil"
 	"net/http"
+	"time"
 )
 
-const metadataURL = "http://169.254.169.254/metadata/"
+const (
+	metadataCacheTTL = time.Minute
+	metadataCacheKey = "InstanceMetadata"
+	metadataURL      = "http://169.254.169.254/metadata/instance"
+)
 
 // NetworkMetadata contains metadata about an instance's network
 type NetworkMetadata struct {
@@ -54,67 +60,100 @@ type Subnet struct {
 	Prefix  string `json:"prefix"`
 }
 
-// InstanceMetadata knows how to query the Azure instance metadata server.
-type InstanceMetadata struct {
-	baseURL string
-}
-
 // ComputeMetadata represents compute information
 type ComputeMetadata struct {
-	Name   string `json:"name,omitempty"`
-	Zone   string `json:"zone,omitempty"`
-	VMSize string `json:"vmSize,omitempty"`
+	SKU            string `json:"sku,omitempty"`
+	Name           string `json:"name,omitempty"`
+	Zone           string `json:"zone,omitempty"`
+	VMSize         string `json:"vmSize,omitempty"`
+	OSType         string `json:"osType,omitempty"`
+	Location       string `json:"location,omitempty"`
+	FaultDomain    string `json:"platformFaultDomain,omitempty"`
+	UpdateDomain   string `json:"platformUpdateDomain,omitempty"`
+	ResourceGroup  string `json:"resourceGroupName,omitempty"`
+	VMScaleSetName string `json:"vmScaleSetName,omitempty"`
 }
 
-// NewInstanceMetadata creates an instance of the InstanceMetadata accessor object.
-func NewInstanceMetadata() *InstanceMetadata {
-	return &InstanceMetadata{
-		baseURL: metadataURL,
-	}
+// InstanceMetadata represents instance information.
+type InstanceMetadata struct {
+	Compute *ComputeMetadata `json:"compute,omitempty"`
+	Network *NetworkMetadata `json:"network,omitempty"`
 }
 
-// makeMetadataURL makes a complete metadata URL from the given path.
-func (i *InstanceMetadata) makeMetadataURL(path string) string {
-	return i.baseURL + path
+// InstanceMetadataService knows how to query the Azure instance metadata server.
+type InstanceMetadataService struct {
+	metadataURL string
+	imsCache    *timedCache
 }
 
-// Object queries the metadata server and populates the passed in object
-func (i *InstanceMetadata) Object(path string, obj interface{}) error {
-	data, err := i.queryMetadataBytes(path, "json")
-	if err != nil {
-		return err
+// NewInstanceMetadataService creates an instance of the InstanceMetadataService accessor object.
+func NewInstanceMetadataService(metadataURL string) (*InstanceMetadataService, error) {
+	ims := &InstanceMetadataService{
+		metadataURL: metadataURL,
 	}
-	return json.Unmarshal(data, obj)
-}
 
-// Text queries the metadata server and returns the corresponding text
-func (i *InstanceMetadata) Text(path string) (string, error) {
-	data, err := i.queryMetadataBytes(path, "text")
+	imsCache, err := newTimedcache(metadataCacheTTL, ims.getInstanceMetadata)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
-	return string(data), err
-}
 
-func (i *InstanceMetadata) queryMetadataBytes(path, format string) ([]byte, error) {
-	client := &http.Client{}
+	ims.imsCache = imsCache
+	return ims, nil
+}
 
-	req, err := http.NewRequest("GET", i.makeMetadataURL(path), nil)
+func (ims *InstanceMetadataService) getInstanceMetadata(key string) (interface{}, error) {
+	req, err := http.NewRequest("GET", ims.metadataURL, nil)
 	if err != nil {
 		return nil, err
 	}
 	req.Header.Add("Metadata", "True")
+	req.Header.Add("User-Agent", "golang/kubernetes-cloud-provider")
 
 	q := req.URL.Query()
-	q.Add("format", format)
+	q.Add("format", "json")
 	q.Add("api-version", "2017-12-01")
 	req.URL.RawQuery = q.Encode()
 
+	client := &http.Client{}
 	resp, err := client.Do(req)
 	if err != nil {
 		return nil, err
 	}
 	defer resp.Body.Close()
 
-	return ioutil.ReadAll(resp.Body)
+	if resp.StatusCode != 200 {
+		return nil, fmt.Errorf("failure of getting instance metadata with response %q", resp.Status)
+	}
+
+	data, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return nil, err
+	}
+
+	obj := InstanceMetadata{}
+	err = json.Unmarshal(data, &obj)
+	if err != nil {
+		return nil, err
+	}
+
+	return &obj, nil
+}
+
+// GetMetadata gets instance metadata from cache.
+func (ims *InstanceMetadataService) GetMetadata() (*InstanceMetadata, error) {
+	cache, err := ims.imsCache.Get(metadataCacheKey)
+	if err != nil {
+		return nil, err
+	}
+
+	// Cache shouldn't be nil, but added a check incase something wrong.
+	if cache == nil {
+		return nil, fmt.Errorf("failure of getting instance metadata")
+	}
+
+	if metadata, ok := cache.(*InstanceMetadata); ok {
+		return metadata, nil
+	}
+
+	return nil, fmt.Errorf("failure of getting instance metadata")
 }

pkg/cloudprovider/providers/azure/azure_instances.go

@@ -18,6 +18,7 @@ package azure
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"strings"
 
@@ -67,12 +68,16 @@ func (az *Cloud) NodeAddresses(ctx context.Context, name types.NodeName) ([]v1.N
 	}
 
 	if az.UseInstanceMetadata {
-		computeMetadata, err := az.getComputeMetadata()
+		metadata, err := az.metadata.GetMetadata()
 		if err != nil {
 			return nil, err
 		}
 
-		isLocalInstance, err := az.isCurrentInstance(name, computeMetadata.Name)
+		if metadata.Compute == nil || metadata.Network == nil {
+			return nil, fmt.Errorf("failure of getting instance metadata")
+		}
+
+		isLocalInstance, err := az.isCurrentInstance(name, metadata.Compute.Name)
 		if err != nil {
 			return nil, err
 		}
@@ -82,30 +87,38 @@ func (az *Cloud) NodeAddresses(ctx context.Context, name types.NodeName) ([]v1.N
 			return addressGetter(name)
 		}
 
-		ipAddress := IPAddress{}
-		err = az.metadata.Object("instance/network/interface/0/ipv4/ipAddress/0", &ipAddress)
-		if err != nil {
-			return nil, err
-		}
-
-		// Fall back to ARM API if the address is empty string.
-		// TODO: this is a workaround because IMDS is not stable enough.
-		// It should be removed after IMDS fixing the issue.
-		if strings.TrimSpace(ipAddress.PrivateIP) == "" {
-			return addressGetter(name)
+		if len(metadata.Network.Interface) == 0 {
+			return nil, fmt.Errorf("no interface is found for the instance")
 		}
 
 		// Use ip address got from instance metadata.
+		ipAddress := metadata.Network.Interface[0]
 		addresses := []v1.NodeAddress{
-			{Type: v1.NodeInternalIP, Address: ipAddress.PrivateIP},
 			{Type: v1.NodeHostName, Address: string(name)},
 		}
-		if len(ipAddress.PublicIP) > 0 {
-			addr := v1.NodeAddress{
-				Type:    v1.NodeExternalIP,
-				Address: ipAddress.PublicIP,
+		for _, address := range ipAddress.IPV4.IPAddress {
+			addresses = append(addresses, v1.NodeAddress{
+				Type:    v1.NodeInternalIP,
+				Address: address.PrivateIP,
+			})
+			if len(address.PublicIP) > 0 {
+				addresses = append(addresses, v1.NodeAddress{
+					Type:    v1.NodeExternalIP,
+					Address: address.PublicIP,
+				})
+			}
+		}
+		for _, address := range ipAddress.IPV6.IPAddress {
+			addresses = append(addresses, v1.NodeAddress{
+				Type:    v1.NodeInternalIP,
+				Address: address.PrivateIP,
+			})
+			if len(address.PublicIP) > 0 {
+				addresses = append(addresses, v1.NodeAddress{
+					Type:    v1.NodeExternalIP,
+					Address: address.PublicIP,
+				})
 			}
-			addresses = append(addresses, addr)
 		}
 		return addresses, nil
 	}
@@ -172,17 +185,6 @@ func (az *Cloud) InstanceShutdownByProviderID(ctx context.Context, providerID st
 	return strings.ToLower(powerStatus) == vmPowerStateStopped || strings.ToLower(powerStatus) == vmPowerStateDeallocated, nil
 }
 
-// getComputeMetadata gets compute information from instance metadata.
-func (az *Cloud) getComputeMetadata() (*ComputeMetadata, error) {
-	computeInfo := ComputeMetadata{}
-	err := az.metadata.Object(computeMetadataURI, &computeInfo)
-	if err != nil {
-		return nil, err
-	}
-
-	return &computeInfo, nil
-}
-
 func (az *Cloud) isCurrentInstance(name types.NodeName, metadataVMName string) (bool, error) {
 	var err error
 	nodeName := mapNodeNameToVMName(name)
@@ -213,12 +215,16 @@ func (az *Cloud) InstanceID(ctx context.Context, name types.NodeName) (string, e
 	}
 
 	if az.UseInstanceMetadata {
-		computeMetadata, err := az.getComputeMetadata()
+		metadata, err := az.metadata.GetMetadata()
 		if err != nil {
 			return "", err
 		}
 
-		isLocalInstance, err := az.isCurrentInstance(name, computeMetadata.Name)
+		if metadata.Compute == nil {
+			return "", fmt.Errorf("failure of getting instance metadata")
+		}
+
+		isLocalInstance, err := az.isCurrentInstance(name, metadata.Compute.Name)
 		if err != nil {
 			return "", err
 		}
@@ -229,18 +235,15 @@ func (az *Cloud) InstanceID(ctx context.Context, name types.NodeName) (string, e
 		}
 
 		// Get resource group name.
-		resourceGroup, err := az.metadata.Text("instance/compute/resourceGroupName")
-		if err != nil {
-			return "", err
-		}
+		resourceGroup := metadata.Compute.ResourceGroup
 
 		// Compose instanceID based on nodeName for standard instance.
 		if az.VMType == vmTypeStandard {
 			return az.getStandardMachineID(resourceGroup, nodeName), nil
 		}
 
 		// Get scale set name and instanceID from vmName for vmss.
-		ssName, instanceID, err := extractVmssVMName(computeMetadata.Name)
+		ssName, instanceID, err := extractVmssVMName(metadata.Compute.Name)
 		if err != nil {
 			if err == ErrorNotVmssInstance {
 				// Compose machineID for standard Node.
@@ -289,18 +292,22 @@ func (az *Cloud) InstanceType(ctx context.Context, name types.NodeName) (string,
 	}
 
 	if az.UseInstanceMetadata {
-		computeMetadata, err := az.getComputeMetadata()
+		metadata, err := az.metadata.GetMetadata()
 		if err != nil {
 			return "", err
 		}
 
-		isLocalInstance, err := az.isCurrentInstance(name, computeMetadata.Name)
+		if metadata.Compute == nil {
+			return "", fmt.Errorf("failure of getting instance metadata")
+		}
+
+		isLocalInstance, err := az.isCurrentInstance(name, metadata.Compute.Name)
 		if err != nil {
 			return "", err
 		}
 		if isLocalInstance {
-			if computeMetadata.VMSize != "" {
-				return computeMetadata.VMSize, nil
+			if metadata.Compute.VMSize != "" {
+				return metadata.Compute.VMSize, nil
 			}
 		}
 	}

pkg/cloudprovider/providers/azure/azure_instances_test.go

@@ -66,7 +66,6 @@ func setTestVirtualMachines(c *Cloud, vmList map[string]string) {
 
 func TestInstanceID(t *testing.T) {
 	cloud := getTestCloud()
-	cloud.metadata = &InstanceMetadata{}
 
 	testcases := []struct {
 		name         string
@@ -105,15 +104,18 @@ func TestInstanceID(t *testing.T) {
 		}
 
 		mux := http.NewServeMux()
-		mux.Handle("/instance/compute", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			fmt.Fprintf(w, fmt.Sprintf("{\"name\":\"%s\"}", test.metadataName))
+		mux.Handle("/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			fmt.Fprintf(w, fmt.Sprintf(`{"compute":{"name":"%s"}}`, test.metadataName))
 		}))
 		go func() {
 			http.Serve(listener, mux)
 		}()
 		defer listener.Close()
 
-		cloud.metadata.baseURL = "http://" + listener.Addr().String() + "/"
+		cloud.metadata, err = NewInstanceMetadataService("http://" + listener.Addr().String() + "/")
+		if err != nil {
+			t.Errorf("Test [%s] unexpected error: %v", test.name, err)
+		}
 		vmListWithPowerState := make(map[string]string)
 		for _, vm := range test.vmList {
 			vmListWithPowerState[vm] = ""