Merge pull request #91 from alphagov/storage-profile-href

[#62454712] Storage profile should be provisioned using name only

README.md

@@ -79,11 +79,29 @@ VM. The easiest thing to do is create a local shell script called
     export VCLOUD\_NETWORK1\_IP="ip-on-primary-network"
     export VCLOUD\_NETWORK2\_IP="ip-on-secondary-network"
     export VCLOUD\_TEST\_STORAGE\_PROFILE="storage-profile-name"
-    export VCLOUD\_TEST\_STORAGE\_PROFILE_HREF="url-of-named-storage-profile"
     export VCLOUD\_EDGE\_GATEWAY="name-of-edge-gateway-in-vdc"
 
 Then run this before you run the integration tests.
 
+### Specific integration tests
+
+#### Storage profile tests
+
+There is an integration test to check storage profile behaviour, but it requires a lot of set-up so it is not called by the rake task. If you wish to run it you need access to an environment that has two VDCs, each one containing a storage profile with the same name. This named storage profile needs to be different from teh default storage profile.
+
+You will need to set the following environment variables:
+
+      export VDC\_NAME\_1="Name of the first vDC"
+      export VDC\_NAME\_2="Name of the second vDC"
+      export VCLOUD\_CATALOG\_NAME="Catalog name" # Can be the same as above settings if appropriate
+      export VCLOUD\_TEMPLATE\_NAME="Template name" # Can be the same as above setting if appropriate
+      export VCLOUD\_STORAGE\_PROFILE\_NAME="Storage profile name" # This needs to exist in both vDCs
+      export VDC\_1\_STORAGE\_PROFILE\_HREF="Href of the named storage profile in vDC 1"
+      export VDC\_2\_STORAGE\_PROFILE\_HREF="Href of the named storage profile in vDC 2"
+      export DEFAULT\_STORAGE\_PROFILE\_NAME="Default storage profile name"
+      export DEFAULT\_STORAGE\_PROFILE\_HREF="Href of default storage profile"
+
+To run this test: `rspec spec/integration/launcher/storage_profile_integration_test.rb`
 
 [vcloudwalker]: https://github.com/alphagov/vcloud-walker
 [edgegateway]: docs/edgegateway.md

lib/vcloud/core/vm.rb

@@ -113,25 +113,32 @@ def generate_preamble(script_path, script_post_processor, vars)
       end
 
       def update_storage_profile storage_profile
-        if valid_storage_profile?(storage_profile)
-          @fog_interface.put_vm(id, name, {:StorageProfile => storage_profile})
-        else
-          Vcloud.logger.info("bad storage profile: #{storage_profile}")
-          false
-        end
+        storage_profile_href = get_storage_profile_href_by_name(storage_profile, @vapp.name)
+        @fog_interface.put_vm(id, name, {:StorageProfile => { name: storage_profile, href: storage_profile_href } })
       end
 
       private
       def virtual_hardware_section
         @vcloud_attributes[:'ovf:VirtualHardwareSection'][:'ovf:Item']
       end
 
-      def id_prefix
-        'vm'
+      def get_storage_profile_href_by_name(storage_profile_name, vapp_name)
+        q = Query.new('vApp', :filter => "name==#{vapp_name}")
+        vdc_results = q.get_all_results
+        vdc_name = vdc_results.first[:vdcName]
+
+        q = Query.new('orgVdcStorageProfile', :filter => "name==#{storage_profile_name};vdcName==#{vdc_name}")
+        sp_results = q.get_all_results
+
+        if sp_results.empty? or !sp_results.first.has_key?(:href)
+          raise "storage profile not found"
+        else
+          return sp_results.first[:href]
+        end
       end
 
-      def valid_storage_profile?(storage_profile)
-        storage_profile.key?(:name) && storage_profile.key?(:href)
+      def id_prefix
+        'vm'
       end
 
     end

lib/vcloud/version.rb

@@ -1,3 +1,3 @@
 module Vcloud
-  VERSION = '0.6.0'
+  VERSION = '0.7.0'
 end

spec/integration/launcher/data/happy_path.yaml.erb

@@ -29,6 +29,4 @@ vapps:
         script_path: <%= bootstrap_script %>
         vars:
           message: hello world
-      storage_profile:
-        name: <%= storage_profile %>
-        href: <%= storage_profile_href %>
+      storage_profile: <%= storage_profile %>

spec/integration/launcher/data/storage_profile.yaml.erb

@@ -0,0 +1,48 @@
+---
+vapps:
+  - name: <%= vapp_name_1 %>
+    vdc_name: <%= vdc_name_1 %>
+    catalog: <%= catalog %>
+    catalog_item: <%= vapp_template %>
+    vm:
+      hardware_config: &hardware_config
+        memory: 4096
+        cpu: 2
+      bootstrap: &bootstrap
+        script_path: <%= bootstrap_script %>
+        vars:
+          message: hello world
+      storage_profile: <%= storage_profile %>
+  - name: <%= vapp_name_2 %>
+    vdc_name: <%= vdc_name_2 %>
+    catalog: <%= catalog %>
+    catalog_item: <%= vapp_template %>
+    vm:
+      hardware_config:
+        <<: *hardware_config
+      bootstrap:
+        <<: *bootstrap
+      storage_profile: <%= storage_profile %>
+  - name: <%= vapp_name_3 %>
+    vdc_name: <%= vdc_name_1 %>
+    catalog: <%= catalog %>
+    catalog_item: <%= vapp_template %>
+    vm:
+      hardware_config:
+        <<: *hardware_config
+      metadata:
+        storage_profile_test_vm: true
+      bootstrap:
+        <<: *bootstrap
+  - name: <%= vapp_name_4 %>
+    vdc_name: <%= vdc_name_1 %>
+    catalog: <%= catalog %>
+    catalog_item: <%= vapp_template %>
+    vm:
+      hardware_config:
+        <<: *hardware_config
+      metadata:
+        storage_profile_test_vm: true
+      bootstrap:
+        <<: *bootstrap
+      storage_profile: <%= nonsense_storage_profile %>

spec/integration/launcher/storage_profile_integration_test.rb

@@ -0,0 +1,113 @@
+require 'spec_helper'
+
+describe Vcloud::Launch do
+  context "storage profile" do
+    before(:all) do
+      @test_data = define_test_data
+      @config_yaml = generate_input_yaml_config(@test_data, File.join(File.dirname(__FILE__), 'data/storage_profile.yaml.erb'))
+      @fog_interface = Vcloud::Fog::ServiceInterface.new
+      Vcloud::Launch.new.run(@config_yaml, {:no_power_on => true})
+
+      @vapp_query_result_1 = @fog_interface.get_vapp_by_name_and_vdc_name(@test_data[:vapp_name_1], @test_data[:vdc_name_1])
+      @vapp_id_1 = @vapp_query_result_1[:href].split('/').last
+      @vapp_1 = @fog_interface.get_vapp @vapp_id_1
+      @vm_1 = @vapp_1[:Children][:Vm].first
+
+      @vapp_query_result_2 = @fog_interface.get_vapp_by_name_and_vdc_name(@test_data[:vapp_name_2], @test_data[:vdc_name_2])
+      @vapp_id_2 = @vapp_query_result_2[:href].split('/').last
+      @vapp_2 = @fog_interface.get_vapp @vapp_id_2
+      @vm_2 = @vapp_2[:Children][:Vm].first
+
+      @vapp_query_result_3 = @fog_interface.get_vapp_by_name_and_vdc_name(@test_data[:vapp_name_3], @test_data[:vdc_name_1])
+      @vapp_id_3 = @vapp_query_result_3[:href].split('/').last
+      @vapp_3 = @fog_interface.get_vapp @vapp_id_3
+      @vm_3 = @vapp_3[:Children][:Vm].first
+
+      @vapp_query_result_4 = @fog_interface.get_vapp_by_name_and_vdc_name(@test_data[:vapp_name_4], @test_data[:vdc_name_1])
+      @vapp_id_4 = @vapp_query_result_4[:href].split('/').last
+      @vapp_4 = @fog_interface.get_vapp @vapp_id_4
+      @vm_4 = @vapp_4[:Children][:Vm].first
+    end
+
+    it "vdc 1 should have a storage profile without the href being specified" do
+        @vm_1[:StorageProfile][:name].should == @test_data[:storage_profile]
+    end
+
+    it "vdc 1's storage profile should have the expected href" do
+        @vm_1[:StorageProfile][:href].should == @test_data[:vdc_1_sp_href]
+    end
+
+    it "vdc 2 should have the same named storage profile as vdc 1" do
+        @vm_2[:StorageProfile][:name].should == @test_data[:storage_profile]
+    end
+
+    it "the storage profile in vdc 2 should have a different href to the storage profile in vdc 1" do
+        @vm_2[:StorageProfile][:href].should == @test_data[:vdc_2_sp_href]
+    end
+
+    it "when a storage profile is not specified, vm uses the default and continues" do
+        @vm_3[:StorageProfile][:name].should == @test_data[:default_storage_profile_name]
+        @vm_3[:StorageProfile][:href].should == @test_data[:default_storage_profile_href]
+    end
+
+    it "when a storage profile is not specified, customize continues with other customizations" do
+        @vm_3_id = @vm_3[:href].split('/').last
+        @vm_3_metadata = Vcloud::Core::Vm.get_metadata @vm_3_id
+        @vm_3_metadata[:storage_profile_test_vm].should == true
+    end
+
+    it "when a storage profile specified does not exist, vm uses the default" do
+        @vm_4[:StorageProfile][:name].should == @test_data[:default_storage_profile_name]
+        @vm_4[:StorageProfile][:href].should == @test_data[:default_storage_profile_href]
+    end
+
+    # This is a bug - if it has failed customization it should let the user know
+    it "when storage profile specified doesn't exist, it errors and continues" do
+        @vm_4_id = @vm_4[:href].split('/').last
+        @vm_4_metadata = Vcloud::Core::Vm.get_metadata @vm_4_id
+        @vm_4_metadata[:storage_profile_test_vm].should be_nil
+    end
+
+    after(:all) do
+      unless ENV['VCLOUD_TOOLS_RSPEC_NO_DELETE_VAPP']
+        File.delete @config_yaml
+        @fog_interface.delete_vapp(@vapp_id_1).should == true
+        @fog_interface.delete_vapp(@vapp_id_2).should == true
+        @fog_interface.delete_vapp(@vapp_id_3).should == true
+        @fog_interface.delete_vapp(@vapp_id_4).should == true
+      end
+    end
+
+  end
+
+end
+
+def generate_input_yaml_config test_namespace, input_erb_config
+  input_erb_config = input_erb_config
+  e = ERB.new(File.open(input_erb_config).read)
+  output_yaml_config = File.join(File.dirname(input_erb_config), "output_#{Time.now.strftime('%s')}.yaml")
+  File.open(output_yaml_config, 'w') { |f|
+    f.write e.result(OpenStruct.new(test_namespace).instance_eval { binding })
+  }
+  output_yaml_config
+end
+
+def define_test_data
+  {
+      vapp_name_1: "vdc-1-sp-#{Time.now.strftime('%s')}",
+      vapp_name_2: "vdc-2-sp-#{Time.now.strftime('%s')}",
+      vapp_name_3: "vdc-3-sp-#{Time.now.strftime('%s')}",
+      vapp_name_4: "vdc-4-sp-#{Time.now.strftime('%s')}",
+      vdc_name_1: ENV['VDC_NAME_1'],
+      vdc_name_2: ENV['VDC_NAME_2'],
+      catalog: ENV['VCLOUD_CATALOG_NAME'],
+      vapp_template: ENV['VCLOUD_TEMPLATE_NAME'],
+      storage_profile: ENV['VCLOUD_STORAGE_PROFILE_NAME'],
+      vdc_1_sp_href: ENV['VDC_1_STORAGE_PROFILE_HREF'],
+      vdc_2_sp_href: ENV['VDC_2_STORAGE_PROFILE_HREF'],
+      default_storage_profile_name: ENV['DEFAULT_STORAGE_PROFILE_NAME'],
+      default_storage_profile_href: ENV['DEFAULT_STORAGE_PROFILE_HREF'],
+      nonsense_storage_profile: "nonsense-storage-profile-name",
+      bootstrap_script: File.join(File.dirname(__FILE__), "data/basic_preamble_test.erb"),
+  }
+end

spec/integration/launcher/vcloud_launcher_spec.rb

@@ -112,7 +112,6 @@
 
       it "should assign storage profile to the VM" do
         @vm[:StorageProfile][:name].should == @test_data[:storage_profile]
-        @vm[:StorageProfile][:href].should == @test_data[:storage_profile_href]
       end
 
     end
@@ -169,4 +168,19 @@ def define_test_data
     }
   end
 
+  def define_test_data
+    {
+        vapp_name: "vapp-vcloud-tools-tests-#{Time.now.strftime('%s')}",
+        vdc_name: ENV['VCLOUD_VDC_NAME'],
+        catalog: ENV['VCLOUD_CATALOG_NAME'],
+        vapp_template: ENV['VCLOUD_TEMPLATE_NAME'],
+        network1: ENV['VCLOUD_NETWORK1_NAME'],
+        network2: ENV['VCLOUD_NETWORK2_NAME'],
+        network1_ip: ENV['VCLOUD_NETWORK1_IP'],
+        network2_ip: ENV['VCLOUD_NETWORK2_IP'],
+        storage_profile: ENV['VCLOUD_STORAGE_PROFILE_NAME'],
+        bootstrap_script: File.join(File.dirname(__FILE__), "data/basic_preamble_test.erb"),
+        date_metadata: DateTime.parse('2013-10-23 15:34:00 +0000')
+    }
+  end
 end

spec/vcloud/core/vm_spec.rb

@@ -190,33 +190,58 @@ module Core
       end
 
       context "update storage profiles" do
-        context "return false and log error if" do
-          before(:each) do
-            @fog_interface.should_not_receive(:put_vm)
-          end
+        it "should update the storage profile" do
+          storage_profile = 'storage_profile_name'
+          vdc_results = [
+            { :vdcName => 'vdc-test-1' }
+          ]
+          mock_vdc_query = double(:query, :get_all_results => vdc_results)
+
+          storage_profile_results = [
+            { :href => 'test-href' }
+          ]
+          mock_sp_query = double(:query, :get_all_results => storage_profile_results)
 
-          it "storage profile config has no name" do
-            bad_storage_profile = { :href => 'https://api.example.com/api/vdcStorageProfile/1234d210-92c9-4514-b146-4b625e6c74dd' }
-            Vcloud.logger.should_receive(:info).with("bad storage profile: #{bad_storage_profile}")
+          Vcloud::Query.should_receive(:new).with('vApp', :filter => "name==test-vm-1").and_return(mock_vdc_query)
+          Vcloud::Query.should_receive(:new).with('orgVdcStorageProfile', :filter => "name==storage_profile_name;vdcName==vdc-test-1").and_return(mock_sp_query)
+
+          generated_storage_profile = { name: 'storage_profile_name', href: 'test-href' }
+          @fog_interface.should_receive(:put_vm).with('vm-1234', 'test-vapp-1', { :StorageProfile => generated_storage_profile} ).and_return(true)
+          @vm.update_storage_profile(storage_profile).should == true
+        end
 
-            @vm.update_storage_profile(bad_storage_profile).should == false
-          end
+        it "should raise an error if storage profile is not found" do
+          storage_profile = 'storage_profile_name'
+          vdc_results = [
+            { :vdcName => 'vdc-test-1' }
+          ]
+          mock_vdc_query = double(:query, :get_all_results => vdc_results)
 
-          it "storage profile has no href" do
-            bad_storage_profile = { :name => "basic-storage-profile" }
-            Vcloud.logger.should_receive(:info).with("bad storage profile: #{bad_storage_profile}")
+          storage_profile_results = []
+          mock_sp_query = double(:query, :get_all_results => storage_profile_results)
 
-            @vm.update_storage_profile(bad_storage_profile).should == false
-          end
+          Vcloud::Query.should_receive(:new).with('vApp', :filter => "name==test-vm-1").and_return(mock_vdc_query)
+          Vcloud::Query.should_receive(:new).with('orgVdcStorageProfile', :filter => "name==storage_profile_name;vdcName==vdc-test-1").and_return(mock_sp_query)
 
+          expect{ @vm.update_storage_profile(storage_profile) }.to raise_error("storage profile not found" )
         end
 
-        it "should update the storage profile" do
-          storage_profile = {:name => "basic-storage-profile", :href => 'https://api.example.com/api/vdcStorageProfile/1234d210-92c9-4514-b146-4b625e6c74dd'}
-          @fog_interface.should_receive(:put_vm).with('vm-1234', 'test-vapp-1', { :StorageProfile => storage_profile} ).and_return(true)
+        it "should raise an error if storage profile id is in unexpected format" do
+          storage_profile = 'storage_profile_name'
+          vdc_results = [
+            { :vdcName => 'vdc-test-1' }
+          ]
+          mock_vdc_query = double(:query, :get_all_results => vdc_results)
 
-          @vm.update_storage_profile(storage_profile).should == true
+          storage_profile_results = [ { :id => 'test-href'  }]
+          mock_sp_query = double(:query, :get_all_results => storage_profile_results)
+
+          Vcloud::Query.should_receive(:new).with('vApp', :filter => "name==test-vm-1").and_return(mock_vdc_query)
+          Vcloud::Query.should_receive(:new).with('orgVdcStorageProfile', :filter => "name==storage_profile_name;vdcName==vdc-test-1").and_return(mock_sp_query)
+
+          expect{ @vm.update_storage_profile(storage_profile) }.to raise_error("storage profile not found" )
         end
+
       end
 
     end