From 0e157dabacd5720a8a5d786daf567c69a188b8be Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Sun, 1 Oct 2017 09:04:05 +0200 Subject: [PATCH 01/24] Add flag --no-deregister to prevent deregistration of old task definition --- ecs_deploy/cli.py | 18 ++++++++++++------ ecs_deploy/ecs.py | 4 +++- tests/test_cli.py | 13 +++++++++++++ tests/test_ecs.py | 7 +++++++ 4 files changed, 35 insertions(+), 7 deletions(-) diff --git a/ecs_deploy/cli.py b/ecs_deploy/cli.py index 1704bdf..8770451 100644 --- a/ecs_deploy/cli.py +++ b/ecs_deploy/cli.py @@ -64,9 +64,11 @@ def get_client(access_key_id, secret_access_key, region, profile): help='User who executes the deployment (used for recording)') @click.option('--diff/--no-diff', default=True, help='Print what values were changed in the task definition') +@click.option('--deregister/--no-deregister', default=True, + help='Deregister (default) or keep the old task definition') def deploy(cluster, service, tag, image, command, env, role, task, region, access_key_id, secret_access_key, profile, timeout, newrelic_apikey, - newrelic_appid, comment, user, ignore_warnings, diff): + newrelic_appid, comment, user, ignore_warnings, diff, deregister): """ Redeploy or modify a service. @@ -101,14 +103,18 @@ def deploy(cluster, service, tag, image, command, env, role, task, region, new_td = deployment.update_task_definition(td) click.secho( - 'Successfully created revision: %d' % new_td.revision, - fg='green' - ) - click.secho( - 'Successfully deregistered revision: %d\n' % td.revision, + 'Successfully created revision: %d\n' % new_td.revision, fg='green' ) + if deregister: + click.secho('Deregister old task definition revision') + deployment.deregister_task_definition(td) + click.secho( + 'Successfully deregistered revision: %d\n' % td.revision, + fg='green' + ) + record_deployment(tag, newrelic_apikey, newrelic_appid, comment, user) click.secho('Updating service') diff --git a/ecs_deploy/ecs.py b/ecs_deploy/ecs.py index 6b47673..d3a803b 100644 --- a/ecs_deploy/ecs.py +++ b/ecs_deploy/ecs.py @@ -375,9 +375,11 @@ def update_task_definition(self, task_definition): additional_properties=task_definition.additional_properties ) new_task_definition = EcsTaskDefinition(**response[u'taskDefinition']) - self._client.deregister_task_definition(task_definition.arn) return new_task_definition + def deregister_task_definition(self, task_definition): + self._client.deregister_task_definition(task_definition.arn) + def update_service(self, service): response = self._client.update_service( cluster=service.cluster, diff --git a/tests/test_cli.py b/tests/test_cli.py index f576319..931e6d0 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -70,6 +70,19 @@ def test_deploy(get_client, runner): assert u"Updating task definition" not in result.output +@patch('ecs_deploy.cli.get_client') +def test_deploy_without_deregister(get_client, runner): + get_client.return_value = EcsTestClient('acces_key', 'secret_key') + result = runner.invoke(cli.deploy, (CLUSTER_NAME, SERVICE_NAME, '--no-deregister')) + assert result.exit_code == 0 + assert not result.exception + assert u'Successfully created revision: 2' in result.output + assert u'Successfully deregistered revision: 1' not in result.output + assert u'Successfully changed task definition to: test-task:2' in result.output + assert u'Deployment successful' in result.output + assert u"Updating task definition" not in result.output + + @patch('ecs_deploy.cli.get_client') def test_deploy_with_role_arn(get_client, runner): get_client.return_value = EcsTestClient('acces_key', 'secret_key') diff --git a/tests/test_ecs.py b/tests/test_ecs.py index eeb8e41..c0bb268 100644 --- a/tests/test_ecs.py +++ b/tests/test_ecs.py @@ -586,6 +586,13 @@ def test_update_task_definition(client, task_definition): u'unknownProperty': u'lorem-ipsum' } ) + + +@patch.object(EcsClient, '__init__') +def test_deregister_task_definition(client, task_definition): + action = EcsAction(client, u'test-cluster', u'test-service') + action.deregister_task_definition(task_definition) + client.deregister_task_definition.assert_called_once_with( task_definition.arn ) From 7a8fce2f7aed134144bf0902b790f0b476771097 Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Sun, 1 Oct 2017 09:57:36 +0200 Subject: [PATCH 02/24] Refactor cli.py: extract sub tasks into separate functions --- ecs_deploy/cli.py | 76 ++++++++++++++++++++++++++++++----------------- 1 file changed, 49 insertions(+), 27 deletions(-) diff --git a/ecs_deploy/cli.py b/ecs_deploy/cli.py index 8770451..9510c1d 100644 --- a/ecs_deploy/cli.py +++ b/ecs_deploy/cli.py @@ -85,12 +85,7 @@ def deploy(cluster, service, tag, image, command, env, role, task, region, client = get_client(access_key_id, secret_access_key, region, profile) deployment = DeployAction(client, cluster, service) - if task: - td = deployment.get_task_definition(task) - click.secho('Deploying based on task definition: %s' % task) - else: - td = deployment.get_current_task_definition(deployment.service) - + td = get_task_definition(deployment, task) td.set_images(tag, **{key: value for (key, value) in image}) td.set_commands(**{key: value for (key, value) in command}) td.set_environment(env) @@ -99,28 +94,9 @@ def deploy(cluster, service, tag, image, command, env, role, task, region, if diff: print_diff(td) - click.secho('Creating new task definition revision') - new_td = deployment.update_task_definition(td) - - click.secho( - 'Successfully created revision: %d\n' % new_td.revision, - fg='green' - ) - - if deregister: - click.secho('Deregister old task definition revision') - deployment.deregister_task_definition(td) - click.secho( - 'Successfully deregistered revision: %d\n' % td.revision, - fg='green' - ) - + new_task_definition = create_task_definition(deployment, td) record_deployment(tag, newrelic_apikey, newrelic_appid, comment, user) - - click.secho('Updating service') - deployment.deploy(new_td) - click.secho('Successfully changed task definition to: %s:%s\n' % - (new_td.family, new_td.revision), fg='green') + deploy_task_definition(deployment, new_task_definition) wait_for_finish( action=deployment, @@ -131,6 +107,9 @@ def deploy(cluster, service, tag, image, command, env, role, task, region, ignore_warnings=ignore_warnings ) + if deregister: + deregister_task_definition(deployment, td) + except Exception as e: click.secho('%s\n' % str(e), fg='red', err=True) exit(1) @@ -277,6 +256,49 @@ def wait_for_finish(action, timeout, title, success_message, failure_message, click.secho('\n%s\n' % success_message, fg='green') +def deploy_task_definition(deployment, task_definition): + click.secho('Updating service') + deployment.deploy(task_definition) + + message = 'Successfully changed task definition to: %s:%s\n' % ( + task_definition.family, + task_definition.revision + ) + + click.secho(message, fg='green') + + +def get_task_definition(action, task): + if task: + task_definition = action.get_task_definition(task) + click.secho('Deploying based on task definition: %s' % task) + else: + task_definition = action.get_current_task_definition(action.service) + + return task_definition + + +def create_task_definition(action, task_definition): + click.secho('Creating new task definition revision') + new_td = action.update_task_definition(task_definition) + + click.secho( + 'Successfully created revision: %d\n' % new_td.revision, + fg='green' + ) + + return new_td + + +def deregister_task_definition(action, task_definition): + click.secho('Deregister old task definition revision') + action.deregister_task_definition(task_definition) + click.secho( + 'Successfully deregistered revision: %d\n' % task_definition.revision, + fg='green' + ) + + def record_deployment(revision, api_key, app_id, comment, user): api_key = getenv('NEW_RELIC_API_KEY', api_key) app_id = getenv('NEW_RELIC_APP_ID', app_id) From 9dde963ab20e9104b8217ea526cc3f8cd7e6194f Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Sun, 1 Oct 2017 11:17:30 +0200 Subject: [PATCH 03/24] Only sleep if still waiting for finish --- ecs_deploy/cli.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ecs_deploy/cli.py b/ecs_deploy/cli.py index 9510c1d..568d132 100644 --- a/ecs_deploy/cli.py +++ b/ecs_deploy/cli.py @@ -234,7 +234,6 @@ def wait_for_finish(action, timeout, title, success_message, failure_message, inspected_until = None while waiting and datetime.now() < waiting_timeout: click.secho('.', nl=False) - sleep(1) service = action.get_service() inspected_until = inspect_errors( service=service, @@ -245,6 +244,9 @@ def wait_for_finish(action, timeout, title, success_message, failure_message, ) waiting = not action.is_deployed(service) + if waiting: + sleep(1) + inspect_errors( service=service, failure_message=failure_message, From d012ce6d9c4481aa83c33eed68456013e6459070 Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Sun, 1 Oct 2017 13:45:40 +0200 Subject: [PATCH 04/24] Rollback to previous task definition revision of deployment failure --- ecs_deploy/cli.py | 74 +++++++++++++++++++++++++++++++++++++---------- tests/test_cli.py | 36 +++++++++++++++++++++++ 2 files changed, 94 insertions(+), 16 deletions(-) diff --git a/ecs_deploy/cli.py b/ecs_deploy/cli.py index 568d132..0e298be 100644 --- a/ecs_deploy/cli.py +++ b/ecs_deploy/cli.py @@ -63,12 +63,18 @@ def get_client(access_key_id, secret_access_key, region, profile): @click.option('--user', required=False, help='User who executes the deployment (used for recording)') @click.option('--diff/--no-diff', default=True, - help='Print what values were changed in the task definition') + help='Print which values were changed in the task definition ' + '(default: --diff)') @click.option('--deregister/--no-deregister', default=True, - help='Deregister (default) or keep the old task definition') + help='Deregister or keep the old task definition ' + '(default: --deregister)') +@click.option('--rollback/--no-rollback', default=True, + help='Rollback to previous revision, if deployment failed ' + '(default: --rollback)') def deploy(cluster, service, tag, image, command, env, role, task, region, access_key_id, secret_access_key, profile, timeout, newrelic_apikey, - newrelic_appid, comment, user, ignore_warnings, diff, deregister): + newrelic_appid, comment, user, ignore_warnings, diff, deregister, + rollback): """ Redeploy or modify a service. @@ -94,22 +100,33 @@ def deploy(cluster, service, tag, image, command, env, role, task, region, if diff: print_diff(td) - new_task_definition = create_task_definition(deployment, td) - record_deployment(tag, newrelic_apikey, newrelic_appid, comment, user) - deploy_task_definition(deployment, new_task_definition) + new_td = create_task_definition(deployment, td) - wait_for_finish( - action=deployment, - timeout=timeout, - title='Deploying task definition', - success_message='Deployment successful', - failure_message='Deployment failed', - ignore_warnings=ignore_warnings - ) + try: + deploy_task_definition(deployment, new_td) + + wait_for_finish( + action=deployment, + timeout=timeout, + title='Deploying new task definition', + success_message='Deployment successful', + failure_message='Deployment failed', + ignore_warnings=ignore_warnings + ) + + except TaskPlacementError as e: + if rollback: + click.secho('%s\n' % str(e), fg='red', err=True) + rollback_task_definition(deployment, td, new_td) + exit(1) + else: + raise if deregister: deregister_task_definition(deployment, td) + record_deployment(tag, newrelic_apikey, newrelic_appid, comment, user) + except Exception as e: click.secho('%s\n' % str(e), fg='red', err=True) exit(1) @@ -273,9 +290,11 @@ def deploy_task_definition(deployment, task_definition): def get_task_definition(action, task): if task: task_definition = action.get_task_definition(task) - click.secho('Deploying based on task definition: %s' % task) else: task_definition = action.get_current_task_definition(action.service) + task = task_definition.family_revision + + click.secho('Deploying based on task definition: %s\n' % task) return task_definition @@ -293,7 +312,7 @@ def create_task_definition(action, task_definition): def deregister_task_definition(action, task_definition): - click.secho('Deregister old task definition revision') + click.secho('Deregister task definition revision') action.deregister_task_definition(task_definition) click.secho( 'Successfully deregistered revision: %d\n' % task_definition.revision, @@ -301,6 +320,28 @@ def deregister_task_definition(action, task_definition): ) +def rollback_task_definition(deployment, old, new, timeout=600): + click.secho( + 'Rolling back to task definition: %s\n' % old.family_revision, + fg='yellow', + ) + deploy_task_definition(deployment, old) + wait_for_finish( + action=deployment, + timeout=timeout, + title='Deploying previous task definition', + success_message='Rollback successful', + failure_message='Rollback failed. Please check ECS Console', + ignore_warnings=False + ) + deregister_task_definition(deployment, new) + + click.secho( + 'Deployment failed, but service has been rolled back to previous ' + 'task definition: %s\n' % old.family_revision, fg='yellow', err=True + ) + + def record_deployment(revision, api_key, app_id, comment, user): api_key = getenv('NEW_RELIC_API_KEY', api_key) app_id = getenv('NEW_RELIC_APP_ID', app_id) @@ -365,6 +406,7 @@ def inspect_errors(service, failure_message, ignore_warnings, since, timeout): if timeout: error = True failure_message += ' (timeout)' + click.secho('') if error: raise TaskPlacementError(failure_message) diff --git a/tests/test_cli.py b/tests/test_cli.py index 931e6d0..b86d2a3 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -63,6 +63,7 @@ def test_deploy(get_client, runner): result = runner.invoke(cli.deploy, (CLUSTER_NAME, SERVICE_NAME)) assert result.exit_code == 0 assert not result.exception + assert u"Deploying based on task definition: test-task:1" in result.output assert u'Successfully created revision: 2' in result.output assert u'Successfully deregistered revision: 1' in result.output assert u'Successfully changed task definition to: test-task:2' in result.output @@ -76,6 +77,7 @@ def test_deploy_without_deregister(get_client, runner): result = runner.invoke(cli.deploy, (CLUSTER_NAME, SERVICE_NAME, '--no-deregister')) assert result.exit_code == 0 assert not result.exception + assert u"Deploying based on task definition: test-task:1" in result.output assert u'Successfully created revision: 2' in result.output assert u'Successfully deregistered revision: 1' not in result.output assert u'Successfully changed task definition to: test-task:2' in result.output @@ -89,6 +91,7 @@ def test_deploy_with_role_arn(get_client, runner): result = runner.invoke(cli.deploy, (CLUSTER_NAME, SERVICE_NAME, '-r', 'arn:new:role')) assert result.exit_code == 0 assert not result.exception + assert u"Deploying based on task definition: test-task:1" in result.output assert u'Successfully created revision: 2' in result.output assert u'Successfully deregistered revision: 1' in result.output assert u'Successfully changed task definition to: test-task:2' in result.output @@ -103,6 +106,7 @@ def test_deploy_new_tag(get_client, runner): result = runner.invoke(cli.deploy, (CLUSTER_NAME, SERVICE_NAME, '-t', 'latest')) assert result.exit_code == 0 assert not result.exception + assert u"Deploying based on task definition: test-task:1" in result.output assert u"Updating task definition" in result.output assert u'Changed image of container "webserver" to: "webserver:latest" (was: "webserver:123")' in result.output assert u'Changed image of container "application" to: "application:latest" (was: "application:123")' in result.output @@ -118,6 +122,7 @@ def test_deploy_one_new_image(get_client, runner): result = runner.invoke(cli.deploy, (CLUSTER_NAME, SERVICE_NAME, '-i', 'application', 'application:latest')) assert result.exit_code == 0 assert not result.exception + assert u"Deploying based on task definition: test-task:1" in result.output assert u"Updating task definition" in result.output assert u'Changed image of container "application" to: "application:latest" (was: "application:123")' in result.output assert u'Successfully created revision: 2' in result.output @@ -133,6 +138,7 @@ def test_deploy_two_new_images(get_client, runner): '-i', 'webserver', 'webserver:latest')) assert result.exit_code == 0 assert not result.exception + assert u"Deploying based on task definition: test-task:1" in result.output assert u"Updating task definition" in result.output assert u'Changed image of container "webserver" to: "webserver:latest" (was: "webserver:123")' in result.output assert u'Changed image of container "application" to: "application:latest" (was: "application:123")' in result.output @@ -148,6 +154,7 @@ def test_deploy_one_new_command(get_client, runner): result = runner.invoke(cli.deploy, (CLUSTER_NAME, SERVICE_NAME, '-c', 'application', 'foobar')) assert result.exit_code == 0 assert not result.exception + assert u"Deploying based on task definition: test-task:1" in result.output assert u"Updating task definition" in result.output assert u'Changed command of container "application" to: "foobar" (was: "run")' in result.output assert u'Successfully created revision: 2' in result.output @@ -166,6 +173,7 @@ def test_deploy_one_new_environment_variable(get_client, runner): assert result.exit_code == 0 assert not result.exception + assert u"Deploying based on task definition: test-task:1" in result.output assert u"Updating task definition" in result.output assert u'Changed environment "foo" of container "application" to: "bar"' in result.output assert u'Changed environment "foo" of container "webserver" to: "baz"' in result.output @@ -184,6 +192,7 @@ def test_deploy_without_changing_environment_value(get_client, runner): assert result.exit_code == 0 assert not result.exception + assert u"Deploying based on task definition: test-task:1" in result.output assert u"Updating task definition" not in result.output assert u'Changed environment' not in result.output assert u'Successfully created revision: 2' in result.output @@ -200,6 +209,7 @@ def test_deploy_without_diff(get_client, runner): assert result.exit_code == 0 assert not result.exception + assert u"Deploying based on task definition: test-task:1" in result.output assert u"Updating task definition" not in result.output assert u'Changed environment' not in result.output assert u'Successfully created revision: 2' in result.output @@ -224,6 +234,7 @@ def test_deploy_ignore_warnings(get_client, runner): assert result.exit_code == 0 assert not result.exception + assert u"Deploying based on task definition: test-task:1" in result.output assert u'Successfully created revision: 2' in result.output assert u'Successfully deregistered revision: 1' in result.output assert u'Successfully changed task definition to: test-task:2' in result.output @@ -232,6 +243,31 @@ def test_deploy_ignore_warnings(get_client, runner): assert u'Deployment successful' in result.output +@patch('ecs_deploy.newrelic.Deployment.deploy') +@patch('ecs_deploy.cli.get_client') +def test_deploy_with_newrelic(get_client, newrelic, runner): + get_client.return_value = EcsTestClient('acces_key', 'secret_key') + result = runner.invoke(cli.deploy, (CLUSTER_NAME, SERVICE_NAME, + '-t', 'my-tag', + '--newrelic-apikey', 'test', + '--newrelic-appid', 'test', + '--comment', 'Lorem Ipsum')) + assert result.exit_code == 0 + assert not result.exception + assert u"Deploying based on task definition: test-task:1" in result.output + assert u'Successfully created revision: 2' in result.output + assert u'Successfully deregistered revision: 1' in result.output + assert u'Successfully changed task definition to: test-task:2' in result.output + assert u'Deployment successful' in result.output + assert u"Recording deployment in New Relic" in result.output + + newrelic.assert_called_once_with( + 'my-tag', + '', + 'Lorem Ipsum' + ) + + @patch('ecs_deploy.newrelic.Deployment.deploy') @patch('ecs_deploy.cli.get_client') def test_deploy_with_newrelic_errors(get_client, deploy, runner): From d8676a4c718495daceafd107be7a90ebf032c363 Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Fri, 13 Oct 2017 07:45:51 +0200 Subject: [PATCH 05/24] Extract version into constant --- ecs_deploy/ecs.py | 3 +++ setup.py | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/ecs_deploy/ecs.py b/ecs_deploy/ecs.py index d3a803b..6072bd1 100644 --- a/ecs_deploy/ecs.py +++ b/ecs_deploy/ecs.py @@ -5,6 +5,9 @@ from dateutil.tz.tz import tzlocal +VERSION = '1.3.1' + + class EcsClient(object): def __init__(self, access_key_id=None, secret_access_key=None, region=None, profile=None): diff --git a/setup.py b/setup.py index e1c1d4e..12cdf34 100644 --- a/setup.py +++ b/setup.py @@ -3,13 +3,15 @@ """ from setuptools import find_packages, setup +from ecs_deploy.ecs import VERSION + dependencies = ['click', 'botocore', 'boto3>=1.4.7', 'future', 'requests'] setup( name='ecs-deploy', - version='1.3.1', + version=VERSION, url='https://github.com/fabfuel/ecs-deploy', - download_url='https://github.com/fabfuel/ecs-deploy/archive/1.3.1.tar.gz', + download_url='https://github.com/fabfuel/ecs-deploy/archive/%s.tar.gz' % VERSION, license='BSD', author='Fabian Fuelling', author_email='pypi@fabfuel.de', From 813ac2d8b6ee54b03e2815d2597769378f6d8c3b Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Fri, 13 Oct 2017 08:07:53 +0200 Subject: [PATCH 06/24] Add troubleshooting section to README --- README.rst | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/README.rst b/README.rst index 4411379..81dce31 100644 --- a/README.rst +++ b/README.rst @@ -253,6 +253,27 @@ Or implicitly via environment variables ``NEW_RELIC_API_KEY`` and ``NEW_RELIC_AP Optionally you can provide an additional comment to the deployment via ``--comment "New feature X"`` and the name of the user who deployed with ``--user john.doe`` +Troubleshooting +--------------- +If the service configuration in ECS is not optimally set, you might be seeing +timeout or other errors during the deployment. + +Timeout +======= +The timeout error means, that AWS ECS takes longer for the full deployment cycle then ecs-deploy is told to wait. The deployment itself still might finish successfully, if there are not other problems with the deployed containers. + +This time includes the full cycle of stopping all old containers and (re)starting all new containers. + +You can increase the time (in seconds) to wait for finishing the deployment via the ``--timeout`` parameter. Different stacks require different timeout values, the default is 300 seconds. + +The overall deployment time depends on different things: +- the type of the application. For example node.js containers tend to take a long time to get stopped. But nginx containers tend to stop immediately, etc. +- are old and new containers able to run in parallel (e.g. using dynamic ports)? +- the deployment options and strategy (Maximum percent > 100)? +- the desired count of running tasks, compared to +- the number of ECS instances in the cluster + + Alternative Implementation -------------------------- There are some other libraries/tools available on GitHub, which also handle the deployment of containers in AWS ECS. If you prefer another language over Python, have a look at these projects: From 60bb35a311b49ae8b0a71998e719d9de0930f5c0 Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Fri, 13 Oct 2017 08:08:38 +0200 Subject: [PATCH 07/24] README: Add blank line to fix list --- README.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/README.rst b/README.rst index 81dce31..eb8dfdf 100644 --- a/README.rst +++ b/README.rst @@ -267,6 +267,7 @@ This time includes the full cycle of stopping all old containers and (re)startin You can increase the time (in seconds) to wait for finishing the deployment via the ``--timeout`` parameter. Different stacks require different timeout values, the default is 300 seconds. The overall deployment time depends on different things: + - the type of the application. For example node.js containers tend to take a long time to get stopped. But nginx containers tend to stop immediately, etc. - are old and new containers able to run in parallel (e.g. using dynamic ports)? - the deployment options and strategy (Maximum percent > 100)? From 7fa472f6c5203324052f89f77cb3dd2e9502bfcb Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Fri, 13 Oct 2017 08:10:48 +0200 Subject: [PATCH 08/24] README: fix typo and reorder text --- README.rst | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/README.rst b/README.rst index eb8dfdf..a223199 100644 --- a/README.rst +++ b/README.rst @@ -260,11 +260,9 @@ timeout or other errors during the deployment. Timeout ======= -The timeout error means, that AWS ECS takes longer for the full deployment cycle then ecs-deploy is told to wait. The deployment itself still might finish successfully, if there are not other problems with the deployed containers. +The timeout error means, that AWS ECS takes longer for the full deployment cycle then ecs-deploy is told to wait. The deployment itself still might finish successfully, if there are no other problems with the deployed containers. -This time includes the full cycle of stopping all old containers and (re)starting all new containers. - -You can increase the time (in seconds) to wait for finishing the deployment via the ``--timeout`` parameter. Different stacks require different timeout values, the default is 300 seconds. +You can increase the time (in seconds) to wait for finishing the deployment via the ``--timeout`` parameter. This time includes the full cycle of stopping all old containers and (re)starting all new containers. Different stacks require different timeout values, the default is 300 seconds. The overall deployment time depends on different things: From 66ba99f2fbd0bc550ee6610024e5e31c5922818d Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Fri, 13 Oct 2017 08:15:09 +0200 Subject: [PATCH 09/24] Add link to Troubleshooting section in timeout error message --- ecs_deploy/cli.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ecs_deploy/cli.py b/ecs_deploy/cli.py index 8770451..ec7e46a 100644 --- a/ecs_deploy/cli.py +++ b/ecs_deploy/cli.py @@ -340,7 +340,8 @@ def inspect_errors(service, failure_message, ignore_warnings, since, timeout): if timeout: error = True - failure_message += ' (timeout)' + failure_message += ' due to timeout. Please see: ' \ + 'https://github.com/fabfuel/ecs-deploy#timeout' if error: raise TaskPlacementError(failure_message) From bbdae7ddba184837c30828ab87a638ead7951a27 Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Fri, 13 Oct 2017 08:18:56 +0200 Subject: [PATCH 10/24] Test deployment with timeout --- tests/test_cli.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 931e6d0..75de367 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -261,6 +261,15 @@ def test_deploy_task_definition_arn(get_client, runner): assert u'Deployment successful' in result.output +@patch('ecs_deploy.cli.get_client') +def test_deploy_with_timeout(get_client, runner): + get_client.return_value = EcsTestClient('acces_key', 'secret_key', wait=2) + result = runner.invoke(cli.deploy, (CLUSTER_NAME, SERVICE_NAME, '--timeout', '1')) + assert result.exit_code == 1 + assert u"Deployment failed due to timeout. Please see: " \ + u"https://github.com/fabfuel/ecs-deploy#timeout" in result.output + + @patch('ecs_deploy.cli.get_client') def test_deploy_unknown_task_definition_arn(get_client, runner): get_client.return_value = EcsTestClient('acces_key', 'secret_key') @@ -331,7 +340,8 @@ def test_scale_with_timeout(get_client, runner): get_client.return_value = EcsTestClient('acces_key', 'secret_key', wait=2) result = runner.invoke(cli.scale, (CLUSTER_NAME, SERVICE_NAME, '2', '--timeout', '1')) assert result.exit_code == 1 - assert u"Scaling failed (timeout)" in result.output + assert u"Scaling failed due to timeout. Please see: " \ + u"https://github.com/fabfuel/ecs-deploy#timeout" in result.output @patch('ecs_deploy.cli.get_client') From 95b4321d5dd7e908ae8ea85702cf3b5ec8ff0923 Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Sat, 21 Oct 2017 12:21:37 +0200 Subject: [PATCH 11/24] More explicit error handling --- ecs_deploy/cli.py | 10 +++++----- ecs_deploy/ecs.py | 35 ++++++++++++++++++++++------------- tests/test_cli.py | 27 ++++++++++++++++++++++----- tests/test_ecs.py | 16 +++++++++++----- 4 files changed, 60 insertions(+), 28 deletions(-) diff --git a/ecs_deploy/cli.py b/ecs_deploy/cli.py index ec7e46a..39813ca 100644 --- a/ecs_deploy/cli.py +++ b/ecs_deploy/cli.py @@ -8,8 +8,8 @@ from datetime import datetime, timedelta from ecs_deploy.ecs import DeployAction, ScaleAction, RunAction, EcsClient, \ - TaskPlacementError -from ecs_deploy.newrelic import Deployment + TaskPlacementError, EcsError +from ecs_deploy.newrelic import Deployment, NewRelicException @click.group() @@ -131,7 +131,7 @@ def deploy(cluster, service, tag, image, command, env, role, task, region, ignore_warnings=ignore_warnings ) - except Exception as e: + except (EcsError, NewRelicException) as e: click.secho('%s\n' % str(e), fg='red', err=True) exit(1) @@ -181,7 +181,7 @@ def scale(cluster, service, desired_count, access_key_id, secret_access_key, ignore_warnings=ignore_warnings ) - except Exception as e: + except EcsError as e: click.secho('%s\n' % str(e), fg='red', err=True) exit(1) @@ -241,7 +241,7 @@ def run(cluster, task, count, command, env, region, access_key_id, click.secho('- %s' % started_task['taskArn'], fg='green') click.secho(' ') - except Exception as e: + except EcsError as e: click.secho('%s\n' % str(e), fg='red', err=True) exit(1) diff --git a/ecs_deploy/ecs.py b/ecs_deploy/ecs.py index d3a803b..18e0e6e 100644 --- a/ecs_deploy/ecs.py +++ b/ecs_deploy/ecs.py @@ -436,14 +436,20 @@ def service_name(self): class DeployAction(EcsAction): def deploy(self, task_definition): - self._service.set_task_definition(task_definition) - return self.update_service(self._service) + try: + self._service.set_task_definition(task_definition) + return self.update_service(self._service) + except ClientError as e: + raise EcsError(str(e)) class ScaleAction(EcsAction): def scale(self, desired_count): - self._service.set_desired_count(desired_count) - return self.update_service(self._service) + try: + self._service.set_desired_count(desired_count) + return self.update_service(self._service) + except ClientError as e: + raise EcsError(str(e)) class RunAction(EcsAction): @@ -454,15 +460,18 @@ def __init__(self, client, cluster_name): self.started_tasks = [] def run(self, task_definition, count, started_by): - result = self._client.run_task( - cluster=self._cluster_name, - task_definition=task_definition.family_revision, - count=count, - started_by=started_by, - overrides=dict(containerOverrides=task_definition.get_overrides()) - ) - self.started_tasks = result['tasks'] - return True + try: + result = self._client.run_task( + cluster=self._cluster_name, + task_definition=task_definition.family_revision, + count=count, + started_by=started_by, + overrides=dict(containerOverrides=task_definition.get_overrides()) + ) + self.started_tasks = result['tasks'] + return True + except ClientError as e: + raise EcsError(str(e)) class EcsError(Exception): diff --git a/tests/test_cli.py b/tests/test_cli.py index 75de367..1e923ee 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -210,16 +210,24 @@ def test_deploy_without_diff(get_client, runner): @patch('ecs_deploy.cli.get_client') def test_deploy_with_errors(get_client, runner): - get_client.return_value = EcsTestClient('acces_key', 'secret_key', errors=True) + get_client.return_value = EcsTestClient('acces_key', 'secret_key', deployment_errors=True) result = runner.invoke(cli.deploy, (CLUSTER_NAME, SERVICE_NAME)) assert result.exit_code == 1 assert u"Deployment failed" in result.output assert u"ERROR: Service was unable to Lorem Ipsum" in result.output +@patch('ecs_deploy.cli.get_client') +def test_deploy_with_client_errors(get_client, runner): + get_client.return_value = EcsTestClient('acces_key', 'secret_key', client_errors=True) + result = runner.invoke(cli.deploy, (CLUSTER_NAME, SERVICE_NAME)) + assert result.exit_code == 1 + assert u"Something went wrong" in result.output + + @patch('ecs_deploy.cli.get_client') def test_deploy_ignore_warnings(get_client, runner): - get_client.return_value = EcsTestClient('acces_key', 'secret_key', errors=True) + get_client.return_value = EcsTestClient('acces_key', 'secret_key', deployment_errors=True) result = runner.invoke(cli.deploy, (CLUSTER_NAME, SERVICE_NAME, '--ignore-warnings')) assert result.exit_code == 0 @@ -315,16 +323,24 @@ def test_scale(get_client, runner): @patch('ecs_deploy.cli.get_client') def test_scale_with_errors(get_client, runner): - get_client.return_value = EcsTestClient('acces_key', 'secret_key', errors=True) + get_client.return_value = EcsTestClient('acces_key', 'secret_key', deployment_errors=True) result = runner.invoke(cli.scale, (CLUSTER_NAME, SERVICE_NAME, '2')) assert result.exit_code == 1 assert u"Scaling failed" in result.output assert u"ERROR: Service was unable to Lorem Ipsum" in result.output +@patch('ecs_deploy.cli.get_client') +def test_scale_with_client_errors(get_client, runner): + get_client.return_value = EcsTestClient('acces_key', 'secret_key', client_errors=True) + result = runner.invoke(cli.scale, (CLUSTER_NAME, SERVICE_NAME, '2')) + assert result.exit_code == 1 + assert u"Something went wrong" in result.output + + @patch('ecs_deploy.cli.get_client') def test_scale_ignore_warnings(get_client, runner): - get_client.return_value = EcsTestClient('acces_key', 'secret_key', errors=True) + get_client.return_value = EcsTestClient('acces_key', 'secret_key', deployment_errors=True) result = runner.invoke(cli.scale, (CLUSTER_NAME, SERVICE_NAME, '2', '--ignore-warnings')) assert not result.exception @@ -412,8 +428,9 @@ def test_run_task_without_diff(get_client, runner): @patch('ecs_deploy.cli.get_client') def test_run_task_with_errors(get_client, runner): - get_client.return_value = EcsTestClient('acces_key', 'secret_key', errors=True) + get_client.return_value = EcsTestClient('acces_key', 'secret_key', deployment_errors=True) result = runner.invoke(cli.run, (CLUSTER_NAME, 'test-task')) + assert result.exception assert result.exit_code == 1 assert u"An error occurred (123) when calling the fake_error operation: Something went wrong" in result.output diff --git a/tests/test_ecs.py b/tests/test_ecs.py index c0bb268..b8466b8 100644 --- a/tests/test_ecs.py +++ b/tests/test_ecs.py @@ -740,13 +740,16 @@ def test_run_action_run(client, task_definition): class EcsTestClient(object): - def __init__(self, access_key_id=None, secret_access_key=None, region=None, profile=None, errors=False, wait=0): + def __init__(self, access_key_id=None, secret_access_key=None, region=None, + profile=None, deployment_errors=False, client_errors=False, + wait=0): super(EcsTestClient, self).__init__() self.access_key_id = access_key_id self.secret_access_key = secret_access_key self.region = region self.profile = profile - self.errors = errors + self.deployment_errors = deployment_errors + self.client_errors = client_errors self.wait_until = datetime.now() + timedelta(seconds=wait) def describe_services(self, cluster_name, service_name): @@ -757,7 +760,7 @@ def describe_services(self, cluster_name, service_name): raise ClientError(error_response, u'DescribeServices') if service_name != u'test-service': return {u'services': []} - if self.errors: + if self.deployment_errors: return { u"services": [PAYLOAD_SERVICE_WITH_ERRORS], u"failures": [] @@ -787,7 +790,10 @@ def deregister_task_definition(self, task_definition_arn): return deepcopy(RESPONSE_TASK_DEFINITION) def update_service(self, cluster, service, desired_count, task_definition): - if self.errors: + if self.client_errors: + error = dict(Error=dict(Code=123, Message="Something went wrong")) + raise ClientError(error, 'fake_error') + if self.deployment_errors: return deepcopy(RESPONSE_SERVICE_WITH_ERRORS) return deepcopy(RESPONSE_SERVICE) @@ -796,7 +802,7 @@ def run_task(self, cluster, task_definition, count, started_by, overrides): raise EcsConnectionError(u'Unable to locate credentials. Configure credentials by running "aws configure".') if cluster == 'unknown-cluster': raise EcsConnectionError(u'An error occurred (ClusterNotFoundException) when calling the RunTask operation: Cluster not found.') - if self.errors: + if self.deployment_errors: error = dict(Error=dict(Code=123, Message="Something went wrong")) raise ClientError(error, 'fake_error') return dict(tasks=[dict(taskArn='arn:foo:bar'), dict(taskArn='arn:lorem:ipsum')]) From f14bbaf0fde5db82770ee00a354928072f1c5615 Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Sat, 21 Oct 2017 12:57:01 +0200 Subject: [PATCH 12/24] No rollback by default --- ecs_deploy/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ecs_deploy/cli.py b/ecs_deploy/cli.py index c549d3a..6980ca0 100644 --- a/ecs_deploy/cli.py +++ b/ecs_deploy/cli.py @@ -68,7 +68,7 @@ def get_client(access_key_id, secret_access_key, region, profile): @click.option('--deregister/--no-deregister', default=True, help='Deregister or keep the old task definition ' '(default: --deregister)') -@click.option('--rollback/--no-rollback', default=True, +@click.option('--rollback/--no-rollback', default=False, help='Rollback to previous revision, if deployment failed ' '(default: --rollback)') def deploy(cluster, service, tag, image, command, env, role, task, region, From 6131ac478247cf2a312364f1b2a527bedab13ae1 Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Sat, 21 Oct 2017 13:04:43 +0200 Subject: [PATCH 13/24] Add CLI test with rollback option enabled --- tests/test_cli.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_cli.py b/tests/test_cli.py index 6b27dc9..6109456 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -71,6 +71,25 @@ def test_deploy(get_client, runner): assert u"Updating task definition" not in result.output +@patch('ecs_deploy.cli.get_client') +def test_deploy_with_rollback(get_client, runner): + get_client.return_value = EcsTestClient('acces_key', 'secret_key', wait=2) + result = runner.invoke(cli.deploy, (CLUSTER_NAME, SERVICE_NAME, '--timeout=1', '--rollback')) + + print(result.output) + assert result.exit_code == 1 + assert result.exception + assert u"Deploying based on task definition: test-task:1" in result.output + + assert u"Deployment failed" in result.output + assert u"Rolling back to task definition: test-task:1" in result.output + assert u'Successfully changed task definition to: test-task:1' in result.output + + assert u"Rollback successful" in result.output + assert u'Deployment failed, but service has been rolled back to ' \ + u'previous task definition: test-task:1' in result.output + + @patch('ecs_deploy.cli.get_client') def test_deploy_without_deregister(get_client, runner): get_client.return_value = EcsTestClient('acces_key', 'secret_key') From edd2a185e8385757c3ce431510153ffe77ae96e8 Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Sat, 21 Oct 2017 15:24:11 +0200 Subject: [PATCH 14/24] Do not break click.option decorators for better readability --- ecs_deploy/cli.py | 119 ++++++++++++++-------------------------------- 1 file changed, 35 insertions(+), 84 deletions(-) diff --git a/ecs_deploy/cli.py b/ecs_deploy/cli.py index 6980ca0..96e74ed 100644 --- a/ecs_deploy/cli.py +++ b/ecs_deploy/cli.py @@ -24,57 +24,26 @@ def get_client(access_key_id, secret_access_key, region, profile): @click.command() @click.argument('cluster') @click.argument('service') -@click.option('-t', '--tag', - help='Changes the tag for ALL container images') -@click.option('-i', '--image', type=(str, str), multiple=True, - help='Overwrites the image for a container: ' - ' ') -@click.option('-c', '--command', type=(str, str), multiple=True, - help='Overwrites the command in a container: ' - ' ') -@click.option('-e', '--env', type=(str, str, str), multiple=True, - help='Adds or changes an environment variable: ' - ' ') -@click.option('-r', '--role', type=str, - help='Sets the task\'s role ARN: ') -@click.option('--task', type=str, - help='Task definition to be deployed. Can be a task ARN ' - 'or a task family with optional revision') -@click.option('--region', required=False, - help='AWS region (e.g. eu-central-1)') -@click.option('--access-key-id', required=False, - help='AWS access key id') -@click.option('--secret-access-key', required=False, - help='AWS secret access key') -@click.option('--profile', required=False, - help='AWS configuration profile name') -@click.option('--timeout', required=False, default=300, type=int, - help='Amount of seconds to wait for deployment before ' - 'command fails (default: 300)') -@click.option('--ignore-warnings', is_flag=True, - help='Do not fail deployment on warnings (port already in use ' - 'or insufficient memory/CPU)') -@click.option('--newrelic-apikey', required=False, - help='New Relic API Key for recording the deployment') -@click.option('--newrelic-appid', required=False, - help='New Relic App ID for recording the deployment') -@click.option('--comment', required=False, - help='Description/comment for recording the deployment') -@click.option('--user', required=False, - help='User who executes the deployment (used for recording)') -@click.option('--diff/--no-diff', default=True, - help='Print which values were changed in the task definition ' - '(default: --diff)') -@click.option('--deregister/--no-deregister', default=True, - help='Deregister or keep the old task definition ' - '(default: --deregister)') -@click.option('--rollback/--no-rollback', default=False, - help='Rollback to previous revision, if deployment failed ' - '(default: --rollback)') -def deploy(cluster, service, tag, image, command, env, role, task, region, - access_key_id, secret_access_key, profile, timeout, newrelic_apikey, - newrelic_appid, comment, user, ignore_warnings, diff, deregister, - rollback): +@click.option('-t', '--tag', help='Changes the tag for ALL container images') +@click.option('-i', '--image', type=(str, str), multiple=True, help='Overwrites the image for a container: ') +@click.option('-c', '--command', type=(str, str), multiple=True, help='Overwrites the command in a container: ') +@click.option('-e', '--env', type=(str, str, str), multiple=True, help='Adds or changes an environment variable: ') +@click.option('-r', '--role', type=str, help='Sets the task\'s role ARN: ') +@click.option('--task', type=str, help='Task definition to be deployed. Can be a task ARN or a task family with optional revision') +@click.option('--region', required=False, help='AWS region (e.g. eu-central-1)') +@click.option('--access-key-id', required=False, help='AWS access key id') +@click.option('--secret-access-key', required=False, help='AWS secret access key') +@click.option('--profile', required=False, help='AWS configuration profile name') +@click.option('--timeout', required=False, default=300, type=int, help='Amount of seconds to wait for deployment before command fails (default: 300)') +@click.option('--ignore-warnings', is_flag=True, help='Do not fail deployment on warnings (port already in use or insufficient memory/CPU)') +@click.option('--newrelic-apikey', required=False, help='New Relic API Key for recording the deployment') +@click.option('--newrelic-appid', required=False, help='New Relic App ID for recording the deployment') +@click.option('--comment', required=False, help='Description/comment for recording the deployment') +@click.option('--user', required=False, help='User who executes the deployment (used for recording)') +@click.option('--diff/--no-diff', default=True, help='Print which values were changed in the task definition (default: --diff)') +@click.option('--deregister/--no-deregister', default=True, help='Deregister or keep the old task definition (default: --deregister)') +@click.option('--rollback/--no-rollback', default=False, help='Rollback to previous revision, if deployment failed (default: --rollback)') +def deploy(cluster, service, tag, image, command, env, role, task, region, access_key_id, secret_access_key, profile, timeout, newrelic_apikey, newrelic_appid, comment, user, ignore_warnings, diff, deregister, rollback): """ Redeploy or modify a service. @@ -136,21 +105,13 @@ def deploy(cluster, service, tag, image, command, env, role, task, region, @click.argument('cluster') @click.argument('service') @click.argument('desired_count', type=int) -@click.option('--region', - help='AWS region (e.g. eu-central-1)') -@click.option('--access-key-id', - help='AWS access key id') -@click.option('--secret-access-key', - help='AWS secret access key') -@click.option('--profile', - help='AWS configuration profile name') -@click.option('--timeout', default=300, type=int, - help='AWS configuration profile') -@click.option('--ignore-warnings', is_flag=True, - help='Do not fail deployment on warnings (port already in use ' - 'or insufficient memory/CPU)') -def scale(cluster, service, desired_count, access_key_id, secret_access_key, - region, profile, timeout, ignore_warnings): +@click.option('--region', help='AWS region (e.g. eu-central-1)') +@click.option('--access-key-id', help='AWS access key id') +@click.option('--secret-access-key', help='AWS secret access key') +@click.option('--profile', help='AWS configuration profile name') +@click.option('--timeout', default=300, type=int, help='AWS configuration profile') +@click.option('--ignore-warnings', is_flag=True, help='Do not fail deployment on warnings (port already in use or insufficient memory/CPU)') +def scale(cluster, service, desired_count, access_key_id, secret_access_key, region, profile, timeout, ignore_warnings): """ Scale a service up or down. @@ -186,24 +147,14 @@ def scale(cluster, service, desired_count, access_key_id, secret_access_key, @click.argument('cluster') @click.argument('task') @click.argument('count', required=False, default=1) -@click.option('-c', '--command', type=(str, str), multiple=True, - help='Overwrites the command in a container: ' - ' ') -@click.option('-e', '--env', type=(str, str, str), multiple=True, - help='Adds or changes an environment variable: ' - ' ') -@click.option('--region', - help='AWS region (e.g. eu-central-1)') -@click.option('--access-key-id', - help='AWS access key id') -@click.option('--secret-access-key', - help='AWS secret access key') -@click.option('--profile', - help='AWS configuration profile name') -@click.option('--diff/--no-diff', default=True, - help='Print what values were changed in the task definition') -def run(cluster, task, count, command, env, region, access_key_id, - secret_access_key, profile, diff): +@click.option('-c', '--command', type=(str, str), multiple=True, help='Overwrites the command in a container: ') +@click.option('-e', '--env', type=(str, str, str), multiple=True, help='Adds or changes an environment variable: ') +@click.option('--region', help='AWS region (e.g. eu-central-1)') +@click.option('--access-key-id', help='AWS access key id') +@click.option('--secret-access-key', help='AWS secret access key') +@click.option('--profile', help='AWS configuration profile name') +@click.option('--diff/--no-diff', default=True, help='Print what values were changed in the task definition') +def run(cluster, task, count, command, env, region, access_key_id, secret_access_key, profile, diff): """ Run a one-off task. From 8187f124ef6148bdf5b6e9868c1d03e3aacab881 Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Sun, 22 Oct 2017 08:33:55 +0200 Subject: [PATCH 15/24] Refactor deploy_task_definition(), indlude waiting und deregistration here --- ecs_deploy/cli.py | 44 ++++++++++++++++++++++++++++---------------- tests/test_cli.py | 1 - 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/ecs_deploy/cli.py b/ecs_deploy/cli.py index 96e74ed..4346d18 100644 --- a/ecs_deploy/cli.py +++ b/ecs_deploy/cli.py @@ -72,15 +72,16 @@ def deploy(cluster, service, tag, image, command, env, role, task, region, acces new_td = create_task_definition(deployment, td) try: - deploy_task_definition(deployment, new_td) - - wait_for_finish( - action=deployment, - timeout=timeout, + deploy_task_definition( + deployment=deployment, + task_definition=new_td, title='Deploying new task definition', success_message='Deployment successful', failure_message='Deployment failed', - ignore_warnings=ignore_warnings + timeout=timeout, + deregister=deregister, + previous_task_definition=td, + ignore_warnings=ignore_warnings, ) except TaskPlacementError as e: @@ -91,9 +92,6 @@ def deploy(cluster, service, tag, image, command, env, role, task, region, acces else: raise - if deregister: - deregister_task_definition(deployment, td) - record_deployment(tag, newrelic_apikey, newrelic_appid, comment, user) except (EcsError, NewRelicException) as e: @@ -226,7 +224,9 @@ def wait_for_finish(action, timeout, title, success_message, failure_message, click.secho('\n%s\n' % success_message, fg='green') -def deploy_task_definition(deployment, task_definition): +def deploy_task_definition(deployment, task_definition, title, success_message, + failure_message, timeout, deregister, + previous_task_definition, ignore_warnings): click.secho('Updating service') deployment.deploy(task_definition) @@ -237,6 +237,18 @@ def deploy_task_definition(deployment, task_definition): click.secho(message, fg='green') + wait_for_finish( + action=deployment, + timeout=timeout, + title=title, + success_message=success_message, + failure_message=failure_message, + ignore_warnings=ignore_warnings + ) + + if deregister: + deregister_task_definition(deployment, previous_task_definition) + def get_task_definition(action, task): if task: @@ -276,17 +288,17 @@ def rollback_task_definition(deployment, old, new, timeout=600): 'Rolling back to task definition: %s\n' % old.family_revision, fg='yellow', ) - deploy_task_definition(deployment, old) - wait_for_finish( - action=deployment, - timeout=timeout, + deploy_task_definition( + deployment=deployment, + task_definition=old, title='Deploying previous task definition', success_message='Rollback successful', failure_message='Rollback failed. Please check ECS Console', + timeout=timeout, + deregister=True, + previous_task_definition=new, ignore_warnings=False ) - deregister_task_definition(deployment, new) - click.secho( 'Deployment failed, but service has been rolled back to previous ' 'task definition: %s\n' % old.family_revision, fg='yellow', err=True diff --git a/tests/test_cli.py b/tests/test_cli.py index 6109456..b619871 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -76,7 +76,6 @@ def test_deploy_with_rollback(get_client, runner): get_client.return_value = EcsTestClient('acces_key', 'secret_key', wait=2) result = runner.invoke(cli.deploy, (CLUSTER_NAME, SERVICE_NAME, '--timeout=1', '--rollback')) - print(result.output) assert result.exit_code == 1 assert result.exception assert u"Deploying based on task definition: test-task:1" in result.output From 04d0dc662749c45cc22cd32677979b841c2879e2 Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Sun, 22 Oct 2017 08:59:54 +0200 Subject: [PATCH 16/24] Add --version option --- ecs_deploy/cli.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ecs_deploy/cli.py b/ecs_deploy/cli.py index 4346d18..8c3662f 100644 --- a/ecs_deploy/cli.py +++ b/ecs_deploy/cli.py @@ -8,11 +8,12 @@ from datetime import datetime, timedelta from ecs_deploy.ecs import DeployAction, ScaleAction, RunAction, EcsClient, \ - TaskPlacementError, EcsError + TaskPlacementError, EcsError, VERSION from ecs_deploy.newrelic import Deployment, NewRelicException @click.group() +@click.version_option(version=VERSION, prog_name='ecs-deploy') def ecs(): # pragma: no cover pass From ae4369bc0bec4ce1103d922b0b8d263c4370ca45 Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Sun, 22 Oct 2017 09:00:38 +0200 Subject: [PATCH 17/24] Change rollback help text: default is --no-rollback --- ecs_deploy/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ecs_deploy/cli.py b/ecs_deploy/cli.py index 8c3662f..55180a3 100644 --- a/ecs_deploy/cli.py +++ b/ecs_deploy/cli.py @@ -43,7 +43,7 @@ def get_client(access_key_id, secret_access_key, region, profile): @click.option('--user', required=False, help='User who executes the deployment (used for recording)') @click.option('--diff/--no-diff', default=True, help='Print which values were changed in the task definition (default: --diff)') @click.option('--deregister/--no-deregister', default=True, help='Deregister or keep the old task definition (default: --deregister)') -@click.option('--rollback/--no-rollback', default=False, help='Rollback to previous revision, if deployment failed (default: --rollback)') +@click.option('--rollback/--no-rollback', default=False, help='Rollback to previous revision, if deployment failed (default: --no-rollback)') def deploy(cluster, service, tag, image, command, env, role, task, region, access_key_id, secret_access_key, profile, timeout, newrelic_apikey, newrelic_appid, comment, user, ignore_warnings, diff, deregister, rollback): """ Redeploy or modify a service. From 699fc4ee4407d84e1a2b12ef6a80f8287a206712 Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Sun, 22 Oct 2017 09:12:43 +0200 Subject: [PATCH 18/24] Add boto3 as test dependency requirement --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index cd59364..1404890 100644 --- a/tox.ini +++ b/tox.ini @@ -15,6 +15,7 @@ deps= pytest-mock mock requests + boto3 [testenv:flake8] basepython = python2.7 From 0e05394628cc2fccec86e8606d328f97d41072e7 Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Sun, 22 Oct 2017 09:13:01 +0200 Subject: [PATCH 19/24] Change pytest section in setup.cfg (was deprecated) --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 5f04a8c..cc54d9f 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,7 +1,7 @@ [wheel] universal = 1 -[pytest] +[tool:pytest] testpaths = tests flake8-max-line-length = 120 From 3e76c8f8e9c6edba941d4853fa5080d4c04b90fe Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Sun, 22 Oct 2017 09:13:27 +0200 Subject: [PATCH 20/24] Tox: test with Python 3.6 --- tox.ini | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 1404890..bafa81a 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist=py27, py35, flake8 +envlist=py27, py35, py36, flake8 [testenv:py27] basepython = python2.7 @@ -7,6 +7,9 @@ basepython = python2.7 [testenv:py35] basepython = python3.5 +[testenv:py36] +basepython = python3.6 + [testenv] commands=py.test --cov ecs_deploy {posargs} deps= From b2fce6ae3e7910e5bf452800acec70517929ac44 Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Sun, 22 Oct 2017 09:18:33 +0200 Subject: [PATCH 21/24] Travis: test with Python 3.6 and install boto3 --- .travis.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 9f02b29..eb392e7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,8 @@ language: python python: - "2.7" - "3.5" -install: pip install tox-travis + - "3.5" +install: pip install tox-travis boto3 script: tox after_script: - pip install scrutinizer-ocular From 3e1dbfe819087b320917c04aa79a589d510cafb6 Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Sun, 22 Oct 2017 09:20:18 +0200 Subject: [PATCH 22/24] Travis: fix typo: test with Python 3.6 --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index eb392e7..4b25a86 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,7 @@ language: python python: - "2.7" - "3.5" - - "3.5" + - "3.6" install: pip install tox-travis boto3 script: tox after_script: From 29a6e5d889a7f8a3bd0ac461038a25603ee3b3a0 Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Sun, 22 Oct 2017 09:23:03 +0200 Subject: [PATCH 23/24] Bump version to 1.4.0 --- ecs_deploy/ecs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ecs_deploy/ecs.py b/ecs_deploy/ecs.py index e33cadd..2bbc368 100644 --- a/ecs_deploy/ecs.py +++ b/ecs_deploy/ecs.py @@ -5,7 +5,7 @@ from dateutil.tz.tz import tzlocal -VERSION = '1.3.1' +VERSION = '1.4.0' class EcsClient(object): From fd0ad7098c274cd9b28d4c4abb836f20c5f7be1d Mon Sep 17 00:00:00 2001 From: Fabian Fuelling Date: Sun, 22 Oct 2017 09:34:00 +0200 Subject: [PATCH 24/24] Remove development status "alpha" from setup.py --- setup.py | 1 - 1 file changed, 1 deletion(-) diff --git a/setup.py b/setup.py index 12cdf34..d6dec7b 100644 --- a/setup.py +++ b/setup.py @@ -35,7 +35,6 @@ 'coverage' ], classifiers=[ - 'Development Status :: 3 - Alpha', 'Environment :: Console', 'Intended Audience :: Developers', 'License :: OSI Approved :: BSD License',