From b656e18d20ec7dc796453339f2fcd7d0c7c76c3f Mon Sep 17 00:00:00 2001 From: Rokibul Islam Date: Thu, 4 May 2023 13:09:20 +0530 Subject: [PATCH 1/3] fix(update_service): fixes TaskDefinition inactive exception - Added a new tag in task definition "task_definition_source" to identify if task definition was created with cloudformation or with boto - Included a preflight step to verify whether the existing deployment is dirty, and if it is, whether an image with the tag 'master' is available on ECR. --- cloudlift/deployment/ecs.py | 31 ++++++++++++++++--- cloudlift/deployment/service_creator.py | 13 ++++++++ .../deployment/service_information_fetcher.py | 4 +-- .../deployment/service_template_generator.py | 2 +- 4 files changed, 43 insertions(+), 7 deletions(-) diff --git a/cloudlift/deployment/ecs.py b/cloudlift/deployment/ecs.py index 77904fcf..4f4078f4 100644 --- a/cloudlift/deployment/ecs.py +++ b/cloudlift/deployment/ecs.py @@ -38,7 +38,8 @@ def describe_services(self, cluster_name, service_name): def describe_task_definition(self, task_definition_arn): try: return self.boto.describe_task_definition( - taskDefinition=task_definition_arn + taskDefinition=task_definition_arn, + include=['TAGS'] ) except ClientError: raise UnknownTaskDefinitionError( @@ -59,7 +60,7 @@ def describe_tasks(self, cluster_name, task_arns): return self.boto.describe_tasks(cluster=cluster_name, tasks=task_arns) def register_task_definition(self, family, containers, volumes, role_arn, cpu=False, memory=False, execution_role_arn=None, - requires_compatibilities=[], network_mode='bridge'): + requires_compatibilities=[], network_mode='bridge', tags=[]): fargate_td = {} if 'FARGATE' in requires_compatibilities: fargate_td = { @@ -74,6 +75,7 @@ def register_task_definition(self, family, containers, volumes, role_arn, cpu=Fa executionRoleArn=execution_role_arn, taskRoleArn=role_arn or u'', networkMode=network_mode, + tags=tags, **fargate_td ) @@ -223,6 +225,10 @@ def revision(self): @property def family_revision(self): return '%s:%d' % (self.get(u'family'), self.get(u'revision')) + + @property + def tags(self): + return self.get(u'tags') @property def diff(self): @@ -407,7 +413,8 @@ def get_current_task_definition(self, service): task_definition_arn=service.task_definition ) task_definition = EcsTaskDefinition( - task_definition=task_definition_payload[u'taskDefinition'] + task_definition=task_definition_payload[u'taskDefinition'], + tags=task_definition_payload[u'tags'] ) return task_definition @@ -429,6 +436,14 @@ def update_task_definition(self, task_definition): 'memory' : task_definition.memory or u'', } + + new_tags = [] + for tag in task_definition.tags: + if tag['key'] == 'task_definition_source': + new_tags.append({'key': tag['key'], 'value': 'boto3'}) + else: + new_tags.append(tag) + response = self._client.register_task_definition( family=task_definition.family, containers=task_definition.containers, @@ -436,10 +451,18 @@ def update_task_definition(self, task_definition): role_arn=task_definition.role_arn, execution_role_arn=task_definition.execution_role_arn if task_definition.execution_role_arn else boto3.resource('iam').Role('ecsTaskExecutionRole').arn, network_mode=task_definition.network_mode or u'bridge', + tags=new_tags, **fargate_td ) + # deregister old task definition only if it was created manually (not by cloudformation) + if task_definition.tags: + for tag in task_definition.tags: + if tag['key'] == 'task_definition_source': + task_definition_source = tag['value'] + if task_definition_source != 'cloudformation': + self._client.deregister_task_definition(task_definition.arn) + break new_task_definition = EcsTaskDefinition(response[u'taskDefinition']) - self._client.deregister_task_definition(task_definition.arn) return new_task_definition def update_service(self, service): diff --git a/cloudlift/deployment/service_creator.py b/cloudlift/deployment/service_creator.py index 6eefc93d..8451e417 100644 --- a/cloudlift/deployment/service_creator.py +++ b/cloudlift/deployment/service_creator.py @@ -15,6 +15,7 @@ from cloudlift.config.logging import log, log_bold, log_err from cloudlift.deployment.progress import get_stack_events, print_new_events from cloudlift.deployment.service_template_generator import ServiceTemplateGenerator +from cloudlift.deployment.service_information_fetcher import ServiceInformationFetcher class ServiceCreator(object): @@ -29,6 +30,7 @@ def __init__(self, name, environment): self.stack_name = get_service_stack_name(environment, name) self.client = get_client_for('cloudformation', self.environment) self.s3client = get_client_for('s3', self.environment) + self.ecrClient = get_client_for('ecr', self.environment) self.bucket_name = 'cloudlift-service-template' self.environment_stack = self._get_environment_stack() self.existing_events = get_stack_events(self.client, self.stack_name) @@ -106,6 +108,7 @@ def update(self): ''' log_bold("Starting to update service") + self.service_update_preflight_checks() self.service_configuration.edit_config() try: template_generator = ServiceTemplateGenerator( @@ -165,3 +168,13 @@ def _print_progress(self): log_err("Finished with status: %s" % (final_status)) else: log_bold("Finished with status: %s" % (final_status)) + + def service_update_preflight_checks(self): + # If the current deployment is considered dirty, make sure that an image tagged as 'master' is uploaded to ECR otherwise on service update, the service will try to use an image tagged as 'master' which does not exist + current_version = ServiceInformationFetcher( + self.name, self.environment).get_current_version(skip_master_reset=True) + if current_version == 'dirty': + repo_name = self.name + '-repo' + res = self.ecrClient.batch_get_image(repositoryName=repo_name, imageIds=[{'imageTag': 'master'}]) + if res['images'] == []: + raise UnrecoverableException("Current deployment is dirty. Please push an image tagged as 'master' to ECR.") \ No newline at end of file diff --git a/cloudlift/deployment/service_information_fetcher.py b/cloudlift/deployment/service_information_fetcher.py index 284240b4..75034a52 100644 --- a/cloudlift/deployment/service_information_fetcher.py +++ b/cloudlift/deployment/service_information_fetcher.py @@ -43,9 +43,9 @@ def init_stack_info(self): self.ecs_service_names = [] log_warning("Could not determine services.") - def get_current_version(self): + def get_current_version(self, skip_master_reset=False): commit_sha = self._fetch_current_task_definition_tag() - if commit_sha is None or commit_sha == 'dirty': + if commit_sha is None or commit_sha == 'dirty' and not skip_master_reset: log_warning("Currently deployed tag could not be found or is dirty,\ resetting to master") commit_sha = "master" diff --git a/cloudlift/deployment/service_template_generator.py b/cloudlift/deployment/service_template_generator.py index adc62c6e..da292553 100644 --- a/cloudlift/deployment/service_template_generator.py +++ b/cloudlift/deployment/service_template_generator.py @@ -309,7 +309,7 @@ def _add_service(self, service_name, config): ContainerDefinitions=[cd], ExecutionRoleArn=boto3.resource('iam').Role('ecsTaskExecutionRole').arn, TaskRoleArn=Ref(task_role), - Tags=Tags(Team=self.team_name, environment=self.env), + Tags=Tags(Team=self.team_name, environment=self.env, task_definition_source="cloudformation"), **launch_type_td ) From e5453851c24c7ef78d3faef54d6e8f7c99110a8e Mon Sep 17 00:00:00 2001 From: Rokibul Islam Date: Mon, 29 May 2023 17:03:25 +0530 Subject: [PATCH 2/3] refactor(update_service): move preflight checks to pre-flight.py Moved `update_service` preflight checks function to pre-flight.py --- cloudlift/config/pre_flight.py | 10 +++++++++- cloudlift/deployment/service_creator.py | 18 +++++------------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/cloudlift/config/pre_flight.py b/cloudlift/config/pre_flight.py index 0c243215..f03cb05a 100644 --- a/cloudlift/config/pre_flight.py +++ b/cloudlift/config/pre_flight.py @@ -46,4 +46,12 @@ def check_aws_instance_type(instance_type): continue else: return False, i - return True, "" \ No newline at end of file + return True, "" + +def service_update_preflight_checks(current_version, service_name, environment, ecr_client): + # If the current deployment is considered dirty, make sure that an image tagged as 'master' is uploaded to ECR otherwise on service update, the service will try to use an image tagged as 'master' which does not exist + if current_version == 'dirty': + repo_name = service_name + '-repo' + res = ecr_client.batch_get_image(repositoryName=repo_name, imageIds=[{'imageTag': 'master'}]) + if res['images'] == []: + raise UnrecoverableException("Current deployment is dirty. Please push an image tagged as 'master' to ECR.") \ No newline at end of file diff --git a/cloudlift/deployment/service_creator.py b/cloudlift/deployment/service_creator.py index 8451e417..1e745aae 100644 --- a/cloudlift/deployment/service_creator.py +++ b/cloudlift/deployment/service_creator.py @@ -16,7 +16,7 @@ from cloudlift.deployment.progress import get_stack_events, print_new_events from cloudlift.deployment.service_template_generator import ServiceTemplateGenerator from cloudlift.deployment.service_information_fetcher import ServiceInformationFetcher - +from cloudlift.config.pre_flight import service_update_preflight_checks class ServiceCreator(object): ''' @@ -108,7 +108,9 @@ def update(self): ''' log_bold("Starting to update service") - self.service_update_preflight_checks() + current_version = ServiceInformationFetcher( + self.name, self.environment).get_current_version(skip_master_reset=True) + service_update_preflight_checks(current_version=current_version, service_name=self.name, environment=self.environment, ecr_client=self.ecrClient) self.service_configuration.edit_config() try: template_generator = ServiceTemplateGenerator( @@ -167,14 +169,4 @@ def _print_progress(self): if "FAIL" in final_status: log_err("Finished with status: %s" % (final_status)) else: - log_bold("Finished with status: %s" % (final_status)) - - def service_update_preflight_checks(self): - # If the current deployment is considered dirty, make sure that an image tagged as 'master' is uploaded to ECR otherwise on service update, the service will try to use an image tagged as 'master' which does not exist - current_version = ServiceInformationFetcher( - self.name, self.environment).get_current_version(skip_master_reset=True) - if current_version == 'dirty': - repo_name = self.name + '-repo' - res = self.ecrClient.batch_get_image(repositoryName=repo_name, imageIds=[{'imageTag': 'master'}]) - if res['images'] == []: - raise UnrecoverableException("Current deployment is dirty. Please push an image tagged as 'master' to ECR.") \ No newline at end of file + log_bold("Finished with status: %s" % (final_status)) \ No newline at end of file From 30f6a39101f6463736f5c69c62ee98e776bb05ae Mon Sep 17 00:00:00 2001 From: Rokibul Islam Date: Mon, 25 Sep 2023 12:58:21 +0530 Subject: [PATCH 3/3] fix(create_task_def/update_task_def): handle cases for empty tags --- cloudlift/deployment/ecs.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/cloudlift/deployment/ecs.py b/cloudlift/deployment/ecs.py index 4f4078f4..95d0a153 100644 --- a/cloudlift/deployment/ecs.py +++ b/cloudlift/deployment/ecs.py @@ -68,6 +68,10 @@ def register_task_definition(self, family, containers, volumes, role_arn, cpu=Fa 'cpu': cpu, 'memory': memory, } + + if not any(tag['key'] == 'task_definition_source' for tag in tags): + tags.append({'key': 'task_definition_source', 'value': 'boto3'}) + return self.boto.register_task_definition( family=family, containerDefinitions=containers, @@ -436,9 +440,12 @@ def update_task_definition(self, task_definition): 'memory' : task_definition.memory or u'', } - new_tags = [] - for tag in task_definition.tags: + tags = task_definition.get('tags', []) + if not tags: + new_tags.append({'key': 'task_definition_source', 'value': 'boto3'}) + + for tag in tags: if tag['key'] == 'task_definition_source': new_tags.append({'key': tag['key'], 'value': 'boto3'}) else: