Perform renames, related to Bosh v1 -> v2 evolution

- Rename job_template(s) [Bosh v1] -> instance_job(s) [Bosh v2]
- Rename spec [ambiguous] -> spec_object [clarified: related to ERB-templates]
- Rename remove_unused_properties [double-negative] -> pick_job_properties [affirmative]
- Rename hash [imprecise] -> renderers_hash [clarified contents]
- Ensure that (job_name_from_manifest == instance_job.name) while we re-read
  the job manifest (though there is actually no reason to re-read it)
- Reword "Cannot unpack job template" [incorrect] -> "Cannot unpack job blob" [correct]
- Rename @name [imprecise] -> @job_name [precised]
- Remove duplicate @template_name [Bosh v1] identical to @job_name [Bosh v2]
- Rename template_name [Bosh v1] -> job_name [Bosh v2]
- Rename template_spec_properties [incorrect] -> instance_group_properties [correct]
- Rename instance_name [incorrect] -> instance_group_name [correct]
- Rename acc [accessory?] -> accu [accumulator!]
- Alias instance_plan.templates [Bosh v1] -> instance_jobs [Bosh v2]
- Don't rename fields in the 'template' model (don't touch models)
- Favor using spec['name'] over spec['job']['name'] (Bosh v1 naming) for
  obtaining the instance group name.

Author: bgandon

src/bosh-director-core/lib/bosh/director/core/templates/job_instance_renderer.rb

@@ -7,8 +7,8 @@ module Bosh::Director::Core::Templates
   # @param [Array<DeploymentPlan::Job>] instance_jobs
   # @param [JobTemplateLoader] job_template_loader
   class JobInstanceRenderer
-    def initialize(templates, job_template_loader)
-      @templates = templates
+    def initialize(instance_jobs, job_template_loader)
+      @instance_jobs = instance_jobs
       @job_template_loader = job_template_loader
     end
 
@@ -36,22 +36,22 @@ def initialize(templates, job_template_loader)
     #                           in the `spec` object exposed to ERB templates
     # @return [RenderedJobInstance] An object containing the rendering results
     #                               (when successful)
-    def render(spec)
+    def render(spec_object)
       errors = []
 
-      rendered_templates = @templates.map do |template|
-        job_template_renderer = job_template_renderers[template.name]
+      rendered_templates = @instance_jobs.map do |instance_job|
+        job_template_renderer = job_template_renderers[instance_job.name]
 
         begin
-          job_template_renderer.render(spec)
+          job_template_renderer.render(spec_object)
         rescue Exception => e
           errors.push e
         end
       end
 
       if errors.length > 0
         combined_errors = errors.map{|error| error.message.strip }.join("\n")
-        header = "- Unable to render jobs for instance group '#{spec['job']['name']}'. Errors are:"
+        header = "- Unable to render jobs for instance group '#{spec_object['name']}'. Errors are:"
         message = Bosh::Director::FormatterHelper.new.prepend_header_and_indent_body(header, combined_errors.strip, {:indent_by => 2})
         raise message
       end
@@ -62,9 +62,9 @@ def render(spec)
     private
 
     def job_template_renderers
-      @job_template_renderers ||= @templates.reduce({}) do |hash, template|
-        hash[template.name] = @job_template_loader.process(template)
-        hash
+      @job_template_renderers ||= @instance_jobs.reduce({}) do |renderers_hash, instance_job|
+        renderers_hash[instance_job.name] = @job_template_loader.process(instance_job)
+        renderers_hash
       end
     end
   end

src/bosh-director-core/lib/bosh/director/core/templates/job_template_loader.rb

@@ -33,24 +33,28 @@ def initialize(logger, template_blob_cache, link_provider_intents, dns_encoder)
       # @param [DeploymentPlan::Job] instance_job The job whose templates
       #                                           should be rendered
       # @return [JobTemplateRenderer] Object that can render the templates
-      def process(job_template)
-        template_dir = extract_template(job_template)
+      def process(instance_job)
+        template_dir = extract_template(instance_job)
         manifest = Psych.load_file(File.join(template_dir, 'job.MF'), aliases: true)
 
         monit_erb_file = File.read(File.join(template_dir, 'monit'))
-        monit_source_erb = SourceErb.new('monit', 'monit', monit_erb_file, job_template.name)
+        monit_source_erb = SourceErb.new('monit', 'monit', monit_erb_file, instance_job.name)
 
         source_erbs = []
 
-        template_name = manifest.fetch('name', {})
+        job_name_from_manifest = manifest.fetch('name', {})
+        if job_name_from_manifest != instance_job.name
+          raise Bosh::Director::JobTemplateUnpackFailed,
+            "Inconsistent name in extracted job.MF manifest " +
+              "(exptected: '#{instance_job.name}', got: '#{job_name_from_manifest}')"
+        end
 
         manifest.fetch('templates', {}).each_pair do |src_name, dest_name|
           erb_file = File.read(File.join(template_dir, 'templates', src_name))
-          source_erbs << SourceErb.new(src_name, dest_name, erb_file, job_template.name)
+          source_erbs << SourceErb.new(src_name, dest_name, erb_file, instance_job.name)
         end
 
-        JobTemplateRenderer.new(job_template: job_template,
-                                template_name: template_name,
+        JobTemplateRenderer.new(instance_job: instance_job,
                                 monit_erb: monit_source_erb,
                                 source_erbs: source_erbs,
                                 logger: @logger,
@@ -62,15 +66,15 @@ def process(job_template)
 
       private
 
-      def extract_template(job_template)
-        @logger.debug("Extracting job #{job_template.name}")
-        cached_blob_path = @template_blob_cache.download_blob(job_template)
+      def extract_template(instance_job)
+        @logger.debug("Extracting job #{instance_job.name}")
+        cached_blob_path = @template_blob_cache.download_blob(instance_job)
         template_dir = Dir.mktmpdir('template_dir')
 
         output = `tar -C #{template_dir} -xzf #{cached_blob_path} 2>&1`
         if $?.exitstatus != 0
           raise Bosh::Director::JobTemplateUnpackFailed,
-            "Cannot unpack '#{job_template.name}' job template, " +
+            "Cannot unpack '#{instance_job.name}' job blob, " +
               "tar returned #{$?.exitstatus}, " +
               "tar output: #{output}"
         end

src/bosh-director-core/lib/bosh/director/core/templates/job_template_renderer.rb

@@ -11,17 +11,15 @@ class JobTemplateRenderer
 
     attr_reader :monit_erb, :source_erbs
 
-    def initialize(job_template:,
-                   template_name:,
+    def initialize(instance_job:,
                    monit_erb:,
                    source_erbs:,
                    logger:,
                    link_provider_intents:,
                    dns_encoder: nil)
-      @links_provided = job_template.model.provides
-      @name = job_template.name
-      @release = job_template.release
-      @template_name = template_name
+      @links_provided = instance_job.model.provides
+      @job_name = instance_job.name
+      @release = instance_job.release
       @monit_erb = monit_erb
       @source_erbs = source_erbs
       @logger = logger
@@ -33,7 +31,7 @@ def render(spec)
       spec = Bosh::Common::DeepCopy.copy(spec)
 
       if spec['properties_need_filtering']
-        spec = remove_unused_properties(spec)
+        spec = pick_job_properties(spec)
       end
 
       spec = namespace_links_to_current_job(spec)
@@ -64,15 +62,15 @@ def render(spec)
 
       if errors.length > 0
         combined_errors = errors.map{|error| "- #{error.message.strip}"}.join("\n")
-        header = "- Unable to render templates for job '#{@name}'. Errors are:"
+        header = "- Unable to render templates for job '#{@job_name}'. Errors are:"
         message = Bosh::Director::FormatterHelper.new.prepend_header_and_indent_body(header, combined_errors.strip, {:indent_by => 2})
 
         raise message
       end
 
       rendered_files << RenderedFileTemplate.new('.bosh/links.json', '.bosh/links.json', links_data(spec))
 
-      RenderedJobTemplate.new(@name, monit, rendered_files)
+      RenderedJobTemplate.new(@job_name, monit, rendered_files)
     end
 
     private
@@ -83,8 +81,8 @@ def namespace_links_to_current_job(spec)
       modified_spec = spec
 
       if modified_spec.has_key?('links')
-        if modified_spec['links'][@template_name]
-          links_spec = modified_spec['links'][@template_name]
+        if modified_spec['links'][@job_name]
+          links_spec = modified_spec['links'][@job_name]
           modified_spec['links'] = links_spec
         else
           modified_spec['links'] = {}
@@ -93,15 +91,15 @@ def namespace_links_to_current_job(spec)
       modified_spec
     end
 
-    def remove_unused_properties(spec)
+    def pick_job_properties(spec)
       return nil if spec.nil?
 
       modified_spec = spec
 
       if modified_spec.has_key?('properties')
-        if modified_spec['properties'][@template_name]
-          properties_template = modified_spec['properties'][@template_name]
-          modified_spec['properties'] = properties_template
+        if modified_spec['properties'][@job_name]
+          job_properties = modified_spec['properties'][@job_name]
+          modified_spec['properties'] = job_properties
         end
       end
 
@@ -111,7 +109,7 @@ def remove_unused_properties(spec)
     def links_data(spec)
       provider_intents = @link_provider_intents.select do |provider_intent|
         provider_intent.link_provider.instance_group == spec['name'] &&
-          provider_intent.link_provider.name == @name
+          provider_intent.link_provider.name == @job_name
       end
 
       data = provider_intents.map do |provider_intent|

src/bosh-director-core/lib/bosh/director/core/templates/source_erb.rb

@@ -7,11 +7,11 @@ class SourceErb
 
     attr_reader :src_name, :dest_name, :erb
 
-    def initialize(src_name, dest_name, erb_contents, template_name)
+    def initialize(src_name, dest_name, erb_contents, job_name)
       @src_name = src_name
       @dest_name = dest_name
       erb = ERB.new(erb_contents, trim_mode: "-")
-      erb.filename = File.join(template_name, src_name)
+      erb.filename = File.join(job_name, src_name)
       @erb = erb
     end
 

src/bosh-director-core/spec/bosh/director/core/templates/job_instance_renderer_spec.rb

@@ -10,7 +10,8 @@ module Bosh::Director::Core::Templates
 
       let(:spec) do
         {
-          'job' => {
+          'name' => 'fake-instance-group-name',
+          'job' => { # <- here 'job' is the Bosh v1 term for 'instance group'
             'name' => 'fake-instance-group-name'
           }
         }

src/bosh-director-core/spec/bosh/director/core/templates/job_template_loader_spec.rb

@@ -79,7 +79,7 @@ module Bosh::Director::Core::Templates
 
       it 'returns the jobs template erb objects' do
         tarball_path = create_job_tarball(
-          'release-job-name',
+          'fake-job-name-1',
           'monit file erb contents',
           tmp_file,
           { 'test' => {
@@ -90,7 +90,7 @@ module Bosh::Director::Core::Templates
 
         job = double('Bosh::Director::DeploymentPlan::Job',
           download_blob: tarball_path,
-          name: 'plan-job-name',
+          name: 'fake-job-name-1',
           blobstore_id: 'blob-id',
           release: release
         )
@@ -103,19 +103,18 @@ module Bosh::Director::Core::Templates
           'monit',
           'monit',
           'monit file erb contents',
-          'plan-job-name',
+          'fake-job-name-1',
         ).and_return(monit_erb)
 
         expect(SourceErb).to receive(:new).with(
           'test',
           'test_dst',
           'test contents',
-          'plan-job-name'
+          'fake-job-name-1'
         ).and_return(job_template_erb)
 
         expect(JobTemplateRenderer).to receive(:new).with(
-          job_template: job,
-          template_name: 'release-job-name',
+          instance_job: job,
           monit_erb: monit_erb,
           source_erbs: [job_template_erb],
           logger: logger,
@@ -128,12 +127,12 @@ module Bosh::Director::Core::Templates
       end
 
       it 'includes only monit erb object when no other templates exist' do
-        tarball_path = create_job_tarball('release-job-no-templates', 'monit file erb contents', tmp_file, {})
+        tarball_path = create_job_tarball('fake-job-name-2', 'monit file erb contents', tmp_file, {})
 
         job = double(
           'Bosh::Director::DeploymentPlan::Job',
           download_blob: tarball_path,
-          name: 'plan-job-name',
+          name: 'fake-job-name-2',
           blobstore_id: 'blob-id',
           release: release,
         )
@@ -145,12 +144,11 @@ module Bosh::Director::Core::Templates
           'monit',
           'monit',
           'monit file erb contents',
-          'plan-job-name',
+          'fake-job-name-2',
         ).and_return(monit_erb)
 
         expect(JobTemplateRenderer).to receive(:new).with(
-          job_template: job,
-          template_name: 'release-job-no-templates',
+          instance_job: job,
           monit_erb: monit_erb,
           source_erbs: [],
           logger: logger,
@@ -167,7 +165,7 @@ module Bosh::Director::Core::Templates
           manifest = <<~EOF
             ---
             bogus_key: &empty_hash {}
-            name: test
+            name: test-job-name
             templates: *empty_hash
             packages: []
           EOF
@@ -178,7 +176,7 @@ module Bosh::Director::Core::Templates
 
           job = double('Bosh::Director::DeploymentPlan::Job',
             download_blob: tmp_file.path,
-            name: 'plan-job-name',
+            name: 'test-job-name',
             blobstore_id: 'blob-id',
             release: release,
             model: double('Bosh::Director::Models::Template', provides: [])