Improve traffic handling

Pipfile

@@ -0,0 +1,30 @@
+[[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]
+
+tox = "*"
+
+
+[requires]
+
+python_version = "3"

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']))

senza/traffic.py

@@ -26,14 +26,10 @@
 DNS_ZONE_CACHE = {}
 
 
-def get_weights(dns_names: list, identifier: str, all_identifiers) -> ({str: int}, int, int):
+def get_weights_for_dns(dns_names):
     """
-    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.
+    Gets all traffic weights for provided dns_names
     """
-    partial_count = 0
-    partial_sum = 0
     known_record_weights = {}
     for dns_name in dns_names:
         for record in Route53.get_records(name=dns_name):
@@ -47,12 +43,33 @@ 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
+    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:
+        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 +137,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

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()
 
@@ -1093,6 +1147,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 +1279,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()
 

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)