ERB template rendering code review: rename old bosh v1 names to v2

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

@@ -3,28 +3,55 @@
 require 'bosh/director/formatter_helper'
 
 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
 
-    def render(spec)
+    # Render all templates for a Bosh instance.
+    #
+    # From a list of instance jobs (typically comming from a single instance
+    # plan, so they cover all templates of some instance) this method is
+    # responsible for orchestrating several tasks.
+    #
+    # Lazily-delegated work to a 'JobTemplateLoader' object:
+    #   - Load all templates of the release job that the instance job is
+    #     referring to
+    #   - Convert each of these to a 'JobTemplateRenderer' object
+    #
+    # Work done here on top of this:
+    #   - Render each template with the necessary bindings (comming from
+    #     deployment manifest properties) for building the special 'spec'
+    #     object that the ERB rendring code can use.
+    #
+    # The actual rendering of each template is delegated to its related
+    # 'JobTemplateRenderer' object, as created in the first place by the
+    # 'JobTemplateLoader' object.
+    #
+    # @param [Hash] spec_object A hash of properties that will finally result
+    #                           in the `spec` object exposed to ERB templates
+    # @return [RenderedJobInstance] An object containing the rendering results
+    #                               (when successful)
+    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
@@ -35,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

@@ -15,24 +15,38 @@ def initialize(logger, template_blob_cache, link_provider_intents, dns_encoder)
         @dns_encoder = dns_encoder
       end
 
-      def process(job_template)
-        template_dir = extract_template(job_template)
-        manifest = Psych.load_file(File.join(template_dir, 'job.MF'), aliases: true)
+      # Perform these operations in order:
+      #
+      #  1. Download (or fetch from cache) a single job blob tarball.
+      #
+      #  2. Extract the job blob tarball in a temporary directory.
+      #
+      #  3. Access the extracted 'job.MF' manifest from the job blob.
+      #
+      #  4. Load all ERB templates (including the 'monit' file and all other
+      #     declared files within the 'templates' subdir) and create one
+      #     'SourceErb' object for each of these.
+      #
+      #  5. Create and return a 'JobTemplateRenderer' object, populated with
+      #     all created 'SourceErb' objects.
+      #
+      # @param [DeploymentPlan::Job] instance_job The job whose templates
+      #                                           should be rendered
+      # @return [JobTemplateRenderer] Object that can render the templates
+      def process(instance_job)
+        template_dir = extract_template(instance_job)
 
         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', {})
-
-        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)
+        instance_job.model.spec.fetch('templates', {}).each_pair do |src_filepath, dest_filepath|
+          erb_contents = File.read(File.join(template_dir, 'templates', src_filepath))
+          source_erbs << SourceErb.new(src_filepath, dest_filepath, erb_contents, 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,
@@ -44,15 +58,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)
@@ -59,20 +57,20 @@ def render(spec)
           errors.push e
         end
 
-        RenderedFileTemplate.new(source_erb.src_name, source_erb.dest_name, file_contents)
+        RenderedFileTemplate.new(source_erb.src_filepath, source_erb.dest_filepath, file_contents)
       end
 
       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/rendered_file_template.rb

@@ -1,5 +1,5 @@
 require 'bosh/director/core/templates'
 
 module Bosh::Director::Core::Templates
-  RenderedFileTemplate = Struct.new(:src_name, :dest_name, :contents)
+  RenderedFileTemplate = Struct.new(:src_filepath, :dest_filepath, :contents)
 end

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

@@ -20,9 +20,9 @@ def configuration_hash
         bound_templates << rendered_job_template.monit
         bound_templates << rendered_job_template.name
 
-        rendered_job_template.templates.sort { |x, y| x.src_name <=> y.src_name }.each do |template_file|
+        rendered_job_template.templates.sort { |x, y| x.src_filepath <=> y.src_filepath }.each do |template_file|
           bound_templates << template_file.contents
-          bound_templates << template_file.dest_name
+          bound_templates << template_file.dest_filepath
         end
 
         instance_digest << bound_templates

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

@@ -14,7 +14,7 @@ def initialize(name, monit, templates)
     def template_hash
       template_digest = Digest::SHA1.new
       template_digest << monit
-      templates.sort { |x, y| x.src_name <=> y.src_name }.each do |template_file|
+      templates.sort { |x, y| x.src_filepath <=> y.src_filepath }.each do |template_file|
         template_digest << template_file.contents
       end
 

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

@@ -22,7 +22,7 @@ def self.produce_gzipped_tarball(rendered_job_templates)
           end
 
           rendered_job_template.templates.each do |rendered_file_template|
-            template_path = File.join(job_name, rendered_file_template.dest_name)
+            template_path = File.join(job_name, rendered_file_template.dest_filepath)
 
             tar.add_file template_path, CREATED_FILES_PERMISSIONS do |tf|
               tf.write(rendered_file_template.contents)

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

@@ -13,7 +13,7 @@ def write(rendered_templates, output_dir)
         end
 
         job_template.templates.each do |file_template|
-          file_template_dest = File.join(job_template_dir, file_template.dest_name)
+          file_template_dest = File.join(job_template_dir, file_template.dest_filepath)
           FileUtils.mkdir_p(File.dirname(file_template_dest))
           File.open(file_template_dest, 'w') do |f|
             f.write(file_template.contents)

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

@@ -5,13 +5,13 @@ module Bosh::Director::Core::Templates
   class SourceErb
     @@mutex = Mutex.new
 
-    attr_reader :src_name, :dest_name, :erb
+    attr_reader :src_filepath, :dest_filepath, :erb
 
-    def initialize(src_name, dest_name, erb_contents, template_name)
-      @src_name = src_name
-      @dest_name = dest_name
+    def initialize(src_filepath, dest_filepath, erb_contents, job_name)
+      @src_filepath = src_filepath
+      @dest_filepath = dest_filepath
       erb = ERB.new(erb_contents, trim_mode: "-")
-      erb.filename = File.join(template_name, src_name)
+      erb.filename = File.join(job_name, src_filepath)
       @erb = erb
     end
 
@@ -28,7 +28,7 @@ def render(context, logger)
       line = line_index ? e.backtrace[line_index] : '(unknown):(unknown)'
       template_name, line = line.split(':')
 
-      message = "Error filling in template '#{File.basename(template_name)}' (line #{line}: #{e})"
+      message = "Error filling in template '#{@src_filepath}' (line #{line}: #{e})"
 
       logger.debug("#{message}\n#{e.backtrace.join("\n")}")
       raise message

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