compute: fix detachDisk bug

lib/compute/vm.js

@@ -20,6 +20,7 @@
 
 'use strict';
 
+var createErrorClass = require('create-error-class');
 var extend = require('extend');
 var is = require('is');
 
@@ -35,6 +36,16 @@ var Disk = require('./disk.js');
  */
 var util = require('../common/util.js');
 
+/**
+ * Custom error type for errors related to detaching a disk.
+ *
+ * @param {string} message - Custom error message.
+ * @return {Error}
+ */
+var DetachDiskError = createErrorClass('DetachDiskError', function(message) {
+  this.message = message;
+});
+
 /*! Developer Documentation
  *
  * @param {module:zone} zone - Zone object this instance belongs to.
@@ -65,6 +76,7 @@ var util = require('../common/util.js');
 function VM(zone, name) {
   this.zone = zone;
   this.name = name;
+  this.metadata = {};
 }
 
 /**
@@ -174,12 +186,48 @@ VM.prototype.delete = function(callback) {
  * });
  */
 VM.prototype.detachDisk = function(disk, callback) {
+  var self = this;
+
   if (!(disk instanceof Disk)) {
     throw new Error('A Disk object must be provided.');
   }
 
+  var disks = this.metadata.disks;
+
+  if (is.empty(disks)) {
+    this.getMetadata(function(err) {
+      if (err) {
+        callback(new DetachDiskError(err.message));
+        return;
+      }
+
+      self.detachDisk(disk, callback);
+    });
+
+    return;
+  }
+
+  var deviceName;
+  var baseUrl = 'https://www.googleapis.com/compute/v1/';
+
+  // Try to find the deviceName by matching the source of the attached disks to
+  // the name of the disk provided by the user.
+  for (var i = 0; !deviceName && i < disks.length; i++) {
+    var attachedDisk = disks[i];
+    var source = attachedDisk.source.replace(baseUrl, '');
+
+    if (source === disk.formattedName) {
+      deviceName = attachedDisk.deviceName;
+    }
+  }
+
+  if (!deviceName) {
+    callback(new DetachDiskError('A device name for this disk was not found.'));
+    return;
+  }
+
   var query = {
-    deviceName: disk.name
+    deviceName: deviceName
   };
 
   this.makeReq_('POST', '/detachDisk', query, null, callback || util.noop);

system-test/compute.js

@@ -472,20 +472,32 @@ describe('Compute', function() {
     });
 
     it('should attach and detach a disk', function(done) {
-      compute.getDisks()
-        .on('error', done)
-        .once('data', function(disk) {
-          this.end();
+      var name = generateName();
 
-          vm.attachDisk(disk, function(err) {
-            assert.ifError(err);
+      // This test waits on a lot of operations.
+      this.timeout(90000);
 
-            vm.detachDisk(disk, function(err, operation) {
-              assert.ifError(err);
-              operation.onComplete(done);
-            });
-          });
-        });
+      async.series([
+        createDisk,
+        attachDisk,
+        detachDisk
+      ], done);
+
+      function createDisk(callback) {
+        var config = {
+          os: 'ubuntu'
+        };
+
+        zone.createDisk(name, config, execAfterOperationComplete(callback));
+      }
+
+      function attachDisk(callback) {
+        vm.attachDisk(zone.disk(name), execAfterOperationComplete(callback));
+      }
+
+      function detachDisk(callback) {
+        vm.detachDisk(zone.disk(name), execAfterOperationComplete(callback));
+      }
     });
 
     it('should get serial port output', function(done) {
@@ -624,4 +636,18 @@ describe('Compute', function() {
       async.each(objects, exec('delete'), callback);
     });
   }
+
+  function execAfterOperationComplete(options, callback) {
+    callback = callback || options;
+
+    return function(err) {
+      if (err) {
+        callback(err);
+        return;
+      }
+
+      var operation = arguments[arguments.length - 2]; // [..., op, apiResponse]
+      operation.onComplete(options || {}, callback);
+    };
+  }
 });

test/compute/vm.js

@@ -18,6 +18,7 @@
 
 var assert = require('assert');
 var extend = require('extend');
+var format = require('string-format-obj');
 
 var Disk = require('../../lib/compute/disk.js');
 var util = require('../../lib/common/util.js');
@@ -135,6 +136,22 @@ describe('VM', function() {
   });
 
   describe('detachDisk', function() {
+    var DEVICE_NAME = format('{uri}/{name}', {
+      uri: 'https://www.googleapis.com/compute/v1',
+      name: DISK.formattedName
+    });
+
+    beforeEach(function() {
+      vm.metadata = {
+        disks: [
+          {
+            source: DEVICE_NAME,
+            deviceName: DEVICE_NAME
+          }
+        ]
+      };
+    });
+
     it('should throw if a Disk is not provided', function() {
       assert.throws(function() {
         vm.detachDisk('disk-name');
@@ -146,11 +163,31 @@ describe('VM', function() {
       });
     });
 
+    it('should return an error if device name not found', function(done) {
+      vm.metadata = {
+        disks: [
+          {
+            source: 'a',
+            deviceName: 'b'
+          }
+        ]
+      };
+
+      vm.detachDisk(DISK, function(err) {
+        assert.strictEqual(err.name, 'DetachDiskError');
+
+        var errorMessage = 'A device name for this disk was not found.';
+        assert.strictEqual(err.message, errorMessage);
+
+        done();
+      });
+    });
+
     it('should make the correct API request', function(done) {
       vm.makeReq_ = function(method, path, query, body, callback) {
         assert.strictEqual(method, 'POST');
         assert.strictEqual(path, '/detachDisk');
-        assert.deepEqual(query, { deviceName: DISK.name });
+        assert.deepEqual(query, { deviceName: DEVICE_NAME });
         assert.strictEqual(body, null);
 
         callback();
@@ -169,6 +206,51 @@ describe('VM', function() {
 
       vm.detachDisk(DISK);
     });
+
+    describe('refreshing metadata', function() {
+      beforeEach(function() {
+        vm.metadata = {};
+      });
+
+      describe('error', function() {
+        var ERROR = new Error('Error.');
+
+        beforeEach(function() {
+          vm.getMetadata = function(callback) {
+            callback(ERROR);
+          };
+        });
+
+        it('should return DetachDisk error', function(done) {
+          vm.detachDisk(DISK, function(err) {
+            assert.strictEqual(err.name, 'DetachDiskError');
+            assert.strictEqual(err.message, ERROR.message);
+            done();
+          });
+        });
+      });
+
+      describe('success', function() {
+        beforeEach(function() {
+          vm.getMetadata = function(callback) {
+            callback();
+          };
+        });
+
+        it('should call detachDisk again', function(done) {
+          vm.getMetadata = function(callback) {
+            vm.detachDisk = function(disk, callback) {
+              assert.strictEqual(disk, DISK);
+              callback(); // done()
+            };
+
+            callback();
+          };
+
+          vm.detachDisk(DISK, done);
+        });
+      });
+    });
   });
 
   describe('getMetadata', function() {