Spruce up attaching and detaching disks on running servers

examples/create_instance_and_attach_disk_later.rb

@@ -0,0 +1,86 @@
+# All examples presume that you have a ~/.fog credentials file set up.
+# More info on it can be found here: http://fog.io/about/getting_started.html
+
+require "bundler"
+Bundler.require(:default, :development)
+
+ZONE = "us-central1-f"
+PROJECT = Fog.credentials[:google_project]
+
+def example
+  p "Connecting to Google API"
+  connection = Fog::Compute.new(:provider => "Google")
+
+  p "Creating disk"
+  disk = connection.disks.create(
+    :name => "fog-smoke-test-#{Time.now.to_i}",
+    :size_gb => 10,
+    :zone => ZONE,
+    :source_image => "debian-11-bullseye-v20220920"
+  )
+
+  p "Creating a second disk"
+  attached_disk = connection.disks.create(
+    :name => "fog-smoke-test-#{Time.now.to_i}",
+    :size_gb => 10,
+    :zone => ZONE
+  )
+
+  p "Waiting for disks to be ready"
+  disk.wait_for { ready? }
+  attached_disk.wait_for { ready? }
+
+  p "Creating a server"
+  server = connection.servers.create(
+    :name => "fog-smoke-test-#{Time.now.to_i}",
+    :disks => [disk.attached_disk_obj(boot: true, auto_delete: true)],
+    :machine_type => "n1-standard-1",
+    :private_key_path => File.expand_path("~/.ssh/id_rsa"),
+    :public_key_path => File.expand_path("~/.ssh/id_rsa.pub"),
+    :zone => ZONE,
+    # Will be simplified, see https://github.com/fog/fog-google/issues/360
+    :network_interfaces => [{ :network => "global/networks/default",
+                              :access_configs => [{
+                                :name => "External NAT",
+                                :type => "ONE_TO_ONE_NAT"
+                              }] }],
+    :username => ENV["USER"]
+  )
+
+  p "Attach second disk to the running server"
+  device_name = "fog-smoke-test-device-#{Time.now.to_i}"
+  # See https://github.com/fog/fog-google/blob/master/lib/fog/compute/google/models/disk.rb#L75-L107
+  # See https://github.com/fog/fog-google/blob/master/lib/fog/compute/google/models/server.rb#L35-L50
+  config_hash = {
+    :device_name => device_name,
+    :source => "https://www.googleapis.com/compute/v1/projects/#{PROJECT}/zones/#{ZONE}/disks/#{attached_disk.name}"
+  }
+  raise "Could not attach second disk" unless connection.attach_disk(server.name, ZONE, config_hash)
+
+  p "Waiting for disk to be attached"
+  attached_disk.wait_for { ! users.nil? && users != []}
+
+  p "Detach second disk"
+  raise "Could not detach second disk" unless connection.detach_disk(server.name, ZONE, device_name)
+
+  p "Waiting for second disk to be detached"
+  attached_disk.wait_for { users.nil? || users == []}
+
+  p "Deleting server"
+  raise "Could not delete server." unless server.destroy
+
+  p "Destroying second disk"
+  raise "Could not delete second disk." unless attached_disk.destroy
+
+  p "Waiting for second disk to be destroyed"
+  begin
+    rc = attached_disk.wait_for { status.nil? || status == 'DELETING' }
+
+  rescue => e
+    if e.message !~ /not found/ && e.message !~ /notFound/
+      raise e
+    end
+  end
+end
+
+example

lib/fog/compute/google/models/disks.rb

@@ -36,6 +36,10 @@ def all(zone: nil, filter: nil, max_results: nil, order_by: nil,
         def get(identity, zone = nil)
           if zone
             disk = service.get_disk(identity, zone).to_h
+
+            # Force the hash to contain a :users key so that it will override any :users key in the existing object
+            disk[:users] = nil unless disk.include?(:users)
+
             return new(disk)
           elsif identity
             response = all(:filter => "name eq #{identity}",
@@ -45,7 +49,8 @@ def get(identity, zone = nil)
           end
         rescue ::Google::Apis::ClientError => e
           raise e unless e.status_code == 404
-          nil
+          # Return an empty object so that wait_for processes the block
+          return new({:status => nil})
         end
 
         # Returns an attached disk configuration hash.

lib/fog/compute/google/models/server.rb

@@ -279,7 +279,7 @@ def attach_disk(disk, async = true, attached_disk_options = {})
           if disk.is_a? Disk
             disk_obj = disk.get_attached_disk
           elsif disk.is_a? String
-            disk_obj = service.disks.attached_disk_obj(disk, attached_disk_options)
+            disk_obj = service.disks.attached_disk_obj(disk, **attached_disk_options)
           end
 
           data = service.attach_disk(identity, zone_name, disk_obj)

test/integration/compute/core_compute/test_servers.rb

@@ -132,6 +132,80 @@ def test_start_stop_reboot
     assert server.ready?
   end
 
+  def test_attach_disk
+    # Creating server
+    server = @factory.create
+    server.wait_for { ready? }
+
+    disk_name = "fog-test-1-testservers-test-attach-disk-attachable"  # suffix forces disk name to differ from the existing disk
+    # Creating disk #{disk_name}
+    disk = @disks.create(
+      :name => disk_name,
+      :source_image => TEST_IMAGE,
+      :size_gb => 64
+    )
+    device_name = "#{disk.name}-device"
+
+    # Attaching disk #{disk.name} as device #{device_name}
+    self_link = "https://www.googleapis.com/compute/v1/projects/#{TEST_PROJECT}/zones/#{TEST_ZONE}/disks/#{disk.name}"
+    server.attach_disk(self_link, true, device_name: device_name)
+
+    # Waiting for attachment
+    disk.wait_for { ! users.nil? && users != []}
+
+    assert_equal "https://www.googleapis.com/compute/v1/projects/#{TEST_PROJECT}/zones/#{TEST_ZONE}/instances/#{server.name}", disk.users[0]
+
+    server.reload
+    server_attached_disk = server.disks.select{|d| d[:boot] == false}[0]
+    assert_equal device_name, server_attached_disk[:device_name]
+  end
+
+  def test_detach_disk
+    # Creating server
+    server = @factory.create
+    server.wait_for { ready? }
+
+    disk_name = "fog-test-1-testservers-test-detach-attachable"  # suffix forces disk name to differ from the existing disk
+    # Creating disk #{disk_name}
+    disk = @disks.create(
+      :name => disk_name,
+      :source_image => TEST_IMAGE,
+      :size_gb => 64
+    )
+    device_name = "#{disk.name}-device"
+
+    # Attaching disk #{disk.name} as device #{device_name}
+    self_link = "https://www.googleapis.com/compute/v1/projects/#{TEST_PROJECT}/zones/#{TEST_ZONE}/disks/#{disk.name}"
+    server.attach_disk(self_link, true, device_name: device_name)
+    disk.wait_for { ! users.nil? && users != []}
+
+    server.reload
+    server_attached_disk = server.disks.select{|d| d[:boot] == false}[0]
+    assert_equal device_name, server_attached_disk[:device_name]
+
+    # Detaching (synchronous) disk #{disk.name}
+    server.detach_disk(device_name, false)
+
+    disk.reload
+    assert disk.users.nil? || disk.users == []
+
+    # Re-attaching disk #{disk.name} as device #{device_name}
+    server.attach_disk(self_link, true, device_name: device_name)
+    disk.wait_for { ! users.nil? && users != []}
+
+    server.reload
+    server_attached_disk = server.disks.select{|d| d[:boot] == false}[0]
+    assert_equal device_name, server_attached_disk[:device_name]
+
+    # Detaching (async) disk #{disk.name}
+    server.detach_disk(device_name, true)
+
+    # Waiting for detachment
+    disk.wait_for { users.nil? || users == []}
+
+    assert disk.users.nil? || disk.users == []
+  end
+
   def test_reset_windows_password
     win_disk = @disks.create(
       :name => "fog-test-1-testservers-test-reset-windows-password-2",

test/unit/compute/test_disk.rb

@@ -0,0 +1,26 @@
+require "helpers/test_helper"
+
+class UnitTestDisk < Minitest::Test
+  def setup
+    Fog.mock!
+    @client = Fog::Compute.new(provider: "google",
+                               google_project: "foo")
+  end
+
+  def teardown
+    Fog.unmock!
+  end
+
+  def test_new_disk
+    disk = Fog::Compute::Google::Disk.new(
+      :name         => "fog-1",
+      :size_gb      => 10,
+      :zone         => "us-central1-a",
+      :source_image => "debian-7-wheezy-v20131120"
+    )
+    assert_equal("fog-1",                     disk.name,         "Fog::Compute::Google::Disk name is incorrect: #{disk.name}")
+    assert_equal(10,                          disk.size_gb,      "Fog::Compute::Google::Disk size_gb is incorrect: #{disk.size_gb}")
+    assert_equal("us-central1-a",             disk.zone,         "Fog::Compute::Google::Disk zone is incorrect: #{disk.zone}")
+    assert_equal("debian-7-wheezy-v20131120", disk.source_image, "Fog::Compute::Google::Disk source_image is incorrect: #{disk.source_image}")
+  end
+end