Skip to content

Revert "Merge branch 'revert-104302' into 'master'"

Zack Cuddy requested to merge revert-d1332794 into master

What does this MR do and why?

This reverts the revert of !104302 (merged) and fixes the collision of cache values by redefining the cache id mechanism.

There were production issues due to the follow:

  1. VersionCheck is checked at the Application level for all Admin users
  2. We switched from String => Hash in the cache for Version Check
  3. The cache key is Gitlab::VERSION
    • Since Continuous Deployment on GitLab instances doesn't release a new version the cache didn't get busted
  4. With old String cached value we try to check data[:error] which gives us a Type error and wrenches the entire application.

Screenshots or screen recordings

N/A

How to set up and validate locally

important: This validates the caching logic works, not that the keys are different

  1. Checkout this branch
  2. Apply Patch 1 (dropdown on next line):
Patch 1
diff --git a/lib/version_check.rb b/lib/version_check.rb
index 92af5ec1f1f..656d174a41f 100644
--- a/lib/version_check.rb
+++ b/lib/version_check.rb
@@ -61,14 +61,14 @@ def self.from_cache(*)
   end
 
   def id
-    [Gitlab::VERSION, Gitlab.revision, CACHE_VERSION].join('-')
+    "codereview99"
   end
 
   def calculate_reactive_cache(*)
     response = Gitlab::HTTP.try_get(self.class.url, headers: self.class.headers)
 
     case response&.code
-    when 200
+    when 999
       Gitlab::Json.parse(response.body)
     else
       { error: 'version check failed', status: response&.code }
  1. Hit the /admin/version_check.json endpoint
  2. Ensure it returns null
  3. Wait a minute or so (this will allow sidekiq to run the async job to fetch the data)
  4. Revert Patch 1 and apply Patch 2 (dropdown on next line)
Patch 2
diff --git a/lib/version_check.rb b/lib/version_check.rb
index 92af5ec1f1f..c8cd6a24bb9 100644
--- a/lib/version_check.rb
+++ b/lib/version_check.rb
@@ -61,7 +61,7 @@ def self.from_cache(*)
   end
 
   def id
-    [Gitlab::VERSION, Gitlab.revision, CACHE_VERSION].join('-')
+    "codereview99"
   end
 
   def calculate_reactive_cache(*)
  1. Hit the /admin/version_check.json endpoint
  2. Ensure it returns null
  3. Wait a minute or so (this will allow sidekiq to run the async job to fetch the data)
  4. Ensure it returns an object of version check data

MR acceptance checklist

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

Edited by Zack Cuddy

Merge request reports