From 5829963b034d83cc572885df1fd3c91dbbc13240 Mon Sep 17 00:00:00 2001 From: Maxime Ginters Date: Mon, 29 Jul 2024 10:45:11 -0400 Subject: [PATCH] Add stack & accessors to Filesystem --- CHANGELOG.md | 2 ++ app/jobs/shipit/cache_deploy_spec_job.rb | 2 +- app/models/shipit/deploy_spec/file_system.rb | 7 +++++-- app/models/shipit/ephemeral_commit_checks.rb | 2 +- app/models/shipit/task.rb | 2 +- lib/shipit/stack_commands.rb | 4 ++-- lib/shipit/task_commands.rb | 2 +- test/models/deploy_spec_test.rb | 13 ++++++++----- .../models/shipit/deploy_spec/file_system_test.rb | 15 ++++++++++----- 9 files changed, 31 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 679df9436..5b4cf78e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Unreleased +* Pass `Shipit::Stack` to `DeploySpec::FileSystem.new` and make it accessible through an accessor. (#1356) + # 0.39.0 * Minimum Ruby version is now Ruby 3.0 diff --git a/app/jobs/shipit/cache_deploy_spec_job.rb b/app/jobs/shipit/cache_deploy_spec_job.rb index 12163c10e..d239d00b5 100644 --- a/app/jobs/shipit/cache_deploy_spec_job.rb +++ b/app/jobs/shipit/cache_deploy_spec_job.rb @@ -11,7 +11,7 @@ def perform(stack) commands = Commands.for(stack) commands.with_temporary_working_directory(commit: stack.commits.reachable.last) do |path| - stack.update!(cached_deploy_spec: DeploySpec::FileSystem.new(path, stack.environment)) + stack.update!(cached_deploy_spec: DeploySpec::FileSystem.new(path, stack)) end end end diff --git a/app/models/shipit/deploy_spec/file_system.rb b/app/models/shipit/deploy_spec/file_system.rb index 3708b9df3..e5efe2491 100644 --- a/app/models/shipit/deploy_spec/file_system.rb +++ b/app/models/shipit/deploy_spec/file_system.rb @@ -10,9 +10,12 @@ class FileSystem < DeploySpec include BundlerDiscovery include KubernetesDiscovery - def initialize(app_dir, env) + attr_reader :stack + + def initialize(app_dir, stack) @app_dir = Pathname(app_dir) - @env = env + @env = stack.environment + @stack = stack super(nil) end diff --git a/app/models/shipit/ephemeral_commit_checks.rb b/app/models/shipit/ephemeral_commit_checks.rb index 14000a822..7bb7fbf5c 100644 --- a/app/models/shipit/ephemeral_commit_checks.rb +++ b/app/models/shipit/ephemeral_commit_checks.rb @@ -14,7 +14,7 @@ def run self.status = 'running' commands = StackCommands.new(stack) commands.with_temporary_working_directory(commit: commit) do |directory| - deploy_spec = DeploySpec::FileSystem.new(directory, stack.environment) + deploy_spec = DeploySpec::FileSystem.new(directory, stack) capture_all(build_commands(deploy_spec.dependencies_steps, chdir: directory)) capture_all(build_commands(deploy_spec.review_checks, chdir: directory)) end diff --git a/app/models/shipit/task.rb b/app/models/shipit/task.rb index c0baa4688..43a9e2bc6 100644 --- a/app/models/shipit/task.rb +++ b/app/models/shipit/task.rb @@ -219,7 +219,7 @@ def duration end def spec - @spec ||= DeploySpec::FileSystem.new(working_directory, stack.environment) + @spec ||= DeploySpec::FileSystem.new(working_directory, stack) end def enqueue diff --git a/lib/shipit/stack_commands.rb b/lib/shipit/stack_commands.rb index 587d06a1b..00ec01079 100644 --- a/lib/shipit/stack_commands.rb +++ b/lib/shipit/stack_commands.rb @@ -49,7 +49,7 @@ def fetched?(commit) def fetch_deployed_revision with_temporary_working_directory(commit: @stack.commits.reachable.last) do |dir| - spec = DeploySpec::FileSystem.new(dir, @stack.environment) + spec = DeploySpec::FileSystem.new(dir, @stack) outputs = spec.fetch_deployed_revision_steps!.map do |command_line| Command.new(command_line, env: env, chdir: dir).run end @@ -59,7 +59,7 @@ def fetch_deployed_revision def build_cacheable_deploy_spec with_temporary_working_directory(recursive: false) do |dir| - DeploySpec::FileSystem.new(dir, @stack.environment).cacheable + DeploySpec::FileSystem.new(dir, @stack).cacheable end end diff --git a/lib/shipit/task_commands.rb b/lib/shipit/task_commands.rb index 275d14ba7..53c61ca87 100644 --- a/lib/shipit/task_commands.rb +++ b/lib/shipit/task_commands.rb @@ -10,7 +10,7 @@ def initialize(task) end def deploy_spec - @deploy_spec ||= DeploySpec::FileSystem.new(@task.working_directory, @stack.environment) + @deploy_spec ||= DeploySpec::FileSystem.new(@task.working_directory, @stack) end def install_dependencies diff --git a/test/models/deploy_spec_test.rb b/test/models/deploy_spec_test.rb index 6343629af..ecd136b62 100644 --- a/test/models/deploy_spec_test.rb +++ b/test/models/deploy_spec_test.rb @@ -5,7 +5,8 @@ module Shipit class DeploySpecTest < ActiveSupport::TestCase setup do @app_dir = '/tmp/' - @spec = DeploySpec::FileSystem.new(@app_dir, 'env') + @stack = shipit_stacks(:shipit) + @spec = DeploySpec::FileSystem.new(@app_dir, @stack) @spec.stubs(:load_config).returns({}) end @@ -287,7 +288,7 @@ class DeploySpecTest < ActiveSupport::TestCase end test '#gemspec gives the path of the repo gemspec if present' do - spec = DeploySpec::FileSystem.new('foobar/', 'production') + spec = DeploySpec::FileSystem.new('foobar/', @stack) Dir.expects(:[]).with('foobar/*.gemspec').returns(['foobar/foobar.gemspec']) assert_equal 'foobar/foobar.gemspec', spec.gemspec end @@ -309,7 +310,7 @@ class DeploySpecTest < ActiveSupport::TestCase end test '#setup_dot_py gives the path of the repo setup.py if present' do - spec = DeploySpec::FileSystem.new('foobar/', 'production') + spec = DeploySpec::FileSystem.new('foobar/', @stack) assert_equal Pathname.new('foobar/setup.py'), spec.setup_dot_py end @@ -438,7 +439,8 @@ def discover_task_definitions DuplicateCustomizedDeploySpec.include(TestTaskDiscovery) # Setup the spec as we would normally, but use the customized version - @spec = DuplicateCustomizedDeploySpec.new(@app_dir, 'env') + stack = shipit_stacks(:shipit) + @spec = DuplicateCustomizedDeploySpec.new(@app_dir, stack) @spec.stubs(:load_config).returns( 'tasks' => { 'config_task' => { 'steps' => %w(foo) } }, ) @@ -468,7 +470,8 @@ def discover_task_definitions CustomizedDeploySpec.include(TestTaskDiscovery) # Setup the spec as we would normally, but use the customized version - @spec = CustomizedDeploySpec.new(@app_dir, 'env') + stack = shipit_stacks(:shipit) + @spec = CustomizedDeploySpec.new(@app_dir, stack) @spec.stubs(:load_config).returns( 'tasks' => { 'config_task' => { 'steps' => %w(foo) } }, 'kubernetes' => { diff --git a/test/models/shipit/deploy_spec/file_system_test.rb b/test/models/shipit/deploy_spec/file_system_test.rb index e65480592..ece77e67b 100644 --- a/test/models/shipit/deploy_spec/file_system_test.rb +++ b/test/models/shipit/deploy_spec/file_system_test.rb @@ -7,7 +7,8 @@ class DeploySpec class FileSystemTest < ActiveSupport::TestCase test 'deploy.pre calls "exit 1" if there is a bare shipit file and Shipit is configured to ignore' do Shipit.expects(:respect_bare_shipit_file?).returns(false).at_least_once - deploy_spec = Shipit::DeploySpec::FileSystem.new(Dir.tmpdir, 'env') + stack = shipit_stacks(:shipit) + deploy_spec = Shipit::DeploySpec::FileSystem.new(Dir.tmpdir, stack) deploy_spec.expects(:config_file_path).returns(Pathname.new(Dir.tmpdir) + '/shipit.yml').at_least_once deploy_spec.expects(:read_config).never pre_commands = deploy_spec.send(:config, 'deploy', 'pre') @@ -18,7 +19,8 @@ class FileSystemTest < ActiveSupport::TestCase test 'deploy.pre does not call "exit 1" if Shipit is not configured to do so' do Shipit.expects(:respect_bare_shipit_file?).returns(true).at_least_once - deploy_spec = Shipit::DeploySpec::FileSystem.new(Dir.tmpdir, 'env') + stack = shipit_stacks(:shipit) + deploy_spec = Shipit::DeploySpec::FileSystem.new(Dir.tmpdir, stack) deploy_spec.expects(:config_file_path).returns(Pathname.new(Dir.tmpdir) + '/shipit.yml').at_least_once deploy_spec.expects(:read_config).returns(SafeYAML.load(deploy_spec_yaml)) pre_commands = deploy_spec.send(:config, 'deploy', 'pre') @@ -29,7 +31,8 @@ class FileSystemTest < ActiveSupport::TestCase test 'Shipit.respect_bare_shipit_file? has no effect if the file is not a bare file' do [true, false].each do |obey_val| Shipit.expects(:respect_bare_shipit_file?).returns(obey_val).at_least_once - deploy_spec = Shipit::DeploySpec::FileSystem.new(Dir.tmpdir, 'env') + stack = shipit_stacks(:shipit) + deploy_spec = Shipit::DeploySpec::FileSystem.new(Dir.tmpdir, stack) deploy_spec.expects(:config_file_path).returns(Pathname.new(Dir.tmpdir) + '/shipit.env.yml').at_least_once deploy_spec.expects(:read_config).returns(SafeYAML.load(deploy_spec_yaml)) pre_commands = deploy_spec.send(:config, 'deploy', 'pre') @@ -40,7 +43,8 @@ class FileSystemTest < ActiveSupport::TestCase test '#load_config does not error if the file is empty' do Shipit.expects(:respect_bare_shipit_file?).returns(true).at_least_once - deploy_spec = Shipit::DeploySpec::FileSystem.new(Dir.tmpdir, 'env') + stack = shipit_stacks(:shipit) + deploy_spec = Shipit::DeploySpec::FileSystem.new(Dir.tmpdir, stack) deploy_spec.expects(:config_file_path).returns(Pathname.new(Dir.tmpdir) + '/shipit.env.yml').at_least_once deploy_spec.expects(:read_config).at_least_once.returns(false) loaded_config = deploy_spec.send(:cacheable_config) @@ -49,7 +53,8 @@ class FileSystemTest < ActiveSupport::TestCase test '#load_config does not error if there is no "deploy" key' do Shipit.expects(:respect_bare_shipit_file?).returns(false).at_least_once - deploy_spec = Shipit::DeploySpec::FileSystem.new(Dir.tmpdir, 'env') + stack = shipit_stacks(:shipit) + deploy_spec = Shipit::DeploySpec::FileSystem.new(Dir.tmpdir, stack) deploy_spec.expects(:config_file_path).returns(Pathname.new(Dir.tmpdir) + '/shipit.yml').at_least_once deploy_spec.expects(:read_config).never loaded_config = deploy_spec.send(:load_config)