Skip to content

Commit

Permalink
Allow ignoring ECS Immutable fields if they aren't supplied (#463)
Browse files Browse the repository at this point in the history
* Allow ignoring ECS Immutable fields if they aren't supplied in the first place.

* Bump version
  • Loading branch information
diranged authored Feb 22, 2018
1 parent ef33184 commit 681cd77
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 2 deletions.
16 changes: 15 additions & 1 deletion kingpin/actors/aws/ecs.py
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,11 @@ def _execute(self):
class Service(ECSBaseActor):
"""Register and run a service on ECS.
This actor will loop indefinitely until the task is complete.
This actor will loop indefinitely until the task is complete. If any
Service parameters are not supplied, then Amazon supplies the defaults and
manages them. If these are immutable in Amazon, then you cannot change them
in the ECS Service Definition down in a future update, and Kingpin will
error out.
If the service already exists, it is upgraded.
Expand Down Expand Up @@ -1129,6 +1133,16 @@ def _check_immutable_field_errors(self, old_params, new_params,
new_field = new_params.get(immutable_field_name)
old_field = old_params.get(immutable_field_name)

# If the newly supplied fields are None, then they aren't defined
# in the service definition at all. Its OK to ignore them.
if new_field is None:
continue

# If the supplied fields are an empty list, treat them also as an
# null field and move on.
if isinstance(new_field, list) and len(new_field) == 0:
continue

if new_field != old_field:
has_error = True
self.log.error(
Expand Down
8 changes: 8 additions & 0 deletions kingpin/actors/aws/test/test_ecs.py
Original file line number Diff line number Diff line change
Expand Up @@ -1577,6 +1577,12 @@ def test_same_no_immutable(self):
new={'a': 1},
immutable=[], expected_error_count=0)

@testing.gen_test
def test_not_set_but_different(self):
self._test(old={'a': 1, 'b': [1, 2, 3]},
new={'a': None, 'b': []},
immutable=['a', 'b'], expected_error_count=0)

@testing.gen_test
def test_different_no_immutable(self):
self._test(old={'a': 1},
Expand Down Expand Up @@ -1681,6 +1687,8 @@ def test_seen_all_two_events(self):
self.assertEqual(events, [])

def test_timestamp_filtering(self):
# If the supplied fields are an empty list, treat them also
# as an null field and move on.
events_after = [{
'id': 0,
'message': 'message1',
Expand Down
2 changes: 1 addition & 1 deletion kingpin/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@
# Copyright 2018 Nextdoor.com, Inc


__version__ = '0.5.2'
__version__ = '0.5.3'

0 comments on commit 681cd77

Please sign in to comment.