Conversation
Add new scale command to adjust the number of running tasks for a service. Command accepts a value parameter (desired task count) and requires the --service flag. Currently implements Hello World output for testing. - Add scale.go with command structure following existing patterns - Implement value validation (0-1000 tasks) - Add structured logging with slog - Initialize AWS clients for future ECS integration - Follow Go idiomatic patterns with proper error handling
- Add Scale function in internal/ecs/scale.go to update service desired count - Add validation to prevent scaling to 0 tasks (minimum is 1) - Add ScaleResult type to track scaling operation details - Update scale command to use new Scale function instead of placeholder - Use lipgloss for bold formatting of cluster/service names in output - Change minimum task count validation from 0 to 1
Document the new scale feature that uses UpdateService API for direct task count adjustment
WalkthroughAdds a new CLI command to scale ECS services, internal ECS scaling logic with validation and UpdateService call, supporting types for results, and README documentation describing the new command. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CLI as runecs scale
participant ECS as ecs.Scale
participant AWS as AWS ECS API
U->>CLI: runecs scale <value> --service <cluster>/<service>
CLI->>CLI: Validate args, parse flags, setup context
CLI->>ECS: Scale(ctx, clients, cluster, service, desiredCount)
ECS->>AWS: DescribeServices(cluster, service)
AWS-->>ECS: Service details (state, desiredCount)
ECS->>AWS: UpdateService(DesiredCount)
AWS-->>ECS: UpdateService response
ECS-->>CLI: ScaleResult(previous, new, identifiers)
CLI-->>U: Print scaled from <prev> to <new>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.2.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/product/migration-guide for migration instructions ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
meap
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
README.md (1)
73-82: Document the valid scaling range (1–1000) and that scaling to 0 is not allowed.The CLI and ECS layer enforce 1–1000; adding this note will save users a round trip and align expectations.
Apply this diff to add the note:
### Scale ECS Services The desired count of tasks for an ECS service can be adjusted instantly: ```bash runecs scale 5 --service mycanvas-ecs-staging-cluster/web-This command directly modifies the service's desired count using
UpdateService, providing immediate scaling without creating task sets or managing deployment configurations.
+This command directly modifies the service's desired count usingUpdateService, providing immediate scaling without creating task sets or managing deployment configurations.
+Note: Desired count must be between 1 and 1000; scaling to 0 is not permitted.</blockquote></details> <details> <summary>cmd/scale.go (3)</summary><blockquote> `43-47`: **Remove redundant arg count check in PreRunE (cobra.ExactArgs already enforces this).** You already set `Args: cobra.ExactArgs(1)`. The manual `len(args)` check is duplicate. Apply this diff: ```diff -func scalePreRunE(cmd *cobra.Command, args []string) error { - if len(args) != 1 { - return errors.New("scale command requires exactly one argument: the desired task count") - } +func scalePreRunE(cmd *cobra.Command, args []string) error {
48-58: Avoid duplicating validation logic between CLI and ECS layer.You parse and range-check here and then again in the ECS layer (which also maintains the canonical min/max). Consider either:
- Keeping only the parse here and delegating range validation to ecs.Scale (preferred to avoid drift), or
- Importing and reusing shared bounds from ecs, if you want early CLI validation.
Current duplication increases maintenance burden if limits change.
Would you like a follow-up patch to move all range checks into ecs.Scale and report user-friendly messages at the CLI?
92-95: Use Cobra’s output writer for better testability and consistency.Prefer
cmd.Printfoverfmt.Printfso output respects command’s configured writer and is easier to capture in tests.Apply this diff:
-fmt.Printf("Service %s scaled from %d to %d tasks\n", - boldStyle.Render(fmt.Sprintf("%s/%s", result.ClusterName, result.ServiceName)), - result.PreviousDesiredCount, result.NewDesiredCount) +cmd.Printf("Service %s scaled from %d to %d tasks\n", + boldStyle.Render(fmt.Sprintf("%s/%s", result.ClusterName, result.ServiceName)), + result.PreviousDesiredCount, result.NewDesiredCount)internal/ecs/scale.go (3)
52-56: Guard against DAEMON services (cannot scale desired count).DAEMON-scheduled services ignore DesiredCount and UpdateService with DesiredCount is invalid. Proactively return a clear error.
Apply this diff (adds a types import and the guard):
import ( "context" "errors" "fmt" "github.com/aws/aws-sdk-go-v2/service/ecs" + "github.com/aws/aws-sdk-go-v2/service/ecs/types" ) @@ svc := describeResp.Services[0] if svc.Status == nil || *svc.Status != "ACTIVE" { return nil, fmt.Errorf("service %s is not in ACTIVE state", service) } + if svc.SchedulingStrategy != nil && *svc.SchedulingStrategy == types.SchedulingStrategyDaemon { + return nil, fmt.Errorf("service %s uses DAEMON scheduling and cannot be scaled by desired count", service) + }
75-77: Validate UpdateService response structure for consistency with other operations.Elsewhere (deploy/restart) you error if
ServiceorServiceArnis missing. Consider doing the same here to keep invariants consistent and catch unexpected API responses.Apply this diff:
-if updateResp.Service != nil && updateResp.Service.ServiceArn != nil { - result.ServiceArn = *updateResp.Service.ServiceArn -} +if updateResp.Service == nil { + return nil, errors.New("invalid service update response: missing service") +} +if updateResp.Service.ServiceArn == nil { + return nil, errors.New("invalid service update response: missing service ARN") +} +result.ServiceArn = *updateResp.Service.ServiceArn
29-38: Align error semantics for out-of-range desired counts.
ErrInvalidDesiredCountmentions only “greater than 0” but not the upper bound. Either expand its meaning (min/max) or use two sentinel errors (e.g.,ErrDesiredCountTooLow,ErrDesiredCountTooHigh). As-is, callers checking forErrInvalidDesiredCountwon’t catch the “too high” case.If you want, I can draft a small error utility to format consistent messages and expose min/max for reuse in the CLI.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
README.md(1 hunks)cmd/scale.go(1 hunks)internal/ecs/scale.go(1 hunks)internal/ecs/types.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
internal/ecs/scale.go (3)
internal/ecs/types.go (2)
AWSClients(28-32)ScaleResult(136-142)internal/ecs/restart.go (2)
forceNewDeploy(61-91)Restart(93-114)internal/ecs/deploy.go (1)
Deploy(78-103)
🔇 Additional comments (1)
internal/ecs/types.go (1)
135-142: LGTM: ScaleResult is clear and consistent with existing result types.Field names and doc comment are consistent with other result structs. No issues.
| previousDesiredCount := svc.DesiredCount | ||
|
|
There was a problem hiding this comment.
Fix potential compile error: DesiredCount is a pointer in aws-sdk-go-v2.
svc.DesiredCount is *int32 in ECS v2 models. Assigning it directly to an int32 field will not compile. Safely dereference (and guard nil).
Apply this diff:
-previousDesiredCount := svc.DesiredCount
+var previousDesiredCount int32
+if svc.DesiredCount != nil {
+ previousDesiredCount = *svc.DesiredCount
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| previousDesiredCount := svc.DesiredCount | |
| var previousDesiredCount int32 | |
| if svc.DesiredCount != nil { | |
| previousDesiredCount = *svc.DesiredCount | |
| } |
🤖 Prompt for AI Agents
In internal/ecs/scale.go around lines 57-58, svc.DesiredCount is a *int32 in
aws-sdk-go-v2 so assigning it directly to an int32 will fail to compile; update
the assignment to safely dereference with a nil guard (e.g., if svc.DesiredCount
!= nil { previousDesiredCount = *svc.DesiredCount } else { previousDesiredCount
= 0 }) or use the SDK helper aws.ToInt32(svc.DesiredCount) to get an int32
value.
1 participant
Summary
scalecommand to adjust ECS service task countsImplementation Details
internal/ecs/scale.gowith Scale function that:ScaleResulttype to track operation detailscmd/scale.goto use the new Scale functionTest Plan
Summary by CodeRabbit
New Features
Documentation