Fix spotinst stack details (#538)

fixed get_spotinst_account_data

examples/elastigroup.yaml

@@ -8,7 +8,7 @@ SenzaComponents:
         Type: Senza::StupsAutoConfiguration # auto-detect network setup
 
   - AppServerElastigroup:
-      Type: Spotinst::Elastigroup
+      Type: Senza::Elastigroup
       InstanceType: m3.large
       SpotAlternatives:
           - m3.large

senza/cli.py

@@ -26,6 +26,7 @@
                         fatal_error, info, ok)
 from clickclick.console import print_table
 
+from senza.components.elastigroup import ELASTIGROUP_RESOURCE_TYPE
 from .spotinst.components import elastigroup_api
 from .arguments import (GLOBAL_OPTIONS, json_output_option, output_option,
                         parameter_file_option, region_option,
@@ -119,11 +120,9 @@
 
 SENZA_KMS_PREFIX = 'senza:kms:'
 
-ELASTIGROUP_TYPE = 'Custom::elastigroup'
-
 AUTO_SCALING_GROUP_TYPE = 'AWS::AutoScaling::AutoScalingGroup'
 
-VALID_AUTO_SCALING_GROUPS = [AUTO_SCALING_GROUP_TYPE, ELASTIGROUP_TYPE]
+VALID_AUTO_SCALING_GROUPS = [AUTO_SCALING_GROUP_TYPE, ELASTIGROUP_RESOURCE_TYPE]
 
 
 def filter_output_columns(output_columns, filter_columns):
@@ -1501,7 +1500,7 @@ def patch(stack_ref, region, image, instance_type, user_data):
 
     stacks = get_stacks(stack_refs, region)
     for group in get_auto_scaling_groups_and_elasti_groups(stacks, region):
-        if group['type'] == ELASTIGROUP_TYPE:
+        if group['type'] == ELASTIGROUP_RESOURCE_TYPE:
             patch_spotinst_elastigroup(properties, group['resource_id'], region, group['stack_name'])
         elif group['type'] == AUTO_SCALING_GROUP_TYPE:
             patch_aws_asg(properties, region, asg, group['resource_id'])
@@ -1560,7 +1559,7 @@ def respawn_instances(stack_ref, inplace, force, batch_size_percentage, region):
     for group in get_auto_scaling_groups_and_elasti_groups(stacks, region):
         if group['type'] == AUTO_SCALING_GROUP_TYPE:
             respawn.respawn_auto_scaling_group(group['resource_id'], region, inplace=inplace, force=force)
-        elif group['type'] == ELASTIGROUP_TYPE:
+        elif group['type'] == ELASTIGROUP_RESOURCE_TYPE:
             respawn.respawn_elastigroup(group['resource_id'], group['stack_name'], region, batch_size_percentage)
 
 
@@ -1589,7 +1588,7 @@ def scale(stack_ref, region, desired_capacity, force):
     for group in get_auto_scaling_groups_and_elasti_groups(stacks, region):
         if group['type'] == AUTO_SCALING_GROUP_TYPE:
             scale_auto_scaling_group(asg, group['resource_id'], desired_capacity)
-        elif group['type'] == ELASTIGROUP_TYPE:
+        elif group['type'] == ELASTIGROUP_RESOURCE_TYPE:
             scale_elastigroup(group['resource_id'], group['stack_name'], desired_capacity, region)
 
 

senza/components/elastigroup.py

@@ -17,6 +17,7 @@
 from senza.utils import ensure_keys
 from senza.spotinst import MissingSpotinstAccount
 
+ELASTIGROUP_RESOURCE_TYPE = 'Custom::elastigroup'
 SPOTINST_LAMBDA_FORMATION_ARN = 'arn:aws:lambda:{}:178579023202:function:spotinst-cloudformation'
 SPOTINST_API_URL = 'https://api.spotinst.io'
 ELASTIGROUP_DEFAULT_STRATEGY = {
@@ -66,7 +67,7 @@ def component_elastigroup(definition, configuration, args, info, force, account_
     access_token = _extract_spotinst_access_token(definition)
     config_name = configuration["Name"]
     definition["Resources"][config_name] = {
-        "Type": "Custom::elastigroup",
+        "Type": ELASTIGROUP_RESOURCE_TYPE,
         "Properties": {
             "ServiceToken": create_service_token(args.region),
             "accessToken": access_token,
@@ -76,7 +77,7 @@ def component_elastigroup(definition, configuration, args, info, force, account_
     }
 
     if "SpotPrice" in configuration:
-        print("warning: SpotPrice is ignored when using Spotinst::Elastigroup", file=sys.stderr)
+        print("warning: SpotPrice is ignored when using Senza::Elastigroup", file=sys.stderr)
     return definition
 
 
@@ -495,7 +496,7 @@ def _extract_spotinst_access_token(definition: dict):
     """
     extract the provided access token
     """
-    return definition["Mappings"]["Senza"]["Info"]["SpotinstAccessToken"]
+    return definition["Mappings"]["Senza"]["Info"].pop("SpotinstAccessToken")
 
 
 def extract_spotinst_account_id(access_token: str, definition: dict, account_info):

senza/spotinst/components/elastigroup_api.py

@@ -1,10 +1,12 @@
 '''
 Wrapper methods for ElastiGroup's API
 '''
+import click
 import requests
 import json
 import boto3
 
+from senza.components.elastigroup import ELASTIGROUP_RESOURCE_TYPE
 
 SPOTINST_API_URL = 'https://api.spotinst.io'
 
@@ -22,16 +24,22 @@ def __init__(self, account_id, access_token):
 
 
 def get_spotinst_account_data(region, stack_name):
-    '''
-    Extracts required parameters required to access SpotInst API
-    '''
+    """
+    Extracts the Spotinst API access token and cloud account ID required to use the SpotInst API
+    It returns those parameters from the first resource of Type ``Custom::elastigroup``
+    found in the stack with the name and region provided as arguments
+    """
     cf = boto3.client('cloudformation', region)
     template = cf.get_template(StackName=stack_name)['TemplateBody']
 
-    spotinst_token = template['Mappings']['Senza']['Info']['SpotinstAccessToken']
-    spotinst_account_id = template['Resources']['AppServerConfig']['Properties']['accountId']
+    resources = template.get('Resources', [])
+    for name, resource in resources.items():
+        if resource.get("Type", None) == ELASTIGROUP_RESOURCE_TYPE:
+            spotinst_token = resource['Properties']['accessToken']
+            spotinst_account_id = resource['Properties']['accountId']
+            return SpotInstAccountData(spotinst_account_id, spotinst_token)
 
-    return SpotInstAccountData(spotinst_account_id, spotinst_token)
+    raise click.Abort()
 
 
 def update_elastigroup(body, elastigroup_id, spotinst_account_data):

tests/test_cli.py

@@ -13,6 +13,8 @@
 import yaml
 import base64
 from click.testing import CliRunner
+
+from senza.components.elastigroup import ELASTIGROUP_RESOURCE_TYPE
 from senza.aws import SenzaStackSummary
 from senza.cli import (KeyValParamType, StackReference,
                        all_with_version, create_cf_template, failure_event,
@@ -41,7 +43,7 @@ def test_invalid_definition():
 
         result = runner.invoke(cli, ['print', 'myapp.yaml', '--region=aa-fakeregion-1', '123'], catch_exceptions=False)
 
-    assert 'Error: Invalid value for "definition"' in result.output
+    assert 'Error: Invalid value for "DEFINITION"' in result.output
 
 
 def test_file_not_found():
@@ -910,6 +912,7 @@ def my_client(rtype, *args):
 
     assert 'test-stack 1' in result.output
 
+
 def test_list_version(monkeypatch):
     def my_resource(rtype, *args):
         return MagicMock()
@@ -938,6 +941,7 @@ def my_client(rtype, *args):
     assert '1' in result.output
     assert 'test-stack' not in result.output
 
+
 def test_images(monkeypatch):
     def my_resource(rtype, *args):
         if rtype == 'ec2':
@@ -1635,11 +1639,11 @@ def test_respawn(monkeypatch):
     boto3.list_stacks.return_value = {'StackSummaries': [{'StackName': 'myapp-1',
                                                           'CreationTime': '2016-06-14'}]}
     boto3.describe_stack_resources.return_value = {'StackResources': [
-                                                       {
-                                                           'ResourceType': 'AWS::AutoScaling::AutoScalingGroup',
-                                                           'PhysicalResourceId': 'myasg',
-                                                           'StackName': 'myapp-1'
-                                                       }]}
+        {
+            'ResourceType': 'AWS::AutoScaling::AutoScalingGroup',
+            'PhysicalResourceId': 'myasg',
+            'StackName': 'myapp-1'
+        }]}
     monkeypatch.setattr('senza.respawn.respawn_auto_scaling_group', lambda *args, **kwargs: None)
     runner = CliRunner()
     runner.invoke(cli, ['respawn', 'myapp', '1', '--region=aa-fakeregion-1'],
@@ -1654,7 +1658,7 @@ def test_respawn_elastigroup(monkeypatch):
 
     elastigroup_id = 'myelasti'
     boto3.describe_stack_resources.return_value = {'StackResources':
-                                                       [{'ResourceType': 'Custom::elastigroup',
+                                                       [{'ResourceType': ELASTIGROUP_RESOURCE_TYPE,
                                                          'PhysicalResourceId': elastigroup_id,
                                                          'StackName': 'myapp-1'}]}
 
@@ -1676,9 +1680,9 @@ def test_scale(monkeypatch):
     boto3.list_stacks.return_value = {'StackSummaries': [{'StackName': 'myapp-1',
                                                           'CreationTime': '2016-06-14'}]}
     boto3.describe_stack_resources.return_value = {'StackResources': [
-                                                       {'ResourceType': 'AWS::AutoScaling::AutoScalingGroup',
-                                                        'PhysicalResourceId': 'myasg',
-                                                        'StackName': 'myapp-1'}]}
+        {'ResourceType': 'AWS::AutoScaling::AutoScalingGroup',
+         'PhysicalResourceId': 'myasg',
+         'StackName': 'myapp-1'}]}
     # NOTE: we are using invalid MinSize (< capacity) here to get one more line covered ;-)
     group = {'AutoScalingGroupName': 'myasg', 'DesiredCapacity': 1, 'MinSize': 3, 'MaxSize': 1}
     boto3.describe_auto_scaling_groups.return_value = {'AutoScalingGroups': [group]}
@@ -1696,22 +1700,18 @@ def test_scale_elastigroup(monkeypatch):
     boto3.list_stacks.return_value = {'StackSummaries': [{'StackName': 'myapp-1',
                                                           'CreationTime': '2016-06-14'}]}
     boto3.describe_stack_resources.return_value = {'StackResources':
-                                                       [{'ResourceType': 'Custom::elastigroup',
+                                                       [{'ResourceType': ELASTIGROUP_RESOURCE_TYPE,
                                                          'PhysicalResourceId': elastigroup_id,
                                                          'StackName': 'myapp-1'}]}
     boto3.get_template.return_value = {
         'TemplateBody': {
-            'Mappings': {
-                'Senza': {
-                    'Info': {
-                        'SpotinstAccessToken': 'faketoken'
-                    }
-                }
-            },
+            'Mappings': {'Senza': {'Info': {}}},
             'Resources': {
                 'AppServerConfig': {
+                    'Type': ELASTIGROUP_RESOURCE_TYPE,
                     'Properties': {
-                        'accountId': spotinst_account_id
+                        'accountId': spotinst_account_id,
+                        'accessToken': 'faketoken',
                     }
                 }
             }
@@ -2067,7 +2067,7 @@ def test_traffic_fallback_route53api(monkeypatch, boto_client, boto_resource):
     referenced_stacks = [
         SenzaStackSummary({'StackName': s.name, 'StackStatus': 'UPDATE_COMPLETE'})
         for s in stacks
-        ]
+    ]
     monkeypatch.setattr('senza.cli.get_stacks', MagicMock(name="fake_get_stacks", return_value=referenced_stacks))
 
     def _record(dns_identifier, weight):

tests/test_elastigroup_api.py

@@ -1,9 +1,14 @@
+import click
 import responses
+from mock import mock
+from pytest import raises
+
+from senza.components.elastigroup import ELASTIGROUP_RESOURCE_TYPE
 from senza.spotinst.components.elastigroup_api import update_capacity, get_elastigroup, patch_elastigroup, deploy, \
-    deploy_status, SPOTINST_API_URL, SpotInstAccountData
+    deploy_status, SPOTINST_API_URL, SpotInstAccountData, get_spotinst_account_data
 
 
-def test_update_capacity(monkeypatch):
+def test_update_capacity():
     update = {
         'response': {
             'items': [{
@@ -36,7 +41,7 @@ def test_update_capacity(monkeypatch):
         assert update_response['capacity']['target'] == 3
 
 
-def test_get_elastigroup(monkeypatch):
+def test_get_elastigroup():
     group = {
         'response': {
             'items': [{
@@ -60,7 +65,7 @@ def test_get_elastigroup(monkeypatch):
         assert group['name'] == 'my-app-1'
 
 
-def test_patch_elastigroup(monkeypatch):
+def test_patch_elastigroup():
     patch = {
         'ImageId': 'image-foo',
         'InstanceType': 'm1.micro',
@@ -99,7 +104,7 @@ def test_patch_elastigroup(monkeypatch):
         assert patch_response['compute']['launchSpecification']['userData'] == 'user-data-value'
 
 
-def test_deploy(monkeypatch):
+def test_deploy():
     response_json = {
         "response": {
             "items": [
@@ -133,7 +138,7 @@ def test_deploy(monkeypatch):
         assert deploy_response['numOfBatches'] == 1
 
 
-def test_deploy_status(monkeypatch):
+def test_deploy_status():
     deploy_id = 'deploy-id-x'
     response_json = {
         "response": {
@@ -168,3 +173,45 @@ def test_deploy_status(monkeypatch):
         assert deploy_status_response['numOfBatches'] == 20
         assert deploy_status_response['progress']['value'] == 65
 
+
+def test_get_spotinst_account_data():
+    template = {
+        "TemplateBody": {
+            "Mappings": {"Senza": {"Info": {"dont": "care"}}},
+            "Resources": {
+                "FakeResource1": {"Type": "Fake"},
+                "TheOneWeCare": {
+                    "Properties": {
+                        "accessToken": "fake-token",
+                        "accountId": "act-1234",
+                        "group": {"dont": "care"}
+                    },
+                    "Type": ELASTIGROUP_RESOURCE_TYPE
+                },
+                "FakeResource2": {"Type": "Fake"},
+            }
+        }
+    }
+
+    with mock.patch('boto3.client') as MockHelper:
+        MockHelper.return_value.get_template.return_value = template
+        account_data = get_spotinst_account_data('fake-region', 'fake-stack-name')
+        assert account_data.account_id == 'act-1234'
+        assert account_data.access_token == 'fake-token'
+
+
+def test_get_spotinst_account_data_failure():
+    template = {
+        "TemplateBody": {
+            "Mappings": {"Senza": {"Info": {"dont": "care"}}},
+            "Resources": {
+                "FakeResource1": {"Type": "Fake"},
+                "FakeResource2": {"Type": "Fake"},
+            }
+        }
+    }
+
+    with mock.patch('boto3.client') as MockHelper:
+        MockHelper.return_value.get_template.return_value = template
+        with raises(click.Abort, message="Expecting click.Abort error"):
+            get_spotinst_account_data('fake-region', 'fake-stack-name')