Skip to content

Commit

Permalink
fix: remove Volume#server= to attach volumes
Browse files Browse the repository at this point in the history
new fog-core model#reload resets the model the current remote state.
The previous implementation of volume relied upon initializing with a
device, reloading the model several times, and then attaching with a
server instance is set.  IMO, the new fog-core is doing the correct
thing.

To support this behavior, make #attach and #detach public methods and
require a device argument in #attach.  #server= does exist but only as
an attr_writer.  This may cause some confusion in the future release
and must be included in the CHANGELOG.
  • Loading branch information
lanej committed Apr 9, 2018
1 parent 770629e commit 24ea467
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 35 deletions.
41 changes: 14 additions & 27 deletions lib/fog/aws/models/compute/volume.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,25 +59,19 @@ def save
requires :availability_zone
requires_one :size, :snapshot_id

if type == 'io1'
requires :iops
end
requires :iops if type == 'io1'

data = service.create_volume(availability_zone, size, create_params).body
merge_attributes(data)

if tags = self.tags
# expect eventual consistency
Fog.wait_for { self.reload rescue nil }
service.create_tags(
self.identity,
tags
)
Fog.wait_for { service.volumes.get(identity) }
service.create_tags(identity, tags)
end

if @server
self.server = @server
end
attach(@service, device) if @server

true
end
end
Expand All @@ -87,13 +81,7 @@ def server
service.servers.get(server_id)
end

def server=(new_server)
if new_server
attach(new_server)
else
detach
end
end
attr_writer :server

def snapshots
requires :id
Expand All @@ -109,22 +97,15 @@ def force_detach
detach(true)
end

private

def attachmentSet=(new_attachment_set)
merge_attributes(new_attachment_set.first || {})
end

def attach(new_server)
def attach(new_server, new_device)
if !persisted?
@server = new_server
self.availability_zone = new_server.availability_zone
elsif new_server
requires :device
wait_for { ready? }
@server = nil
self.server_id = new_server.id
service.attach_volume(server_id, id, device)
service.attach_volume(server_id, id, new_device)
reload
end
end
Expand All @@ -138,6 +119,12 @@ def detach(force = false)
end
end

private

def attachmentSet=(new_attachment_set)
merge_attributes(new_attachment_set.first || {})
end

def create_params
{
'Encrypted' => encrypted,
Expand Down
25 changes: 17 additions & 8 deletions tests/models/compute/volume_tests.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
Shindo.tests("Fog::Compute[:aws] | volume", ['aws']) do

Shindo.tests('Fog::Compute[:aws] | volume', ['aws']) do
@server = Fog::Compute[:aws].servers.create
@server.wait_for { ready? }

model_tests(Fog::Compute[:aws].volumes, {:availability_zone => @server.availability_zone, :size => 1, :device => '/dev/sdz1', :tags => {"key" => "value"}, :type => 'gp2'}, true) do
model_tests(
Fog::Compute[:aws].volumes,
{
availability_zone: @server.availability_zone,
size: 1,
tags: { 'key' => 'value' },
type: 'gp2'
},
true
) do

@instance.wait_for { ready? }

tests('#server = @server').succeeds do
@instance.server = @server
tests('#attach(server, device)').succeeds do
@instance.attach(@server, '/dev/sdz1')
end

@instance.wait_for { state == 'in-use' }
Expand All @@ -17,13 +25,14 @@
@instance.server.id == @server.id
end

tests('#server = nil').succeeds do
(@instance.server = nil).nil?
tests('#detach').succeeds do
@instance.detach
@instance.server.nil?
end

@instance.wait_for { ready? }

@instance.server = @server
@instance.attach(@server, '/dev/sdz1')
@instance.wait_for { state == 'in-use' }

tests('#force_detach').succeeds do
Expand Down

0 comments on commit 24ea467

Please sign in to comment.