chore: configure 1 for desired_count on stage #8615
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WalkthroughUpdated 10 service Terraform locals to use environment-based conditionals for ECS desired_count and auto-scaling capacities (stage set to lower values). Also moved/added Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
💰 Infracost reportMonthly estimate decreased by $187 📉
*Usage costs can be estimated by updating Infracost Cloud settings, see docs for other options. Estimate details (includes details of unsupported resources) |
|
View your CI Pipeline Execution ↗ for commit 8b27998
☁️ Nx Cloud last updated this comment at |
…auto-scaling for staging (#8615)
tataihono
force-pushed
the
tataihono/eng-3564-configure-environment-based-desired_count-and-auto-scaling
branch
from
55b808b to
ec15aeb
Compare
github-actions
bot
requested a deployment
to
Preview - journeys-admin
tataihono
changed the title
tataihono
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
infrastructure/environments/stage/main.tf (1)
172-187: Duplicateenvargument will cause Terraform error.The
envvariable is assigned twice in thearclightmodule block (lines 180 and 186). Terraform does not allow duplicate arguments within the same block and will fail with an error likeDuplicate argument "env".🐛 Proposed fix - remove duplicate assignment
module "arclight" { source = "../../../apps/arclight/infrastructure" ecs_config = merge(local.public_ecs_config, { alb_target_group = merge(local.alb_target_group, { health_check_path = "/" health_check_port = "3000" }) }) env = "stage" doppler_token = data.aws_ssm_parameter.doppler_arclight_stage_token.value alb_listener_arn = module.stage.public_alb.alb_listener.arn alb_dns_name = module.stage.public_alb.dns_name host_name = "core-stage.arclight.org" host_names = ["*.arclight.org", "*.arc.gt"] // handle any arclight or arc.gt subdomain passed to stage alb - env = "stage" }
🧹 Nitpick comments (8)
apis/api-analytics/infrastructure/locals.tf (1)
16-23: Redundant conditionals return the same value on both branches.Lines 16 and 23 use ternary operators where both branches evaluate to
1:
desired_count = var.env == "stage" ? 1 : 1min_capacity = var.env == "stage" ? 1 : 1This adds cognitive overhead without providing any differentiation. Either simplify to a constant
1, or specify the intended production values if they should differ.♻️ Suggested simplification
- desired_count = var.env == "stage" ? 1 : 1 + desired_count = 1 zone_id = var.ecs_config.zone_id alb_target_group = merge(var.ecs_config.alb_target_group, { port = local.port }) auto_scaling = { max_capacity = var.env == "stage" ? 1 : 4 - min_capacity = var.env == "stage" ? 1 : 1 + min_capacity = 1apps/arclight/infrastructure/locals.tf (1)
36-37: Redundant conditional formin_capacity.Line 37 evaluates to
1regardless of environment. Consider simplifying to a constant or using the intended production value if it should differ.♻️ Suggested simplification
max_capacity = var.env == "stage" ? 1 : 4 - min_capacity = var.env == "stage" ? 1 : 1 + min_capacity = 1apis/api-users/infrastructure/locals.tf (1)
24-31: Redundant conditionals return the same value on both branches.Lines 24 and 31 use ternary operators where both branches evaluate to
1:
desired_count = var.env == "stage" ? 1 : 1min_capacity = var.env == "stage" ? 1 : 1This follows the same pattern flagged in other files. Consider simplifying to constants or using intended production values.
♻️ Suggested simplification
- desired_count = var.env == "stage" ? 1 : 1 + desired_count = 1 zone_id = var.ecs_config.zone_id alb_target_group = merge(var.ecs_config.alb_target_group, { port = local.port }) auto_scaling = { max_capacity = var.env == "stage" ? 1 : 2 - min_capacity = var.env == "stage" ? 1 : 1 + min_capacity = 1apps/cms/infrastructure/locals.tf (1)
24-24: Redundant ternary expressions fordesired_countandmin_capacity.The conditionals on lines 24 and 35 evaluate to
1in both branches, making them no-ops. While this establishes a consistent pattern for future differentiation, consider simplifying to reduce noise:- desired_count = var.env == "stage" ? 1 : 1 + desired_count = 1- min_capacity = var.env == "stage" ? 1 : 1 + min_capacity = 1The
max_capacityconditional on line 34 is correctly implemented with meaningful differentiation (stage: 1, prod: 4).Also applies to: 34-35
apis/api-media/infrastructure/locals.tf (1)
51-51: Same redundant ternary pattern as other services.Lines 51 and 58 have ternaries that always return
1. Themax_capacityon line 57 correctly differentiates stage (1) from prod (3).Consider simplifying the no-op conditionals:
- desired_count = var.env == "stage" ? 1 : 1 + desired_count = 1 ... - min_capacity = var.env == "stage" ? 1 : 1 + min_capacity = 1Also applies to: 57-58
apis/api-languages/infrastructure/locals.tf (1)
27-27: Consistent with PR pattern; same no-op ternary observation.Same as other services —
desired_count(line 27) andmin_capacity(line 34) conditionals are no-ops. Themax_capacity(line 33) correctly varies between stage (1) and prod (3).Also applies to: 33-34
apis/api-journeys-modern/infrastructure/locals.tf (1)
62-62: Same pattern applies here.
desired_count(line 62) andmin_capacity(line 69) are no-op ternaries.max_capacity(line 68) correctly differentiates stage (1) from prod (4).Also applies to: 68-69
apps/journeys-admin/infrastructure/locals.tf (1)
39-39: Same pattern; consider PR-wide simplification.This file follows the same pattern:
desired_count(line 39) andmin_capacity(line 50) are no-ops, whilemax_capacity(line 49) correctly varies.If the intent is to prepare for future per-environment values, consider documenting this in a comment. Otherwise, simplifying the no-ops across all 10 services would reduce cognitive load. As per coding guidelines, conditional expressions should handle meaningful edge cases.
Also applies to: 49-50
|
Contributor
Author
|
atlantis plan |
Contributor
Author
|
atlantis plan |
|
Ran Plan for dir: Plan Error Show Output |
…auto-scaling for staging (#8615)
Contributor
Author
|
atlantis plan |
|
Ran Plan for dir: Plan Error |
|
Ran Plan for dir: Show OutputTerraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
~ update in-place
Terraform will perform the following actions:
# module.prod.module.arclight.module.ecs-task.aws_ecs_service.ecs_service will be updated in-place
~ resource "aws_ecs_service" "ecs_service" {
~ desired_count = 2 -> 4
id = "arn:aws:ecs:us-east-2:410965620680:service/jfp-ecs-cluster-prod/arclight-prod-service"
name = "arclight-prod-service"
tags = {}
# (18 unchanged attributes hidden)
# (4 unchanged blocks hidden)
}
# module.stage.module.api-analytics.module.ecs-task.aws_appautoscaling_target.service_autoscaling will be updated in-place
~ resource "aws_appautoscaling_target" "service_autoscaling" {
id = "service/jfp-ecs-cluster-stage/api-analytics-stage-service"
~ max_capacity = 4 -> 1
tags = {}
# (8 unchanged attributes hidden)
# (1 unchanged block hidden)
}
# module.stage.module.api-gateway-stage.module.ecs-task.aws_appautoscaling_target.service_autoscaling will be updated in-place
~ resource "aws_appautoscaling_target" "service_autoscaling" {
id = "service/jfp-ecs-cluster-stage/api-gateway-stage-service"
~ max_capacity = 4 -> 1
~ min_capacity = 2 -> 1
tags = {}
# (7 unchanged attributes hidden)
# (1 unchanged block hidden)
}
# module.stage.module.api-gateway-stage.module.ecs-task.aws_ecs_service.ecs_service will be updated in-place
~ resource "aws_ecs_service" "ecs_service" {
~ desired_count = 2 -> 1
id = "arn:aws:ecs:us-east-2:410965620680:service/jfp-ecs-cluster-stage/api-gateway-stage-service"
name = "api-gateway-stage-service"
tags = {}
# (18 unchanged attributes hidden)
# (4 unchanged blocks hidden)
}
# module.stage.module.api-journeys.module.ecs-task.aws_appautoscaling_target.service_autoscaling will be updated in-place
~ resource "aws_appautoscaling_target" "service_autoscaling" {
id = "service/jfp-ecs-cluster-stage/api-journeys-stage-service"
~ max_capacity = 4 -> 1
~ min_capacity = 2 -> 1
tags = {}
# (7 unchanged attributes hidden)
# (1 unchanged block hidden)
}
# module.stage.module.api-journeys.module.ecs-task.aws_ecs_service.ecs_service will be updated in-place
~ resource "aws_ecs_service" "ecs_service" {
~ desired_count = 2 -> 1
id = "arn:aws:ecs:us-east-2:410965620680:service/jfp-ecs-cluster-stage/api-journeys-stage-service"
name = "api-journeys-stage-service"
tags = {}
# (18 unchanged attributes hidden)
# (4 unchanged blocks hidden)
}
# module.stage.module.api-journeys-modern.module.ecs-task.aws_appautoscaling_target.service_autoscaling will be updated in-place
~ resource "aws_appautoscaling_target" "service_autoscaling" {
id = "service/jfp-ecs-cluster-stage/api-journeys-modern-stage-service"
~ max_capacity = 4 -> 1
tags = {}
# (8 unchanged attributes hidden)
# (1 unchanged block hidden)
}
# module.stage.module.api-languages.module.ecs-task.aws_appautoscaling_target.service_autoscaling will be updated in-place
~ resource "aws_appautoscaling_target" "service_autoscaling" {
id = "service/jfp-ecs-cluster-stage/api-languages-stage-service"
~ max_capacity = 3 -> 1
tags = {}
# (8 unchanged attributes hidden)
# (1 unchanged block hidden)
}
# module.stage.module.api-media.module.ecs-task.aws_appautoscaling_target.service_autoscaling will be updated in-place
~ resource "aws_appautoscaling_target" "service_autoscaling" {
id = "service/jfp-ecs-cluster-stage/api-media-stage-service"
~ max_capacity = 3 -> 1
tags = {}
# (8 unchanged attributes hidden)
# (1 unchanged block hidden)
}
# module.stage.module.api-users.module.ecs-task.aws_appautoscaling_target.service_autoscaling will be updated in-place
~ resource "aws_appautoscaling_target" "service_autoscaling" {
id = "service/jfp-ecs-cluster-stage/api-users-stage-service"
~ max_capacity = 2 -> 1
tags = {}
# (8 unchanged attributes hidden)
# (1 unchanged block hidden)
}
# module.stage.module.arclight.module.ecs-task.aws_appautoscaling_target.service_autoscaling will be updated in-place
~ resource "aws_appautoscaling_target" "service_autoscaling" {
id = "service/jfp-ecs-cluster-stage/arclight-stage-service"
~ max_capacity = 4 -> 1
tags = {}
# (8 unchanged attributes hidden)
# (1 unchanged block hidden)
}
# module.stage.module.cms.module.ecs-task.aws_appautoscaling_target.service_autoscaling will be updated in-place
~ resource "aws_appautoscaling_target" "service_autoscaling" {
id = "service/jfp-ecs-cluster-stage/cms-stage-service"
~ max_capacity = 4 -> 1
tags = {}
# (8 unchanged attributes hidden)
# (1 unchanged block hidden)
}
# module.stage.module.journeys-admin.module.ecs-task.aws_appautoscaling_target.service_autoscaling will be updated in-place
~ resource "aws_appautoscaling_target" "service_autoscaling" {
id = "service/jfp-ecs-cluster-stage/journeys-admin-stage-service"
~ max_capacity = 4 -> 1
tags = {}
# (8 unchanged attributes hidden)
# (1 unchanged block hidden)
}
Plan: 0 to add, 13 to change, 0 to destroy.
╷
│ Warning: Deprecated Resource
│
│ with module.datadog.datadog_integration_aws.sandbox,
│ on modules/aws/datadog/main.tf line 118, in resource "datadog_integration_aws" "sandbox":
│ 118: resource "datadog_integration_aws" "sandbox" {
│
│ **This resource is deprecated - use the `datadog_integration_aws_account`
│ resource instead**:
│ https://registry.terraform.io/providers/DataDog/datadog/latest/docs/resources/integration_aws_account
╵
╷
│ Warning: Deprecated attribute
│
│ on .terraform/modules/datadog.datadog_log_forwarder/modules/log_forwarder/main.tf line 2, in locals:
│ 2: bucket_name = var.bucket_name != "" ? var.bucket_name : "datadog-forwarder-${data.aws_caller_identity.current.account_id}-${data.aws_region.current.name}"
│
│ The attribute "name" is deprecated. Refer to the provider documentation for
│ details.
│
│ (and 2 more similar warnings elsewhere)
╵
Plan: 0 to add, 13 to change, 0 to destroy.
|
Contributor
|
The latest updates on your projects.
|
tataihono
deleted the
tataihono/eng-3564-configure-environment-based-desired_count-and-auto-scaling
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Updated all service infrastructure to use environment-based configuration for
desired_count,max_capacity, andmin_capacity. Staging environment now uses lower values (1) across all services.Changes
locals.tffiles to makedesired_countconditional based onvar.envmax_capacityandmin_capacityto 1 for stagingenv = "stage"parameter to arclight module ininfrastructure/environments/stage/main.tfFiles Modified
apps/arclight/infrastructure/locals.tfapps/cms/infrastructure/locals.tfapps/journeys-admin/infrastructure/locals.tfapis/api-analytics/infrastructure/locals.tfapis/api-gateway/infrastructure/locals.tfapis/api-journeys/infrastructure/locals.tfapis/api-journeys-modern/infrastructure/locals.tfapis/api-languages/infrastructure/locals.tfapis/api-media/infrastructure/locals.tfapis/api-users/infrastructure/locals.tfinfrastructure/environments/stage/main.tfStaging Configuration
For staging environment, all services now have:
desired_count: 1max_capacity: 1min_capacity: 1Production values remain unchanged.
Closes ENG-3564
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.