From a2519a710b31a7ff2a7500e3ebaf88327bf9cc68 Mon Sep 17 00:00:00 2001 From: Manjunath PV Date: Fri, 3 Nov 2023 16:05:24 +0530 Subject: [PATCH 1/5] Fixed minor issues for ecs IAM scripts --- .gitignore | 7 +++ CHANGELOG.md | 9 +++ .../assets/infra/ecs.conf.tftpl | 15 +++++ covalent_ecs_plugin/assets/infra/iam.tf | 6 +- covalent_ecs_plugin/assets/infra/main.tf | 35 +++++++++-- .../assets/infra/networking.tf | 4 +- covalent_ecs_plugin/assets/infra/variables.tf | 37 ++++++++++- covalent_ecs_plugin/ecs.py | 61 +++++++++++++------ 8 files changed, 146 insertions(+), 28 deletions(-) create mode 100644 covalent_ecs_plugin/assets/infra/ecs.conf.tftpl diff --git a/.gitignore b/.gitignore index 8596cc9..e9c0b1b 100644 --- a/.gitignore +++ b/.gitignore @@ -113,3 +113,10 @@ ipython_config.py # Coverage .coverage + +# Terraform +**/.terraform/** +**/*.tfstate* +**/.terraform.lock.hcl +**/*.tfvars +**/*.plan \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 8108d77..6089761 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [UNRELEASED] +### Changed + +- Changed the `name` variable / attribute to `prefix` to follow uniformity across all executor plugins +- Changed the **dict** type of defaults to `ExecutorInfraDefaults` & `ExecutorPluginDefaults` + +### Added + +- Added a tftpl file for ECS executor to output the configuration + ## [0.32.0] - 2023-10-06 ### Changed diff --git a/covalent_ecs_plugin/assets/infra/ecs.conf.tftpl b/covalent_ecs_plugin/assets/infra/ecs.conf.tftpl new file mode 100644 index 0000000..869da80 --- /dev/null +++ b/covalent_ecs_plugin/assets/infra/ecs.conf.tftpl @@ -0,0 +1,15 @@ +[ecs] +credentials=${credentials} +profile=${profile} +region=${region} +s3_bucket_name=${s3_bucket_name} +ecs_cluster_name=${ecs_cluster_name} +ecs_task_execution_role_name=${ecs_task_execution_role_name} +ecs_task_role_name=${ecs_task_role_name} +ecs_task_subnet_id=${ecs_task_subnet_id} +ecs_task_security_group_id=${ecs_task_security_group_id} +ecs_task_log_group_name=${ecs_task_log_group_name} +vcpu=${vcpu} +memory=${memory} +cache_dir=${cache_dir} +poll_freq=${poll_freq} diff --git a/covalent_ecs_plugin/assets/infra/iam.tf b/covalent_ecs_plugin/assets/infra/iam.tf index cd2a540..aba1376 100644 --- a/covalent_ecs_plugin/assets/infra/iam.tf +++ b/covalent_ecs_plugin/assets/infra/iam.tf @@ -10,7 +10,7 @@ data "aws_iam_policy_document" "ecs_tasks_execution_role" { } resource "aws_iam_role" "ecs_tasks_execution_role" { - name = "${var.name}-task-execution-role" + name = "${var.prefix}-task-execution-role" assume_role_policy = data.aws_iam_policy_document.ecs_tasks_execution_role.json } @@ -20,7 +20,7 @@ resource "aws_iam_role_policy_attachment" "ecs_tasks_execution_role" { } resource "aws_iam_role_policy" "task_policy" { - name = "${var.name}-task-policy" + name = "${var.prefix}-task-policy" role = aws_iam_role.task_role.id policy = jsonencode({ @@ -49,7 +49,7 @@ resource "aws_iam_role_policy" "task_policy" { } resource "aws_iam_role" "task_role" { - name = "${var.name}-task-role" + name = "${var.prefix}-task-role" assume_role_policy = jsonencode({ "Version" : "2012-10-17", diff --git a/covalent_ecs_plugin/assets/infra/main.tf b/covalent_ecs_plugin/assets/infra/main.tf index 30a6452..0473193 100644 --- a/covalent_ecs_plugin/assets/infra/main.tf +++ b/covalent_ecs_plugin/assets/infra/main.tf @@ -19,7 +19,7 @@ provider "aws" { } resource "aws_s3_bucket" "bucket" { - bucket = "${var.name}-bucket" + bucket = "${var.prefix}-bucket" force_destroy = true } @@ -38,7 +38,7 @@ resource "aws_s3_bucket_acl" "bucket_acl" { } resource "aws_ecr_repository" "ecr_repository" { - name = "${var.name}-ecr-repo" + name = "${var.prefix}-ecr-repo" image_tag_mutability = "IMMUTABLE" force_delete = true @@ -49,11 +49,11 @@ resource "aws_ecr_repository" "ecr_repository" { } resource "aws_cloudwatch_log_group" "log_group" { - name = "${var.name}-log-group" + name = "${var.prefix}-log-group" } resource "aws_ecs_cluster" "ecs_cluster" { - name = "${var.name}-ecs-cluster" + name = "${var.prefix}-ecs-cluster" configuration { execute_command_configuration { @@ -64,3 +64,30 @@ resource "aws_ecs_cluster" "ecs_cluster" { } } } + +# Executor Covalent config section +data template_file executor_config { + template = "${file("${path.module}/ecs.conf.tftpl")}" + + vars = { + credentials=var.credentials + profile=var.profile + region=var.aws_region + s3_bucket_name=aws_s3_bucket.bucket.id + ecs_cluster_name=aws_ecs_cluster.ecs_cluster.name + ecs_task_execution_role_name=aws_iam_role.ecs_tasks_execution_role.name + ecs_task_role_name=aws_iam_role.task_role.name + ecs_task_subnet_id=module.vpc.public_subnets[0] + ecs_task_security_group_id=aws_security_group.sg.id + ecs_task_log_group_name=aws_cloudwatch_log_group.log_group.name + vcpu=var.vcpus + memory=var.memory + cache_dir=var.cache_dir + poll_freq=var.poll_freq + } +} + +resource local_file executor_config { + content = data.template_file.executor_config.rendered + filename = "${path.module}/ecs.conf" +} \ No newline at end of file diff --git a/covalent_ecs_plugin/assets/infra/networking.tf b/covalent_ecs_plugin/assets/infra/networking.tf index b9c1322..43983ef 100644 --- a/covalent_ecs_plugin/assets/infra/networking.tf +++ b/covalent_ecs_plugin/assets/infra/networking.tf @@ -19,7 +19,7 @@ module "vpc" { create_vpc = (var.vpc_id == "") - name = "${var.name}-vpc" + name = "${var.prefix}-vpc" cidr = var.vpc_cidr azs = ["${var.aws_region}a"] @@ -35,7 +35,7 @@ module "vpc" { } resource "aws_security_group" "sg" { - name = "${var.name}-sg" + name = "${var.prefix}-sg" description = "Allow traffic to Covalent server" vpc_id = "${var.vpc_id == "" ? module.vpc.vpc_id : var.vpc_id}" diff --git a/covalent_ecs_plugin/assets/infra/variables.tf b/covalent_ecs_plugin/assets/infra/variables.tf index 10510f2..7f6b25a 100644 --- a/covalent_ecs_plugin/assets/infra/variables.tf +++ b/covalent_ecs_plugin/assets/infra/variables.tf @@ -14,7 +14,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -variable "name" { +variable "prefix" { default = "covalent-ecs-ft" description = "Name used to prefix AWS resources" } @@ -38,3 +38,38 @@ variable "vpc_cidr" { default = "10.0.0.0/24" description = "VPC CIDR range" } + +variable "cache_dir" { + type = string + default = "/tmp/covalent" + description = "Path on local machine where temporary files are generated" +} + +variable "poll_freq" { + type = number + default = 5 + description = "Frequency with which to poll AWS batch for the result object" +} + +variable "vcpus" { + type = number + default = 2 + description = "Number of vcpus a batch job will consume by default" +} + +variable "memory" { + type = number + default = 2 + description = "Memory in GB for the batch job" +} + +variable "credentials" { + type = string + default = "" + description = "Path to the AWS shared configuration file" +} + +variable "profile" { + type = string + description = "AWS profile used during execution" +} diff --git a/covalent_ecs_plugin/ecs.py b/covalent_ecs_plugin/ecs.py index 0cdb6bb..e76dba6 100644 --- a/covalent_ecs_plugin/ecs.py +++ b/covalent_ecs_plugin/ecs.py @@ -29,25 +29,50 @@ from covalent._shared_files.config import get_config from covalent._shared_files.logger import app_log from covalent_aws_plugins import AWSExecutor +from pydantic import BaseModel from .utils import _execute_partial_in_threadpool, _load_pickle_file -_EXECUTOR_PLUGIN_DEFAULTS = { - "credentials": "", - "profile": "", - "region": "", - "s3_bucket_name": "covalent-fargate-task-resources", - "ecs_cluster_name": "covalent-fargate-cluster", - "ecs_task_execution_role_name": "ecsTaskExecutionRole", - "ecs_task_role_name": "CovalentFargateTaskRole", - "ecs_task_subnet_id": "", - "ecs_task_security_group_id": "", - "ecs_task_log_group_name": "covalent-fargate-task-logs", - "vcpu": 0.25, - "memory": 0.5, - "cache_dir": "/tmp/covalent", - "poll_freq": 10, -} + +class ExecutorPluginDefaults(BaseModel): + credentials: str = "" + profile: str = "" + region: str = "" + s3_bucket_name: str = "covalent-fargate-task-resources" + ecs_cluster_name: str = "covalent-fargate-cluster" + ecs_task_execution_role_name: str = "ecsTaskExecutionRole" + ecs_task_role_name: str = "CovalentFargateTaskRole" + ecs_task_subnet_id: str = "" + ecs_task_security_group_id: str = "" + ecs_task_log_group_name: str = "covalent-fargate-task-logs" + vcpu: float = 0.25 + memory: float = 0.5 + cache_dir: str = "/tmp/covalent" + poll_freq: int = 10 + + +class ExecutorInfraDefaults(BaseModel): + """ + Configuration values for provisioning AWS Batch cloud infrastructure + """ + prefix: str = "" + credentials: str = "" + profile: str = "" + region: str = "" + s3_bucket_name: str = "covalent-fargate-task-resources" + ecs_cluster_name: str = "covalent-fargate-cluster" + ecs_task_execution_role_name: str = "ecsTaskExecutionRole" + ecs_task_role_name: str = "CovalentFargateTaskRole" + ecs_task_subnet_id: str = "" + ecs_task_security_group_id: str = "" + ecs_task_log_group_name: str = "covalent-fargate-task-logs" + vcpu: float = 0.25 + memory: float = 0.5 + cache_dir: str = "/tmp/covalent" + poll_freq: int = 10 + + +_EXECUTOR_PLUGIN_DEFAULTS = ExecutorPluginDefaults().dict() EXECUTOR_PLUGIN_NAME = "ECSExecutor" @@ -55,7 +80,7 @@ RESULT_FILENAME = "result-{dispatch_id}-{node_id}.pkl" CONTAINER_NAME = "covalent-task-{dispatch_id}-{node_id}" COVALENT_EXEC_BASE_URI = os.getenv( - "COVALENT_EXEC_BASE_URI", "public.ecr.aws/covalent/covalent-executor-base:stable" + "COVALENT_EXEC_BASE_URI", "public.ecr.aws/covalent/covalent-executor-base:latest" ) @@ -215,7 +240,7 @@ async def submit_task(self, task_metadata: Dict, identity: Dict) -> Any: ], }, ], - cpu=str(int(self.vcpu * 1024)), + cpu=str(int(self.vcpu)), memory=str(int(self.memory * 1024)), ) await _execute_partial_in_threadpool(partial_func) From 8a8ecf1ae1e0038753a67d3ef50adac157ce9676 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 3 Nov 2023 10:38:23 +0000 Subject: [PATCH 2/5] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- covalent_ecs_plugin/assets/infra/main.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/covalent_ecs_plugin/assets/infra/main.tf b/covalent_ecs_plugin/assets/infra/main.tf index 0473193..41ad381 100644 --- a/covalent_ecs_plugin/assets/infra/main.tf +++ b/covalent_ecs_plugin/assets/infra/main.tf @@ -90,4 +90,4 @@ data template_file executor_config { resource local_file executor_config { content = data.template_file.executor_config.rendered filename = "${path.module}/ecs.conf" -} \ No newline at end of file +} From 73833534f05a0322f675d0d4d07cee0687ca5430 Mon Sep 17 00:00:00 2001 From: Manjunath PV Date: Fri, 3 Nov 2023 16:15:26 +0530 Subject: [PATCH 3/5] MANIFEST.in file minor changes --- CHANGELOG.md | 1 + MANIFEST.in | 2 ++ 2 files changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6089761..da6fb52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Changed the `name` variable / attribute to `prefix` to follow uniformity across all executor plugins - Changed the **dict** type of defaults to `ExecutorInfraDefaults` & `ExecutorPluginDefaults` +- Minor changes in `MANIFEST.in` file ### Added diff --git a/MANIFEST.in b/MANIFEST.in index f6c805e..f9e6e7c 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,3 +1,5 @@ include VERSION include requirements.txt +include covalent_ecs_plugin/assets/infra/* +include covalent_ecs_plugin/assets/infra/*.swp include covalent_ecs_plugin/assets/infra/*.tf From f428e52e01350f3cd3521be7d99d23c8c48052fb Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 3 Nov 2023 10:45:37 +0000 Subject: [PATCH 4/5] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- covalent_ecs_plugin/ecs.py | 1 + 1 file changed, 1 insertion(+) diff --git a/covalent_ecs_plugin/ecs.py b/covalent_ecs_plugin/ecs.py index e76dba6..99f39bb 100644 --- a/covalent_ecs_plugin/ecs.py +++ b/covalent_ecs_plugin/ecs.py @@ -55,6 +55,7 @@ class ExecutorInfraDefaults(BaseModel): """ Configuration values for provisioning AWS Batch cloud infrastructure """ + prefix: str = "" credentials: str = "" profile: str = "" From e13baefc267038e8886587d72c1b4697e06e2aea Mon Sep 17 00:00:00 2001 From: Manjunath PV Date: Mon, 20 Nov 2023 18:01:54 +0530 Subject: [PATCH 5/5] Minor code change --- covalent_ecs_plugin/ecs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/covalent_ecs_plugin/ecs.py b/covalent_ecs_plugin/ecs.py index 99f39bb..1986427 100644 --- a/covalent_ecs_plugin/ecs.py +++ b/covalent_ecs_plugin/ecs.py @@ -81,7 +81,7 @@ class ExecutorInfraDefaults(BaseModel): RESULT_FILENAME = "result-{dispatch_id}-{node_id}.pkl" CONTAINER_NAME = "covalent-task-{dispatch_id}-{node_id}" COVALENT_EXEC_BASE_URI = os.getenv( - "COVALENT_EXEC_BASE_URI", "public.ecr.aws/covalent/covalent-executor-base:latest" + "COVALENT_EXEC_BASE_URI", "public.ecr.aws/covalent/covalent-executor-base:stable" )