Address review feedback for StorageShard implementation

app/assets/javascripts/geo_nodes.js

@@ -78,15 +78,6 @@ class GeoNodeStatus {
     };
   }
 
-  static sortByStorageName(config) {
-    return config.sort((a, b) => a.name.localeCompare(b.name));
-  }
-
-  static storageConfigEquals(first, second) {
-    return _.isEqual(GeoNodeStatus.sortByStorageName(first),
-                     GeoNodeStatus.sortByStorageName(second));
-  }
-
   static renderSyncGraph($itemEl, syncStats) {
     const graphItems = [
       {
@@ -224,10 +215,9 @@ class GeoNodeStatus {
       this.$secondaryVersion.text(`${status.version} (${status.revision}) - ${versionMismatch}`);
     }
 
-    if (!this.primaryStorageConfiguration || !status.storage_shards) {
+    if (!status.storage_shards_match) {
       this.$secondaryStorage.text('UNKNOWN');
-    } else if (GeoNodeStatus.storageConfigEquals(
-      this.primaryStorageConfiguration, status.storage_shards)) {
+    } else if (status.storage_shards_match) {
       this.$secondaryStorage.removeClass(`${storageMismatchClass}`);
       this.$secondaryStorage.text('OK');
     } else {

app/models/storage_shard.rb

@@ -5,11 +5,10 @@ class StorageShard
   include ActiveModel::Model
 
   attr_accessor :name, :path, :gitaly_address, :gitaly_token
-  attr_accessor :failure_count_threshold, :failure_reset_time, :failure_wait_time
-  attr_accessor :storage_timeout
 
   validates :name, presence: true
   validates :path, presence: true
+  validates :gitaly_address, presence: true
 
   # Generates an array of StorageShard objects from the currrent storage
   # configuration using the gitlab.yml array of key/value pairs:
@@ -17,10 +16,15 @@ class StorageShard
   # {"default"=>{"path"=>"/home/git/repositories", ...}
   #
   # The key is the shard name, and the values are the parameters for that shard.
-  def self.current_shards
+  def self.all
     Settings.repositories.storages.map do |name, params|
       config = params.symbolize_keys.merge(name: name)
+      config.slice!(allowed_params)
       StorageShard.new(config)
     end
   end
+
+  def self.allowed_params
+    return %w(name path gitaly_address gitaly_token).freeze
+  end
 end

ee/app/controllers/admin/geo_nodes_controller.rb

@@ -8,7 +8,7 @@ class Admin::GeoNodesController < Admin::ApplicationController
   def index
     @nodes = GeoNode.all.order(:id)
     @node = GeoNode.new
-    @current_storage_shards = StorageShardSerializer.new.represent(StorageShard.current_shards)
+    @current_storage_shards = StorageShardSerializer.new.represent(StorageShard.all)
 
     unless Gitlab::Geo.license_allows?
       flash_now(:alert, 'You need a different license to enable Geo replication')

ee/app/models/geo_node_status.rb

@@ -75,7 +75,7 @@ class GeoNodeStatus < ActiveRecord::Base
     self.lfs_objects_count = lfs_objects_finder.count_lfs_objects
     self.attachments_count = attachments_finder.count_attachments
     self.last_successful_status_check_at = Time.now
-    self.storage_shards = StorageShard.current_shards
+    self.storage_shards = StorageShard.all
 
     if Gitlab::Geo.primary?
       self.replication_slots_count = geo_node.replication_slots_count
@@ -154,6 +154,12 @@ class GeoNodeStatus < ActiveRecord::Base
     calc_percentage(replication_slots_count, replication_slots_used_count)
   end
 
+  def storage_shards_match?
+    return unless Gitlab::Geo.primary?
+
+    storage_shards.as_json == StorageShard.all.as_json
+  end
+
   def [](key)
     public_send(key) # rubocop:disable GitlabSecurity/PublicSend
   end

ee/app/serializers/geo_node_status_entity.rb

@@ -69,6 +69,10 @@ class GeoNodeStatusEntity < Grape::Entity
     status.storage_shards.present?
   end
 
+  expose :storage_shards_match?, as: :storage_shards_match, if: -> (status, options) do
+    Gitlab::Geo.primary? && status.storage_shards.present?
+  end
+
   private
 
   def namespaces

ee/app/views/admin/geo_nodes/index.html.haml

@@ -29,10 +29,7 @@
             - if node.current?
               .node-badge.current-node Current node
             - if node.primary?
-              - if node.current?
-                .node-badge.primary-node{ data: { storage_shards: @current_storage_shards.to_json } } Primary
-              - else
-                .node-badge.primary-node
+              .node-badge.primary-node Primary
               %p
                 %span.help-block Primary node
               %p

spec/ee/spec/models/geo_node_status_spec.rb

@@ -398,7 +398,7 @@ describe GeoNodeStatus, :geo do
 
   describe '#storage_shards' do
     it "returns the current node's shard config" do
-      expect(subject[:storage_shards].as_json).to eq(StorageShard.current_shards.as_json)
+      expect(subject[:storage_shards].as_json).to eq(StorageShard.all.as_json)
     end
   end
 

spec/factories/geo_node_statuses.rb

@@ -2,7 +2,7 @@ FactoryBot.define do
   factory :geo_node_status do
     sequence(:id)
     geo_node
-    storage_shards { [{ name: 'default', path: '/tmp/test' }] }
+    storage_shards { StorageShard.all.as_json }
 
     trait :healthy do
       health nil