diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/manifest_validator.rb b/src/bosh-director/lib/bosh/director/deployment_plan/manifest_validator.rb index 74c22892243..8e690fe9306 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/manifest_validator.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/manifest_validator.rb @@ -28,6 +28,14 @@ def validate(manifest) 'resource_pools is no longer supported. You must now define resources in a cloud-config' end + if manifest.key?('stemcells') + manifest['stemcells'].each do |stemcell| + if stemcell['name'] && stemcell['os'] + raise StemcellBothNameAndOS, "Properties 'os' and 'name' are both specified for stemcell, choose one. (#{stemcell})" + end + end + end + Config.event_log.warn_deprecated("Global 'properties' are deprecated. Please define 'properties' at the job level.") if manifest.key?('properties') end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/stemcell.rb b/src/bosh-director/lib/bosh/director/deployment_plan/stemcell.rb index 46496437cec..57dd4484d6a 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/stemcell.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/stemcell.rb @@ -20,10 +20,6 @@ def self.parse(spec) raise ValidationMissingField, "Required property 'os' or 'name' was not specified in object (#{spec})" end - if !name.nil? && !os.nil? - raise StemcellBothNameAndOS, "Properties 'os' and 'name' are both specified for stemcell, choose one. (#{spec})" - end - new(name_alias, name, os, version) end @@ -33,15 +29,9 @@ def initialize(name_alias, name, os, version) @os = os @version = version @manager = Api::StemcellManager.new - if @os.blank? - models = @manager.all_by_name_and_version(@name, @version) - unless models.empty? - @os = models.first.operating_system - end - end end - def is_using_os? + def is_only_using_os? !@os.nil? && @name.nil? end @@ -55,7 +45,7 @@ def bind_model(deployment_model) end def add_stemcell_models - if is_using_os? + if is_only_using_os? @models = @manager.all_by_os_and_version(@os, @version) raise StemcellNotFound, "Stemcell version '#{@version}' for OS '#{@os}' doesn't exist" if models.empty? else diff --git a/src/bosh-director/lib/bosh/director/manifest/manifest.rb b/src/bosh-director/lib/bosh/director/manifest/manifest.rb index ed60a05961a..45cbaddc702 100644 --- a/src/bosh-director/lib/bosh/director/manifest/manifest.rb +++ b/src/bosh-director/lib/bosh/director/manifest/manifest.rb @@ -124,6 +124,8 @@ def resolve_aliases_for_generic_hash(generic_hash) generic_hash['stemcells'].to_a.each do |stemcell| if stemcell.is_a?(Hash) stemcell['version'] = resolve_stemcell_version(stemcell) + resolved_os = resolve_stemcell_os(stemcell) + stemcell['os'] = resolved_os if resolved_os end end end @@ -168,10 +170,10 @@ def resolve_stemcell_version(stemcell) resolvable_version = match_resolvable_version(stemcell['version']) if resolvable_version - if stemcell['os'] - latest_stemcell = stemcell_manager.latest_by_os(stemcell['os'], resolvable_version[:prefix]) - elsif stemcell['name'] + if stemcell['name'] latest_stemcell = stemcell_manager.latest_by_name(stemcell['name'], resolvable_version[:prefix]) + elsif stemcell['os'] + latest_stemcell = stemcell_manager.latest_by_os(stemcell['os'], resolvable_version[:prefix]) else raise 'Stemcell definition must contain either name or os' end @@ -181,6 +183,17 @@ def resolve_stemcell_version(stemcell) stemcell['version'].to_s end + def resolve_stemcell_os(stemcell) + return stemcell['os'] if stemcell['os'] + + stemcell_manager = Api::StemcellManager.new + + models = stemcell_manager.all_by_name_and_version(stemcell['name'], stemcell['version']) + unless models.empty? + return models.first.operating_system + end + end + def resolve_release_version(release_def) release_manager = Api::ReleaseManager.new resolvable_version = match_resolvable_version(release_def['version']) diff --git a/src/bosh-director/spec/unit/deployment_plan/deployment_spec_parser_spec.rb b/src/bosh-director/spec/unit/deployment_plan/deployment_spec_parser_spec.rb index f60b428376a..240862cb0bb 100644 --- a/src/bosh-director/spec/unit/deployment_plan/deployment_spec_parser_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/deployment_spec_parser_spec.rb @@ -127,19 +127,6 @@ module Bosh::Director end end - context 'when there are stemcells with both name and OS' do - before do - stemcell_hash1 = { 'alias' => 'stemcell1', 'name' => 'bosh-aws-xen-hvm-ubuntu-trusty-go_agent', 'os' => 'ubuntu-trusty', 'version' => '1234' } - manifest_hash['stemcells'] = [stemcell_hash1] - end - - it 'errors out' do - expect do - parsed_deployment.stemcells - end.to raise_error Bosh::Director::StemcellBothNameAndOS - end - end - context 'when there are 2 stemcells' do before do stemcell_hash0 = { 'alias' => 'stemcell0', 'name' => 'bosh-aws-xen-hvm-ubuntu-trusty-go_agent', 'version' => '1234' } diff --git a/src/bosh-director/spec/unit/deployment_plan/manifest_validator_spec.rb b/src/bosh-director/spec/unit/deployment_plan/manifest_validator_spec.rb index 28dce3ca782..557f501019f 100644 --- a/src/bosh-director/spec/unit/deployment_plan/manifest_validator_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/manifest_validator_spec.rb @@ -86,6 +86,17 @@ module Director 'resource_pools is no longer supported. You must now define resources in a cloud-config', ) end + + it 'raises error when both os and name are specified for a stemcell' do + manifest_hash['stemcells'][0]['name'] = 'the-name' + + expect do + manifest_validator.validate(manifest_hash) + end.to raise_error( + Bosh::Director::StemcellBothNameAndOS, + %[Properties 'os' and 'name' are both specified for stemcell, choose one. ({"alias"=>"default", "os"=>"toronto-os", "version"=>"latest", "name"=>"the-name"})], + ) + end end end end diff --git a/src/bosh-director/spec/unit/deployment_plan/planner_factory_spec.rb b/src/bosh-director/spec/unit/deployment_plan/planner_factory_spec.rb index bc7b1e0c980..3b92e9b66ac 100644 --- a/src/bosh-director/spec/unit/deployment_plan/planner_factory_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/planner_factory_spec.rb @@ -85,7 +85,7 @@ module DeploymentPlan planner expected_deployment_manifest_log = <<~LOGMESSAGE Deployment manifest: - {"name"=>"simple", "releases"=>[{"name"=>"bosh-release", "version"=>"0.1-dev"}], "stemcells"=>[{"name"=>"ubuntu-stemcell", "version"=>"1", "alias"=>"default"}], "update"=>{"canaries"=>2, "canary_watch_time"=>4000, "max_in_flight"=>1, "update_watch_time"=>20}, "instance_groups"=>[{"name"=>"foobar", "stemcell"=>"default", "vm_type"=>"a", "instances"=>3, "networks"=>[{"name"=>"a"}], "jobs"=>[{"name"=>"foobar", "release"=>"bosh-release", "properties"=>{}}]}]} + {"name"=>"simple", "releases"=>[{"name"=>"bosh-release", "version"=>"0.1-dev"}], "stemcells"=>[{"name"=>"ubuntu-stemcell", "version"=>"1", "alias"=>"default", "os"=>"stemcell-os"}], "update"=>{"canaries"=>2, "canary_watch_time"=>4000, "max_in_flight"=>1, "update_watch_time"=>20}, "instance_groups"=>[{"name"=>"foobar", "stemcell"=>"default", "vm_type"=>"a", "instances"=>3, "networks"=>[{"name"=>"a"}], "jobs"=>[{"name"=>"foobar", "release"=>"bosh-release", "properties"=>{}}]}]} LOGMESSAGE expected_cloud_manifest_log = <<~LOGMESSAGE Cloud config manifest: @@ -459,7 +459,7 @@ def upload_releases def upload_stemcell stemcell_entry = manifest_hash['stemcells'].first - Models::Stemcell.make(name: stemcell_entry['name'], version: stemcell_entry['version']) + Models::Stemcell.make(name: stemcell_entry['name'], version: stemcell_entry['version'], operating_system: 'stemcell-os') end end end diff --git a/src/bosh-director/spec/unit/deployment_plan/stemcell_spec.rb b/src/bosh-director/spec/unit/deployment_plan/stemcell_spec.rb index ef534f00ba2..0e5bc07fc33 100644 --- a/src/bosh-director/spec/unit/deployment_plan/stemcell_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/stemcell_spec.rb @@ -48,13 +48,6 @@ def make_stemcell(name, version, os = 'os1', params = {}) valid_spec['name'] = 'stemcell-name' expect { make(valid_spec) }.to_not raise_error end - it 'populates os' do - make_stemcell('stemcell-name','0.5.2','plan-9') - valid_spec.delete('os') - valid_spec['name'] = 'stemcell-name' - stemcell = make(valid_spec) - expect(stemcell.os).to eq('plan-9') - end end context 'when neither os or name are specified' do @@ -67,17 +60,6 @@ def make_stemcell(name, version, os = 'os1', params = {}) ) end end - context 'when both os and name are specified' do - it 'raises' do - valid_spec['name'] = 'stemcell-name' - valid_spec['os'] = 'os1' - expect { make(valid_spec) }.to raise_error( - BD::StemcellBothNameAndOS, - "Properties 'os' and 'name' are both specified for stemcell, choose one. "\ - '({"name"=>"stemcell-name", "version"=>"0.5.2", "os"=>"os1"})', - ) - end - end end context 'stemcell with latest version' do diff --git a/src/bosh-director/spec/unit/manifest/manifest_spec.rb b/src/bosh-director/spec/unit/manifest/manifest_spec.rb index baff26137ee..4c682ff4248 100644 --- a/src/bosh-director/spec/unit/manifest/manifest_spec.rb +++ b/src/bosh-director/spec/unit/manifest/manifest_spec.rb @@ -31,11 +31,11 @@ module Bosh::Director Models::ReleaseVersion.make(version: '1+dev.5', release: release_1) Models::ReleaseVersion.make(version: '1+dev.7', release: release_1) - Models::Stemcell.make(name: 'simple', version: '3163') - Models::Stemcell.make(name: 'simple', version: '3169') + Models::Stemcell.make(name: 'simple', version: '3163', operating_system: 'simple-os') + Models::Stemcell.make(name: 'simple', version: '3169', operating_system: 'simple-os') - Models::Stemcell.make(name: 'hard', version: '3146') - Models::Stemcell.make(name: 'hard', version: '3146.1') + Models::Stemcell.make(name: 'hard', version: '3146', operating_system: 'hard-os') + Models::Stemcell.make(name: 'hard', version: '3146.1', operating_system: 'hard-os') allow(Bosh::Director::ConfigServer::VariablesInterpolator).to receive(:new).and_return(variables_interpolator) allow(variables_interpolator).to receive(:interpolate_cloud_manifest) { |cloud_manifest| Bosh::Common::DeepCopy.copy(cloud_manifest) } @@ -338,8 +338,8 @@ module Bosh::Director it 'replaces latest with the latest version number' do manifest_object.resolve_aliases expect(manifest_object.to_hash['stemcells']).to eq( - [{ 'name' => 'simple', 'version' => '3169' }, - { 'name' => 'hard', 'version' => '3146.1' }], + [{ 'name' => 'simple', 'version' => '3169', 'os'=>'simple-os' }, + { 'name' => 'hard', 'version' => '3146.1', 'os'=>'hard-os' }], ) end end @@ -357,8 +357,8 @@ module Bosh::Director it 'replaces prefixed-latest with the latest version number' do manifest_object.resolve_aliases expect(manifest_object.to_hash['stemcells']).to eq( - [{ 'name' => 'simple', 'version' => '3169' }, - { 'name' => 'hard', 'version' => '3146.1' }], + [{ 'name' => 'simple', 'version' => '3169', 'os'=>'simple-os' }, + { 'name' => 'hard', 'version' => '3146.1', 'os'=>'hard-os' }], ) end end @@ -377,7 +377,7 @@ module Bosh::Director manifest_object.resolve_aliases expect(manifest_object.to_hash['stemcells']).to eq( [{ 'name' => 'simple', 'version' => '42' }, - { 'name' => 'hard', 'version' => '3146.1' }], + { 'name' => 'hard', 'version' => '3146.1', 'os'=>'hard-os' }], ) end end @@ -407,6 +407,25 @@ module Bosh::Director expect(manifest_object.to_hash['stemcells']).to eq(['((foo))']) end end + + context 'when manifest has stemcells specified by name without os' do + let(:manifest_hash) do + { + 'stemcells' => [ + { 'name' => 'simple', 'version' => 'latest' }, + { 'name' => 'hard', 'version' => 'latest' }, + ], + } + end + + it 'adds the os' do + manifest_object.resolve_aliases + expect(manifest_object.to_hash['stemcells']).to eq( + [{ 'name' => 'simple', 'version' => '3169', 'os' => 'simple-os' }, + { 'name' => 'hard', 'version' => '3146.1', 'os' => 'hard-os' }], + ) + end + end end end