From 260a9005a08c0bdfc7dbf46396a33fcc9a96d247 Mon Sep 17 00:00:00 2001 From: louisgong Date: Wed, 6 May 2020 10:21:00 +0800 Subject: [PATCH 1/6] define cloud-provider InstanceMetadata and InstanceMetadataByProviderID function --- pkg/volume/cinder/attacher_test.go | 4 ++++ staging/src/k8s.io/cloud-provider/cloud.go | 15 +++++++++++++++ staging/src/k8s.io/cloud-provider/fake/fake.go | 12 ++++++++++++ .../src/k8s.io/legacy-cloud-providers/aws/aws.go | 5 +++++ .../azure/azure_instances.go | 5 +++++ .../legacy-cloud-providers/gce/gce_instances.go | 5 +++++ .../openstack/openstack_instances.go | 5 +++++ .../legacy-cloud-providers/vsphere/vsphere.go | 5 +++++ 8 files changed, 56 insertions(+) diff --git a/pkg/volume/cinder/attacher_test.go b/pkg/volume/cinder/attacher_test.go index 00e85ede2c6..e08817e5995 100644 --- a/pkg/volume/cinder/attacher_test.go +++ b/pkg/volume/cinder/attacher_test.go @@ -734,6 +734,10 @@ func (instances *instances) InstanceShutdownByProviderID(ctx context.Context, pr return false, errors.New("unimplemented") } +func (instances *instances) InstanceMetadataByProviderID(ctx context.Context, providerID string) (*cloudprovider.InstanceMetadata, error) { + return nil, errors.New("unimplemented") +} + func (instances *instances) List(filter string) ([]types.NodeName, error) { return []types.NodeName{}, errors.New("Not implemented") } diff --git a/staging/src/k8s.io/cloud-provider/cloud.go b/staging/src/k8s.io/cloud-provider/cloud.go index de64419e298..35b59e74daa 100644 --- a/staging/src/k8s.io/cloud-provider/cloud.go +++ b/staging/src/k8s.io/cloud-provider/cloud.go @@ -163,6 +163,7 @@ type Instances interface { // ProviderID is a unique identifier of the node. This will not be called // from the node whose nodeaddresses are being queried. i.e. local metadata // services cannot be used in this method to obtain nodeaddresses + // Deprecated: Remove once all calls are migrated to InstanceMetadataByProviderID NodeAddressesByProviderID(ctx context.Context, providerID string) ([]v1.NodeAddress, error) // InstanceID returns the cloud provider ID of the node with the specified NodeName. // Note that if the instance does not exist, we must return ("", cloudprovider.InstanceNotFound) @@ -171,6 +172,7 @@ type Instances interface { // InstanceType returns the type of the specified instance. InstanceType(ctx context.Context, name types.NodeName) (string, error) // InstanceTypeByProviderID returns the type of the specified instance. + // Deprecated: Remove once all calls are migrated to InstanceMetadataByProviderID InstanceTypeByProviderID(ctx context.Context, providerID string) (string, error) // AddSSHKeyToAllInstances adds an SSH public key as a legal identity for all instances // expected format for the key is standard ssh-keygen format: @@ -181,9 +183,12 @@ type Instances interface { // InstanceExistsByProviderID returns true if the instance for the given provider exists. // If false is returned with no error, the instance will be immediately deleted by the cloud controller manager. // This method should still return true for instances that exist but are stopped/sleeping. + // Deprecated: Remove once all calls are migrated to InstanceMetadataByProviderID InstanceExistsByProviderID(ctx context.Context, providerID string) (bool, error) // InstanceShutdownByProviderID returns true if the instance is shutdown in cloudprovider InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) + // InstanceMetadataByProviderID returns the instance's metadata. + InstanceMetadataByProviderID(ctx context.Context, providerID string) (*InstanceMetadata, error) } // Route is a representation of an advanced routing rule. @@ -250,3 +255,13 @@ type Zones interface { type PVLabeler interface { GetLabelsForVolume(ctx context.Context, pv *v1.PersistentVolume) (map[string]string, error) } + +// InstanceMetadata contains metadata about the specific instance. +type InstanceMetadata struct { + // ProviderID is provider's id that instance belongs to. + ProviderID string + // Type is instance's type. + Type string + // NodeAddress contains information for the instance's address. + NodeAddresses []v1.NodeAddress +} diff --git a/staging/src/k8s.io/cloud-provider/fake/fake.go b/staging/src/k8s.io/cloud-provider/fake/fake.go index b473ceca6b3..858980558c1 100644 --- a/staging/src/k8s.io/cloud-provider/fake/fake.go +++ b/staging/src/k8s.io/cloud-provider/fake/fake.go @@ -292,6 +292,18 @@ func (f *Cloud) InstanceShutdownByProviderID(ctx context.Context, providerID str return f.NodeShutdown, f.ErrShutdownByProviderID } +// InstanceMetadataByProviderID returns metadata of the specified instance. +func (f *Cloud) InstanceMetadataByProviderID(ctx context.Context, providerID string) (*cloudprovider.InstanceMetadata, error) { + f.addCall("instance-metadata-by-provider-id") + f.addressesMux.Lock() + defer f.addressesMux.Unlock() + return &cloudprovider.InstanceMetadata{ + ProviderID: providerID, + Type: f.InstanceTypes[types.NodeName(providerID)], + NodeAddresses: f.Addresses, + }, f.Err +} + // List is a test-spy implementation of Instances.List. // It adds an entry "list" into the internal method call record. func (f *Cloud) List(filter string) ([]types.NodeName, error) { diff --git a/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go b/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go index 8e198fdb9c3..40ea45eb134 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go +++ b/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go @@ -1646,6 +1646,11 @@ func (c *Cloud) InstanceShutdownByProviderID(ctx context.Context, providerID str return false, nil } +// InstanceMetadataByProviderID returns metadata of the specified instance. +func (c *Cloud) InstanceMetadataByProviderID(ctx context.Context, providerID string) (*cloudprovider.InstanceMetadata, error) { + return nil, fmt.Errorf("unimplemented") +} + // InstanceID returns the cloud provider ID of the node with the specified nodeName. func (c *Cloud) InstanceID(ctx context.Context, nodeName types.NodeName) (string, error) { // In the future it is possible to also return an endpoint as: diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances.go index 86c30d44f5a..8f1b123e3b5 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances.go @@ -230,6 +230,11 @@ func (az *Cloud) InstanceShutdownByProviderID(ctx context.Context, providerID st return strings.ToLower(powerStatus) == vmPowerStateStopped || strings.ToLower(powerStatus) == vmPowerStateDeallocated, nil } +// InstanceMetadataByProviderID returns metadata of the specified instance. +func (az *Cloud) InstanceMetadataByProviderID(ctx context.Context, providerID string) (*cloudprovider.InstanceMetadata, error) { + return nil, fmt.Errorf("unimplemented") +} + func (az *Cloud) isCurrentInstance(name types.NodeName, metadataVMName string) (bool, error) { nodeName := mapNodeNameToVMName(name) diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_instances.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_instances.go index 71aa4c59e94..ec475ae87dc 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_instances.go +++ b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_instances.go @@ -161,6 +161,11 @@ func (g *Cloud) InstanceShutdownByProviderID(ctx context.Context, providerID str return false, cloudprovider.NotImplemented } +// InstanceMetadataByProviderID returns metadata of the specified instance. +func (g *Cloud) InstanceMetadataByProviderID(ctx context.Context, providerID string) (*cloudprovider.InstanceMetadata, error) { + return nil, fmt.Errorf("unimplemented") +} + // InstanceTypeByProviderID returns the cloudprovider instance type of the node // with the specified unique providerID This method will not be called from the // node that is requesting this ID. i.e. metadata service and other local diff --git a/staging/src/k8s.io/legacy-cloud-providers/openstack/openstack_instances.go b/staging/src/k8s.io/legacy-cloud-providers/openstack/openstack_instances.go index 3d817985da5..c354e9f1657 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/openstack/openstack_instances.go +++ b/staging/src/k8s.io/legacy-cloud-providers/openstack/openstack_instances.go @@ -152,6 +152,11 @@ func (i *Instances) InstanceShutdownByProviderID(ctx context.Context, providerID return false, nil } +// InstanceMetadataByProviderID returns metadata of the specified instance. +func (i *Instances) InstanceMetadataByProviderID(ctx context.Context, providerID string) (*cloudprovider.InstanceMetadata, error) { + return nil, fmt.Errorf("unimplemented") +} + // InstanceID returns the kubelet's cloud provider ID. func (os *OpenStack) InstanceID() (string, error) { if len(os.localInstanceID) == 0 { diff --git a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go index 9caf1d813c3..04331ee50d2 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go +++ b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go @@ -770,6 +770,11 @@ func (vs *VSphere) InstanceShutdownByProviderID(ctx context.Context, providerID return !isActive, nil } +// InstanceMetadataByProviderID returns metadata of the specified instance. +func (vs *VSphere) InstanceMetadataByProviderID(ctx context.Context, providerID string) (*cloudprovider.InstanceMetadata, error) { + return nil, fmt.Errorf("unimplemented") +} + // InstanceID returns the cloud provider ID of the node with the specified Name. func (vs *VSphere) InstanceID(ctx context.Context, nodeName k8stypes.NodeName) (string, error) { From da4b5e8dd998485a486b0e30d64648991f57d1a5 Mon Sep 17 00:00:00 2001 From: louisgong Date: Wed, 13 May 2020 11:23:54 +0800 Subject: [PATCH 2/6] implement aws InstanceMetadataByProviderID function --- .../k8s.io/legacy-cloud-providers/aws/aws.go | 22 ++++++++++++++++++- .../legacy-cloud-providers/aws/aws_test.go | 17 ++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go b/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go index 40ea45eb134..91ab6b5b4e9 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go +++ b/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go @@ -1648,7 +1648,27 @@ func (c *Cloud) InstanceShutdownByProviderID(ctx context.Context, providerID str // InstanceMetadataByProviderID returns metadata of the specified instance. func (c *Cloud) InstanceMetadataByProviderID(ctx context.Context, providerID string) (*cloudprovider.InstanceMetadata, error) { - return nil, fmt.Errorf("unimplemented") + instanceID, err := KubernetesInstanceID(providerID).MapToAWSInstanceID() + if err != nil { + return nil, err + } + + instance, err := describeInstance(c.ec2, instanceID) + if err != nil { + return nil, err + } + + // TODO ignore checking whether `*instance.State.Name == ec2.InstanceStateNameTerminated` here. + // If not behave as expected, add it. + addresses, err := extractNodeAddresses(instance) + if err != nil { + return nil, err + } + return &cloudprovider.InstanceMetadata{ + ProviderID: providerID, + Type: aws.StringValue(instance.InstanceType), + NodeAddresses: addresses, + }, nil } // InstanceID returns the cloud provider ID of the node with the specified nodeName. diff --git a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_test.go b/staging/src/k8s.io/legacy-cloud-providers/aws/aws_test.go index 1f8978270c5..bd045ecce92 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/aws/aws_test.go @@ -681,6 +681,23 @@ func TestNodeAddressesWithMetadata(t *testing.T) { } } +func TestInstanceMetadataByProviderID(t *testing.T) { + instance0 := makeInstance(0, "192.168.0.1", "1.2.3.4", "instance-same.ec2.internal", "instance-same.ec2.external", true) + aws1, _ := mockInstancesResp(&instance0, []*ec2.Instance{&instance0}) + // change node name so it uses the instance instead of metadata + aws1.selfAWSInstance.nodeName = "foo" + + md, err := aws1.InstanceMetadataByProviderID(context.TODO(), "/us-east-1a/i-0") + if err != nil { + t.Errorf("should not error when instance found") + } + if md.ProviderID != "/us-east-1a/i-0" || md.Type != "c3.large" { + t.Errorf("expect providerID %s get %s, expect type %s get %s", "/us-east-1a/i-0", md.ProviderID, "c3.large", md.Type) + } + testHasNodeAddress(t, md.NodeAddresses, v1.NodeInternalIP, "192.168.0.1") + testHasNodeAddress(t, md.NodeAddresses, v1.NodeExternalIP, "1.2.3.4") +} + func TestParseMetadataLocalHostname(t *testing.T) { tests := []struct { name string From 6a7bb312510f2e883c93b645d8ff835c15399d81 Mon Sep 17 00:00:00 2001 From: louisgong Date: Wed, 13 May 2020 11:24:31 +0800 Subject: [PATCH 3/6] implement azure InstanceMetadataByProviderID function --- .../azure/azure_instances.go | 250 ++++++++++++------ .../azure/azure_instances_test.go | 149 +++++++++++ 2 files changed, 312 insertions(+), 87 deletions(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances.go index 8f1b123e3b5..1fe7237c1b0 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances.go @@ -41,6 +41,26 @@ var ( errNodeNotInitialized = fmt.Errorf("providerID is empty, the node is not initialized yet") ) +func (az *Cloud) addressGetter(nodeName types.NodeName) ([]v1.NodeAddress, error) { + ip, publicIP, err := az.getIPForMachine(nodeName) + if err != nil { + klog.V(2).Infof("NodeAddresses(%s) abort backoff: %v", nodeName, err) + return nil, err + } + + addresses := []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: ip}, + {Type: v1.NodeHostName, Address: string(nodeName)}, + } + if len(publicIP) > 0 { + addresses = append(addresses, v1.NodeAddress{ + Type: v1.NodeExternalIP, + Address: publicIP, + }) + } + return addresses, nil +} + // NodeAddresses returns the addresses of the specified instance. func (az *Cloud) NodeAddresses(ctx context.Context, name types.NodeName) ([]v1.NodeAddress, error) { // Returns nil for unmanaged nodes because azure cloud provider couldn't fetch information for them. @@ -53,26 +73,6 @@ func (az *Cloud) NodeAddresses(ctx context.Context, name types.NodeName) ([]v1.N return nil, nil } - addressGetter := func(nodeName types.NodeName) ([]v1.NodeAddress, error) { - ip, publicIP, err := az.getIPForMachine(nodeName) - if err != nil { - klog.V(2).Infof("NodeAddresses(%s) abort backoff: %v", nodeName, err) - return nil, err - } - - addresses := []v1.NodeAddress{ - {Type: v1.NodeInternalIP, Address: ip}, - {Type: v1.NodeHostName, Address: string(name)}, - } - if len(publicIP) > 0 { - addresses = append(addresses, v1.NodeAddress{ - Type: v1.NodeExternalIP, - Address: publicIP, - }) - } - return addresses, nil - } - if az.UseInstanceMetadata { metadata, err := az.metadata.GetMetadata(azcache.CacheReadTypeDefault) if err != nil { @@ -91,59 +91,62 @@ func (az *Cloud) NodeAddresses(ctx context.Context, name types.NodeName) ([]v1.N // Not local instance, get addresses from Azure ARM API. if !isLocalInstance { if az.vmSet != nil { - return addressGetter(name) + return az.addressGetter(name) } // vmSet == nil indicates credentials are not provided. return nil, fmt.Errorf("no credentials provided for Azure cloud provider") } - 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.NodeHostName, Address: string(name)}, - } - if len(ipAddress.IPV4.IPAddress) > 0 && len(ipAddress.IPV4.IPAddress[0].PrivateIP) > 0 { - address := ipAddress.IPV4.IPAddress[0] - 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, - }) - } - } - if len(ipAddress.IPV6.IPAddress) > 0 && len(ipAddress.IPV6.IPAddress[0].PrivateIP) > 0 { - address := ipAddress.IPV6.IPAddress[0] - 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, - }) - } - } - - if len(addresses) == 1 { - // No IP addresses is got from instance metadata service, clean up cache and report errors. - az.metadata.imsCache.Delete(metadataCacheKey) - return nil, fmt.Errorf("get empty IP addresses from instance metadata service") - } - - return addresses, nil + return az.getLocalInstanceNodeAddresses(metadata.Network.Interface, string(name)) } - return addressGetter(name) + return az.addressGetter(name) +} + +func (az *Cloud) getLocalInstanceNodeAddresses(netInterfaces []NetworkInterface, nodeName string) ([]v1.NodeAddress, error) { + if len(netInterfaces) == 0 { + return nil, fmt.Errorf("no interface is found for the instance") + } + + // Use ip address got from instance metadata. + netInterface := netInterfaces[0] + addresses := []v1.NodeAddress{ + {Type: v1.NodeHostName, Address: nodeName}, + } + if len(netInterface.IPV4.IPAddress) > 0 && len(netInterface.IPV4.IPAddress[0].PrivateIP) > 0 { + address := netInterface.IPV4.IPAddress[0] + 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, + }) + } + } + if len(netInterface.IPV6.IPAddress) > 0 && len(netInterface.IPV6.IPAddress[0].PrivateIP) > 0 { + address := netInterface.IPV6.IPAddress[0] + 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, + }) + } + } + + if len(addresses) == 1 { + // No IP addresses is got from instance metadata service, clean up cache and report errors. + az.metadata.imsCache.Delete(metadataCacheKey) + return nil, fmt.Errorf("get empty IP addresses from instance metadata service") + } + return addresses, nil } // NodeAddressesByProviderID returns the node addresses of an instances with the specified unique providerID @@ -232,7 +235,77 @@ func (az *Cloud) InstanceShutdownByProviderID(ctx context.Context, providerID st // InstanceMetadataByProviderID returns metadata of the specified instance. func (az *Cloud) InstanceMetadataByProviderID(ctx context.Context, providerID string) (*cloudprovider.InstanceMetadata, error) { - return nil, fmt.Errorf("unimplemented") + if providerID == "" { + return nil, errNodeNotInitialized + } + + nodeName, err := az.vmSet.GetNodeNameByProviderID(providerID) + if err != nil { + return nil, err + } + + // Returns "" for unmanaged nodes because azure cloud provider couldn't fetch information for them. + unmanaged, err := az.IsNodeUnmanaged(string(nodeName)) + if err != nil { + return nil, err + } + if unmanaged { + klog.V(4).Infof("InstanceType: omitting unmanaged node %q", string(nodeName)) + return nil, nil + } + + md := &cloudprovider.InstanceMetadata{} + md.ProviderID = providerID + if az.UseInstanceMetadata { + metadata, err := az.metadata.GetMetadata(azcache.CacheReadTypeUnsafe) + if err != nil { + return nil, err + } + + if metadata.Compute == nil || metadata.Network == nil { + return nil, fmt.Errorf("failure of getting instance metadata") + } + + isLocalInstance, err := az.isCurrentInstance(nodeName, metadata.Compute.Name) + if err != nil { + return nil, err + } + + // Not local instance, get metadata from Azure ARM API. + if !isLocalInstance { + if az.vmSet != nil { + if md.Type, err = az.vmSet.GetInstanceTypeByNodeName(string(nodeName)); err != nil { + return nil, err + } + if md.NodeAddresses, err = az.addressGetter(nodeName); err != nil { + return nil, err + } + return md, nil + } + // vmSet == nil indicates credentials are not provided. + return nil, fmt.Errorf("no credentials provided for Azure cloud provider") + } + + if metadata.Compute.VMSize != "" { + md.Type = metadata.Compute.VMSize + } else { + if md.Type, err = az.vmSet.GetInstanceTypeByNodeName(string(nodeName)); err != nil { + return nil, err + } + } + if md.NodeAddresses, err = az.getLocalInstanceNodeAddresses(metadata.Network.Interface, string(nodeName)); err != nil { + return nil, err + } + return md, nil + } + + if md.Type, err = az.vmSet.GetInstanceTypeByNodeName(string(nodeName)); err != nil { + return nil, err + } + if md.NodeAddresses, err = az.addressGetter(nodeName); err != nil { + return nil, err + } + return md, err } func (az *Cloud) isCurrentInstance(name types.NodeName, metadataVMName string) (bool, error) { @@ -292,32 +365,35 @@ func (az *Cloud) InstanceID(ctx context.Context, name types.NodeName) (string, e // vmSet == nil indicates credentials are not provided. return "", fmt.Errorf("no credentials provided for Azure cloud provider") } - - // Get resource group name and subscription ID. - resourceGroup := strings.ToLower(metadata.Compute.ResourceGroup) - subscriptionID := strings.ToLower(metadata.Compute.SubscriptionID) - - // Compose instanceID based on nodeName for standard instance. - if metadata.Compute.VMScaleSetName == "" { - return az.getStandardMachineID(subscriptionID, resourceGroup, nodeName), nil - } - - // Get scale set name and instanceID from vmName for vmss. - ssName, instanceID, err := extractVmssVMName(metadata.Compute.Name) - if err != nil { - if err == ErrorNotVmssInstance { - // Compose machineID for standard Node. - return az.getStandardMachineID(subscriptionID, resourceGroup, nodeName), nil - } - return "", err - } - // Compose instanceID based on ssName and instanceID for vmss instance. - return az.getVmssMachineID(subscriptionID, resourceGroup, ssName, instanceID), nil + return az.getLocalInstanceProviderID(metadata, nodeName) } return az.vmSet.GetInstanceIDByNodeName(nodeName) } +func (az *Cloud) getLocalInstanceProviderID(metadata *InstanceMetadata, nodeName string) (string, error) { + // Get resource group name and subscription ID. + resourceGroup := strings.ToLower(metadata.Compute.ResourceGroup) + subscriptionID := strings.ToLower(metadata.Compute.SubscriptionID) + + // Compose instanceID based on nodeName for standard instance. + if metadata.Compute.VMScaleSetName == "" { + return az.getStandardMachineID(subscriptionID, resourceGroup, nodeName), nil + } + + // Get scale set name and instanceID from vmName for vmss. + ssName, instanceID, err := extractVmssVMName(metadata.Compute.Name) + if err != nil { + if err == ErrorNotVmssInstance { + // Compose machineID for standard Node. + return az.getStandardMachineID(subscriptionID, resourceGroup, nodeName), nil + } + return "", err + } + // Compose instanceID based on ssName and instanceID for vmss instance. + return az.getVmssMachineID(subscriptionID, resourceGroup, ssName, instanceID), nil +} + // InstanceTypeByProviderID returns the cloudprovider instance type of the node with the specified unique providerID // This method will not be called from the node that is requesting this ID. i.e. metadata service // and other local methods cannot be used here diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances_test.go index 1f989c2ab94..ed364c5f835 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances_test.go @@ -57,6 +57,9 @@ func setTestVirtualMachines(c *Cloud, vmList map[string]string, isDataDisksFull }, } vm.VirtualMachineProperties = &compute.VirtualMachineProperties{ + HardwareProfile: &compute.HardwareProfile{ + VMSize: compute.VirtualMachineSizeTypesStandardA0, + }, InstanceView: &compute.VirtualMachineInstanceView{ Statuses: &status, }, @@ -395,6 +398,152 @@ func TestNodeAddresses(t *testing.T) { } } +func TestInstanceMetadataByProviderID(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + cloud := GetTestCloud(ctrl) + cloud.Config.UseInstanceMetadata = true + metadataTemplate := `{"compute":{"name":"%s","subscriptionId":"subscription","resourceGroupName":"rg"},"network":{"interface":[{"ipv4":{"ipAddress":[{"privateIpAddress":"%s","publicIpAddress":"%s"}]},"ipv6":{"ipAddress":[{"privateIpAddress":"%s","publicIpAddress":"%s"}]}}]}}` + + testcases := []struct { + name string + vmList []string + nodeName string + ipV4 string + ipV6 string + ipV4Public string + ipV6Public string + providerID string + expectedAddr []v1.NodeAddress + expectError bool + }{ + { + name: "NodeAddresses should get both ipV4 and ipV6 private addresses, InstanceID should get instanceID if node's name are equal to metadataName", + vmList: []string{"vm1"}, + nodeName: "vm1", + ipV4: "10.240.0.1", + ipV6: "1111:11111:00:00:1111:1111:000:111", + providerID: "/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/vm1", + expectedAddr: []v1.NodeAddress{ + { + Type: v1.NodeHostName, + Address: "vm1", + }, + { + Type: v1.NodeInternalIP, + Address: "10.240.0.1", + }, + { + Type: v1.NodeInternalIP, + Address: "1111:11111:00:00:1111:1111:000:111", + }, + }, + }, + { + name: "NodeAddresses should report error when IPs are empty", + nodeName: "vm1", + expectError: true, + }, + { + name: "NodeAddresses should get ipV4 private and public addresses, InstanceID should get instanceID from Azure API if node is not local instance", + vmList: []string{"vm2"}, + nodeName: "vm2", + providerID: "/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/vm2", + ipV4: "10.240.0.1", + ipV4Public: "9.9.9.9", + expectedAddr: []v1.NodeAddress{ + { + Type: v1.NodeHostName, + Address: "vm2", + }, + { + Type: v1.NodeInternalIP, + Address: "10.240.0.1", + }, + { + Type: v1.NodeExternalIP, + Address: "9.9.9.9", + }, + }, + }, + { + name: "InstanceID should report error if VM doesn't exist", + vmList: []string{"vm1"}, + nodeName: "vm3", + expectError: true, + }, + { + name: "NodeAddresses should get ipV6 private and public addresses", + nodeName: "vm1", + providerID: "/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/vm1", + ipV6: "1111:11111:00:00:1111:1111:000:111", + ipV6Public: "2222:22221:00:00:2222:2222:000:111", + expectedAddr: []v1.NodeAddress{ + { + Type: v1.NodeHostName, + Address: "vm1", + }, + { + Type: v1.NodeInternalIP, + Address: "1111:11111:00:00:1111:1111:000:111", + }, + { + Type: v1.NodeExternalIP, + Address: "2222:22221:00:00:2222:2222:000:111", + }, + }, + }, + } + + for _, test := range testcases { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Errorf("Test [%s] unexpected error: %v", test.name, err) + } + + mux := http.NewServeMux() + mux.Handle("/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, fmt.Sprintf(metadataTemplate, test.nodeName, test.ipV4, test.ipV4Public, test.ipV6, test.ipV6Public)) + })) + go func() { + http.Serve(listener, mux) + }() + defer listener.Close() + + 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] = "" + } + expectedVMs := setTestVirtualMachines(cloud, vmListWithPowerState, false) + mockVMsClient := cloud.VirtualMachinesClient.(*mockvmclient.MockInterface) + for _, vm := range expectedVMs { + mockVMsClient.EXPECT().Get(gomock.Any(), cloud.ResourceGroup, *vm.Name, gomock.Any()).Return(vm, nil).AnyTimes() + } + mockVMsClient.EXPECT().Get(gomock.Any(), cloud.ResourceGroup, "vm3", gomock.Any()).Return(compute.VirtualMachine{}, &retry.Error{HTTPStatusCode: http.StatusNotFound, RawError: cloudprovider.InstanceNotFound}).AnyTimes() + mockVMsClient.EXPECT().Update(gomock.Any(), cloud.ResourceGroup, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() + + md, err := cloud.InstanceMetadataByProviderID(context.Background(), test.providerID) + if test.expectError { + if err == nil { + t.Errorf("Test [%s] unexpected nil err", test.name) + } + } else { + if err != nil { + t.Errorf("Test [%s] unexpected error: %v", test.name, err) + } + } + + if len(test.expectedAddr) > 0 && !reflect.DeepEqual(md.NodeAddresses, test.expectedAddr) { + t.Errorf("Test [%s] unexpected ipAddresses: %s, expected %q", test.name, md.NodeAddresses, test.expectedAddr) + } + } +} + func TestIsCurrentInstance(t *testing.T) { cloud := &Cloud{ Config: Config{ From 030c4e7845637d81c5b751e5bacd4e3b8418f839 Mon Sep 17 00:00:00 2001 From: louisgong Date: Wed, 6 May 2020 17:08:41 +0800 Subject: [PATCH 4/6] implement gce InstanceMetadataByProviderID function --- .../gce/gce_instances.go | 52 ++++++++++++++----- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_instances.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_instances.go index ec475ae87dc..0026e24138e 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_instances.go +++ b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_instances.go @@ -124,17 +124,7 @@ func (g *Cloud) NodeAddressesByProviderID(ctx context.Context, providerID string return []v1.NodeAddress{}, fmt.Errorf("error while querying for providerID %q: %v", providerID, err) } - if len(instance.NetworkInterfaces) < 1 { - return []v1.NodeAddress{}, fmt.Errorf("could not find network interfaces for providerID %q", providerID) - } - networkInterface := instance.NetworkInterfaces[0] - - nodeAddresses := []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: networkInterface.NetworkIP}} - for _, config := range networkInterface.AccessConfigs { - nodeAddresses = append(nodeAddresses, v1.NodeAddress{Type: v1.NodeExternalIP, Address: config.NatIP}) - } - - return nodeAddresses, nil + return nodeAddressesFromInstance(instance) } // instanceByProviderID returns the cloudprovider instance of the node @@ -163,7 +153,45 @@ func (g *Cloud) InstanceShutdownByProviderID(ctx context.Context, providerID str // InstanceMetadataByProviderID returns metadata of the specified instance. func (g *Cloud) InstanceMetadataByProviderID(ctx context.Context, providerID string) (*cloudprovider.InstanceMetadata, error) { - return nil, fmt.Errorf("unimplemented") + timeoutCtx, cancel := context.WithTimeout(ctx, 1*time.Hour) + defer cancel() + + if providerID == "" { + return nil, fmt.Errorf("couldn't compute InstanceMetadata for empty providerID") + } + + _, zone, name, err := splitProviderID(providerID) + if err != nil { + return nil, err + } + + instance, err := g.c.Instances().Get(timeoutCtx, meta.ZonalKey(canonicalizeInstanceName(name), zone)) + if err != nil { + return nil, fmt.Errorf("error while querying for providerID %q: %v", providerID, err) + } + + addresses, err := nodeAddressesFromInstance(instance) + if err != nil { + return nil, err + } + return &cloudprovider.InstanceMetadata{ + ProviderID: providerID, + Type: lastComponent(instance.MachineType), + NodeAddresses: addresses, + }, nil +} + +func nodeAddressesFromInstance(instance *compute.Instance) ([]v1.NodeAddress, error) { + if len(instance.NetworkInterfaces) < 1 { + return nil, fmt.Errorf("could not find network interfaces for instanceID %q", instance.Id) + } + networkInterface := instance.NetworkInterfaces[0] + + nodeAddresses := []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: networkInterface.NetworkIP}} + for _, config := range networkInterface.AccessConfigs { + nodeAddresses = append(nodeAddresses, v1.NodeAddress{Type: v1.NodeExternalIP, Address: config.NatIP}) + } + return nodeAddresses, nil } // InstanceTypeByProviderID returns the cloudprovider instance type of the node From cf154e9d1f2327dc93878c2e881a55d0bc8f1bf9 Mon Sep 17 00:00:00 2001 From: louisgong Date: Wed, 13 May 2020 11:45:07 +0800 Subject: [PATCH 5/6] implement openstack InstanceMetadataByProviderID function --- .../openstack/openstack_instances.go | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/openstack/openstack_instances.go b/staging/src/k8s.io/legacy-cloud-providers/openstack/openstack_instances.go index c354e9f1657..8c10e50fe45 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/openstack/openstack_instances.go +++ b/staging/src/k8s.io/legacy-cloud-providers/openstack/openstack_instances.go @@ -154,7 +154,33 @@ func (i *Instances) InstanceShutdownByProviderID(ctx context.Context, providerID // InstanceMetadataByProviderID returns metadata of the specified instance. func (i *Instances) InstanceMetadataByProviderID(ctx context.Context, providerID string) (*cloudprovider.InstanceMetadata, error) { - return nil, fmt.Errorf("unimplemented") + if providerID == "" { + return nil, fmt.Errorf("couldn't compute InstanceMetadata for empty providerID") + } + + instanceID, err := instanceIDFromProviderID(providerID) + if err != nil { + return nil, err + } + srv, err := servers.Get(i.compute, instanceID).Extract() + if err != nil { + return nil, err + } + + instanceType, err := srvInstanceType(srv) + if err != nil { + return nil, err + } + addresses, err := nodeAddresses(srv) + if err != nil { + return nil, err + } + + return &cloudprovider.InstanceMetadata{ + ProviderID: providerID, + Type: instanceType, + NodeAddresses: addresses, + }, nil } // InstanceID returns the kubelet's cloud provider ID. From 24fe349ddf29ff599772cb9f3e640a8675cfa291 Mon Sep 17 00:00:00 2001 From: louisgong Date: Wed, 13 May 2020 11:45:29 +0800 Subject: [PATCH 6/6] implement vsphere InstanceMetadataByProviderID function --- .../legacy-cloud-providers/vsphere/vsphere.go | 56 ++++++++++++++++--- 1 file changed, 47 insertions(+), 9 deletions(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go index 04331ee50d2..6424b9fd59f 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go +++ b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go @@ -639,17 +639,25 @@ func (vs *VSphere) getVMFromNodeName(ctx context.Context, nodeName k8stypes.Node // NodeAddresses is an implementation of Instances.NodeAddresses. func (vs *VSphere) NodeAddresses(ctx context.Context, nodeName k8stypes.NodeName) ([]v1.NodeAddress, error) { - // Get local IP addresses if node is local node if vs.hostName == convertToString(nodeName) { - addrs, err := getLocalIP() - if err != nil { - return nil, err - } - // add the hostname address - nodehelpers.AddToNodeAddresses(&addrs, v1.NodeAddress{Type: v1.NodeHostName, Address: vs.hostName}) - return addrs, nil + return vs.getNodeAddressesFromLocalIP() } + return vs.getNodeAddressesFromVM(ctx, nodeName) +} +// getNodeAddressesFromLocalIP get local IP addresses if node is local node. +func (vs *VSphere) getNodeAddressesFromLocalIP() ([]v1.NodeAddress, error) { + addrs, err := getLocalIP() + if err != nil { + return nil, err + } + // add the hostname address + nodehelpers.AddToNodeAddresses(&addrs, v1.NodeAddress{Type: v1.NodeHostName, Address: vs.hostName}) + return addrs, nil +} + +// getNodeAddressesFromVM get vm IP addresses if node is vm. +func (vs *VSphere) getNodeAddressesFromVM(ctx context.Context, nodeName k8stypes.NodeName) ([]v1.NodeAddress, error) { if vs.cfg == nil { return nil, cloudprovider.InstanceNotFound } @@ -772,7 +780,37 @@ func (vs *VSphere) InstanceShutdownByProviderID(ctx context.Context, providerID // InstanceMetadataByProviderID returns metadata of the specified instance. func (vs *VSphere) InstanceMetadataByProviderID(ctx context.Context, providerID string) (*cloudprovider.InstanceMetadata, error) { - return nil, fmt.Errorf("unimplemented") + if providerID == "" { + return nil, fmt.Errorf("couldn't compute InstanceMetadata for empty providerID") + } + + // TODO dropped get nodeName by GetNodeNameFromProviderID here. If it not behave as expected, + // get nodeName by vm.GetNodeNameFromProviderID. + return vs.instanceMetadataByNodeName(ctx, convertToK8sType(providerID)) +} + +func (vs *VSphere) instanceMetadataByNodeName(ctx context.Context, nodeName k8stypes.NodeName) (*cloudprovider.InstanceMetadata, error) { + if vs.hostName == convertToString(nodeName) { + addresses, err := vs.getNodeAddressesFromLocalIP() + if err != nil { + return nil, err + } + return &cloudprovider.InstanceMetadata{ + ProviderID: vs.vmUUID, + Type: "", + NodeAddresses: addresses, + }, nil + } + + addresses, err := vs.getNodeAddressesFromVM(ctx, nodeName) + if err != nil { + return nil, err + } + return &cloudprovider.InstanceMetadata{ + ProviderID: vs.vmUUID, + Type: "", + NodeAddresses: addresses, + }, nil } // InstanceID returns the cloud provider ID of the node with the specified Name.