Merge branch '3195-geo-admin-enhancements' into 'master'

Enhancements for Geo admin screen

Closes #3195

See merge request gitlab-org/gitlab-ee!3545

app/assets/stylesheets/framework.scss

@@ -58,3 +58,4 @@
 @import "framework/snippets";
 @import "framework/memory_graph";
 @import "framework/responsive_tables";
+@import "framework/stacked-progress-bar";

app/assets/stylesheets/framework/stacked-progress-bar.scss

+.stacked-progress-bar {
+  display: flex;
+  height: 20px;
+  width: 350px;
+  border-radius: 10px;
+  overflow: hidden;
+  background-color: $theme-gray-100;
+
+  .status-unavailable,
+  .status-green,
+  .status-neutral,
+  .status-red, {
+    height: 100%;
+    font-weight: normal;
+    color: $white-light;
+    line-height: 20px;
+
+    &.has-value {
+      padding: 0 10px;
+    }
+
+    &:hover {
+      cursor: pointer;
+    }
+  }
+
+  .status-unavailable {
+    padding: 0 10px;
+    color: $theme-gray-700;
+  }
+
+  .status-green {
+    background-color: $green-500;
+
+    &:hover {
+      background-color: $green-600;
+    }
+  }
+
+  .status-neutral {
+    background-color: $theme-gray-200;
+    color: $gl-gray-dark;
+
+    &:hover {
+      background-color: $theme-gray-300;
+    }
+  }
+
+  .status-red {
+    background-color: $red-500;
+
+    &:hover {
+      background-color: $red-600;
+    }
+  }
+}

app/assets/stylesheets/pages/geo_nodes.scss

+.geo-admin-container {
+  .page-title,
+  .page-title-separator {
+    margin-top: 10px;
+  }
+
+  .title-text {
+    line-height: 34px;
+  }
+
+  .page-subtitle {
+    margin-bottom: 24px;
+  }
+}
+
+.geo-node-status {
+  td {
+    vertical-align: top;
+  }
+
+  .help-block {
+    width: 135px;
+    text-align: right;
+  }
+
+  .node-info {
+    font-weight: $gl-font-weight-bold;
+  }
+
+  .event-timestamp {
+    font-weight: normal;
+    color: $theme-gray-800;
+  }
+
+  .sync-status {
+    font-weight: normal;
+
+    svg {
+      vertical-align: middle;
+    }
+
+    .sync-status-icon svg,
+    .sync-status-timestamp {
+      fill: $theme-gray-700;
+      color: $theme-gray-700;
+    }
+
+    &.sync-status-failure {
+      .sync-status-icon svg,
+      .sync-status-timestamp {
+        fill: $red-700;
+        color: $red-700;
+      }
+    }
+  }
+}
+
+.advanced-geo-node-status-toggler {
+  display: block;
+
+  .show-advance-chevron {
+    margin-top: 2px;
+  }
+}
+
 .geo-node-healthy {
   color: $gl-success;
 }
@@ -52,6 +117,16 @@
   white-space: pre-wrap;
 }
 
+.geo-nodes {
+  .health-message {
+    padding: 1px 8px;
+    background-color: $red-100;
+    color: $red-500;
+    border-radius: $border-radius-default;
+    font-weight: 500;
+  }
+}
+
 .node-badge {
   color: $white-light;
   display: inline-block;

app/models/geo_node_status.rb

@@ -3,6 +3,7 @@ class GeoNodeStatus < ActiveRecord::Base
 
   # Whether we were successful in reaching this node
   attr_accessor :success
+  attr_accessor :health_status
 
   # Be sure to keep this consistent with Prometheus naming conventions
   PROMETHEUS_METRICS = {
@@ -47,8 +48,8 @@ class GeoNodeStatus < ActiveRecord::Base
   end
 
   def self.allowed_params
-    excluded_params = %w(id last_successful_status_check_at created_at updated_at).freeze
-    extra_params = %w(success health last_event_timestamp cursor_last_event_timestamp).freeze
+    excluded_params = %w(id created_at updated_at).freeze
+    extra_params = %w(success health health_status last_event_timestamp cursor_last_event_timestamp).freeze
     self.column_names - excluded_params + extra_params
   end
 
@@ -89,6 +90,10 @@ class GeoNodeStatus < ActiveRecord::Base
     status_message.blank? || status_message == 'Healthy'.freeze
   end
 
+  def health_status
+    @health_status || (healthy? ? 'Healthy' : 'Unhealthy')
+  end
+
   def last_successful_status_check_timestamp
     self.last_successful_status_check_at.to_i
   end

app/serializers/geo_node_status_entity.rb

@@ -7,6 +7,7 @@ class GeoNodeStatusEntity < Grape::Entity
   expose :health do |node|
     node.healthy? ? 'Healthy' : node.health
   end
+  expose :health_status
 
   expose :attachments_count
   expose :attachments_synced_count
@@ -37,4 +38,10 @@ class GeoNodeStatusEntity < Grape::Entity
   expose :cursor_last_event_timestamp
 
   expose :last_successful_status_check_timestamp
+
+  expose :namespaces, using: NamespaceEntity
+
+  def namespaces
+    object.geo_node.namespaces
+  end
 end

app/serializers/namespace_entity.rb

+class NamespaceEntity < Grape::Entity
+  expose :id, :name, :path, :kind, :full_path, :parent_id
+end

app/services/geo/node_status_fetch_service.rb

@@ -6,7 +6,8 @@ module Geo
     def call(geo_node)
       return GeoNodeStatus.current_node_status if geo_node.current?
 
-      data = { success: false }
+      data = GeoNodeStatus.find_or_initialize_by(geo_node: geo_node).attributes
+      data = data.merge(success: false, health_status: 'Offline')
 
       begin
         response = self.class.get(geo_node.status_url, headers: headers, timeout: timeout)
@@ -29,8 +30,10 @@ module Geo
         end
       rescue Gitlab::Geo::GeoNodeNotFoundError
         data[:health] = 'This GitLab instance does not appear to be configured properly as a Geo node. Make sure the URLs are using the correct fully-qualified domain names.'
+        data[:health_status] = 'Unhealthy'
       rescue OpenSSL::Cipher::CipherError
         data[:health] = 'Error decrypting the Geo secret from the database. Check that the primary uses the correct db_key_base.'
+        data[:health_status] = 'Unhealthy'
       rescue HTTParty::Error, Timeout::Error, SocketError, SystemCallError, OpenSSL::SSL::SSLError => e
         data[:health] = e.message
       end

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

 - page_title 'Geo nodes'
-%h3.page-title
-  Geo Nodes
+- @content_class = "geo-admin-container"
+%h2.page-title.clearfix
+  %span.title-text.pull-left
+    Geo Nodes
+  = link_to "New node", new_admin_geo_node_path, class: 'btn btn-create pull-right'
+%hr.page-title-separator
 
-%p.light
+%p.page-subtitle.light
   With #{link_to 'GitLab Geo', help_page_path('gitlab-geo/README'), class: 'vlink'} you can install a special
   read-only and replicated instance anywhere.
   Before you add nodes, follow the
@@ -11,16 +15,6 @@
   %strong exact order
   they appear.
 
-%hr
-
-- if Gitlab::Geo.license_allows?
-  = form_for [:admin, @node], as: :geo_node, url: admin_geo_nodes_path, html: { class: 'form-horizontal js-geo-node-form' } do |f|
-    = render partial: 'form', locals: { form: f, geo_node: @node }
-    .form-actions
-      = f.submit 'Add Node', class: 'btn btn-create'
-
-%hr
-
 - if @nodes.any?
   .panel.panel-default
     .panel-heading
@@ -38,59 +32,84 @@
               %span.help-block Primary node
             - else
               = status_loading_icon
-              - if node.restricted_project_ids
-                %p
-                  %span.help-block
-                    Namespaces to replicate:
-                    %strong.node-info
-                      = node_selected_namespaces_to_replicate(node)
-              .js-geo-node-status{ style: 'display: none' }
+              %table.geo-node-status.js-geo-node-status.hidden
                 - if node.enabled?
-                  %p
-                    %span.help-block
-                      Health Status:
-                      %span.js-health-status
-                %p
-                  %span.help-block
-                    Repositories synced:
-                    %strong.node-info.js-repositories-synced
-                %p
-                  %span.help-block
-                    Repositories failed:
-                    %strong.node-info.js-repositories-failed
-                %p
-                  %span.help-block
-                    LFS objects synced:
-                    %strong.node-info.js-lfs-objects-synced
-                %p
-                  %span.help-block
-                    LFS objects failed:
-                    %strong.node-info.js-lfs-objects-failed
-                %p
-                  %span.help-block
-                    Attachments synced:
-                    %strong.node-info.js-attachments-synced
-                %p
-                  %span.help-block
-                    Attachments failed:
-                    %strong.node-info.js-attachments-failed
-                %p
-                .advanced-geo-node-status-container
-                  .advanced-status.hidden
-                    %span.help-block
+                  %tr
+                    %td
+                      .help-block
+                        Health Status:
+                    %td
+                      .health-status.prepend-top-10.prepend-left-5.js-health-status
+                %tr
+                  %td
+                    .help-block.prepend-top-10
+                      Repositories:
+                  %td
+                    .node-info.prepend-top-10.prepend-left-5.stacked-progress-bar.js-repositories
+                      %span.status-unavailable.js-stats-unavailable
+                        Not available
+                      %span.status-green.js-synced
+                      %span.status-neutral.js-waiting
+                      %span.status-red.js-failed
+                %tr
+                  %td
+                    .help-block.prepend-top-10
+                      LFS objects:
+                  %td
+                    .node-info.prepend-top-10.prepend-left-5.stacked-progress-bar.js-lfs-objects
+                      %span.status-unavailable.js-stats-unavailable
+                        Not available
+                      %span.status-green.js-synced
+                      %span.status-neutral.js-waiting
+                      %span.status-red.js-failed
+                %tr
+                  %td
+                    .help-block.prepend-top-10
+                      Attachments:
+                  %td
+                    .node-info.prepend-top-10.prepend-left-5.stacked-progress-bar.js-attachments
+                      %span.status-unavailable.js-stats-unavailable
+                        Not available
+                      %span.status-green.js-synced
+                      %span.status-neutral.js-waiting
+                      %span.status-red.js-failed
+                %tr
+                  %td
+                    .help-block.prepend-top-10
+                      Sync settings:
+                  %td
+                    .node-info.prepend-top-10.prepend-left-5.js-sync-settings
+                      %span.js-sync-type
+                      %span.has-tooltip.sync-status.js-sync-status
+                        %i.sync-status-icon.js-sync-status-icon
+                        %span.sync-status-timestamp.js-sync-status-timestamp
+                %tr.js-advanced-status.hidden
+                  %td
+                    .help-block.prepend-top-10
                       Database replication lag:
-                      %strong.node-info.js-db-replication-lag
-                    %span.help-block
+                  %td
+                    .node-info.prepend-top-10.prepend-left-5.js-db-replication-lag
+                %tr.js-advanced-status.hidden
+                  %td
+                    .help-block.prepend-top-10
                       Last event ID seen from primary:
-                      %strong.node-info.js-last-event-seen
-                    %span.help-block
+                  %td
+                    .node-info.prepend-top-10.prepend-left-5.js-last-event-seen
+                      %span.js-event-id
+                      %span.event-timestamp.js-event-timestamp.has-tooltip
+                %tr.js-advanced-status.hidden
+                  %td
+                    .help-block.prepend-top-10
                       Last event ID processed by cursor:
-                      %strong.node-info.js-last-cursor-event
-                  %button.btn-link.js-advanced-geo-node-status-toggler
-                    %span> Advanced
-                    = icon('angle-down')
-                %p
-                  .js-health
+                  %td
+                    .node-info.prepend-top-10.prepend-left-5.js-last-cursor-event
+                      %span.js-event-id
+                      %span.event-timestamp.js-event-timestamp.has-tooltip
+              %button.btn-link.advanced-geo-node-status-toggler.js-advanced-geo-node-status-toggler
+                %span> Advanced
+                %span.js-advance-toggle.show-advance-chevron.pull-right.inline.prepend-left-5
+                  = sprite_icon('angle-down', css_class: 's16')
+              %p.health-message.hidden.js-health-message
 
           - if Gitlab::Database.read_write?
             .node-actions

app/views/admin/geo_nodes/new.html.haml

+- page_title 'New node'
+- @content_class = "geo-admin-container"
+
+%h2.page-title
+  %span.title-text
+    New node
+%hr.page-title-separator
+
+- if Gitlab::Geo.license_allows?
+  = form_for [:admin, @node], as: :geo_node, url: admin_geo_nodes_path, html: { class: 'form-horizontal js-geo-node-form' } do |f|
+    = render partial: 'form', locals: { form: f, geo_node: @node }
+    .form-actions
+      = f.submit 'Add Node', class: 'btn btn-create'

changelogs/unreleased-ee/3195-geo-admin-enhancements.yml

+---
+title: Enhancements for Geo admin screen
+merge_request: 3545
+author:
+type: changed

config/routes/admin.rb

@@ -127,7 +127,7 @@ namespace :admin do
     get :download, on: :member
   end
 
-  resources :geo_nodes, only: [:index, :create, :edit, :update, :destroy] do
+  resources :geo_nodes, only: [:index, :create, :new, :edit, :update, :destroy] do
     member do
       post :repair
       post :toggle

ee/app/controllers/admin/geo_nodes_controller.rb

@@ -22,10 +22,14 @@ class Admin::GeoNodesController < Admin::ApplicationController
     else
       @nodes = GeoNode.all
 
-      render :index
+      render :new
     end
   end
 
+  def new
+    @node = GeoNode.new
+  end
+
   def update
     if Geo::NodeUpdateService.new(@node, geo_node_params).execute
       redirect_to admin_geo_nodes_path, notice: 'Node was successfully updated.'

spec/controllers/admin/geo_nodes_controller_spec.rb

@@ -30,8 +30,10 @@ describe Admin::GeoNodesController, :postgresql do
         allow(Gitlab::Geo).to receive(:license_allows?).and_return(true)
       end
 
-      it 'renders creation form' do
-        expect(go).to render_template(partial: 'admin/geo_nodes/_form')
+      it 'does not display a flash message' do
+        go
+
+        expect(flash).not_to include(:alert)
       end
     end
 
@@ -40,10 +42,6 @@ describe Admin::GeoNodesController, :postgresql do
         allow(Gitlab::Geo).to receive(:license_allows?).and_return(false)
       end
 
-      it 'does not render the creation form' do
-        expect(go).not_to render_template(partial: 'admin/geo_nodes/_form')
-      end
-
       it 'displays a flash message' do
         go
 

spec/factories/geo_node_statuses.rb

@@ -18,6 +18,7 @@ FactoryGirl.define do
       last_event_timestamp Time.now.to_i
       cursor_last_event_id 1
       cursor_last_event_timestamp Time.now.to_i
+      last_successful_status_check_timestamp Time.now.beginning_of_day
     end
 
     trait :unhealthy do

spec/features/admin/admin_geo_nodes_spec.rb

@@ -8,9 +8,10 @@ RSpec.describe 'admin Geo Nodes', type: :feature do
     sign_in(create(:admin))
   end
 
-  it 'show all public Geo Nodes' do
+  it 'show all public Geo Nodes and create new node link' do
     visit admin_geo_nodes_path
 
+    expect(page).to have_link('New node', href: new_admin_geo_node_path)
     page.within(find('.geo-nodes', match: :first)) do
       expect(page).to have_content(geo_node.url)
     end
@@ -20,7 +21,7 @@ RSpec.describe 'admin Geo Nodes', type: :feature do
     let(:new_ssh_key) { attributes_for(:key)[:key] }
 
     before do
-      visit admin_geo_nodes_path
+      visit new_admin_geo_node_path
     end
 
     it 'creates a new Geo Node' do
@@ -36,12 +37,10 @@ RSpec.describe 'admin Geo Nodes', type: :feature do
     end
 
     it 'returns an error message when a duplicate primary is added' do
-      check 'This is a primary node'
-      fill_in 'geo_node_url', with: 'https://test.example.com'
-      click_button 'Add Node'
+      create(:geo_node, :primary)
 
       check 'This is a primary node'
-      fill_in 'geo_node_url', with: 'https://secondary.example.com'
+      fill_in 'geo_node_url', with: 'https://another-primary.example.com'
       click_button 'Add Node'
 
       expect(current_path).to eq admin_geo_nodes_path

spec/fixtures/api/schemas/geo_node_status.json

@@ -4,6 +4,7 @@
     "geo_node_id",
     "healthy",
     "health",
+    "health_status",
     "attachments_count",
     "attachments_failed_count",
     "attachments_synced_count",
@@ -17,12 +18,14 @@
     "last_event_id",
     "last_event_timestamp",
     "cursor_last_event_id",
-    "cursor_last_event_timestamp"
+    "cursor_last_event_timestamp",
+    "namespaces"
   ],
   "properties" : {
     "geo_node_id": { "type": "integer" },
     "healthy": { "type": "boolean" },
     "health": { "type": ["string", "null"] },
+    "health_status": { "type": "string" },
     "attachments_count": { "type": "integer" },
     "attachments_failed_count": { "type": "integer" },
     "attachments_synced_count": { "type": "integer" },
@@ -40,7 +43,8 @@
     "last_event_timestamp": { "type": ["integer", "null"] },
     "cursor_last_event_id": { "type": ["integer", "null"] },
     "cursor_last_event_timestamp": { "type": ["integer", "null"] },
-    "last_successful_status_check_timestamp": { "type": ["integer", "null"] }
+    "last_successful_status_check_timestamp": { "type": ["integer", "null"] },
+    "namespaces": { "type": "array" }
   },
   "additionalProperties": false
 }

spec/serializers/geo_node_status_entity_spec.rb

 require 'spec_helper'
 
 describe GeoNodeStatusEntity, :postgresql do
-  let(:geo_node_status) do
-    GeoNodeStatus.new(
-      geo_node_id: 1,
-      health: '',
-      attachments_count: 329,
-      attachments_failed_count: 25,
-      attachments_synced_count: 141,
-      lfs_objects_count: 256,
-      lfs_objects_failed_count: 12,
-      lfs_objects_synced_count: 123,
-      repositories_count: 10,
-      repositories_synced_count: 5,
-      repositories_failed_count: 0,
-      last_successful_status_check_timestamp: Time.now.beginning_of_day
-    )
-  end
-
-  let(:entity) do
-    described_class.new(geo_node_status, request: double)
-  end
-
-  let(:error) do
-    'Could not connect to Geo database'
-  end
+  let(:geo_node_status) { build(:geo_node_status) }
+  let(:entity) { described_class.new(geo_node_status, request: double) }
+  let(:error) { 'Could not connect to Geo database' }
 
   subject { entity.as_json }
 
@@ -44,6 +23,7 @@ describe GeoNodeStatusEntity, :postgresql do
   it { is_expected.to have_key(:repositories_synced_count)}
   it { is_expected.to have_key(:repositories_synced_in_percentage) }
   it { is_expected.to have_key(:last_successful_status_check_timestamp) }
+  it { is_expected.to have_key(:namespaces) }
 
   describe '#healthy' do
     context 'when node is healthy' do
@@ -87,19 +67,47 @@ describe GeoNodeStatusEntity, :postgresql do
 
   describe '#attachments_synced_in_percentage' do
     it 'formats as percentage' do
+      geo_node_status.assign_attributes(attachments_count: 329,
+                                        attachments_failed_count: 25,
+                                        attachments_synced_count: 141)
+
       expect(subject[:attachments_synced_in_percentage]).to eq '42.86%'
     end
   end
 
   describe '#lfs_objects_synced_in_percentage' do
     it 'formats as percentage' do
+      geo_node_status.assign_attributes(lfs_objects_count: 256,
+                                        lfs_objects_failed_count: 12,
+                                        lfs_objects_synced_count: 123)
+
       expect(subject[:lfs_objects_synced_in_percentage]).to eq '48.05%'
     end
   end
 
   describe '#repositories_synced_in_percentage' do
     it 'formats as percentage' do
+      geo_node_status.assign_attributes(repositories_count: 10,
+                                        repositories_synced_count: 5,
+                                        repositories_failed_count: 0)
+
       expect(subject[:repositories_synced_in_percentage]).to eq '50.00%'
     end
   end
+
+  describe '#namespaces' do
+    it 'returns empty array when full sync is active' do
+      expect(subject[:namespaces]).to be_empty
+    end
+
+    it 'returns array of namespace ids and paths for selective sync' do
+      namespace = create(:namespace)
+      geo_node_status.geo_node.namespaces << namespace
+
+      expect(subject[:namespaces]).not_to be_empty
+      expect(subject[:namespaces]).to be_an(Array)
+      expect(subject[:namespaces].first[:id]).to eq(namespace.id)
+      expect(subject[:namespaces].first[:path]).to eq(namespace.path)
+    end
+  end
 end

spec/services/geo/node_status_fetch_service_spec.rb

@@ -121,5 +121,29 @@ describe Geo::NodeStatusFetchService, :geo do
 
       expect(status.status_message).to eq('This GitLab instance does not appear to be configured properly as a Geo node. Make sure the URLs are using the correct fully-qualified domain names.')
     end
+
+    it 'returns the status from database if it could not fetch it' do
+      allow(described_class).to receive(:get).and_raise(Errno::ECONNREFUSED.new('bad connection'))
+      db_status = create(:geo_node_status, :healthy, geo_node: secondary)
+
+      status = subject.call(secondary)
+
+      expect(status.status_message).to eq('Connection refused - bad connection')
+      expect(status).not_to be_healthy
+      expect(status.attachments_count).to eq(db_status.attachments_count)
+      expect(status.attachments_failed_count).to eq(db_status.attachments_failed_count)
+      expect(status.attachments_synced_count).to eq(db_status.attachments_synced_count)
+      expect(status.lfs_objects_count).to eq(db_status.lfs_objects_count)
+      expect(status.lfs_objects_failed_count).to eq(db_status.lfs_objects_failed_count)
+      expect(status.lfs_objects_synced_count).to eq(db_status.lfs_objects_synced_count)
+      expect(status.repositories_count).to eq(db_status.repositories_count)
+      expect(status.repositories_synced_count).to eq(db_status.repositories_synced_count)
+      expect(status.repositories_failed_count).to eq(db_status.repositories_failed_count)
+      expect(status.last_event_id).to eq(db_status.last_event_id)
+      expect(status.last_event_timestamp).to eq(db_status.last_event_timestamp)
+      expect(status.cursor_last_event_id).to eq(db_status.cursor_last_event_id)
+      expect(status.cursor_last_event_timestamp).to eq(db_status.cursor_last_event_timestamp)
+      expect(status.last_successful_status_check_timestamp).to eq(db_status.last_successful_status_check_timestamp)
+    end
   end
 end