Report the correct version and revision for Geo node status requests

changelogs/unreleased-ee/4782-fix-geo-node-status-version-and-revision.yml

+---
+title: Report the correct version and revision for Geo node status requests
+merge_request: 4353
+author:
+type: fixed

db/schema.rb

@@ -999,6 +999,8 @@ ActiveRecord::Schema.define(version: 20180202111106) do
     t.integer "job_artifacts_count"
     t.integer "job_artifacts_synced_count"
     t.integer "job_artifacts_failed_count"
+    t.string "version"
+    t.string "revision"
   end
 
   add_index "geo_node_statuses", ["geo_node_id"], name: "index_geo_node_statuses_on_geo_node_id", unique: true, using: :btree

ee/app/models/geo_node_status.rb

@@ -4,7 +4,7 @@ class GeoNodeStatus < ActiveRecord::Base
   delegate :selective_sync_type, to: :geo_node
 
   # Whether we were successful in reaching this node
-  attr_accessor :success, :version, :revision
+  attr_accessor :success
   attr_writer :health_status
   attr_accessor :storage_shards
 
@@ -58,10 +58,18 @@ class GeoNodeStatus < ActiveRecord::Base
     GeoNodeStatus.new(HashWithIndifferentAccess.new(json_data))
   end
 
+  EXCLUDED_PARAMS = %w[id created_at].freeze
+  EXTRA_PARAMS = %w[
+    success
+    health
+    health_status
+    last_event_timestamp
+    cursor_last_event_timestamp
+    storage_shards
+  ].freeze
+
   def self.allowed_params
-    excluded_params = %w(id created_at updated_at).freeze
-    extra_params = %w(success health health_status last_event_timestamp cursor_last_event_timestamp version revision storage_shards updated_at).freeze
-    self.column_names - excluded_params + extra_params
+    self.column_names - EXCLUDED_PARAMS + EXTRA_PARAMS
   end
 
   def load_data_from_current_node
@@ -83,6 +91,9 @@ class GeoNodeStatus < ActiveRecord::Base
     self.last_successful_status_check_at = Time.now
     self.storage_shards = StorageShard.all
 
+    self.version = Gitlab::VERSION
+    self.revision = Gitlab::REVISION
+
     load_primary_data
     load_secondary_data
 

ee/db/migrate/20180201192230_store_version_and_revision_in_geo_node_status.rb

+class StoreVersionAndRevisionInGeoNodeStatus < ActiveRecord::Migration
+  DOWNTIME = false
+
+  def change
+    add_column :geo_node_statuses, :version, :string
+    add_column :geo_node_statuses, :revision, :string
+  end
+end

lib/api/entities.rb

@@ -1267,14 +1267,6 @@ module API
       def missing_oauth_application
         object.geo_node.missing_oauth_application?
       end
-
-      def version
-        Gitlab::VERSION
-      end
-
-      def revision
-        Gitlab::REVISION
-      end
     end
 
     class PersonalAccessToken < Grape::Entity

spec/ee/spec/models/geo_node_status_spec.rb

@@ -12,7 +12,7 @@ describe GeoNodeStatus, :geo do
   let!(:project_3) { create(:project) }
   let!(:project_4) { create(:project) }
 
-  subject { described_class.current_node_status }
+  subject(:status) { described_class.current_node_status }
 
   before do
     stub_current_geo_node(secondary)
@@ -425,6 +425,14 @@ describe GeoNodeStatus, :geo do
     end
   end
 
+  describe '#version' do
+    it {  expect(status.version).to eq(Gitlab::VERSION) }
+  end
+
+  describe '#revision' do
+    it {  expect(status.revision).to eq(Gitlab::REVISION) }
+  end
+
   describe '#[]' do
     it 'returns values for each attribute' do
       expect(subject[:repositories_count]).to eq(4)

spec/factories/geo_node_statuses.rb

@@ -25,6 +25,8 @@ FactoryBot.define do
       cursor_last_event_id 1
       cursor_last_event_timestamp { Time.now.to_i }
       last_successful_status_check_timestamp { Time.now.beginning_of_day }
+      version { Gitlab::VERSION }
+      revision { Gitlab::REVISION }
     end
 
     trait :unhealthy do

spec/requests/api/geo_nodes_spec.rb

@@ -69,6 +69,7 @@ describe API::GeoNodes, :geo, api: true do
   describe 'GET /geo_nodes/:id/status' do
     it 'retrieves the Geo nodes status if admin is logged in' do
       stub_current_geo_node(primary)
+      secondary_status.update!(version: 'secondary-version', revision: 'secondary-revision')
 
       expect(GeoNodeStatus).not_to receive(:current_node_status)
 
@@ -77,14 +78,17 @@ describe API::GeoNodes, :geo, api: true do
       expect(response).to have_gitlab_http_status(200)
       expect(response).to match_response_schema('public_api/v4/geo_node_status', dir: 'ee')
 
+      expect(json_response['version']).to eq('secondary-version')
+      expect(json_response['revision']).to eq('secondary-revision')
+
       links = json_response['_links']
+
       expect(links['self']).to end_with("/api/v4/geo_nodes/#{secondary.id}/status")
       expect(links['node']).to end_with("/api/v4/geo_nodes/#{secondary.id}")
     end
 
     it 'fetches the current node status' do
       stub_current_geo_node(secondary)
-
       expect(GeoNode).to receive(:find).and_return(secondary)
       expect(GeoNodeStatus).to receive(:current_node_status).and_call_original