Skip to content

Use templatesymlink :delete to cleanup redis config

Sylvester Chin requested to merge sc1-redis-templatesymlink-delete into master

What does this MR do?

This MR uses :delete action in templatesymlink resource to clean up empty redis config files instead of running delete on the 2 files.

With the following diff applied on master, we encounter failing specs.

diff --git a/spec/chef/cookbooks/gitlab/recipes/gitlab-rails_spec.rb b/spec/chef/cookbooks/gitlab/recipes/gitlab-rails_spec.rb
index c70a10e59..767e20f3c 100644
--- a/spec/chef/cookbooks/gitlab/recipes/gitlab-rails_spec.rb
+++ b/spec/chef/cookbooks/gitlab/recipes/gitlab-rails_spec.rb
@@ -356,6 +356,9 @@ RSpec.describe 'gitlab::gitlab-rails' do
             redis_sentinels: [{ "host" => instance, "port" => "1234" }, { "host" => instance, "port" => "3456" }],
             redis_enable_client: false
           )
+
+          expect(chef_run).to render_file("/var/opt/gitlab/gitlab-rails/etc/redis.#{instance}.yml")
+
           expect(chef_run).not_to delete_file("/var/opt/gitlab/gitlab-rails/etc/redis.#{instance}.yml")
         end
       end
expected Chef run to render "/var/opt/gitlab/gitlab-rails/etc/redis.cache.yml"

  0) gitlab::gitlab-rails with redis settings with multiple instances render separate config files
     Failure/Error: expect(chef_run).to render_file("/var/opt/gitlab/gitlab-rails/etc/redis.#{instance}.yml")
       expected Chef run to render "/var/opt/gitlab/gitlab-rails/etc/redis.cache.yml"
     # ./spec/chef/cookbooks/gitlab/recipes/gitlab-rails_spec.rb:360:in `block (5 levels) in <top (required)>'
     # ./spec/chef/cookbooks/gitlab/recipes/gitlab-rails_spec.rb:353:in `each'
     # ./spec/chef/cookbooks/gitlab/recipes/gitlab-rails_spec.rb:353:in `block (4 levels) in <top (required)>'

Verification

Using instructions from https://gitlab.com/gitlab-org/omnibus-gitlab/-/blob/master/doc/development/setup.md#get-the-source-of-omnibus-gitlab. I ran the verification on a gcloud vm e2-standard-2 with 20GB harddisk.

  1. Setup omnibus on debian (https://about.gitlab.com/install/#debian)
  2. Run setup in https://gitlab.com/gitlab-org/omnibus-gitlab/-/blob/master/doc/development/setup.md#get-the-source-of-omnibus-gitlab
  3. Using sudo vi /opt/gitlab/embedded/cookbooks/gitlab/attributes/default.rb, update
# change this 
default['gitlab']['gitlab-rails']['redis_cache_instance'] = nil

# to this
default['gitlab']['gitlab-rails']['redis_cache_instance'] = 'redis://127.0.0.1:6379'
  1. Run sudo gitlab-ctl reconfigure and check config folder for symlink + file
sylvesterchin@instance-1:~$ ls -l  /opt/gitlab/embedded/service/gitlab-rails/config | grep redis
lrwxrwxrwx 1 root root     48 Jan 27 06:13 redis.cache.yml -> /var/opt/gitlab/gitlab-rails/etc/redis.cache.yml
  1. Change to current branch
  2. Undo changes in step 3
  3. Run sudo gitlab-ctl reconfigure and confirm that config is deleted (both link and file)
sylvesterchin@instance-1:~$ ls -l  /opt/gitlab/embedded/service/gitlab-rails/config | grep redis
# nothing will show

Related issues

Originated from !6548 (merged). expect(chef_run).to render_file is failing due to the file deletion in L264. Technically it should not since the only_if condition is working.

Using the debugger, it seems that https://github.com/chefspec/chefspec/blob/main/lib/chefspec/matchers/render_file_matcher.rb#L102 performed_actions is empty. This could be a spec issue since the omnibus release is working and rendering redis configurations just fine.

Checklist

See Definition of done.

For anything in this list which will not be completed, please provide a reason in the MR discussion

Required

  • Merge Request Title, and Description are up to date, accurate, and descriptive
  • MR targeting the appropriate branch
  • MR has a green pipeline on GitLab.com
  • Pipeline is green on dev.gitlab.org if the change is touching anything besides documentation or internal cookbooks
  • trigger-package has a green pipeline running against latest commit

Expected (please provide an explanation if not completing)

  • Test plan indicating conditions for success has been posted and passes
  • Documentation created/updated
  • Tests added
  • Integration tests added to GitLab QA
  • Equivalent MR/issue for the GitLab Chart opened
Edited by Sylvester Chin

Merge request reports