Populate stemcell os when manifest contains only the name

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
 

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

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'])

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' }

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

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

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

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