New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Get rid of default predeploy_commands.rb #59
Changes from 16 commits
8c175d0
7085d75
ed154f1
b8db3b7
72af1ce
ad01133
e0de1f6
ff4d61c
d34e981
471ba9d
c917024
67a50b3
b52da6e
0e63131
58cc4a5
23fbe5d
d70654a
8a73eae
d2a1cb8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,12 +57,12 @@ def deploy_rollback(options) | |
end | ||
|
||
def status(options) | ||
target = Broadside.config.target_from_name!(options[:target]) | ||
target = Broadside.config.get_target_by_name!(options[:target]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lint/UselessAssignment: Useless assignment to variable - target. |
||
cluster = target.cluster | ||
family = target.family | ||
pastel = Pastel.new | ||
|
||
debug "Getting status information about #{family}" | ||
|
||
output = [ | ||
"\n---------------", | ||
pastel.underline('Current task definition information:'), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need for the user requirement (someone may have configured a default user in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would the command then be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good point. i'll address the |
||
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) { raise ArgumentError, "Deploy target '#{name}' does not exist!" } | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,17 +63,16 @@ def get_running_instance_ips(cluster, family, task_arns = nil) | |
end | ||
|
||
def get_task_arns(cluster, family, filter = {}) | ||
opts = { | ||
options = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🆒 |
||
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) | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
require 'spec_helper' | ||
|
||
describe Broadside::Configuration do | ||
include_context 'deploy configuration' | ||
|
||
it 'should be able to find a target' do | ||
expect { Broadside.config.get_target_by_name!(test_target_name) }.to_not raise_error | ||
end | ||
|
||
it 'should raise an error when a target is missing' do | ||
expect { Broadside.config.get_target_by_name!('barf') }.to raise_error(ArgumentError) | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,11 +3,9 @@ | |
describe Broadside::Target do | ||
include_context 'deploy configuration' | ||
|
||
let(:target_name) { 'tarbaby_target' } | ||
|
||
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 +28,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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW i am super annoyed that you can't use |
||
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 +54,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 +67,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 | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, does this just reraise the same error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea