feat: add command field to Swarm services and improve service deployments
#56
Conversation
…yment - Introduced optional `command` field in Swarm service schema and API procedures. - Added schema migrations to include `command` column in the database. - Implemented `splitCommand` utility for parsing shell commands. - Updated service deployment logic to handle `command` field, passing parsed commands to the Swarm TaskTemplate. - Enhanced service overview form to allow specifying a command for deployment.
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR adds command field support to Swarm services across the full stack: form UI for editing, API schema validation, database schema with migration, deployment logic with command parsing and enhanced monitoring, and a new utility function for tokenizing command strings. Changes
Sequence DiagramsequenceDiagram
participant User
participant Form as Web Form
participant API as API Router
participant DB as Database
participant Docker as Docker API
participant Monitor as Monitoring
User->>Form: Submit command "npm start"
Form->>API: POST updateSwarmServiceOverview(command)
API->>API: Validate schema
API->>DB: Update SwarmService.command
DB-->>API: Confirm update
API-->>Form: Success
Note over API,Docker: Service Deployment Phase
API->>Docker: Deploy with command
Docker->>Docker: splitCommand("npm start") → ["npm", "start"]
Docker->>Docker: Set TaskTemplate.ContainerSpec.Command
Docker-->>Monitor: Service deployed
Monitor->>Monitor: Poll task status
alt Task Failed
Monitor->>Docker: Fetch container logs via remoteExec
Docker-->>Monitor: Container logs
Monitor->>Monitor: Log error & break
else Task Running
Monitor->>Monitor: Mark service as up
end
Monitor-->>User: Deployment complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/app/dashboard/services/[serviceId]/components/tabs/overview/service-overview-form-swarm.tsx (1)
134-147: LGTM! Well-integrated form field.The command field follows the existing form patterns and provides a helpful placeholder. The default value handling (
swarmService.command ?? '') correctly converts null to empty string for display.Consider using a textarea instead of a single-line input if commands might be long or if users might benefit from multi-line formatting for readability. However, this is purely optional since commands are typically single-line.
packages/schemas/src/services.ts (1)
18-18: LGTM! Consider optional validation enhancements.The schema correctly defines command as an optional string.
Consider adding validation refinements if helpful:
- command: z.string().optional(), + command: z.string().trim().max(1000).optional(),This would enforce a maximum length and automatically trim whitespace. However, the current implementation is acceptable since the API handler already trims the value (Line 46 in updateSwarmServiceOverview.ts).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/web/app/dashboard/services/[serviceId]/components/tabs/overview/service-overview-form-swarm.tsx(1 hunks)packages/api/src/routers/services/updateSwarmServiceOverview.ts(1 hunks)packages/db/prisma/migrations/20251210015009_add_command_on_swarm_service/migration.sql(1 hunks)packages/db/prisma/schema.prisma(1 hunks)packages/schemas/src/services.ts(1 hunks)packages/utils/src/cli.ts(1 hunks)packages/utils/src/docker/swarm/deploySwarmService.ts(2 hunks)packages/utils/src/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/utils/src/docker/swarm/deploySwarmService.ts (1)
packages/utils/src/cli.ts (1)
splitCommand(1-51)
apps/web/app/dashboard/services/[serviceId]/components/tabs/overview/service-overview-form-swarm.tsx (2)
apps/web/components/ui/form.tsx (5)
FormField(167-167)FormItem(162-162)FormLabel(163-163)FormControl(164-164)FormMessage(166-166)apps/web/components/ui/input.tsx (1)
Input(21-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (7)
packages/api/src/routers/services/updateSwarmServiceOverview.ts (1)
45-48: LGTM! Clean handling of optional command field.The logic correctly normalizes empty/whitespace-only strings to null while preserving provided values. This prevents storing meaningless commands in the database.
packages/db/prisma/schema.prisma (1)
223-223: LGTM! Standard schema addition.The optional
commandfield is properly typed and aligns with the database migration.packages/db/prisma/migrations/20251210015009_add_command_on_swarm_service/migration.sql (1)
1-2: LGTM! Safe migration.Adding a nullable column is a safe operation that doesn't require data migration or downtime.
packages/utils/src/index.ts (1)
16-16: LGTM! Standard re-export.The cli module is now properly exposed through the utils package index.
packages/utils/src/docker/swarm/deploySwarmService.ts (2)
12-12: LGTM! Proper import.
102-105: LGTM! Clean integration of command field.The command is correctly parsed using
splitCommandand set on the ContainerSpec only when provided. The placement after image configuration and before networks is logical.packages/utils/src/cli.ts (1)
1-51: [rewritten comment]
[classification tag]
There was a problem hiding this comment.
Pull request overview
This PR adds support for custom command overrides in Docker Swarm services, allowing users to specify commands that override the default CMD from container images. The implementation includes database schema changes, a new command parsing utility, API updates, and UI enhancements.
- Introduced a new
splitCommandutility function to parse shell-like command strings into arrays for Docker Swarm API compatibility - Added database schema migration and API/schema changes to support the optional
commandfield on Swarm services - Updated service deployment logic to apply parsed commands to the Swarm TaskTemplate when specified
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/utils/src/index.ts | Exports the new cli module containing the splitCommand utility |
| packages/utils/src/cli.ts | Implements splitCommand function to parse shell commands with quote and escape handling |
| packages/utils/src/docker/swarm/deploySwarmService.ts | Integrates command parsing into service deployment, setting Command field in TaskTemplate when provided |
| packages/schemas/src/services.ts | Adds optional command field to updateSwarmServiceOverviewSchema |
| packages/db/prisma/schema.prisma | Adds nullable command column to SwarmService model |
| packages/db/prisma/migrations/20251210015009_add_command_on_swarm_service/migration.sql | Database migration to add command TEXT column to SwarmService table |
| packages/api/src/routers/services/updateSwarmServiceOverview.ts | Updates API procedure to handle command field, converting empty strings to null |
| apps/web/app/dashboard/services/[serviceId]/components/tabs/overview/service-overview-form-swarm.tsx | Adds command input field to the service overview form UI |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,51 @@ | |||
| export function splitCommand(cmd: string): string[] { | |||
There was a problem hiding this comment.
The splitCommand function lacks documentation. Add a JSDoc comment explaining:
- The purpose of the function
- How it handles quotes (single and double)
- How it handles escape sequences
- Example usage and expected output
- Any limitations or edge cases (e.g., unclosed quotes behavior)
This is particularly important since this function implements shell-like parsing logic that may not be immediately obvious to other developers.
packages/api/src/routers/services/updateSwarmServiceOverview.ts
Outdated
Show resolved
Hide resolved
| <FormLabel>Command</FormLabel> | ||
| <FormControl> | ||
| <Input placeholder='node index.js' {...field} /> |
There was a problem hiding this comment.
The command input field lacks helpful description or hint text to guide users. Consider adding a FormDescription component (similar to other form fields in the codebase) that explains:
- The format expected (e.g., "The command to run in the container, e.g., 'node index.js' or 'python app.py'")
- That it overrides the default CMD from the Docker image
- How quotes and special characters are handled
This would improve the user experience and reduce potential configuration errors.
| export function splitCommand(cmd: string): string[] { | ||
| const result: string[] = []; | ||
| let current = ''; | ||
| let quote: "'" | '"' | null = null; | ||
| let escaped = false; | ||
|
|
||
| for (let i = 0; i < cmd.length; i++) { | ||
| const c = cmd[i]; | ||
|
|
||
| if (escaped) { | ||
| current += c; | ||
| escaped = false; | ||
| continue; | ||
| } | ||
|
|
||
| if (c === '\\') { | ||
| escaped = true; | ||
| continue; | ||
| } | ||
|
|
||
| if (quote) { | ||
| if (c === quote) { | ||
| quote = null; | ||
| } else { | ||
| current += c; | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| if (c === "'" || c === '"') { | ||
| quote = c; | ||
| continue; | ||
| } | ||
|
|
||
| if (c === ' ') { | ||
| if (current.length > 0) { | ||
| result.push(current); | ||
| current = ''; | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| current += c; | ||
| } | ||
|
|
||
| if (current.length > 0) { | ||
| result.push(current); | ||
| } | ||
|
|
||
| return result; | ||
| } |
There was a problem hiding this comment.
The splitCommand function lacks test coverage. This is a critical utility function that parses shell commands with complex logic for handling quotes, escapes, and spaces. Edge cases that should be tested include:
- Commands with single and double quotes
- Escaped characters within and outside quotes
- Multiple consecutive spaces
- Trailing/leading spaces
- Unclosed quotes
- Empty strings
- Commands with only spaces
Given that other utility functions in this package have comprehensive test coverage (e.g., backups/cron.test.ts and backups/retention.test.ts), this function should follow the same pattern with a colocated cli.test.ts file.
| export function splitCommand(cmd: string): string[] { | ||
| const result: string[] = []; | ||
| let current = ''; | ||
| let quote: "'" | '"' | null = null; | ||
| let escaped = false; | ||
|
|
||
| for (let i = 0; i < cmd.length; i++) { | ||
| const c = cmd[i]; | ||
|
|
||
| if (escaped) { | ||
| current += c; | ||
| escaped = false; | ||
| continue; | ||
| } | ||
|
|
||
| if (c === '\\') { | ||
| escaped = true; | ||
| continue; | ||
| } | ||
|
|
||
| if (quote) { | ||
| if (c === quote) { | ||
| quote = null; | ||
| } else { | ||
| current += c; | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| if (c === "'" || c === '"') { | ||
| quote = c; | ||
| continue; | ||
| } | ||
|
|
||
| if (c === ' ') { | ||
| if (current.length > 0) { | ||
| result.push(current); | ||
| current = ''; | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| current += c; | ||
| } | ||
|
|
||
| if (current.length > 0) { | ||
| result.push(current); | ||
| } | ||
|
|
||
| return result; | ||
| } |
There was a problem hiding this comment.
The splitCommand function doesn't handle unclosed quotes, which will silently consume the rest of the command string. If a user inputs a command like node "index.js (missing closing quote), the function will treat everything after the opening quote as part of a single argument, which may lead to unexpected behavior. Consider either:
- Throwing an error when quotes are unclosed
- Auto-closing quotes at the end of the string
- Documenting this behavior if it's intentional
This is especially important since this function is used in service deployments where malformed commands could cause deployment failures that are difficult to debug.
…failure handling - Added container inspection in `deploySwarmService` for better debugging on task failure. - Improved service task monitoring by logging detailed container statuses and logs. - Added support for removing services upon deployment failure when not updating. - Enhanced error handling in `updateSwarmServiceOverview` by simplifying the command assignment logic. - Introduced `inspectContainer` and `rmService` utilities in Docker module.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/utils/src/docker/swarm/deploySwarmService.ts (1)
104-108: Address previous feedback: missing braces and command clearing behavior.The past review comments on these lines remain unaddressed:
- Missing braces: The multi-line if statement lacks braces, which reduces readability and safety.
- Empty array vs undefined: Setting
Command = []when the command is falsy differs from setting it toundefined. An empty array overrides the image's default CMD/ENTRYPOINT, whileundefinedpreserves it. When updating a service where the user has removed the command, the expected behavior is likely to revert to the image's default, not execute an empty command.Apply this diff to address both concerns:
- if (service.swarmService.command) + if (service.swarmService.command) { spec.TaskTemplate!.ContainerSpec!.Command = splitCommand( service.swarmService.command ); - else spec.TaskTemplate!.ContainerSpec!.Command = []; + } else { + spec.TaskTemplate!.ContainerSpec!.Command = undefined; + }
🧹 Nitpick comments (4)
packages/utils/src/docker/Docker.ts (1)
142-147: Add return type annotation for type safety.The
rmServicemethod lacks a return type annotation. While TypeScript can infer the type fromdockerRequest, explicitly typing the return value improves code clarity and type safety.Consider applying this diff to add the return type:
- async rmService(serviceId: string) { + async rmService(serviceId: string): Promise<void> { return await dockerRequest(this.connection, `/services/${serviceId}`, { method: 'DELETE', headers: {}, }); }Additionally, the empty
headers: {}object could potentially be omitted if thedockerRequestsignature allows it, simplifying the call.packages/utils/src/docker/swarm/deploySwarmService.ts (3)
237-238: Add braces for multi-line if statement.The if statement spans multiple lines visually but lacks braces, which reduces readability.
Apply this diff:
- if (firstTask.Status?.ContainerStatus?.ContainerID) + if (firstTask.Status?.ContainerStatus?.ContainerID) { containerId = firstTask.Status.ContainerStatus.ContainerID; + }
243-256: Refactor to use Docker API for container logs instead of SSH execution.The current implementation uses
remoteExecvia SSH to fetch container logs (docker container logscommand), which is inefficient and adds unnecessary complexity. The Docker API provides a dedicated/containers/{id}/logsendpoint that would be more direct and reliable.Additionally, several concerns:
- Line 248-249: The 5-second sleep before fetching logs appears arbitrary. What's the rationale?
- Line 250:
containerInfo.Name?.replace('/', '')could result inundefinedbeing passed to the template ifNameis undefined, though the optional chaining provides safety.- Line 254: The error message "The container exited with status 'exited'" is redundant.
Consider adding a
getContainerLogsmethod to theDockerclass inDocker.ts:async getContainerLogs(containerId: string, options?: { timestamps?: boolean }) { return await dockerRequest( this.connection, `/containers/${containerId}/logs?stdout=true&stderr=true${options?.timestamps ? '×tamps=true' : ''}`, { method: 'GET', headers: {}, } ); }Then refactor this section:
if (containerId !== '') { logger.debug(`Container ID : ${containerId}`); const containerInfo = await docker.inspectContainer(containerId); if (containerInfo.State?.Status === 'exited') { - logger.info('Waiting for container logs'); - await sleep(5000); - const command = sh`docker container logs ${containerInfo.Name?.replace('/', '')} --timestamps`; - logger.debug('Running ' + command); - const logs = await remoteExec(connection, command); + logger.info('Fetching container logs'); + const logs = await docker.getContainerLogs(containerId, { timestamps: true }); logger.info(logs); - logger.error("The container exited with status 'exited'"); + logger.error('The container exited unexpectedly'); break; } }
271-274: Consider adding error handling for service removal.The cleanup logic appropriately removes failed new deployments. However, the
rmServicecall could fail (e.g., service already removed, Docker daemon issues). While the outer try-catch (lines 277-281) will catch errors, adding specific error handling here would provide clearer feedback.Consider this refinement:
if (!isUp && !isUpdate) { - logger.info('Removing service'); - await docker.rmService(service.id); + try { + logger.info('Removing failed service'); + await docker.rmService(service.id); + logger.info('Service removed'); + } catch (e) { + logger.warn('Failed to remove service: ' + (e instanceof Error ? e.message : 'Unknown error')); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/api/src/routers/services/index.ts(0 hunks)packages/api/src/routers/services/updateSwarmServiceOverview.ts(1 hunks)packages/utils/src/docker/Docker.ts(2 hunks)packages/utils/src/docker/swarm/deploySwarmService.ts(5 hunks)
💤 Files with no reviewable changes (1)
- packages/api/src/routers/services/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/api/src/routers/services/updateSwarmServiceOverview.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/utils/src/docker/swarm/deploySwarmService.ts (3)
packages/utils/src/cli.ts (1)
splitCommand(1-51)packages/utils/src/sh.ts (1)
sh(35-46)packages/utils/src/interactiveRemoteCommand.ts (1)
remoteExec(22-60)
🔇 Additional comments (8)
packages/utils/src/docker/Docker.ts (2)
1-1: LGTM: Import addition supports new rmService method.The addition of
dockerRequestto the imports is necessary for thermServicemethod implementation.
135-140: LGTM: Container inspection method follows established patterns.The
inspectContainermethod implementation is consistent with other inspection methods in this class (inspectService,inspectNetwork). Error handling is delegated to callers, which aligns with the existing pattern.packages/utils/src/docker/swarm/deploySwarmService.ts (6)
12-14: LGTM: Imports support enhanced deployment monitoring.The new imports (
splitCommand,remoteExec,sh) are correctly added and utilized for command parsing (line 105), log retrieval (line 252), and shell command construction (line 250).
138-139: LGTM: Service ID label aids service tracking.Adding the
app.seastack.serviceIdlabel enables easier identification and filtering of services managed by SeaStack.
205-206: LGTM: State tracking variables for deployment monitoring.The
containerIdandlastTaskIdvariables appropriately track container and task progression across wait loop iterations.
208-208: LGTM: Improved log message timing.Moving the waiting message before the sleep call provides better user feedback by indicating the wait state immediately.
224-230: LGTM: Early failure detection improves deployment monitoring.The check for previous task failures enables faster failure detection. The code safely handles cases where the task is no longer in the list.
264-268: LGTM: Task running state detection.The logic correctly identifies when the service is running and maintains the
lastTaskIdfor subsequent iterations.
- Updated Prisma schema and config paths to align with the apps/web directory structure. - Adjusted `PRISMA_SCHEMA_PATH` environment variable accordingly.
commandfield in Swarm service schema and API procedures.commandcolumn in the database.splitCommandutility for parsing shell commands.commandfield, passing parsed commands to the Swarm TaskTemplate.Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.