diff --git a/source/lib/vagrant-openstack-provider/action/create_server.rb b/source/lib/vagrant-openstack-provider/action/create_server.rb index 3a083a8..33b4314 100644 --- a/source/lib/vagrant-openstack-provider/action/create_server.rb +++ b/source/lib/vagrant-openstack-provider/action/create_server.rb @@ -31,7 +31,7 @@ def execute(env) options = { flavor: @resolver.resolve_flavor(env), - image: @resolver.resolve_image(env, config.image), + image: @resolver.resolve_image(env), volume_boot: @resolver.resolve_volume_boot(env), networks: @resolver.resolve_networks(env), volumes: @resolver.resolve_volumes(env), diff --git a/source/lib/vagrant-openstack-provider/client/nova.rb b/source/lib/vagrant-openstack-provider/client/nova.rb index 5ed2431..5940029 100644 --- a/source/lib/vagrant-openstack-provider/client/nova.rb +++ b/source/lib/vagrant-openstack-provider/client/nova.rb @@ -52,7 +52,7 @@ def create_server(env, options) s['name'] = options[:name] if options[:image_ref].nil? s['block_device_mapping'] = [{ volume_id: options[:volume_boot][:id], - device_name: options[:volume_boot][:device] }] unless options[:volume_boot][:id].nil? + device_name: options[:volume_boot][:device] }] if options[:volume_boot].key?(:id) s['block_device_mapping_v2'] = [{ boot_index: '0', volume_size: options[:volume_boot][:size], uuid: options[:volume_boot][:image], @@ -60,7 +60,7 @@ def create_server(env, options) source_type: 'image', destination_type: 'volume', delete_on_termination: options[:volume_boot][:delete_on_destroy] }]\ - unless options[:volume_boot][:image].nil? + if options[:volume_boot].key?(:image) else s['imageRef'] = options[:image_ref] end diff --git a/source/lib/vagrant-openstack-provider/config_resolver.rb b/source/lib/vagrant-openstack-provider/config_resolver.rb index 6541056..e1b3b88 100644 --- a/source/lib/vagrant-openstack-provider/config_resolver.rb +++ b/source/lib/vagrant-openstack-provider/config_resolver.rb @@ -23,16 +23,14 @@ def resolve_flavor(env) flavor end - def resolve_image(env, image_name) + def resolve_image(env) @logger.info 'Resolving image' - return nil if image_name.nil? + resolve_image_internal(env, env[:machine].provider_config.image) + end - nova = env[:openstack_client].nova - env[:ui].info(I18n.t('vagrant_openstack.finding_image')) - images = nova.get_all_images(env) - image = find_matching(images, image_name) - fail Errors::NoMatchingImage unless image - image + def resolve_volume_boot_image(env) + @logger.info 'Resolving image to create a volume from' + resolve_image_internal(env, env[:machine].provider_config.volume_boot[:image]) end def resolve_floating_ip(env) @@ -90,9 +88,13 @@ def resolve_volume_boot(env) size = (volume[:size].nil?) ? nil : volume[:size] delete_on_destroy = (volume[:delete_on_destroy].nil?) ? nil : volume[:delete_on_destroy] - image = resolve_image(env, volume[:image]) unless volume[:image].nil? + image = resolve_volume_boot_image(env) unless volume[:image].nil? image_id = (image.nil?) ? nil : image.id - { id: volume[:id], image: image_id, device: device, size: size, delete_on_destroy: delete_on_destroy } + if image.nil? + return { id: volume[:id], device: device } + else + { image: image_id, device: device, size: size, delete_on_destroy: delete_on_destroy } + end end def resolve_volumes(env) @@ -138,6 +140,17 @@ def resolve_security_groups(env) private + def resolve_image_internal(env, image_name) + return nil if image_name.nil? + + nova = env[:openstack_client].nova + env[:ui].info(I18n.t('vagrant_openstack.finding_image')) + images = nova.get_all_images(env) + image = find_matching(images, image_name) + fail Errors::NoMatchingImage unless image + image + end + def search_free_ip(config, nova, env) @logger.debug 'Retrieving all allocated floating ips on tenant' all_floating_ips = nova.get_all_floating_ips(env) @@ -261,31 +274,37 @@ def resolve_volume_from_string(volume, volume_list) def resolve_volume_from_hash(volume, volume_list, volume_ids) device = nil device = volume[:device] if volume.key?(:device) + delete_on_destroy = (volume[:delete_on_destroy].nil?) ? 'true' : volume[:delete_on_destroy] volume_id = nil if volume.key?(:id) fail Errors::ConflictVolumeNameId, volume: volume if volume.key?(:name) - fail Errors::ConflictVolumeNameId, volume: volume if volume.key?(:image) + check_boot_volume_conflict(volume) volume_id = volume[:id] fail Errors::UnresolvedVolumeId, id: volume_id unless volume_ids.include? volume_id elsif volume.key?(:name) volume_list.each do |v| next unless v.name.eql? volume[:name] fail Errors::MultipleVolumeName, name: volume[:name] unless volume_id.nil? - fail Errors::ConflictVolumeNameId, volume: volume if volume.key?(:image) + check_boot_volume_conflict(volume) volume_id = v.id end fail Errors::UnresolvedVolumeName, name: volume[:name] unless volume_ids.include? volume_id elsif volume.key?(:image) fail Errors::UnresolvedVolume, volume: volume unless volume.key?(:size) - delete_on_destroy = (volume[:delete_on_destroy].nil?) ? 'true' : volume[:delete_on_destroy] - return { id: volume_id, image: volume[:image], device: device, size: volume[:size], delete_on_destroy: delete_on_destroy } + fail Errors::ConflictBootVolume, volume: volume if volume.key?(:id) + fail Errors::ConflictBootVolume, volume: volume if volume.key?(:name) + return { image: volume[:image], device: device, size: volume[:size], delete_on_destroy: delete_on_destroy } else - fail Errors::ConflictVolumeNameId, volume: volume + fail Errors::ConflictBootVolume, volume: volume end { id: volume_id, device: device } end + def check_boot_volume_conflict(volume) + fail Errors::ConflictBootVolume, volume: volume if volume.key?(:image) || volume.key?(:size) || volume.key?(:delete_on_destroy) + end + # This method finds any matching _thing_ from a list of names # in a collection of _things_. The first to match is the returned # one. Names in list can be a regexp, a partial match is chosen diff --git a/source/lib/vagrant-openstack-provider/errors.rb b/source/lib/vagrant-openstack-provider/errors.rb index 0693c73..756d6c4 100644 --- a/source/lib/vagrant-openstack-provider/errors.rb +++ b/source/lib/vagrant-openstack-provider/errors.rb @@ -51,8 +51,8 @@ class NoMatchingImage < VagrantOpenstackError error_key(:no_matching_image) end - class ConflictImageName < VagrantOpenstackError - error_key(:conflict_image_name) + class ConflictBootVolume < VagrantOpenstackError + error_key(:conflict_boot_volume) end class SyncMethodError < VagrantOpenstackError diff --git a/source/locales/en.yml b/source/locales/en.yml index 4219f44..fde378b 100644 --- a/source/locales/en.yml +++ b/source/locales/en.yml @@ -167,9 +167,9 @@ en: no_matching_image: |- No matching image was found! Please check your image setting to make sure you have a valid image chosen. - conflict_image_name: |- - One (and only one) image must be specified in the configuration. - Please check that an image name is not defined also in the 'volume_boot' definition. + conflict_boot_volume: |- + When booting from an existing volume it is not authorized to specify in your Vagrantfile either 'image' or 'size' or 'delete_on_destroy'. + When booting from a newly creating volume it is not authorized to specify in your Vagrantfile either 'id' or 'name'. sync_method_error: |- Value '%{sync_method_value}' is not allowed for 'sync_method' configuration parameter. Valid values are 'rsync' and 'none' rsync_error: |- diff --git a/source/spec/vagrant-openstack-provider/action/create_server_spec.rb b/source/spec/vagrant-openstack-provider/action/create_server_spec.rb index 2e8711e..f0758aa 100644 --- a/source/spec/vagrant-openstack-provider/action/create_server_spec.rb +++ b/source/spec/vagrant-openstack-provider/action/create_server_spec.rb @@ -58,7 +58,7 @@ r.stub(:resolve_flavor).with(anything) do Flavor.new('flavor-01', 'small', nil, nil, nil) end - r.stub(:resolve_image).with(anything, anything) do + r.stub(:resolve_image).with(anything) do Item.new('image-01', 'ubuntu') end r.stub(:resolve_volume_boot).with(anything) { 'ubuntu-drive' } diff --git a/source/spec/vagrant-openstack-provider/client/nova_spec.rb b/source/spec/vagrant-openstack-provider/client/nova_spec.rb index 3a4ddd2..0dac018 100644 --- a/source/spec/vagrant-openstack-provider/client/nova_spec.rb +++ b/source/spec/vagrant-openstack-provider/client/nova_spec.rb @@ -222,10 +222,7 @@ headers: { 'Accept' => 'application/json', - 'Accept-Encoding' => 'gzip, deflate', - 'Content-Length' => '248', 'Content-Type' => 'application/json', - 'User-Agent' => 'Ruby', 'X-Auth-Token' => '123456' }) .to_return(status: 202, body: '{ "server": { "id": "o1o2o3" } }') @@ -250,10 +247,7 @@ headers: { 'Accept' => 'application/json', - 'Accept-Encoding' => 'gzip, deflate', - 'Content-Length' => '127', 'Content-Type' => 'application/json', - 'User-Agent' => 'Ruby', 'X-Auth-Token' => '123456' }) .to_return(status: 202, body: '{ "server": { "id": "o1o2o3" } }') diff --git a/source/spec/vagrant-openstack-provider/config_resolver_spec.rb b/source/spec/vagrant-openstack-provider/config_resolver_spec.rb index 100b6da..3a25366 100644 --- a/source/spec/vagrant-openstack-provider/config_resolver_spec.rb +++ b/source/spec/vagrant-openstack-provider/config_resolver_spec.rb @@ -190,7 +190,7 @@ [Item.new('img-001', 'image-01'), Item.new('img-002', 'image-02')] end - @action.resolve_image(env, config.image).should eq(Item.new('img-001', 'image-01')) + @action.resolve_image(env).should eq(Item.new('img-001', 'image-01')) end end context 'with name' do @@ -201,7 +201,7 @@ [Item.new('img-001', 'image-01'), Item.new('img-002', 'image-02')] end - @action.resolve_image(env, config.image).should eq(Item.new('img-002', 'image-02')) + @action.resolve_image(env).should eq(Item.new('img-002', 'image-02')) end end context 'with invalid identifier' do @@ -212,7 +212,7 @@ [Item.new('img-001', 'image-01'), Item.new('img-002', 'image-02')] end - expect { @action.resolve_image(env, config.image) }.to raise_error(Errors::NoMatchingImage) + expect { @action.resolve_image(env) }.to raise_error(Errors::NoMatchingImage) end end context 'with no images in config' do @@ -223,7 +223,7 @@ [Item.new('img-001', 'image-01'), Item.new('img-002', 'image-02')] end - @action.resolve_image(env, config.image).should eq(nil) + @action.resolve_image(env).should eq(nil) end end end @@ -520,11 +520,7 @@ context 'with string volume id' do it 'returns normalized volume' do config.stub(:volume_boot) { '001' } - nova.stub(:get_all_images).with(anything) do - [Item.new('img-001', 'image-01'), - Item.new('img-002', 'image-02')] - end - expect(@action.resolve_volume_boot(env)).to eq id: '001', image: nil, device: 'vda', size: nil, delete_on_destroy: nil + expect(@action.resolve_volume_boot(env)).to eq id: '001', device: 'vda' end end @@ -535,18 +531,14 @@ [Item.new('img-001', 'image-01'), Item.new('img-002', 'image-02')] end - expect(@action.resolve_volume_boot(env)).to eq id: '001', image: nil, device: 'vda', size: nil, delete_on_destroy: nil + expect(@action.resolve_volume_boot(env)).to eq id: '001', device: 'vda' end end context 'with hash volume id' do it 'returns normalized volume' do config.stub(:volume_boot) { { id: '001' } } - nova.stub(:get_all_images).with(anything) do - [Item.new('img-001', 'image-01'), - Item.new('img-002', 'image-02')] - end - expect(@action.resolve_volume_boot(env)).to eq id: '001', image: nil, device: 'vda', size: nil, delete_on_destroy: nil + expect(@action.resolve_volume_boot(env)).to eq id: '001', device: 'vda' end end @@ -557,18 +549,14 @@ [Item.new('img-001', 'image-01'), Item.new('img-002', 'image-02')] end - expect(@action.resolve_volume_boot(env)).to eq id: '001', image: nil, device: 'vda', size: nil, delete_on_destroy: nil + expect(@action.resolve_volume_boot(env)).to eq id: '001', device: 'vda' end end context 'with hash volume id and device' do it 'returns normalized volume' do config.stub(:volume_boot) { { id: '001', device: 'vdb' } } - nova.stub(:get_all_images).with(anything) do - [Item.new('img-001', 'image-01'), - Item.new('img-002', 'image-02')] - end - expect(@action.resolve_volume_boot(env)).to eq id: '001', image: nil, device: 'vdb', size: nil, delete_on_destroy: nil + expect(@action.resolve_volume_boot(env)).to eq id: '001', device: 'vdb' end end @@ -579,14 +567,14 @@ [Item.new('img-001', 'image-01'), Item.new('img-002', 'image-02')] end - expect(@action.resolve_volume_boot(env)).to eq id: '001', image: nil, device: 'vdb', size: nil, delete_on_destroy: nil + expect(@action.resolve_volume_boot(env)).to eq id: '001', device: 'vdb' end end context 'with empty hash' do it 'raises an error' do config.stub(:volume_boot) { {} } - expect { @action.resolve_volume_boot(env) }.to raise_error(Errors::ConflictVolumeNameId) + expect { @action.resolve_volume_boot(env) }.to raise_error(Errors::ConflictBootVolume) end end @@ -628,14 +616,42 @@ context 'with hash containing a name and an image_name' do it 'raises an error' do config.stub(:volume_boot) { { name: 'vol-01', image: 'img_001' } } - expect { @action.resolve_volume_boot(env) }.to raise_error(Errors::ConflictVolumeNameId) + expect { @action.resolve_volume_boot(env) }.to raise_error(Errors::ConflictBootVolume) + end + end + + context 'with hash containing a name and a size' do + it 'raises an error' do + config.stub(:volume_boot) { { name: 'vol-01', size: '10' } } + expect { @action.resolve_volume_boot(env) }.to raise_error(Errors::ConflictBootVolume) + end + end + + context 'with hash containing a name and a delete_on_destroy indication' do + it 'raises an error' do + config.stub(:volume_boot) { { name: 'vol-01', delete_on_destroy: 'true' } } + expect { @action.resolve_volume_boot(env) }.to raise_error(Errors::ConflictBootVolume) end end context 'with hash containing a volume_id and an image_name' do it 'raises an error' do config.stub(:volume_boot) { { id: 'id', image: 'img_001' } } - expect { @action.resolve_volume_boot(env) }.to raise_error(Errors::ConflictVolumeNameId) + expect { @action.resolve_volume_boot(env) }.to raise_error(Errors::ConflictBootVolume) + end + end + + context 'with hash containing a volume_id and a size' do + it 'raises an error' do + config.stub(:volume_boot) { { id: 'id', size: '10' } } + expect { @action.resolve_volume_boot(env) }.to raise_error(Errors::ConflictBootVolume) + end + end + + context 'with hash containing a volume_id and a delete_on_destroy indication' do + it 'raises an error' do + config.stub(:volume_boot) { { id: 'id', delete_on_destroy: 'true' } } + expect { @action.resolve_volume_boot(env) }.to raise_error(Errors::ConflictBootVolume) end end @@ -653,7 +669,7 @@ [Item.new('img-001', 'image-01'), Item.new('img-002', 'image-02')] end - expect(@action.resolve_volume_boot(env)).to eq id: nil, image: 'img-001', device: 'vda', size: '10', delete_on_destroy: 'true' + expect(@action.resolve_volume_boot(env)).to eq image: 'img-001', device: 'vda', size: '10', delete_on_destroy: 'true' end end @@ -664,7 +680,7 @@ [Item.new('img-001', 'image-01'), Item.new('img-002', 'image-02')] end - expect(@action.resolve_volume_boot(env)).to eq id: nil, image: 'img-001', device: 'vdb', size: '10', delete_on_destroy: 'false' + expect(@action.resolve_volume_boot(env)).to eq image: 'img-001', device: 'vdb', size: '10', delete_on_destroy: 'false' end end end @@ -781,7 +797,7 @@ context 'with empty hash' do it 'raises an error' do config.stub(:volumes) { [{}] } - expect { @action.resolve_volumes(env) }.to raise_error(Errors::ConflictVolumeNameId) + expect { @action.resolve_volumes(env) }.to raise_error(Errors::ConflictBootVolume) end end