Skip to content
Snippets Groups Projects

Add feature to delete inactive projects

Merged Huzaifa Iftikhar requested to merge 357376_add_inactive_projects_deletion_feature into master
All threads resolved!

What does this MR do and why?

  1. This MR currently targets !86211 (merged), once that MR is merged to master we can update the base branch to master for this MR as well. The dependent MR has been merged and this MR now targets master.
  2. Add inactive projects deletion feature.
  3. Add a cronjob that runs once daily, finds the inactive projects and perform the following action:
  4. Send deletion warning email to the inactive project's maintainers and owners. Store the date when the email was sent in redis hash.
  5. If we've already sent an email then it checks whether the project is still inactive for a duration greater than inactive_projects_delete_after_months defined in the application_settings table. In case the project is still inactive it permanently deletes the project.
  6. When an activity is performed on an inactive project after the deletion warning email was sent, we remove that project's key from the redis hash.
  7. The documentation is added in a separate MR !86907 (merged)

Screenshots or screen recordings

HTML version Plain text
image image
Instance audit events Project audit events
image image
image

How to set up and validate locally

  1. Create a project under a group A. Let's call it as Project A.
  2. Create another project in group B. Let's call it Project B.
  3. Modify the last_activity_at of both the projects so that they are considered as inactive. For example:
project = Project.find(<id of Project A>)
project.last_activity_at = 3.years.ago
project.save!
  1. Enable feature flag for group A only. Feature.enable(:inactive_projects_deletion, Namespace.without_project_namespaces.find_by_full_path('<name of Group A>'))
  2. Enable inactive projects deletion settings using the application_settings API. Example : https://gdk.test:3000/api/v4/application/settings?delete_inactive_projects=true&inactive_projects_send_warning_email_after_months=6&inactive_projects_delete_after_months=10&inactive_projects_min_size_mb=0
  3. Connect to your local redis using CLI - redis-cli -s <path-to-gitlab-development-kit>/redis/redis.socket. Verify that redis hash is empty by running hgetall inactive_projects_deletion_warning_email_notified inside the redis CLI.
  4. Manually modify 1_settings.rb file and update the inactive_projects_deletion_cron_worker cron timing to run every minute. This will help us test the feature fast. Restart the background jobs using gdk restart rails-background-jobs
    • Settings.cron_jobs['inactive_projects_deletion_cron_worker']['cron'] ||= '0 1 * * *'
    • Settings.cron_jobs['inactive_projects_deletion_cron_worker']['cron'] ||= '* * * * *'
  5. Wait for Projects::InactiveProjectsDeletionCronWorker to run. You can tail the logs gdk tail rails-background-jobs | grep -i inactive.
  6. After the cron has executed once, visit https://gdk.test:3000/rails/letter_opener/ and verify that we've received the deletion warning email for Project A. Also verify that no email should have been received for Project B (we didn't enable the feature flag for this project).
  7. Verify that the redis key has been updated. Run hgetall inactive_projects_deletion_warning_email_notified in redis-cli and verify that the key for project A exists. The value should be the current date which signifies the date when the deletion warning email was sent.
  8. Manually update the redis value to simulate that the deletion warning email was sent more than (inactive_projects_delete_after_months - inactive_projects_send_warning_email_after_months) months ago (4 months as per our example). Run the following to update via redis-cli hset inactive_projects_deletion_warning_email_notified project:<project_if of Project A> 2021-04-25
  9. Wait for Projects::InactiveProjectsDeletionCronWorker to run again. This time the project should get deleted immediately or should get marked for deletion if the project had enabled delayed project deletion.
  10. Visit the homepage of Project A and verify that the project is either deleted or marked for deletion.
  11. Visit the homepage of Project B and verify that the project is neither deleted nor marked for deletion.
  12. Verify that the redis key has been deleted. hgetall inactive_projects_deletion_warning_email_notified
  13. Visit the instance audit events dashboard and verify that audit events for deletion warning emails are captured successfully.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/357376

Edited by Huzaifa Iftikhar

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Allure report

    allure-report-publisher generated test report!

    review-qa-blocking: :pencil: test report

    review-qa-blocking: :exclamation: test report for 17569f44

    expand test summary
    +-------------------------------------------------------------------+
    |                          suites summary                           |
    +----------------------+--------+--------+---------+-------+--------+
    |                      | passed | failed | skipped | flaky | result |
    +----------------------+--------+--------+---------+-------+--------+
    | Create               | 24     | 0      | 2       | 22    | ❗     |
    | Manage               | 31     | 0      | 2       | 32    | ❗     |
    | Plan                 | 41     | 0      | 1       | 41    | ❗     |
    | Verify               | 12     | 0      | 1       | 12    | ❗     |
    | Configure            | 0      | 0      | 1       | 0     | ➖     |
    | Protect              | 2      | 0      | 0       | 2     | ❗     |
    | Version sanity check | 0      | 0      | 1       | 0     | ➖     |
    | Package              | 0      | 0      | 1       | 0     | ➖     |
    +----------------------+--------+--------+---------+-------+--------+
    | Total                | 110    | 0      | 9       | 109   | ❗     |
    +----------------------+--------+--------+---------+-------+--------+

    package-and-qa-ff-enabled: :x: test report for 17569f44

    expand test summary
    +-------------------------------------------------------------------+
    |                          suites summary                           |
    +----------------------+--------+--------+---------+-------+--------+
    |                      | passed | failed | skipped | flaky | result |
    +----------------------+--------+--------+---------+-------+--------+
    | Manage               | 91     | 0      | 6       | 0     | ✅     |
    | Verify               | 33     | 0      | 8       | 5     | ❗     |
    | Create               | 143    | 6      | 6       | 11    | ❌     |
    | Secure               | 13     | 0      | 6       | 7     | ❗     |
    | Package              | 0      | 0      | 3       | 0     | ➖     |
    | Plan                 | 52     | 1      | 0       | 2     | ❌     |
    | Release              | 6      | 0      | 0       | 0     | ✅     |
    | Protect              | 2      | 0      | 0       | 0     | ✅     |
    | Fulfillment          | 2      | 0      | 10      | 0     | ✅     |
    | Non-devops           | 2      | 0      | 0       | 0     | ✅     |
    | Configure            | 0      | 0      | 3       | 0     | ➖     |
    | Version sanity check | 0      | 0      | 1       | 0     | ➖     |
    +----------------------+--------+--------+---------+-------+--------+
    | Total                | 344    | 7      | 43      | 25    | ❌     |
    +----------------------+--------+--------+---------+-------+--------+

    package-and-qa-ff-disabled: :x: test report for 17569f44

    expand test summary
    +-------------------------------------------------------------------+
    |                          suites summary                           |
    +----------------------+--------+--------+---------+-------+--------+
    |                      | passed | failed | skipped | flaky | result |
    +----------------------+--------+--------+---------+-------+--------+
    | Create               | 144    | 5      | 6       | 11    | ❌     |
    | Verify               | 33     | 0      | 8       | 5     | ❗     |
    | Manage               | 91     | 0      | 6       | 0     | ✅     |
    | Secure               | 13     | 0      | 6       | 7     | ❗     |
    | Non-devops           | 2      | 0      | 0       | 0     | ✅     |
    | Plan                 | 52     | 1      | 0       | 2     | ❌     |
    | Release              | 6      | 0      | 0       | 0     | ✅     |
    | Configure            | 0      | 0      | 3       | 0     | ➖     |
    | Package              | 0      | 0      | 3       | 0     | ➖     |
    | Fulfillment          | 2      | 0      | 10      | 0     | ✅     |
    | Version sanity check | 0      | 0      | 1       | 0     | ➖     |
    | Protect              | 2      | 0      | 0       | 0     | ✅     |
    +----------------------+--------+--------+---------+-------+--------+
    | Total                | 345    | 6      | 43      | 25    | ❌     |
    +----------------------+--------+--------+---------+-------+--------+
  • Max Woolf
  • Max Woolf
  • Max Woolf
  • Max Woolf
  • Max Woolf removed review request for @mwoolf

    removed review request for @mwoolf

  • Huzaifa Iftikhar added 533 commits

    added 533 commits

    Compare with previous version

  • added 1 commit

    • 5bf97665 - Add inactive projects deletion feature

    Compare with previous version

  • Huzaifa Iftikhar changed the description

    changed the description

  • added 1 commit

    • 8c228b31 - Add inactive projects deletion feature

    Compare with previous version

  • added 1 commit

    • bc84c9b1 - Add inactive projects deletion feature

    Compare with previous version

  • Robert Hunt
  • Huzaifa Iftikhar added 254 commits

    added 254 commits

    Compare with previous version

  • Huzaifa Iftikhar changed the description

    changed the description

  • Huzaifa Iftikhar added 333 commits

    added 333 commits

    Compare with previous version

  • Huzaifa Iftikhar
  • Huzaifa Iftikhar added 197 commits

    added 197 commits

    Compare with previous version

  • added 1 commit

    • 8f601018 - Add inactive projects deletion feature

    Compare with previous version

  • Huzaifa Iftikhar marked the checklist item I have evaluated the MR acceptance checklist for this MR. as completed

    marked the checklist item I have evaluated the MR acceptance checklist for this MR. as completed

  • Huzaifa Iftikhar added 96 commits

    added 96 commits

    • 8f601018...e3a60158 - 93 commits from branch master
    • 10eab92e - Add deletion warning email template for inactive projects
    • 44ae2ce2 - Make the date of deletion bold in the deletion warning email
    • 3a296f61 - Add inactive projects deletion feature

    Compare with previous version

  • Huzaifa Iftikhar changed the description

    changed the description

  • Huzaifa Iftikhar changed target branch from master to 356805_add_inactive_projection_deletion_warning_email_template

    changed target branch from master to 356805_add_inactive_projection_deletion_warning_email_template

  • Huzaifa Iftikhar requested review from @mwoolf

    requested review from @mwoolf

  • Nick Malcolm
  • Nick Malcolm
  • Ok, I tested the feature locally and it works exactly as expected. Nice work @huzaifaiftikhar1 :thumbsup: I'll start reviewing the code itself now!

  • Max Woolf
  • Max Woolf
  • Max Woolf
  • Max Woolf
  • Max Woolf
  • Max Woolf
  • Max Woolf removed review request for @mwoolf

    removed review request for @mwoolf

  • Huzaifa Iftikhar added 852 commits

    added 852 commits

    • 3a296f61...8c1b15be - 848 commits from branch 356805_add_inactive_projection_deletion_warning_email_template
    • 79da7736 - Add inactive projects deletion feature
    • 0d10e057 - Remove the feature flag from `inactive?` method in project.rb
    • c1e82605 - Update RSpecs to use `to_date` method to make the examples descriptive
    • cd0c653b - Add audit event for sending deletion warning email to inactive projects

    Compare with previous version

  • Huzaifa Iftikhar requested review from @mwoolf

    requested review from @mwoolf

  • Huzaifa Iftikhar changed the description

    changed the description

  • Huzaifa Iftikhar marked this merge request as ready

    marked this merge request as ready

  • Huzaifa Iftikhar added 4 commits

    added 4 commits

    • be08aaf0 - Remove the feature flag from `inactive?` method in project.rb
    • 8125999a - Update RSpecs to use `to_date` method to make the examples descriptive
    • 52ec2288 - Add audit event for sending deletion warning email to inactive projects
    • db92450c - Use the first active administrator user for inactive projects deletion

    Compare with previous version

  • Max Woolf approved this merge request

    approved this merge request

  • Max Woolf requested review from @mkozono and removed review request for @mwoolf

    requested review from @mkozono and removed review request for @mwoolf

  • :wave: @mwoolf, thanks for approving this merge request.

    This is the first time the merge request is approved. To ensure full test coverage, a new pipeline has been started.

    For more info, please refer to the following links:

  • added 1 commit

    • ba26c1c4 - Remove default_enabled as per the new guideline

    Compare with previous version

  • Huzaifa Iftikhar deleted the 356805_add_inactive_projection_deletion_warning_email_template branch. This merge request now targets the master branch

    deleted the 356805_add_inactive_projection_deletion_warning_email_template branch. This merge request now targets the master branch

  • Huzaifa Iftikhar added 358 commits

    added 358 commits

    • ba26c1c4...82dd0471 - 352 commits from branch master
    • a6d2bf5e - Add inactive projects deletion feature
    • 844ee8d3 - Remove the feature flag from `inactive?` method in project.rb
    • 8410fec9 - Update RSpecs to use `to_date` method to make the examples descriptive
    • bc673922 - Add audit event for sending deletion warning email to inactive projects
    • 5636f3d4 - Use the first active administrator user for inactive projects deletion
    • b8cf25a5 - Remove default_enabled as per the new guideline

    Compare with previous version

  • Huzaifa Iftikhar mentioned in merge request !86907 (merged)

    mentioned in merge request !86907 (merged)

  • Huzaifa Iftikhar changed the description

    changed the description

  • Huzaifa Iftikhar changed the description

    changed the description

  • Michael Kozono
  • Michael Kozono
  • Michael Kozono
  • Michael Kozono
  • Michael Kozono
  • Michael Kozono
  • Michael Kozono
  • Michael Kozono
  • Michael Kozono
  • Michael Kozono removed review request for @mkozono

    removed review request for @mkozono

  • Huzaifa Iftikhar added 759 commits

    added 759 commits

    • b8cf25a5...e9caf579 - 752 commits from branch master
    • 0a7455d6 - Add inactive projects deletion feature
    • 604d0faa - Remove the feature flag from `inactive?` method in project.rb
    • efa7196b - Update RSpecs to use `to_date` method to make the examples descriptive
    • c55d158c - Add audit event for sending deletion warning email to inactive projects
    • 0ed0329f - Use the first active administrator user for inactive projects deletion
    • b1a6d8bf - Remove default_enabled as per the new guideline
    • 17569f44 - Add InactiveProjectsDeletionWarningTracker and minor refactoring

    Compare with previous version

  • Huzaifa Iftikhar requested review from @mkozono

    requested review from @mkozono

  • @mkozono I see the undercoverage job is failing, however, the lines it shows as uncovered do have a coverage.

    image

    I also verified the same by running the following locally

    SIMPLECOV=true bundle exec rspec ee/spec/models/project_spec.rb ee/spec/workers/ee/projects/inactive_projects_deletion_cron_worker_spec.rb

    image

    image__1_

    We can also see that previous pipelines were successful and after that no additional code changes related to those files have been made. I would suggest we add pipeline:skip-undercoverage label before setting MWPS.

    Edited by Huzaifa Iftikhar
  • Michael Kozono approved this merge request

    approved this merge request

  • Michael Kozono resolved all threads

    resolved all threads

  • Michael Kozono enabled an automatic merge when the pipeline for 981292d7 succeeds

    enabled an automatic merge when the pipeline for 981292d7 succeeds

  • Michael Kozono mentioned in commit cc1fa6f3

    mentioned in commit cc1fa6f3

  • added workflowstaging label and removed workflowcanary label

  • Huzaifa Iftikhar mentioned in merge request !88751 (merged)

    mentioned in merge request !88751 (merged)

  • Harsimar Sandhu mentioned in merge request !104946 (merged)

    mentioned in merge request !104946 (merged)

  • Please register or sign in to reply
    Loading