From 89fd358c52ae612a713753af426bbadef69dbe6d Mon Sep 17 00:00:00 2001 From: saadali Date: Fri, 22 Jul 2016 17:07:47 -0700 Subject: [PATCH] Assume volume detached if node doesn't exist Fixes #29358 --- pkg/cloudprovider/providers/aws/aws.go | 30 +++++++++++++++---- pkg/cloudprovider/providers/gce/gce.go | 18 +++++++++++ .../statusupdater/node_status_updater.go | 6 ++-- 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index e5897123ae0..b348a6e560a 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -812,7 +812,7 @@ func (c *Cloud) NodeAddresses(name string) ([]api.NodeAddress, error) { } instance, err := c.getInstanceByNodeName(name) if err != nil { - return nil, err + return nil, fmt.Errorf("getInstanceByNodeName failed for %q with %v", name, err) } addresses := []api.NodeAddress{} @@ -869,7 +869,7 @@ func (c *Cloud) InstanceID(name string) (string, error) { } inst, err := c.getInstanceByNodeName(name) if err != nil { - return "", err + return "", fmt.Errorf("getInstanceByNodeName failed for %q with %v", name, err) } return "/" + orEmpty(inst.Placement.AvailabilityZone) + "/" + orEmpty(inst.InstanceId), nil } @@ -881,7 +881,7 @@ func (c *Cloud) InstanceType(name string) (string, error) { } inst, err := c.getInstanceByNodeName(name) if err != nil { - return "", err + return "", fmt.Errorf("getInstanceByNodeName failed for %q with %v", name, err) } return orEmpty(inst.InstanceType), nil } @@ -1336,7 +1336,7 @@ func (c *Cloud) getAwsInstance(nodeName string) (*awsInstance, error) { } else { instance, err := c.getInstanceByNodeName(nodeName) if err != nil { - return nil, fmt.Errorf("error finding instance %s: %v", nodeName, err) + return nil, err } awsInstance = newAWSInstance(c.ec2, instance) @@ -1354,7 +1354,7 @@ func (c *Cloud) AttachDisk(diskName string, instanceName string, readOnly bool) awsInstance, err := c.getAwsInstance(instanceName) if err != nil { - return "", err + return "", fmt.Errorf("error finding instance %s: %v", instanceName, err) } if readOnly { @@ -1419,6 +1419,15 @@ func (c *Cloud) DetachDisk(diskName string, instanceName string) (string, error) awsInstance, err := c.getAwsInstance(instanceName) if err != nil { + if err == cloudprovider.InstanceNotFound { + // If instance no longer exists, safe to assume volume is not attached. + glog.Warningf( + "Instance %q does not exist. DetachDisk will assume disk %q is not attached to it.", + instanceName, + diskName) + return "", nil + } + return "", err } @@ -1562,6 +1571,15 @@ func (c *Cloud) GetDiskPath(volumeName string) (string, error) { func (c *Cloud) DiskIsAttached(diskName, instanceID string) (bool, error) { awsInstance, err := c.getAwsInstance(instanceID) if err != nil { + if err == cloudprovider.InstanceNotFound { + // If instance no longer exists, safe to assume volume is not attached. + glog.Warningf( + "Instance %q does not exist. DiskIsAttached will assume disk %q is not attached to it.", + instanceID, + diskName) + return false, nil + } + return false, err } @@ -2918,7 +2936,7 @@ func (c *Cloud) findInstanceByNodeName(nodeName string) (*ec2.Instance, error) { func (c *Cloud) getInstanceByNodeName(nodeName string) (*ec2.Instance, error) { instance, err := c.findInstanceByNodeName(nodeName) if err == nil && instance == nil { - return nil, fmt.Errorf("no instances found for name: %s", nodeName) + return nil, cloudprovider.InstanceNotFound } return instance, err } diff --git a/pkg/cloudprovider/providers/gce/gce.go b/pkg/cloudprovider/providers/gce/gce.go index ea1e644e0e5..8f95ff8f0b0 100644 --- a/pkg/cloudprovider/providers/gce/gce.go +++ b/pkg/cloudprovider/providers/gce/gce.go @@ -2367,6 +2367,15 @@ func (gce *GCECloud) AttachDisk(diskName, instanceID string, readOnly bool) erro func (gce *GCECloud) DetachDisk(devicePath, instanceID string) error { inst, err := gce.getInstanceByName(instanceID) if err != nil { + if err == cloudprovider.InstanceNotFound { + // If instance no longer exists, safe to assume volume is not attached. + glog.Warningf( + "Instance %q does not exist. DetachDisk will assume PD %q is not attached to it.", + instanceID, + devicePath) + return nil + } + return fmt.Errorf("error getting instance %q", instanceID) } @@ -2381,6 +2390,15 @@ func (gce *GCECloud) DetachDisk(devicePath, instanceID string) error { func (gce *GCECloud) DiskIsAttached(diskName, instanceID string) (bool, error) { instance, err := gce.getInstanceByName(instanceID) if err != nil { + if err == cloudprovider.InstanceNotFound { + // If instance no longer exists, safe to assume volume is not attached. + glog.Warningf( + "Instance %q does not exist. DiskIsAttached will assume PD %q is not attached to it.", + instanceID, + diskName) + return false, nil + } + return false, err } diff --git a/pkg/controller/volume/attachdetach/statusupdater/node_status_updater.go b/pkg/controller/volume/attachdetach/statusupdater/node_status_updater.go index 591f0275fa4..a9d5cc61d1e 100644 --- a/pkg/controller/volume/attachdetach/statusupdater/node_status_updater.go +++ b/pkg/controller/volume/attachdetach/statusupdater/node_status_updater.go @@ -62,10 +62,12 @@ func (nsu *nodeStatusUpdater) UpdateNodeStatuses() error { for nodeName, attachedVolumes := range nodesToUpdate { nodeObj, exists, err := nsu.nodeInformer.GetStore().GetByKey(nodeName) if nodeObj == nil || !exists || err != nil { - return fmt.Errorf( - "failed to find node %q in NodeInformer cache. %v", + // If node does not exist, its status cannot be updated, log error and move on. + glog.Warningf( + "Could not update node status. Failed to find node %q in NodeInformer cache. %v", nodeName, err) + return nil } node, ok := nodeObj.(*api.Node)