From 796b7c8962a597de9c622010e5fd83487fda2709 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Santos?= Date: Tue, 16 Jan 2018 09:07:26 +0100 Subject: [PATCH 1/9] Add pipfile --- Pipfile | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 Pipfile diff --git a/Pipfile b/Pipfile new file mode 100644 index 00000000..6b0592e6 --- /dev/null +++ b/Pipfile @@ -0,0 +1,29 @@ +[[source]] + +url = "https://pypi.python.org/simple" +verify_ssl = true +name = "pypi" + + +[packages] + +arrow = "*" +clickclick = ">=1.0" +pystache = "*" +PyYAML = "*" +dnspython = ">=1.15.0" +stups-pierone = ">=1.0.34" +"boto3" = ">=1.3.0" +botocore = ">=1.4.10" +pytest = ">=2.7.3" +raven = "*" +typing = "*" + + +[dev-packages] + + + +[requires] + +python_version = "3" From f10b5354fdecb66004f2452a8e2f139191cb6524 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Santos?= Date: Tue, 16 Jan 2018 09:37:44 +0100 Subject: [PATCH 2/9] Change all stacks' traffic if all stacks have 0 traffic --- senza/traffic.py | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/senza/traffic.py b/senza/traffic.py index 1eff94cd..19d1976b 100644 --- a/senza/traffic.py +++ b/senza/traffic.py @@ -47,12 +47,21 @@ def get_weights(dns_names: list, identifier: str, all_identifiers) -> ({str: int continue else: known_record_weights[record.set_identifier] = weight - if record.set_identifier != identifier and weight > 0: - # we should ignore all versions that do not get any - # traffic to not give traffic to disabled versions when - # redistributing traffic weights - partial_sum += weight - partial_count += 1 + + all_records_have_0_weight = sum(known_record_weights.values()) == 0 + if all_records_have_0_weight: + warning("All stacks have 0 weight, so all of them will be considered " + "for traffic changing.") + for record_identifier, weight in known_record_weights.items(): + record_has_traffic = weight > 0 + change_record_traffic = record_has_traffic or all_records_have_0_weight + if record_identifier != identifier and change_record_traffic: + # we should ignore all versions that do not get any + # traffic to not give traffic to disabled versions when + # redistributing traffic weights unless all records have 0 weight + # see https://github.com/zalando-stups/senza/issues/490 + partial_sum += weight + partial_count += 1 if identifier not in known_record_weights: known_record_weights[identifier] = 0 for ident in all_identifiers: @@ -120,7 +129,7 @@ def compensate(calculation_error, compensations, identifier, new_record_weights, compensations[identifier] = calculation_error calculation_error = 0 warning( - ("Changing given percentage from {} to {} " + + ("Changing given percentage from {} to {} " "because all other versions are already getting the possible minimum traffic").format( percentage / PERCENT_RESOLUTION, adjusted_percentage / PERCENT_RESOLUTION)) percentage = adjusted_percentage From 26720bd10c3de55ae58885efc7b78199eb304f19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Santos?= Date: Tue, 16 Jan 2018 09:38:01 +0100 Subject: [PATCH 3/9] Add tox to Pipfile --- Pipfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Pipfile b/Pipfile index 6b0592e6..8c022475 100644 --- a/Pipfile +++ b/Pipfile @@ -10,7 +10,7 @@ name = "pypi" arrow = "*" clickclick = ">=1.0" pystache = "*" -PyYAML = "*" +pyyaml = "*" dnspython = ">=1.15.0" stups-pierone = ">=1.0.34" "boto3" = ">=1.3.0" @@ -22,6 +22,7 @@ typing = "*" [dev-packages] +tox = "*" [requires] From ce23d0dbeb0d0a2f0f58ccc5f1e5faae749ebb61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Santos?= Date: Tue, 16 Jan 2018 10:58:45 +0100 Subject: [PATCH 4/9] Split traffic.get_weights function to enable code reuse --- senza/traffic.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/senza/traffic.py b/senza/traffic.py index 19d1976b..9881d3bd 100644 --- a/senza/traffic.py +++ b/senza/traffic.py @@ -26,14 +26,7 @@ DNS_ZONE_CACHE = {} -def get_weights(dns_names: list, identifier: str, all_identifiers) -> ({str: int}, int, int): - """ - For the given dns_name, get the dns record weights from provided dns record set - followed by partial count and partial weight sum. - Here partial means without the element that we are operating now on. - """ - partial_count = 0 - partial_sum = 0 +def get_weights_for_dns(dns_names): known_record_weights = {} for dns_name in dns_names: for record in Route53.get_records(name=dns_name): @@ -47,6 +40,18 @@ def get_weights(dns_names: list, identifier: str, all_identifiers) -> ({str: int continue else: known_record_weights[record.set_identifier] = weight + return known_record_weights + + +def get_weights(dns_names: list, identifier: str, all_identifiers) -> ({str: int}, int, int): + """ + For the given dns_name, get the dns record weights from provided dns record set + followed by partial count and partial weight sum. + Here partial means without the element that we are operating now on. + """ + partial_count = 0 + partial_sum = 0 + known_record_weights = get_weights_for_dns(dns_names) all_records_have_0_weight = sum(known_record_weights.values()) == 0 if all_records_have_0_weight: From 3fc0df1d25a131adaba1554745d7942612ea2850 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Santos?= Date: Tue, 16 Jan 2018 11:31:26 +0100 Subject: [PATCH 5/9] Check traffic on create --- senza/cli.py | 43 ++++++++++++++++++++++++++++++++++++++----- tests/test_cli.py | 2 ++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/senza/cli.py b/senza/cli.py index bef078e0..fb2a76b1 100755 --- a/senza/cli.py +++ b/senza/cli.py @@ -20,7 +20,7 @@ import yaml from botocore.exceptions import ClientError from clickclick import (Action, FloatRange, OutputFormat, choice, error, - fatal_error, info, ok) + warning, fatal_error, info, ok) from clickclick.console import print_table from .arguments import (GLOBAL_OPTIONS, json_output_option, output_option, @@ -46,8 +46,9 @@ from .subcommands.config import cmd_config from .subcommands.root import cli from .templates import get_template_description, get_templates -from .traffic import (change_version_traffic, get_records, - print_version_traffic, resolve_to_ip_addresses) +from .traffic import (change_version_traffic, get_records, get_weights_for_dns, + print_version_traffic, resolve_to_ip_addresses, + get_stack_versions) from .utils import (camel_case_to_underscore, ensure_keys, named_value, pystache_render, get_load_balancer_name) @@ -592,10 +593,13 @@ def health(region, stack_ref, all, output, field, w, watch): help='Ignore failing validation checks') @click.option('--update-if-exists', is_flag=True, help='Updates the the stack if it exists') +@click.option('--ensure-no-traffic', is_flag=True, + help="Don't create a stack if it would automatically receive " + "traffic") @click.option('-t', '--tag', help='Tags to associate with the stack.', multiple=True) @stacktrace_visible_option def create(definition, region, version, parameter, disable_rollback, dry_run, - force, tag, parameter_file, update_if_exists): + force, tag, parameter_file, update_if_exists, ensure_no_traffic): '''Create a new Cloud Formation stack from the given Senza definition file''' region = get_region(region) @@ -612,7 +616,36 @@ def create(definition, region, version, parameter, disable_rollback, dry_run, update_stack = False - with Action('Creating Cloud Formation stack {}..'.format(data['StackName'])) as act: + full_stack_name = data['StackName'] + stack_name, _ = full_stack_name.rsplit('-', 1) # stack name without version + versions = list(get_stack_versions(stack_name, region)) + if versions: + domain_names = versions[0].domain + else: + domain_names = None + + if domain_names: + known_record_weights = get_weights_for_dns(domain_names) + all_records_have_0_weight = sum(known_record_weights.values()) == 0 + if all_records_have_0_weight: + if ensure_no_traffic: + fatal_error("All current stacks have 0 traffic weight. " + "Aborting creation to ensure no traffic to new " + "stack.") + else: + warning("Current stacks have 0 traffic weight. The {} will " + "get a proportional amount of " + "traffic.".format(full_stack_name)) + elif not versions: + if ensure_no_traffic: + fatal_error("There are no other instances of {}. " + "Aborting creation to ensure no traffic " + "to new stack.".format(stack_name)) + else: + warning("There are no other instances of {} so {} will get 100% " + "of traffic.".format(stack_name, full_stack_name)) + + with Action('Creating Cloud Formation stack {}..'.format(full_stack_name)) as act: try: if dry_run: info('**DRY-RUN** {}'.format(data['NotificationARNs'])) diff --git a/tests/test_cli.py b/tests/test_cli.py index e4031a01..d47558b9 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1093,6 +1093,7 @@ def my_client(rtype, *args): monkeypatch.setattr('boto3.client', my_client) monkeypatch.setattr('boto3.resource', my_resource) + monkeypatch.setattr('senza.cli.get_stack_versions', MagicMock(return_value=[])) runner = CliRunner() @@ -1224,6 +1225,7 @@ def my_client(rtype, *args): return MagicMock() monkeypatch.setattr('boto3.client', my_client) + monkeypatch.setattr('senza.cli.get_stack_versions', MagicMock(return_value=[])) runner = CliRunner() From 91746ff532f01a1361e902f279627bf6fd4168c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Santos?= Date: Tue, 16 Jan 2018 11:49:14 +0100 Subject: [PATCH 6/9] Test ensure-no-traffic flag --- tests/test_cli.py | 54 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/tests/test_cli.py b/tests/test_cli.py index d47558b9..716c7132 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1075,6 +1075,60 @@ def test_delete_with_traffic(monkeypatch, boto_resource, boto_client): # noqa: assert 'OK' in result.output +def test_create_fail_traffic(monkeypatch): + cf = MagicMock() + + def my_resource(rtype, *args): + if rtype == 'sns': + sns = MagicMock() + topic = MagicMock(arn='arn:123:my-topic') + sns.topics.all.return_value = [topic] + return sns + return MagicMock() + + def my_client(rtype, *args): + if rtype == 'cloudformation': + return cf + return MagicMock() + + monkeypatch.setattr('boto3.client', my_client) + monkeypatch.setattr('boto3.resource', my_resource) + monkeypatch.setattr('senza.cli.get_stack_versions', MagicMock(return_value=[])) + runner = CliRunner() + + data = {'SenzaComponents': [{'Config': {'Type': 'Senza::Configuration'}}], + 'SenzaInfo': {'OperatorTopicId': 'my-topic', + 'Parameters': [{'MyParam': {'Type': 'String'}}, {'ExtraParam': {'Type': 'String'}}], + 'StackName': 'test'}} + with runner.isolated_filesystem(): + with open('myapp.yaml', 'w') as fd: + yaml.dump(data, fd) + + result = runner.invoke(cli, ['create', 'myapp.yaml', '--dry-run', + '--region=aa-fakeregion-1', + '--ensure-no-traffic', '1', + 'my-param-value', 'extra-param-value'], + catch_exceptions=False) + assert result.exit_code == 1 + + version = MagicMock(domains=['test.example']) + monkeypatch.setattr('senza.cli.get_stack_versions', + MagicMock(return_value=[version])) + monkeypatch.setattr('senza.cli.get_weights_for_dns', + MagicMock(return_value={'test.example': 0})) + + with runner.isolated_filesystem(): + with open('myapp.yaml', 'w') as fd: + yaml.dump(data, fd) + + result = runner.invoke(cli, ['create', 'myapp.yaml', '--dry-run', + '--region=aa-fakeregion-1', + '--ensure-no-traffic', '1', + 'my-param-value', 'extra-param-value'], + catch_exceptions=False) + assert result.exit_code == 1 + + def test_create(monkeypatch): cf = MagicMock() From 846f44def1e116ea68e8b604f00a7e9c805854c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Santos?= Date: Tue, 16 Jan 2018 11:58:06 +0100 Subject: [PATCH 7/9] Test traffic switch with 0 weights --- tests/test_traffic.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/test_traffic.py b/tests/test_traffic.py index a378ba75..5f389a98 100644 --- a/tests/test_traffic.py +++ b/tests/test_traffic.py @@ -81,14 +81,25 @@ def test_get_weights(monkeypatch): type=RecordType.A, weight=None, set_identifier='app-1') - mock_route53.get_records.return_value = [mock_record2] + mock_record3 = MagicMock(name='app1.example.com', + type=RecordType.A, + weight=None, + set_identifier='app-2') + mock_record4 = MagicMock(name='app1.example.com', + type=RecordType.A, + weight=None, + set_identifier='app-3') + mock_route53.get_records.return_value = [mock_record2, + mock_record3, + mock_record4] - all_identifiers = ['app-1', 'app-2', 'app-3'] + all_identifiers = ['app-1', 'app-2', 'app-3', 'app-4'] domains = ['app1.example.com'] assert get_weights(domains, 'app-1', all_identifiers) == ({'app-1': 0, 'app-2': 0, - 'app-3': 0}, - 0, + 'app-3': 0, + 'app-4': 0}, + 2, 0) From 2dc74b7e21cb66c044a42f7f43d90617ad229978 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Santos?= Date: Tue, 16 Jan 2018 11:59:02 +0100 Subject: [PATCH 8/9] Add docstring --- senza/traffic.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/senza/traffic.py b/senza/traffic.py index 9881d3bd..54717a86 100644 --- a/senza/traffic.py +++ b/senza/traffic.py @@ -27,6 +27,9 @@ def get_weights_for_dns(dns_names): + """ + Gets all traffic weights for provided dns_names + """ known_record_weights = {} for dns_name in dns_names: for record in Route53.get_records(name=dns_name): From 7c01fedd53da91a226aafdf35d6150e4dda29ac8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Santos?= Date: Tue, 16 Jan 2018 15:26:42 +0100 Subject: [PATCH 9/9] Check if fatal_error was called --- senza/traffic.py | 2 +- tests/test_cli.py | 5 +++++ tox.ini | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/senza/traffic.py b/senza/traffic.py index 54717a86..e28202a0 100644 --- a/senza/traffic.py +++ b/senza/traffic.py @@ -26,7 +26,7 @@ DNS_ZONE_CACHE = {} -def get_weights_for_dns(dns_names): +def get_weights_for_dns(dns_names: list): """ Gets all traffic weights for provided dns_names """ diff --git a/tests/test_cli.py b/tests/test_cli.py index 716c7132..aef44147 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1094,6 +1094,8 @@ def my_client(rtype, *args): monkeypatch.setattr('boto3.client', my_client) monkeypatch.setattr('boto3.resource', my_resource) monkeypatch.setattr('senza.cli.get_stack_versions', MagicMock(return_value=[])) + mock_error = MagicMock(side_effect=SystemExit) + monkeypatch.setattr('senza.cli.fatal_error', mock_error) runner = CliRunner() data = {'SenzaComponents': [{'Config': {'Type': 'Senza::Configuration'}}], @@ -1110,7 +1112,9 @@ def my_client(rtype, *args): 'my-param-value', 'extra-param-value'], catch_exceptions=False) assert result.exit_code == 1 + assert mock_error.call_count == 1 + mock_error.reset_mock() version = MagicMock(domains=['test.example']) monkeypatch.setattr('senza.cli.get_stack_versions', MagicMock(return_value=[version])) @@ -1127,6 +1131,7 @@ def my_client(rtype, *args): 'my-param-value', 'extra-param-value'], catch_exceptions=False) assert result.exit_code == 1 + assert mock_error.call_count == 1 def test_create(monkeypatch): diff --git a/tox.ini b/tox.ini index da03af68..5012b880 100644 --- a/tox.ini +++ b/tox.ini @@ -4,7 +4,7 @@ select = E,W,F,N,I ignore = W503 [tox] -envlist=py34,py35,flake8 +envlist=py34,py36,flake8 [tox:travis] 3.4=py34