Skip to content
This repository has been archived by the owner on Oct 24, 2020. It is now read-only.

bug: Do not check 'metrics' instance for draining #60

Merged
merged 1 commit into from
Apr 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 2 additions & 9 deletions ardere/aws.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"""AWS Helper Classes"""
import json
import logging
import os
import time
Expand All @@ -9,13 +8,7 @@
import boto3
import botocore
from concurrent.futures import ThreadPoolExecutor
from typing import (
Any,
Dict,
List,
Optional,
Tuple
) # noqa
from typing import Any, Dict, List, Optional, Tuple # noqa

logger = logging.getLogger()
logger.setLevel(logging.INFO)
Expand Down Expand Up @@ -314,7 +307,7 @@ def request_instances(self, instances, security_group_ids,
def locate_metrics_container_ip(self):
# type: () -> Tuple[Optional[str], Optional[str]]
"""Locates the metrics container IP and container instance arn

Returns a tuple of (public_ip, container_arn)

"""
Expand Down
1 change: 0 additions & 1 deletion ardere/scripts/__init__.py
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
#

31 changes: 19 additions & 12 deletions ardere/step_functions.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import json
import logging
import os
import re
Expand All @@ -7,7 +6,6 @@

import boto3
import botocore
import requests
import toml
from marshmallow import (
Schema,
Expand Down Expand Up @@ -279,7 +277,8 @@ def ensure_metric_sources_created(self):
raise CreatingMetricSourceException("Started metric creation")

if not self.ecs.has_finished_metric_creation():
raise CreatingMetricSourceException("Metric creation still running")
raise CreatingMetricSourceException("Metric creation still "
"running")

metric_ip = self.event["influxdb_private_ip"]
self.event["grafana_dashboard"] = "http://{}:3000".format(metric_ip)
Expand Down Expand Up @@ -368,15 +367,23 @@ def check_drained(self):

"""
client = self.boto.client('ecs')
actives = len(
client.list_container_instances(
cluster=self.event["ecs_name"],
maxResults=1,
status="ACTIVE",
).get('containerInstanceArns', []))
if actives:
actives = client.list_container_instances(
cluster=self.event["ecs_name"],
maxResults=1,
status="ACTIVE",
).get('containerInstanceArns', [])
# filter out metric servers
if self.event["metrics_options"]["enabled"]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shutdown plan already locates the metrics service, we should probably determine when we call it the first time and stick it in the event to avoid having to keep searching (and slowing execution time).

metrics = self.ecs.locate_metrics_service()
if metrics:
metrics_arn = metrics.get("serviceArn")
try:
actives.remove(metrics_arn)
except ValueError:
pass
if len(actives):
raise UndrainedInstancesException(
"Still {} active.".format(actives))
"Still active: {}.".format(actives))
draining = len(
client.list_container_instances(
cluster=self.event["ecs_name"],
Expand All @@ -385,5 +392,5 @@ def check_drained(self):
).get('containerInstanceArns', []))
if draining:
raise UndrainedInstancesException(
"Still {} draining.".format(draining))
"Still draining: {}.".format(draining))
return self.event
20 changes: 18 additions & 2 deletions tests/test_step_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ def test_drain_check_active(self):
mock_client.list_container_instances.return_value = {
'containerInstanceArns': [
'Some-Arn-01234567890',
'Metric-Arn-01234567890',
],
"nextToken": "token-8675309"
}
Expand All @@ -325,10 +326,25 @@ def test_drain_check_draining(self):
self.runner.check_drained)

def test_drain_check(self):
# Include a "metrics" instance to show that we ignore it.
self.plan["metrics_options"] = dict(enabled=True)
self.mock_ecs.locate_metrics_service.return_value = {
"deployments": [{
"desiredCount": 1,
"runningCount": 1
}],
"serviceArn": "Metric-Arn-01234567890"
}

mock_client = mock.Mock()
mock_client.list_container_instances.side_effect = [
{},
{}
{ # Actives
'containerInstanceArns': [
'Metric-Arn-01234567890',
],
"nextToken": "token-8675309"
},
{} # Draining
]
self.mock_boto.client.return_value = mock_client
self.runner.check_drained()
Expand Down