Skip to content
This repository is currently being migrated. It's locked while the migration is in progress.

Conversation

@singhals
Copy link

@singhals singhals commented Mar 3, 2025

  • Move cube cluster to a module where the inputs required are a vpc, some default resource configurations for the cluster, environment and secret vars
  • Add health checks to containers (except cubestore for now)
  • Make cubestore independently deployable through a script

@singhals singhals marked this pull request as ready for review March 5, 2025 20:22
task-definition: ${{ steps.cube-api-task-def.outputs.task-definition }}
service: cube_api
cluster: production
cluster: prod-sync_cluster

Choose a reason for hiding this comment

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

mixing - and _ 😢

Copy link
Author

Choose a reason for hiding this comment

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

yep yep! i noticed it as well. I will fix in a follow up!

set -e

AWS_REGION="us-east-1"
ECR_REPOSITORY="prod-sync-cubestore-ecr"
Copy link

@shughes-uk shughes-uk Mar 5, 2025

Choose a reason for hiding this comment

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

Maybe just store the full ECR repo in a SSM parameter and pull that? Would be less fragile.

Copy link
Author

Choose a reason for hiding this comment

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

Ya let me do this in a follow up - I need to do multiple envs for this as well


aws ecr get-login-password --region $AWS_REGION | docker login --username AWS --password-stdin $REGISTRY

docker build --platform linux/amd64 -t $REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG -f docker/cubestore/Dockerfile .
Copy link

@shughes-uk shughes-uk Mar 5, 2025

Choose a reason for hiding this comment

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

I would maybe add linux/arm64 as a second platform target. The rationale is

  • Local dev is almost always going to be arm64, we want to ensure we still work for devs
  • Graviton (aws ARM) is great and we will probably want to use that ASAP honestly.

Copy link
Author

Choose a reason for hiding this comment

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

agreed so local dev is going to use docker-compose so ill update in my next pr for that - I also need to test the ARM cube image to see if it works!

}
}

module "vpc" {

Choose a reason for hiding this comment

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

Seems sensible we will probably want to use the same VPC for sync_backend/tasks

Co-authored-by: Samantha Hughes <shughes.uk@gmail.com>
repository = aws_ecr_repository.cube_repo.name

policy = <<EOF
{

Choose a reason for hiding this comment

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

@@ -1,5 +1,5 @@
resource "aws_ecs_cluster" "main" {
name = "production"
name = "${var.cluster_prefix}_cluster"

Choose a reason for hiding this comment

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

Suggested change
name = "${var.cluster_prefix}_cluster"
name = "${var.cluster_prefix}-cluster"

@singhals singhals merged commit 3912cf5 into main Mar 5, 2025
@singhals singhals deleted the singhals/time-to-parameterize branch March 5, 2025 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants