Assume volume detached if node doesn't exist

Fixes kubernetes#29358
saad-ali · Jul 25, 2016 · 1bc5468 · 1bc5468
1 parent 285ecb2
commit 1bc5468
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 8 deletions.
diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go
@@ -797,7 +797,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{}
@@ -854,7 +854,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
 }
@@ -866,7 +866,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
 }
@@ -1321,7 +1321,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)
@@ -1339,7 +1339,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 {
@@ -1404,6 +1404,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
 	}
 
@@ -1547,6 +1556,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
 	}
 
@@ -2903,7 +2921,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
@@ -2363,6 +2363,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)
 	}
 
@@ -2377,6 +2386,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/statusupdater/node_status_updater.go b/pkg/controller/volume/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)