From ec620d8e4b2af4ae5d61e23e66f9c6194003e258 Mon Sep 17 00:00:00 2001 From: Joshua Lane Date: Mon, 26 Feb 2018 09:58:10 -0800 Subject: [PATCH 1/4] fog-core 2.x, fog-json 1.x References #227 --- fog-aws.gemspec | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fog-aws.gemspec b/fog-aws.gemspec index 9c278d29d3..92fd9f6a00 100644 --- a/fog-aws.gemspec +++ b/fog-aws.gemspec @@ -23,11 +23,11 @@ Gem::Specification.new do |spec| spec.add_development_dependency 'bundler', '~> 1.15' spec.add_development_dependency 'rake', '~> 10.0' - spec.add_development_dependency 'shindo', '~> 0.3' spec.add_development_dependency 'rubyzip', '~> 1.2.1' + spec.add_development_dependency 'shindo', '~> 0.3' - spec.add_dependency 'fog-core', '~> 1.38' - spec.add_dependency 'fog-json', '~> 1.0' + spec.add_dependency 'fog-core', '~> 2.1' + spec.add_dependency 'fog-json', '~> 1.1' spec.add_dependency 'fog-xml', '~> 0.1' spec.add_dependency 'ipaddress', '~> 0.8' end From 199cbf8e40d6a3999227e1f94f0e2f49928f4f86 Mon Sep 17 00:00:00 2001 From: Joshua Lane Date: Wed, 14 Mar 2018 16:16:17 -0700 Subject: [PATCH 2/4] fix(elb): override #all_associations_and_attributes ListenerDescriptions and BackendServerDescriptions are used to build ad-hoc collections but are not registered as attributes of the LoadBalancer model. --- .../aws/models/elb/backend_server_descriptions.rb | 2 +- lib/fog/aws/models/elb/load_balancer.rb | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/fog/aws/models/elb/backend_server_descriptions.rb b/lib/fog/aws/models/elb/backend_server_descriptions.rb index 1463ce24f0..e0de6c3b14 100644 --- a/lib/fog/aws/models/elb/backend_server_descriptions.rb +++ b/lib/fog/aws/models/elb/backend_server_descriptions.rb @@ -12,7 +12,7 @@ def all end def get(instance_port) - all.find{|e| e.instance_port == instance_port} + all.find { |e| e.instance_port == instance_port } end end end diff --git a/lib/fog/aws/models/elb/load_balancer.rb b/lib/fog/aws/models/elb/load_balancer.rb index a21949aa31..09cf7ef3a3 100644 --- a/lib/fog/aws/models/elb/load_balancer.rb +++ b/lib/fog/aws/models/elb/load_balancer.rb @@ -223,16 +223,24 @@ def save end def reload - super @instance_health = nil @policy_descriptions = nil - self + super end def destroy requires :id service.delete_load_balancer(id) end + + protected + + def all_associations_and_attributes + super.merge( + 'ListenerDescriptions' => attributes['ListenerDescriptions'], + 'BackendServerDescriptions' => attributes['BackendServerDescriptions'], + ) + end end end end From 770629e1d1e292616467966d24ca954006b04289 Mon Sep 17 00:00:00 2001 From: Joshua Lane Date: Wed, 14 Mar 2018 16:17:57 -0700 Subject: [PATCH 3/4] style(elb): reduce branch conditions and increase readability --- lib/fog/aws/models/elb/listeners.rb | 7 ++- lib/fog/aws/models/elb/load_balancers.rb | 11 +++-- .../elb/create_load_balancer_listeners.rb | 45 ++++++++++--------- 3 files changed, 32 insertions(+), 31 deletions(-) diff --git a/lib/fog/aws/models/elb/listeners.rb b/lib/fog/aws/models/elb/listeners.rb index 98b0201e61..3c411cbab7 100644 --- a/lib/fog/aws/models/elb/listeners.rb +++ b/lib/fog/aws/models/elb/listeners.rb @@ -12,17 +12,16 @@ def all end def get(lb_port) - all.find{|listener| listener.lb_port == lb_port} + all.find { |listener| listener.lb_port == lb_port } end private + # Munge an array of ListenerDescription hashes like: # {'Listener' => listener, 'PolicyNames' => []} # to an array of listeners with a PolicyNames key def munged_data - data.map {|description| - description['Listener'].merge('PolicyNames' => description['PolicyNames']) - } + data.map { |description| description['Listener'].merge('PolicyNames' => description['PolicyNames']) } end end end diff --git a/lib/fog/aws/models/elb/load_balancers.rb b/lib/fog/aws/models/elb/load_balancers.rb index 889e44a529..c8685066f5 100644 --- a/lib/fog/aws/models/elb/load_balancers.rb +++ b/lib/fog/aws/models/elb/load_balancers.rb @@ -6,7 +6,7 @@ class LoadBalancers < Fog::Collection model Fog::AWS::ELB::LoadBalancer # Creates a new load balancer - def initialize(attributes={}) + def initialize(attributes = {}) super end @@ -14,7 +14,7 @@ def all result = [] marker = nil finished = false - while !finished + until finished data = service.describe_load_balancers('Marker' => marker).body result.concat(data['DescribeLoadBalancersResult']['LoadBalancerDescriptions']) marker = data['DescribeLoadBalancersResult']['NextMarker'] @@ -24,10 +24,9 @@ def all end def get(identity) - if identity - data = service.describe_load_balancers('LoadBalancerNames' => identity).body['DescribeLoadBalancersResult']['LoadBalancerDescriptions'].first - new(data) - end + return unless identity + data = service.describe_load_balancers('LoadBalancerNames' => identity).body['DescribeLoadBalancersResult']['LoadBalancerDescriptions'].first + new(data) rescue Fog::AWS::ELB::NotFound nil end diff --git a/lib/fog/aws/requests/elb/create_load_balancer_listeners.rb b/lib/fog/aws/requests/elb/create_load_balancer_listeners.rb index 9e28a7507a..46868b3e55 100644 --- a/lib/fog/aws/requests/elb/create_load_balancer_listeners.rb +++ b/lib/fog/aws/requests/elb/create_load_balancer_listeners.rb @@ -51,34 +51,37 @@ def create_load_balancer_listeners(lb_name, listeners) class Mock def create_load_balancer_listeners(lb_name, listeners) - if load_balancer = self.data[:load_balancers][lb_name] - response = Excon::Response.new + load_balancer = data[:load_balancers][lb_name] + raise Fog::AWS::ELB::NotFound unless load_balancer + response = Excon::Response.new - certificate_ids = Fog::AWS::IAM::Mock.data[@aws_access_key_id][:server_certificates].map {|n, c| c['Arn'] } + certificate_ids = Fog::AWS::IAM::Mock.data[@aws_access_key_id][:server_certificates].map { |_n, c| c['Arn'] } - listeners.each do |listener| - if listener['SSLCertificateId'] and !certificate_ids.include? listener['SSLCertificateId'] - raise Fog::AWS::IAM::NotFound.new('CertificateNotFound') - end + listeners.each do |listener| + if listener['SSLCertificateId'] && !certificate_ids.include?(listener['SSLCertificateId']) + raise Fog::AWS::IAM::NotFound, 'CertificateNotFound' + end - if (%w( HTTP HTTPS).include?(listener['Protocol']) && !%w( HTTP HTTPS ).include?(listener['InstanceProtocol'])) || - (%w( TCP SSL).include?(listener['Protocol']) && !%w( TCP SSL ).include?(listener['InstanceProtocol'])) - raise Fog::AWS::ELB::ValidationError - end if listener['Protocol'] && listener['InstanceProtocol'] - load_balancer['ListenerDescriptions'] << {'Listener' => listener, 'PolicyNames' => []} + if listener['Protocol'] && listener['InstanceProtocol'] + if ( + %w[HTTP HTTPS].include?(listener['Protocol']) && !%w[HTTP HTTPS].include?(listener['InstanceProtocol']) + ) || ( + %w[TCP SSL].include?(listener['Protocol']) && !%w[TCP SSL].include?(listener['InstanceProtocol']) + ) + raise Fog::AWS::ELB::ValidationError + end end + load_balancer['ListenerDescriptions'] << { 'Listener' => listener, 'PolicyNames' => [] } + end - response.status = 200 - response.body = { - 'ResponseMetadata' => { - 'RequestId' => Fog::AWS::Mock.request_id - } + response.status = 200 + response.body = { + 'ResponseMetadata' => { + 'RequestId' => Fog::AWS::Mock.request_id } + } - response - else - raise Fog::AWS::ELB::NotFound - end + response end end end From 24ea4675bfd28c93d1344bf666ebafd0f4826b8f Mon Sep 17 00:00:00 2001 From: Joshua Lane Date: Mon, 9 Apr 2018 11:13:14 -0700 Subject: [PATCH 4/4] fix: remove Volume#server= to attach volumes 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. --- lib/fog/aws/models/compute/volume.rb | 41 ++++++++++------------------ tests/models/compute/volume_tests.rb | 25 +++++++++++------ 2 files changed, 31 insertions(+), 35 deletions(-) diff --git a/lib/fog/aws/models/compute/volume.rb b/lib/fog/aws/models/compute/volume.rb index dec8b01474..e4fd36a708 100644 --- a/lib/fog/aws/models/compute/volume.rb +++ b/lib/fog/aws/models/compute/volume.rb @@ -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 @@ -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 @@ -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 @@ -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, diff --git a/tests/models/compute/volume_tests.rb b/tests/models/compute/volume_tests.rb index 3445de6641..116873b833 100644 --- a/tests/models/compute/volume_tests.rb +++ b/tests/models/compute/volume_tests.rb @@ -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' } @@ -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