From f74b6100362ecff70f2a70350f9728a974e78074 Mon Sep 17 00:00:00 2001 From: Pengfei Ni Date: Mon, 22 Jul 2019 16:14:53 +0800 Subject: [PATCH] Report NodeNotInitialized error when providerId is empty string This could be happening when cloud-controller-manager is used. --- .../azure/azure_instances.go | 32 +++++++++++++++++++ .../azure/azure_zones.go | 4 +++ 2 files changed, 36 insertions(+) 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 ae6442ddb88..3c47d7f3208 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 @@ -34,6 +34,10 @@ const ( vmPowerStateDeallocated = "deallocated" ) +var ( + errNodeNotInitialized = fmt.Errorf("providerID is empty, the node is not initialized yet") +) + // 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. @@ -143,6 +147,10 @@ func (az *Cloud) NodeAddresses(ctx context.Context, name types.NodeName) ([]v1.N // 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 func (az *Cloud) NodeAddressesByProviderID(ctx context.Context, providerID string) ([]v1.NodeAddress, 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) @@ -160,6 +168,10 @@ func (az *Cloud) NodeAddressesByProviderID(ctx context.Context, providerID strin // InstanceExistsByProviderID returns true if the instance with the given provider id still exists and is running. // If false is returned with no error, the instance will be immediately deleted by the cloud controller manager. func (az *Cloud) InstanceExistsByProviderID(ctx context.Context, providerID string) (bool, error) { + if providerID == "" { + return false, errNodeNotInitialized + } + // Returns true for unmanaged nodes because azure cloud provider always assumes them exists. if az.IsNodeUnmanagedByProviderID(providerID) { klog.V(4).Infof("InstanceExistsByProviderID: assuming unmanaged node %q exists", providerID) @@ -187,13 +199,29 @@ func (az *Cloud) InstanceExistsByProviderID(ctx context.Context, providerID stri // InstanceShutdownByProviderID returns true if the instance is in safe state to detach volumes func (az *Cloud) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) { + if providerID == "" { + return false, nil + } + nodeName, err := az.vmSet.GetNodeNameByProviderID(providerID) if err != nil { + // returns false, because otherwise node is not deleted from cluster + // false means that it will continue to check InstanceExistsByProviderID + if err == cloudprovider.InstanceNotFound { + return false, nil + } + return false, err } powerStatus, err := az.vmSet.GetPowerStatusByNodeName(string(nodeName)) if err != nil { + // returns false, because otherwise node is not deleted from cluster + // false means that it will continue to check InstanceExistsByProviderID + if err == cloudprovider.InstanceNotFound { + return false, nil + } + return false, err } klog.V(5).Infof("InstanceShutdownByProviderID gets power status %q for node %q", powerStatus, nodeName) @@ -283,6 +311,10 @@ func (az *Cloud) InstanceID(ctx context.Context, name types.NodeName) (string, e // 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 func (az *Cloud) InstanceTypeByProviderID(ctx context.Context, providerID string) (string, error) { + if providerID == "" { + return "", errNodeNotInitialized + } + // Returns "" for unmanaged nodes because azure cloud provider couldn't fetch information for them. if az.IsNodeUnmanagedByProviderID(providerID) { klog.V(4).Infof("InstanceTypeByProviderID: omitting unmanaged node %q", providerID) diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_zones.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_zones.go index 5b18005453e..8ad5d14b921 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_zones.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_zones.go @@ -89,6 +89,10 @@ func (az *Cloud) GetZone(ctx context.Context) (cloudprovider.Zone, error) { // This is particularly useful in external cloud providers where the kubelet // does not initialize node data. func (az *Cloud) GetZoneByProviderID(ctx context.Context, providerID string) (cloudprovider.Zone, error) { + if providerID == "" { + return cloudprovider.Zone{}, errNodeNotInitialized + } + // Returns nil for unmanaged nodes because azure cloud provider couldn't fetch information for them. if az.IsNodeUnmanagedByProviderID(providerID) { klog.V(2).Infof("GetZoneByProviderID: omitting unmanaged node %q", providerID)