From 60de6fa4ec7bd04d3dd9a755b45ca1854ac19067 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Tue, 9 Sep 2025 11:07:34 +0100 Subject: [PATCH 1/3] Remove Good Job tables This removes the tables related to Good Job now that we're using Sidekiq to run background jobs. Jira-Issue: MAV-1948 --- db/migrate/20250909100315_drop_good_job.rb | 11 +++ db/schema.rb | 88 ---------------------- 2 files changed, 11 insertions(+), 88 deletions(-) create mode 100644 db/migrate/20250909100315_drop_good_job.rb diff --git a/db/migrate/20250909100315_drop_good_job.rb b/db/migrate/20250909100315_drop_good_job.rb new file mode 100644 index 0000000000..319e1598b8 --- /dev/null +++ b/db/migrate/20250909100315_drop_good_job.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class DropGoodJob < ActiveRecord::Migration[8.0] + def up + drop_table :good_jobs + drop_table :good_job_batches + drop_table :good_job_executions + drop_table :good_job_processes + drop_table :good_job_settings + end +end diff --git a/db/schema.rb b/db/schema.rb index e0520ce23d..bf1ee1f52b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -313,94 +313,6 @@ t.index ["session_date_id"], name: "index_gillick_assessments_on_session_date_id" end - create_table "good_job_batches", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.text "description" - t.jsonb "serialized_properties" - t.text "on_finish" - t.text "on_success" - t.text "on_discard" - t.text "callback_queue_name" - t.integer "callback_priority" - t.datetime "enqueued_at" - t.datetime "discarded_at" - t.datetime "finished_at" - end - - create_table "good_job_executions", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.uuid "active_job_id", null: false - t.text "job_class" - t.text "queue_name" - t.jsonb "serialized_params" - t.datetime "scheduled_at" - t.datetime "finished_at" - t.text "error" - t.integer "error_event", limit: 2 - t.text "error_backtrace", array: true - t.uuid "process_id" - t.interval "duration" - t.index ["active_job_id", "created_at"], name: "index_good_job_executions_on_active_job_id_and_created_at" - t.index ["process_id", "created_at"], name: "index_good_job_executions_on_process_id_and_created_at" - end - - create_table "good_job_processes", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.jsonb "state" - t.integer "lock_type", limit: 2 - end - - create_table "good_job_settings", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.text "key" - t.jsonb "value" - t.index ["key"], name: "index_good_job_settings_on_key", unique: true - end - - create_table "good_jobs", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.text "queue_name" - t.integer "priority" - t.jsonb "serialized_params" - t.datetime "scheduled_at" - t.datetime "performed_at" - t.datetime "finished_at" - t.text "error" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.uuid "active_job_id" - t.text "concurrency_key" - t.text "cron_key" - t.uuid "retried_good_job_id" - t.datetime "cron_at" - t.uuid "batch_id" - t.uuid "batch_callback_id" - t.boolean "is_discrete" - t.integer "executions_count" - t.text "job_class" - t.integer "error_event", limit: 2 - t.text "labels", array: true - t.uuid "locked_by_id" - t.datetime "locked_at" - t.index ["active_job_id", "created_at"], name: "index_good_jobs_on_active_job_id_and_created_at" - t.index ["batch_callback_id"], name: "index_good_jobs_on_batch_callback_id", where: "(batch_callback_id IS NOT NULL)" - t.index ["batch_id"], name: "index_good_jobs_on_batch_id", where: "(batch_id IS NOT NULL)" - t.index ["concurrency_key"], name: "index_good_jobs_on_concurrency_key_when_unfinished", where: "(finished_at IS NULL)" - t.index ["cron_key", "created_at"], name: "index_good_jobs_on_cron_key_and_created_at_cond", where: "(cron_key IS NOT NULL)" - t.index ["cron_key", "cron_at"], name: "index_good_jobs_on_cron_key_and_cron_at_cond", unique: true, where: "(cron_key IS NOT NULL)" - t.index ["finished_at"], name: "index_good_jobs_jobs_on_finished_at", where: "((retried_good_job_id IS NULL) AND (finished_at IS NOT NULL))" - t.index ["labels"], name: "index_good_jobs_on_labels", where: "(labels IS NOT NULL)", using: :gin - t.index ["locked_by_id"], name: "index_good_jobs_on_locked_by_id", where: "(locked_by_id IS NOT NULL)" - t.index ["priority", "created_at"], name: "index_good_job_jobs_for_candidate_lookup", where: "(finished_at IS NULL)" - t.index ["priority", "created_at"], name: "index_good_jobs_jobs_on_priority_created_at_when_unfinished", order: { priority: "DESC NULLS LAST" }, where: "(finished_at IS NULL)" - t.index ["priority", "scheduled_at"], name: "index_good_jobs_on_priority_scheduled_at_unfinished_unlocked", where: "((finished_at IS NULL) AND (locked_by_id IS NULL))" - t.index ["queue_name", "scheduled_at"], name: "index_good_jobs_on_queue_name_and_scheduled_at", where: "(finished_at IS NULL)" - t.index ["scheduled_at"], name: "index_good_jobs_on_scheduled_at", where: "(finished_at IS NULL)" - end - create_table "health_questions", force: :cascade do |t| t.string "title", null: false t.bigint "vaccine_id", null: false From f44ae7b4d785b3ece4108cec046810859946b5d1 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Tue, 9 Sep 2025 11:21:38 +0100 Subject: [PATCH 2/3] Remove Good Job from application This removes references to Good Job from the application code as it is no longer being used. Jira-Issue: MAV-1948 --- Gemfile | 1 - Gemfile.lock | 12 ++---------- config/application.rb | 1 - config/environments/development.rb | 3 --- config/environments/test.rb | 3 --- config/initializers/good_job.rb | 14 -------------- config/puma.rb | 17 ----------------- config/routes.rb | 2 -- 8 files changed, 2 insertions(+), 51 deletions(-) delete mode 100644 config/initializers/good_job.rb diff --git a/Gemfile b/Gemfile index 96ceffda29..19a74ba86a 100644 --- a/Gemfile +++ b/Gemfile @@ -39,7 +39,6 @@ gem "fhir_models" gem "flipper" gem "flipper-active_record" gem "flipper-ui" -gem "good_job" gem "govuk-components" gem "govuk_design_system_formbuilder" gem "govuk_markdown" diff --git a/Gemfile.lock b/Gemfile.lock index c537d89ac4..d6cdaee172 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -220,7 +220,7 @@ GEM activemodel erb (5.0.2) erubi (1.13.1) - et-orbi (1.2.11) + et-orbi (1.3.0) tzinfo factory_bot (6.5.5) activesupport (>= 6.1.0) @@ -265,18 +265,11 @@ GEM rack-protection (>= 1.5.3, < 5.0.0) rack-session (>= 1.0.2, < 3.0.0) sanitize (< 8) - fugit (1.11.1) + fugit (1.11.2) et-orbi (~> 1, >= 1.2.11) raabro (~> 1.4) globalid (1.2.1) activesupport (>= 6.1) - good_job (4.11.2) - activejob (>= 6.1.0) - activerecord (>= 6.1.0) - concurrent-ruby (>= 1.3.1) - fugit (>= 1.11.0) - railties (>= 6.1.0) - thor (>= 1.0.0) govuk-components (5.11.3) html-attributes-utils (~> 1.0.0, >= 1.0.0) pagy (>= 6, < 10) @@ -800,7 +793,6 @@ DEPENDENCIES flipper flipper-active_record flipper-ui - good_job govuk-components govuk_design_system_formbuilder govuk_markdown diff --git a/config/application.rb b/config/application.rb index a2ce2634da..630da8996c 100644 --- a/config/application.rb +++ b/config/application.rb @@ -76,7 +76,6 @@ class Application < Rails::Application config.active_model.i18n_customize_full_message = true config.active_job.queue_adapter = :sidekiq - config.good_job.execution_mode = :external config.view_component.default_preview_layout = "component_preview" config.view_component.previews.controller = "ComponentPreviewsController" diff --git a/config/environments/development.rb b/config/environments/development.rb index 7f57ba92bb..d22f42f946 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -81,9 +81,6 @@ # Prevent health checks from clogging up the logs. config.silence_healthcheck_path = "/up" - # Set up GoodJob for async execution in development mode - config.good_job.execution_mode = :async - # Enable strict loading to catch N+1 problems. config.active_record.strict_loading_by_default = true config.active_record.strict_loading_mode = :n_plus_one_only diff --git a/config/environments/test.rb b/config/environments/test.rb index 21549b8d52..e11ee6722e 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -47,9 +47,6 @@ # Raise error when a before_action's only/except options reference missing actions. config.action_controller.raise_on_missing_callback_actions = true - # Set up GoodJob for inline execution in test mode - config.good_job.execution_mode = :inline - # Enable strict loading to catch N+1 problems. config.active_record.strict_loading_by_default = true config.active_record.strict_loading_mode = :n_plus_one_only diff --git a/config/initializers/good_job.rb b/config/initializers/good_job.rb deleted file mode 100644 index 416e1bf1f1..0000000000 --- a/config/initializers/good_job.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -GoodJob::Engine - .middleware - .use(Rack::Auth::Basic) do |username, password| - ActiveSupport::SecurityUtils.secure_compare( - Rails.application.credentials.support_username, - username - ) && - ActiveSupport::SecurityUtils.secure_compare( - Rails.application.credentials.support_password, - password - ) - end diff --git a/config/puma.rb b/config/puma.rb index d3f0183ec2..aea37387fa 100644 --- a/config/puma.rb +++ b/config/puma.rb @@ -57,22 +57,5 @@ # processes). workers Settings.web_concurrency -if Settings.web_concurrency > 1 && - Rails.configuration.good_job.execution_mode != :external - # Cleanly shut down GoodJob when Puma is shut down. - # See https://github.com/bensheldon/good_job#execute-jobs-async--in-process - MAIN_PID = Process.pid - before_fork { GoodJob.shutdown } - before_worker_boot { GoodJob.restart } - before_worker_shutdown { GoodJob.shutdown } - at_exit { GoodJob.shutdown if Process.pid == MAIN_PID } - - # Use the `preload_app!` method when specifying a `workers` number. - # This directive tells Puma to first boot the application and load code - # before forking the application. This takes advantage of Copy On Write - # process behavior so workers use less memory. - preload_app! -end - # Re-open appenders after forking the process; needed for Semantic Logger before_worker_boot { SemanticLogger.reopen } diff --git a/config/routes.rb b/config/routes.rb index 40573f46e8..609c8375cb 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -42,8 +42,6 @@ root to: redirect("/start") - mount GoodJob::Engine => "/good-job" - Sidekiq::Web.use Rack::Auth::Basic do |username, password| ActiveSupport::SecurityUtils.secure_compare( Rails.application.credentials.support_username, From 796e58e1c792a0f9ffdf2ac5aee834c535bb27f8 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Tue, 9 Sep 2025 11:26:07 +0100 Subject: [PATCH 3/3] Remove Good Job from infrastructure This removes references to Good Job from the infrastructure code as it is no longer being used. Jira-Issue: MAV-1948 --- .github/workflows/deploy-application.yml | 47 ------------------------ .github/workflows/deploy.yml | 1 - Dockerfile | 1 - bin/docker-start | 5 +-- bin/good_job | 5 --- config/database.yml | 2 +- docs/terraform.md | 2 +- terraform/app/ecs.tf | 26 ------------- terraform/app/env/sandbox-alpha.tfvars | 1 - terraform/app/env/sandbox-beta.tfvars | 1 - terraform/app/outputs.tf | 4 -- terraform/app/variables.tf | 9 +---- 12 files changed, 4 insertions(+), 100 deletions(-) delete mode 100755 bin/good_job diff --git a/.github/workflows/deploy-application.yml b/.github/workflows/deploy-application.yml index ba470fa7da..47492a77ad 100644 --- a/.github/workflows/deploy-application.yml +++ b/.github/workflows/deploy-application.yml @@ -23,7 +23,6 @@ on: options: - all - web - - good-job - sidekiq default: all workflow_call: @@ -82,8 +81,6 @@ jobs: "application=" + .codedeploy_application_name.value, "application_group=" + .codedeploy_deployment_group_name.value, "cluster_name=" + .ecs_variables.value.cluster_name, - "good_job_service=" + .ecs_variables.value.good_job.service_name, - "good_job_task_definition=" + .ecs_variables.value.good_job.task_definition.arn, "sidekiq_service=" + .ecs_variables.value.sidekiq.service_name, "sidekiq_task_definition=" + .ecs_variables.value.sidekiq.task_definition.arn ' > ${{ runner.temp }}/DEPLOYMENT_ENVS @@ -126,50 +123,6 @@ jobs: aws deploy wait deployment-successful --deployment-id "$deployment_id" echo "Deployment successful" - create-good-job-deployment: - name: Create good-job deployment - runs-on: ubuntu-latest - needs: prepare-deployment - if: inputs.server_types == 'good-job' || inputs.server_types == 'all' - permissions: - id-token: write - steps: - - name: Download Artifact - uses: actions/download-artifact@v5 - with: - name: DEPLOYMENT_ENVS-${{ inputs.environment }} - path: ${{ runner.temp }} - - name: Configure AWS Credentials - uses: aws-actions/configure-aws-credentials@v5 - with: - role-to-assume: ${{ env.aws-role }} - aws-region: eu-west-2 - - name: Trigger ECS Deployment - run: | - set -e - source ${{ runner.temp }}/DEPLOYMENT_ENVS - DEPLOYMENT_ID=$(aws ecs update-service --cluster $cluster_name --service $good_job_service \ - --task-definition $good_job_task_definition --force-new-deployment \ - --query 'service.deployments[?rolloutState==`IN_PROGRESS`].[id][0]' --output text) - echo "Deployment started: $DEPLOYMENT_ID" - echo "deployment_id=$DEPLOYMENT_ID" >> $GITHUB_ENV - - name: Wait for deployment to complete - run: | - set -e - source ${{ runner.temp }}/DEPLOYMENT_ENVS - DEPLOYMENT_STATE=IN_PROGRESS - while [ "$DEPLOYMENT_STATE" == "IN_PROGRESS" ]; do - echo "Waiting for deployment to complete..." - sleep 30 - DEPLOYMENT_STATE="$(aws ecs describe-services --cluster $cluster_name --services $good_job_service \ - --query "services[0].deployments[?id == \`$deployment_id\`].[rolloutState][0]" --output text)" - done - if [ "$DEPLOYMENT_STATE" != "COMPLETED" ]; then - echo "Deployment failed with state: $DEPLOYMENT_STATE" - exit 1 - fi - echo "Deployment successful" - create-sidekiq-deployment: name: Create sidekiq deployment runs-on: ubuntu-latest diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 37da0c0f51..81f5d5351d 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -40,7 +40,6 @@ on: options: - all - web - - good-job - sidekiq - none default: all diff --git a/Dockerfile b/Dockerfile index c3ff7323ba..ec24aee4a3 100644 --- a/Dockerfile +++ b/Dockerfile @@ -85,6 +85,5 @@ VOLUME ["/rails/db/data", "/rails/tmp", "/rails/log", "/tmp", "/var/log", "/var/ # Start web server by default, this can be overwritten by environment variable EXPOSE 4000 ENV HTTP_PORT=4000 -ENV GOOD_JOB_PROBE_PORT=4000 ENV SERVER_TYPE=web CMD ["./bin/docker-start"] diff --git a/bin/docker-start b/bin/docker-start index 17fa5448d7..52fe86d30d 100755 --- a/bin/docker-start +++ b/bin/docker-start @@ -5,9 +5,6 @@ BIN_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) if [ "$SERVER_TYPE" == "web" ]; then echo "Starting web server..." exec "$BIN_DIR"/thrust "$BIN_DIR"/rails server -elif [ "$SERVER_TYPE" == "good-job" ]; then - echo "Starting good-job server..." - exec "$BIN_DIR"/good_job start elif [ "$SERVER_TYPE" == "sidekiq" ]; then echo "Starting sidekiq server..." exec "$BIN_DIR"/sidekiq @@ -15,6 +12,6 @@ elif [ "$SERVER_TYPE" == "none" ]; then echo "No server started" exec tail -f /dev/null # Keep container running else - echo "SERVER_TYPE variable: '$SERVER_TYPE' unknown. Allowed values ['web','good-job', 'none']" + echo "SERVER_TYPE variable: '$SERVER_TYPE' unknown. Allowed values: web, sidekiq, none" exit 1 fi diff --git a/bin/good_job b/bin/good_job deleted file mode 100755 index e0e8f970f4..0000000000 --- a/bin/good_job +++ /dev/null @@ -1,5 +0,0 @@ -#!/usr/bin/env ruby -require "rubygems" -require "bundler/setup" - -load Gem.bin_path("good_job", "good_job") diff --git a/config/database.yml b/config/database.yml index 37e22c602d..2539122e18 100644 --- a/config/database.yml +++ b/config/database.yml @@ -1,7 +1,7 @@ default: &default adapter: postgresql encoding: unicode - pool: <%= ENV.fetch("RAILS_MAX_THREADS", 5).to_i + ENV.fetch("GOOD_JOB_MAX_THREADS", 4).to_i + ENV.fetch("SIDEKIQ_CONCURRENCY", 4).to_i %> + pool: <%= ENV.fetch("RAILS_MAX_THREADS", 5).to_i + ENV.fetch("SIDEKIQ_CONCURRENCY", 4).to_i %> development: <<: *default diff --git a/docs/terraform.md b/docs/terraform.md index e1bae1bc42..ff404a5418 100644 --- a/docs/terraform.md +++ b/docs/terraform.md @@ -106,6 +106,6 @@ tf apply -var-file=env/$env.tfvars -var="image_digest=" ``` Step 3: Run Codedeploy from the AWS Console -Step 4: If needed, trigger a deployment for the good-job service from the AWS ECS Console +Step 4: If needed, trigger a deployment for the sidekiq service from the AWS ECS Console For a more high-level description of the process see [deployment-process.md](../terraform/documentation/deployment-process.md) diff --git a/terraform/app/ecs.tf b/terraform/app/ecs.tf index 47f3642243..a7daabcf5f 100644 --- a/terraform/app/ecs.tf +++ b/terraform/app/ecs.tf @@ -58,32 +58,6 @@ module "web_service" { deployment_controller = "CODE_DEPLOY" } -module "good_job_service" { - source = "./modules/ecs_service" - task_config = { - environment = local.task_envs - secrets = local.task_secrets - cpu = 1024 - memory = 2048 - docker_image = "${var.account_id}.dkr.ecr.eu-west-2.amazonaws.com/${var.docker_image}@${var.image_digest}" - execution_role_arn = aws_iam_role.ecs_task_execution_role.arn - task_role_arn = aws_iam_role.ecs_task_role.arn - log_group_name = aws_cloudwatch_log_group.ecs_log_group.name - region = var.region - health_check_command = ["CMD-SHELL", "./bin/internal_healthcheck http://localhost:4000/status/connected"] - } - network_params = { - subnets = [aws_subnet.private_subnet_a.id, aws_subnet.private_subnet_b.id] - vpc_id = aws_vpc.application_vpc.id - } - minimum_replica_count = var.good_job_replicas - maximum_replica_count = var.good_job_replicas - cluster_id = aws_ecs_cluster.cluster.id - cluster_name = aws_ecs_cluster.cluster.name - environment = var.environment - server_type = "good-job" -} - module "sidekiq_service" { source = "./modules/ecs_service" task_config = { diff --git a/terraform/app/env/sandbox-alpha.tfvars b/terraform/app/env/sandbox-alpha.tfvars index b92dcf7197..b07127fc38 100644 --- a/terraform/app/env/sandbox-alpha.tfvars +++ b/terraform/app/env/sandbox-alpha.tfvars @@ -21,7 +21,6 @@ minimum_web_replicas = 1 maximum_web_replicas = 2 minimum_sidekiq_replicas = 1 maximum_sidekiq_replicas = 2 -good_job_replicas = 1 valkey_node_type = "cache.t4g.micro" valkey_log_retention_days = 3 diff --git a/terraform/app/env/sandbox-beta.tfvars b/terraform/app/env/sandbox-beta.tfvars index 87644ae9cf..0daa27a468 100644 --- a/terraform/app/env/sandbox-beta.tfvars +++ b/terraform/app/env/sandbox-beta.tfvars @@ -21,7 +21,6 @@ minimum_web_replicas = 1 maximum_web_replicas = 2 minimum_sidekiq_replicas = 1 maximum_sidekiq_replicas = 2 -good_job_replicas = 1 # Valkey serverless configuration - minimal settings for sandbox valkey_node_type = "cache.t4g.micro" diff --git a/terraform/app/outputs.tf b/terraform/app/outputs.tf index cf4458c0d7..ba58c5f4de 100644 --- a/terraform/app/outputs.tf +++ b/terraform/app/outputs.tf @@ -26,10 +26,6 @@ output "codedeploy_deployment_group_name" { output "ecs_variables" { value = { cluster_name = aws_ecs_cluster.cluster.name - good_job = { - service_name = module.good_job_service.service.name - task_definition = module.good_job_service.task_definition - } sidekiq = { service_name = module.sidekiq_service.service.name task_definition = module.sidekiq_service.task_definition diff --git a/terraform/app/variables.tf b/terraform/app/variables.tf index 92aa246c96..3ad3e274dd 100644 --- a/terraform/app/variables.tf +++ b/terraform/app/variables.tf @@ -196,7 +196,6 @@ locals { MAVIS__ACADEMIC_YEAR_NUMBER_OF_PREPARATION_DAYS = var.academic_year_number_of_preparation_days MAVIS__PDS__ENQUEUE_BULK_UPDATES = var.enable_pds_enqueue_bulk_updates ? "true" : "false" MAVIS__PDS__RATE_LIMIT_PER_SECOND = var.pds_rate_limit_per_second - GOOD_JOB_MAX_THREADS = 5 SIDEKIQ_CONCURRENCY = 5 }) parameter_store_config_list = [for key, value in local.parameter_store_variables : { @@ -328,12 +327,6 @@ variable "maximum_web_replicas" { description = "Maximum amount of allowed replicas for web service" } -variable "good_job_replicas" { - type = number - default = 2 - description = "Amount of replicas for the good-job service" -} - variable "minimum_sidekiq_replicas" { type = number default = 2 @@ -434,6 +427,6 @@ variable "valkey_log_retention_days" { locals { ecs_initial_lb_target_group = var.active_lb_target_group == "green" ? aws_lb_target_group.green.arn : aws_lb_target_group.blue.arn - ecs_sg_ids = [module.web_service.security_group_id, module.good_job_service.security_group_id, module.sidekiq_service.security_group_id] + ecs_sg_ids = [module.web_service.security_group_id, module.sidekiq_service.security_group_id] valkey_cache_availability_zones = var.valkey_failover_enabled ? [aws_subnet.private_subnet_a.availability_zone, aws_subnet.private_subnet_b.availability_zone] : [aws_subnet.private_subnet_a.availability_zone] }