From 6aa55e9cf292821f31524a56c8012a767f82a2ec Mon Sep 17 00:00:00 2001 From: Benjamin Gandon Date: Mon, 27 Nov 2023 01:39:54 +0100 Subject: [PATCH 1/5] Document classes and methods, as part of the template rendering documentation review --- .../core/templates/job_instance_renderer.rb | 27 ++++++++++++++ .../core/templates/job_template_loader.rb | 18 ++++++++++ .../config_server/variables_interpolator.rb | 17 ++++++--- .../deployment_plan/instance_group.rb | 9 +++++ .../director/deployment_plan/instance_plan.rb | 5 +++ .../director/deployment_plan/instance_spec.rb | 2 +- .../lib/bosh/director/deployment_plan/job.rb | 5 ++- .../deployment_plan/release_version.rb | 36 +++++++++++++------ .../lib/bosh/director/job_renderer.rb | 23 ++++++++++++ .../lib/bosh/director/models/template.rb | 4 +++ 10 files changed, 130 insertions(+), 16 deletions(-) diff --git a/src/bosh-director-core/lib/bosh/director/core/templates/job_instance_renderer.rb b/src/bosh-director-core/lib/bosh/director/core/templates/job_instance_renderer.rb index 7ab32a77840..46d91b213ea 100644 --- a/src/bosh-director-core/lib/bosh/director/core/templates/job_instance_renderer.rb +++ b/src/bosh-director-core/lib/bosh/director/core/templates/job_instance_renderer.rb @@ -3,12 +3,39 @@ require 'bosh/director/formatter_helper' module Bosh::Director::Core::Templates + + # @param [Array] instance_jobs + # @param [JobTemplateLoader] job_template_loader class JobInstanceRenderer def initialize(templates, job_template_loader) @templates = templates @job_template_loader = job_template_loader end + # 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) errors = [] diff --git a/src/bosh-director-core/lib/bosh/director/core/templates/job_template_loader.rb b/src/bosh-director-core/lib/bosh/director/core/templates/job_template_loader.rb index f5097534c7a..60c7b61b7c3 100644 --- a/src/bosh-director-core/lib/bosh/director/core/templates/job_template_loader.rb +++ b/src/bosh-director-core/lib/bosh/director/core/templates/job_template_loader.rb @@ -15,6 +15,24 @@ def initialize(logger, template_blob_cache, link_provider_intents, dns_encoder) @dns_encoder = dns_encoder end + # 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(job_template) template_dir = extract_template(job_template) manifest = Psych.load_file(File.join(template_dir, 'job.MF'), aliases: true) diff --git a/src/bosh-director/lib/bosh/director/config_server/variables_interpolator.rb b/src/bosh-director/lib/bosh/director/config_server/variables_interpolator.rb index a8862db6a8e..ff2c28e9c3d 100644 --- a/src/bosh-director/lib/bosh/director/config_server/variables_interpolator.rb +++ b/src/bosh-director/lib/bosh/director/config_server/variables_interpolator.rb @@ -7,10 +7,19 @@ def initialize @cache_by_job_name = {} end - # @param [Hash] template_spec_properties Hash to be interpolated - # @param [Hash] deployment_name The deployment context in-which the interpolation will occur - # @param [VariableSet] variable_set The variable set which the interpolation will use. - # @return [Hash] A Deep copy of the interpolated template_spec_properties + # From a `job_name => job_properties` hash of instance group properties, + # resolve the `((var_name))`` placeholders, fetching their values from the + # config server. Most often, these placeholders refers to secrets stored + # in CredHub. + # + # @param [Hash] instance_group_properties a hash of per-job properties, to + # be interpolated + # @param [String] deployment_name The name of the deployment, as context + # in-which the interpolation will occur + # @param [String] instance_group_name The name of the instance group + # @param [VariableSet] variable_set The variable set which the + # interpolation will use. + # @return [Hash] A Deep copy of the interpolated instance_group_properties def interpolate_template_spec_properties(template_spec_properties, deployment_name, instance_name, variable_set) if template_spec_properties.nil? return template_spec_properties diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/instance_group.rb b/src/bosh-director/lib/bosh/director/deployment_plan/instance_group.rb index 9854d74b93d..cd1d525ce31 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/instance_group.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/instance_group.rb @@ -409,10 +409,19 @@ def create_desired_instances(count, deployment) private + # Returns the aggregated list of package names, considering all jobs + # desired on this instance group, with no duplicate. + # + # @return [Array] The list of package names def run_time_dependencies run_time_packages.map(&:name) end + # The aggregated list of models for all packages to install on instances + # of the group, considering all jobs desired on this instance group, + # with no duplicate. + # + # @return [Array] The list of packages def run_time_packages jobs.flat_map(&:package_models).uniq end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/instance_plan.rb b/src/bosh-director/lib/bosh/director/deployment_plan/instance_plan.rb index 396ac29c461..6346577b1f8 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/instance_plan.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/instance_plan.rb @@ -379,6 +379,11 @@ def spec InstanceSpec.create_from_instance_plan(self) end + # Returns the desired jobs for the instance + # + # Here “template” is the old Bosh v1 name for “job”. + # + # @return [Array] List of desired jobs def templates @desired_instance.instance_group.jobs end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/instance_spec.rb b/src/bosh-director/lib/bosh/director/deployment_plan/instance_spec.rb index ed790fb14b8..5bb2b8febc4 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/instance_spec.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/instance_spec.rb @@ -17,7 +17,7 @@ def self.create_from_instance_plan(instance_plan) spec = { 'deployment' => deployment_name, - 'job' => instance_group.spec, + 'job' => instance_group.spec, # <- here 'job' is old Bosh v1 naming, meaning 'instance group' 'index' => instance.index, 'bootstrap' => instance.bootstrap?, 'lifecycle' => instance_group.lifecycle, diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/job.rb b/src/bosh-director/lib/bosh/director/deployment_plan/job.rb index 31e4ff83d74..7be1144ae4c 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/job.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/job.rb @@ -34,7 +34,10 @@ def initialize(release, name) @properties = {} end - # Looks up template model and its package models in DB + # Looks up job model and its package models in DB. + # + # Here “template” is the old Bosh v1 name for “job”. + # # @return [void] def bind_models @model = @release.get_template_model_by_name(@name) diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/release_version.rb b/src/bosh-director/lib/bosh/director/deployment_plan/release_version.rb index 26bd367e4e3..449f8a81da7 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/release_version.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/release_version.rb @@ -91,8 +91,11 @@ def spec } end - # Looks up up template model by template name - # @param [String] name Template name + # Looks up up job model by job name. + # + # Here “template” is the old Bosh v1 name for “job”. + # + # @param [String] name Job name # @return [Models::Template] def get_template_model_by_name(name) @all_jobs ||= @model.templates.each_with_object({}) do |job, all_jobs| @@ -109,25 +112,38 @@ def get_package_model_by_name(name) @model.package_by_name(name) end - # Adds template to a list of templates used by this release for the - # current deployment + # Adds a job to a list of jobs used by this release for the current + # deployment. + # + # Here “template” is the old Bosh v1 name for “job”. + # # @param [String] options Template name def get_or_create_template(name) @jobs[name] ||= Job.new(self, name) end + # Return a given job, identified by name. + # + # Here “template” is the old Bosh v1 name for “job”. + # # @param [String] name Job name # @return [DeploymentPlan::Job] Job with given name used by this - # release (if any) + # release (if any) def template(name) @jobs[name] end - # Returns a list of job templates that need to be included into this - # release. Note that this is not just a list of all templates existing - # in the release but rather a list of templates for jobs that are included - # into current deployment plan. - # @return [Array] List of job templates + # Returns a list of jobs from the release that are used by the + # deployment. + # + # Note that this is not the full list of all jobs existing in the + # release, but a subset list of the jobs defined in that release that + # are used by the current deployment, and thus included in its plan. + # + # Here “template” is the old Bosh v1 name for “job”. + # + # @return [Array] List of the release jobs used by + # the deployment def templates @jobs.values end diff --git a/src/bosh-director/lib/bosh/director/job_renderer.rb b/src/bosh-director/lib/bosh/director/job_renderer.rb index f0880db5258..a9c467f1701 100644 --- a/src/bosh-director/lib/bosh/director/job_renderer.rb +++ b/src/bosh-director/lib/bosh/director/job_renderer.rb @@ -4,6 +4,20 @@ module Bosh::Director class JobRenderer + + # Render the related job templates for each instance plan passed as + # argument in the 'instance_plans' array. + # + # @param [Logging::Logger] logger A logger where to log activity + # @param [Array] instance_plans A list of instance plans + # @param [TemplateBlobCache] cache A cache through which job blobs are to + # be fetched + # @param [DnsEncoder] dns_encoder A DNS encoder for generating Bosh DNS + # queries out of context and criteria + # @param [Array] link_provider_intents Relevant + # context-dependant + # link provider + # intents def self.render_job_instances_with_cache(logger, instance_plans, cache, dns_encoder, link_provider_intents) job_template_loader = Core::Templates::JobTemplateLoader.new( logger, @@ -17,6 +31,15 @@ def self.render_job_instances_with_cache(logger, instance_plans, cache, dns_enco end end + # For one instance plan, create a 'JobInstanceRenderer' object that will + # lazily load the ERB templates for all desired jobs on the instance, then + # render these templates with the bindings populated by the + # 'spec' properties of the instance plan. + # + # @param [DeploymentPlan::InstancePlan] instance_plan An instance plan + # @param [JobTemplateLoader] loader The object that will load the ERB + # templates + # @param [Logging::Logger] logger A logger where to log activity def self.render_job_instance(instance_plan, loader, logger) instance = instance_plan.instance diff --git a/src/bosh-director/lib/bosh/director/models/template.rb b/src/bosh-director/lib/bosh/director/models/template.rb index c8492c9f7c7..8838dc265c7 100644 --- a/src/bosh-director/lib/bosh/director/models/template.rb +++ b/src/bosh-director/lib/bosh/director/models/template.rb @@ -1,4 +1,8 @@ module Bosh::Director::Models + + # This class models a job, as defined within a Bosh Release. + # + # Here “template” is the old Bosh v1 name for “job”. class Template < Sequel::Model(Bosh::Director::Config.db) many_to_one :release many_to_many :release_versions From 977d6b7c95258feffb05a5a442c9dda2143b8afe Mon Sep 17 00:00:00 2001 From: Benjamin Gandon Date: Mon, 27 Nov 2023 02:53:38 +0100 Subject: [PATCH 2/5] 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. --- .../core/templates/job_instance_renderer.rb | 20 ++--- .../core/templates/job_template_loader.rb | 26 ++++--- .../core/templates/job_template_renderer.rb | 30 ++++---- .../director/core/templates/source_erb.rb | 4 +- .../templates/job_instance_renderer_spec.rb | 3 +- .../templates/job_template_loader_spec.rb | 24 +++--- .../templates/job_template_renderer_spec.rb | 76 ++++++++++--------- .../config_server/variables_interpolator.rb | 10 +-- .../deployment_plan/instance_group.rb | 8 +- .../lib/bosh/director/job_renderer.rb | 7 +- .../spec/unit/job_renderer_spec.rb | 2 +- .../lib/bosh/template/evaluation_context.rb | 6 +- .../bosh/template/evaluation_context_spec.rb | 1 + 13 files changed, 109 insertions(+), 108 deletions(-) diff --git a/src/bosh-director-core/lib/bosh/director/core/templates/job_instance_renderer.rb b/src/bosh-director-core/lib/bosh/director/core/templates/job_instance_renderer.rb index 46d91b213ea..641f194f4fe 100644 --- a/src/bosh-director-core/lib/bosh/director/core/templates/job_instance_renderer.rb +++ b/src/bosh-director-core/lib/bosh/director/core/templates/job_instance_renderer.rb @@ -7,8 +7,8 @@ module Bosh::Director::Core::Templates # @param [Array] 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,14 +36,14 @@ 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 @@ -51,7 +51,7 @@ def render(spec) 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 diff --git a/src/bosh-director-core/lib/bosh/director/core/templates/job_template_loader.rb b/src/bosh-director-core/lib/bosh/director/core/templates/job_template_loader.rb index 60c7b61b7c3..86b29e7f84c 100644 --- a/src/bosh-director-core/lib/bosh/director/core/templates/job_template_loader.rb +++ b/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 diff --git a/src/bosh-director-core/lib/bosh/director/core/templates/job_template_renderer.rb b/src/bosh-director-core/lib/bosh/director/core/templates/job_template_renderer.rb index 403e9519ed1..cbc8a96019c 100644 --- a/src/bosh-director-core/lib/bosh/director/core/templates/job_template_renderer.rb +++ b/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,7 +62,7 @@ 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 @@ -72,7 +70,7 @@ def render(spec) 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| diff --git a/src/bosh-director-core/lib/bosh/director/core/templates/source_erb.rb b/src/bosh-director-core/lib/bosh/director/core/templates/source_erb.rb index 0f4fdcd8f09..f638fc17fe6 100644 --- a/src/bosh-director-core/lib/bosh/director/core/templates/source_erb.rb +++ b/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 diff --git a/src/bosh-director-core/spec/bosh/director/core/templates/job_instance_renderer_spec.rb b/src/bosh-director-core/spec/bosh/director/core/templates/job_instance_renderer_spec.rb index 8e07021009e..bf955086994 100644 --- a/src/bosh-director-core/spec/bosh/director/core/templates/job_instance_renderer_spec.rb +++ b/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' } } diff --git a/src/bosh-director-core/spec/bosh/director/core/templates/job_template_loader_spec.rb b/src/bosh-director-core/spec/bosh/director/core/templates/job_template_loader_spec.rb index b1bbefdb7a8..44b529f2aa4 100644 --- a/src/bosh-director-core/spec/bosh/director/core/templates/job_template_loader_spec.rb +++ b/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: []) diff --git a/src/bosh-director-core/spec/bosh/director/core/templates/job_template_renderer_spec.rb b/src/bosh-director-core/spec/bosh/director/core/templates/job_template_renderer_spec.rb index 93c3387f10e..b0000cb29f2 100644 --- a/src/bosh-director-core/spec/bosh/director/core/templates/job_template_renderer_spec.rb +++ b/src/bosh-director-core/spec/bosh/director/core/templates/job_template_renderer_spec.rb @@ -26,7 +26,7 @@ module Bosh::Director::Core::Templates { 'index' => 1, 'job' => { - 'name' => 'fake-job-name', + 'name' => 'fake-instance-group-name', }, } end @@ -34,7 +34,7 @@ module Bosh::Director::Core::Templates let(:links_provided) { [] } let(:release) { double('Bosh::Director::DeploymentPlan::ReleaseVersion', name: 'fake-release-name', version: '0.1') } let(:job_template_model) { double('Bosh::Director::Models::Template', provides: links_provided) } - let(:job_template) do + let(:instance_job) do double('Bosh::Director::DeploymentPlan::Job', name: 'fake-job-name', release: release, model: job_template_model) end let(:logger) { instance_double('Logger', debug: nil) } @@ -43,8 +43,7 @@ module Bosh::Director::Core::Templates subject(:job_template_renderer) do JobTemplateRenderer.new( - job_template: job_template, - template_name: 'template-name', + instance_job: instance_job, monit_erb: monit_erb, source_erbs: [source_erb], logger: logger, @@ -81,16 +80,17 @@ module Bosh::Director::Core::Templates let(:spec) do { 'index' => 1, - 'job' => { - 'name' => 'reg-job-name', - 'templates' => - [{ 'name' => 'template-name', + 'name' => 'instance-group-name', + 'job' => { # <- here 'job' is the Bosh v1 term for 'instance group' + 'name' => 'reg-instance-group-name', + 'templates' => # <- here 'template' is the Bosh v1 term for 'job' + [{ 'name' => 'fake-job-name', 'version' => '1bbe5ab00082797999e2101d730de64aeb601b6a', 'sha1' => '728399f9ef342532c6224bce4eb5331b5c38d595', 'blobstore_id' => '6c1eec85-3c08-4464-8b11-dc43acaa79f9' }], }, 'properties' => { - 'template-name' => { + 'fake-job-name' => { 'inside' => 'insideValue', 'smurfs' => { 'name' => 'snoopy' }, }, @@ -109,15 +109,16 @@ module Bosh::Director::Core::Templates expect(Bosh::Template::EvaluationContext).to have_received(:new).with( { 'index' => 1, - 'job' => { - 'name' => 'reg-job-name', - 'templates' => - [{ 'name' => 'template-name', + 'name' => 'instance-group-name', + 'job' => { # <- here 'job' is the Bosh v1 term for 'instance group' + 'name' => 'reg-instance-group-name', + 'templates' => # <- here 'template' is the Bosh v1 term for 'job' + [{ 'name' => 'fake-job-name', 'version' => '1bbe5ab00082797999e2101d730de64aeb601b6a', 'sha1' => '728399f9ef342532c6224bce4eb5331b5c38d595', 'blobstore_id' => '6c1eec85-3c08-4464-8b11-dc43acaa79f9' }], }, - 'properties' => { # note: loses 'template-name' from :spec + 'properties' => { # note: loses 'fake-job-name' from :spec 'inside' => 'insideValue', 'smurfs' => { 'name' => 'snoopy' }, }, @@ -130,8 +131,7 @@ module Bosh::Director::Core::Templates context 'rendering templates returns errors' do let(:job_template_renderer) do JobTemplateRenderer.new( - job_template: job_template, - template_name: 'template-name', + instance_job: instance_job, monit_erb: monit_erb, source_erbs: [source_erb, source_erb], logger: logger, @@ -161,18 +161,22 @@ module Bosh::Director::Core::Templates context 'when spec has links' do let(:raw_spec) do { - 'name' => 'joao-da-silva', + 'name' => 'fake-instance-group-name', 'index' => 1, - 'job' => { - 'name' => 'template-name', + 'job' => { # <- here 'job' is the Bosh v1 term for 'instance group' + 'name' => 'fake-instance-group-name', }, 'properties_need_filtering' => true, 'links' => { - 'template-name' => { - 'db_link' => - { 'properties' => { 'foo' => 'bar' }, 'instances' => [{ 'name' => 'mysql1' }, { 'name' => 'mysql' }] }, - 'backup_db' => - { 'properties' => { 'moop' => 'yar' }, 'instances' => [{ 'name' => 'postgres1' }, { 'name' => 'postgres' }] }, + 'fake-job-name' => { + 'db_link' => { + 'properties' => { 'foo' => 'bar' }, + 'instances' => [{ 'name' => 'mysql1' }, { 'name' => 'mysql' }] + }, + 'backup_db' => { + 'properties' => { 'moop' => 'yar' }, + 'instances' => [{ 'name' => 'postgres1' }, { 'name' => 'postgres' }] + }, }, }, 'release' => { 'name' => 'fake-release-name', 'version' => '0.1' }, @@ -181,38 +185,36 @@ module Bosh::Director::Core::Templates let(:modified_spec) do { - 'name' => 'joao-da-silva', + 'name' => 'fake-instance-group-name', 'index' => 1, - 'job' => { - 'name' => 'template-name', + 'job' => { # <- here 'job' is the Bosh v1 term for 'instance group' + 'name' => 'fake-instance-group-name', }, 'properties_need_filtering' => true, 'links' => { - 'db_link' => - { + 'db_link' => { 'properties' => { 'foo' => 'bar' }, 'instances' => [{ 'name' => 'mysql1' }, { 'name' => 'mysql' }], }, - 'backup_db' => - { - 'properties' => { 'moop' => 'yar' }, - 'instances' => [{ 'name' => 'postgres1' }, { 'name' => 'postgres' }], - }, + 'backup_db' => { + 'properties' => { 'moop' => 'yar' }, + 'instances' => [{ 'name' => 'postgres1' }, { 'name' => 'postgres' }], + }, }, 'release' => { 'name' => 'fake-release-name', 'version' => '0.1' }, } end let(:provider1) do - double('provider1', instance_group: 'joao-da-silva', name: 'fake-job-name') + double('provider1', instance_group: 'fake-instance-group-name', name: 'fake-job-name') end let(:provider2) do - double('provider2', instance_group: 'bob-de-smith') + double('provider2', instance_group: 'another-instance-group-name') end let(:provider3) do - double('provider3', instance_group: 'joao-da-silva', name: 'another-job-name') + double('provider3', instance_group: 'fake-instance-group-name', name: 'another-job-name') end let(:provider_intent) do diff --git a/src/bosh-director/lib/bosh/director/config_server/variables_interpolator.rb b/src/bosh-director/lib/bosh/director/config_server/variables_interpolator.rb index ff2c28e9c3d..88bf593f9fd 100644 --- a/src/bosh-director/lib/bosh/director/config_server/variables_interpolator.rb +++ b/src/bosh-director/lib/bosh/director/config_server/variables_interpolator.rb @@ -20,9 +20,9 @@ def initialize # @param [VariableSet] variable_set The variable set which the # interpolation will use. # @return [Hash] A Deep copy of the interpolated instance_group_properties - def interpolate_template_spec_properties(template_spec_properties, deployment_name, instance_name, variable_set) - if template_spec_properties.nil? - return template_spec_properties + def interpolate_template_spec_properties(instance_group_properties, deployment_name, instance_group_name, variable_set) + if instance_group_properties.nil? + return instance_group_properties end if deployment_name.nil? @@ -32,9 +32,9 @@ def interpolate_template_spec_properties(template_spec_properties, deployment_na result = {} errors = [] - template_spec_properties.each do |job_name, job_properties| + instance_group_properties.each do |job_name, job_properties| begin - key = job_name + "-" + instance_name + key = job_name + "-" + instance_group_name if @cache_by_job_name.has_key?(key) interpolated_hash = @cache_by_job_name[key] else diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/instance_group.rb b/src/bosh-director/lib/bosh/director/deployment_plan/instance_group.rb index cd1d525ce31..8ba6b0bb855 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/instance_group.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/instance_group.rb @@ -239,8 +239,8 @@ def update_spec # name. To be used by all instances to populate agent state. # @return [Hash] All package specs indexed by package name def package_spec - @packages.each_with_object({}) do |(name, package), acc| - acc[name] = package.spec + @packages.each_with_object({}) do |(name, package), accu| + accu[name] = package.spec end.select { |name, _| run_time_dependencies.include? name } end @@ -272,9 +272,9 @@ def use_compiled_package(compiled_package_model) # before 'bind_properties' is being called (as we persist instance group template # property definitions in DB). def bind_properties - @properties = @jobs.each_with_object({}) do |job, acc| + @properties = @jobs.each_with_object({}) do |job, accu| job.bind_properties(@name) - acc[job.name] = job.properties[@name] + accu[job.name] = job.properties[@name] end end diff --git a/src/bosh-director/lib/bosh/director/job_renderer.rb b/src/bosh-director/lib/bosh/director/job_renderer.rb index a9c467f1701..5d7e4edb89f 100644 --- a/src/bosh-director/lib/bosh/director/job_renderer.rb +++ b/src/bosh-director/lib/bosh/director/job_renderer.rb @@ -43,14 +43,15 @@ def self.render_job_instances_with_cache(logger, instance_plans, cache, dns_enco def self.render_job_instance(instance_plan, loader, logger) instance = instance_plan.instance - if instance_plan.templates.empty? - logger.debug("Skipping rendering templates for '#{instance}', no templates") + instance_jobs = instance_plan.templates + if instance_jobs.empty? + logger.debug("Skipping rendering templates for '#{instance}': no job") return end logger.debug("Rendering templates for instance #{instance}") - instance_renderer = Core::Templates::JobInstanceRenderer.new(instance_plan.templates, loader) + instance_renderer = Core::Templates::JobInstanceRenderer.new(instance_jobs, loader) rendered_job_instance = instance_renderer.render(get_templates_spec(instance_plan)) instance_plan.rendered_templates = rendered_job_instance diff --git a/src/bosh-director/spec/unit/job_renderer_spec.rb b/src/bosh-director/spec/unit/job_renderer_spec.rb index 83db0bc0a10..5b90bcabc61 100644 --- a/src/bosh-director/spec/unit/job_renderer_spec.rb +++ b/src/bosh-director/spec/unit/job_renderer_spec.rb @@ -59,7 +59,7 @@ def perform end it 'does not render' do - expect(logger).to receive(:debug).with("Skipping rendering templates for 'test-instance-group/5', no templates") + expect(logger).to receive(:debug).with("Skipping rendering templates for 'test-instance-group/5': no job") expect { perform }.not_to change { instance_plan.rendered_templates } end end diff --git a/src/bosh-template/lib/bosh/template/evaluation_context.rb b/src/bosh-template/lib/bosh/template/evaluation_context.rb index b7ef17d303f..dc2927ce360 100644 --- a/src/bosh-template/lib/bosh/template/evaluation_context.rb +++ b/src/bosh-template/lib/bosh/template/evaluation_context.rb @@ -40,11 +40,7 @@ def initialize(spec, dns_encoder) "Hash expected, #{spec.class} given" end - if spec['job'].is_a?(Hash) - @name = spec['job']['name'] - else - @name = nil - end + @name = spec['name'] @index = spec['index'] @spec = openstruct(spec, BackCompatOpenStruct) diff --git a/src/bosh-template/spec/unit/bosh/template/evaluation_context_spec.rb b/src/bosh-template/spec/unit/bosh/template/evaluation_context_spec.rb index 5065bc7cf48..d94269fec6b 100644 --- a/src/bosh-template/spec/unit/bosh/template/evaluation_context_spec.rb +++ b/src/bosh-template/spec/unit/bosh/template/evaluation_context_spec.rb @@ -24,6 +24,7 @@ def eval_template(erb, context) let(:spec) do { + 'name' => 'foobar', 'job' => { 'name' => 'foobar', }, From d9a7db1eb1a8bd29e9c85e2d9548b731094084db Mon Sep 17 00:00:00 2001 From: Benjamin Gandon Date: Mon, 27 Nov 2023 19:47:47 +0100 Subject: [PATCH 3/5] Stop unnecessarily scan the 'job.MF' file from job blob when rendering ERBs Indeed we already know the job name and template list, as these data are persisted when a release is uploaded to the director. This makes unnecessary to verify that the job name is consistent at ERB rendering time. And this consistency check is more relevant when done at the time a release job is uploaded to the director. --- .../core/templates/job_template_loader.rb | 10 +- .../templates/job_template_loader_spec.rb | 12 ++ src/bosh-director/lib/bosh/director/errors.rb | 1 + .../bosh/director/jobs/release/release_job.rb | 8 ++ .../spec/unit/job_renderer_spec.rb | 16 ++- .../unit/jobs/release/release_job_spec.rb | 136 ++++++++++++------ 6 files changed, 130 insertions(+), 53 deletions(-) diff --git a/src/bosh-director-core/lib/bosh/director/core/templates/job_template_loader.rb b/src/bosh-director-core/lib/bosh/director/core/templates/job_template_loader.rb index 86b29e7f84c..9a50bbe21c4 100644 --- a/src/bosh-director-core/lib/bosh/director/core/templates/job_template_loader.rb +++ b/src/bosh-director-core/lib/bosh/director/core/templates/job_template_loader.rb @@ -35,21 +35,13 @@ def initialize(logger, template_blob_cache, link_provider_intents, dns_encoder) # @return [JobTemplateRenderer] Object that can render the templates 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, instance_job.name) source_erbs = [] - 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| + instance_job.model.spec.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, instance_job.name) end diff --git a/src/bosh-director-core/spec/bosh/director/core/templates/job_template_loader_spec.rb b/src/bosh-director-core/spec/bosh/director/core/templates/job_template_loader_spec.rb index 44b529f2aa4..2aca1e885a2 100644 --- a/src/bosh-director-core/spec/bosh/director/core/templates/job_template_loader_spec.rb +++ b/src/bosh-director-core/spec/bosh/director/core/templates/job_template_loader_spec.rb @@ -95,6 +95,10 @@ module Bosh::Director::Core::Templates release: release ) + job_model = double('Bosh::Director::Models::Template') + expect(job).to receive(:model).and_return(job_model) + expect(job_model).to receive(:spec).and_return({ "templates" => { "test" => "test_dst" } }) + monit_erb = instance_double(SourceErb) job_template_erb = instance_double(SourceErb) fake_renderer = instance_double(JobTemplateRenderer) @@ -137,6 +141,10 @@ module Bosh::Director::Core::Templates release: release, ) + job_model = double('Bosh::Director::Models::Template') + expect(job).to receive(:model).and_return(job_model) + expect(job_model).to receive(:spec).and_return({ "templates" => {} }) + monit_erb = instance_double(SourceErb) fake_renderer = instance_double(JobTemplateRenderer) @@ -182,6 +190,10 @@ module Bosh::Director::Core::Templates model: double('Bosh::Director::Models::Template', provides: []) ) + job_model = double('Bosh::Director::Models::Template') + expect(job).to receive(:model).and_return(job_model) + expect(job_model).to receive(:spec).and_return({ "templates" => {} }) + job_template_renderer = job_template_loader.process(job) expect(job_template_renderer.source_erbs).to eq([]) end diff --git a/src/bosh-director/lib/bosh/director/errors.rb b/src/bosh-director/lib/bosh/director/errors.rb index 62f32cc0fd1..bdbe0b9c2e2 100644 --- a/src/bosh-director/lib/bosh/director/errors.rb +++ b/src/bosh-director/lib/bosh/director/errors.rb @@ -134,6 +134,7 @@ def self.err(error_code, response_code = BAD_REQUEST) JobInvalidLinkSpec = err(80013) JobDuplicateLinkName = err(80014) JobWithExportedFromMismatch = err(80015) + JobInvalidName = err(80016) ResourceError = err(100001) ResourceNotFound = err(100002, NOT_FOUND) diff --git a/src/bosh-director/lib/bosh/director/jobs/release/release_job.rb b/src/bosh-director/lib/bosh/director/jobs/release/release_job.rb index a63cd10dd0f..4b64a3d800a 100644 --- a/src/bosh-director/lib/bosh/director/jobs/release/release_job.rb +++ b/src/bosh-director/lib/bosh/director/jobs/release/release_job.rb @@ -15,6 +15,7 @@ def update job_manifest = load_manifest + validate_name(job_manifest) validate_templates(job_manifest) validate_monit validate_logs(job_manifest) @@ -75,6 +76,13 @@ def load_manifest YAML.load_file(manifest_file, aliases: true) end + def validate_name(job_manifest) + unless name == job_manifest['name'] + raise JobInvalidName, "Inconsistent name for job '#{name}'" + + "(exptected: '#{name}', got: '#{job_manifest['name']}')" + end + end + def validate_templates(job_manifest) if job_manifest['templates'] job_manifest['templates'].each_key do |relative_path| diff --git a/src/bosh-director/spec/unit/job_renderer_spec.rb b/src/bosh-director/spec/unit/job_renderer_spec.rb index 5b90bcabc61..c191a834820 100644 --- a/src/bosh-director/spec/unit/job_renderer_spec.rb +++ b/src/bosh-director/spec/unit/job_renderer_spec.rb @@ -41,13 +41,21 @@ def perform before do release_version = DeploymentPlan::ReleaseVersion.parse(deployment_model, 'name' => 'fake-release', 'version' => '123') + job1 = DeploymentPlan::Job.new(release_version, 'dummy') - job1.bind_existing_model( - Models::Template.make(blobstore_id: 'my-blobstore-id', sha1: '16baf0c24e2dac2a21ccdcd4655be403a602f573'), - ) + job1.bind_existing_model(Models::Template.make( + name: 'dummy', + spec_json: '{ "templates": { "ctl.erb": "bin/dummy_ctl" } }', + blobstore_id: 'my-blobstore-id', + sha1: '16baf0c24e2dac2a21ccdcd4655be403a602f573' + )) job2 = DeploymentPlan::Job.new(release_version, 'dummy') - job2.bind_existing_model(Models::Template.make(blobstore_id: 'my-blobstore-id')) + job2.bind_existing_model(Models::Template.make( + name: 'dummy', + spec_json: '{ "templates": { "ctl.erb": "bin/dummy_ctl" } }', + blobstore_id: 'my-blobstore-id') + ) allow(instance_plan).to receive_message_chain(:spec, :as_template_spec).and_return('template' => 'spec') allow(instance_plan).to receive(:templates).and_return([job1, job2]) diff --git a/src/bosh-director/spec/unit/jobs/release/release_job_spec.rb b/src/bosh-director/spec/unit/jobs/release/release_job_spec.rb index e3ffc221f6d..eb0343a80e3 100644 --- a/src/bosh-director/spec/unit/jobs/release/release_job_spec.rb +++ b/src/bosh-director/spec/unit/jobs/release/release_job_spec.rb @@ -9,14 +9,14 @@ module Bosh::Director after { FileUtils.rm_rf(release_dir) } let(:release_model) { Models::Release.make } let(:job_meta) do - { 'name' => 'foo', 'version' => '1', 'sha1' => 'deadbeef', 'fingerprint' => 'bar' } + { 'name' => 'foo-job', 'version' => '1', 'sha1' => 'deadbeef', 'fingerprint' => 'bar' } end before { allow(App).to receive_message_chain(:instance, :blobstores, :blobstore).and_return(blobstore) } let(:blobstore) { instance_double('Bosh::Blobstore::BaseClient') } - let(:job_tarball_path) { File.join(release_dir, 'jobs', 'foo.tgz') } + let(:job_tarball_path) { File.join(release_dir, 'jobs', 'foo-job.tgz') } - let(:job_bits) { create_job('foo', 'monit', {'foo' => {'destination' => 'foo', 'contents' => 'bar'}}) } + let(:job_bits) { create_job('foo-job', 'monit', {'foo-erb' => {'destination' => 'foo-rendered', 'contents' => 'bar'}}) } before { FileUtils.mkdir_p(File.dirname(job_tarball_path)) } before { allow(blobstore).to receive(:create).and_return('fake-blobstore-id') } @@ -25,7 +25,7 @@ module Bosh::Director before do Models::Template.make( blobstore_id: 'original-blobstore-id', - name: 'foo', + name: 'foo-job', version: '1', sha1: 'deadbeef', fingerprint: 'bar', @@ -39,13 +39,13 @@ module Bosh::Director expect(blobstore).to receive(:delete).with('original-blobstore-id') expect(blobstore).to receive(:create).and_return('fake-blobstore-id') - saved_template = release_job.update + saved_job = release_job.update - expect(saved_template.name).to eq('foo') - expect(saved_template.version).to eq('1') - expect(saved_template.release).to eq(release_model) - expect(saved_template.sha1).to eq('deadbeef') - expect(saved_template.blobstore_id).to eq('fake-blobstore-id') + expect(saved_job.name).to eq('foo-job') + expect(saved_job.version).to eq('1') + expect(saved_job.release).to eq(release_model) + expect(saved_job.sha1).to eq('deadbeef') + expect(saved_job.blobstore_id).to eq('fake-blobstore-id') end it 'does not bail if blobstore deletion fails' do @@ -54,9 +54,9 @@ module Bosh::Director expect(blobstore).to receive(:delete).and_raise Bosh::Blobstore::BlobstoreError expect(blobstore).to receive(:create) - saved_template = release_job.update + saved_job = release_job.update - expect(saved_template.blobstore_id).to eq('fake-blobstore-id') + expect(saved_job.blobstore_id).to eq('fake-blobstore-id') end end @@ -67,9 +67,9 @@ module Bosh::Director expect(blobstore).to_not receive(:delete) expect(blobstore).to receive(:create) - saved_template = release_job.update + saved_job = release_job.update - expect(saved_template.blobstore_id).to eq('fake-blobstore-id') + expect(saved_job.blobstore_id).to eq('fake-blobstore-id') end end @@ -86,11 +86,11 @@ module Bosh::Director expect(Models::Template.count).to eq(0) release_job.update - template = Models::Template.first - expect(template.name).to eq('foo') - expect(template.version).to eq('1') - expect(template.release).to eq(release_model) - expect(template.sha1).to eq('deadbeef') + job_model = Models::Template.first + expect(job_model.name).to eq('foo-job') + expect(job_model.version).to eq('1') + expect(job_model.release).to eq(release_model) + expect(job_model.sha1).to eq('deadbeef') end it 'should fail when it cannot extract job archive' do @@ -102,16 +102,25 @@ module Bosh::Director it 'whines on missing manifest' do job_without_manifest = - create_job('foo', 'monit', {'foo' => {'destination' => 'foo', 'contents' => 'bar'}}, skip_manifest: true) + create_job('foo-job', 'monit', {'foo-erb' => {'destination' => 'foo-rendered', 'contents' => 'bar'}}, skip_manifest: true) File.open(job_tarball_path, 'w') { |f| f.write(job_without_manifest) } expect { release_job.update }.to raise_error(JobMissingManifest) end + it 'whines on inconsistent job name' do + job_without_manifest = + create_job('different-job-name', 'monit', {'foo-erb' => {'destination' => 'foo-rendered', 'contents' => 'bar'}}) + + File.open(job_tarball_path, 'w') { |f| f.write(job_without_manifest) } + + expect { release_job.update }.to raise_error(JobInvalidName) + end + it 'whines on missing monit file' do job_without_monit = - create_job('foo', 'monit', {'foo' => {'destination' => 'foo', 'contents' => 'bar'}}, skip_monit: true) + create_job('foo-job', 'monit', {'foo-erb' => {'destination' => 'foo-rendered', 'contents' => 'bar'}}, skip_monit: true) File.open(job_tarball_path, 'w') { |f| f.write(job_without_monit) } expect { release_job.update }.to raise_error(JobMissingMonit) @@ -119,7 +128,7 @@ module Bosh::Director it 'does not whine when it has a foo.monit file' do job_without_monit = - create_job('foo', 'monit', {'foo' => {'destination' => 'foo', 'contents' => 'bar'}}, monit_file: 'foo.monit') + create_job('foo-job', 'monit', {'foo-erb' => {'destination' => 'foo-rendered', 'contents' => 'bar'}}, monit_file: 'foo.monit') File.open(job_tarball_path, 'w') { |f| f.write(job_without_monit) } @@ -128,7 +137,7 @@ module Bosh::Director it 'saves the templates hash in the template spec' do job_with_interesting_templates = - create_job('foo', 'monit', { + create_job('foo-job', 'monit', { 'template source path' => {'destination' => 'rendered template path', 'contents' => 'whatever'} }, monit_file: 'foo.monit') @@ -141,7 +150,7 @@ module Bosh::Director it 'whines on missing template' do job_without_template = - create_job('foo', 'monit', {'foo' => {'destination' => 'foo', 'contents' => 'bar'}}, skip_templates: ['foo']) + create_job('foo-job', 'monit', {'foo-erb' => {'destination' => 'foo-rendered', 'contents' => 'bar'}}, skip_templates: ['foo-erb']) File.open(job_tarball_path, 'w') { |f| f.write(job_without_template) } @@ -150,8 +159,11 @@ module Bosh::Director it 'does not whine when no packages are specified' do job_without_packages = - create_job('foo', 'monit', {'foo' => {'destination' => 'foo', 'contents' => 'bar'}}, - manifest: { 'name' => 'foo', 'templates' => {} }) + create_job('foo-job', 'monit', {'foo-erb' => {'destination' => 'foo-renderd', 'contents' => 'bar'}}, + manifest: { + 'name' => 'foo-job', + 'templates' => {} + }) File.open(job_tarball_path, 'w') { |f| f.write(job_without_packages) } job = nil @@ -161,8 +173,12 @@ module Bosh::Director it 'whines when packages is not an array' do job_with_invalid_packages = - create_job('foo', 'monit', {'foo' => {'destination' => 'foo', 'contents' => 'bar'}}, - manifest: { 'name' => 'foo', 'templates' => {}, 'packages' => 'my-awesome-package' }) + create_job('foo-job', 'monit', {'foo-erb' => {'destination' => 'foo-rendered', 'contents' => 'bar'}}, + manifest: { + 'name' => 'foo-job', + 'templates' => {}, + 'packages' => 'my-awesome-package' + }) File.open(job_tarball_path, 'w') { |f| f.write(job_with_invalid_packages) } expect { release_job.update }.to raise_error(JobInvalidPackageSpec) @@ -170,38 +186,58 @@ module Bosh::Director context 'when job spec file includes provides' do it 'verifies it is an array' do - job_with_invalid_spec = create_job('foo', 'monit', {}, manifest: {'provides' => 'Invalid'}) + job_with_invalid_spec = create_job('foo-job', 'monit', {}, + manifest: { + 'name' => 'foo-job', + 'provides' => 'Invalid' + }) File.open(job_tarball_path, 'w') { |f| f.write(job_with_invalid_spec) } expect { release_job.update }.to raise_error(JobInvalidLinkSpec) end it 'verifies that it is an array of hashes' do - job_with_invalid_spec = create_job('foo', 'monit', {}, manifest: {'provides' => ['Invalid', 1]}) + job_with_invalid_spec = create_job('foo-job', 'monit', {}, + manifest: { + 'name' => 'foo-job', + 'provides' => ['Invalid', 1] + }) File.open(job_tarball_path, 'w') { |f| f.write(job_with_invalid_spec) } expect { release_job.update }.to raise_error(JobInvalidLinkSpec) end it 'verifies hash contains name and type' do - job_with_invalid_spec = create_job('foo', 'monit', {}, manifest: {'provides' => [{'name' => 'db'}]}) + job_with_invalid_spec = create_job('foo-job', 'monit', {}, + manifest: { + 'name' => 'foo-job', + 'provides' => [{'name' => 'db'}] + }) File.open(job_tarball_path, 'w') { |f| f.write(job_with_invalid_spec) } expect { release_job.update }.to raise_error(JobInvalidLinkSpec) end it 'verifies names are unique' do - job_with_invalid_spec = create_job('foo', 'monit', {}, manifest: {'provides' => [{'name' => 'db', 'type' => 'first'}, {'name' => 'db', 'type' => 'second'}]}) + job_with_invalid_spec = create_job('foo-job', 'monit', {}, + manifest: { + 'name' => 'foo-job', + 'provides' => [{'name' => 'db', 'type' => 'first'}, {'name' => 'db', 'type' => 'second'}] + }) File.open(job_tarball_path, 'w') { |f| f.write(job_with_invalid_spec) } expect { release_job.update }.to raise_error( JobDuplicateLinkName, - "Job 'foo' specifies duplicate provides link with name 'db'" + "Job 'foo-job' specifies duplicate provides link with name 'db'" ) end it 'saves them on template' do - job_with_invalid_spec = create_job('foo', 'monit', {}, manifest: {'provides' => [{'name' => 'db1', 'type' =>'db'}, {'name' => 'db2', 'type' =>'db'}]}) + job_with_invalid_spec = create_job('foo-job', 'monit', {}, + manifest: { + 'name' => 'foo-job', + 'provides' => [{'name' => 'db1', 'type' =>'db'}, {'name' => 'db2', 'type' =>'db'}] + }) File.open(job_tarball_path, 'w') { |f| f.write(job_with_invalid_spec) } expect(Models::Template.count).to eq(0) @@ -216,38 +252,58 @@ module Bosh::Director it 'verifies it is an array' do allow(blobstore).to receive(:create).and_return('fake-blobstore-id') - job_with_invalid_spec = create_job('foo', 'monit', {}, manifest: {'consumes' => 'Invalid'}) + job_with_invalid_spec = create_job('foo-job', 'monit', {}, + manifest: { + 'name' => 'foo-job', + 'consumes' => 'Invalid' + }) File.open(job_tarball_path, 'w') { |f| f.write(job_with_invalid_spec) } expect { release_job.update }.to raise_error(JobInvalidLinkSpec) end it 'verifies that it is an array of string' do - job_with_invalid_spec = create_job('foo', 'monit', {}, manifest: {'consumes' => ['Invalid', 1]}) + job_with_invalid_spec = create_job('foo-job', 'monit', {}, + manifest: { + 'name' => 'foo-job', + 'consumes' => ['Invalid', 1] + }) File.open(job_tarball_path, 'w') { |f| f.write(job_with_invalid_spec) } expect { release_job.update }.to raise_error(JobInvalidLinkSpec) end it 'verifies hash contains name and type' do - job_with_invalid_spec = create_job('foo', 'monit', {}, manifest: {'consumes' => [{'name' => 'db'}]}) + job_with_invalid_spec = create_job('foo-job', 'monit', {}, + manifest: { + 'name' => 'foo-job', + 'consumes' => [{'name' => 'db'}] + }) File.open(job_tarball_path, 'w') { |f| f.write(job_with_invalid_spec) } expect { release_job.update }.to raise_error(JobInvalidLinkSpec) end it 'verifies names are unique' do - job_with_invalid_spec = create_job('foo', 'monit', {}, manifest: {'consumes' => [{'name' => 'db', 'type' => 'one'}, {'name' => 'db', 'type' => 'two'}]}) + job_with_invalid_spec = create_job('foo-job', 'monit', {}, + manifest: { + 'name' => 'foo-job', + 'consumes' => [{'name' => 'db', 'type' => 'one'}, {'name' => 'db', 'type' => 'two'}] + }) File.open(job_tarball_path, 'w') { |f| f.write(job_with_invalid_spec) } expect { release_job.update }.to raise_error( JobDuplicateLinkName, - "Job 'foo' specifies duplicate consumes link with name 'db'" + "Job 'foo-job' specifies duplicate consumes link with name 'db'" ) end it 'saves them on template' do - job_with_invalid_spec = create_job('foo', 'monit', {}, manifest: {'consumes' => [{'name' => 'db1', 'type' =>'db'}, {'name' => 'db2', 'type' =>'db'}]}) + job_with_invalid_spec = create_job('foo-job', 'monit', {}, + manifest: { + 'name' => 'foo-job', + 'consumes' => [{'name' => 'db1', 'type' =>'db'}, {'name' => 'db2', 'type' =>'db'}] + }) File.open(job_tarball_path, 'w') { |f| f.write(job_with_invalid_spec) } expect(Models::Template.count).to eq(0) From f43d4da92aaf58e8f3da8b28fde148964127bdc7 Mon Sep 17 00:00:00 2001 From: Benjamin Gandon Date: Tue, 28 Nov 2023 01:02:24 +0100 Subject: [PATCH 4/5] More Bosh v1->v2 renames and cleanup --- .../lib/bosh/director/deployment_plan/job.rb | 5 +- .../bosh/director/jobs/release/release_job.rb | 16 ++--- .../lib/bosh/director/jobs/update_release.rb | 64 ++++++++++--------- .../spec/unit/instance_deleter_spec.rb | 3 - .../lib/bosh/template/property_helper.rb | 2 +- 5 files changed, 46 insertions(+), 44 deletions(-) diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/job.rb b/src/bosh-director/lib/bosh/director/deployment_plan/job.rb index 7be1144ae4c..5a56330c693 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/job.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/job.rb @@ -55,11 +55,12 @@ def bind_existing_model(model) @model = model end - # Downloads template blob to a given path + # Downloads job blob to a given path + # # @return [String] Path to downloaded blob def download_blob uuid = SecureRandom.uuid - path = File.join(Dir.tmpdir, "template-#{uuid}") + path = File.join(Dir.tmpdir, "job-#{uuid}") @logger.debug("Downloading job '#{@name}' (#{blobstore_id})...") t1 = Time.now diff --git a/src/bosh-director/lib/bosh/director/jobs/release/release_job.rb b/src/bosh-director/lib/bosh/director/jobs/release/release_job.rb index 4b64a3d800a..e61204f14b2 100644 --- a/src/bosh-director/lib/bosh/director/jobs/release/release_job.rb +++ b/src/bosh-director/lib/bosh/director/jobs/release/release_job.rb @@ -22,25 +22,25 @@ def update validate_properties(job_manifest) validate_links(job_manifest) - template = Models::Template.find_or_init_from_release_meta( + job_model = Models::Template.find_or_init_from_release_meta( release: @release_model, job_meta: @job_meta, job_manifest: job_manifest, ) - if template.blobstore_id + if job_model.blobstore_id begin - @logger.info("Deleting blob for template '#{name}/#{@version}' with blobstore_id '#{template.blobstore_id}'") - BlobUtil.delete_blob(template.blobstore_id) - template.blobstore_id = nil + @logger.info("Deleting blob for job '#{name}/#{@version}' with blobstore_id '#{job_model.blobstore_id}'") + BlobUtil.delete_blob(job_model.blobstore_id) + job_model.blobstore_id = nil rescue Bosh::Blobstore::BlobstoreError => e - @logger.info("Error deleting blob for template '#{name}/#{@version}' with blobstore_id '#{template.blobstore_id}': #{e.inspect}") + @logger.info("Error deleting blob for job '#{name}/#{@version}' with blobstore_id '#{job_model.blobstore_id}': #{e.inspect}") end end - template.blobstore_id = BlobUtil.create_blob(job_tgz) + job_model.blobstore_id = BlobUtil.create_blob(job_tgz) - template.save + job_model.save end private diff --git a/src/bosh-director/lib/bosh/director/jobs/update_release.rb b/src/bosh-director/lib/bosh/director/jobs/update_release.rb index d856f60966d..1113278fac6 100644 --- a/src/bosh-director/lib/bosh/director/jobs/update_release.rb +++ b/src/bosh-director/lib/bosh/director/jobs/update_release.rb @@ -230,10 +230,11 @@ def process_packages(release_dir) ) end - # Finds job template definitions in release manifest and sorts them into - # two buckets: new and existing job templates, then creates new job - # template records in the database and points release version to existing ones. - # @param [String] release_dir local path to the unpacked release + # Finds job definitions in release manifest and sorts them into two + # buckets: new and existing jobs, then creates new job template records + # in the database and points release version to existing ones. + # + # @param [String] release_dir Local path to the unpacked release # @return [void] def process_jobs(release_dir) logger.info('Checking for new jobs in release') @@ -242,29 +243,29 @@ def process_jobs(release_dir) existing_jobs = [] manifest_jobs = @manifest['jobs'] || [] - manifest_jobs.each do |job_meta| + manifest_jobs.each do |manifest_job| # Checking whether we might have the same bits somewhere - @release_version_model.templates.select { |t| t.name == job_meta['name'] }.each do |tmpl| - next unless tmpl.fingerprint != job_meta['fingerprint'] + @release_version_model.templates.select { |t| t.name == manifest_job['name'] }.each do |release_job| + next unless release_job.fingerprint != manifest_job['fingerprint'] raise( ReleaseExistingJobFingerprintMismatch, - "job '#{job_meta['name']}' had different fingerprint in previously uploaded release '#{@name}/#{@version}'", + "job '#{manifest_job['name']}' had different fingerprint in previously uploaded release '#{@name}/#{@version}'", ) end - jobs = Models::Template.where(fingerprint: job_meta['fingerprint']).all + job_models = Models::Template.where(fingerprint: manifest_job['fingerprint']).all - template = jobs.find do |job| + job_model = job_models.find do |job| job.release_id == @release_model.id && - job.name == job_meta['name'] && - job.version == job_meta['version'] + job.name == manifest_job['name'] && + job.version == manifest_job['version'] end - if template.nil? - new_jobs << job_meta + if job_model.nil? + new_jobs << manifest_job else - existing_jobs << [template, job_meta] + existing_jobs << [job_model, manifest_job] end end @@ -279,21 +280,21 @@ def create_jobs(jobs, release_dir) return false if jobs.empty? event_log_stage = Config.event_log.begin_stage('Creating new jobs', jobs.size) - jobs.each do |job_meta| - job_desc = "#{job_meta['name']}/#{job_meta['version']}" + jobs.each do |manifest_job| + job_desc = "#{manifest_job['name']}/#{manifest_job['version']}" event_log_stage.advance_and_track(job_desc) do - logger.info("Creating new template '#{job_desc}'") - template = create_job(job_meta, release_dir) - register_template(template) + logger.info("Creating new job '#{job_desc}'") + job = create_job(manifest_job, release_dir) + register_template(job) end end true end - def create_job(job_meta, release_dir) - release_job = ReleaseJob.new(job_meta, @release_model, release_dir, logger) - logger.info("Creating job template '#{job_meta['name']}/#{job_meta['version']}' " \ + def create_job(manifest_job, release_dir) + release_job = ReleaseJob.new(manifest_job, @release_model, release_dir, logger) + logger.info("Creating job '#{manifest_job['name']}/#{manifest_job['version']}' " \ 'from provided bits') release_job.update end @@ -304,18 +305,18 @@ def use_existing_jobs(jobs, release_dir) return false if jobs.empty? single_step_stage("Processing #{jobs.size} existing job#{'s' if jobs.size > 1}") do - jobs.each do |template, job_meta| - job_desc = "#{template.name}/#{template.version}" + jobs.each do |job_model, manifest_job| + job_desc = "#{job_model.name}/#{job_model.version}" if @fix logger.info("Fixing existing job '#{job_desc}'") - release_job = ReleaseJob.new(job_meta, @release_model, release_dir, logger) + release_job = ReleaseJob.new(manifest_job, @release_model, release_dir, logger) release_job.update else logger.info("Using existing job '#{job_desc}'") end - register_template(template) unless template.release_versions.include? @release_version_model + register_template(job_model) unless job_model.release_versions.include? @release_version_model end end @@ -325,10 +326,13 @@ def use_existing_jobs(jobs, release_dir) private # Marks job template model as being used by release version - # @param [Models::Template] template Job template model + # + # Here “template” is the old Bosh v1 name for “job”. + # + # @param [Models::Template] job The job model # @return [void] - def register_template(template) - @release_version_model.add_template(template) + def register_template(job) + @release_version_model.add_template(job) end def manifest_packages diff --git a/src/bosh-director/spec/unit/instance_deleter_spec.rb b/src/bosh-director/spec/unit/instance_deleter_spec.rb index 050c208b259..2fe826971c1 100644 --- a/src/bosh-director/spec/unit/instance_deleter_spec.rb +++ b/src/bosh-director/spec/unit/instance_deleter_spec.rb @@ -68,9 +68,6 @@ module Bosh::Director describe 'deleting instances' do let(:deployment_model) { Models::Deployment.make(name: 'deployment-name') } - let(:deployment_plan) { instance_double(DeploymentPlan::Planner, model: deployment_model) } - let(:job) { DeploymentPlan::InstanceGroup.new(logger) } - let(:network) { instance_double(DeploymentPlan::ManualNetwork, name: 'manual-network') } let(:vm_deleter) { VmDeleter.new(logger, false, false) } let(:job_templates_cleaner) do diff --git a/src/bosh-template/lib/bosh/template/property_helper.rb b/src/bosh-template/lib/bosh/template/property_helper.rb index 007f4f30fb9..9274b9b7b1e 100644 --- a/src/bosh-template/lib/bosh/template/property_helper.rb +++ b/src/bosh-template/lib/bosh/template/property_helper.rb @@ -71,7 +71,7 @@ def set_property(dst, name, value) dst_ref[keys[-1]] = value end - + def validate_properties_format(properties, name) keys = name.split('.') properties_ref = properties From ce48af2f71c1c88d2a26aa61147789ed9141220a Mon Sep 17 00:00:00 2001 From: Benjamin Gandon Date: Tue, 28 Nov 2023 13:08:51 +0100 Subject: [PATCH 5/5] Rename '(src|dst)_name' -> '(src|dst)_filepath' --- .../core/templates/job_template_loader.rb | 6 ++-- .../core/templates/job_template_renderer.rb | 2 +- .../core/templates/rendered_file_template.rb | 2 +- .../core/templates/rendered_job_instance.rb | 4 +-- .../core/templates/rendered_job_template.rb | 2 +- ...endered_templates_in_memory_tar_gzipper.rb | 2 +- .../templates/rendered_templates_writer.rb | 2 +- .../director/core/templates/source_erb.rb | 12 +++---- .../templates/job_template_renderer_spec.rb | 12 +++---- .../templates/rendered_job_instance_spec.rb | 32 +++++++++---------- .../templates/rendered_job_template_spec.rb | 4 +-- .../rendered_templates_writer_spec.rb | 4 +-- 12 files changed, 42 insertions(+), 42 deletions(-) diff --git a/src/bosh-director-core/lib/bosh/director/core/templates/job_template_loader.rb b/src/bosh-director-core/lib/bosh/director/core/templates/job_template_loader.rb index 9a50bbe21c4..41259be4f5b 100644 --- a/src/bosh-director-core/lib/bosh/director/core/templates/job_template_loader.rb +++ b/src/bosh-director-core/lib/bosh/director/core/templates/job_template_loader.rb @@ -41,9 +41,9 @@ def process(instance_job) source_erbs = [] - instance_job.model.spec.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, instance_job.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(instance_job: instance_job, diff --git a/src/bosh-director-core/lib/bosh/director/core/templates/job_template_renderer.rb b/src/bosh-director-core/lib/bosh/director/core/templates/job_template_renderer.rb index cbc8a96019c..47536b55f4f 100644 --- a/src/bosh-director-core/lib/bosh/director/core/templates/job_template_renderer.rb +++ b/src/bosh-director-core/lib/bosh/director/core/templates/job_template_renderer.rb @@ -57,7 +57,7 @@ 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 diff --git a/src/bosh-director-core/lib/bosh/director/core/templates/rendered_file_template.rb b/src/bosh-director-core/lib/bosh/director/core/templates/rendered_file_template.rb index 2444fca9ee4..7d770f54f00 100644 --- a/src/bosh-director-core/lib/bosh/director/core/templates/rendered_file_template.rb +++ b/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 diff --git a/src/bosh-director-core/lib/bosh/director/core/templates/rendered_job_instance.rb b/src/bosh-director-core/lib/bosh/director/core/templates/rendered_job_instance.rb index 3fff536427f..7bac1107748 100644 --- a/src/bosh-director-core/lib/bosh/director/core/templates/rendered_job_instance.rb +++ b/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 diff --git a/src/bosh-director-core/lib/bosh/director/core/templates/rendered_job_template.rb b/src/bosh-director-core/lib/bosh/director/core/templates/rendered_job_template.rb index 1a04312d9ad..d0b2c351c86 100644 --- a/src/bosh-director-core/lib/bosh/director/core/templates/rendered_job_template.rb +++ b/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 diff --git a/src/bosh-director-core/lib/bosh/director/core/templates/rendered_templates_in_memory_tar_gzipper.rb b/src/bosh-director-core/lib/bosh/director/core/templates/rendered_templates_in_memory_tar_gzipper.rb index cb0ccf9d419..38eb673488e 100644 --- a/src/bosh-director-core/lib/bosh/director/core/templates/rendered_templates_in_memory_tar_gzipper.rb +++ b/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) diff --git a/src/bosh-director-core/lib/bosh/director/core/templates/rendered_templates_writer.rb b/src/bosh-director-core/lib/bosh/director/core/templates/rendered_templates_writer.rb index e05d4be600b..83554eaaa92 100644 --- a/src/bosh-director-core/lib/bosh/director/core/templates/rendered_templates_writer.rb +++ b/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) diff --git a/src/bosh-director-core/lib/bosh/director/core/templates/source_erb.rb b/src/bosh-director-core/lib/bosh/director/core/templates/source_erb.rb index f638fc17fe6..7575f2c3178 100644 --- a/src/bosh-director-core/lib/bosh/director/core/templates/source_erb.rb +++ b/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, job_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(job_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 diff --git a/src/bosh-director-core/spec/bosh/director/core/templates/job_template_renderer_spec.rb b/src/bosh-director-core/spec/bosh/director/core/templates/job_template_renderer_spec.rb index b0000cb29f2..7c5238a1dee 100644 --- a/src/bosh-director-core/spec/bosh/director/core/templates/job_template_renderer_spec.rb +++ b/src/bosh-director-core/spec/bosh/director/core/templates/job_template_renderer_spec.rb @@ -16,8 +16,8 @@ module Bosh::Director::Core::Templates let(:source_erb) do instance_double( 'Bosh::Director::Core::Templates::SourceErb', - src_name: 'fake-template-src-name', - dest_name: 'fake-template-dest-name', + src_filepath: 'fake-template-src-name', + dest_filepath: 'fake-template-dest-name', render: 'test template', ) end @@ -67,8 +67,8 @@ module Bosh::Director::Core::Templates expect(rendered_templates.monit).to eq('monit file') rendered_file_template = rendered_templates.templates.first - expect(rendered_file_template.src_name).to eq('fake-template-src-name') - expect(rendered_file_template.dest_name).to eq('fake-template-dest-name') + expect(rendered_file_template.src_filepath).to eq('fake-template-src-name') + expect(rendered_file_template.dest_filepath).to eq('fake-template-dest-name') expect(rendered_file_template.contents).to eq('test template') expect(monit_erb).to have_received(:render).with(context_copy, logger) @@ -268,8 +268,8 @@ module Bosh::Director::Core::Templates expect(dns_encoder).to have_received(:id_for_group_tuple).once rendered_links_file = rendered_files.pop - expect(rendered_links_file.src_name).to(eq('.bosh/links.json')) - expect(rendered_links_file.dest_name).to(eq('.bosh/links.json')) + expect(rendered_links_file.src_filepath).to(eq('.bosh/links.json')) + expect(rendered_links_file.dest_filepath).to(eq('.bosh/links.json')) expect(JSON.parse(rendered_links_file.contents)).to eq( [ diff --git a/src/bosh-director-core/spec/bosh/director/core/templates/rendered_job_instance_spec.rb b/src/bosh-director-core/spec/bosh/director/core/templates/rendered_job_instance_spec.rb index 17f9b44d5d9..bb8330c3e59 100644 --- a/src/bosh-director-core/spec/bosh/director/core/templates/rendered_job_instance_spec.rb +++ b/src/bosh-director-core/spec/bosh/director/core/templates/rendered_job_instance_spec.rb @@ -18,8 +18,8 @@ module Bosh::Director::Core::Templates [ instance_double( 'Bosh::Director::Core::Templates::RenderedFileTemplate', - src_name: 'template-file1', - dest_name: 'template-file1', + src_filepath: 'template-file1', + dest_filepath: 'template-file1', contents: 'template file contents 1', ), ], @@ -30,14 +30,14 @@ module Bosh::Director::Core::Templates [ instance_double( 'Bosh::Director::Core::Templates::RenderedFileTemplate', - src_name: 'template-file3', - dest_name: 'template-file3', + src_filepath: 'template-file3', + dest_filepath: 'template-file3', contents: 'template file contents 3', ), instance_double( 'Bosh::Director::Core::Templates::RenderedFileTemplate', - src_name: 'template-file2', - dest_name: 'template-file2', + src_filepath: 'template-file2', + dest_filepath: 'template-file2', contents: 'template file contents 2', ), ], @@ -57,8 +57,8 @@ module Bosh::Director::Core::Templates 'monit file contents 1', [ instance_double('Bosh::Director::Core::Templates::RenderedFileTemplate', - src_name: 'template-file1', - dest_name: 'template-file1', + src_filepath: 'template-file1', + dest_filepath: 'template-file1', contents: 'template file contents 1'), ], ), @@ -72,8 +72,8 @@ module Bosh::Director::Core::Templates 'monit file contents 1', [ instance_double('Bosh::Director::Core::Templates::RenderedFileTemplate', - src_name: 'template-file1', - dest_name: 'template-file1', + src_filepath: 'template-file1', + dest_filepath: 'template-file1', contents: 'template file contents 1'), ], ), @@ -82,8 +82,8 @@ module Bosh::Director::Core::Templates '', [ instance_double('Bosh::Director::Core::Templates::RenderedFileTemplate', - src_name: 'template-file1', - dest_name: 'template-file1', + src_filepath: 'template-file1', + dest_filepath: 'template-file1', contents: ''), ], ), @@ -103,8 +103,8 @@ module Bosh::Director::Core::Templates 'monit file contents 1', [ instance_double('Bosh::Director::Core::Templates::RenderedFileTemplate', - src_name: 'template-file1', - dest_name: 'template-file1', + src_filepath: 'template-file1', + dest_filepath: 'template-file1', contents: 'template file contents 1'), ], ), @@ -118,8 +118,8 @@ module Bosh::Director::Core::Templates 'monit file contents 1', [ instance_double('Bosh::Director::Core::Templates::RenderedFileTemplate', - src_name: 'template-file1', - dest_name: 'template-file1', + src_filepath: 'template-file1', + dest_filepath: 'template-file1', contents: 'template file contents 1'), ], ), diff --git a/src/bosh-director-core/spec/bosh/director/core/templates/rendered_job_template_spec.rb b/src/bosh-director-core/spec/bosh/director/core/templates/rendered_job_template_spec.rb index b7597279adf..2c92d87ce15 100644 --- a/src/bosh-director-core/spec/bosh/director/core/templates/rendered_job_template_spec.rb +++ b/src/bosh-director-core/spec/bosh/director/core/templates/rendered_job_template_spec.rb @@ -9,12 +9,12 @@ module Bosh::Director::Core::Templates [ instance_double( 'Bosh::Director::Core::Templates::RenderedFileTemplate', - src_name: 'foo.erb', + src_filepath: 'foo.erb', contents: 'rendered foo erb', ), instance_double( 'Bosh::Director::Core::Templates::RenderedFileTemplate', - src_name: 'bar.erb', + src_filepath: 'bar.erb', contents: 'rendered bar erb', ), ] diff --git a/src/bosh-director-core/spec/bosh/director/core/templates/rendered_templates_writer_spec.rb b/src/bosh-director-core/spec/bosh/director/core/templates/rendered_templates_writer_spec.rb index 6885769377a..88ec93acb26 100644 --- a/src/bosh-director-core/spec/bosh/director/core/templates/rendered_templates_writer_spec.rb +++ b/src/bosh-director-core/spec/bosh/director/core/templates/rendered_templates_writer_spec.rb @@ -10,14 +10,14 @@ module Bosh::Director::Core::Templates let(:rendered_file_template) do instance_double('Bosh::Director::Core::Templates::RenderedFileTemplate', - dest_name: 'bin/script-filename', + dest_filepath: 'bin/script-filename', contents: 'script file contents' ) end let(:rendered_file_template_with_deep_path) do instance_double('Bosh::Director::Core::Templates::RenderedFileTemplate', - dest_name: 'config/with/deeper/path/filename.cfg', + dest_filepath: 'config/with/deeper/path/filename.cfg', contents: 'config file contents' ) end