mitchellh / vagrant-rackspace

Use Vagrant to manage Rackspace Cloud instances.
MIT License
235 stars 155 forks source link

Looking up images/flavors by exact name is broken #109

Closed maxlinc closed 10 years ago

maxlinc commented 10 years ago

Vagrant rackspace is supposed to be able to look up images as a regex, an exact string, or by ID.

So the vagrant file below should work (vagrant up --provider rackspace brings them up all, or specify a machine name to test one at a time), but the "exact_name" was broken by the recent changes in #101.

# -*- mode: ruby -*-
# vi: set ft=ruby :

# Vagrantfile API/syntax version. Don't touch unless you know what you're doing!
VAGRANTFILE_API_VERSION = "2"

Vagrant.configure(VAGRANTFILE_API_VERSION) do |config|
  config.vm.box = "dummy"

  config.vm.provider :rackspace do |rs|
    rs.username = ENV['RAX_USERNAME']
    rs.api_key  = ENV['RAX_API_KEY']
    rs.rackspace_region   = :ord
  end

  config.vm.define :exact_name do |box|
    box.vm.provider :rackspace do |rs|
      rs.flavor = '1 GB Performance'
      rs.image  = 'Ubuntu 14.04 LTS (Trusty Tahr) (PVHVM)'
    end
  end

  config.vm.define :regex do |box|
    box.vm.provider :rackspace do |rs|
      rs.flavor = /1\s+GB\s+Performance/
      rs.image  = /Ubuntu.*Trusty Tahr.*(PVHVM)/
    end
  end

  config.vm.define :id do |box|
    box.vm.provider :rackspace do |rs|
      rs.flavor = 'performance1-1'
      rs.image  = 'bb02b1a3-bc77-4d17-ab5b-421d89850fca'
    end
  end
end

The error is:

Bringing machine 'exact_name' up with 'rackspace' provider...
Bringing machine 'regex' up with 'rackspace' provider...
Bringing machine 'id' up with 'rackspace' provider...
==> exact_name: Finding flavor for server...
/opt/boxen/rbenv/versions/2.1.0/lib/ruby/gems/2.1.0/gems/excon-0.37.0/lib/excon/middlewares/expects.rb:6:in `response_call': [HTTP 400 | ] Expected([200, 203]) <=> Actual(400 Bad Request) (Fog::Compute::RackspaceV2::BadRequest)
  response => #<Excon::Response:0x007fafdd67c8e0 @data={:body=>"", :headers=>{"Content-Length"=>"0", "Connection"=>"close", "Server"=>"Jetty(8.0.y.z-SNAPSHOT)"}, :status=>400, :remote_ip=>"198.101.165.140", :local_port=>61865, :local_address=>"192.168.1.23"}, @body="", @headers={"Content-Length"=>"0", "Connection"=>"close", "Server"=>"Jetty(8.0.y.z-SNAPSHOT)"}, @status=400, @remote_ip="198.101.165.140", @local_port=61865, @local_address="192.168.1.23"> -
    from /opt/boxen/rbenv/versions/2.1.0/lib/ruby/gems/2.1.0/gems/excon-0.37.0/lib/excon/middlewares/response_parser.rb:26:in `response_call'
maxlinc commented 10 years ago

The fix works for all the happy path scenarios, but it still blows up in a really nasty way if you have an image that isn't found. E.g. if you have a typo like rs.image = 'xUbuntu 14.04 LTS (Trusty Tahr) (PVHVM)' you'll get this:

Bringing machine 'exact_name' up with 'rackspace' provider...
==> exact_name: Finding flavor for server...
==> exact_name: Finding image for server...
/opt/boxen/rbenv/versions/2.1.0/lib/ruby/gems/2.1.0/gems/excon-0.37.0/lib/excon/middlewares/expects.rb:6:in `response_call': [HTTP 400 | ] Expected([200, 203]) <=> Actual(400 Bad Request) (Fog::Compute::RackspaceV2::BadRequest)
  response => #<Excon::Response:0x007fa6f4537288 @data={:body=>"", :headers=>{"Content-Length"=>"0", "Connection"=>"close", "Server"=>"Jetty(8.0.y.z-SNAPSHOT)"}, :status=>400, :remote_ip=>"198.101.165.140", :local_port=>52406, :local_address=>"10.3.20.36"}, @body="", @headers={"Content-Length"=>"0", "Connection"=>"close", "Server"=>"Jetty(8.0.y.z-SNAPSHOT)"}, @status=400, @remote_ip="198.101.165.140", @local_port=52406, @local_address="10.3.20.36"> -
    from /opt/boxen/rbenv/versions/2.1.0/lib/ruby/gems/2.1.0/gems/excon-0.37.0/lib/excon/middlewares/response_parser.rb:26:in `response_call'

You seem to get a much nicer error if you escape the name:

item = collection.get URI.escape(name)
Bringing machine 'exact_name' up with 'rackspace' provider...
==> exact_name: Finding flavor for server...
==> exact_name: Finding image for server...
No matching image was found! Please check your image setting to
make sure you have a valid image chosen.

That's probably more monitoring friendly as well, because a spike in 400 Bad Request responses is usually more alarming than 404 Not Found.

You might just want to short-circuit it if it's not escaped, because I think you can only get by IDs and that IDs will never require escaping. So if name != URI.escape(name), then the parameter probably is probably a name, not an ID and you'll always get a 404 response.

maxlinc commented 10 years ago

P.s. I abbreviated that stacktrace. The user will see a long stacktrace, and won't see the more useful "No matching image was found!" message.

krames commented 10 years ago

@maxlinc I figured the escape functionality should be in fog so I created this PR https://github.com/fog/fog/pull/3013.

Now, I could try to add some logic to detect the fog version and use escape if necessary. Do you think that's worth it?

maxlinc commented 10 years ago

Make sense to patch it in fog.

I think it's worth detecting the fog version and putting in a fallback, unless you want to bump the minimum fog version. In this case I'd lean towards broader compatibility w/ plugins that require older fogs, since it's just a one-liner.