Introduce Solid Queue worker task#1558
Closed
sarahseewhy wants to merge 5 commits into
Closed
Conversation
sarahseewhy
force-pushed
the
run-solid-queue-as-ecs-task
branch
from
8c70c6f to
1f22ea2
Compare
sarahseewhy
force-pushed
the
run-solid-queue-as-ecs-task
branch
11 times, most recently
from
aa50231 to
67bebf8
Compare
AP-Hunt
suggested changes
May 23, 2025
Contributor
AP-Hunt
left a comment
AP-Hunt
left a comment
There was a problem hiding this comment.
This looks good to me, bar a couple small style things.
I reviewed it commit by commit without reading all the commits before I started, so I've made some comments you can dismiss because they're not relevant to the final product.
sarahseewhy
force-pushed
the
run-solid-queue-as-ecs-task
branch
from
67bebf8 to
ce7d9a2
Compare
sarahseewhy
force-pushed
the
run-solid-queue-as-ecs-task
branch
8 times, most recently
from
752d237 to
3695c70
Compare
* Create ECS service, ECS task definition, security group, and relevant security group rules. I used a pattern I noticed in mailchimp sync where I took the exported ECS task container definition and override some parts of it * Remove lock file which shouldn't have been committed
* Add a Parameter Store entry for a forms-runner-queue-worker specific Sentry DSN * Provide the task access to a subset of secrets (I checked with the devs and it will only need access to these secrets, the worker won't need access to NOTIFY_API_KEY or SUBMISSION_STATUS_API)
* Create an IAM task role for the queue worker with permissions to write to CloudWatch (I don't think it needs the other permissions forms-runner has, e.g., SMS, SQS, KMS decryption) * Create an IAM task execution role for the queue worker with permissions to read relevant Parameter Store values (again, I don't think the queue worker needs access to all the secrets forms-runner has access to). * This code somewhat duplicates what's in `ecs-service/iam.tf` and the policies declared in `forms-runner/main.tf` but I think the duplication is simpler than forcing the ECS task into the ecs-service module format.
* Use the `forms-runner` task role (`module.ecs_service.task_role_arn`). The previous configuration generated an error because the queue worker didn't have IAM permissions to access the SQS `submission_email_ses_bounces_and_complaints` queue * Extract queue worker name into a local variable to make it easier to change the name in the future. Possibly unnecessary but it was a right faff having to write `forms-runner-queue-worker` all over the place * Rename file to make it a bit more generic
* Remove SSM Parameter and raise this in a separate PR. * This is to ensure the secret exists before the ECS tasks are created in all environments so there are no failures on creation. * If I leave the SSM Parameter in this PR the dummy value will exist when the ECS task gets created and there will be failures.
sarahseewhy
force-pushed
the
run-solid-queue-as-ecs-task
branch
from
430c628 to
356052e
Compare
Contributor
Author
|
Closing this PR because I believe the history is messy to the point of confusion. Recreated the PR here: #1592 |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
What problem does this pull request solve?
Trello card: https://trello.com/c/f7js82Cm/635-run-solid-queue-as-its-own-ecs-task
This pull request creates new ECS task, service, and relevant configuration to run Solid Queue as a separate worker task in our ECS cluster. It contains the same ENV variables as the
forms-runnerweb app (including Sentry config like DSN, theoretically this means the work will also send alerts to Sentry).Where possible I reused code, for example the
task_container_definition, with a few overrides (similar to themailchimp-syncapproach).The new ECS task should have egress access to the VPC, RDS, and the internet, but no ingress access (which happens by default unless otherwise specified in Terraform). The new ECS task should also exist in the same VPC and private subnet group as the other
forms-runnertasks/services.I used the same security group rules configured in
modules/ecs-service/security-groups.tfsince they should match the existingforms-runnerECS task with the exception of the restricted ingress access.I've added a HealthCheck which depends on a PR in forms-runner (some Rails code to create the
healthcheckfile, see here).I can also confirm that logs from the new ECS task are shipped to Splunk, use query
index="gds_dsp_dev_forms" log_stream="forms-runner-dev-queue-worker*"to view.I've tested the changes in
dev(and locally) and have attached some screen shots:Things to consider when reviewing