Get rid of default predeploy_commands.rb

CHANGELOG.md

@@ -1,22 +1,23 @@
 # 3.0.0
-#### Breaking Changes
+### Breaking Changes
 - `ssh`, `bash`, `logtail`, `status`, and `run` are now top level commands, not subcommands of `deploy`
 - `config.git_repo=` and `config.type=` were removed.
 - `config.base` and `config.deploy` are no longer backwards compatible
 - `instance` can no longer be configured on a per `Target` basis
+- No more `RAKE_DB_MIGRATE` constant
 
 #### Added Features
-- Allow configuration of separate docker images per target
-- Readd ability to configured a default tag per target
+- Allow configuration of separate `:docker_image` per target
+- Readd ability to configured a default `:tag` per target
 - Add `targets` command to display all the targets' deployed images and CPU/memory allocations
+- `broadside status` has an added `--verbose` switch that displays service and task information
 
 #### General Improvements
 - Only load `env_files` for the selected target (rather than preloading from unrelated targets)
 - Make `env_files` configuration optional
 - `Utils` has been replaced in favor of `LoggingUtils`
 - Exceptions will be raised if a target is configured with an invalid hash key
 - Tasks run have a more relevant `started_by` tag
-- `broadside status` has an added `--verbose` switch that displays service and task information
 
 # 2.0.0
 #### Breaking Changes

README.md

@@ -20,7 +20,7 @@ Broadside.configure do |config|
       command: ['bundle', 'exec', 'unicorn', '-c', 'config/unicorn.conf.rb'],
       env_file: '.env.production'
       predeploy_commands: [
-        Broadside::Predeploy::RAKE_DB_MIGRATE, # RAKE_DB_MIGRATE is just a constant for your convenience
+        ['bundle', 'exec', 'rake', 'db:migrate']
         ['bundle', 'exec', 'rake', 'data:migrate']
       ]
     },

lib/broadside.rb

@@ -10,7 +10,6 @@
 require 'broadside/command'
 require 'broadside/target'
 require 'broadside/deploy'
-require 'broadside/predeploy_commands'
 require 'broadside/ecs/ecs_deploy'
 require 'broadside/ecs/ecs_manager'
 require 'broadside/version'
@@ -25,21 +24,15 @@ def self.configure
   end
 
   def self.load_config(config_file)
-    begin
-      load USER_CONFIG_FILE if File.exists?(USER_CONFIG_FILE)
-    rescue LoadError => e
-      error "Encountered an error loading system configuration file '#{USER_CONFIG_FILE}' !"
-      raise e
-    end
+    raise ArgumentError, "#{config_file} does not exist" unless File.exist?(config_file)
 
+    config.config_file = config_file
     begin
-      config.config_file = config_file
-      load config_file
-    rescue LoadError => e
-      error "Encountered an error loading required configuration file '#{config_file}' !"
-      raise e
+      [config_file, USER_CONFIG_FILE].each { |file| load file if File.exist?(file) }
+    rescue LoadError
+      error 'Encountered an error loading broadside configuration'
+      raise
     end
-
     raise ArgumentError, config.errors.full_messages unless config.valid?
   end
 

lib/broadside/cli/gli_commands.rb

@@ -27,7 +27,7 @@ def add_instance_flag(cmd)
   end
 end
 
-desc 'Gives an overview of the deploy targets'
+desc 'Gives an overview of all of the deploy targets'
 command :targets do |targets|
   targets.action do |_, options, _|
     Broadside::Command.targets(options)

lib/broadside/cli/global.rb

@@ -16,20 +16,20 @@
 
 def call_hook(type, command)
   hook = Broadside.config.public_send(type)
+  return if hook.nil?
+  raise "#{type} hook is not a callable proc" unless hook.is_a?(Proc)
 
-  if hook.is_a?(Proc)
-    hook_args =
-      if command.parent.is_a?(GLI::Command)
-        {
-          command: command.parent.name,
-          subcommand: command.name
-        }
-      else
-        { command: command.name }
-      end
-    debug "Calling", type, "with args", hook_args
-    hook.call(hook_args)
-  end
+  hook_args =
+    if command.parent.is_a?(GLI::Command)
+      {
+        command: command.parent.name,
+        subcommand: command.name
+      }
+    else
+      { command: command.name }
+    end
+  debug "Calling #{type} with args '#{hook_args}'"
+  hook.call(hook_args)
 end
 
 pre do |global, command, options, args|
@@ -44,11 +44,10 @@ def call_hook(type, command)
 end
 
 on_error do |exception|
-  # false skips default error handling
   case exception
   when Broadside::MissingVariableError
     error exception.message, "Run your last command with --help for more information."
-    false
+    false # false skips default error handling
   else
     true
   end

lib/broadside/command.rb

@@ -57,12 +57,9 @@ def deploy_rollback(options)
       end
 
       def status(options)
-        target = Broadside.config.target_from_name!(options[:target])
-        cluster = target.cluster
-        family = target.family
-        pastel = Pastel.new
-
+        target = Broadside.config.get_target_by_name!(options[:target])
         debug "Getting status information about #{family}"
+
         output = [
           "\n---------------",
           pastel.underline('Current task definition information:'),

lib/broadside/configuration.rb

@@ -25,15 +25,17 @@ class Configuration
     validates_each(:aws) do |record, _, val|
       [:region, :credentials].each { |v| record.errors.add("aws.#{v}") unless val.public_send(v) }
     end
+    validates_each :ssh, allow_nil: true do |record, attr, val|
+      record.errors.add(attr, 'is not a hash') unless val.is_a?(Hash)
+      record.errors.add(attr, 'must contain a user') unless val[:user]
+    end
 
     def initialize
       @logger = ::Logger.new(STDOUT)
       @logger.level = ::Logger::DEBUG
       @logger.datetime_format = '%Y-%m-%d_%H:%M:%S'
       @timeout = 600
       @type = 'ecs'
-      @ssh = {}
-      @targets = {}
     end
 
     def aws
@@ -44,29 +46,31 @@ def ecs
       @ecs ||= EcsConfig.new
     end
 
-    def ssh_cmd(ip, options = { tty: false })
+    def ssh_cmd(ip, options = {})
+      raise MissingVariableError, 'ssh not configured' unless @ssh
+
       cmd = 'ssh -o StrictHostKeyChecking=no'
       cmd << ' -t -t' if options[:tty]
       cmd << " -i #{@ssh[:keyfile]}" if @ssh[:keyfile]
       if (proxy = @ssh[:proxy])
-        raise ArgumentError, "Bad proxy host/port: #{proxy[:host]}/#{proxy[:port]}" unless proxy[:host] && proxy[:port]
+        raise ArgumentError, "Bad proxy: #{proxy[:host]}/#{proxy[:port]}" unless proxy[:host] && proxy[:port]
         cmd << " -o ProxyCommand=\"ssh #{proxy[:host]} nc #{ip} #{proxy[:port]}\""
       end
       cmd << " #{@ssh[:user]}@#{ip}"
       cmd
     end
 
+    # Transform deploy target configs to Target objects
     def targets=(_targets)
       raise ArgumentError, ":targets must be a hash" unless _targets.is_a?(Hash)
-      # transform deploy target configs to target objects
+
       @targets = _targets.inject({}) do |h, (target_name, config)|
-        h[target_name] = Target.new(target_name, config)
-        h
+        h.merge(target_name => Target.new(target_name, config))
       end
     end
 
-    def target_from_name!(name)
-      @targets.fetch(name) { |k| raise Error, "Deploy target '#{name}' does not exist!" }
+    def get_target_by_name!(name)
+      @targets.fetch(name) { |k| raise ArgumentError, "Deploy target '#{name}' does not exist!" }
     end
   end
 end

lib/broadside/deploy.rb

@@ -1,5 +1,3 @@
-require 'active_support/core_ext/module/delegation'
-
 module Broadside
   class Deploy
     include LoggingUtils
@@ -8,7 +6,7 @@ class Deploy
     delegate :family, to: :target
 
     def initialize(target_name, options = {})
-      @target   = Broadside.config.target_from_name!(target_name)
+      @target   = Broadside.config.get_target_by_name!(target_name)
       @command  = options[:command]  || @target.command
       @instance = options[:instance] || 0
       @lines    = options[:lines]    || 10

lib/broadside/ecs/ecs_deploy.rb

@@ -246,7 +246,7 @@ def container_definition
       configured_containers = (@target.task_definition_config || {})[:container_definitions]
 
       if configured_containers && configured_containers.size > 1
-        raise ArgumentError, 'Creating > 1 container definition not supported yet'
+        raise Error, 'Creating > 1 container definition not supported yet'
       end
 
       (configured_containers.try(:first) || {}).merge(

lib/broadside/ecs/ecs_manager.rb

@@ -63,17 +63,16 @@ def get_running_instance_ips(cluster, family, task_arns = nil)
       end
 
       def get_task_arns(cluster, family, filter = {})
-        opts = {
+        options = {
           cluster: cluster,
-          family: family,
+          # Strange AWS restriction requires absence of family if service_name specified
+          family: filter[:service_name] ? nil : family,
           desired_status: filter[:desired_status],
           service_name: filter[:service_name],
           started_by: filter[:started_by]
-        }
-        # strange AWS restriction requires absence of family if service_name specified
-        opts[:family] = nil if opts[:service_name]
-        opts.delete_if { |_k, v| v.nil? }
-        all_results(:list_tasks, :task_arns, opts)
+        }.reject { |_, v| v.nil? }
+
+        all_results(:list_tasks, :task_arns, options)
       end
 
       def get_task_definition_arns(family)

lib/broadside/error.rb

@@ -1,6 +1,5 @@
 module Broadside
-  class MissingVariableError < StandardError
-  end
+  class MissingVariableError < StandardError; end
 
   class Error < StandardError
     def initialize(msg = 'Broadside encountered an error !')

spec/broadside/ecs/ecs_deploy_spec.rb

@@ -5,13 +5,12 @@
   include_context 'ecs stubs'
 
   let(:family) { deploy.target.family }
-  let(:target) { Broadside::Target.new(test_target_name, test_target_config.merge(local_target_config)) }
+  let(:target) { Broadside::Target.new(test_target_name, test_target_config) }
   let(:local_deploy_config) { {} }
   let(:deploy) { described_class.new(test_target_name, local_deploy_config.merge(tag: 'tag_the_bag')) }
   let(:desired_count) { 2 }
   let(:cpu) { 1 }
   let(:memory) { 2000 }
-  let(:arn) { 'arn:aws:ecs:us-east-1:1234' }
   let(:service_config) do
     {
       desired_count: desired_count,
@@ -21,10 +20,6 @@
     }
   end
 
-  it 'should instantiate an object' do
-    expect { deploy }.to_not raise_error
-  end
-
   context 'bootstrap' do
     it 'fails without task_definition_config' do
       expect { deploy.bootstrap }.to raise_error(/No first task definition and no :task_definition_config/)
@@ -108,7 +103,6 @@
 
             register_requests = api_request_log.select { |cmd| cmd.keys.first == :register_task_definition }
             expect(register_requests.size).to eq(1)
-
             expect(register_requests.first.values.first[:container_definitions].first[:cpu]).to eq(cpu)
             expect(register_requests.first.values.first[:container_definitions].first[:memory]).to eq(memory)
           end
@@ -194,12 +188,12 @@
 
       before do
         ecs_stub.stub_responses(:run_task, tasks: [task_arn: 'task_arn'])
-        allow(ecs_stub).to receive(:wait_until)
-        allow(deploy).to receive(:get_container_logs)
-        allow(Broadside::EcsManager).to receive(:get_task_exit_code).and_return(0)
       end
 
       it 'runs' do
+        expect(ecs_stub).to receive(:wait_until)
+        expect(deploy).to receive(:get_container_logs)
+        expect(Broadside::EcsManager).to receive(:get_task_exit_code).and_return(0)
         expect { deploy.run }.to_not raise_error
       end
     end

spec/broadside/target_spec.rb

@@ -1,13 +1,12 @@
 require 'spec_helper'
 
 describe Broadside::Target do
-  include_context 'deploy configuration'
 
-  let(:target_name) { 'tarbaby_target' }
+  include_context 'deploy configuration'
 
   shared_examples 'valid_configuration?' do |succeeds, config_hash|
     let(:valid_options) { { scale: 100 } }
-    let(:target) { described_class.new(target_name, valid_options.merge(config_hash) )}
+    let(:target) { described_class.new(test_target_name, valid_options.merge(config_hash)) }
 
     it 'validates target configuration' do
       if succeeds
@@ -30,24 +29,21 @@
     it_behaves_like 'valid_configuration?', true,  env_files: ['file', 'file2']
 
     it_behaves_like 'valid_configuration?', true,  command: nil
-    it_behaves_like 'valid_configuration?', true,  command: ['bundle', 'exec', 'resque:work']
-    it_behaves_like 'valid_configuration?', false, command: 'bundle exec rails s'
+    it_behaves_like 'valid_configuration?', true,  command: %w(do something)
+    it_behaves_like 'valid_configuration?', false, command: 'do something'
 
     it_behaves_like 'valid_configuration?', false, not_a_param: 'foo'
 
     it_behaves_like 'valid_configuration?', true,  predeploy_commands: nil
-    it_behaves_like 'valid_configuration?', false, predeploy_commands: Broadside::PredeployCommands::RAKE_DB_MIGRATE
-    it_behaves_like 'valid_configuration?', false, predeploy_commands: 'bundle exec rake db:migrate'
-    it_behaves_like 'valid_configuration?', true,  predeploy_commands: [Broadside::PredeployCommands::RAKE_DB_MIGRATE]
-    it_behaves_like 'valid_configuration?', true,  predeploy_commands: [
-      Broadside::PredeployCommands::RAKE_DB_MIGRATE,
-      ['bundle', 'exec', 'rake' 'assets:precompile']
-    ]
+    it_behaves_like 'valid_configuration?', false, predeploy_commands: %w(do something)
+    it_behaves_like 'valid_configuration?', true,  predeploy_commands: [%w(do something)]
+    it_behaves_like 'valid_configuration?', true,  predeploy_commands: [%w(do something), %w(other command)]
   end
 
   describe '#ecs_env_vars' do
     let(:valid_options) { { scale: 1, env_files: env_files } }
-    let(:target) { described_class.new(target_name, valid_options) }
+    let(:target) { described_class.new(test_target_name, valid_options) }
+    let(:dot_env_file) { File.join(FIXTURES_PATH, '.env.rspec') }
 
     shared_examples 'successfully loaded env_files' do
       it 'loads environment variables from a file' do
@@ -59,7 +55,7 @@
       let(:env_files) { dot_env_file }
       let(:expected_env_vars) do
         [
-          { 'name' => 'TEST_KEY1', 'value' => 'TEST_VALUE1'},
+          { 'name' => 'TEST_KEY1', 'value' => 'TEST_VALUE1' },
           { 'name' => 'TEST_KEY2', 'value' => 'TEST_VALUE2'}
         ]
       end
@@ -72,8 +68,8 @@
       let(:expected_env_vars) do
         [
           { 'name' => 'TEST_KEY1', 'value' => 'TEST_VALUE1' },
-          { 'name' => 'TEST_KEY2', 'value' => 'TEST_VALUE_OVERRIDE'},
-          { 'name' => 'TEST_KEY3', 'value' => 'TEST_VALUE3'}
+          { 'name' => 'TEST_KEY2', 'value' => 'TEST_VALUE_OVERRIDE' },
+          { 'name' => 'TEST_KEY3', 'value' => 'TEST_VALUE3' }
         ]
       end