From cc1e0d420c21b78b223855b6b09a66362497f9e3 Mon Sep 17 00:00:00 2001 From: Andrew Sy Kim Date: Thu, 30 Jul 2020 15:28:57 -0400 Subject: [PATCH 1/3] cloud provider: remove provider ID references and improve documentation Signed-off-by: Andrew Sy Kim --- staging/src/k8s.io/cloud-provider/cloud.go | 47 ++++++++++++++----- .../controllers/node/node_controller.go | 20 ++++---- .../src/k8s.io/cloud-provider/fake/fake.go | 21 +++++++-- 3 files changed, 61 insertions(+), 27 deletions(-) diff --git a/staging/src/k8s.io/cloud-provider/cloud.go b/staging/src/k8s.io/cloud-provider/cloud.go index c39cf463d0f..98fb5c347c8 100644 --- a/staging/src/k8s.io/cloud-provider/cloud.go +++ b/staging/src/k8s.io/cloud-provider/cloud.go @@ -49,8 +49,11 @@ type Interface interface { LoadBalancer() (LoadBalancer, bool) // Instances returns an instances interface. Also returns true if the interface is supported, false otherwise. Instances() (Instances, bool) - // InstancesV2 is an implementation for instances only used by cloud node-controller now. + // InstancesV2 is an implementation for instances and should only be implemented by external cloud providers. + // Implementing InstancesV2 is behaviorally identical to Instances but is optimized to significantly reduce + // API calls to the cloud provider when registering and syncing nodes. // Also returns true if the interface is supported, false otherwise. + // WARNING: InstancesV2 is an experimental interface and is subject to change in v1.20. InstancesV2() (InstancesV2, bool) // Zones returns a zones interface. Also returns true if the interface is supported, false otherwise. Zones() (Zones, bool) @@ -189,15 +192,20 @@ type Instances interface { InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) } -// InstancesV2 is an abstract, pluggable interface for sets of instances. -// Unlike Instances, it is only used by cloud node-controller now. +// InstancesV2 is an abstract, pluggable interface for cloud provider instances. +// Unlike the Instances interface, it is designed for external cloud providers and should only be used by them. +// WARNING: InstancesV2 is an experimental interface and is subject to change in v1.20. type InstancesV2 interface { - // InstanceExistsByProviderID returns true if the instance for the given provider exists. - 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) + // InstanceExists returns true if the instance for the given node exists according to the cloud provider. + // Use the node.name or node.spec.providerID field to find the node in the cloud provider. + InstanceExists(ctx context.Context, node *v1.Node) (bool, error) + // InstanceShutdown returns true if the instance is shutdown according to the cloud provider. + // Use the node.name or node.spec.providerID field to find the node in the cloud provider. + InstanceShutdown(ctx context.Context, node *v1.Node) (bool, error) + // InstanceMetadata returns the instance's metadata. The values returned in InstanceMetadata are + // translated into specific fields in the Node object on registration. + // Use the node.name or node.spec.providerID field to find the node in the cloud provider. + InstanceMetadata(ctx context.Context, node *v1.Node) (*InstanceMetadata, error) } // Route is a representation of an advanced routing rule. @@ -265,12 +273,25 @@ type PVLabeler interface { GetLabelsForVolume(ctx context.Context, pv *v1.PersistentVolume) (map[string]string, error) } -// InstanceMetadata contains metadata about the specific instance. +// InstanceMetadata contains metadata about a specific instance. +// Values returned in InstanceMetadata are translated into specific fields in Node. type InstanceMetadata struct { - // ProviderID is provider's id that instance belongs to. + // ProviderID is a unique ID used to idenfitify an instance on the cloud provider. + // The ProviderID set here will be set on the node's spec.providerID field. + // The provider ID format can be set by the cloud provider but providers should + // ensure the format does not change in any incompatible way. + // + // The provider ID format used by existing cloud provider has been: + // :// + // Existing providers setting this field should preserve the existing format + // currently being set in node.spec.providerID. ProviderID string - // Type is instance's type. - Type string + // InstanceType is the instance's type. + // The InstanceType set here will be set using the following labels on the node object: + // * node.kubernetes.io/instance-type= + // * beta.kubernetes.io/instance-type= (DEPRECATED) + InstanceType string // NodeAddress contains information for the instance's address. + // The node addresses returned here will be set on the node's status.addresses field. NodeAddresses []v1.NodeAddress } diff --git a/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go b/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go index 83e0b1d9d64..de553979500 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go +++ b/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go @@ -225,7 +225,7 @@ func (cnc *CloudNodeController) updateNodeAddress(ctx context.Context, node *v1. instanceMetadataGetter := func(providerID string, node *v1.Node) (*cloudprovider.InstanceMetadata, error) { if instancesV2, ok := cnc.cloud.InstancesV2(); instancesV2 != nil && ok { - return instancesV2.InstanceMetadataByProviderID(ctx, providerID) + return instancesV2.InstanceMetadata(ctx, node) } // If InstancesV2 not implement, use Instances. @@ -435,9 +435,9 @@ func (cnc *CloudNodeController) getNodeModifiersFromCloudProvider(ctx context.Co providerID = node.Spec.ProviderID } - instanceMetadataGetter := func(providerID string, nodeName string) (*cloudprovider.InstanceMetadata, error) { + instanceMetadataGetter := func(providerID string, nodeName string, node *v1.Node) (*cloudprovider.InstanceMetadata, error) { if instancesV2, ok := cnc.cloud.InstancesV2(); instancesV2 != nil && ok { - return instancesV2.InstanceMetadataByProviderID(ctx, providerID) + return instancesV2.InstanceMetadata(ctx, node) } // If InstancesV2 not implement, use Instances. @@ -454,12 +454,12 @@ func (cnc *CloudNodeController) getNodeModifiersFromCloudProvider(ctx context.Co return nil, err } return &cloudprovider.InstanceMetadata{ - Type: instanceType, + InstanceType: instanceType, NodeAddresses: nodeAddresses, }, nil } - instanceMeta, err := instanceMetadataGetter(providerID, node.Name) + instanceMeta, err := instanceMetadataGetter(providerID, node.Name, node) if err != nil { return nil, err } @@ -470,15 +470,15 @@ func (cnc *CloudNodeController) getNodeModifiersFromCloudProvider(ctx context.Co return nil, errors.New("failed to find kubelet node IP from cloud provider") } - if instanceMeta.Type != "" { - klog.V(2).Infof("Adding node label from cloud provider: %s=%s", v1.LabelInstanceType, instanceMeta.Type) - klog.V(2).Infof("Adding node label from cloud provider: %s=%s", v1.LabelInstanceTypeStable, instanceMeta.Type) + if instanceMeta.InstanceType != "" { + klog.V(2).Infof("Adding node label from cloud provider: %s=%s", v1.LabelInstanceType, instanceMeta.InstanceType) + klog.V(2).Infof("Adding node label from cloud provider: %s=%s", v1.LabelInstanceTypeStable, instanceMeta.InstanceType) nodeModifiers = append(nodeModifiers, func(n *v1.Node) { if n.Labels == nil { n.Labels = map[string]string{} } - n.Labels[v1.LabelInstanceType] = instanceMeta.Type - n.Labels[v1.LabelInstanceTypeStable] = instanceMeta.Type + n.Labels[v1.LabelInstanceType] = instanceMeta.InstanceType + n.Labels[v1.LabelInstanceTypeStable] = instanceMeta.InstanceType }) } diff --git a/staging/src/k8s.io/cloud-provider/fake/fake.go b/staging/src/k8s.io/cloud-provider/fake/fake.go index 22d1a915d8d..1637dd868a9 100644 --- a/staging/src/k8s.io/cloud-provider/fake/fake.go +++ b/staging/src/k8s.io/cloud-provider/fake/fake.go @@ -303,14 +303,27 @@ 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) { +// InstanceExists returns true if the instance corresponding to a node still exists and is running. +// If false is returned with no error, the instance will be immediately deleted by the cloud controller manager. +func (f *Cloud) InstanceExists(ctx context.Context, node *v1.Node) (bool, error) { + f.addCall("instance-exists") + return f.ExistsByProviderID, f.ErrByProviderID +} + +// InstanceShutdown returns true if the instances is in safe state to detach volumes +func (f *Cloud) InstanceShutdown(ctx context.Context, node *v1.Node) (bool, error) { + f.addCall("instance-shutdown") + return f.NodeShutdown, f.ErrShutdownByProviderID +} + +// InstanceMetadata returns metadata of the specified instance. +func (f *Cloud) InstanceMetadata(ctx context.Context, node *v1.Node) (*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)], + ProviderID: node.Spec.ProviderID, + InstanceType: f.InstanceTypes[types.NodeName(node.Spec.ProviderID)], NodeAddresses: f.Addresses, }, f.Err } From 82bdba99070b5ab066b42ece420d4c1c8b4bf439 Mon Sep 17 00:00:00 2001 From: Andrew Sy Kim Date: Thu, 30 Jul 2020 15:29:25 -0400 Subject: [PATCH 2/3] cloud provider: remove redundant implementation of InstanceMetadataByProviderID since instanceV2 is disabled for all providers Signed-off-by: Andrew Sy Kim --- .../k8s.io/legacy-cloud-providers/aws/aws.go | 26 +- .../legacy-cloud-providers/aws/aws_test.go | 17 - .../legacy-cloud-providers/azure/azure.go | 1 + .../azure/azure_instances.go | 74 ---- .../azure/azure_instances_test.go | 322 ------------------ .../k8s.io/legacy-cloud-providers/gce/gce.go | 1 + .../gce/gce_instances.go | 30 -- .../openstack/openstack_instances.go | 32 +- .../legacy-cloud-providers/vsphere/vsphere.go | 36 +- 9 files changed, 5 insertions(+), 534 deletions(-) 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 ece2cf5597c..37da2ac9220 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go +++ b/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go @@ -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: 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 091f8df73c1..69564f6357c 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,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 diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go index 14032092ed4..fcccf578680 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go @@ -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 } 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 b06c16a618c..a0f19f85fbe 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 @@ -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) 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 b2a7a3a46b9..6e4a3cb8321 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 @@ -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{ diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce.go index e9077f93bc7..61d30b5b8e0 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce.go +++ b/staging/src/k8s.io/legacy-cloud-providers/gce/gce.go @@ -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 } 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 7cf3282307f..6a1e1acb9f9 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 @@ -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) 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 c1741f06933..48f9a65be78 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 @@ -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 { 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 d7b39151855..12da586427a 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go +++ b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go @@ -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) { From 46d522d0c8e94edb6d29606a28785e35259badd7 Mon Sep 17 00:00:00 2001 From: Andrew Sy Kim Date: Sun, 2 Aug 2020 09:34:53 -0400 Subject: [PATCH 3/3] cloud-node-lifecycle controller: add missing instancev2 calls for node exists and node shutdown A cloud provider should have the option to only implement the new InstanceV2 interface. Currently the cloud nodelifecycle controller only makes instancev1 calls when it should prefer instancev2 if supported and fallback to the existing instancev1 otherwise. Signed-off-by: Andrew Sy Kim --- .../controllers/node/node_controller.go | 4 +- .../node_lifecycle_controller.go | 27 +- .../node_lifecycle_controller_test.go | 231 ++++++++++++++++++ 3 files changed, 252 insertions(+), 10 deletions(-) diff --git a/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go b/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go index de553979500..7aa0e8f5b70 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go +++ b/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go @@ -108,7 +108,9 @@ func NewCloudNodeController( klog.Infof("Sending events to api server.") eventBroadcaster.StartRecordingToSink(&v1core.EventSinkImpl{Interface: kubeClient.CoreV1().Events("")}) - if _, ok := cloud.Instances(); !ok { + _, instancesSupported := cloud.Instances() + _, instancesV2Supported := cloud.InstancesV2() + if !instancesSupported && !instancesV2Supported { return nil, errors.New("cloud provider does not support instances") } diff --git a/staging/src/k8s.io/cloud-provider/controllers/nodelifecycle/node_lifecycle_controller.go b/staging/src/k8s.io/cloud-provider/controllers/nodelifecycle/node_lifecycle_controller.go index 3df72855a0b..332ee8bc4d8 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/nodelifecycle/node_lifecycle_controller.go +++ b/staging/src/k8s.io/cloud-provider/controllers/nodelifecycle/node_lifecycle_controller.go @@ -85,7 +85,9 @@ func NewCloudNodeLifecycleController( return nil, errors.New("no cloud provider provided") } - if _, ok := cloud.Instances(); !ok { + _, instancesSupported := cloud.Instances() + _, instancesV2Supported := cloud.InstancesV2() + if !instancesSupported && !instancesV2Supported { return nil, errors.New("cloud provider does not support instances") } @@ -118,12 +120,6 @@ func (c *CloudNodeLifecycleController) Run(stopCh <-chan struct{}) { // or shutdown. If deleted, it deletes the node resource. If shutdown it // applies a shutdown taint to the node func (c *CloudNodeLifecycleController) MonitorNodes() { - instances, ok := c.cloud.Instances() - if !ok { - utilruntime.HandleError(fmt.Errorf("failed to get instances from cloud provider")) - return - } - nodes, err := c.nodeLister.List(labels.Everything()) if err != nil { klog.Errorf("error listing nodes from cache: %s", err) @@ -148,7 +144,7 @@ func (c *CloudNodeLifecycleController) MonitorNodes() { // At this point the node has NotReady status, we need to check if the node has been removed // from the cloud provider. If node cannot be found in cloudprovider, then delete the node - exists, err := ensureNodeExistsByProviderID(context.TODO(), instances, node) + exists, err := ensureNodeExistsByProviderID(context.TODO(), c.cloud, node) if err != nil { klog.Errorf("error checking if node %s exists: %v", node.Name, err) continue @@ -195,6 +191,10 @@ func (c *CloudNodeLifecycleController) MonitorNodes() { // shutdownInCloudProvider returns true if the node is shutdown on the cloud provider func shutdownInCloudProvider(ctx context.Context, cloud cloudprovider.Interface, node *v1.Node) (bool, error) { + if instanceV2, ok := cloud.InstancesV2(); ok { + return instanceV2.InstanceShutdown(ctx, node) + } + instances, ok := cloud.Instances() if !ok { return false, errors.New("cloud provider does not support instances") @@ -210,7 +210,16 @@ func shutdownInCloudProvider(ctx context.Context, cloud cloudprovider.Interface, // ensureNodeExistsByProviderID checks if the instance exists by the provider id, // If provider id in spec is empty it calls instanceId with node name to get provider id -func ensureNodeExistsByProviderID(ctx context.Context, instances cloudprovider.Instances, node *v1.Node) (bool, error) { +func ensureNodeExistsByProviderID(ctx context.Context, cloud cloudprovider.Interface, node *v1.Node) (bool, error) { + if instanceV2, ok := cloud.InstancesV2(); ok { + return instanceV2.InstanceExists(ctx, node) + } + + instances, ok := cloud.Instances() + if !ok { + return false, errors.New("instances interface not supported in the cloud provider") + } + providerID := node.Spec.ProviderID if providerID == "" { var err error diff --git a/staging/src/k8s.io/cloud-provider/controllers/nodelifecycle/node_lifecycle_controller_test.go b/staging/src/k8s.io/cloud-provider/controllers/nodelifecycle/node_lifecycle_controller_test.go index b12fde0e943..8059fa0379f 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/nodelifecycle/node_lifecycle_controller_test.go +++ b/staging/src/k8s.io/cloud-provider/controllers/nodelifecycle/node_lifecycle_controller_test.go @@ -269,6 +269,237 @@ func Test_NodesDeleted(t *testing.T) { ExistsByProviderID: false, }, }, + { + name: "[instance2] node is not ready and does not exist", + existingNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + }, + expectedDeleted: true, + fakeCloud: &fakecloud.Cloud{ + EnableInstancesV2: true, + ExistsByProviderID: false, + }, + }, + { + name: "[instancev2] node is not ready and provider returns err", + existingNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Spec: v1.NodeSpec{ + ProviderID: "node0", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + }, + expectedNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Spec: v1.NodeSpec{ + ProviderID: "node0", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + }, + expectedDeleted: false, + fakeCloud: &fakecloud.Cloud{ + EnableInstancesV2: true, + ExistsByProviderID: false, + ErrByProviderID: errors.New("err!"), + }, + }, + { + name: "[instancev2] node is not ready but still exists", + existingNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Spec: v1.NodeSpec{ + ProviderID: "node0", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + }, + expectedNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Spec: v1.NodeSpec{ + ProviderID: "node0", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + }, + expectedDeleted: false, + fakeCloud: &fakecloud.Cloud{ + EnableInstancesV2: true, + ExistsByProviderID: true, + }, + }, + { + name: "[instancev2] node ready condition is unknown, node doesn't exist", + existingNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionUnknown, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + }, + expectedDeleted: true, + fakeCloud: &fakecloud.Cloud{ + EnableInstancesV2: true, + ExistsByProviderID: false, + }, + }, + { + name: "[instancev2] node ready condition is unknown, node exists", + existingNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionUnknown, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + }, + expectedNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionUnknown, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + }, + expectedDeleted: false, + fakeCloud: &fakecloud.Cloud{ + EnableInstancesV2: true, + NodeShutdown: false, + ExistsByProviderID: true, + ExtID: map[types.NodeName]string{ + types.NodeName("node0"): "foo://12345", + }, + }, + }, + { + name: "[instancev2] node is ready, but provider said it is deleted (maybe a bug in provider)", + existingNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Spec: v1.NodeSpec{ + ProviderID: "node0", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + }, + expectedNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Spec: v1.NodeSpec{ + ProviderID: "node0", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + }, + expectedDeleted: false, + fakeCloud: &fakecloud.Cloud{ + EnableInstancesV2: true, + ExistsByProviderID: false, + }, + }, } for _, testcase := range testcases {