Outdated backup files were not cleaned as expected in JiHu GitLab

Summary

After I set up gitlab_rails['backup_keep_time'] in gitlab.rb, backup files should only keep in the backup path for the duration as specified and those who are outdated should be deleted while creating new backups. This expected behavior is described in official documentation as Limit backup lifetime for local files (prune old backups) .

But in JiHu GitLab version, the outdated backup files had never been cleaned.

It looks like the root cause is the regular expression that match backup tar files is not correct. It only recognizes GitLab EE backup files and misses those created by JiHu GitLab. Please check the Possible fixes part for the source file and code where the regular expression located.

Steps to reproduce

1. configure backup_keep_time in gitlab.rb as below which means keeping backup files for 3 days:

gitlab_rails['backup_keep_time'] = 259200

2. reconfigure JiHu GitLab to make it effective

gitlab-ctl reconfigure

3. set up a cron job as below which creates an backup at 1:10 am each day

10 1 * * * /opt/gitlab/bin/gitlab-backup create CRON=1

4. after 4 days, you will find all the backup files are still in the default backup path /var/opt/gitlab/backups

Example Project

What is the current bug behavior?

Outdated backup files had not been deleted.

What is the expected correct behavior?

Only those created within the duration set up via gitlab_rails['backup_keep_time'] should be kept.

Relevant logs and/or screenshots

Output of checks

Results of GitLab environment info

Expand for output related to GitLab environment info

(For installations with omnibus-gitlab package run and paste the output of:
`sudo gitlab-rake gitlab:env:info`)

(For installations from source run and paste the output of:
`sudo -u git -H bundle exec rake gitlab:env:info RAILS_ENV=production`)
root@master:/var/opt/gitlab/backups# gitlab-rake gitlab:env:info

System information
System:		Ubuntu 20.04
Proxy:		no
Current User:	git
Using RVM:	no
Ruby Version:	2.7.2p137
Gem Version:	3.1.4
Bundler Version:2.1.4
Rake Version:	13.0.3
Redis Version:	6.0.14
Git Version:	2.32.0
Sidekiq Version:5.2.9
Go Version:	unknown

GitLab information
Version:	14.0.10-jh
Revision:	ec736a6ead0
Directory:	/opt/gitlab/embedded/service/gitlab-rails
DB Adapter:	PostgreSQL
DB Version:	12.7
URL:		https://mygitlab.rjdong.cn
HTTP Clone URL:	https://mygitlab.rjdong.cn/some-group/some-project.git
SSH Clone URL:	git@mygitlab.rjdong.cn:some-group/some-project.git
Elasticsearch:	no
Geo:		no
Using LDAP:	no
Using Omniauth:	yes
Omniauth Providers:

GitLab Shell
Version:	13.19.1
Repository storage paths:
- default: 	/var/opt/gitlab/git-data/repositories
GitLab Shell path:		/opt/gitlab/embedded/service/gitlab-shell
Git:		/opt/gitlab/embedded/bin/git

Results of GitLab application Check

Expand for output related to the GitLab application check

(For installations with omnibus-gitlab package run and paste the output of:
sudo gitlab-rake gitlab:check SANITIZE=true)
(For installations from source run and paste the output of:
sudo -u git -H bundle exec rake gitlab:check RAILS_ENV=production SANITIZE=true)
(we will only investigate if the tests are passing)

Possible fixes

It looks like there is an bug in this source file on the JiHu GitLab server: /opt/gitlab/embedded/service/gitlab-rails/lib/backup/manager.rb source code:

    def remove_old
      # delete backups
      progress.print "Deleting old backups ... "
      keep_time = Gitlab.config.backup.keep_time.to_i

      if keep_time > 0
        removed = 0

        Dir.chdir(backup_path) do
          backup_file_list.each do |file|
            # For backward compatibility, there are 3 names the backups can have:
            # - 1495527122_gitlab_backup.tar
            # - 1495527068_2017_05_23_gitlab_backup.tar
            # - 1495527097_2017_05_23_9.3.0-pre_gitlab_backup.tar
            next unless file =~ /^(\d{10})(?:_\d{4}_\d{2}_\d{2}(_\d+\.\d+\.\d+((-|\.)(pre|rc\d))?(-ee)?)?)?_gitlab_backup\.tar$/

if I modify the last line as below, the outdated backup files will be deleted as expected

            next unless file =~ /^(\d{10})(?:_\d{4}_\d{2}_\d{2}(_\d+\.\d+\.\d+((-|\.)(pre|rc\d))?(-ee|-jh)?)?)?_gitlab_backup\.tar$/