From 30407da627b68f8d2afbfd12e27deb1864c0fedf Mon Sep 17 00:00:00 2001 From: Abel Hu Date: Fri, 31 Mar 2017 10:53:46 +0800 Subject: [PATCH] Fix #259: Use Json format to store light stemcell information. --- src/bosh_azure_cpi/lib/cloud/azure/helpers.rb | 5 +- .../lib/cloud/azure/light_stemcell_manager.rb | 10 +- src/bosh_azure_cpi/spec/unit/helpers_spec.rb | 89 ++++------- .../spec/unit/light_stemcell_manager_spec.rb | 144 ++++++++---------- 4 files changed, 101 insertions(+), 147 deletions(-) diff --git a/src/bosh_azure_cpi/lib/cloud/azure/helpers.rb b/src/bosh_azure_cpi/lib/cloud/azure/helpers.rb index 483b96ede..c3e6105fa 100644 --- a/src/bosh_azure_cpi/lib/cloud/azure/helpers.rb +++ b/src/bosh_azure_cpi/lib/cloud/azure/helpers.rb @@ -413,10 +413,7 @@ def initialize(uri, metadata) @name = @metadata['name'] @version = @metadata['version'] @disk_size = @metadata['disk'].nil? ? 3072 : @metadata['disk'].to_i - - if @metadata.has_key?('image') - @image = @metadata['image'].kind_of?(Hash) ? @metadata['image'] : eval(@metadata['image']) - end + @image = @metadata['image'] end def is_light_stemcell? diff --git a/src/bosh_azure_cpi/lib/cloud/azure/light_stemcell_manager.rb b/src/bosh_azure_cpi/lib/cloud/azure/light_stemcell_manager.rb index 5875f2a7f..590cbbc94 100644 --- a/src/bosh_azure_cpi/lib/cloud/azure/light_stemcell_manager.rb +++ b/src/bosh_azure_cpi/lib/cloud/azure/light_stemcell_manager.rb @@ -25,7 +25,9 @@ def create_stemcell(stemcell_properties) cloud_error("Cannot find the light stemcell (#{stemcell_properties['image']}) in the location `#{@default_location}'") unless platform_image_exists?(@default_location, stemcell_properties) stemcell_name = "#{LIGHT_STEMCELL_PREFIX}-#{SecureRandom.uuid}" - @blob_manager.create_empty_page_blob(@default_storage_account_name, STEMCELL_CONTAINER, "#{stemcell_name}.vhd", 1, stemcell_properties) + metadata = stemcell_properties.dup + metadata['image'] = JSON.dump(metadata['image']) + @blob_manager.create_empty_page_blob(@default_storage_account_name, STEMCELL_CONTAINER, "#{stemcell_name}.vhd", 1, metadata) stemcell_name end @@ -47,7 +49,11 @@ def get_stemcell_info(name) private def get_metadata(name) - @blob_manager.get_blob_metadata(@default_storage_account_name, STEMCELL_CONTAINER, "#{name}.vhd") + metadata = @blob_manager.get_blob_metadata(@default_storage_account_name, STEMCELL_CONTAINER, "#{name}.vhd") + return nil if metadata.nil? + + metadata['image'] = JSON.parse(metadata['image']) + metadata end def platform_image_exists?(location, stemcell_properties) diff --git a/src/bosh_azure_cpi/spec/unit/helpers_spec.rb b/src/bosh_azure_cpi/spec/unit/helpers_spec.rb index aed95c1f8..b1deda975 100644 --- a/src/bosh_azure_cpi/spec/unit/helpers_spec.rb +++ b/src/bosh_azure_cpi/spec/unit/helpers_spec.rb @@ -617,70 +617,35 @@ def set_logger(logger) end context "when metadata contains image" do - context "and image is a hash" do - let(:uri) { "fake-uri" } - let(:metadata) { - { - "name" => "fake-name", - "version" => "fake-version", - "infrastructure" => "azure", - "hypervisor" => "hyperv", - "disk" => "3072", - "disk_format" => "vhd", - "container_format" => "bare", - "os_type" => "linux", - "os_distro" => "ubuntu", - "architecture" => "x86_64", - "image" => "{\"publisher\"=>\"bosh\", \"offer\"=>\"UbuntuServer\", \"sku\"=>\"trusty\", \"version\"=>\"fake-version\"}" - } - } - - it "should return correct values" do - stemcell_info = Bosh::AzureCloud::Helpers::StemcellInfo.new(uri, metadata) - expect(stemcell_info.uri).to eq("fake-uri") - expect(stemcell_info.os_type).to eq("linux") - expect(stemcell_info.name).to eq("fake-name") - expect(stemcell_info.version).to eq("fake-version") - expect(stemcell_info.disk_size).to eq(3072) - expect(stemcell_info.is_light_stemcell?).to be(true) - expect(stemcell_info.image_reference['publisher']).to eq('bosh') - expect(stemcell_info.image_reference['offer']).to eq('UbuntuServer') - expect(stemcell_info.image_reference['sku']).to eq('trusty') - expect(stemcell_info.image_reference['version']).to eq('fake-version') - end - end - - context "and image is a string" do - let(:uri) { "fake-uri" } - let(:metadata) { - { - "name" => "fake-name", - "version" => "fake-version", - "infrastructure" => "azure", - "hypervisor" => "hyperv", - "disk" => "3072", - "disk_format" => "vhd", - "container_format" => "bare", - "os_type" => "linux", - "os_distro" => "ubuntu", - "architecture" => "x86_64", - "image" => {"publisher"=>"bosh", "offer"=>"UbuntuServer", "sku"=>"trusty", "version"=>"fake-version"} - } + let(:uri) { "fake-uri" } + let(:metadata) { + { + "name" => "fake-name", + "version" => "fake-version", + "infrastructure" => "azure", + "hypervisor" => "hyperv", + "disk" => "3072", + "disk_format" => "vhd", + "container_format" => "bare", + "os_type" => "linux", + "os_distro" => "ubuntu", + "architecture" => "x86_64", + "image" => {"publisher"=>"bosh", "offer"=>"UbuntuServer", "sku"=>"trusty", "version"=>"fake-version"} } + } - it "should return correct values" do - stemcell_info = Bosh::AzureCloud::Helpers::StemcellInfo.new(uri, metadata) - expect(stemcell_info.uri).to eq("fake-uri") - expect(stemcell_info.os_type).to eq("linux") - expect(stemcell_info.name).to eq("fake-name") - expect(stemcell_info.version).to eq("fake-version") - expect(stemcell_info.disk_size).to eq(3072) - expect(stemcell_info.is_light_stemcell?).to be(true) - expect(stemcell_info.image_reference['publisher']).to eq('bosh') - expect(stemcell_info.image_reference['offer']).to eq('UbuntuServer') - expect(stemcell_info.image_reference['sku']).to eq('trusty') - expect(stemcell_info.image_reference['version']).to eq('fake-version') - end + it "should return correct values" do + stemcell_info = Bosh::AzureCloud::Helpers::StemcellInfo.new(uri, metadata) + expect(stemcell_info.uri).to eq("fake-uri") + expect(stemcell_info.os_type).to eq("linux") + expect(stemcell_info.name).to eq("fake-name") + expect(stemcell_info.version).to eq("fake-version") + expect(stemcell_info.disk_size).to eq(3072) + expect(stemcell_info.is_light_stemcell?).to be(true) + expect(stemcell_info.image_reference['publisher']).to eq('bosh') + expect(stemcell_info.image_reference['offer']).to eq('UbuntuServer') + expect(stemcell_info.image_reference['sku']).to eq('trusty') + expect(stemcell_info.image_reference['version']).to eq('fake-version') end end end diff --git a/src/bosh_azure_cpi/spec/unit/light_stemcell_manager_spec.rb b/src/bosh_azure_cpi/spec/unit/light_stemcell_manager_spec.rb index 532c74e58..181c5e189 100644 --- a/src/bosh_azure_cpi/spec/unit/light_stemcell_manager_spec.rb +++ b/src/bosh_azure_cpi/spec/unit/light_stemcell_manager_spec.rb @@ -7,7 +7,9 @@ let(:light_stemcell_manager) { Bosh::AzureCloud::LightStemcellManager.new(blob_manager, storage_account_manager, azure_client2) } let(:stemcell_container) { 'stemcell' } - let(:stemcell_name) { "fake-stemcell-name" } + let(:prefix) { 'bosh-light-stemcell' } + let(:uuid) { 'fac96afe-6953-4e74-b9a8-80ae571dfca7' } + let(:stemcell_name) { "#{prefix}-#{uuid}" } let(:location) { 'fake-location' } let(:version) { "fake-version" } let(:storage_account) { @@ -21,6 +23,59 @@ :storage_table_host => "fake-table-endpoint" } } + let(:stemcell_properties) { + { + "name" => "fake-name", + "version" => version, + "infrastructure" => "azure", + "hypervisor" => "hyperv", + "disk" => "3072", + "disk_format" => "vhd", + "container_format" => "bare", + "os_type" => "linux", + "os_distro" => "ubuntu", + "architecture" => "x86_64", + "image" => { + "publisher" => "bosh", + "offer" => "UbuntuServer", + "sku" => "trusty", + "version" => version + } + } + } + let(:metadata) { + { + "name" => "fake-name", + "version" => version, + "infrastructure" => "azure", + "hypervisor" => "hyperv", + "disk" => "3072", + "disk_format" => "vhd", + "container_format" => "bare", + "os_type" => "linux", + "os_distro" => "ubuntu", + "architecture" => "x86_64", + "image" => JSON.dump({ + "publisher" => "bosh", + "offer" => "UbuntuServer", + "sku" => "trusty", + "version" => version + }) + } + } + let(:versions) { + [ + { + :id => 'a', + :name => version, + :location => location + }, { + :id => 'c', + :name => 'd', + :location => location + } + ] + } before do allow(storage_account_manager).to receive(:default_storage_account). @@ -29,16 +84,14 @@ describe "#delete_stemcell" do context "when the stemcell exists" do - let(:metadata) { { "foo" => "bar" } } - before do allow(blob_manager).to receive(:get_blob_metadata). - with(MOCK_DEFAULT_STORAGE_ACCOUNT_NAME, stemcell_container, "#{stemcell_name}.vhd"). and_return(metadata) end it "should delete the stemcell" do - expect(blob_manager).to receive(:delete_blob) + expect(blob_manager).to receive(:delete_blob). + with(MOCK_DEFAULT_STORAGE_ACCOUNT_NAME, stemcell_container, "#{stemcell_name}.vhd") light_stemcell_manager.delete_stemcell(stemcell_name) end @@ -59,54 +112,19 @@ end describe "#create_stemcell" do - let(:stemcell_properties) { - { - "name" => "fake-name", - "version" => version, - "infrastructure" => "azure", - "hypervisor" => "hyperv", - "disk" => "3072", - "disk_format" => "vhd", - "container_format" => "bare", - "os_type" => "linux", - "os_distro" => "ubuntu", - "architecture" => "x86_64", - "image" => { - "publisher" => "bosh", - "offer" => "UbuntuServer", - "sku" => "trusty", - "version" => version - } - } - } - context "when the platform image exists" do - let(:versions) { - [ - { - :id => 'a', - :name => version, - :location => location - }, { - :id => 'c', - :name => 'd', - :location => location - } - ] - } - before do + allow(SecureRandom).to receive(:uuid).and_return(uuid) allow(azure_client2).to receive(:list_platform_image_versions). - and_return(versions) - allow(blob_manager).to receive(:create_empty_page_blob). - and_return(true) + and_return(versions) end it "should create the stemcell" do - expect(azure_client2).to receive(:list_platform_image_versions) - expect(blob_manager).to receive(:create_empty_page_blob) + expect(blob_manager).to receive(:create_empty_page_blob). + with(MOCK_DEFAULT_STORAGE_ACCOUNT_NAME, stemcell_container, "#{stemcell_name}.vhd", 1, metadata). + and_return(true) - expect(light_stemcell_manager.create_stemcell(stemcell_properties)).to start_with('bosh-light-stemcell') + expect(light_stemcell_manager.create_stemcell(stemcell_properties)).to eq(stemcell_name) end end @@ -158,22 +176,6 @@ end context "when the blob exists in the default storage account" do - let(:metadata) { - { - "name" => "fake-name", - "version" => version, - "infrastructure" => "azure", - "hypervisor" => "hyperv", - "disk" => "3072", - "disk_format" => "vhd", - "container_format" => "bare", - "os_type" => "linux", - "os_distro" => "ubuntu", - "architecture" => "x86_64", - "image" => "{\"publisher\"=>\"bosh\", \"offer\"=>\"UbuntuServer\", \"sku\"=>\"trusty\", \"version\"=>\"#{version}\"}" - } - } - before do allow(blob_manager).to receive(:get_blob_metadata). with(MOCK_DEFAULT_STORAGE_ACCOUNT_NAME, stemcell_container, "#{stemcell_name}.vhd"). @@ -197,20 +199,6 @@ end context "and the platform image exists" do - let(:versions) { - [ - { - :id => 'a', - :name => version, - :location => location - }, { - :id => 'c', - :name => 'd', - :location => location - } - ] - } - before do allow(azure_client2).to receive(:list_platform_image_versions). and_return(versions) @@ -244,8 +232,6 @@ end context "when the blob exists in the default storage account" do - let(:metadata) { { "foo" => "bar" } } - before do allow(blob_manager).to receive(:get_blob_metadata). with(MOCK_DEFAULT_STORAGE_ACCOUNT_NAME, stemcell_container, "#{stemcell_name}.vhd"). @@ -255,7 +241,7 @@ it "should return stemcell info" do stemcell_info = light_stemcell_manager.get_stemcell_info(stemcell_name) expect(stemcell_info.uri).to eq('') - expect(stemcell_info.metadata).to eq(metadata) + expect(stemcell_info.metadata).to eq(stemcell_properties) end end end