Skip to content

Add an inner class in the cleanup container repository worker [RUN ALL RSPEC] [RUN AS-IF-FOSS]

🎱 Context

Cleanup policies are tasks executed by a background worker. We're not going to detail the task here but it's a non trivial process.

Currently, each enabled cleanup policy ready to run is read by a cron worker. This cron worker will:

  1. "Mark" container repositories
  2. Enqueue jobs for the related limited capacity worker

The main problem is that (1.) is not scalable. The SQL query executed to get all the runnable policies is quite complex. In addition, the worker loops on each row to then mark the linked container repositories.

This is issue #267546 (closed).

The found solution is detailed in #267546 (closed).

Since we're changing the whole logic for both workers, this effort is splitted in steps and gated behind a feature flag so that we can "rollback" to the current logic at will:

  1. Introduce the feature flag + update the cron worker according to #267546 (closed). This is !56962 (merged).
  2. (You are here ) Prepare the limited worker for changes.
  3. Update the limited worker to support the new logic

🔬 What does this MR do?

  • Prepare the limited worker for changes
    • Basically, I wanted to come up with a clear way to implement both logics: with looping on policies and without looping.
    • At the same time, I have this constraint that the worker class can't change because we do have a dashboard built out of the logs generated by the worker and those logs have the worker class name.
    • Those two needs led me to add inner classes that would encapsulate the logic. In other words, we will have two inner classes: one for the "old" logic and one for the new one (loopless)
    • Inner classes help me to organize the code in a more clearer way and they also provide a nice way to separate the old logic from the new one = it will be way easier to cleanup the old logic once the feature flag will be removed.
    • Common functions such as logging are done using the instance of the parent class which is the worker class. This way, from the outside world, the worker class is behaving as usual but internally, we're using internal classes to switch the logic.
  • Add the NonLoopless inner class.
    • This class holds the old logic.
  • Add the Loopless inner class.
    • It's an empty structure for now. Step (3.) from the plan above will fill it.
  • Update the related specs
    • I created dedicated contexts for the "non loopless" mode. This is mainly to ease the feature flag removal.
  • Please note that this MR doesn't add or remove any functionality. It merely moves the existing code.

🚫 What this MR does not do

  • Implement the loopless logic
    • That's step (3.) and it is done in a follow up MR

📷 Screenshots (strongly suggested)

N / a.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team
Edited by David Fernandez

Merge request reports