WIP update-safe service config for load balancing

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}"

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

lib/broadside/version.rb

@@ -1,3 +1,3 @@
 module Broadside
-  VERSION = '3.0.4'.freeze
+  VERSION = '3.0.5'.freeze
 end

spec/broadside/ecs/ecs_deploy_spec.rb

@@ -10,14 +10,27 @@
   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: {
         minimum_healthy_percent: 40,
       }
     }
   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
 

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