Skip to content

Set cron source to `schedule` to ensure clean up on load

Sylvester Chin requested to merge sc1-fix-cron-cleanup-on-load into master

What does this MR do and why?

Set cron source to schedule to ensure clean up on load.

This fixes the problem of crons lingering after the worker class is removed from Gitlab::SidekiqConfig.cron_jobs.

Addresses the missed change when we bumped sidekiq-cron to v1.12.0 in !139267 (merged). In particular https://github.com/sidekiq-cron/sidekiq-cron/pull/431 introduction of dynamic/schedule source types.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

  1. Open gdk rails console
  2. Run Sidekiq::Cron::Job.all.filter_map { |j| j.name if j.source == "schedule" } after doing a gdk restart rails
  3. On master branch, this should be [] while on the fix, it should be non-empty.

This shows that https://github.com/sidekiq-cron/sidekiq-cron/blob/v1.12.0/lib/sidekiq/cron/job.rb#L534 would perform a set subtract to obtain a list of removed crons and perform Sidekiq::Cron::Job.destroy(j) on the list.

Edited by Sylvester Chin

Merge request reports