From 82e36a29f0feac4e32052d851a4f2cdc4d264a22 Mon Sep 17 00:00:00 2001 From: Joshua Lane Date: Thu, 19 Apr 2018 10:04:30 -0700 Subject: [PATCH 1/2] fix: attach volume on #save, remove #server= s/service/server/ in Volume#save. Fixes attaching after initialization. Require users to call #attach(server, device) to attach a volume after it's created. Volume#server=(new_server) having a side-effect of attaching a volume is confusing. Volume#device set on #initialize will be lost when calling Volume#wait_for in fog-core ~> 2.0. BREAKING CHANGE Volume#server= is removed. --- lib/fog/aws/models/compute/volume.rb | 10 ++++---- tests/models/compute/volume_tests.rb | 34 ++++++++++++---------------- 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/lib/fog/aws/models/compute/volume.rb b/lib/fog/aws/models/compute/volume.rb index e4fd36a70..b4817c2ee 100644 --- a/lib/fog/aws/models/compute/volume.rb +++ b/lib/fog/aws/models/compute/volume.rb @@ -21,7 +21,7 @@ module Fog def initialize(attributes = {}) # assign server first to prevent race condition with persisted? - self.server = attributes.delete(:server) + @server = attributes.delete(:server) super end @@ -70,10 +70,10 @@ module Fog service.create_tags(identity, tags) end - attach(@service, device) if @server - - true + attach(@server, device) if @server && device end + + true end def server @@ -81,8 +81,6 @@ module Fog service.servers.get(server_id) end - attr_writer :server - def snapshots requires :id service.snapshots(:volume => self) diff --git a/tests/models/compute/volume_tests.rb b/tests/models/compute/volume_tests.rb index 116873b83..87ca6ed71 100644 --- a/tests/models/compute/volume_tests.rb +++ b/tests/models/compute/volume_tests.rb @@ -8,39 +8,34 @@ Shindo.tests('Fog::Compute[:aws] | volume', ['aws']) do availability_zone: @server.availability_zone, size: 1, tags: { 'key' => 'value' }, - type: 'gp2' + type: 'gp2', + server: @server, + device: '/dev/sdz1' }, true ) do - @instance.wait_for { ready? } - - tests('#attach(server, device)').succeeds do - @instance.attach(@server, '/dev/sdz1') - end - - @instance.wait_for { state == 'in-use' } - - tests('#server').succeeds do - @instance.server.id == @server.id + tests('attached').succeeds do + @instance.server == @server end tests('#detach').succeeds do @instance.detach + @instance.wait_for { ready? } @instance.server.nil? end - @instance.wait_for { ready? } - - @instance.attach(@server, '/dev/sdz1') - @instance.wait_for { state == 'in-use' } + tests('#attach(server, device)').succeeds do + @instance.attach(@server, '/dev/sdz1') + @instance.server == @server + end tests('#force_detach').succeeds do @instance.force_detach + @instance.wait_for { ready? } + @instance.server.nil? end - @instance.wait_for { ready? } - @instance.type = 'io1' @instance.iops = 5000 @instance.size = 100 @@ -56,10 +51,9 @@ Shindo.tests('Fog::Compute[:aws] | volume', ['aws']) do returns(5000) { @instance.iops } returns(100) { @instance.size } - tests('@instance.reload.tags').returns({'key' => 'value'}) do - @instance.reload.tags + tests('@instance.tags').returns({'key' => 'value'}) do + @instance.tags end - end @server.destroy From 1c938fb023d3e81df22f01f20c88c16cef1a9b62 Mon Sep 17 00:00:00 2001 From: Joshua Lane Date: Mon, 23 Apr 2018 08:41:45 -0700 Subject: [PATCH 2/2] doc: provide useful error message when using Volume#server= Direct users to use #attach --- lib/fog/aws/models/compute/volume.rb | 4 ++++ tests/models/compute/volume_tests.rb | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/lib/fog/aws/models/compute/volume.rb b/lib/fog/aws/models/compute/volume.rb index b4817c2ee..0d894b98b 100644 --- a/lib/fog/aws/models/compute/volume.rb +++ b/lib/fog/aws/models/compute/volume.rb @@ -117,6 +117,10 @@ module Fog end end + def server=(_) + raise NoMethodError, 'use Fog::Compute::AWS::Volume#attach(server, device)' + end + private def attachmentSet=(new_attachment_set) diff --git a/tests/models/compute/volume_tests.rb b/tests/models/compute/volume_tests.rb index 87ca6ed71..37b30cfbf 100644 --- a/tests/models/compute/volume_tests.rb +++ b/tests/models/compute/volume_tests.rb @@ -25,6 +25,10 @@ Shindo.tests('Fog::Compute[:aws] | volume', ['aws']) do @instance.server.nil? end + tests('#server=').raises(NoMethodError, 'use Fog::Compute::AWS::Volume#attach(server, device)') do + @instance.server = @server + end + tests('#attach(server, device)').succeeds do @instance.attach(@server, '/dev/sdz1') @instance.server == @server