cloud provider: remove redundant implementation of InstanceMetadataByProviderID since instanceV2 is disabled for all providers

Signed-off-by: Andrew Sy Kim <kim.andrewsy@gmail.com>
This commit is contained in:
Andrew Sy Kim 2020-07-30 15:29:25 -04:00
parent cc1e0d420c
commit 82bdba9907
9 changed files with 5 additions and 534 deletions

View File

@ -1395,6 +1395,7 @@ func (c *Cloud) Instances() (cloudprovider.Instances, bool) {
}
// InstancesV2 returns an implementation of InstancesV2 for Amazon Web Services.
// TODO: implement ONLY for external cloud provider
func (c *Cloud) InstancesV2() (cloudprovider.InstancesV2, bool) {
return nil, false
}
@ -1670,31 +1671,6 @@ 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) {
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.
func (c *Cloud) InstanceID(ctx context.Context, nodeName types.NodeName) (string, error) {
// In the future it is possible to also return an endpoint as:

View File

@ -681,23 +681,6 @@ 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

View File

@ -665,6 +665,7 @@ func (az *Cloud) Instances() (cloudprovider.Instances, bool) {
}
// InstancesV2 returns an instancesV2 interface. Also returns true if the interface is supported, false otherwise.
// TODO: implement ONLY for external cloud provider
func (az *Cloud) InstancesV2() (cloudprovider.InstancesV2, bool) {
return nil, false
}

View File

@ -239,80 +239,6 @@ func (az *Cloud) InstanceShutdownByProviderID(ctx context.Context, providerID st
return status == vmPowerStateStopped || status == vmPowerStateDeallocated || status == vmPowerStateDeallocating, nil
}
// InstanceMetadataByProviderID returns metadata of the specified instance.
// InstanceMetadataByProviderID is part of InstancesV2 interface and is only used in cloud node-controller.
func (az *Cloud) InstanceMetadataByProviderID(ctx context.Context, providerID string) (*cloudprovider.InstanceMetadata, error) {
if providerID == "" {
return nil, errNodeNotInitialized
}
// Returns nil for unmanaged nodes because azure cloud provider couldn't fetch information for them.
if az.IsNodeUnmanagedByProviderID(providerID) {
klog.V(4).Infof("NodeAddressesByProviderID: omitting unmanaged node %q", providerID)
return nil, nil
}
nodeName, err := az.vmSet.GetNodeNameByProviderID(providerID)
if err != nil {
return nil, err
}
md := &cloudprovider.InstanceMetadata{}
md.ProviderID = providerID
if az.UseInstanceMetadata {
metadata, err := az.metadata.GetMetadata(azcache.CacheReadTypeDefault)
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")
}
// Get instance metadata from IMDS for local instance.
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
}
// Get instance metadata from ARM API when UseInstanceMetadata is disabled.
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) {
var err error
nodeName := mapNodeNameToVMName(name)

View File

@ -571,328 +571,6 @@ func TestNodeAddresses(t *testing.T) {
}
}
func TestInstanceMetadataByProviderID(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
cloud := GetTestCloud(ctrl)
expectedVM := compute.VirtualMachine{
Name: to.StringPtr("vm1"),
VirtualMachineProperties: &compute.VirtualMachineProperties{
HardwareProfile: &compute.HardwareProfile{
VMSize: compute.VirtualMachineSizeTypesStandardA0,
},
NetworkProfile: &compute.NetworkProfile{
NetworkInterfaces: &[]compute.NetworkInterfaceReference{
{
NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{
Primary: to.BoolPtr(true),
},
ID: to.StringPtr("/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces/nic"),
},
},
},
},
}
expectedPIP := network.PublicIPAddress{
ID: to.StringPtr("/subscriptions/subscriptionID/resourceGroups/rg/providers/Microsoft.Network/publicIPAddresses/pip1"),
PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{
IPAddress: to.StringPtr("192.168.1.12"),
},
}
expectedInterface := network.Interface{
InterfacePropertiesFormat: &network.InterfacePropertiesFormat{
IPConfigurations: &[]network.InterfaceIPConfiguration{
{
InterfaceIPConfigurationPropertiesFormat: &network.InterfaceIPConfigurationPropertiesFormat{
PrivateIPAddress: to.StringPtr("172.1.0.3"),
PublicIPAddress: &expectedPIP,
},
},
},
},
}
nodeAddressFromAPI := []v1.NodeAddress{
{
Type: v1.NodeInternalIP,
Address: "172.1.0.3",
},
{
Type: v1.NodeHostName,
Address: "vm1",
},
{
Type: v1.NodeExternalIP,
Address: "192.168.1.12",
},
}
nodeAddressFromLocal := []v1.NodeAddress{
{
Type: v1.NodeHostName,
Address: "vm1",
},
{
Type: v1.NodeInternalIP,
Address: "10.240.0.1",
},
{
Type: v1.NodeExternalIP,
Address: "192.168.1.121",
},
{
Type: v1.NodeInternalIP,
Address: "1111:11111:00:00:1111:1111:000:111",
},
{
Type: v1.NodeExternalIP,
Address: "2222:22221:00:00:2222:2222:000:111",
},
}
providerID := "azure:///subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/vm1"
metadataTemplate := `{"compute":{"name":"%s","vmsize":"%s","subscriptionId":"subscription","resourceGroupName":"rg"},"network":{"interface":[{"ipv4":{"ipAddress":[{"privateIpAddress":"%s","publicIpAddress":"%s"}]},"ipv6":{"ipAddress":[{"privateIpAddress":"%s","publicIpAddress":"%s"}]}}]}}`
testcases := []struct {
name string
ipV4 string
ipV6 string
ipV4Public string
ipV6Public string
providerID string
metadataName string
metadataTemplate string
vmType string
vmSize string
expectedMetadata *cloudprovider.InstanceMetadata
useCustomImsCache bool
useInstanceMetadata bool
nilVMSet bool
expectedErrMsg error
}{
{
name: "InstanceMetadataByProviderID should report error if providerID is null",
expectedErrMsg: fmt.Errorf("providerID is empty, the node is not initialized yet"),
},
{
name: "InstanceMetadataByProviderID should return nil if the node is unmanaged",
providerID: "baremental-node",
expectedMetadata: nil,
},
{
name: "InstanceMetadataByProviderID should report error if providerID is invalid",
providerID: "azure:///subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Compute/virtualMachine/vm3",
vmType: vmTypeStandard,
useInstanceMetadata: true,
expectedErrMsg: fmt.Errorf("error splitting providerID"),
},
{
name: "InstanceMetadataByProviderID should report error if metadata.Compute is nil",
providerID: providerID,
useInstanceMetadata: true,
metadataTemplate: `{"network":{"interface":[]}}`,
expectedErrMsg: fmt.Errorf("failure of getting instance metadata"),
},
{
name: "InstanceMetadataByProviderID should report error if metadata.Network is nil",
providerID: providerID,
useInstanceMetadata: true,
metadataTemplate: `{"compute":{}}`,
expectedErrMsg: fmt.Errorf("failure of getting instance metadata"),
},
{
name: "InstanceMetadataByProviderID should report error when IPs are empty",
metadataName: "vm1",
vmSize: "Standard_A0",
providerID: providerID,
useInstanceMetadata: true,
expectedErrMsg: fmt.Errorf("get empty IP addresses from instance metadata service"),
},
{
name: "InstanceMetadataByProviderID should report error if node is not local instance and doesn't exist",
providerID: "azure:///subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/vm2",
vmType: vmTypeStandard,
useInstanceMetadata: true,
expectedErrMsg: fmt.Errorf("instance not found"),
},
{
name: "InstanceMetadataByProviderID should report error if node doesn't exist when cloud.useInstanceMetadata is false",
metadataName: "vm2",
providerID: "azure:///subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/vm2",
vmType: vmTypeStandard,
expectedErrMsg: fmt.Errorf("instance not found"),
},
{
name: "InstanceMetadataByProviderID should report error if node doesn't exist and metadata.Compute.VMSize is nil",
metadataName: "vm2",
providerID: "azure:///subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/vm2",
vmType: vmTypeStandard,
useInstanceMetadata: true,
expectedErrMsg: fmt.Errorf("instance not found"),
},
{
name: "InstanceMetadataByProviderID should report error if node don't have primary nic",
providerID: "azure:///subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/vm4",
vmType: vmTypeStandard,
useInstanceMetadata: true,
expectedErrMsg: fmt.Errorf("timed out waiting for the condition"),
},
{
name: "InstanceMetadataByProviderID should report error if node don't have primary nic when cloud.useInstanceMetadata is false",
metadataName: "vm4",
providerID: "azure:///subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/vm4",
vmType: vmTypeStandard,
expectedErrMsg: fmt.Errorf("timed out waiting for the condition"),
},
{
name: "InstanceMetadataByProviderID should report error when invoke GetMetadata",
metadataName: "vm1",
providerID: providerID,
vmType: vmTypeStandard,
useInstanceMetadata: true,
useCustomImsCache: true,
expectedErrMsg: fmt.Errorf("getError"),
},
{
name: "InstanceMetadataByProviderID should get metadata from Azure API if cloud.UseInstanceMetadata is false",
metadataName: "vm1",
providerID: providerID,
vmType: vmTypeStandard,
expectedMetadata: &cloudprovider.InstanceMetadata{
ProviderID: providerID,
Type: "Standard_A0",
NodeAddresses: nodeAddressFromAPI,
},
},
{
name: "InstanceMetadataByProviderID should get metadata from Azure API if node is not local instance",
metadataName: "vm3",
providerID: providerID,
vmType: vmTypeStandard,
useInstanceMetadata: true,
expectedMetadata: &cloudprovider.InstanceMetadata{
ProviderID: providerID,
Type: "Standard_A0",
NodeAddresses: nodeAddressFromAPI,
},
},
{
name: "InstanceMetadataByProviderID should get IP addresses from local if node is local instance",
metadataName: "vm1",
providerID: providerID,
vmType: vmTypeStandard,
vmSize: "Standard_A1",
ipV4: "10.240.0.1",
ipV4Public: "192.168.1.121",
ipV6: "1111:11111:00:00:1111:1111:000:111",
ipV6Public: "2222:22221:00:00:2222:2222:000:111",
useInstanceMetadata: true,
expectedMetadata: &cloudprovider.InstanceMetadata{
ProviderID: providerID,
Type: "Standard_A1",
NodeAddresses: nodeAddressFromLocal,
},
},
{
name: "InstanceMetadataByProviderID should get IP addresses from local and get InstanceType from API if node is local instance and metadata.Compute.VMSize is nil",
metadataName: "vm1",
providerID: providerID,
vmType: vmTypeStandard,
ipV4: "10.240.0.1",
ipV4Public: "192.168.1.121",
ipV6: "1111:11111:00:00:1111:1111:000:111",
ipV6Public: "2222:22221:00:00:2222:2222:000:111",
useInstanceMetadata: true,
expectedMetadata: &cloudprovider.InstanceMetadata{
ProviderID: providerID,
Type: "Standard_A0",
NodeAddresses: nodeAddressFromLocal,
},
},
}
for _, test := range testcases {
if test.nilVMSet {
cloud.vmSet = nil
} else {
cloud.vmSet = newAvailabilitySet(cloud)
}
cloud.Config.VMType = test.vmType
cloud.Config.UseInstanceMetadata = test.useInstanceMetadata
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) {
if test.metadataTemplate != "" {
fmt.Fprintf(w, test.metadataTemplate)
} else {
fmt.Fprint(w, fmt.Sprintf(metadataTemplate, test.metadataName, test.vmSize, 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)
}
if test.useCustomImsCache {
cloud.metadata.imsCache, err = azcache.NewTimedcache(metadataCacheTTL, func(key string) (interface{}, error) {
return nil, fmt.Errorf("getError")
})
if err != nil {
t.Errorf("Test [%s] unexpected error: %v", test.name, err)
}
}
mockVMClient := cloud.VirtualMachinesClient.(*mockvmclient.MockInterface)
mockVMClient.EXPECT().Get(gomock.Any(), cloud.ResourceGroup, "vm1", gomock.Any()).Return(expectedVM, nil).AnyTimes()
mockVMClient.EXPECT().Get(gomock.Any(), cloud.ResourceGroup, "vm2", gomock.Any()).Return(compute.VirtualMachine{}, &retry.Error{HTTPStatusCode: http.StatusNotFound, RawError: cloudprovider.InstanceNotFound}).AnyTimes()
mockVMClient.EXPECT().Get(gomock.Any(), cloud.ResourceGroup, "vm4", gomock.Any()).Return(compute.VirtualMachine{
Name: to.StringPtr("vm4"),
VirtualMachineProperties: &compute.VirtualMachineProperties{
HardwareProfile: &compute.HardwareProfile{
VMSize: compute.VirtualMachineSizeTypesStandardA0,
},
NetworkProfile: &compute.NetworkProfile{
NetworkInterfaces: &[]compute.NetworkInterfaceReference{
{
NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{
Primary: to.BoolPtr(false),
},
ID: to.StringPtr("/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces/nic1"),
},
{
NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{
Primary: to.BoolPtr(false),
},
ID: to.StringPtr("/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces/nic2"),
},
},
},
},
}, nil).AnyTimes()
mockPublicIPAddressesClient := cloud.PublicIPAddressesClient.(*mockpublicipclient.MockInterface)
mockPublicIPAddressesClient.EXPECT().Get(gomock.Any(), cloud.ResourceGroup, "pip1", gomock.Any()).Return(expectedPIP, nil).AnyTimes()
mockInterfaceClient := cloud.InterfacesClient.(*mockinterfaceclient.MockInterface)
mockInterfaceClient.EXPECT().Get(gomock.Any(), cloud.ResourceGroup, "nic", gomock.Any()).Return(expectedInterface, nil).AnyTimes()
md, err := cloud.InstanceMetadataByProviderID(context.Background(), test.providerID)
assert.Equal(t, test.expectedErrMsg, err, test.name)
assert.Equal(t, test.expectedMetadata, md, test.name)
}
}
func TestIsCurrentInstance(t *testing.T) {
cloud := &Cloud{
Config: Config{

View File

@ -660,6 +660,7 @@ func (g *Cloud) Instances() (cloudprovider.Instances, bool) {
}
// InstancesV2 returns an implementation of InstancesV2 for Google Compute Engine.
// TODO: implement ONLY for external cloud provider
func (g *Cloud) InstancesV2() (cloudprovider.InstancesV2, bool) {
return nil, false
}

View File

@ -210,36 +210,6 @@ 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) {
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)

View File

@ -63,6 +63,7 @@ func (os *OpenStack) Instances() (cloudprovider.Instances, bool) {
}
// InstancesV2 returns an implementation of InstancesV2 for OpenStack.
// TODO: implement ONLY for external cloud provider
func (os *OpenStack) InstancesV2() (cloudprovider.InstancesV2, bool) {
return nil, false
}
@ -157,37 +158,6 @@ 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) {
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.
func (os *OpenStack) InstanceID() (string, error) {
if len(os.localInstanceID) == 0 {

View File

@ -561,6 +561,7 @@ func (vs *VSphere) Instances() (cloudprovider.Instances, bool) {
}
// InstancesV2 returns an implementation of InstancesV2 for vSphere.
// TODO: implement ONLY for external cloud provider
func (vs *VSphere) InstancesV2() (cloudprovider.InstancesV2, bool) {
return nil, false
}
@ -797,41 +798,6 @@ 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) {
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.
func (vs *VSphere) InstanceID(ctx context.Context, nodeName k8stypes.NodeName) (string, error) {