Repository: kubernetes-sigs/cloud-provider-azure (go)

Issue:
Get instance public IP from instance metadata service with standard SKU
**Is your feature request related to a problem?/Why is this needed**
<!-- A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] -->

When VM or VMSS is bounded with "standard" LoadBalancer, the instance's public IP address would not be part of `http://169.254.169.254/metadata/instance`. Hence, `kubectl get nodes` would not show public IP address when using "standard" LoadBalancer.

**Describe the solution you'd like in detail**
<!-- A clear and concise description of what you want to happen. -->

Azure network now exposes load balancer profile with instance metadata endpoint `http://169.254.169.254/metadata/loadbalancer?api-version=2020-10-01`. Here is an example:

```json
# curl -sSL -H Metadata:true "http://169.254.169.254/metadata/loadbalancer?api-version=2020-10-01" | jq
{
  "loadbalancer": {
    "publicIpAddresses": [
      {
        "frontendIpAddress": "100.100.100.100",
        "privateIpAddress": "10.240.0.2"
      }
    ],
    "inboundRules": [
      {
        "frontendIpAddress": "20.20.20.20",
        "protocol": "Tcp",
        "frontendPort": 99,
        "backendPort": 99,
        "privateIpAddress": "20.20.20.20"
      },
      {
        "frontendIpAddress": "11.11.11.11",
        "protocol": "Tcp",
        "frontendPort": 80,
        "backendPort": 80,
        "privateIpAddress": "11.11.11.11"
      }
    ],
    "outboundRules": []
  }
}
```

Azure node manager should try this endpoint to get the instance public IP when it doesn't get it from `/metadata/instance` API.

**Describe alternatives you've considered**
<!-- A clear and concise description of any alternative solutions or features you've considered. -->

**Additional context**
<!-- Add any other context or screenshots about the feature request here. -->


Pull request:
Fix node public IP fetching from instance metadata service
<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. For reference on required PR/issue labels, read here:
https://git.k8s.io/community/contributors/devel/release.md#issue-kind-label
3. Ensure you have added or ran the appropriate tests for your PR: https://git.k8s.io/community/contributors/devel/testing.md
4. If you want *faster* PR reviews, read how: https://git.k8s.io/community/contributors/guide/pull-requests.md#best-practices-for-faster-reviews
5. Follow the instructions for writing a release note: https://git.k8s.io/community/contributors/guide/release-notes.md
6. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

**What type of PR is this?**
> Uncomment only one ` /kind <>` line, hit enter to put that in a new line, and remove leading whitespaces from that line:
>
> /kind api-change
> /kind bug
> /kind cleanup
> /kind design
> /kind documentation
> /kind failing-test
> /kind feature
> /kind flake
/kind bug

**What this PR does / why we need it**:

When VM or VMSS is bounded with "standard" LoadBalancer, the instance's public IP address would not be part of http://169.254.169.254/metadata/instance. Hence, kubectl get nodes would not show public IP address when using "standard" LoadBalancer.

This PR fixes the issue by fetching public IP from IMDS loadbalancer endpoint `http://169.254.169.254/metadata/loadbalancer `.

**Which issue(s) this PR fixes**:
<!-- 
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #535

**Special notes for your reviewer**:


**Release note**:
```
Fix node public IP fetching from instance metadata service when the node is part of standard load balancer backend pool.
```


Patch:
diff --git a/pkg/provider/azure.go b/pkg/provider/azure.go
index 4455d63e4..67d1a5a90 100644
--- a/pkg/provider/azure.go
+++ b/pkg/provider/azure.go
@@ -462,7 +462,7 @@ func (az *Cloud) InitializeCloudFromConfig(config *Config, fromSecret bool) erro
 	az.Config = *config
 	az.Environment = *env
 	az.ResourceRequestBackoff = resourceRequestBackoff
-	az.metadata, err = NewInstanceMetadataService(metadataURL)
+	az.metadata, err = NewInstanceMetadataService(imdsServer)
 	if err != nil {
 		return err
 	}
diff --git a/pkg/provider/azure_instance_metadata.go b/pkg/provider/azure_instance_metadata.go
index 797fe09c0..37491742b 100644
--- a/pkg/provider/azure_instance_metadata.go
+++ b/pkg/provider/azure_instance_metadata.go
@@ -23,13 +23,19 @@ import (
 	"net/http"
 	"time"
 
+	"k8s.io/klog/v2"
 	azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
 )
 
 const (
 	metadataCacheTTL = time.Minute
 	metadataCacheKey = "InstanceMetadata"
-	metadataURL      = "http://169.254.169.254/metadata/instance"
+
+	imdsInstanceAPIVersion     = "2019-03-11"
+	imdsLoadBalancerAPIVersion = "2020-10-01"
+	imdsServer                 = "http://169.254.169.254"
+	imdsInstanceURI            = "/metadata/instance"
+	imdsLoadBalancerURI        = "/metadata/loadbalancer"
 )
 
 // NetworkMetadata contains metadata about an instance's network
@@ -84,19 +90,35 @@ type InstanceMetadata struct {
 	Network *NetworkMetadata `json:"network,omitempty"`
 }
 
+// PublicIPMetadata represents the public IP metadata.
+type PublicIPMetadata struct {
+	FrontendIPAddress string `json:"frontendIpAddress,omitempty"`
+	PrivateIPAddress  string `json:"privateIpAddress,omitempty"`
+}
+
+// LoadbalancerProfile represents load balancer profile in IMDS.
+type LoadbalancerProfile struct {
+	PublicIPAddresses []PublicIPMetadata `json:"publicIpAddresses,omitempty"`
+}
+
+// LoadBalancerMetadata represents load balancer metadata.
+type LoadBalancerMetadata struct {
+	LoadBalancer *LoadbalancerProfile `json:"loadbalancer,omitempty"`
+}
+
 // InstanceMetadataService knows how to query the Azure instance metadata server.
 type InstanceMetadataService struct {
-	metadataURL string
-	imsCache    *azcache.TimedCache
+	imdsServer string
+	imsCache   *azcache.TimedCache
 }
 
 // NewInstanceMetadataService creates an instance of the InstanceMetadataService accessor object.
-func NewInstanceMetadataService(metadataURL string) (*InstanceMetadataService, error) {
+func NewInstanceMetadataService(imdsServer string) (*InstanceMetadataService, error) {
 	ims := &InstanceMetadataService{
-		metadataURL: metadataURL,
+		imdsServer: imdsServer,
 	}
 
-	imsCache, err := azcache.NewTimedcache(metadataCacheTTL, ims.getInstanceMetadata)
+	imsCache, err := azcache.NewTimedcache(metadataCacheTTL, ims.getMetadata)
 	if err != nil {
 		return nil, err
 	}
@@ -105,8 +127,52 @@ func NewInstanceMetadataService(metadataURL string) (*InstanceMetadataService, e
 	return ims, nil
 }
 
-func (ims *InstanceMetadataService) getInstanceMetadata(key string) (interface{}, error) {
-	req, err := http.NewRequest("GET", ims.metadataURL, nil)
+func (ims *InstanceMetadataService) getMetadata(key string) (interface{}, error) {
+	instanceMetadata, err := ims.getInstanceMetadata(key)
+	if err != nil {
+		return nil, err
+	}
+
+	if instanceMetadata.Network != nil && len(instanceMetadata.Network.Interface) > 0 {
+		netInterface := instanceMetadata.Network.Interface[0]
+		if (len(netInterface.IPV4.IPAddress) > 0 && len(netInterface.IPV4.IPAddress[0].PublicIP) > 0) ||
+			(len(netInterface.IPV6.IPAddress) > 0 && len(netInterface.IPV6.IPAddress[0].PublicIP) > 0) {
+			// Return if public IP address has already part of instance metadata.
+			return instanceMetadata, nil
+		}
+
+		loadBalancerMetadata, err := ims.getLoadBalancerMetadata()
+		if err != nil || loadBalancerMetadata == nil || loadBalancerMetadata.LoadBalancer == nil {
+			// Log a warning since loadbalancer metadata may not be available when the VM
+			// is not in standard LoadBalancer backend address pool.
+			klog.V(4).Infof("Warning: failed to get loadbalancer metadata: %v", err)
+			return instanceMetadata, nil
+		}
+
+		publicIPs := loadBalancerMetadata.LoadBalancer.PublicIPAddresses
+		if len(netInterface.IPV4.IPAddress) > 0 && len(netInterface.IPV4.IPAddress[0].PrivateIP) > 0 {
+			for _, pip := range publicIPs {
+				if pip.PrivateIPAddress == netInterface.IPV4.IPAddress[0].PrivateIP {
+					netInterface.IPV4.IPAddress[0].PublicIP = pip.FrontendIPAddress
+					break
+				}
+			}
+		}
+		if len(netInterface.IPV6.IPAddress) > 0 && len(netInterface.IPV6.IPAddress[0].PrivateIP) > 0 {
+			for _, pip := range publicIPs {
+				if pip.PrivateIPAddress == netInterface.IPV6.IPAddress[0].PrivateIP {
+					netInterface.IPV6.IPAddress[0].PublicIP = pip.FrontendIPAddress
+					break
+				}
+			}
+		}
+	}
+
+	return instanceMetadata, nil
+}
+
+func (ims *InstanceMetadataService) getInstanceMetadata(key string) (*InstanceMetadata, error) {
+	req, err := http.NewRequest("GET", ims.imdsServer+imdsInstanceURI, nil)
 	if err != nil {
 		return nil, err
 	}
@@ -115,7 +181,7 @@ func (ims *InstanceMetadataService) getInstanceMetadata(key string) (interface{}
 
 	q := req.URL.Query()
 	q.Add("format", "json")
-	q.Add("api-version", "2019-03-11")
+	q.Add("api-version", imdsInstanceAPIVersion)
 	req.URL.RawQuery = q.Encode()
 
 	client := &http.Client{}
@@ -143,6 +209,44 @@ func (ims *InstanceMetadataService) getInstanceMetadata(key string) (interface{}
 	return &obj, nil
 }
 
+func (ims *InstanceMetadataService) getLoadBalancerMetadata() (*LoadBalancerMetadata, error) {
+	req, err := http.NewRequest("GET", ims.imdsServer+imdsLoadBalancerURI, 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", "json")
+	q.Add("api-version", imdsLoadBalancerAPIVersion)
+	req.URL.RawQuery = q.Encode()
+
+	client := &http.Client{}
+	resp, err := client.Do(req)
+	if err != nil {
+		return nil, err
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusOK {
+		return nil, fmt.Errorf("failure of getting loadbalancer metadata with response %q", resp.Status)
+	}
+
+	data, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return nil, err
+	}
+
+	obj := LoadBalancerMetadata{}
+	err = json.Unmarshal(data, &obj)
+	if err != nil {
+		return nil, err
+	}
+
+	return &obj, nil
+}
+
 // GetMetadata gets instance metadata from cache.
 // crt determines if we can get data from stalled cache/need fresh if cache expired.
 func (ims *InstanceMetadataService) GetMetadata(crt azcache.AzureCacheReadType) (*InstanceMetadata, error) {


Test patch:
diff --git a/pkg/provider/azure_instances_test.go b/pkg/provider/azure_instances_test.go
index 13cad4e27..5fab461b6 100644
--- a/pkg/provider/azure_instances_test.go
+++ b/pkg/provider/azure_instances_test.go
@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"strings"
 	"testing"
 
 	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2020-12-01/compute"
@@ -399,6 +400,7 @@ func TestNodeAddresses(t *testing.T) {
 		},
 	}
 	metadataTemplate := `{"compute":{"name":"%s"},"network":{"interface":[{"ipv4":{"ipAddress":[{"privateIpAddress":"%s","publicIpAddress":"%s"}]},"ipv6":{"ipAddress":[{"privateIpAddress":"%s","publicIpAddress":"%s"}]}}]}}`
+	loadbalancerTemplate := `{"loadbalancer": {"publicIpAddresses": [{"frontendIpAddress": "%s","privateIpAddress": "%s"},{"frontendIpAddress": "%s","privateIpAddress": "%s"}]}}`
 
 	testcases := []struct {
 		name                string
@@ -410,6 +412,7 @@ func TestNodeAddresses(t *testing.T) {
 		ipV6                string
 		ipV4Public          string
 		ipV6Public          string
+		loadBalancerSku     string
 		expectedAddress     []v1.NodeAddress
 		useInstanceMetadata bool
 		useCustomImsCache   bool
@@ -484,7 +487,7 @@ func TestNodeAddresses(t *testing.T) {
 			expectedAddress: expectedNodeAddress,
 		},
 		{
-			name:                "NodeAddresses should get IP addresses from local if node's name is equal to metadataName",
+			name:                "NodeAddresses should get IP addresses from local IMDS if node's name is equal to metadataName",
 			nodeName:            "vm1",
 			metadataName:        "vm1",
 			vmType:              vmTypeStandard,
@@ -492,6 +495,41 @@ func TestNodeAddresses(t *testing.T) {
 			ipV4Public:          "192.168.1.12",
 			ipV6:                "1111:11111:00:00:1111:1111:000:111",
 			ipV6Public:          "2222:22221:00:00:2222:2222:000:111",
+			loadBalancerSku:     "basic",
+			useInstanceMetadata: true,
+			expectedAddress: []v1.NodeAddress{
+				{
+					Type:    v1.NodeHostName,
+					Address: "vm1",
+				},
+				{
+					Type:    v1.NodeInternalIP,
+					Address: "10.240.0.1",
+				},
+				{
+					Type:    v1.NodeExternalIP,
+					Address: "192.168.1.12",
+				},
+				{
+					Type:    v1.NodeInternalIP,
+					Address: "1111:11111:00:00:1111:1111:000:111",
+				},
+				{
+					Type:    v1.NodeExternalIP,
+					Address: "2222:22221:00:00:2222:2222:000:111",
+				},
+			},
+		},
+		{
+			name:                "NodeAddresses should get IP addresses from local IMDS for standard LoadBalancer if node's name is equal to metadataName",
+			nodeName:            "vm1",
+			metadataName:        "vm1",
+			vmType:              vmTypeStandard,
+			ipV4:                "10.240.0.1",
+			ipV4Public:          "192.168.1.12",
+			ipV6:                "1111:11111:00:00:1111:1111:000:111",
+			ipV6Public:          "2222:22221:00:00:2222:2222:000:111",
+			loadBalancerSku:     "standard",
 			useInstanceMetadata: true,
 			expectedAddress: []v1.NodeAddress{
 				{
@@ -533,10 +571,19 @@ func TestNodeAddresses(t *testing.T) {
 
 		mux := http.NewServeMux()
 		mux.Handle("/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			if strings.Contains(r.RequestURI, imdsLoadBalancerURI) {
+				fmt.Fprintf(w, loadbalancerTemplate, test.ipV4Public, test.ipV4, test.ipV6Public, test.ipV6)
+				return
+			}
+
 			if test.metadataTemplate != "" {
 				fmt.Fprintf(w, test.metadataTemplate)
 			} else {
-				fmt.Fprintf(w, metadataTemplate, test.metadataName, test.ipV4, test.ipV4Public, test.ipV6, test.ipV6Public)
+				if test.loadBalancerSku == "standard" {
+					fmt.Fprintf(w, metadataTemplate, test.metadataName, test.ipV4, "", test.ipV6, "")
+				} else {
+					fmt.Fprintf(w, metadataTemplate, test.metadataName, test.ipV4, test.ipV4Public, test.ipV6, test.ipV6Public)
+				}
 			}
 		}))
 		go func() {