From a1a0b2a557e297e513680710dd1d2ef6145f0266 Mon Sep 17 00:00:00 2001 From: Asaf Erlich Date: Mon, 27 Nov 2017 14:59:07 -0800 Subject: [PATCH 01/17] Allowing for concurrent deployment (multithreading the agent) 1. Merging in Deepankar (intern) concurrent deployment changes - Launching a separate ruby thread for executing commands Issue he solved: If a CodeDeployment runs for more than 5 mins and the user starts another deployment, it fails after 5 minutes due to timeout as the Agent is busy executing the scripts. These changes makes the Agent to execute commands on separate threads which fixes the above mentioned issue.. Original cr shipped for Deepankar: https://code.amazon.com/reviews/CR-320735 2. Had to make modifications to fix bug: convert single thread.new to threadpool and gracefully shutdown waiting for all threads to finish, otherwise windows agent kills agent on restart in the middle of a deployment step Dry-run build: https://build.amazon.com/1810132858 cr https://code.amazon.com/reviews/CR-1007891 --- codedeploy_agent-1.1.0.gemspec | 1 + .../plugins/codedeploy/command_poller.rb | 67 ++++++++++++++----- lib/winagent.rb | 5 +- .../plugins/codedeploy/command_poller_test.rb | 22 ++++-- 4 files changed, 71 insertions(+), 24 deletions(-) diff --git a/codedeploy_agent-1.1.0.gemspec b/codedeploy_agent-1.1.0.gemspec index f7a03d90..e3138485 100644 --- a/codedeploy_agent-1.1.0.gemspec +++ b/codedeploy_agent-1.1.0.gemspec @@ -17,6 +17,7 @@ Gem::Specification.new do |spec| spec.add_dependency('aws-sdk-core', '~> 2.9') spec.add_dependency('simple_pid', '~> 0.2.1') spec.add_dependency('docopt', '~> 0.5.0') + spec.add_dependency('concurrent-ruby', '~> 1.0.5') spec.add_development_dependency('rake', '~> 10.0') spec.add_development_dependency('rspec', '~> 3.2.0') diff --git a/lib/instance_agent/plugins/codedeploy/command_poller.rb b/lib/instance_agent/plugins/codedeploy/command_poller.rb index 11866ffd..85dedcec 100644 --- a/lib/instance_agent/plugins/codedeploy/command_poller.rb +++ b/lib/instance_agent/plugins/codedeploy/command_poller.rb @@ -1,4 +1,5 @@ require 'socket' +require 'concurrent' require 'instance_metadata' require 'instance_agent/agent/base' @@ -22,6 +23,8 @@ class CommandPoller < InstanceAgent::Agent::Base "AfterAllowTraffic"=>["AfterAllowTraffic"], "ValidateService"=>["ValidateService"]} + WAIT_FOR_ALL_THREADS_TIMEOUT_SECONDS = 60 * 60 # One hour timeout in seconds + def initialize test_profile = InstanceAgent::Config.config[:codedeploy_test_profile] unless ["beta", "gamma"].include?(test_profile.downcase) @@ -44,6 +47,13 @@ def initialize @plugin = InstanceAgent::Plugins::CodeDeployPlugin::CommandExecutor.new(:hook_mapping => DEFAULT_HOOK_MAPPING) + @thread_pool = Concurrent::ThreadPoolExecutor.new( + #TODO: Make these values configurable in agent configuration + min_threads: 1, + max_threads: 16, + max_queue: 0 # unbounded work queue + ) + log(:debug, "Initializing Host Agent: " + "Host Identifier = #{@host_identifier}") end @@ -63,22 +73,11 @@ def perform begin spec = get_deployment_specification(command) - #Successful commands will complete without raising an exception - script_output = process_command(command, spec) - log(:debug, 'Calling PutHostCommandComplete: "Succeeded"') - @deploy_control_client.put_host_command_complete( - :command_status => 'Succeeded', - :diagnostics => {:format => "JSON", :payload => gather_diagnostics()}, - :host_command_identifier => command.host_command_identifier) - + #Commands will be executed on a separate thread. + @thread_pool.post { + process_command(command, spec) + } #Commands that throw an exception will be considered to have failed - rescue ScriptError => e - log(:debug, 'Calling PutHostCommandComplete: "Code Error" ') - @deploy_control_client.put_host_command_complete( - :command_status => "Failed", - :diagnostics => {:format => "JSON", :payload => gather_diagnostics_from_script_error(e)}, - :host_command_identifier => command.host_command_identifier) - raise e rescue Exception => e log(:debug, 'Calling PutHostCommandComplete: "Code Error" ') @deploy_control_client.put_host_command_complete( @@ -89,6 +88,16 @@ def perform end end + def graceful_shutdown + log(:info, "Gracefully shutting down command execution threads now, will wait up to #{WAIT_FOR_ALL_THREADS_TIMEOUT_SECONDS} seconds") + # tell the pool to shutdown in an orderly fashion, allowing in progress work to complete + @thread_pool.shutdown + # now wait for all work to complete, wait till the timeout value + # TODO: Make the timeout configurable in the agent configuration + @thread_pool.wait_for_termination WAIT_FOR_ALL_THREADS_TIMEOUT_SECONDS + log(:info, 'All command execution threads have been shut down') + end + def next_command log(:debug, "Calling PollHostCommand:") output = @deploy_control_client.poll_host_command(:host_identifier => @host_identifier) @@ -131,7 +140,33 @@ def get_deployment_specification(command) def process_command(command, spec) log(:debug, "Calling #{@plugin.to_s}.execute_command") - @plugin.execute_command(command, spec) + begin + #Successful commands will complete without raising an exception + @plugin.execute_command(command, spec) + + log(:debug, 'Calling PutHostCommandComplete: "Succeeded"') + @deploy_control_client.put_host_command_complete( + :command_status => 'Succeeded', + :diagnostics => {:format => "JSON", :payload => gather_diagnostics()}, + :host_command_identifier => command.host_command_identifier) + #Commands that throw an exception will be considered to have failed + rescue ScriptError => e + log(:debug, 'Calling PutHostCommandComplete: "Code Error" ') + @deploy_control_client.put_host_command_complete( + :command_status => "Failed", + :diagnostics => {:format => "JSON", :payload => gather_diagnostics_from_script_error(e)}, + :host_command_identifier => command.host_command_identifier) + log(:error, "Error during perform: #{e.class} - #{e.message} - #{e.backtrace.join("\n")}") + raise e + rescue Exception => e + log(:debug, 'Calling PutHostCommandComplete: "Code Error" ') + @deploy_control_client.put_host_command_complete( + :command_status => "Failed", + :diagnostics => {:format => "JSON", :payload => gather_diagnostics_from_error(e)}, + :host_command_identifier => command.host_command_identifier) + log(:error, "Error during perform: #{e.class} - #{e.message} - #{e.backtrace.join("\n")}") + raise e + end end private diff --git a/lib/winagent.rb b/lib/winagent.rb index 2141e80c..7ee52d87 100644 --- a/lib/winagent.rb +++ b/lib/winagent.rb @@ -55,9 +55,10 @@ def service_main end def service_stop - log(:info, 'stopping') + log(:info, 'stopping the agent') @polling_mutex.synchronize do - log(:info, 'exited') + @runner.graceful_shutdown + log(:info, 'command execution threads shutdown, agent exiting now') end end diff --git a/test/instance_agent/plugins/codedeploy/command_poller_test.rb b/test/instance_agent/plugins/codedeploy/command_poller_test.rb index 0756ab65..dc99051c 100644 --- a/test/instance_agent/plugins/codedeploy/command_poller_test.rb +++ b/test/instance_agent/plugins/codedeploy/command_poller_test.rb @@ -116,6 +116,7 @@ def gather_diagnostics(script_output) @deploy_control_client.expects(:poll_host_command). with(:host_identifier => @host_identifier). returns(@poll_host_command_output) + Thread.expects(:new).returns(nil) @poller.perform end @@ -178,6 +179,7 @@ def gather_diagnostics(script_output) @deploy_control_client.expects(:poll_host_command). with(:host_identifier => @host_identifier). returns(poll_host_command_output) + Thread.expects(:new).returns(nil) @poller.perform end @@ -252,6 +254,7 @@ def gather_diagnostics(script_output) with(:diagnostics => nil, :host_command_identifier => @command.host_command_identifier). returns(@poll_host_command_acknowledgement_output) + Thread.expects(:new).returns(nil) @poller.perform end @@ -293,6 +296,7 @@ def gather_diagnostics(script_output) with(:deployment_execution_id => @command.deployment_execution_id, :host_identifier => @host_identifier). returns(@get_deploy_specification_output) + Thread.expects(:new).returns(nil) @poller.perform end @@ -315,7 +319,7 @@ def gather_diagnostics(script_output) should 'not dispatch the command to the command executor' do @execute_command_state.become('never') - @executor.expects(:execute_command).never. + Thread.expects(:new).never. when(@execute_command_state.is('never')) assert_raise do @@ -346,7 +350,7 @@ def gather_diagnostics(script_output) should 'not dispatch the command to the command executor' do @execute_command_state.become('never') - @executor.expects(:execute_command).never. + Thread.expects(:new).never. when(@execute_command_state.is('never')) assert_raise do @@ -376,7 +380,7 @@ def gather_diagnostics(script_output) should 'not dispatch the command to the command executor' do @execute_command_state.become('never') - @executor.expects(:execute_command).never. + Thread.expects(:new).never. when(@execute_command_state.is('never')) assert_raise do @@ -401,7 +405,7 @@ def gather_diagnostics(script_output) @executor.expects(:execute_command). with(@command, @deployment_specification.generic_envelope) - @poller.perform + @poller.process_command(@command, @deployment_specification.generic_envelope) end should 'allow exceptions from execute_command to propagate to caller' do @@ -414,7 +418,7 @@ def gather_diagnostics(script_output) :host_command_identifier => @command.host_command_identifier) assert_raise "some error" do - @poller.perform + @poller.process_command(@command, @deployment_specification.generic_envelope) end end @@ -436,7 +440,7 @@ def gather_diagnostics(script_output) :host_command_identifier => @command.host_command_identifier) assert_raise script_error do - @poller.perform + @poller.process_command(@command, @deployment_specification.generic_envelope) end end @@ -449,6 +453,12 @@ def gather_diagnostics(script_output) :diagnostics => {:format => "JSON", :payload => gather_diagnostics("")}, :host_command_identifier => @command.host_command_identifier) + @poller.process_command(@command, @deployment_specification.generic_envelope) + end + + should 'call Thread.new to spin off a new thread for executing commands' do + Thread.expects(:new).returns(nil) + @poller.perform end From 34f8fe1176d43f96eafcd36cef67412369729eec Mon Sep 17 00:00:00 2001 From: Erlich Date: Tue, 28 Nov 2017 17:59:16 -0800 Subject: [PATCH 02/17] Made changes to make integration tests pass in windows 1. Because windows doesn't know about shebang lines it doesn't know bin/codedeploy-local must be called with ruby so I fixed it to run with ruby prefix only if it's windows. 2. The appspec needed to be updated to include all the same hooks as linux. 3. Moved windows certificate configuration to before the first time aws sts is called instead of later on in the code path Dry-run build: https://build.amazon.com/1810143965 cr https://code.amazon.com/reviews/CR-1020258 --- features/resources/sample_app_bundle_windows/appspec.yml | 8 ++++++++ features/step_definitions/agent_steps.rb | 9 --------- features/step_definitions/codedeploy_local_steps.rb | 5 ++++- features/step_definitions/step_constants.rb | 9 +++++++++ 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/features/resources/sample_app_bundle_windows/appspec.yml b/features/resources/sample_app_bundle_windows/appspec.yml index 7cae4016..71e81cea 100644 --- a/features/resources/sample_app_bundle_windows/appspec.yml +++ b/features/resources/sample_app_bundle_windows/appspec.yml @@ -1,6 +1,10 @@ version: 0.0 os: windows hooks: + BeforeBlockTraffic: + - location: scripts/before_block_traffic.bat + AfterBlockTraffic: + - location: scripts/after_block_traffic.bat ApplicationStop: - location: scripts/application_stop.bat BeforeInstall: @@ -11,3 +15,7 @@ hooks: - location: scripts/application_start.bat ValidateService: - location: scripts/validate_service.bat + BeforeAllowTraffic: + - location: scripts/before_allow_traffic.bat + AfterAllowTraffic: + - location: scripts/after_allow_traffic.bat diff --git a/features/step_definitions/agent_steps.rb b/features/step_definitions/agent_steps.rb index 540ba11e..168aa8c6 100644 --- a/features/step_definitions/agent_steps.rb +++ b/features/step_definitions/agent_steps.rb @@ -60,8 +60,6 @@ def configure_local_agent(working_directory) InstanceAgent::Log.init(File.join(working_directory, 'codedeploy-agent.log')) InstanceAgent::Config.init InstanceAgent::Platform.util = StepConstants::IS_WINDOWS ? InstanceAgent::WindowsUtil : InstanceAgent::LinuxUtil - - if StepConstants::IS_WINDOWS then configure_windows_certificate end InstanceAgent::Config.config[:on_premises_config_file] = "#{working_directory}/codedeploy.onpremises.yml" configuration_contents = <<-CONFIG @@ -84,13 +82,6 @@ def configure_local_agent(working_directory) InstanceAgent::Config.load_config end -def configure_windows_certificate - cert_dir = File.expand_path(File.join(File.dirname(__FILE__), '..\certs')) - Aws.config[:ssl_ca_bundle] = File.join(cert_dir, 'windows-ca-bundle.crt') - ENV['AWS_SSL_CA_DIRECTORY'] = File.join(cert_dir, 'windows-ca-bundle.crt') - ENV['SSL_CERT_FILE'] = File.join(cert_dir, 'windows-ca-bundle.crt') -end - After("@codedeploy-agent") do @thread.kill unless @thread.nil? @codedeploy_client.delete_application({:application_name => @application_name}) unless @application_name.nil? diff --git a/features/step_definitions/codedeploy_local_steps.rb b/features/step_definitions/codedeploy_local_steps.rb index 592ac923..2f1b2182 100644 --- a/features/step_definitions/codedeploy_local_steps.rb +++ b/features/step_definitions/codedeploy_local_steps.rb @@ -134,7 +134,10 @@ def create_local_deployment(custom_events = nil, file_exists_behavior = nil) codeedeploy_command_suffix = " --file-exists-behavior #{file_exists_behavior}" end - system "bin/codedeploy-local --bundle-location #{@bundle_location} --type #{@bundle_type} --deployment-group #{LOCAL_DEPLOYMENT_GROUP_ID} --agent-configuration-file #{InstanceAgent::Config.config[:config_file]}#{codeedeploy_command_suffix}" + # Windows doesn't respect shebang lines so ruby needs to be specified + ruby_prefix_for_windows = StepConstants::IS_WINDOWS ? "ruby " : "" + + system "#{ruby_prefix_for_windows}bin/codedeploy-local --bundle-location #{@bundle_location} --type #{@bundle_type} --deployment-group #{LOCAL_DEPLOYMENT_GROUP_ID} --agent-configuration-file #{InstanceAgent::Config.config[:config_file]}#{codeedeploy_command_suffix}" end Then(/^the local deployment command should succeed$/) do diff --git a/features/step_definitions/step_constants.rb b/features/step_definitions/step_constants.rb index f0002421..dfd19adc 100644 --- a/features/step_definitions/step_constants.rb +++ b/features/step_definitions/step_constants.rb @@ -5,9 +5,18 @@ class StepConstants ENV['AWS_REGION'] = Aws.config[:region] def self.current_aws_account + if StepConstants::IS_WINDOWS then StepConstants.configure_windows_certificate end + Aws::STS::Client.new.get_caller_identity.account end + def self.configure_windows_certificate + cert_dir = File.expand_path(File.join(File.dirname(__FILE__), '..\..\certs')) + Aws.config[:ssl_ca_bundle] = File.join(cert_dir, 'windows-ca-bundle.crt') + ENV['AWS_SSL_CA_DIRECTORY'] = File.join(cert_dir, 'windows-ca-bundle.crt') + ENV['SSL_CERT_FILE'] = File.join(cert_dir, 'windows-ca-bundle.crt') + end + CODEDEPLOY_TEST_PREFIX = "codedeploy-agent-integ-test-" unless defined?(CODEDEPLOY_TEST_PREFIX) IS_WINDOWS = (RbConfig::CONFIG['host_os'] =~ /mswin|mingw|cygwin/) unless defined?(IS_WINDOWS) APP_BUNDLE_BUCKET_SUFFIX = IS_WINDOWS ? '-win' : '-linux' unless defined?(APP_BUNDLE_BUCKET_SUFFIX) From 078bd2f7d070b7e0ba7589215b44e6d617e549f5 Mon Sep 17 00:00:00 2001 From: dgupta8 Date: Mon, 4 Dec 2017 15:53:13 -0800 Subject: [PATCH 03/17] Fixing the validator error for local CLI cr https://code.amazon.com/reviews/CR-1044291 --- lib/aws/codedeploy/local/cli_validator.rb | 4 ++-- .../aws/codedeploy/local/cli_validator_spec.rb | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/lib/aws/codedeploy/local/cli_validator.rb b/lib/aws/codedeploy/local/cli_validator.rb index 19ebeeaf..12982067 100644 --- a/lib/aws/codedeploy/local/cli_validator.rb +++ b/lib/aws/codedeploy/local/cli_validator.rb @@ -52,7 +52,7 @@ def validate(args) end if events.include?('Install') && any_new_revision_event_before_install(events) - raise ValidationError.new("The only events that can be specified before Install are #{events_using_previous_successfuly_deployment_revision.push('DownloadBundle').join(',')}. Please fix the order of your specified events: #{args['--events']}") + raise ValidationError.new("The only events that can be specified before Install are #{events_using_previous_successfuly_deployment_revision.push('DownloadBundle', 'BeforeInstall').join(',')}. Please fix the order of your specified events: #{args['--events']}") end end @@ -79,7 +79,7 @@ def events_using_previous_successfuly_deployment_revision def events_using_new_revision InstanceAgent::Plugins::CodeDeployPlugin::HookExecutor::MAPPING_BETWEEN_HOOKS_AND_DEPLOYMENTS.select do |key,value| - value != InstanceAgent::Plugins::CodeDeployPlugin::HookExecutor::LAST_SUCCESSFUL_DEPLOYMENT + value != InstanceAgent::Plugins::CodeDeployPlugin::HookExecutor::LAST_SUCCESSFUL_DEPLOYMENT && key != 'BeforeInstall' end.keys end diff --git a/spec/aws/codedeploy/local/cli_validator_spec.rb b/spec/aws/codedeploy/local/cli_validator_spec.rb index 4b6cad4a..2216abac 100644 --- a/spec/aws/codedeploy/local/cli_validator_spec.rb +++ b/spec/aws/codedeploy/local/cli_validator_spec.rb @@ -208,8 +208,24 @@ allow(File).to receive(:exists?).with(FAKE_DIRECTORY).and_return(true) allow(File).to receive(:directory?).with(FAKE_DIRECTORY).and_return(true) expect(File).to receive(:exists?).with("#{FAKE_DIRECTORY}/appspec.yml").and_return(true) - expect{validator.validate(args)}.to raise_error(AWS::CodeDeploy::Local::CLIValidator::ValidationError, "The only events that can be specified before Install are BeforeBlockTraffic,AfterBlockTraffic,ApplicationStop,DownloadBundle. Please fix the order of your specified events: #{args['--events']}") + expect{validator.validate(args)}.to raise_error(AWS::CodeDeploy::Local::CLIValidator::ValidationError, "The only events that can be specified before Install are BeforeBlockTraffic,AfterBlockTraffic,ApplicationStop,DownloadBundle,BeforeInstall. Please fix the order of your specified events: #{args['--events']}") end end + + context 'when BeforeInstall event specified before Install' do + let(:args) do + {"--bundle-location"=>FAKE_DIRECTORY, + "--type"=>'directory', + '--events'=>'BeforeInstall,Install' + } + end + + it 'returns the same arguments' do + allow(File).to receive(:exists?).with(FAKE_DIRECTORY).and_return(true) + allow(File).to receive(:directory?).with(FAKE_DIRECTORY).and_return(true) + expect(File).to receive(:exists?).with("#{FAKE_DIRECTORY}/appspec.yml").and_return(true) + expect(validator.validate(args)).to equal(args) + end + end end end From ec26edbfc1c3f4662246015cfa045b9ec911d9f4 Mon Sep 17 00:00:00 2001 From: Katyal Date: Wed, 3 Jan 2018 13:27:09 -0800 Subject: [PATCH 04/17] Fixed tests ,making them compatible with Mac Osx and hopefully platform independent cr https://code.amazon.com/reviews/CR-1164333 --- .../codedeploy/install_instruction_test.rb | 68 +++++++++---------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/test/instance_agent/plugins/codedeploy/install_instruction_test.rb b/test/instance_agent/plugins/codedeploy/install_instruction_test.rb index 6be719eb..9fd8821d 100644 --- a/test/instance_agent/plugins/codedeploy/install_instruction_test.rb +++ b/test/instance_agent/plugins/codedeploy/install_instruction_test.rb @@ -363,7 +363,7 @@ class InstallInstructionTest < InstanceAgentTestCase setup do @command_builder = CommandBuilder.new() @command_builder.copy("source", "destination") - @expected_json = {"instructions"=>[{"type"=>"copy","source"=>"source","destination"=>"#{Dir.tmpdir()}/destination"}]}.to_json + @expected_json = {"instructions"=>[{"type"=>"copy","source"=>"source","destination"=>"#{File.realdirpath(Dir.tmpdir())}/destination"}]}.to_json end should "have a single copy in the returned JSON" do @@ -371,7 +371,7 @@ class InstallInstructionTest < InstanceAgentTestCase end should "raise a duplicate exception when a copy collides with another copy" do - assert_raised_with_message("The deployment failed because the application specification file specifies two source files named source and source for the same destination (#{Dir.tmpdir()}/destination). Remove one of the source file paths from the AppSpec file, and then try again.") do + assert_raised_with_message("The deployment failed because the application specification file specifies two source files named source and source for the same destination (#{File.realdirpath(Dir.tmpdir())}/destination). Remove one of the source file paths from the AppSpec file, and then try again.") do @command_builder.copy("source", "destination") end end @@ -381,7 +381,7 @@ class InstallInstructionTest < InstanceAgentTestCase setup do @command_builder = CommandBuilder.new() @command_builder.mkdir("directory") - @expected_json = {"instructions"=>[{"type"=>"mkdir","directory"=>"#{Dir.tmpdir()}/directory"}]}.to_json + @expected_json = {"instructions"=>[{"type"=>"mkdir","directory"=>"#{File.realdirpath(Dir.tmpdir())}/directory"}]}.to_json end should "have a single mkdir in the returned JSON" do @@ -390,7 +390,7 @@ class InstallInstructionTest < InstanceAgentTestCase should "raise a duplicate exception when trying to create a directory collides with a copy" do @command_builder.copy("source", "directory/dir1") - assert_raised_with_message("The deployment failed because the application specification file includes an mkdir command more than once for the same destination path (#{Dir.tmpdir()}/directory/dir1) from (source). Update the files section of the AppSpec file, and then try again.") do + assert_raised_with_message("The deployment failed because the application specification file includes an mkdir command more than once for the same destination path (#{File.realdirpath(Dir.tmpdir())}/directory/dir1) from (source). Update the files section of the AppSpec file, and then try again.") do @command_builder.mkdir("directory/dir1") end end @@ -404,71 +404,71 @@ class InstallInstructionTest < InstanceAgentTestCase end should "raise a duplicate exception when trying to make a copy collides with a mkdir" do - assert_raised_with_message("The deployment failed because the application specification file calls for installing the file target, but a file with that name already exists at the location (#{Dir.tmpdir()}/directory/target). Update your AppSpec file or directory structure, and then try again.") do + assert_raised_with_message("The deployment failed because the application specification file calls for installing the file target, but a file with that name already exists at the location (#{File.realdirpath(Dir.tmpdir())}/directory/target). Update your AppSpec file or directory structure, and then try again.") do @command_builder.copy( "target", "directory/target") end end should "say it is copying the appropriate file" do - assert @command_builder.copying_file?("#{Dir.tmpdir()}/directory/target/file_target") - assert !@command_builder.copying_file?("#{Dir.tmpdir()}/directory/target") + assert @command_builder.copying_file?("#{File.realdirpath(Dir.tmpdir())}/directory/target/file_target") + assert !@command_builder.copying_file?("#{File.realdirpath(Dir.tmpdir())}/directory/target") end should "say it is making the appropriate directory" do - assert !@command_builder.making_directory?("#{Dir.tmpdir()}/directory/target/file_target") - assert @command_builder.making_directory?("#{Dir.tmpdir()}/directory/target") + assert !@command_builder.making_directory?("#{File.realdirpath(Dir.tmpdir())}/directory/target/file_target") + assert @command_builder.making_directory?("#{File.realdirpath(Dir.tmpdir())}/directory/target") end should "match the file when appropriate" do - permission = InstanceAgent::Plugins::CodeDeployPlugin::ApplicationSpecification::LinuxPermissionInfo.new("#{Dir.tmpdir()}/directory/target", { + permission = InstanceAgent::Plugins::CodeDeployPlugin::ApplicationSpecification::LinuxPermissionInfo.new("#{File.realdirpath(Dir.tmpdir())}/directory/target", { :type => ["file"], :pattern => "file*", :except => []}) - assert @command_builder.find_matches(permission).include?("#{Dir.tmpdir()}/directory/target/file_target") + assert @command_builder.find_matches(permission).include?("#{File.realdirpath(Dir.tmpdir())}/directory/target/file_target") - permission = InstanceAgent::Plugins::CodeDeployPlugin::ApplicationSpecification::LinuxPermissionInfo.new("#{Dir.tmpdir()}/directory/target", { + permission = InstanceAgent::Plugins::CodeDeployPlugin::ApplicationSpecification::LinuxPermissionInfo.new("#{File.realdirpath(Dir.tmpdir())}/directory/target", { :type => ["directory"], :pattern => "file*", :except => []}) - assert !@command_builder.find_matches(permission).include?("#{Dir.tmpdir()}/directory/target/file_target") + assert !@command_builder.find_matches(permission).include?("#{File.realdirpath(Dir.tmpdir())}/directory/target/file_target") - permission = InstanceAgent::Plugins::CodeDeployPlugin::ApplicationSpecification::LinuxPermissionInfo.new("#{Dir.tmpdir()}/directory/target", { + permission = InstanceAgent::Plugins::CodeDeployPlugin::ApplicationSpecification::LinuxPermissionInfo.new("#{File.realdirpath(Dir.tmpdir())}/directory/target", { :type => ["file"], :pattern => "filefile*", :except => []}) - assert !@command_builder.find_matches(permission).include?("#{Dir.tmpdir()}/directory/target/file_target") + assert !@command_builder.find_matches(permission).include?("#{File.realdirpath(Dir.tmpdir())}/directory/target/file_target") - permission = InstanceAgent::Plugins::CodeDeployPlugin::ApplicationSpecification::LinuxPermissionInfo.new("#{Dir.tmpdir()}/directory/target", { + permission = InstanceAgent::Plugins::CodeDeployPlugin::ApplicationSpecification::LinuxPermissionInfo.new("#{File.realdirpath(Dir.tmpdir())}/directory/target", { :type => ["file"], :pattern => "file*", :except => ["*target"]}) - assert !@command_builder.find_matches(permission).include?("#{Dir.tmpdir()}/directory/target/file_target") + assert !@command_builder.find_matches(permission).include?("#{File.realdirpath(Dir.tmpdir())}/directory/target/file_target") end should "match the directory when appropriate" do - permission = InstanceAgent::Plugins::CodeDeployPlugin::ApplicationSpecification::LinuxPermissionInfo.new("#{Dir.tmpdir()}/directory/", { + permission = InstanceAgent::Plugins::CodeDeployPlugin::ApplicationSpecification::LinuxPermissionInfo.new("#{File.realdirpath(Dir.tmpdir())}/directory/", { :type => ["directory"], :pattern => "tar*", :except => []}) - assert @command_builder.find_matches(permission).include?("#{Dir.tmpdir()}/directory/target") + assert @command_builder.find_matches(permission).include?("#{File.realdirpath(Dir.tmpdir())}/directory/target") - permission = InstanceAgent::Plugins::CodeDeployPlugin::ApplicationSpecification::LinuxPermissionInfo.new("#{Dir.tmpdir()}/directory/", { + permission = InstanceAgent::Plugins::CodeDeployPlugin::ApplicationSpecification::LinuxPermissionInfo.new("#{File.realdirpath(Dir.tmpdir())}/directory/", { :type => ["file"], :pattern => "tar*", :except => []}) - assert !@command_builder.find_matches(permission).include?("#{Dir.tmpdir()}/directory/target") + assert !@command_builder.find_matches(permission).include?("#{File.realdirpath(Dir.tmpdir())}/directory/target") - permission = InstanceAgent::Plugins::CodeDeployPlugin::ApplicationSpecification::LinuxPermissionInfo.new("#{Dir.tmpdir()}/directory/", { + permission = InstanceAgent::Plugins::CodeDeployPlugin::ApplicationSpecification::LinuxPermissionInfo.new("#{File.realdirpath(Dir.tmpdir())}/directory/", { :type => ["directory"], :pattern => "tarr*", :except => []}) - assert !@command_builder.find_matches(permission).include?("#{Dir.tmpdir()}/directory/target") + assert !@command_builder.find_matches(permission).include?("#{File.realdirpath(Dir.tmpdir())}/directory/target") - permission = InstanceAgent::Plugins::CodeDeployPlugin::ApplicationSpecification::LinuxPermissionInfo.new("#{Dir.tmpdir()}/directory/", { + permission = InstanceAgent::Plugins::CodeDeployPlugin::ApplicationSpecification::LinuxPermissionInfo.new("#{File.realdirpath(Dir.tmpdir())}/directory/", { :type => ["directory"], :pattern => "tar*", :except => ["*et"]}) - assert !@command_builder.find_matches(permission).include?("#{Dir.tmpdir()}/directory/target") + assert !@command_builder.find_matches(permission).include?("#{File.realdirpath(Dir.tmpdir())}/directory/target") end end @@ -477,7 +477,7 @@ class InstallInstructionTest < InstanceAgentTestCase @command_builder = CommandBuilder.new() @command_builder.mkdir("directory") @command_builder.mkdir("directory") - @expected_json = {"instructions"=>[{"type"=>"mkdir","directory"=>"#{Dir.tmpdir()}/directory"}]}.to_json + @expected_json = {"instructions"=>[{"type"=>"mkdir","directory"=>"#{File.realdirpath(Dir.tmpdir())}/directory"}]}.to_json end should "have a single mkdir in the returned JSON" do @@ -490,7 +490,7 @@ class InstallInstructionTest < InstanceAgentTestCase @command_builder = CommandBuilder.new() @command_builder.mkdir("directory") @command_builder.mkdir("directory/") - @expected_json = {"instructions"=>[{"type"=>"mkdir","directory"=>"#{Dir.tmpdir()}/directory"}]}.to_json + @expected_json = {"instructions"=>[{"type"=>"mkdir","directory"=>"#{File.realdirpath(Dir.tmpdir())}/directory"}]}.to_json end should "have a single mkdir in the returned JSON" do @@ -503,7 +503,7 @@ class InstallInstructionTest < InstanceAgentTestCase @permission = InstanceAgent::Plugins::CodeDeployPlugin::ApplicationSpecification::LinuxPermissionInfo.new("testfile.txt") @command_builder = CommandBuilder.new() @command_builder.set_permissions("testfile.txt", @permission) - assert_raised_with_message("The deployment failed because the permissions setting for (#{Dir.tmpdir()}/testfile.txt) is specified more than once in the application specification file. Update the files section of the AppSpec file, and then try again.") do + assert_raised_with_message("The deployment failed because the permissions setting for (#{File.realdirpath(Dir.tmpdir())}/testfile.txt) is specified more than once in the application specification file. Update the files section of the AppSpec file, and then try again.") do @command_builder.set_permissions("testfile.txt", @permission) end end @@ -525,10 +525,10 @@ class InstallInstructionTest < InstanceAgentTestCase :group=>"dev"}) @command_builder = CommandBuilder.new() @command_builder.set_permissions("testfile.txt", @permission) - @expected_json = {"instructions"=>[{"type"=>"chmod","mode"=>"744","file"=>"#{Dir.tmpdir()}/testfile.txt"}, - {"type"=>"setfacl","acl"=>["user:bob:rwx","default:group:dev:r--"],"file"=>"#{Dir.tmpdir()}/testfile.txt"}, - {"type"=>"semanage","context"=>{"user"=>"name","role"=>nil,"type"=>"type","range"=>"s2-s3:c0,c2.c4,c6"},"file"=>"#{Dir.tmpdir()}/testfile.txt"}, - {"type"=>"chown","owner"=>"bob","group"=>"dev","file"=>"#{Dir.tmpdir()}/testfile.txt"} + @expected_json = {"instructions"=>[{"type"=>"chmod","mode"=>"744","file"=>"#{File.realdirpath(Dir.tmpdir())}/testfile.txt"}, + {"type"=>"setfacl","acl"=>["user:bob:rwx","default:group:dev:r--"],"file"=>"#{File.realdirpath(Dir.tmpdir())}/testfile.txt"}, + {"type"=>"semanage","context"=>{"user"=>"name","role"=>nil,"type"=>"type","range"=>"s2-s3:c0,c2.c4,c6"},"file"=>"#{File.realdirpath(Dir.tmpdir())}/testfile.txt"}, + {"type"=>"chown","owner"=>"bob","group"=>"dev","file"=>"#{File.realdirpath(Dir.tmpdir())}/testfile.txt"} ]}.to_json assert_equal JSON.parse(@expected_json), JSON.parse(@command_builder.to_json) end @@ -537,7 +537,7 @@ class InstallInstructionTest < InstanceAgentTestCase @permission = InstanceAgent::Plugins::CodeDeployPlugin::ApplicationSpecification::LinuxPermissionInfo.new("testfile.txt", {:owner=>"bob"}) @command_builder = CommandBuilder.new() @command_builder.set_permissions("testfile.txt", @permission) - @expected_json = {"instructions"=>[{"type"=>"chown","owner"=>"bob","group"=>nil,"file"=>"#{Dir.tmpdir()}/testfile.txt"}]}.to_json + @expected_json = {"instructions"=>[{"type"=>"chown","owner"=>"bob","group"=>nil,"file"=>"#{File.realdirpath(Dir.tmpdir())}/testfile.txt"}]}.to_json assert_equal JSON.parse(@expected_json), JSON.parse(@command_builder.to_json) end @@ -545,7 +545,7 @@ class InstallInstructionTest < InstanceAgentTestCase @permission = InstanceAgent::Plugins::CodeDeployPlugin::ApplicationSpecification::LinuxPermissionInfo.new("testfile.txt", {:group=>"dev"}) @command_builder = CommandBuilder.new() @command_builder.set_permissions("testfile.txt", @permission) - @expected_json = {"instructions"=>[{"type"=>"chown","owner"=>nil,"group"=>"dev","file"=>"#{Dir.tmpdir()}/testfile.txt"}]}.to_json + @expected_json = {"instructions"=>[{"type"=>"chown","owner"=>nil,"group"=>"dev","file"=>"#{File.realdirpath(Dir.tmpdir())}/testfile.txt"}]}.to_json assert_equal JSON.parse(@expected_json), JSON.parse(@command_builder.to_json) end end From 85d5eea4e5fb63916484760bb731d3be7bf2c3a4 Mon Sep 17 00:00:00 2001 From: Katyal Date: Thu, 18 Jan 2018 13:38:43 -0800 Subject: [PATCH 05/17] deployment command /event tracking logic added cr https://code.amazon.com/reviews/CR-1244946 --- .../codedeploy_local_steps.rb | 6 +- features/step_definitions/common_steps.rb | 23 +++---- lib/instance_agent/config.rb | 1 + .../plugins/codedeploy/command_poller.rb | 11 +++- .../codedeploy/deployment_command_tracker.rb | 62 +++++++++++++++++++ .../codedeploy/deployment_specification.rb | 4 +- lib/instance_agent/runner/master.rb | 17 +++-- .../deployment_command_tracker_spec.rb | 61 ++++++++++++++++++ test/instance_agent/config_test.rb | 1 + .../plugins/codedeploy/command_poller_test.rb | 13 +++- 10 files changed, 172 insertions(+), 27 deletions(-) create mode 100644 lib/instance_agent/plugins/codedeploy/deployment_command_tracker.rb create mode 100644 spec/aws/codedeploy/plugins/deployment_command_tracker_spec.rb diff --git a/features/step_definitions/codedeploy_local_steps.rb b/features/step_definitions/codedeploy_local_steps.rb index 2f1b2182..1d236c05 100644 --- a/features/step_definitions/codedeploy_local_steps.rb +++ b/features/step_definitions/codedeploy_local_steps.rb @@ -60,7 +60,7 @@ def tar_app_bundle(temp_directory_to_create_bundle) #Unfortunately Minitar will keep pack all the file paths as given, so unless you change directories into the location where you want to pack the files the bundle won't have the correct files and folders Dir.chdir @bundle_original_directory_location - File.open(tar_file_name, 'wb') { |tar| Minitar.pack(directories_and_files_inside(@bundle_original_directory_location), tar) } + File.open(tar_file_name, 'wb') { |tar| Minitar.pack(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside(@bundle_original_directory_location), tar) } Dir.chdir old_direcory tar_file_name @@ -102,7 +102,7 @@ def tgz_app_bundle(temp_directory_to_create_bundle) File.open(tgz_file_name, 'wb') do |file| Zlib::GzipWriter.wrap(file) do |gz| - Minitar.pack(directories_and_files_inside(@bundle_original_directory_location), gz) + Minitar.pack(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside(@bundle_original_directory_location), gz) end end @@ -149,7 +149,7 @@ def create_local_deployment(custom_events = nil, file_exists_behavior = nil) end Then(/^the expected files should have have been locally deployed to my host(| twice)$/) do |maybe_twice| - deployment_ids = directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{LOCAL_DEPLOYMENT_GROUP_ID}") + deployment_ids = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{LOCAL_DEPLOYMENT_GROUP_ID}") step "the expected files in directory #{bundle_original_directory_location}/scripts should have have been deployed#{maybe_twice} to my host during deployment with deployment group id #{LOCAL_DEPLOYMENT_GROUP_ID} and deployment ids #{deployment_ids.join(' ')}" end diff --git a/features/step_definitions/common_steps.rb b/features/step_definitions/common_steps.rb index 50c11ac8..dda7a9e7 100644 --- a/features/step_definitions/common_steps.rb +++ b/features/step_definitions/common_steps.rb @@ -34,7 +34,7 @@ def zip_app_bundle(temp_directory_to_create_bundle) end def zip_directory(input_dir, output_file) - entries = directories_and_files_inside(input_dir) + entries = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside(input_dir) zip_io = Zip::File.open(output_file, Zip::File::CREATE) write_zip_entries(entries, '', input_dir, zip_io) @@ -47,7 +47,7 @@ def write_zip_entries(entries, path, input_dir, zip_io) diskFilePath = File.join(input_dir, zipFilePath) if File.directory?(diskFilePath) zip_io.mkdir(zipFilePath) - folder_entries = directories_and_files_inside(diskFilePath) + folder_entries = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside(diskFilePath) write_zip_entries(folder_entries, zipFilePath, input_dir, zip_io) else zip_io.get_output_stream(zipFilePath){ |f| f.write(File.open(diskFilePath, "rb").read())} @@ -55,36 +55,33 @@ def write_zip_entries(entries, path, input_dir, zip_io) end end -def directories_and_files_inside(directory) - Dir.entries(directory) - %w(.. .) -end Then(/^the expected files in directory (\S+) should have have been deployed(| twice) to my host during deployment with deployment group id (\S+) and deployment ids (.+)$/) do |expected_scripts_directory, maybe_twice, deployment_group_id, deployment_ids_space_separated| deployment_ids = deployment_ids_space_separated.split(' ') - directories_in_deployment_root_folder = directories_and_files_inside(InstanceAgent::Config.config[:root_dir]) - expect(directories_in_deployment_root_folder.size).to eq(3) + directories_in_deployment_root_folder = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside(InstanceAgent::Config.config[:root_dir]) + expect(directories_in_deployment_root_folder.size).to be >= 3 #ordering of the directories depends on the deployment group id, so using include instead of eq expect(directories_in_deployment_root_folder).to include(*%W(deployment-instructions deployment-logs #{deployment_group_id})) - files_in_deployment_logs_folder = directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/deployment-logs") + files_in_deployment_logs_folder = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/deployment-logs") expect(files_in_deployment_logs_folder.size).to eq(1) expect(files_in_deployment_logs_folder).to eq(%w(codedeploy-agent-deployments.log)) - directories_in_deployment_group_id_folder = directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}") + directories_in_deployment_group_id_folder = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}") expect(directories_in_deployment_group_id_folder.size).to eq(maybe_twice.empty? ? 1 : 2) expect(directories_in_deployment_group_id_folder).to eq(deployment_ids) deployment_id = deployment_ids.first - files_and_directories_in_deployment_id_folder = directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}/#{deployment_id}") + files_and_directories_in_deployment_id_folder = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}/#{deployment_id}") expect(files_and_directories_in_deployment_id_folder).to include(*%w(logs deployment-archive)) - files_and_directories_in_deployment_archive_folder = directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}/#{deployment_id}/deployment-archive") + files_and_directories_in_deployment_archive_folder = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}/#{deployment_id}/deployment-archive") expect(files_and_directories_in_deployment_archive_folder.size).to eq(2) expect(files_and_directories_in_deployment_archive_folder).to include(*%w(appspec.yml scripts)) - files_in_scripts_folder = directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}/#{deployment_id}/deployment-archive/scripts") - sample_app_bundle_script_files = directories_and_files_inside(expected_scripts_directory) + files_in_scripts_folder = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}/#{deployment_id}/deployment-archive/scripts") + sample_app_bundle_script_files = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside(expected_scripts_directory) expect(files_in_scripts_folder.size).to eq(sample_app_bundle_script_files.size) expect(files_in_scripts_folder).to include(*sample_app_bundle_script_files) end diff --git a/lib/instance_agent/config.rb b/lib/instance_agent/config.rb index 13319712..ee2d6d74 100644 --- a/lib/instance_agent/config.rb +++ b/lib/instance_agent/config.rb @@ -23,6 +23,7 @@ def initialize :pid_dir => nil, :shared_dir => nil, :user => nil, + :ongoing_deployment_tracking => 'ongoing-deployment', :children => 1, :http_read_timeout => 80, :instance_service_region => nil, diff --git a/lib/instance_agent/plugins/codedeploy/command_poller.rb b/lib/instance_agent/plugins/codedeploy/command_poller.rb index 85dedcec..118b2e05 100644 --- a/lib/instance_agent/plugins/codedeploy/command_poller.rb +++ b/lib/instance_agent/plugins/codedeploy/command_poller.rb @@ -1,8 +1,9 @@ require 'socket' require 'concurrent' - +require 'pathname' require 'instance_metadata' require 'instance_agent/agent/base' +require_relative 'deployment_command_tracker' module InstanceAgent module Plugins @@ -23,7 +24,7 @@ class CommandPoller < InstanceAgent::Agent::Base "AfterAllowTraffic"=>["AfterAllowTraffic"], "ValidateService"=>["ValidateService"]} - WAIT_FOR_ALL_THREADS_TIMEOUT_SECONDS = 60 * 60 # One hour timeout in seconds + WAIT_FOR_ALL_THREADS_TIMEOUT_SECONDS = 3600 # One hour timeout in seconds def initialize test_profile = InstanceAgent::Config.config[:codedeploy_test_profile] @@ -141,9 +142,11 @@ def get_deployment_specification(command) def process_command(command, spec) log(:debug, "Calling #{@plugin.to_s}.execute_command") begin + deployment_id = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentSpecification.parse(spec).deployment_id + DeploymentCommandTracker.create_ongoing_deployment_tracking_file(deployment_id) #Successful commands will complete without raising an exception @plugin.execute_command(command, spec) - + log(:debug, 'Calling PutHostCommandComplete: "Succeeded"') @deploy_control_client.put_host_command_complete( :command_status => 'Succeeded', @@ -166,6 +169,8 @@ def process_command(command, spec) :host_command_identifier => command.host_command_identifier) log(:error, "Error during perform: #{e.class} - #{e.message} - #{e.backtrace.join("\n")}") raise e + ensure + DeploymentCommandTracker.delete_deployment_command_tracking_file(deployment_id) end end diff --git a/lib/instance_agent/plugins/codedeploy/deployment_command_tracker.rb b/lib/instance_agent/plugins/codedeploy/deployment_command_tracker.rb new file mode 100644 index 00000000..cb423da4 --- /dev/null +++ b/lib/instance_agent/plugins/codedeploy/deployment_command_tracker.rb @@ -0,0 +1,62 @@ +require 'socket' +require 'concurrent' +require 'pathname' +require 'instance_metadata' +require 'instance_agent/agent/base' +require 'fileutils' +require 'instance_agent/log' + +module InstanceAgent + module Plugins + module CodeDeployPlugin + class FileDoesntExistException < Exception; end + class DeploymentCommandTracker + DEPLOYMENT_EVENT_FILE_STALE_TIMELIMIT_SECONDS = 86400 # 24 hour limit in secounds + + def self.create_ongoing_deployment_tracking_file(deployment_id) + FileUtils.mkdir_p(deployment_dir_path()) + FileUtils.touch(deployment_event_tracking_file_path(deployment_id)); + end + + def self.delete_deployment_tracking_file_if_stale?(deployment_id, timeout) + if(Time.now - File.ctime(deployment_event_tracking_file_path(deployment_id)) > timeout) + delete_deployment_command_tracking_file(deployment_id) + return true; + end + return false; + end + + def self.check_deployment_event_inprogress? + directories_and_files_inside(deployment_dir_path()).any?{|deployment_id| check_deployment_tracking_file_exist?(deployment_id)} + end + + def self.delete_deployment_command_tracking_file(deployment_id) + ongoing_deployment_event_file_path = deployment_event_tracking_file_path(deployment_id) + if File.exists?ongoing_deployment_event_file_path + File.delete(ongoing_deployment_event_file_path); + else + InstanceAgent::Log.warn("the tracking file does not exist") + end + end + + def self.directories_and_files_inside(directory) + Dir.entries(directory) - %w(.. .) + end + + private + def self.deployment_dir_path + File.join(InstanceAgent::Config.config[:root_dir], InstanceAgent::Config.config[:ongoing_deployment_tracking]) + end + + def self.check_deployment_tracking_file_exist?(deployment_id) + File.exists?(deployment_event_tracking_file_path(deployment_id)) && !delete_deployment_tracking_file_if_stale?(deployment_id, + DEPLOYMENT_EVENT_FILE_STALE_TIMELIMIT_SECONDS) + end + + def self.deployment_event_tracking_file_path(deployment_id) + ongoing_deployment_file_path = File.join(deployment_dir_path(), deployment_id) + end + end + end + end +end \ No newline at end of file diff --git a/lib/instance_agent/plugins/codedeploy/deployment_specification.rb b/lib/instance_agent/plugins/codedeploy/deployment_specification.rb index 29c4c145..3d0ecab2 100644 --- a/lib/instance_agent/plugins/codedeploy/deployment_specification.rb +++ b/lib/instance_agent/plugins/codedeploy/deployment_specification.rb @@ -97,7 +97,9 @@ def initialize(data) raise 'Exactly one of S3Revision, GitHubRevision, or LocalRevision must be specified' end end - + # Decrypts the envelope /deployment specs + # Params: + # envelope: deployment specification thats to be cheked and decrypted def self.parse(envelope) raise 'Provided deployment spec was nil' if envelope.nil? diff --git a/lib/instance_agent/runner/master.rb b/lib/instance_agent/runner/master.rb index a72ddef7..0255a71c 100644 --- a/lib/instance_agent/runner/master.rb +++ b/lib/instance_agent/runner/master.rb @@ -1,12 +1,14 @@ # encoding: UTF-8 require 'process_manager/master' require 'instance_metadata' +require 'instance_agent/plugins/codedeploy/deployment_command_tracker' module InstanceAgent module Runner - class Master < ProcessManager::Daemon::Master + class DeploymentAlreadyInProgressException < Exception; end - ChildTerminationMaxWaitTime = 80 + class Master < ProcessManager::Daemon::Master + ChildTerminationMaxWaitTime = 3600 #timeout of an hour def self.description(pid = $$) "master #{pid}" @@ -30,9 +32,16 @@ def self.pid_file def stop if (pid = self.class.find_pid) - puts "Stopping #{description(pid)}" - ProcessManager::Log.info("Stopping #{description(pid)}") + puts "Checking first if a deployment is already in progress" + ProcessManager::Log.info("Checking first if any deployment lifecycle event is in progress #{description(pid)}") begin + if(DeploymentCommandTracker.check_deployment_event_inprogress?) + ProcessManager::Log.info("Master process (#{pid}) will not be shut down right now, as a deployment is already in progress") + raise "A deployment is already in Progress",DeploymentAlreadyInProgressException + else + puts "Stopping #{description(pid)}" + ProcessManager::Log.info("Stopping #{description(pid)}") + end Process.kill('TERM', pid) rescue Errno::ESRCH end diff --git a/spec/aws/codedeploy/plugins/deployment_command_tracker_spec.rb b/spec/aws/codedeploy/plugins/deployment_command_tracker_spec.rb new file mode 100644 index 00000000..40ad9258 --- /dev/null +++ b/spec/aws/codedeploy/plugins/deployment_command_tracker_spec.rb @@ -0,0 +1,61 @@ +require 'spec_helper' +require 'instance_agent' +require 'instance_agent/config' +require 'instance_agent/plugins/codedeploy/deployment_command_tracker' +require 'instance_agent/log' + +describe InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker do + describe '.create_ongoing_deployment_tracking_file' do + $deployment_id = 'D-123' + deployment_command_tracker = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker; + context "when the deploymenmt life cycle event is in progress" do + before do + InstanceAgent::Config.config[:root_dir] = File.join(Dir.tmpdir(), 'codeDeploytest') + InstanceAgent::Config.config[:ongoing_deployment_tracking] = 'ongoing-deployment' + InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.create_ongoing_deployment_tracking_file($deployment_id) + end + it 'tries to create ongoing-deployment folder' do + directories_in_deployment_root_folder = deployment_command_tracker.directories_and_files_inside(InstanceAgent::Config.config[:root_dir]); + expect(directories_in_deployment_root_folder).to include(InstanceAgent::Config.config[:ongoing_deployment_tracking]); + end + it 'creates ongoing-deployment file in the tracking folder' do + files_in_deployment_tracking_folder = deployment_command_tracker.directories_and_files_inside(File.join(InstanceAgent::Config.config[:root_dir], InstanceAgent::Config.config[:ongoing_deployment_tracking])) + expect(files_in_deployment_tracking_folder).to include($deployment_id); + end + end + end + describe '.check_deployment_event_inprogress' do + context 'when no deployment life cycle event is in progress' do + before do + InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.delete_deployment_command_tracking_file($deployment_id) + end + it 'checks if any deployment event is in progress' do + expect(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.check_deployment_event_inprogress?).to equal(false); + end + end + context 'when deployment life cycle event is in progress' do + before do + InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.create_ongoing_deployment_tracking_file($deployment_id) + end + it 'checks if any deployment life cycle event is in progress ' do + expect(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.check_deployment_event_inprogress?).to equal(true) + end + end + end + describe '.delete_deployment_tracking_file_if_stale' do + context 'when deployment life cycle event is in progress' do + before do + InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.create_ongoing_deployment_tracking_file($deployment_id) + end + it 'checks if the file is stale or not' do + expect(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.delete_deployment_tracking_file_if_stale?($deployment_id, 2000)).to equal(false) + end + end + context 'when the wait-time has been more than the timeout time' do + it 'checks if the file is stale after the timeout' do + sleep 10 + expect(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.delete_deployment_tracking_file_if_stale?($deployment_id, 5)).to equal(true) + end + end + end +end \ No newline at end of file diff --git a/test/instance_agent/config_test.rb b/test/instance_agent/config_test.rb index 2674cd8e..de2e465d 100644 --- a/test/instance_agent/config_test.rb +++ b/test/instance_agent/config_test.rb @@ -28,6 +28,7 @@ class InstanceAgentConfigTest < InstanceAgentTestCase :wait_after_error => 30, :codedeploy_test_profile => 'prod', :on_premises_config_file => '/etc/codedeploy-agent/conf/codedeploy.onpremises.yml', + :ongoing_deployment_tracking => 'ongoing-deployment', :proxy_uri => nil, :enable_deployments_log => true }, InstanceAgent::Config.config) diff --git a/test/instance_agent/plugins/codedeploy/command_poller_test.rb b/test/instance_agent/plugins/codedeploy/command_poller_test.rb index dc99051c..265eaf09 100644 --- a/test/instance_agent/plugins/codedeploy/command_poller_test.rb +++ b/test/instance_agent/plugins/codedeploy/command_poller_test.rb @@ -1,8 +1,10 @@ require 'test_helper' require 'json' +require 'instance_agent/log' -class CommandPollerTest < InstanceAgentTestCase +class CommandPollerTest < InstanceAgentTestCase + include InstanceAgent::Plugins::CodeDeployPlugin def gather_diagnostics_from_error(error) {'error_code' => InstanceAgent::Plugins::CodeDeployPlugin::ScriptError::UNKNOWN_ERROR_CODE, 'script_name' => "", 'message' => error.message, 'log' => ""}.to_json end @@ -110,6 +112,12 @@ def gather_diagnostics(script_output) starts_as('setup') @deploy_control_client.stubs(:put_host_command_complete). when(@put_host_command_complete_state.is('setup')) + @deployment_id = stub(:deployment_id => "D-1234") + InstanceAgent::Config.config[:root_dir] = File.join(Dir.tmpdir(), "CodeDeploy") + InstanceAgent::Config.config[:ongoing_deployment_tracking] = "ongoing-deployment" + InstanceAgent::Plugins::CodeDeployPlugin::DeploymentSpecification.stubs(:parse).returns(@deployment_id) + InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.stubs(:delete_deployment_command_tracking_file).returns(true) + InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.stubs(:create_ongoing_deployment_tracking_file).returns(true) end should 'call PollHostCommand with the current host name' do @@ -410,8 +418,7 @@ def gather_diagnostics(script_output) should 'allow exceptions from execute_command to propagate to caller' do @executor.expects(:execute_command). - raises("some error") - + raises("some error") @deploy_control_client.expects(:put_host_command_complete). with(:command_status => "Failed", :diagnostics => {:format => "JSON", :payload => gather_diagnostics_from_error(RuntimeError.new("some error"))}, From 96cb087bcbc788d9f77bc6cd5980a07f0fe9fe88 Mon Sep 17 00:00:00 2001 From: Katyal Date: Thu, 18 Jan 2018 18:01:23 -0800 Subject: [PATCH 06/17] Integration test fixes, added a check for ongoing deployment folder's existence cr https://code.amazon.com/reviews/CR-1247966 --- .../plugins/codedeploy/deployment_command_tracker.rb | 6 +++++- lib/instance_agent/runner/master.rb | 2 +- .../plugins/deployment_command_tracker_spec.rb | 12 ++++++++++-- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/instance_agent/plugins/codedeploy/deployment_command_tracker.rb b/lib/instance_agent/plugins/codedeploy/deployment_command_tracker.rb index cb423da4..7e2df86c 100644 --- a/lib/instance_agent/plugins/codedeploy/deployment_command_tracker.rb +++ b/lib/instance_agent/plugins/codedeploy/deployment_command_tracker.rb @@ -27,7 +27,11 @@ def self.delete_deployment_tracking_file_if_stale?(deployment_id, timeout) end def self.check_deployment_event_inprogress? - directories_and_files_inside(deployment_dir_path()).any?{|deployment_id| check_deployment_tracking_file_exist?(deployment_id)} + if(File.exists?deployment_dir_path()) + return directories_and_files_inside(deployment_dir_path()).any?{|deployment_id| check_deployment_tracking_file_exist?(deployment_id)} + else + return false + end end def self.delete_deployment_command_tracking_file(deployment_id) diff --git a/lib/instance_agent/runner/master.rb b/lib/instance_agent/runner/master.rb index 0255a71c..2791987e 100644 --- a/lib/instance_agent/runner/master.rb +++ b/lib/instance_agent/runner/master.rb @@ -35,7 +35,7 @@ def stop puts "Checking first if a deployment is already in progress" ProcessManager::Log.info("Checking first if any deployment lifecycle event is in progress #{description(pid)}") begin - if(DeploymentCommandTracker.check_deployment_event_inprogress?) + if(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.check_deployment_event_inprogress?) ProcessManager::Log.info("Master process (#{pid}) will not be shut down right now, as a deployment is already in progress") raise "A deployment is already in Progress",DeploymentAlreadyInProgressException else diff --git a/spec/aws/codedeploy/plugins/deployment_command_tracker_spec.rb b/spec/aws/codedeploy/plugins/deployment_command_tracker_spec.rb index 40ad9258..d602194d 100644 --- a/spec/aws/codedeploy/plugins/deployment_command_tracker_spec.rb +++ b/spec/aws/codedeploy/plugins/deployment_command_tracker_spec.rb @@ -8,7 +8,7 @@ describe '.create_ongoing_deployment_tracking_file' do $deployment_id = 'D-123' deployment_command_tracker = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker; - context "when the deploymenmt life cycle event is in progress" do + context "when the deployment life cycle event is in progress" do before do InstanceAgent::Config.config[:root_dir] = File.join(Dir.tmpdir(), 'codeDeploytest') InstanceAgent::Config.config[:ongoing_deployment_tracking] = 'ongoing-deployment' @@ -40,7 +40,15 @@ it 'checks if any deployment life cycle event is in progress ' do expect(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.check_deployment_event_inprogress?).to equal(true) end - end + end + context 'when the agent starts for the first time' do + before do + FileUtils.rm_r(File.join(InstanceAgent::Config.config[:root_dir], InstanceAgent::Config.config[:ongoing_deployment_tracking])) + end + it 'checks if any deployment life cycle event is in progress ' do + expect(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.check_deployment_event_inprogress?).to equal(false) + end + end end describe '.delete_deployment_tracking_file_if_stale' do context 'when deployment life cycle event is in progress' do From 634a958379f5745490be2232959c2b678707a643 Mon Sep 17 00:00:00 2001 From: Katyal Date: Mon, 22 Jan 2018 13:56:08 -0800 Subject: [PATCH 07/17] revert event tracking changes cr https://code.amazon.com/reviews/CR-1260836 --- .../codedeploy_local_steps.rb | 6 +- features/step_definitions/common_steps.rb | 23 ++++--- lib/instance_agent/config.rb | 1 - .../plugins/codedeploy/command_poller.rb | 11 +-- .../codedeploy/deployment_command_tracker.rb | 66 ------------------ .../codedeploy/deployment_specification.rb | 4 +- lib/instance_agent/runner/master.rb | 17 ++--- .../deployment_command_tracker_spec.rb | 69 ------------------- test/instance_agent/config_test.rb | 1 - .../plugins/codedeploy/command_poller_test.rb | 13 +--- 10 files changed, 27 insertions(+), 184 deletions(-) delete mode 100644 lib/instance_agent/plugins/codedeploy/deployment_command_tracker.rb delete mode 100644 spec/aws/codedeploy/plugins/deployment_command_tracker_spec.rb diff --git a/features/step_definitions/codedeploy_local_steps.rb b/features/step_definitions/codedeploy_local_steps.rb index 1d236c05..2f1b2182 100644 --- a/features/step_definitions/codedeploy_local_steps.rb +++ b/features/step_definitions/codedeploy_local_steps.rb @@ -60,7 +60,7 @@ def tar_app_bundle(temp_directory_to_create_bundle) #Unfortunately Minitar will keep pack all the file paths as given, so unless you change directories into the location where you want to pack the files the bundle won't have the correct files and folders Dir.chdir @bundle_original_directory_location - File.open(tar_file_name, 'wb') { |tar| Minitar.pack(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside(@bundle_original_directory_location), tar) } + File.open(tar_file_name, 'wb') { |tar| Minitar.pack(directories_and_files_inside(@bundle_original_directory_location), tar) } Dir.chdir old_direcory tar_file_name @@ -102,7 +102,7 @@ def tgz_app_bundle(temp_directory_to_create_bundle) File.open(tgz_file_name, 'wb') do |file| Zlib::GzipWriter.wrap(file) do |gz| - Minitar.pack(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside(@bundle_original_directory_location), gz) + Minitar.pack(directories_and_files_inside(@bundle_original_directory_location), gz) end end @@ -149,7 +149,7 @@ def create_local_deployment(custom_events = nil, file_exists_behavior = nil) end Then(/^the expected files should have have been locally deployed to my host(| twice)$/) do |maybe_twice| - deployment_ids = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{LOCAL_DEPLOYMENT_GROUP_ID}") + deployment_ids = directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{LOCAL_DEPLOYMENT_GROUP_ID}") step "the expected files in directory #{bundle_original_directory_location}/scripts should have have been deployed#{maybe_twice} to my host during deployment with deployment group id #{LOCAL_DEPLOYMENT_GROUP_ID} and deployment ids #{deployment_ids.join(' ')}" end diff --git a/features/step_definitions/common_steps.rb b/features/step_definitions/common_steps.rb index dda7a9e7..50c11ac8 100644 --- a/features/step_definitions/common_steps.rb +++ b/features/step_definitions/common_steps.rb @@ -34,7 +34,7 @@ def zip_app_bundle(temp_directory_to_create_bundle) end def zip_directory(input_dir, output_file) - entries = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside(input_dir) + entries = directories_and_files_inside(input_dir) zip_io = Zip::File.open(output_file, Zip::File::CREATE) write_zip_entries(entries, '', input_dir, zip_io) @@ -47,7 +47,7 @@ def write_zip_entries(entries, path, input_dir, zip_io) diskFilePath = File.join(input_dir, zipFilePath) if File.directory?(diskFilePath) zip_io.mkdir(zipFilePath) - folder_entries = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside(diskFilePath) + folder_entries = directories_and_files_inside(diskFilePath) write_zip_entries(folder_entries, zipFilePath, input_dir, zip_io) else zip_io.get_output_stream(zipFilePath){ |f| f.write(File.open(diskFilePath, "rb").read())} @@ -55,33 +55,36 @@ def write_zip_entries(entries, path, input_dir, zip_io) end end +def directories_and_files_inside(directory) + Dir.entries(directory) - %w(.. .) +end Then(/^the expected files in directory (\S+) should have have been deployed(| twice) to my host during deployment with deployment group id (\S+) and deployment ids (.+)$/) do |expected_scripts_directory, maybe_twice, deployment_group_id, deployment_ids_space_separated| deployment_ids = deployment_ids_space_separated.split(' ') - directories_in_deployment_root_folder = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside(InstanceAgent::Config.config[:root_dir]) - expect(directories_in_deployment_root_folder.size).to be >= 3 + directories_in_deployment_root_folder = directories_and_files_inside(InstanceAgent::Config.config[:root_dir]) + expect(directories_in_deployment_root_folder.size).to eq(3) #ordering of the directories depends on the deployment group id, so using include instead of eq expect(directories_in_deployment_root_folder).to include(*%W(deployment-instructions deployment-logs #{deployment_group_id})) - files_in_deployment_logs_folder = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/deployment-logs") + files_in_deployment_logs_folder = directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/deployment-logs") expect(files_in_deployment_logs_folder.size).to eq(1) expect(files_in_deployment_logs_folder).to eq(%w(codedeploy-agent-deployments.log)) - directories_in_deployment_group_id_folder = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}") + directories_in_deployment_group_id_folder = directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}") expect(directories_in_deployment_group_id_folder.size).to eq(maybe_twice.empty? ? 1 : 2) expect(directories_in_deployment_group_id_folder).to eq(deployment_ids) deployment_id = deployment_ids.first - files_and_directories_in_deployment_id_folder = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}/#{deployment_id}") + files_and_directories_in_deployment_id_folder = directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}/#{deployment_id}") expect(files_and_directories_in_deployment_id_folder).to include(*%w(logs deployment-archive)) - files_and_directories_in_deployment_archive_folder = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}/#{deployment_id}/deployment-archive") + files_and_directories_in_deployment_archive_folder = directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}/#{deployment_id}/deployment-archive") expect(files_and_directories_in_deployment_archive_folder.size).to eq(2) expect(files_and_directories_in_deployment_archive_folder).to include(*%w(appspec.yml scripts)) - files_in_scripts_folder = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}/#{deployment_id}/deployment-archive/scripts") - sample_app_bundle_script_files = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside(expected_scripts_directory) + files_in_scripts_folder = directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}/#{deployment_id}/deployment-archive/scripts") + sample_app_bundle_script_files = directories_and_files_inside(expected_scripts_directory) expect(files_in_scripts_folder.size).to eq(sample_app_bundle_script_files.size) expect(files_in_scripts_folder).to include(*sample_app_bundle_script_files) end diff --git a/lib/instance_agent/config.rb b/lib/instance_agent/config.rb index ee2d6d74..13319712 100644 --- a/lib/instance_agent/config.rb +++ b/lib/instance_agent/config.rb @@ -23,7 +23,6 @@ def initialize :pid_dir => nil, :shared_dir => nil, :user => nil, - :ongoing_deployment_tracking => 'ongoing-deployment', :children => 1, :http_read_timeout => 80, :instance_service_region => nil, diff --git a/lib/instance_agent/plugins/codedeploy/command_poller.rb b/lib/instance_agent/plugins/codedeploy/command_poller.rb index 118b2e05..85dedcec 100644 --- a/lib/instance_agent/plugins/codedeploy/command_poller.rb +++ b/lib/instance_agent/plugins/codedeploy/command_poller.rb @@ -1,9 +1,8 @@ require 'socket' require 'concurrent' -require 'pathname' + require 'instance_metadata' require 'instance_agent/agent/base' -require_relative 'deployment_command_tracker' module InstanceAgent module Plugins @@ -24,7 +23,7 @@ class CommandPoller < InstanceAgent::Agent::Base "AfterAllowTraffic"=>["AfterAllowTraffic"], "ValidateService"=>["ValidateService"]} - WAIT_FOR_ALL_THREADS_TIMEOUT_SECONDS = 3600 # One hour timeout in seconds + WAIT_FOR_ALL_THREADS_TIMEOUT_SECONDS = 60 * 60 # One hour timeout in seconds def initialize test_profile = InstanceAgent::Config.config[:codedeploy_test_profile] @@ -142,11 +141,9 @@ def get_deployment_specification(command) def process_command(command, spec) log(:debug, "Calling #{@plugin.to_s}.execute_command") begin - deployment_id = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentSpecification.parse(spec).deployment_id - DeploymentCommandTracker.create_ongoing_deployment_tracking_file(deployment_id) #Successful commands will complete without raising an exception @plugin.execute_command(command, spec) - + log(:debug, 'Calling PutHostCommandComplete: "Succeeded"') @deploy_control_client.put_host_command_complete( :command_status => 'Succeeded', @@ -169,8 +166,6 @@ def process_command(command, spec) :host_command_identifier => command.host_command_identifier) log(:error, "Error during perform: #{e.class} - #{e.message} - #{e.backtrace.join("\n")}") raise e - ensure - DeploymentCommandTracker.delete_deployment_command_tracking_file(deployment_id) end end diff --git a/lib/instance_agent/plugins/codedeploy/deployment_command_tracker.rb b/lib/instance_agent/plugins/codedeploy/deployment_command_tracker.rb deleted file mode 100644 index 7e2df86c..00000000 --- a/lib/instance_agent/plugins/codedeploy/deployment_command_tracker.rb +++ /dev/null @@ -1,66 +0,0 @@ -require 'socket' -require 'concurrent' -require 'pathname' -require 'instance_metadata' -require 'instance_agent/agent/base' -require 'fileutils' -require 'instance_agent/log' - -module InstanceAgent - module Plugins - module CodeDeployPlugin - class FileDoesntExistException < Exception; end - class DeploymentCommandTracker - DEPLOYMENT_EVENT_FILE_STALE_TIMELIMIT_SECONDS = 86400 # 24 hour limit in secounds - - def self.create_ongoing_deployment_tracking_file(deployment_id) - FileUtils.mkdir_p(deployment_dir_path()) - FileUtils.touch(deployment_event_tracking_file_path(deployment_id)); - end - - def self.delete_deployment_tracking_file_if_stale?(deployment_id, timeout) - if(Time.now - File.ctime(deployment_event_tracking_file_path(deployment_id)) > timeout) - delete_deployment_command_tracking_file(deployment_id) - return true; - end - return false; - end - - def self.check_deployment_event_inprogress? - if(File.exists?deployment_dir_path()) - return directories_and_files_inside(deployment_dir_path()).any?{|deployment_id| check_deployment_tracking_file_exist?(deployment_id)} - else - return false - end - end - - def self.delete_deployment_command_tracking_file(deployment_id) - ongoing_deployment_event_file_path = deployment_event_tracking_file_path(deployment_id) - if File.exists?ongoing_deployment_event_file_path - File.delete(ongoing_deployment_event_file_path); - else - InstanceAgent::Log.warn("the tracking file does not exist") - end - end - - def self.directories_and_files_inside(directory) - Dir.entries(directory) - %w(.. .) - end - - private - def self.deployment_dir_path - File.join(InstanceAgent::Config.config[:root_dir], InstanceAgent::Config.config[:ongoing_deployment_tracking]) - end - - def self.check_deployment_tracking_file_exist?(deployment_id) - File.exists?(deployment_event_tracking_file_path(deployment_id)) && !delete_deployment_tracking_file_if_stale?(deployment_id, - DEPLOYMENT_EVENT_FILE_STALE_TIMELIMIT_SECONDS) - end - - def self.deployment_event_tracking_file_path(deployment_id) - ongoing_deployment_file_path = File.join(deployment_dir_path(), deployment_id) - end - end - end - end -end \ No newline at end of file diff --git a/lib/instance_agent/plugins/codedeploy/deployment_specification.rb b/lib/instance_agent/plugins/codedeploy/deployment_specification.rb index 3d0ecab2..29c4c145 100644 --- a/lib/instance_agent/plugins/codedeploy/deployment_specification.rb +++ b/lib/instance_agent/plugins/codedeploy/deployment_specification.rb @@ -97,9 +97,7 @@ def initialize(data) raise 'Exactly one of S3Revision, GitHubRevision, or LocalRevision must be specified' end end - # Decrypts the envelope /deployment specs - # Params: - # envelope: deployment specification thats to be cheked and decrypted + def self.parse(envelope) raise 'Provided deployment spec was nil' if envelope.nil? diff --git a/lib/instance_agent/runner/master.rb b/lib/instance_agent/runner/master.rb index 2791987e..a72ddef7 100644 --- a/lib/instance_agent/runner/master.rb +++ b/lib/instance_agent/runner/master.rb @@ -1,14 +1,12 @@ # encoding: UTF-8 require 'process_manager/master' require 'instance_metadata' -require 'instance_agent/plugins/codedeploy/deployment_command_tracker' module InstanceAgent module Runner - class DeploymentAlreadyInProgressException < Exception; end - class Master < ProcessManager::Daemon::Master - ChildTerminationMaxWaitTime = 3600 #timeout of an hour + + ChildTerminationMaxWaitTime = 80 def self.description(pid = $$) "master #{pid}" @@ -32,16 +30,9 @@ def self.pid_file def stop if (pid = self.class.find_pid) - puts "Checking first if a deployment is already in progress" - ProcessManager::Log.info("Checking first if any deployment lifecycle event is in progress #{description(pid)}") + puts "Stopping #{description(pid)}" + ProcessManager::Log.info("Stopping #{description(pid)}") begin - if(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.check_deployment_event_inprogress?) - ProcessManager::Log.info("Master process (#{pid}) will not be shut down right now, as a deployment is already in progress") - raise "A deployment is already in Progress",DeploymentAlreadyInProgressException - else - puts "Stopping #{description(pid)}" - ProcessManager::Log.info("Stopping #{description(pid)}") - end Process.kill('TERM', pid) rescue Errno::ESRCH end diff --git a/spec/aws/codedeploy/plugins/deployment_command_tracker_spec.rb b/spec/aws/codedeploy/plugins/deployment_command_tracker_spec.rb deleted file mode 100644 index d602194d..00000000 --- a/spec/aws/codedeploy/plugins/deployment_command_tracker_spec.rb +++ /dev/null @@ -1,69 +0,0 @@ -require 'spec_helper' -require 'instance_agent' -require 'instance_agent/config' -require 'instance_agent/plugins/codedeploy/deployment_command_tracker' -require 'instance_agent/log' - -describe InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker do - describe '.create_ongoing_deployment_tracking_file' do - $deployment_id = 'D-123' - deployment_command_tracker = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker; - context "when the deployment life cycle event is in progress" do - before do - InstanceAgent::Config.config[:root_dir] = File.join(Dir.tmpdir(), 'codeDeploytest') - InstanceAgent::Config.config[:ongoing_deployment_tracking] = 'ongoing-deployment' - InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.create_ongoing_deployment_tracking_file($deployment_id) - end - it 'tries to create ongoing-deployment folder' do - directories_in_deployment_root_folder = deployment_command_tracker.directories_and_files_inside(InstanceAgent::Config.config[:root_dir]); - expect(directories_in_deployment_root_folder).to include(InstanceAgent::Config.config[:ongoing_deployment_tracking]); - end - it 'creates ongoing-deployment file in the tracking folder' do - files_in_deployment_tracking_folder = deployment_command_tracker.directories_and_files_inside(File.join(InstanceAgent::Config.config[:root_dir], InstanceAgent::Config.config[:ongoing_deployment_tracking])) - expect(files_in_deployment_tracking_folder).to include($deployment_id); - end - end - end - describe '.check_deployment_event_inprogress' do - context 'when no deployment life cycle event is in progress' do - before do - InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.delete_deployment_command_tracking_file($deployment_id) - end - it 'checks if any deployment event is in progress' do - expect(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.check_deployment_event_inprogress?).to equal(false); - end - end - context 'when deployment life cycle event is in progress' do - before do - InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.create_ongoing_deployment_tracking_file($deployment_id) - end - it 'checks if any deployment life cycle event is in progress ' do - expect(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.check_deployment_event_inprogress?).to equal(true) - end - end - context 'when the agent starts for the first time' do - before do - FileUtils.rm_r(File.join(InstanceAgent::Config.config[:root_dir], InstanceAgent::Config.config[:ongoing_deployment_tracking])) - end - it 'checks if any deployment life cycle event is in progress ' do - expect(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.check_deployment_event_inprogress?).to equal(false) - end - end - end - describe '.delete_deployment_tracking_file_if_stale' do - context 'when deployment life cycle event is in progress' do - before do - InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.create_ongoing_deployment_tracking_file($deployment_id) - end - it 'checks if the file is stale or not' do - expect(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.delete_deployment_tracking_file_if_stale?($deployment_id, 2000)).to equal(false) - end - end - context 'when the wait-time has been more than the timeout time' do - it 'checks if the file is stale after the timeout' do - sleep 10 - expect(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.delete_deployment_tracking_file_if_stale?($deployment_id, 5)).to equal(true) - end - end - end -end \ No newline at end of file diff --git a/test/instance_agent/config_test.rb b/test/instance_agent/config_test.rb index de2e465d..2674cd8e 100644 --- a/test/instance_agent/config_test.rb +++ b/test/instance_agent/config_test.rb @@ -28,7 +28,6 @@ class InstanceAgentConfigTest < InstanceAgentTestCase :wait_after_error => 30, :codedeploy_test_profile => 'prod', :on_premises_config_file => '/etc/codedeploy-agent/conf/codedeploy.onpremises.yml', - :ongoing_deployment_tracking => 'ongoing-deployment', :proxy_uri => nil, :enable_deployments_log => true }, InstanceAgent::Config.config) diff --git a/test/instance_agent/plugins/codedeploy/command_poller_test.rb b/test/instance_agent/plugins/codedeploy/command_poller_test.rb index 265eaf09..dc99051c 100644 --- a/test/instance_agent/plugins/codedeploy/command_poller_test.rb +++ b/test/instance_agent/plugins/codedeploy/command_poller_test.rb @@ -1,10 +1,8 @@ require 'test_helper' require 'json' -require 'instance_agent/log' - class CommandPollerTest < InstanceAgentTestCase - include InstanceAgent::Plugins::CodeDeployPlugin + def gather_diagnostics_from_error(error) {'error_code' => InstanceAgent::Plugins::CodeDeployPlugin::ScriptError::UNKNOWN_ERROR_CODE, 'script_name' => "", 'message' => error.message, 'log' => ""}.to_json end @@ -112,12 +110,6 @@ def gather_diagnostics(script_output) starts_as('setup') @deploy_control_client.stubs(:put_host_command_complete). when(@put_host_command_complete_state.is('setup')) - @deployment_id = stub(:deployment_id => "D-1234") - InstanceAgent::Config.config[:root_dir] = File.join(Dir.tmpdir(), "CodeDeploy") - InstanceAgent::Config.config[:ongoing_deployment_tracking] = "ongoing-deployment" - InstanceAgent::Plugins::CodeDeployPlugin::DeploymentSpecification.stubs(:parse).returns(@deployment_id) - InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.stubs(:delete_deployment_command_tracking_file).returns(true) - InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.stubs(:create_ongoing_deployment_tracking_file).returns(true) end should 'call PollHostCommand with the current host name' do @@ -418,7 +410,8 @@ def gather_diagnostics(script_output) should 'allow exceptions from execute_command to propagate to caller' do @executor.expects(:execute_command). - raises("some error") + raises("some error") + @deploy_control_client.expects(:put_host_command_complete). with(:command_status => "Failed", :diagnostics => {:format => "JSON", :payload => gather_diagnostics_from_error(RuntimeError.new("some error"))}, From 82b06067724cdddf163495260a3f59b83d491414 Mon Sep 17 00:00:00 2001 From: Drake Tetreault Date: Tue, 16 Jan 2018 19:28:03 +0000 Subject: [PATCH 08/17] Disable certificate pinning. cr https://code.amazon.com/reviews/CR-1228125 --- .../plugins/codedeploy/codedeploy_control.rb | 19 --------- .../codedeploy/deployment_specification.rb | 39 +------------------ 2 files changed, 2 insertions(+), 56 deletions(-) diff --git a/lib/instance_agent/plugins/codedeploy/codedeploy_control.rb b/lib/instance_agent/plugins/codedeploy/codedeploy_control.rb index 26019d6d..c44d6e8b 100644 --- a/lib/instance_agent/plugins/codedeploy/codedeploy_control.rb +++ b/lib/instance_agent/plugins/codedeploy/codedeploy_control.rb @@ -80,27 +80,8 @@ def verify_cert client.proxy_pass = proxy_uri.password if proxy_uri.password end - client.verify_callback = lambda do |preverify_ok, cert_store| - return false unless preverify_ok - @cert = cert_store.chain[0] - verify_subject - end - response = client.get '/' end - - # Do minimal cert pinning - def verify_subject - InstanceAgent::Log.debug("#{self.class.to_s}: Actual certificate subject is '#{@cert.subject.to_s}'") - if "cn-north-1" == @region - @cert.subject.to_s == "/C=CN/ST=Beijing/L=Beijing/O=Amazon Connect Technology Services (Beijing) Co., Ltd./CN=codedeploy-commands."+@region+".amazonaws.com.cn" - elsif "cn-northwest-1" == @region - @cert.subject.to_s == "/C=CN/ST=Ningxia/L=Ningxia/O=Amazon Cloud Technology Services (Ningxia) Co., Ltd./CN=codedeploy-commands."+@region+".amazonaws.com.cn" - else - @cert.subject.to_s == "/C=US/ST=Washington/L=Seattle/O=Amazon.com, Inc./CN=codedeploy-commands."+@region+".amazonaws.com" - end - end - end end end diff --git a/lib/instance_agent/plugins/codedeploy/deployment_specification.rb b/lib/instance_agent/plugins/codedeploy/deployment_specification.rb index 29c4c145..970758ac 100644 --- a/lib/instance_agent/plugins/codedeploy/deployment_specification.rb +++ b/lib/instance_agent/plugins/codedeploy/deployment_specification.rb @@ -104,22 +104,8 @@ def self.parse(envelope) case envelope.format when "PKCS7/JSON" pkcs7 = OpenSSL::PKCS7.new(envelope.payload) - - # The PKCS7_NOCHAIN flag tells OpenSSL to ignore any PKCS7 CA chain that might be attached - # to the message directly and use the certificates from provided one only for validating the. - # signer's certificate. - # - # However, it will allow use the PKCS7 signer certificate provided to validate the signature. - # - # http://www.openssl.org/docs/crypto/PKCS7_verify.html#VERIFY_PROCESS - # - # The ruby wrapper returns true if OpenSSL returns 1 - raise "Validation of PKCS7 signed message failed" unless pkcs7.verify([], @cert_store, nil, OpenSSL::PKCS7::NOCHAIN) - - signer_certs = pkcs7.certificates - raise "Validation of PKCS7 signed message failed" unless signer_certs.size == 1 - raise "Validation of PKCS7 signed message failed" unless verify_pkcs7_signer_cert(signer_certs[0]) - + pkcs7.verify([], @cert_store, nil, OpenSSL::PKCS7::NOVERIFY) + # NOTE: the pkcs7.data field is only populated AFTER pkcs7.verify() is called! parse_deployment_spec_data(pkcs7.data) when "TEXT/JSON" raise "Unsupported DeploymentSpecification format: #{envelope.format}" unless AWS::CodeDeploy::Local::Deployer.running_as_developer_utility? @@ -172,32 +158,11 @@ def valid_local_bundle_type?(revision) revision.nil? || %w(tar zip tgz directory).any? { |k| revision["BundleType"] == k } end - def self.verify_pkcs7_signer_cert(cert) - @@region ||= ENV['AWS_REGION'] || InstanceMetadata.region - - # Do some minimal cert pinning - case InstanceAgent::Config.config()[:codedeploy_test_profile] - when 'beta', 'gamma' - cert.subject.to_s == "/C=US/ST=Washington/L=Seattle/O=Amazon.com, Inc./CN=codedeploy-signer-integ.amazonaws.com" - when 'prod' - if (@@region == "cn-north-1") - cert.subject.to_s == "/C=CN/ST=Beijing/L=Beijing/O=Amazon Connect Technology Services (Beijing) Co., Ltd./CN=codedeploy-signer-"+@@region+".amazonaws.com.cn" - elsif (@@region == "cn-northwest-1") - cert.subject.to_s == "/C=CN/ST=Ningxia/L=Ningxia/O=Amazon Cloud Technology Services (Ningxia) Co., Ltd./CN=codedeploy-signer-"+@@region+".amazonaws.com.cn" - else - cert.subject.to_s == "/C=US/ST=Washington/L=Seattle/O=Amazon.com, Inc./CN=codedeploy-signer-"+@@region+".amazonaws.com" - end - else - raise "Unknown profile '#{Config.config()[:codedeploy_test_profile]}'" - end - end - private def getDeploymentIdFromArn(arn) # example arn format: "arn:aws:codedeploy:us-east-1:123412341234:deployment/12341234-1234-1234-1234-123412341234" arn.split(":", 6)[5].split("/",2)[1] end - end end end From 965b18de2ae0ee7fd4226c0d066df06c4b6bdd0d Mon Sep 17 00:00:00 2001 From: Asaf Erlich Date: Sat, 20 Jan 2018 20:15:19 -0800 Subject: [PATCH 09/17] Created buildspec for CodeBuild to run rake && rake test-integration and other changes that had to be made to fix the tests to run on CodeBuild 1. Only try to upload to s3 bucket once (since both times are identical) - otherwise the permissions get messed up because of the assume role call 2. Not force permissions to be the shared file credentials so it can use the instance profile credentials. 3. Update the assume role policy since CodeBuild uses an assumed role so its role is a new one each time. cr https://code.amazon.com/reviews/CR-1255775 --- buildspec-agent-rake.yml | 19 +++++++++ features/step_definitions/agent_steps.rb | 13 ++++-- .../codedeploy_local_steps.rb | 4 -- features/step_definitions/common_steps.rb | 42 ++++++++++++------- 4 files changed, 55 insertions(+), 23 deletions(-) create mode 100644 buildspec-agent-rake.yml diff --git a/buildspec-agent-rake.yml b/buildspec-agent-rake.yml new file mode 100644 index 00000000..9eeb6d1f --- /dev/null +++ b/buildspec-agent-rake.yml @@ -0,0 +1,19 @@ +version: 0.2 + +phases: + install: + commands: + - echo Installing bundler + - gem install bundler + - echo Using bundler to install remaining gems + - bundle install + build: + commands: + - echo Build started on `date` + - echo Running rake - unit tests + - rake + post_build: + commands: + - echo Build completed on `date` + - echo Running rake test-integration - integ tests + - rake test-integration diff --git a/features/step_definitions/agent_steps.rb b/features/step_definitions/agent_steps.rb index 168aa8c6..ad59b820 100644 --- a/features/step_definitions/agent_steps.rb +++ b/features/step_definitions/agent_steps.rb @@ -46,9 +46,6 @@ def eventually(options = {}, &block) @working_directory = Dir.mktmpdir configure_local_agent(@working_directory) - #Reset aws credentials to default location - Aws.config[:credentials] = Aws::SharedCredentials.new.credentials - #instantiate these clients first so they use user's aws creds instead of assumed role creds @codedeploy_client = Aws::CodeDeploy::Client.new @iam_client = Aws::IAM::Client.new @@ -225,12 +222,20 @@ def create_instance_role rescue Aws::IAM::Errors::EntityAlreadyExists #Using the existing role end - eventually do + + instance_role_arn = eventually do instance_role = @iam_client.get_role({:role_name => INSTANCE_ROLE_NAME}).role expect(instance_role).not_to be_nil expect(instance_role.assume_role_policy_document).not_to be_nil instance_role.arn end + + @iam_client.update_assume_role_policy({ + policy_document: instance_role_policy, + role_name: INSTANCE_ROLE_NAME, + }) + + instance_role_arn end def instance_role_policy diff --git a/features/step_definitions/codedeploy_local_steps.rb b/features/step_definitions/codedeploy_local_steps.rb index 2f1b2182..edb1572e 100644 --- a/features/step_definitions/codedeploy_local_steps.rb +++ b/features/step_definitions/codedeploy_local_steps.rb @@ -14,10 +14,6 @@ Before("@codedeploy-local") do @test_directory = Dir.mktmpdir - - #Reset aws credentials to default location - Aws.config[:credentials] = Aws::SharedCredentials.new.credentials - configure_local_agent(@test_directory) end diff --git a/features/step_definitions/common_steps.rb b/features/step_definitions/common_steps.rb index 50c11ac8..078aa1ad 100644 --- a/features/step_definitions/common_steps.rb +++ b/features/step_definitions/common_steps.rb @@ -3,24 +3,36 @@ $:.unshift File.join(File.dirname(File.expand_path('../..', __FILE__)), 'features') require 'step_definitions/step_constants' +@bucket_creation_count = 0; Given(/^I have a sample bundle uploaded to s3$/) do - s3 = Aws::S3::Client.new - - begin - s3.create_bucket({ - bucket: StepConstants::APP_BUNDLE_BUCKET, # required - create_bucket_configuration: { - location_constraint: Aws.config[:region], - } - }) - rescue Aws::S3::Errors::BucketAlreadyOwnedByYou - #Already created the bucket - end +=begin +This fails if the s3 upload is attempted after assume_role is called in the first integration test. +This is because once you call assume role the next time it instantiates a client it is using different permissions. In my opinion thats a bug because it doesn't match the documentation for the AWS SDK. +https://docs.aws.amazon.com/sdkforruby/api/index.html + +Their documentation says an assumed role is the LAST permission it will try to rely on but it looks like its always the first. But the s3 upload is the only place that this mattered so I simply forced this code so it doesn't do it again since the bundle is identical for both tests. +=end + if @bucket_creation_count == 0 + s3 = Aws::S3::Client.new + + begin + s3.create_bucket({ + bucket: StepConstants::APP_BUNDLE_BUCKET, # required + create_bucket_configuration: { + location_constraint: Aws.config[:region], + } + }) + rescue Aws::S3::Errors::BucketAlreadyOwnedByYou + #Already created the bucket + end - Dir.mktmpdir do |temp_directory_to_create_zip_file| - File.open(zip_app_bundle(temp_directory_to_create_zip_file), 'rb') do |file| - s3.put_object(bucket: StepConstants::APP_BUNDLE_BUCKET, key: StepConstants::APP_BUNDLE_KEY, body: file) + Dir.mktmpdir do |temp_directory_to_create_zip_file| + File.open(zip_app_bundle(temp_directory_to_create_zip_file), 'rb') do |file| + s3.put_object(bucket: StepConstants::APP_BUNDLE_BUCKET, key: StepConstants::APP_BUNDLE_KEY, body: file) + end end + + @bucket_creation_count += 1 end @bundle_type = 'zip' From 3da1ae39d106801319bf1f3156c99c646d8d70f4 Mon Sep 17 00:00:00 2001 From: Asaf Erlich Date: Fri, 26 Jan 2018 14:36:32 -0800 Subject: [PATCH 10/17] Fixed issue with missing require during codedeploy-local invocation. See https://tt.amazon.com/0133019671 cr https://code.amazon.com/reviews/CR-1292436 --- .../application_specification/application_specification.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/instance_agent/plugins/codedeploy/application_specification/application_specification.rb b/lib/instance_agent/plugins/codedeploy/application_specification/application_specification.rb index 9811d0bc..519b3ed9 100644 --- a/lib/instance_agent/plugins/codedeploy/application_specification/application_specification.rb +++ b/lib/instance_agent/plugins/codedeploy/application_specification/application_specification.rb @@ -1,5 +1,7 @@ require 'instance_agent/plugins/codedeploy/application_specification/script_info' require 'instance_agent/plugins/codedeploy/application_specification/file_info' +require 'instance_agent/plugins/codedeploy/application_specification/linux_permission_info' +require 'instance_agent/plugins/codedeploy/application_specification/mode_info' module InstanceAgent module Plugins From 1ef74b581665f45c72bf850ae40991660c28135b Mon Sep 17 00:00:00 2001 From: Katyal Date: Wed, 31 Jan 2018 10:07:58 -0800 Subject: [PATCH 11/17] added changes for graceful shutdown for agent in linux cr https://code.amazon.com/reviews/CR-1313840 --- .../plugins/codedeploy/command_poller.rb | 4 ++-- lib/instance_agent/runner/child.rb | 23 +++++++++++++++++++ lib/instance_agent/runner/master.rb | 2 ++ lib/winagent.rb | 2 +- 4 files changed, 28 insertions(+), 3 deletions(-) diff --git a/lib/instance_agent/plugins/codedeploy/command_poller.rb b/lib/instance_agent/plugins/codedeploy/command_poller.rb index 85dedcec..461da0dc 100644 --- a/lib/instance_agent/plugins/codedeploy/command_poller.rb +++ b/lib/instance_agent/plugins/codedeploy/command_poller.rb @@ -89,13 +89,13 @@ def perform end def graceful_shutdown - log(:info, "Gracefully shutting down command execution threads now, will wait up to #{WAIT_FOR_ALL_THREADS_TIMEOUT_SECONDS} seconds") + log(:info, "Gracefully shutting down agent child threads now, will wait up to #{WAIT_FOR_ALL_THREADS_TIMEOUT_SECONDS} seconds") # tell the pool to shutdown in an orderly fashion, allowing in progress work to complete @thread_pool.shutdown # now wait for all work to complete, wait till the timeout value # TODO: Make the timeout configurable in the agent configuration @thread_pool.wait_for_termination WAIT_FOR_ALL_THREADS_TIMEOUT_SECONDS - log(:info, 'All command execution threads have been shut down') + log(:info, 'All agent child threads have been shut down') end def next_command diff --git a/lib/instance_agent/runner/child.rb b/lib/instance_agent/runner/child.rb index 7bf7f30e..ac96a293 100644 --- a/lib/instance_agent/runner/child.rb +++ b/lib/instance_agent/runner/child.rb @@ -1,5 +1,6 @@ # encoding: UTF-8 require 'process_manager/child' +require 'thread' module InstanceAgent module Runner @@ -38,6 +39,28 @@ def run runner.run end end + + # Stops the master after recieving the kill signal + # is overriden from ProcessManager::Daemon::Child + def stop + @runner.graceful_shutdown + ProcessManager::Log.info('agent exiting now') + super + end + + # Catches the trap signals and does a default or custom action + # is overriden from ProcessManager::Daemon::Child + def trap_signals + [:INT, :QUIT, :TERM].each do |sig| + trap(sig) do + ProcessManager::Log.info "#{description}: Received #{sig} - setting internal shutting down flag and possibly finishing last run" + stop_thread = Thread.new {stop} + stop_thread.join + end + end + # make sure we do not handle children like the master process + trap(:CHLD, 'DEFAULT') + end def description if runner diff --git a/lib/instance_agent/runner/master.rb b/lib/instance_agent/runner/master.rb index a72ddef7..6969bf05 100644 --- a/lib/instance_agent/runner/master.rb +++ b/lib/instance_agent/runner/master.rb @@ -28,6 +28,8 @@ def self.pid_file File.join(ProcessManager::Config.config[:pid_dir], "#{ProcessManager::Config.config[:program_name]}.pid") end + # Stops the master after recieving the kill signal + # is overriden from ProcessManager::Daemon::Master def stop if (pid = self.class.find_pid) puts "Stopping #{description(pid)}" diff --git a/lib/winagent.rb b/lib/winagent.rb index 7ee52d87..e737bf87 100644 --- a/lib/winagent.rb +++ b/lib/winagent.rb @@ -58,7 +58,7 @@ def service_stop log(:info, 'stopping the agent') @polling_mutex.synchronize do @runner.graceful_shutdown - log(:info, 'command execution threads shutdown, agent exiting now') + log(:info, 'agent exiting now') end end From 842b2c8d33e90fd8fcca2168b4925e0d6a4e3eb3 Mon Sep 17 00:00:00 2001 From: Katyal Date: Wed, 31 Jan 2018 16:49:38 -0800 Subject: [PATCH 12/17] increasing termination max wait time for child thread to 2 hours cr https://code.amazon.com/reviews/CR-1318207 --- lib/instance_agent/config.rb | 1 + lib/instance_agent/plugins/codedeploy/command_poller.rb | 6 ++---- lib/instance_agent/runner/master.rb | 6 ++---- test/instance_agent/config_test.rb | 3 ++- 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/instance_agent/config.rb b/lib/instance_agent/config.rb index 13319712..4a3f166f 100644 --- a/lib/instance_agent/config.rb +++ b/lib/instance_agent/config.rb @@ -31,6 +31,7 @@ def initialize :wait_between_runs => 30, :wait_after_error => 30, :codedeploy_test_profile => 'prod', + :kill_agent_max_wait_time_seconds => 7200, :on_premises_config_file => '/etc/codedeploy-agent/conf/codedeploy.onpremises.yml', :proxy_uri => nil, :enable_deployments_log => true diff --git a/lib/instance_agent/plugins/codedeploy/command_poller.rb b/lib/instance_agent/plugins/codedeploy/command_poller.rb index 461da0dc..cc3cc636 100644 --- a/lib/instance_agent/plugins/codedeploy/command_poller.rb +++ b/lib/instance_agent/plugins/codedeploy/command_poller.rb @@ -23,8 +23,6 @@ class CommandPoller < InstanceAgent::Agent::Base "AfterAllowTraffic"=>["AfterAllowTraffic"], "ValidateService"=>["ValidateService"]} - WAIT_FOR_ALL_THREADS_TIMEOUT_SECONDS = 60 * 60 # One hour timeout in seconds - def initialize test_profile = InstanceAgent::Config.config[:codedeploy_test_profile] unless ["beta", "gamma"].include?(test_profile.downcase) @@ -89,12 +87,12 @@ def perform end def graceful_shutdown - log(:info, "Gracefully shutting down agent child threads now, will wait up to #{WAIT_FOR_ALL_THREADS_TIMEOUT_SECONDS} seconds") + log(:info, "Gracefully shutting down agent child threads now, will wait up to #{ProcessManager::Config.config[:kill_agent_max_wait_time_seconds]} seconds") # tell the pool to shutdown in an orderly fashion, allowing in progress work to complete @thread_pool.shutdown # now wait for all work to complete, wait till the timeout value # TODO: Make the timeout configurable in the agent configuration - @thread_pool.wait_for_termination WAIT_FOR_ALL_THREADS_TIMEOUT_SECONDS + @thread_pool.wait_for_termination ProcessManager::Config.config[:kill_agent_max_wait_time_seconds] log(:info, 'All agent child threads have been shut down') end diff --git a/lib/instance_agent/runner/master.rb b/lib/instance_agent/runner/master.rb index 6969bf05..bfaa3e20 100644 --- a/lib/instance_agent/runner/master.rb +++ b/lib/instance_agent/runner/master.rb @@ -6,8 +6,6 @@ module InstanceAgent module Runner class Master < ProcessManager::Daemon::Master - ChildTerminationMaxWaitTime = 80 - def self.description(pid = $$) "master #{pid}" end @@ -40,7 +38,7 @@ def stop end begin - Timeout.timeout(ChildTerminationMaxWaitTime) do + Timeout.timeout(ProcessManager::Config.config[:kill_agent_max_wait_time_seconds]) do loop do begin Process.kill(0, pid) @@ -68,7 +66,7 @@ def kill_children(sig) end begin - Timeout.timeout(ChildTerminationMaxWaitTime) do + Timeout.timeout(ProcessManager::Config.config[:kill_agent_max_wait_time_seconds]) do children.each do |index, child_pid| begin Process.wait(child_pid) diff --git a/test/instance_agent/config_test.rb b/test/instance_agent/config_test.rb index 2674cd8e..74391340 100644 --- a/test/instance_agent/config_test.rb +++ b/test/instance_agent/config_test.rb @@ -29,7 +29,8 @@ class InstanceAgentConfigTest < InstanceAgentTestCase :codedeploy_test_profile => 'prod', :on_premises_config_file => '/etc/codedeploy-agent/conf/codedeploy.onpremises.yml', :proxy_uri => nil, - :enable_deployments_log => true + :enable_deployments_log => true, + :kill_agent_max_wait_time_seconds => 7200 }, InstanceAgent::Config.config) end From 8623c0527cfb5e9091724f6a24c65b1801ec1729 Mon Sep 17 00:00:00 2001 From: Asaf Erlich Date: Fri, 2 Feb 2018 17:25:59 -0800 Subject: [PATCH 13/17] Updating codebuild spec to include uploading an s3 artifact with the commit hash (only available as an environment variable at build time) - this will let TOD check if a build succeeded cr https://code.amazon.com/reviews/CR-1334262 --- buildspec-agent-rake.yml | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/buildspec-agent-rake.yml b/buildspec-agent-rake.yml index 9eeb6d1f..0b5d566f 100644 --- a/buildspec-agent-rake.yml +++ b/buildspec-agent-rake.yml @@ -7,13 +7,19 @@ phases: - gem install bundler - echo Using bundler to install remaining gems - bundle install - build: + pre_build: commands: - echo Build started on `date` - echo Running rake - unit tests - rake - post_build: + build: commands: - echo Build completed on `date` - echo Running rake test-integration - integ tests - rake test-integration + post_build: + commands: + - echo $CODEBUILD_SOURCE_VERSION > codebuild_source_commit_id +artifacts: + files: + - codebuild_source_commit_id From ef65652a2a34896cbb8ec4519e34cfbebfa38c8e Mon Sep 17 00:00:00 2001 From: Asaf Erlich Date: Wed, 7 Mar 2018 09:48:28 -0800 Subject: [PATCH 14/17] Wrap the acknowledgement of commands also into the actions taken by each thread so it never acknowledges a command without processing cr https://code.amazon.com/reviews/CR-1527032 --- .../plugins/codedeploy/command_poller.rb | 98 +++++++++++-------- .../plugins/codedeploy/command_poller_test.rb | 40 ++++---- 2 files changed, 74 insertions(+), 64 deletions(-) diff --git a/lib/instance_agent/plugins/codedeploy/command_poller.rb b/lib/instance_agent/plugins/codedeploy/command_poller.rb index cc3cc636..6c1c72a7 100644 --- a/lib/instance_agent/plugins/codedeploy/command_poller.rb +++ b/lib/instance_agent/plugins/codedeploy/command_poller.rb @@ -67,22 +67,14 @@ def validate def perform return unless command = next_command - return unless acknowledge_command(command) + #Commands will be executed on a separate thread. begin - spec = get_deployment_specification(command) - #Commands will be executed on a separate thread. @thread_pool.post { - process_command(command, spec) + acknowledge_and_process_command(command) } - #Commands that throw an exception will be considered to have failed - rescue Exception => e - log(:debug, 'Calling PutHostCommandComplete: "Code Error" ') - @deploy_control_client.put_host_command_complete( - :command_status => "Failed", - :diagnostics => {:format => "JSON", :payload => gather_diagnostics_from_error(e)}, - :host_command_identifier => command.host_command_identifier) - raise e + rescue Concurrent::RejectedExecutionError + log(:warn, 'Graceful shutdown initiated, skipping any further polling until agent restarts') end end @@ -96,6 +88,55 @@ def graceful_shutdown log(:info, 'All agent child threads have been shut down') end + def acknowledge_and_process_command(command) + return unless acknowledge_command(command) + + begin + spec = get_deployment_specification(command) + process_command(command, spec) + #Commands that throw an exception will be considered to have failed + rescue Exception => e + log(:warn, 'Calling PutHostCommandComplete: "Code Error" ') + @deploy_control_client.put_host_command_complete( + :command_status => "Failed", + :diagnostics => {:format => "JSON", :payload => gather_diagnostics_from_error(e)}, + :host_command_identifier => command.host_command_identifier) + raise e + end + end + + def process_command(command, spec) + log(:debug, "Calling #{@plugin.to_s}.execute_command") + begin + #Successful commands will complete without raising an exception + @plugin.execute_command(command, spec) + + log(:debug, 'Calling PutHostCommandComplete: "Succeeded"') + @deploy_control_client.put_host_command_complete( + :command_status => 'Succeeded', + :diagnostics => {:format => "JSON", :payload => gather_diagnostics()}, + :host_command_identifier => command.host_command_identifier) + #Commands that throw an exception will be considered to have failed + rescue ScriptError => e + log(:debug, 'Calling PutHostCommandComplete: "Code Error" ') + @deploy_control_client.put_host_command_complete( + :command_status => "Failed", + :diagnostics => {:format => "JSON", :payload => gather_diagnostics_from_script_error(e)}, + :host_command_identifier => command.host_command_identifier) + log(:error, "Error during perform: #{e.class} - #{e.message} - #{e.backtrace.join("\n")}") + raise e + rescue Exception => e + log(:debug, 'Calling PutHostCommandComplete: "Code Error" ') + @deploy_control_client.put_host_command_complete( + :command_status => "Failed", + :diagnostics => {:format => "JSON", :payload => gather_diagnostics_from_error(e)}, + :host_command_identifier => command.host_command_identifier) + log(:error, "Error during perform: #{e.class} - #{e.message} - #{e.backtrace.join("\n")}") + raise e + end + end + + private def next_command log(:debug, "Calling PollHostCommand:") output = @deploy_control_client.poll_host_command(:host_identifier => @host_identifier) @@ -114,6 +155,7 @@ def next_command command end + private def acknowledge_command(command) log(:debug, "Calling PutHostCommandAcknowledgement:") output = @deploy_control_client.put_host_command_acknowledgement( @@ -124,6 +166,7 @@ def acknowledge_command(command) true unless status == "Succeeded" || status == "Failed" end + private def get_deployment_specification(command) log(:debug, "Calling GetDeploymentSpecification:") output = @deploy_control_client.get_deployment_specification( @@ -136,37 +179,6 @@ def get_deployment_specification(command) output.deployment_specification.generic_envelope end - def process_command(command, spec) - log(:debug, "Calling #{@plugin.to_s}.execute_command") - begin - #Successful commands will complete without raising an exception - @plugin.execute_command(command, spec) - - log(:debug, 'Calling PutHostCommandComplete: "Succeeded"') - @deploy_control_client.put_host_command_complete( - :command_status => 'Succeeded', - :diagnostics => {:format => "JSON", :payload => gather_diagnostics()}, - :host_command_identifier => command.host_command_identifier) - #Commands that throw an exception will be considered to have failed - rescue ScriptError => e - log(:debug, 'Calling PutHostCommandComplete: "Code Error" ') - @deploy_control_client.put_host_command_complete( - :command_status => "Failed", - :diagnostics => {:format => "JSON", :payload => gather_diagnostics_from_script_error(e)}, - :host_command_identifier => command.host_command_identifier) - log(:error, "Error during perform: #{e.class} - #{e.message} - #{e.backtrace.join("\n")}") - raise e - rescue Exception => e - log(:debug, 'Calling PutHostCommandComplete: "Code Error" ') - @deploy_control_client.put_host_command_complete( - :command_status => "Failed", - :diagnostics => {:format => "JSON", :payload => gather_diagnostics_from_error(e)}, - :host_command_identifier => command.host_command_identifier) - log(:error, "Error during perform: #{e.class} - #{e.message} - #{e.backtrace.join("\n")}") - raise e - end - end - private def gather_diagnostics_from_script_error(script_error) return script_error.to_json diff --git a/test/instance_agent/plugins/codedeploy/command_poller_test.rb b/test/instance_agent/plugins/codedeploy/command_poller_test.rb index dc99051c..b7c515a5 100644 --- a/test/instance_agent/plugins/codedeploy/command_poller_test.rb +++ b/test/instance_agent/plugins/codedeploy/command_poller_test.rb @@ -116,7 +116,6 @@ def gather_diagnostics(script_output) @deploy_control_client.expects(:poll_host_command). with(:host_identifier => @host_identifier). returns(@poll_host_command_output) - Thread.expects(:new).returns(nil) @poller.perform end @@ -179,7 +178,6 @@ def gather_diagnostics(script_output) @deploy_control_client.expects(:poll_host_command). with(:host_identifier => @host_identifier). returns(poll_host_command_output) - Thread.expects(:new).returns(nil) @poller.perform end @@ -254,9 +252,8 @@ def gather_diagnostics(script_output) with(:diagnostics => nil, :host_command_identifier => @command.host_command_identifier). returns(@poll_host_command_acknowledgement_output) - Thread.expects(:new).returns(nil) - @poller.perform + @poller.acknowledge_and_process_command(@command) end should 'return when Succeeded command status is given by PollHostCommandAcknowledgement' do @@ -272,7 +269,7 @@ def gather_diagnostics(script_output) @deploy_control_client.expects(:put_host_command_complete).never. when(@put_host_command_complete_state.is('never')) - @poller.perform + @poller.acknowledge_and_process_command(@command) end should 'return when Failed command status is given by PollHostCommandAcknowledgement' do @@ -288,7 +285,7 @@ def gather_diagnostics(script_output) @deploy_control_client.expects(:put_host_command_complete).never. when(@put_host_command_complete_state.is('never')) - @poller.perform + @poller.acknowledge_and_process_command(@command) end should 'call GetDeploymentSpecification with the host ID and execution ID of the command' do @@ -296,9 +293,8 @@ def gather_diagnostics(script_output) with(:deployment_execution_id => @command.deployment_execution_id, :host_identifier => @host_identifier). returns(@get_deploy_specification_output) - Thread.expects(:new).returns(nil) - @poller.perform + @poller.acknowledge_and_process_command(@command) end should 'allow exceptions from GetDeploymentSpecification to propagate to caller' do @@ -306,7 +302,7 @@ def gather_diagnostics(script_output) raises("some error") assert_raise "some error" do - @poller.perform + @poller.acknowledge_and_process_command(@command) end end @@ -319,11 +315,9 @@ def gather_diagnostics(script_output) should 'not dispatch the command to the command executor' do @execute_command_state.become('never') - Thread.expects(:new).never. - when(@execute_command_state.is('never')) assert_raise do - @poller.perform + @poller.acknowledge_and_process_command(@command) end end @@ -335,7 +329,7 @@ def gather_diagnostics(script_output) :host_command_identifier => @command.host_command_identifier) assert_raise do - @poller.perform + @poller.acknowledge_and_process_command(@command) end end @@ -350,11 +344,9 @@ def gather_diagnostics(script_output) should 'not dispatch the command to the command executor' do @execute_command_state.become('never') - Thread.expects(:new).never. - when(@execute_command_state.is('never')) assert_raise do - @poller.perform + @poller.acknowledge_and_process_command(@command) end end @@ -365,7 +357,7 @@ def gather_diagnostics(script_output) :host_command_identifier => @command.host_command_identifier) assert_raise do - @poller.perform + @poller.acknowledge_and_process_command(@command) end end @@ -380,11 +372,9 @@ def gather_diagnostics(script_output) should 'not dispatch the command to the command executor' do @execute_command_state.become('never') - Thread.expects(:new).never. - when(@execute_command_state.is('never')) assert_raise do - @poller.perform + @poller.acknowledge_and_process_command(@command) end end @@ -395,7 +385,7 @@ def gather_diagnostics(script_output) :host_command_identifier => @command.host_command_identifier) assert_raise do - @poller.perform + @poller.acknowledge_and_process_command(@command) end end @@ -462,6 +452,14 @@ def gather_diagnostics(script_output) @poller.perform end + should 'not try to enter thread if rejected execution error is raised (happens after shutdown initiated)' do + Thread.expects(:new).never + @mock_thread_pool_executor = mock() + Concurrent::ThreadPoolExecutor.expects(:new).returns(@mock_thread_pool_executor) + @mock_thread_pool_executor.stubs(:post).raises(Concurrent::RejectedExecutionError.new('RejectedExecutionError')) + + poller = InstanceAgent::Plugins::CodeDeployPlugin::CommandPoller.new + end end end From e60f8490b7cff2bf60328d6770c4e460d54c11cc Mon Sep 17 00:00:00 2001 From: Katyal Date: Tue, 3 Apr 2018 10:37:51 -0700 Subject: [PATCH 15/17] shelling out the delete commands for Windows and Linux subsystems for deleting the old deployment directories. cr https://code.amazon.com/reviews/CR-1710500 --- lib/instance_agent/platform/linux_util.rb | 28 ++++++++++++++++- lib/instance_agent/platform/windows_util.rb | 30 ++++++++++++++++++- .../plugins/codedeploy/command_executor.rb | 3 +- 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/lib/instance_agent/platform/linux_util.rb b/lib/instance_agent/platform/linux_util.rb index 094728a1..a4512774 100644 --- a/lib/instance_agent/platform/linux_util.rb +++ b/lib/instance_agent/platform/linux_util.rb @@ -67,6 +67,32 @@ def self.codedeploy_version_file def self.fallback_version_file "/opt/codedeploy-agent" end + + # shelling out the rm folder command to native os in this case linux. + def self.delete_dirs_command(dirs_to_delete) + log(:debug,"Dirs to delete: #{dirs_to_delete}"); + for dir in dirs_to_delete do + log(:debug,"Deleting dir: #{dir}"); + delete_folder(dir); + end + end + + private + def self.delete_folder (dir) + if dir != nil && dir != "/" + output = `rm -rf #{dir} 2>&1` + exit_status = $?.exitstatus + log(:debug, "Command status: #{$?}") + log(:debug, "Command output: #{output}") + unless exit_status == 0 + msg = "Error deleting directories: #{exit_status}" + log(:error, msg) + raise msg + end + else + log(:debug, "Empty directory or a wrong directory passed,#{dir}"); + end + end private def self.execute_tar_command(cmd) @@ -84,7 +110,7 @@ def self.execute_tar_command(cmd) raise msg end end - + private def self.log(severity, message) raise ArgumentError, "Unknown severity #{severity.inspect}" unless InstanceAgent::Log::SEVERITIES.include?(severity.to_s) diff --git a/lib/instance_agent/platform/windows_util.rb b/lib/instance_agent/platform/windows_util.rb index 719522c4..2980a155 100644 --- a/lib/instance_agent/platform/windows_util.rb +++ b/lib/instance_agent/platform/windows_util.rb @@ -32,10 +32,12 @@ def self.script_executable?(path) end def self.extract_tar(bundle_file, dst) + log(:warn, "Bundle format 'tar' not supported on Windows platforms. Bundle unpack may fail.") Minitar.unpack(bundle_file, dst) end def self.extract_tgz(bundle_file, dst) + log(:warn, "Bundle format 'tgz' not supported on Windows platforms. Bundle unpack may fail.") compressed = Zlib::GzipReader.open(bundle_file) Minitar.unpack(compressed, dst) end @@ -51,7 +53,33 @@ def self.codedeploy_version_file def self.fallback_version_file File.join(ENV['PROGRAMDATA'], "Amazon/CodeDeploy") end - + + # shelling out the rm folder command to native os in this case Window. + def self.delete_dirs_command(dirs_to_delete) + log(:debug,"Dirs to delete: #{dirs_to_delete}"); + for dir in dirs_to_delete do + log(:debug,"Deleting dir: #{dir}"); + delete_folder(dir); + end + end + + private + def self.delete_folder (dir) + if dir != nil && dir != "/" + output = `rd /s /q "#{dir}" 2>&1` + exit_status = $?.exitstatus + log(:debug, "Command status: #{$?}") + log(:debug, "Command output: #{output}") + unless exit_status == 0 + msg = "Error deleting directories: #{exit_status}" + log(:error, msg) + raise msg + end + else + log(:debug, "Empty directory or a wrong directory passed,#{dir}"); + end + end + private def self.log(severity, message) raise ArgumentError, "Unknown severity #{severity.inspect}" unless InstanceAgent::Log::SEVERITIES.include?(severity.to_s) diff --git a/lib/instance_agent/plugins/codedeploy/command_executor.rb b/lib/instance_agent/plugins/codedeploy/command_executor.rb index 75b998f5..3d1a1def 100644 --- a/lib/instance_agent/plugins/codedeploy/command_executor.rb +++ b/lib/instance_agent/plugins/codedeploy/command_executor.rb @@ -454,7 +454,8 @@ def cleanup_old_archives(deployment_spec) # Absolute path takes care of relative root directories directories = oldest_extra.map{ |f| File.absolute_path(f) } - FileUtils.rm_rf(directories) + log(:debug,"Delete Files #{directories}" ) + InstanceAgent::Platform.util.delete_dirs_command(directories) end From 2beac886957a4ea2a7299dba258c8d62b1a72d73 Mon Sep 17 00:00:00 2001 From: Katyal Date: Mon, 16 Apr 2018 14:58:59 -0700 Subject: [PATCH 16/17] Adds Lifecycle Event Tracking for an ongoing deployment This fix has been added to ensure that agent safely fails to update or rollback during an ongoing deployment.Will make sure that our update event is an idempotent event. https://sim.amazon.com/issues/P11706277 cr https://code.amazon.com/reviews/CR-1814241 --- .gitignore | 2 + .../codedeploy_local_steps.rb | 6 +- features/step_definitions/common_steps.rb | 23 +++---- lib/instance_agent/config.rb | 1 + .../plugins/codedeploy/command_poller.rb | 14 ++-- .../codedeploy/deployment_command_tracker.rb | 65 +++++++++++++++++ .../codedeploy/deployment_specification.rb | 4 +- lib/instance_agent/runner/master.rb | 16 ++++- .../deployment_command_tracker_spec.rb | 69 +++++++++++++++++++ test/instance_agent/config_test.rb | 1 + .../plugins/codedeploy/command_poller_test.rb | 13 +++- 11 files changed, 186 insertions(+), 28 deletions(-) create mode 100644 lib/instance_agent/plugins/codedeploy/deployment_command_tracker.rb create mode 100644 spec/aws/codedeploy/plugins/deployment_command_tracker_spec.rb diff --git a/.gitignore b/.gitignore index 4a2fd9cf..1ba9a25b 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,5 @@ RemoteSystemsTempFiles/ codedeploy-local.log* codedeploy-local.*.log deployment/ +.idea/ +.DS_STORE diff --git a/features/step_definitions/codedeploy_local_steps.rb b/features/step_definitions/codedeploy_local_steps.rb index edb1572e..d12d2410 100644 --- a/features/step_definitions/codedeploy_local_steps.rb +++ b/features/step_definitions/codedeploy_local_steps.rb @@ -56,7 +56,7 @@ def tar_app_bundle(temp_directory_to_create_bundle) #Unfortunately Minitar will keep pack all the file paths as given, so unless you change directories into the location where you want to pack the files the bundle won't have the correct files and folders Dir.chdir @bundle_original_directory_location - File.open(tar_file_name, 'wb') { |tar| Minitar.pack(directories_and_files_inside(@bundle_original_directory_location), tar) } + File.open(tar_file_name, 'wb') { |tar| Minitar.pack(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside(@bundle_original_directory_location), tar) } Dir.chdir old_direcory tar_file_name @@ -98,7 +98,7 @@ def tgz_app_bundle(temp_directory_to_create_bundle) File.open(tgz_file_name, 'wb') do |file| Zlib::GzipWriter.wrap(file) do |gz| - Minitar.pack(directories_and_files_inside(@bundle_original_directory_location), gz) + Minitar.pack(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside(@bundle_original_directory_location), gz) end end @@ -145,7 +145,7 @@ def create_local_deployment(custom_events = nil, file_exists_behavior = nil) end Then(/^the expected files should have have been locally deployed to my host(| twice)$/) do |maybe_twice| - deployment_ids = directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{LOCAL_DEPLOYMENT_GROUP_ID}") + deployment_ids = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{LOCAL_DEPLOYMENT_GROUP_ID}") step "the expected files in directory #{bundle_original_directory_location}/scripts should have have been deployed#{maybe_twice} to my host during deployment with deployment group id #{LOCAL_DEPLOYMENT_GROUP_ID} and deployment ids #{deployment_ids.join(' ')}" end diff --git a/features/step_definitions/common_steps.rb b/features/step_definitions/common_steps.rb index 078aa1ad..296d5a9a 100644 --- a/features/step_definitions/common_steps.rb +++ b/features/step_definitions/common_steps.rb @@ -46,7 +46,7 @@ def zip_app_bundle(temp_directory_to_create_bundle) end def zip_directory(input_dir, output_file) - entries = directories_and_files_inside(input_dir) + entries = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside(input_dir) zip_io = Zip::File.open(output_file, Zip::File::CREATE) write_zip_entries(entries, '', input_dir, zip_io) @@ -59,7 +59,7 @@ def write_zip_entries(entries, path, input_dir, zip_io) diskFilePath = File.join(input_dir, zipFilePath) if File.directory?(diskFilePath) zip_io.mkdir(zipFilePath) - folder_entries = directories_and_files_inside(diskFilePath) + folder_entries = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside(diskFilePath) write_zip_entries(folder_entries, zipFilePath, input_dir, zip_io) else zip_io.get_output_stream(zipFilePath){ |f| f.write(File.open(diskFilePath, "rb").read())} @@ -67,36 +67,33 @@ def write_zip_entries(entries, path, input_dir, zip_io) end end -def directories_and_files_inside(directory) - Dir.entries(directory) - %w(.. .) -end Then(/^the expected files in directory (\S+) should have have been deployed(| twice) to my host during deployment with deployment group id (\S+) and deployment ids (.+)$/) do |expected_scripts_directory, maybe_twice, deployment_group_id, deployment_ids_space_separated| deployment_ids = deployment_ids_space_separated.split(' ') - directories_in_deployment_root_folder = directories_and_files_inside(InstanceAgent::Config.config[:root_dir]) - expect(directories_in_deployment_root_folder.size).to eq(3) + directories_in_deployment_root_folder = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside(InstanceAgent::Config.config[:root_dir]) + expect(directories_in_deployment_root_folder.size).to be >= 3 #ordering of the directories depends on the deployment group id, so using include instead of eq expect(directories_in_deployment_root_folder).to include(*%W(deployment-instructions deployment-logs #{deployment_group_id})) - files_in_deployment_logs_folder = directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/deployment-logs") + files_in_deployment_logs_folder = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/deployment-logs") expect(files_in_deployment_logs_folder.size).to eq(1) expect(files_in_deployment_logs_folder).to eq(%w(codedeploy-agent-deployments.log)) - directories_in_deployment_group_id_folder = directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}") + directories_in_deployment_group_id_folder = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}") expect(directories_in_deployment_group_id_folder.size).to eq(maybe_twice.empty? ? 1 : 2) expect(directories_in_deployment_group_id_folder).to eq(deployment_ids) deployment_id = deployment_ids.first - files_and_directories_in_deployment_id_folder = directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}/#{deployment_id}") + files_and_directories_in_deployment_id_folder = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}/#{deployment_id}") expect(files_and_directories_in_deployment_id_folder).to include(*%w(logs deployment-archive)) - files_and_directories_in_deployment_archive_folder = directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}/#{deployment_id}/deployment-archive") + files_and_directories_in_deployment_archive_folder = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}/#{deployment_id}/deployment-archive") expect(files_and_directories_in_deployment_archive_folder.size).to eq(2) expect(files_and_directories_in_deployment_archive_folder).to include(*%w(appspec.yml scripts)) - files_in_scripts_folder = directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}/#{deployment_id}/deployment-archive/scripts") - sample_app_bundle_script_files = directories_and_files_inside(expected_scripts_directory) + files_in_scripts_folder = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside("#{InstanceAgent::Config.config[:root_dir]}/#{deployment_group_id}/#{deployment_id}/deployment-archive/scripts") + sample_app_bundle_script_files = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.directories_and_files_inside(expected_scripts_directory) expect(files_in_scripts_folder.size).to eq(sample_app_bundle_script_files.size) expect(files_in_scripts_folder).to include(*sample_app_bundle_script_files) end diff --git a/lib/instance_agent/config.rb b/lib/instance_agent/config.rb index 4a3f166f..59a0bc0a 100644 --- a/lib/instance_agent/config.rb +++ b/lib/instance_agent/config.rb @@ -23,6 +23,7 @@ def initialize :pid_dir => nil, :shared_dir => nil, :user => nil, + :ongoing_deployment_tracking => 'ongoing-deployment', :children => 1, :http_read_timeout => 80, :instance_service_region => nil, diff --git a/lib/instance_agent/plugins/codedeploy/command_poller.rb b/lib/instance_agent/plugins/codedeploy/command_poller.rb index 6c1c72a7..e059576a 100644 --- a/lib/instance_agent/plugins/codedeploy/command_poller.rb +++ b/lib/instance_agent/plugins/codedeploy/command_poller.rb @@ -1,8 +1,9 @@ require 'socket' require 'concurrent' - +require 'pathname' require 'instance_metadata' require 'instance_agent/agent/base' +require_relative 'deployment_command_tracker' module InstanceAgent module Plugins @@ -83,7 +84,6 @@ def graceful_shutdown # tell the pool to shutdown in an orderly fashion, allowing in progress work to complete @thread_pool.shutdown # now wait for all work to complete, wait till the timeout value - # TODO: Make the timeout configurable in the agent configuration @thread_pool.wait_for_termination ProcessManager::Config.config[:kill_agent_max_wait_time_seconds] log(:info, 'All agent child threads have been shut down') end @@ -104,13 +104,15 @@ def acknowledge_and_process_command(command) raise e end end - + def process_command(command, spec) log(:debug, "Calling #{@plugin.to_s}.execute_command") begin + deployment_id = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentSpecification.parse(spec).deployment_id + DeploymentCommandTracker.create_ongoing_deployment_tracking_file(deployment_id) #Successful commands will complete without raising an exception @plugin.execute_command(command, spec) - + log(:debug, 'Calling PutHostCommandComplete: "Succeeded"') @deploy_control_client.put_host_command_complete( :command_status => 'Succeeded', @@ -133,9 +135,11 @@ def process_command(command, spec) :host_command_identifier => command.host_command_identifier) log(:error, "Error during perform: #{e.class} - #{e.message} - #{e.backtrace.join("\n")}") raise e + ensure + DeploymentCommandTracker.delete_deployment_command_tracking_file(deployment_id) end end - + private def next_command log(:debug, "Calling PollHostCommand:") diff --git a/lib/instance_agent/plugins/codedeploy/deployment_command_tracker.rb b/lib/instance_agent/plugins/codedeploy/deployment_command_tracker.rb new file mode 100644 index 00000000..1bc2c473 --- /dev/null +++ b/lib/instance_agent/plugins/codedeploy/deployment_command_tracker.rb @@ -0,0 +1,65 @@ +require 'socket' +require 'concurrent' +require 'pathname' +require 'instance_metadata' +require 'instance_agent/agent/base' +require 'fileutils' +require 'instance_agent/log' + +module InstanceAgent + module Plugins + module CodeDeployPlugin + class FileDoesntExistException < Exception; end + class DeploymentCommandTracker + DEPLOYMENT_EVENT_FILE_STALE_TIMELIMIT_SECONDS = 86400 # 24 hour limit in secounds + + def self.create_ongoing_deployment_tracking_file(deployment_id) + FileUtils.mkdir_p(deployment_dir_path()) + FileUtils.touch(deployment_event_tracking_file_path(deployment_id)); + end + + def self.delete_deployment_tracking_file_if_stale?(deployment_id, timeout) + if(Time.now - File.ctime(deployment_event_tracking_file_path(deployment_id)) > timeout) + delete_deployment_command_tracking_file(deployment_id) + return true; + end + return false; + end + + def self.check_deployment_event_inprogress? + if(File.exists?deployment_dir_path()) + return directories_and_files_inside(deployment_dir_path()).any?{|deployment_id| check_if_lifecycle_event_is_stale?(deployment_id)} + else + return false + end + end + + def self.delete_deployment_command_tracking_file(deployment_id) + ongoing_deployment_event_file_path = deployment_event_tracking_file_path(deployment_id) + if File.exists?ongoing_deployment_event_file_path + File.delete(ongoing_deployment_event_file_path); + else + InstanceAgent::Log.warn("the tracking file does not exist") + end + end + + def self.directories_and_files_inside(directory) + Dir.entries(directory) - %w(.. .) + end + + private + def self.deployment_dir_path + File.join(InstanceAgent::Config.config[:root_dir], InstanceAgent::Config.config[:ongoing_deployment_tracking]) + end + + def self.check_if_lifecycle_event_is_stale?(deployment_id) + !delete_deployment_tracking_file_if_stale?(deployment_id,DEPLOYMENT_EVENT_FILE_STALE_TIMELIMIT_SECONDS) + end + + def self.deployment_event_tracking_file_path(deployment_id) + ongoing_deployment_file_path = File.join(deployment_dir_path(), deployment_id) + end + end + end + end +end \ No newline at end of file diff --git a/lib/instance_agent/plugins/codedeploy/deployment_specification.rb b/lib/instance_agent/plugins/codedeploy/deployment_specification.rb index 970758ac..689da1ae 100644 --- a/lib/instance_agent/plugins/codedeploy/deployment_specification.rb +++ b/lib/instance_agent/plugins/codedeploy/deployment_specification.rb @@ -97,7 +97,9 @@ def initialize(data) raise 'Exactly one of S3Revision, GitHubRevision, or LocalRevision must be specified' end end - + # Decrypts the envelope /deployment specs + # Params: + # envelope: deployment specification thats to be cheked and decrypted def self.parse(envelope) raise 'Provided deployment spec was nil' if envelope.nil? diff --git a/lib/instance_agent/runner/master.rb b/lib/instance_agent/runner/master.rb index bfaa3e20..902f15f9 100644 --- a/lib/instance_agent/runner/master.rb +++ b/lib/instance_agent/runner/master.rb @@ -1,11 +1,14 @@ # encoding: UTF-8 require 'process_manager/master' require 'instance_metadata' +require 'instance_agent/plugins/codedeploy/deployment_command_tracker' module InstanceAgent module Runner - class Master < ProcessManager::Daemon::Master + class DeploymentAlreadyInProgressException < Exception; end + class Master < ProcessManager::Daemon::Master + def self.description(pid = $$) "master #{pid}" end @@ -30,9 +33,16 @@ def self.pid_file # is overriden from ProcessManager::Daemon::Master def stop if (pid = self.class.find_pid) - puts "Stopping #{description(pid)}" - ProcessManager::Log.info("Stopping #{description(pid)}") + puts "Checking first if a deployment is already in progress" + ProcessManager::Log.info("Checking first if any deployment lifecycle event is in progress #{description(pid)}") begin + if(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.check_deployment_event_inprogress?) + ProcessManager::Log.info("Master process (#{pid}) will not be shut down right now, as a deployment is already in progress") + raise "A deployment is already in Progress",DeploymentAlreadyInProgressException + else + puts "Stopping #{description(pid)}" + ProcessManager::Log.info("Stopping #{description(pid)}") + end Process.kill('TERM', pid) rescue Errno::ESRCH end diff --git a/spec/aws/codedeploy/plugins/deployment_command_tracker_spec.rb b/spec/aws/codedeploy/plugins/deployment_command_tracker_spec.rb new file mode 100644 index 00000000..d602194d --- /dev/null +++ b/spec/aws/codedeploy/plugins/deployment_command_tracker_spec.rb @@ -0,0 +1,69 @@ +require 'spec_helper' +require 'instance_agent' +require 'instance_agent/config' +require 'instance_agent/plugins/codedeploy/deployment_command_tracker' +require 'instance_agent/log' + +describe InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker do + describe '.create_ongoing_deployment_tracking_file' do + $deployment_id = 'D-123' + deployment_command_tracker = InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker; + context "when the deployment life cycle event is in progress" do + before do + InstanceAgent::Config.config[:root_dir] = File.join(Dir.tmpdir(), 'codeDeploytest') + InstanceAgent::Config.config[:ongoing_deployment_tracking] = 'ongoing-deployment' + InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.create_ongoing_deployment_tracking_file($deployment_id) + end + it 'tries to create ongoing-deployment folder' do + directories_in_deployment_root_folder = deployment_command_tracker.directories_and_files_inside(InstanceAgent::Config.config[:root_dir]); + expect(directories_in_deployment_root_folder).to include(InstanceAgent::Config.config[:ongoing_deployment_tracking]); + end + it 'creates ongoing-deployment file in the tracking folder' do + files_in_deployment_tracking_folder = deployment_command_tracker.directories_and_files_inside(File.join(InstanceAgent::Config.config[:root_dir], InstanceAgent::Config.config[:ongoing_deployment_tracking])) + expect(files_in_deployment_tracking_folder).to include($deployment_id); + end + end + end + describe '.check_deployment_event_inprogress' do + context 'when no deployment life cycle event is in progress' do + before do + InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.delete_deployment_command_tracking_file($deployment_id) + end + it 'checks if any deployment event is in progress' do + expect(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.check_deployment_event_inprogress?).to equal(false); + end + end + context 'when deployment life cycle event is in progress' do + before do + InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.create_ongoing_deployment_tracking_file($deployment_id) + end + it 'checks if any deployment life cycle event is in progress ' do + expect(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.check_deployment_event_inprogress?).to equal(true) + end + end + context 'when the agent starts for the first time' do + before do + FileUtils.rm_r(File.join(InstanceAgent::Config.config[:root_dir], InstanceAgent::Config.config[:ongoing_deployment_tracking])) + end + it 'checks if any deployment life cycle event is in progress ' do + expect(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.check_deployment_event_inprogress?).to equal(false) + end + end + end + describe '.delete_deployment_tracking_file_if_stale' do + context 'when deployment life cycle event is in progress' do + before do + InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.create_ongoing_deployment_tracking_file($deployment_id) + end + it 'checks if the file is stale or not' do + expect(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.delete_deployment_tracking_file_if_stale?($deployment_id, 2000)).to equal(false) + end + end + context 'when the wait-time has been more than the timeout time' do + it 'checks if the file is stale after the timeout' do + sleep 10 + expect(InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.delete_deployment_tracking_file_if_stale?($deployment_id, 5)).to equal(true) + end + end + end +end \ No newline at end of file diff --git a/test/instance_agent/config_test.rb b/test/instance_agent/config_test.rb index 74391340..2b53bbaa 100644 --- a/test/instance_agent/config_test.rb +++ b/test/instance_agent/config_test.rb @@ -28,6 +28,7 @@ class InstanceAgentConfigTest < InstanceAgentTestCase :wait_after_error => 30, :codedeploy_test_profile => 'prod', :on_premises_config_file => '/etc/codedeploy-agent/conf/codedeploy.onpremises.yml', + :ongoing_deployment_tracking => 'ongoing-deployment', :proxy_uri => nil, :enable_deployments_log => true, :kill_agent_max_wait_time_seconds => 7200 diff --git a/test/instance_agent/plugins/codedeploy/command_poller_test.rb b/test/instance_agent/plugins/codedeploy/command_poller_test.rb index b7c515a5..38da1e91 100644 --- a/test/instance_agent/plugins/codedeploy/command_poller_test.rb +++ b/test/instance_agent/plugins/codedeploy/command_poller_test.rb @@ -1,8 +1,10 @@ require 'test_helper' require 'json' +require 'instance_agent/log' -class CommandPollerTest < InstanceAgentTestCase +class CommandPollerTest < InstanceAgentTestCase + include InstanceAgent::Plugins::CodeDeployPlugin def gather_diagnostics_from_error(error) {'error_code' => InstanceAgent::Plugins::CodeDeployPlugin::ScriptError::UNKNOWN_ERROR_CODE, 'script_name' => "", 'message' => error.message, 'log' => ""}.to_json end @@ -110,6 +112,12 @@ def gather_diagnostics(script_output) starts_as('setup') @deploy_control_client.stubs(:put_host_command_complete). when(@put_host_command_complete_state.is('setup')) + @deployment_id = stub(:deployment_id => "D-1234") + InstanceAgent::Config.config[:root_dir] = File.join(Dir.tmpdir(), "CodeDeploy") + InstanceAgent::Config.config[:ongoing_deployment_tracking] = "ongoing-deployment" + InstanceAgent::Plugins::CodeDeployPlugin::DeploymentSpecification.stubs(:parse).returns(@deployment_id) + InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.stubs(:delete_deployment_command_tracking_file).returns(true) + InstanceAgent::Plugins::CodeDeployPlugin::DeploymentCommandTracker.stubs(:create_ongoing_deployment_tracking_file).returns(true) end should 'call PollHostCommand with the current host name' do @@ -400,8 +408,7 @@ def gather_diagnostics(script_output) should 'allow exceptions from execute_command to propagate to caller' do @executor.expects(:execute_command). - raises("some error") - + raises("some error") @deploy_control_client.expects(:put_host_command_complete). with(:command_status => "Failed", :diagnostics => {:format => "JSON", :payload => gather_diagnostics_from_error(RuntimeError.new("some error"))}, From a9026ac342cff47e30bdcd21179c625bcea0a1aa Mon Sep 17 00:00:00 2001 From: Katyal Date: Mon, 14 May 2018 14:46:13 -0700 Subject: [PATCH 17/17] Add back in the verification of the certs by open SSL library Prior to this change, We had removed the validation of the signed message during https://code.amazon.com/reviews/CR-1228125/revisions/1#/diff. This change will address the security concern the AWS Security team had about our validation.https://tt.amazon.com/E035418734 has more details. cr https://code.amazon.com/reviews/CR-2042604 --- .../plugins/codedeploy/deployment_specification.rb | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/instance_agent/plugins/codedeploy/deployment_specification.rb b/lib/instance_agent/plugins/codedeploy/deployment_specification.rb index 689da1ae..a7bbfb42 100644 --- a/lib/instance_agent/plugins/codedeploy/deployment_specification.rb +++ b/lib/instance_agent/plugins/codedeploy/deployment_specification.rb @@ -106,8 +106,17 @@ def self.parse(envelope) case envelope.format when "PKCS7/JSON" pkcs7 = OpenSSL::PKCS7.new(envelope.payload) - pkcs7.verify([], @cert_store, nil, OpenSSL::PKCS7::NOVERIFY) - # NOTE: the pkcs7.data field is only populated AFTER pkcs7.verify() is called! + + # The PKCS7_NOCHAIN flag tells OpenSSL to ignore any PKCS7 CA chain that might be attached + # to the message directly and use the certificates from provided one only for validating the. + # signer's certificate. + # + # However, it will allow use the PKCS7 signer certificate provided to validate the signature. + # + # http://www.openssl.org/docs/crypto/PKCS7_verify.html#VERIFY_PROCESS + # + # The ruby wrapper returns true if OpenSSL returns 1 + raise "Validation of PKCS7 signed message failed" unless pkcs7.verify([], @cert_store, nil, OpenSSL::PKCS7::NOCHAIN) parse_deployment_spec_data(pkcs7.data) when "TEXT/JSON" raise "Unsupported DeploymentSpecification format: #{envelope.format}" unless AWS::CodeDeploy::Local::Deployer.running_as_developer_utility?