diff --git a/lib/broadside/ecs/ecs_deploy.rb b/lib/broadside/ecs/ecs_deploy.rb index 1a655d5..e6f068f 100644 --- a/lib/broadside/ecs/ecs_deploy.rb +++ b/lib/broadside/ecs/ecs_deploy.rb @@ -152,7 +152,7 @@ def update_service(options = {}) desired_count: scale, service: family, task_definition: task_definition_arn - }.deep_merge(@target.service_config || {})) + }.deep_merge(@target.service_config_for_update || {})) unless update_service_response.successful? raise EcsError, "Failed to update service:\n#{update_service_response.pretty_inspect}" diff --git a/lib/broadside/target.rb b/lib/broadside/target.rb index cf8050e..652fa6f 100644 --- a/lib/broadside/target.rb +++ b/lib/broadside/target.rb @@ -44,6 +44,14 @@ class Target record.errors.add(attr, 'is not an array of strings') unless val.is_a?(Array) && val.all? { |v| v.is_a?(String) } end + CREATE_ONLY_SERVICE_ATTRIBUTES = %i( + client_token + load_balancers + placement_constraints + placement_strategy + role + ).freeze + def initialize(name, options = {}) @name = name @@ -93,5 +101,9 @@ def to_h Cluster: @cluster } end + + def service_config_for_update + service_config.try(:except, *CREATE_ONLY_SERVICE_ATTRIBUTES) + end end end diff --git a/lib/broadside/version.rb b/lib/broadside/version.rb index e341902..913ed39 100644 --- a/lib/broadside/version.rb +++ b/lib/broadside/version.rb @@ -1,3 +1,3 @@ module Broadside - VERSION = '3.0.4'.freeze + VERSION = '3.0.5'.freeze end diff --git a/spec/broadside/ecs/ecs_deploy_spec.rb b/spec/broadside/ecs/ecs_deploy_spec.rb index c77da03..75efb39 100644 --- a/spec/broadside/ecs/ecs_deploy_spec.rb +++ b/spec/broadside/ecs/ecs_deploy_spec.rb @@ -10,7 +10,7 @@ let(:desired_count) { 4 } let(:cpu) { 1 } let(:memory) { 2000 } - let(:service_config) do + let(:base_service_config) do { desired_count: desired_count, deployment_configuration: { @@ -18,6 +18,19 @@ } } end + let(:service_config_with_load_balancers) do + base_service_config.merge( + role: 'ecsServiceRole', + load_balancers: [ + { + container_name: 'test-container', + container_port: 8080, + load_balancer_name: 'test-container-elb' + } + ] + ) + end + let(:service_config) { base_service_config } describe '#bootstrap' do it 'fails without task_definition_config' do @@ -40,7 +53,7 @@ end end - context 'with an existing service' do + shared_examples 'correctly-behaving bootstrap' do include_context 'with a running service' it 'succeeds' do @@ -57,6 +70,16 @@ end end end + + context 'with an existing service' do + it_behaves_like 'correctly-behaving bootstrap' + end + + context 'with an existing task definition that has create-only parameters' do + let(:service_config) { service_config_with_load_balancers } + + it_behaves_like 'correctly-behaving bootstrap' + end end end @@ -72,7 +95,7 @@ expect { deploy.short }.to raise_error(/No task definition for/) end - context 'with an existing task definition' do + shared_examples 'correctly-behaving deploy' do include_context 'with a task_definition' it 'short deploy does not fail' do @@ -142,6 +165,16 @@ expect(api_request_methods.include?(:update_service)).to be true end end + + context 'with an existing task definition' do + it_behaves_like 'correctly-behaving deploy' + end + + context 'with an existing task definition that has create-only parameters' do + let(:service_config) { service_config_with_load_balancers } + + it_behaves_like 'correctly-behaving deploy' + end end end diff --git a/spec/broadside/target_spec.rb b/spec/broadside/target_spec.rb index 8a2b2de..20828d7 100644 --- a/spec/broadside/target_spec.rb +++ b/spec/broadside/target_spec.rb @@ -3,23 +3,25 @@ describe Broadside::Target do include_context 'deploy configuration' - describe '#initialize' do - let(:all_possible_options) do - { - bootstrap_commands: [], - cluster: 'some-cluster', - command: %w(some command), - docker_image: 'lumoslabs/hello', - env_file: '.env.test', - predeploy_commands: [], - scale: 9000, - service_config: {}, - tag: 'latest', - task_definition_config: {} - } - end - let(:target) { described_class.new(test_target_name, all_possible_options) } + let(:base_possible_options) do + { + bootstrap_commands: [], + cluster: 'some-cluster', + command: %w(some command), + docker_image: 'lumoslabs/hello', + env_file: '.env.test', + predeploy_commands: [], + scale: 9000, + service_config: {}, + tag: 'latest', + task_definition_config: {} + } + end + let(:all_possible_options) { base_possible_options } + let(:all_possible_options_with_create_only_service_options) { base_possible_options } + let(:target) { described_class.new(test_target_name, all_possible_options) } + describe '#initialize' do it 'should initialize without erroring using all possible options' do expect { target }.to_not raise_error end @@ -46,7 +48,7 @@ it_behaves_like 'valid_configuration?', true, env_files: nil it_behaves_like 'valid_configuration?', true, env_files: 'file' - it_behaves_like 'valid_configuration?', true, env_files: ['file', 'file2'] + it_behaves_like 'valid_configuration?', true, env_files: %w(file file2) it_behaves_like 'valid_configuration?', true, command: nil it_behaves_like 'valid_configuration?', true, command: %w(do something) @@ -76,7 +78,7 @@ let(:expected_env_vars) do [ { 'name' => 'TEST_KEY1', 'value' => 'TEST_VALUE1' }, - { 'name' => 'TEST_KEY2', 'value' => 'TEST_VALUE2'} + { 'name' => 'TEST_KEY2', 'value' => 'TEST_VALUE2' } ] end @@ -96,4 +98,32 @@ it_behaves_like 'successfully loaded env_files' end end + + describe '#service_config_for_update' do + context 'with no service config' do + it 'does not raise an error' do + expect { target }.to_not raise_error + end + end + + shared_examples 'accessor for update-safe service config parameters' do + it 'does not raise an error' do + expect { target }.to_not raise_error + end + + it 'returns the basic serivce config parameters' do + expect(target.service_config).to eq(base_possible_options[:service_config]) + end + end + + context 'with a normal service config' do + it_behaves_like 'accessor for update-safe service config parameters' + end + + context 'with a service config containing create-only parameters' do + let(:all_possible_options) { all_possible_options_with_create_only_service_options } + + it_behaves_like 'accessor for update-safe service config parameters' + end + end end