Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CLOB-976] Add mainnet indexer related config. #48

Merged
merged 4 commits into from
Oct 27, 2023

Conversation

vincentwschau
Copy link
Contributor

No description provided.

@linear
Copy link

linear bot commented Oct 27, 2023

indexer/ecr.tf Outdated
name = var.full_node_ecr_repository_name
}

data "aws_ecr_repository" "snapshot_ful_node_ap_northeast_1" {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: full_node*

@@ -12,7 +12,8 @@ resource "aws_s3_bucket" "load_balancer" {
# TODO: refactor snapshotting full node into a separate module
# AWS S3 bucket to store all Indexer full node snapshots
resource "aws_s3_bucket" "indexer_full_node_snapshots" {
bucket = var.s3_snapshot_bucket
# Use account id for mainnet to avoid name collisions
bucket = var.environment == "mainnet" ? "${local.account_id}-${var.s3_snapshot_bucket}" : var.s3_snapshot_bucket
Copy link
Contributor

Choose a reason for hiding this comment

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

don't we want to use account_id regardless to avoid name collisions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but this would lead to plans / applies that delete existing buckets so a migration is necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add a todo and create a low priority ticket on the backlog?

# in dev environments, we build separate images.
ecr_repository_url = contains(["testnet", "testnet1", "testnet2"], var.environment) ? data.aws_ecr_repository.validator.repository_url : data.aws_ecr_repository.snapshot_validator.repository_url
ecr_repository_url = contains(["testnet", "testnet1", "testnet2"], var.environment) ? local.node_ecr_repository_url : local.snapshot_node_ecr_repository_url
Copy link
Contributor

Choose a reason for hiding this comment

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

should mainnet have the same behavior as our testnet or dev/staging for the snapshot node?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not currently deployed so that's an unknown right now.

Copy link
Contributor

Choose a reason for hiding this comment

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

why do we have a separate ECR repo for the snapshot node if we can just use the indexer full node ECR repo in testnet?

cc: @dydxwill

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't finished changing the deployment of the images for the env, so it's an unknown.

@vincentwschau vincentwschau merged commit 6bdc030 into main Oct 27, 2023
1 check passed
pswies pushed a commit to dydxopsdao/v4-infrastructure that referenced this pull request Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants