Merge branch '4658-geo-admin-fixes' into 'master'

Fixes and enhancements for Geo admin dashboard

Closes #4658, #4864, and #4493

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

app/assets/javascripts/lib/utils/http_status.js

@@ -8,4 +8,5 @@ export default {
   OK: 200,
   MULTIPLE_CHOICES: 300,
   BAD_REQUEST: 400,
+  NOT_FOUND: 404,
 };

doc/api/geo_nodes.md

@@ -96,12 +96,24 @@ Example response:
 }
 ```
 
+## Delete a Geo node
+
+Removes the Geo node.
+
+```
+DELETE /geo_nodes/:id
+```
+
+| Attribute | Type    | Required | Description             |
+|-----------|---------|----------|-------------------------|
+| `id`      | integer | yes      | The ID of the Geo node. |
+
 ## Repair a Geo node
 
 To repair the OAuth authentication of a Geo node.
 
 ```
-PUT /geo_nodes/:id/repair
+POST /geo_nodes/:id/repair
 ```
 
 Example response:
@@ -177,6 +189,10 @@ Example response:
 GET /geo_nodes/:id/status
 ```
 
+| Attribute | Type    | Required | Description |
+| --------- | ------- | -------- | ----------- |
+| `refresh` | boolean | no       | Attempt to fetch the latest status from the Geo node directly, ignoring the cache |
+
 ```bash
 curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/geo_nodes/2/status
 ```

ee/app/assets/javascripts/geo_nodes/components/app.vue

 <script>
+  import { s__ } from '~/locale';
+  import Flash from '~/flash';
+  import statusCodes from '~/lib/utils/http_status';
   import loadingIcon from '~/vue_shared/components/loading_icon.vue';
+  import modal from '~/vue_shared/components/modal.vue';
   import SmartInterval from '~/smart_interval';
 
   import eventHub from '../event_hub';
 
+  import { NODE_ACTIONS } from '../constants';
+
   import geoNodesList from './geo_nodes_list.vue';
 
   export default {
     components: {
       loadingIcon,
+      modal,
       geoNodesList,
     },
     props: {
@@ -33,6 +40,12 @@
       return {
         isLoading: true,
         hasError: false,
+        showModal: false,
+        targetNode: null,
+        targetNodeActionType: '',
+        modalKind: 'warning',
+        modalMessage: '',
+        modalActionLabel: '',
         errorMessage: '',
       };
     },
@@ -43,17 +56,34 @@
     },
     created() {
       eventHub.$on('pollNodeDetails', this.initNodeDetailsPolling);
+      eventHub.$on('showNodeActionModal', this.showNodeActionModal);
+      eventHub.$on('repairNode', this.repairNode);
     },
     mounted() {
       this.fetchGeoNodes();
     },
     beforeDestroy() {
       eventHub.$off('pollNodeDetails', this.initNodeDetailsPolling);
+      eventHub.$off('showNodeActionModal', this.showNodeActionModal);
+      eventHub.$off('repairNode', this.repairNode);
       if (this.nodePollingInterval) {
         this.nodePollingInterval.stopTimer();
       }
     },
     methods: {
+      setNodeActionStatus(node, status) {
+        Object.assign(node, { nodeActionActive: status });
+      },
+      initNodeDetailsPolling(node) {
+        this.nodePollingInterval = new SmartInterval({
+          callback: this.fetchNodeDetails.bind(this, node),
+          startingInterval: 30000,
+          maxInterval: 120000,
+          hiddenInterval: 240000,
+          incrementByFactorOf: 15000,
+          immediateExecution: true,
+        });
+      },
       fetchGeoNodes() {
         this.hasError = false;
         this.service.getGeoNodes()
@@ -67,8 +97,9 @@
             this.errorMessage = err;
           });
       },
-      fetchNodeDetails(nodeId) {
-        return this.service.getGeoNodeDetails(nodeId)
+      fetchNodeDetails(node) {
+        const nodeId = node.id;
+        return this.service.getGeoNodeDetails(node)
           .then(res => res.data)
           .then((nodeDetails) => {
             const primaryNodeVersion = this.store.getPrimaryNodeVersion();
@@ -80,18 +111,81 @@
             eventHub.$emit('nodeDetailsLoaded', this.store.getNodeDetails(nodeId));
           })
           .catch((err) => {
-            eventHub.$emit('nodeDetailsLoadFailed', nodeId, err);
+            if (err.response && err.response.status === statusCodes.NOT_FOUND) {
+              this.store.setNodeDetails(nodeId, {
+                geo_node_id: nodeId,
+                health: err.message,
+                health_status: 'Unknown',
+                missing_oauth_application: false,
+                sync_status_unavailable: true,
+                storage_shards_match: null,
+              });
+              eventHub.$emit('nodeDetailsLoaded', this.store.getNodeDetails(nodeId));
+            } else {
+              eventHub.$emit('nodeDetailsLoadFailed', nodeId, err);
+            }
           });
       },
-      initNodeDetailsPolling(nodeId) {
-        this.nodePollingInterval = new SmartInterval({
-          callback: this.fetchNodeDetails.bind(this, nodeId),
-          startingInterval: 30000,
-          maxInterval: 120000,
-          hiddenInterval: 240000,
-          incrementByFactorOf: 15000,
-          immediateExecution: true,
-        });
+      repairNode(targetNode) {
+        this.setNodeActionStatus(targetNode, true);
+        this.service.repairNode(targetNode)
+          .then(() => {
+            this.setNodeActionStatus(targetNode, false);
+            Flash(s__('GeoNodes|Node Authentication was successfully repaired.'), 'notice');
+          })
+          .catch(() => {
+            this.setNodeActionStatus(targetNode, false);
+            Flash(s__('GeoNodes|Something went wrong while repairing node'));
+          });
+      },
+      toggleNode(targetNode) {
+        this.setNodeActionStatus(targetNode, true);
+        this.service.toggleNode(targetNode)
+          .then(res => res.data)
+          .then((node) => {
+            Object.assign(targetNode, { enabled: node.enabled, nodeActionActive: false });
+          })
+          .catch(() => {
+            this.setNodeActionStatus(targetNode, false);
+            Flash(s__('GeoNodes|Something went wrong while changing node status'));
+          });
+      },
+      removeNode(targetNode) {
+        this.setNodeActionStatus(targetNode, true);
+        this.service.removeNode(targetNode)
+          .then(() => {
+            this.store.removeNode(targetNode);
+            Flash(s__('GeoNodes|Node was successfully removed.'), 'notice');
+          })
+          .catch(() => {
+            this.setNodeActionStatus(targetNode, false);
+            Flash(s__('GeoNodes|Something went wrong while removing node'));
+          });
+      },
+      handleNodeAction() {
+        this.showModal = false;
+
+        if (this.targetNodeActionType === NODE_ACTIONS.TOGGLE) {
+          this.toggleNode(this.targetNode);
+        } else if (this.targetNodeActionType === NODE_ACTIONS.REMOVE) {
+          this.removeNode(this.targetNode);
+        }
+      },
+      showNodeActionModal({ actionType, node, modalKind = 'warning', modalMessage, modalActionLabel }) {
+        this.targetNode = node;
+        this.targetNodeActionType = actionType;
+        this.modalKind = modalKind;
+        this.modalMessage = modalMessage;
+        this.modalActionLabel = modalActionLabel;
+
+        if (actionType === NODE_ACTIONS.TOGGLE && !node.enabled) {
+          this.toggleNode(this.targetNode);
+        } else {
+          this.showModal = true;
+        }
+      },
+      hideNodeActionModal() {
+        this.showModal = false;
       },
     },
   };
@@ -120,5 +214,14 @@
     >
       {{ errorMessage }}
     </p>
+    <modal
+      v-show="showModal"
+      :title="__('Are you sure?')"
+      :kind="modalKind"
+      :text="modalMessage"
+      :primary-button-label="modalActionLabel"
+      @cancel="hideNodeActionModal"
+      @submit="handleNodeAction"
+    />
   </div>
 </template>

ee/app/assets/javascripts/geo_nodes/components/geo_node_actions.vue

@@ -2,7 +2,9 @@
   import { __, s__ } from '~/locale';
   import loadingIcon from '~/vue_shared/components/loading_icon.vue';
 
-  import { NODE_ACTION_BASE_PATH, NODE_ACTIONS } from '../constants';
+  import eventHub from '../event_hub';
+
+  import { NODE_ACTIONS } from '../constants';
 
   export default {
     components: {
@@ -22,11 +24,6 @@
         required: true,
       },
     },
-    data() {
-      return {
-        isNodeToggleInProgress: false,
-      };
-    },
     computed: {
       isToggleAllowed() {
         return !this.node.primary && this.nodeEditAllowed;
@@ -34,20 +31,27 @@
       nodeToggleLabel() {
         return this.node.enabled ? __('Disable') : __('Enable');
       },
-      nodeDisableMessage() {
-        return this.node.enabled ? s__('GeoNodes|Disabling a node stops the sync process. Are you sure?') : '';
-      },
-      nodePath() {
-        return `${NODE_ACTION_BASE_PATH}${this.node.id}`;
-      },
-      nodeRepairAuthPath() {
-        return `${this.nodePath}${NODE_ACTIONS.REPAIR}`;
+    },
+    methods: {
+      onToggleNode() {
+        eventHub.$emit('showNodeActionModal', {
+          actionType: NODE_ACTIONS.TOGGLE,
+          node: this.node,
+          modalMessage: s__('GeoNodes|Disabling a node stops the sync process. Are you sure?'),
+          modalActionLabel: this.nodeToggleLabel,
+        });
       },
-      nodeTogglePath() {
-        return `${this.nodePath}${NODE_ACTIONS.TOGGLE}`;
+      onRemoveNode() {
+        eventHub.$emit('showNodeActionModal', {
+          actionType: NODE_ACTIONS.REMOVE,
+          node: this.node,
+          modalKind: 'danger',
+          modalMessage: s__('GeoNodes|Removing a node stops the sync process. Are you sure?'),
+          modalActionLabel: __('Remove'),
+        });
       },
-      nodeEditPath() {
-        return `${this.nodePath}${NODE_ACTIONS.EDIT}`;
+      onRepairNode() {
+        eventHub.$emit('repairNode', this.node);
       },
     },
   };
@@ -59,30 +63,29 @@
       v-if="nodeMissingOauth"
       class="node-action-container"
     >
-      <a
+      <button
+        type="button"
         class="btn btn-default btn-sm btn-node-action"
-        data-method="post"
-        :href="nodeRepairAuthPath"
+        @click="onRepairNode"
       >
         {{ s__('Repair authentication') }}
-      </a>
+      </button>
     </div>
     <div
       v-if="isToggleAllowed"
       class="node-action-container"
     >
-      <a
+      <button
+        type="button"
         class="btn btn-sm btn-node-action"
-        data-method="post"
-        :href="nodeTogglePath"
-        :data-confirm="nodeDisableMessage"
         :class="{
           'btn-warning': node.enabled,
           'btn-success': !node.enabled
         }"
+        @click="onToggleNode"
       >
         {{ nodeToggleLabel }}
-      </a>
+      </button>
     </div>
     <div
       v-if="nodeEditAllowed"
@@ -90,19 +93,19 @@
     >
       <a
         class="btn btn-sm btn-node-action"
-        :href="nodeEditPath"
+        :href="node.editPath"
       >
         {{ __('Edit') }}
       </a>
     </div>
     <div class="node-action-container">
-      <a
+      <button
+        type="button"
         class="btn btn-sm btn-node-action btn-danger"
-        data-method="delete"
-        :href="nodePath"
+        @click="onRemoveNode"
       >
         {{ __('Remove') }}
-      </a>
+      </button>
     </div>
   </div>
 </template>

ee/app/assets/javascripts/geo_nodes/components/geo_node_detail_item.vue

@@ -106,6 +106,7 @@
       />
       <geo-node-sync-settings
         v-else-if="isCustomTypeSync"
+        :sync-status-unavailable="itemValue.syncStatusUnavailable"
         :selective-sync-type="itemValue.selectiveSyncType"
         :last-event="itemValue.lastEvent"
         :cursor-last-event="itemValue.cursorLastEvent"

ee/app/assets/javascripts/geo_nodes/components/geo_node_details.vue

@@ -97,6 +97,10 @@
         return this.showAdvanceItems ? 'angle-up' : 'angle-down';
       },
       nodeVersion() {
+        if (this.nodeDetails.version == null &&
+            this.nodeDetails.revision == null) {
+          return __('Unknown');
+        }
         return `${this.nodeDetails.version} (${this.nodeDetails.revision})`;
       },
       replicationSlotWAL() {
@@ -113,7 +117,8 @@
 
           return stringifyTime(parsedTime);
         }
-        return 'Unknown';
+
+        return __('Unknown');
       },
       lastEventStatus() {
         return {
@@ -150,6 +155,7 @@
       },
       syncSettings() {
         return {
+          syncStatusUnavailable: this.nodeDetails.syncStatusUnavailable,
           selectiveSyncType: this.nodeDetails.selectiveSyncType,
           lastEvent: this.nodeDetails.lastEvent,
           cursorLastEvent: this.nodeDetails.cursorLastEvent,

ee/app/assets/javascripts/geo_nodes/components/geo_node_item.vue

@@ -112,14 +112,16 @@ export default {
       }
     },
     handleMounted() {
-      eventHub.$emit('pollNodeDetails', this.node.id);
+      eventHub.$emit('pollNodeDetails', this.node);
     },
   },
 };
 </script>
 
 <template>
-  <li>
+  <li
+    :class="{ 'node-action-active': node.nodeActionActive }"
+  >
     <div class="row">
       <div class="col-md-8">
         <div class="row">
@@ -128,7 +130,7 @@ export default {
               {{ node.url }}
             </strong>
             <loading-icon
-              v-if="isNodeDetailsLoading"
+              v-if="isNodeDetailsLoading || node.nodeActionActive"
               class="node-details-loading prepend-left-10 pull-left inline"
               size="1"
             />

ee/app/assets/javascripts/geo_nodes/components/geo_node_sync_settings.vue

@@ -14,6 +14,11 @@
       icon,
     },
     props: {
+      syncStatusUnavailable: {
+        type: Boolean,
+        required: false,
+        default: false,
+      },
       selectiveSyncType: {
         type: String,
         required: false,
@@ -105,6 +110,13 @@
     class="node-detail-value"
   >
     <span
+      v-if="syncStatusUnavailable"
+      class="node-detail-value-bold"
+    >
+      {{ __('Unknown') }}
+    </span>
+    <span
+      v-else
       v-tooltip
       class="node-sync-settings inline"
       data-placement="bottom"

ee/app/assets/javascripts/geo_nodes/constants.js

-export const NODE_ACTION_BASE_PATH = '/admin/geo_nodes/';
-
 export const NODE_ACTIONS = {
-  TOGGLE: '/toggle',
-  EDIT: '/edit',
-  REPAIR: '/repair',
+  TOGGLE: 'toggle',
+  REMOVE: 'remove',
 };
 
 export const VALUE_TYPE = {

ee/app/assets/javascripts/geo_nodes/index.js

@@ -14,11 +14,10 @@ export default () => {
   const el = document.getElementById('js-geo-nodes');
 
   if (!el) {
-    return;
+    return false;
   }
 
-  // eslint-disable-next-line no-new
-  new Vue({
+  return new Vue({
     el,
     components: {
       geoNodesApp,
@@ -28,7 +27,7 @@ export default () => {
       const nodeActionsAllowed = convertPermissionToBoolean(dataset.nodeActionsAllowed);
       const nodeEditAllowed = convertPermissionToBoolean(dataset.nodeEditAllowed);
       const store = new GeoNodesStore(dataset.primaryVersion, dataset.primaryRevision);
-      const service = new GeoNodesService(dataset.nodeDetailsPath);
+      const service = new GeoNodesService();
 
       return {
         store,

ee/app/assets/javascripts/geo_nodes/service/geo_nodes_service.js

@@ -3,8 +3,7 @@ import axios from '~/lib/utils/axios_utils';
 import Api from '~/api';
 
 export default class GeoNodesService {
-  constructor(nodeDetailsBasePath) {
-    this.geoNodeDetailsBasePath = nodeDetailsBasePath;
+  constructor() {
     this.geoNodesPath = Api.buildUrl(Api.geoNodesPath);
   }
 
@@ -12,8 +11,29 @@ export default class GeoNodesService {
     return axios.get(this.geoNodesPath);
   }
 
-  getGeoNodeDetails(nodeId) {
-    const geoNodeDetailsPath = `${this.geoNodeDetailsBasePath}/${nodeId}/status.json`;
-    return axios.get(geoNodeDetailsPath);
+  // eslint-disable-next-line class-methods-use-this
+  getGeoNodeDetails(node) {
+    return axios.get(node.statusPath, {
+      params: {
+        refresh: true,
+      },
+    });
+  }
+
+  // eslint-disable-next-line class-methods-use-this
+  toggleNode(node) {
+    return axios.put(node.basePath, {
+      enabled: !node.enabled, // toggle from existing status
+    });
+  }
+
+  // eslint-disable-next-line class-methods-use-this
+  repairNode(node) {
+    return axios.post(node.repairPath);
+  }
+
+  // eslint-disable-next-line class-methods-use-this
+  removeNode(node) {
+    return axios.delete(node.basePath);
   }
 }

ee/app/assets/javascripts/geo_nodes/store/geo_nodes_store.js

@@ -8,7 +8,9 @@ export default class GeoNodesStore {
   }
 
   setNodes(nodes) {
-    this.state.nodes = nodes;
+    this.state.nodes = nodes.map(
+      node => GeoNodesStore.formatNode(node),
+    );
   }
 
   getNodes() {
@@ -19,6 +21,16 @@ export default class GeoNodesStore {
     this.state.nodeDetails[nodeId] = GeoNodesStore.formatNodeDetails(nodeDetails);
   }
 
+  removeNode(node) {
+    const indexOfRemovedNode = this.state.nodes.indexOf(node);
+    if (indexOfRemovedNode > -1) {
+      this.state.nodes.splice(indexOfRemovedNode, 1);
+      if (this.state.nodeDetails[node.id]) {
+        delete this.state.nodeDetails[node.id];
+      }
+    }
+  }
+
   getPrimaryNodeVersion() {
     return {
       version: this.state.primaryVersion,
@@ -30,6 +42,22 @@ export default class GeoNodesStore {
     return this.state.nodeDetails[nodeId];
   }
 
+  static formatNode(rawNode) {
+    const { id, url, primary, current, enabled } = rawNode;
+    return {
+      id,
+      url,
+      primary,
+      current,
+      enabled,
+      nodeActionActive: false,
+      basePath: rawNode._links.self,
+      repairPath: rawNode._links.repair,
+      editPath: rawNode.web_edit_url,
+      statusPath: rawNode._links.status,
+    };
+  }
+
   static formatNodeDetails(rawNodeDetails) {
     return {
       id: rawNodeDetails.geo_node_id,
@@ -41,8 +69,9 @@ export default class GeoNodesStore {
       primaryVersion: rawNodeDetails.primaryVersion,
       primaryRevision: rawNodeDetails.primaryRevision,
       replicationSlotWAL: rawNodeDetails.replication_slots_max_retained_wal_bytes,
-      missingOAuthApplication: rawNodeDetails.missing_oauth_application,
+      missingOAuthApplication: rawNodeDetails.missing_oauth_application || false,
       storageShardsMatch: rawNodeDetails.storage_shards_match,
+      syncStatusUnavailable: rawNodeDetails.sync_status_unavailable || false,
       replicationSlots: {
         totalCount: rawNodeDetails.replication_slots_count || 0,
         successCount: rawNodeDetails.replication_slots_used_count || 0,

ee/app/assets/stylesheets/pages/geo_nodes.scss

@@ -13,7 +13,7 @@
   }
 
   .health-message {
-    padding: 4px 8px 1px;
+    padding: 2px 8px;
     background-color: $red-100;
     color: $red-500;
     border-radius: $border-radius-default;
@@ -29,9 +29,9 @@
       background: $white-light;
     }
 
-    &.node-disabled,
-    &.node-disabled:hover {
-      background-color: $gray-lightest;
+    &.node-action-active {
+      pointer-events: none;
+      opacity: 0.5;
     }
   }
 }

ee/app/controllers/admin/geo_nodes_controller.rb

@@ -37,50 +37,6 @@ class Admin::GeoNodesController < Admin::ApplicationController
     end
   end
 
-  def destroy
-    @node.destroy
-
-    redirect_to admin_geo_nodes_path, status: 302, notice: 'Node was successfully removed.'
-  end
-
-  def repair
-    if !@node.missing_oauth_application?
-      flash[:notice] = "This node doesn't need to be repaired."
-    elsif @node.repair
-      flash[:notice] = 'Node Authentication was successfully repaired.'
-    else
-      flash[:alert] = 'There was a problem repairing Node Authentication.'
-    end
-
-    redirect_to admin_geo_nodes_path
-  end
-
-  def toggle
-    if @node.primary?
-      flash[:alert] = "Primary node can't be disabled."
-    else
-      if @node.toggle!(:enabled)
-        new_status = @node.enabled? ? 'enabled' : 'disabled'
-        flash[:notice] = "Node #{@node.url} was successfully #{new_status}."
-      else
-        action = @node.enabled? ? 'disabling' : 'enabling'
-        flash[:alert] = "There was a problem #{action} node #{@node.url}."
-      end
-    end
-
-    redirect_to admin_geo_nodes_path
-  end
-
-  def status
-    status = Geo::NodeStatusFetchService.new.call(@node)
-
-    respond_to do |format|
-      format.json do
-        render json: GeoNodeStatusSerializer.new.represent(status)
-      end
-    end
-  end
-
   private
 
   def geo_node_params

ee/app/helpers/ee/geo_helper.rb

@@ -13,7 +13,6 @@ module EE
       {
         primary_version: version.to_s,
         primary_revision: revision.to_s,
-        node_details_path: admin_geo_nodes_path.to_s,
         node_actions_allowed: ::Gitlab::Database.read_write?.to_s,
         node_edit_allowed: ::Gitlab::Geo.license_allows?.to_s
       }

ee/changelogs/unreleased/4658-geo-admin-fixes.yml

+---
+title: Fixes and enhancements for Geo admin dashboard
+merge_request: 4536
+author:
+type: fixed

ee/lib/api/geo_nodes.rb

@@ -66,6 +66,8 @@ module API
             strong_memoize(:geo_node_status) do
               if geo_node.current?
                 GeoNodeStatus.current_node_status
+              elsif to_boolean(declared_params(include_missing: false)[:refresh])
+                ::Geo::NodeStatusFetchService.new.call(geo_node)
               else
                 geo_node.status
               end
@@ -93,6 +95,9 @@ module API
         desc 'Get metrics for a single Geo node' do
           success EE::API::Entities::GeoNodeStatus
         end
+        params do
+          optional :refresh, type: Boolean, desc: 'Attempt to fetch the latest status from the Geo node directly, ignoring the cache'
+        end
         get 'status' do
           not_found!('GeoNode') unless geo_node
 
@@ -145,6 +150,20 @@ module API
             render_validation_error!(geo_node)
           end
         end
+
+        # Delete an existing Geo node
+        #
+        # Example request:
+        #   DELETE /geo_nodes/:id
+        desc 'Delete an existing Geo secondary node' do
+          success EE::API::Entities::GeoNode
+        end
+        delete do
+          not_found!('GeoNode') unless geo_node
+
+          geo_node.destroy!
+          status 204
+        end
       end
     end
   end

ee/lib/ee/api/entities.rb

@@ -224,11 +224,19 @@ module EE
           'http'
         end
 
+        expose :web_edit_url do |geo_node|
+          ::Gitlab::Routing.url_helpers.edit_admin_geo_node_url(geo_node)
+        end
+
         expose :_links do
           expose :self do |geo_node|
             expose_url api_v4_geo_nodes_path(id: geo_node.id)
           end
 
+          expose :status do |geo_node|
+            expose_url api_v4_geo_nodes_status_path(id: geo_node.id)
+          end
+
           expose :repair do |geo_node|
             expose_url api_v4_geo_nodes_repair_path(id: geo_node.id)
           end

ee/spec/controllers/admin/geo_nodes_controller_spec.rb

@@ -55,34 +55,6 @@ describe Admin::GeoNodesController, :postgresql do
     end
   end
 
-  describe '#destroy' do
-    let!(:geo_node) { create(:geo_node) }
-
-    def go
-      delete(:destroy, id: geo_node)
-    end
-
-    context 'without add-on license' do
-      before do
-        allow(Gitlab::Geo).to receive(:license_allows?).and_return(false)
-      end
-
-      it 'deletes the node' do
-        expect { go }.to change { GeoNode.count }.by(-1)
-      end
-    end
-
-    context 'with add-on license' do
-      before do
-        allow(Gitlab::Geo).to receive(:license_allows?).and_return(true)
-      end
-
-      it 'deletes the node' do
-        expect { go }.to change { GeoNode.count }.by(-1)
-      end
-    end
-  end
-
   describe '#create' do
     let(:geo_node_attributes) { { url: 'http://example.com' } }
 
@@ -149,126 +121,4 @@ describe Admin::GeoNodesController, :postgresql do
       end
     end
   end
-
-  describe '#repair' do
-    let(:geo_node) { create(:geo_node) }
-    def go
-      post :repair, id: geo_node
-    end
-
-    before do
-      allow(Gitlab::Geo).to receive(:license_allows?) { false }
-      go
-    end
-
-    it_behaves_like 'unlicensed geo action'
-  end
-
-  describe '#toggle' do
-    context 'without add-on license' do
-      let(:geo_node) { create(:geo_node) }
-
-      before do
-        allow(Gitlab::Geo).to receive(:license_allows?).and_return(false)
-        post :toggle, id: geo_node
-      end
-
-      it_behaves_like 'unlicensed geo action'
-    end
-
-    context 'with add-on license' do
-      before do
-        allow(Gitlab::Geo).to receive(:license_allows?).and_return(true)
-      end
-
-      context 'with a primary node' do
-        before do
-          post :toggle, id: geo_node
-        end
-
-        let(:geo_node) { create(:geo_node, :primary, enabled: true) }
-
-        it 'does not disable the node' do
-          expect(geo_node.reload).to be_enabled
-        end
-
-        it 'displays a flash message' do
-          expect(controller).to set_flash[:alert].to("Primary node can't be disabled.")
-        end
-
-        it 'redirects to the geo nodes page' do
-          expect(response).to redirect_to(admin_geo_nodes_path)
-        end
-      end
-
-      context 'with a secondary node' do
-        let(:geo_node) { create(:geo_node, url: 'http://example.com') }
-
-        context 'when succeed' do
-          before do
-            post :toggle, id: geo_node
-          end
-
-          it 'disables the node' do
-            expect(geo_node.reload).not_to be_enabled
-          end
-
-          it 'displays a flash message' do
-            expect(controller).to set_flash[:notice].to('Node http://example.com/ was successfully disabled.')
-          end
-
-          it 'redirects to the geo nodes page' do
-            expect(response).to redirect_to(admin_geo_nodes_path)
-          end
-        end
-
-        context 'when fail' do
-          before do
-            allow_any_instance_of(GeoNode).to receive(:toggle!).and_return(false)
-            post :toggle, id: geo_node
-          end
-
-          it 'does not disable the node' do
-            expect(geo_node.reload).to be_enabled
-          end
-
-          it 'displays a flash message' do
-            expect(controller).to set_flash[:alert].to('There was a problem disabling node http://example.com/.')
-          end
-
-          it 'redirects to the geo nodes page' do
-            expect(response).to redirect_to(admin_geo_nodes_path)
-          end
-        end
-      end
-    end
-  end
-
-  describe '#status' do
-    let(:geo_node) { create(:geo_node) }
-
-    context 'without add-on license' do
-      before do
-        allow(Gitlab::Geo).to receive(:license_allows?).and_return(false)
-        get :status, id: geo_node, format: :json
-      end
-
-      it_behaves_like 'unlicensed geo action'
-    end
-
-    context 'with add-on license' do
-      let(:geo_node_status) { build(:geo_node_status, :healthy, geo_node: geo_node) }
-
-      before do
-        allow(Gitlab::Geo).to receive(:license_allows?).and_return(true)
-        allow_any_instance_of(Geo::NodeStatusFetchService).to receive(:call).and_return(geo_node_status)
-      end
-
-      it 'returns the status' do
-        get :status, id: geo_node, format: :json
-
-        expect(response).to match_response_schema('public_api/v4/geo_node_status', dir: 'ee')
-      end
-    end
-  end
 end

ee/spec/features/admin/admin_geo_nodes_spec.rb

@@ -83,7 +83,10 @@ describe 'admin Geo Nodes', :js do
 
     it 'removes an existing Geo Node' do
       page.within(find('.geo-node-actions', match: :first)) do
-        page.click_link('Remove')
+        page.click_button('Remove')
+      end
+      page.within('.modal') do
+        page.click_button('Remove')
       end
 
       expect(current_path).to eq admin_geo_nodes_path

ee/spec/fixtures/api/schemas/public_api/v4/geo_node.json

@@ -20,11 +20,13 @@
     "files_max_capacity": { "type": "integer" },
     "repos_max_capacity": { "type": "integer" },
     "clone_protocol": { "type": ["string"] },
+    "web_edit_url": { "type": "string" },
     "_links": {
       "type": "object",
       "required": ["self", "repair"],
       "properties" : {
         "self": { "type": "string" },
+        "status": { "type": "string" },
         "repair": { "type": "string" }
       },
       "additionalProperties": false

ee/spec/requests/api/geo_nodes_spec.rb

@@ -34,9 +34,11 @@ 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', dir: 'ee')
+      expect(json_response['web_edit_url']).to end_with("/admin/geo_nodes/#{primary.id}/edit")
 
       links = json_response['_links']
       expect(links['self']).to end_with("/api/v4/geo_nodes/#{primary.id}")
+      expect(links['status']).to end_with("/api/v4/geo_nodes/#{primary.id}/status")
       expect(links['repair']).to end_with("/api/v4/geo_nodes/#{primary.id}/repair")
     end
 
@@ -99,6 +101,32 @@ describe API::GeoNodes, :geo, api: true do
       expect(response).to match_response_schema('public_api/v4/geo_node_status', dir: 'ee')
     end
 
+    it 'fetches the real-time status with `refresh=true`' do
+      stub_current_geo_node(primary)
+      new_status = build(:geo_node_status, :healthy, geo_node: secondary, attachments_count: 923, lfs_objects_count: 652)
+
+      expect(GeoNode).to receive(:find).and_return(secondary)
+      expect_any_instance_of(Geo::NodeStatusFetchService).to receive(:call).and_return(new_status)
+
+      get api("/geo_nodes/#{secondary.id}/status", admin), refresh: true
+
+      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['attachments_count']).to eq(923)
+      expect(json_response['lfs_objects_count']).to eq(652)
+    end
+
+    it 'returns 404 when no Geo Node status is not found' do
+      stub_current_geo_node(primary)
+      secondary_status.destroy!
+
+      expect(GeoNode).to receive(:find).and_return(secondary)
+
+      get api("/geo_nodes/#{secondary.id}/status", admin)
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+
     it_behaves_like '404 response' do
       let(:request) { get api("/geo_nodes/#{unexisting_node_id}/status", admin) }
     end
@@ -149,7 +177,7 @@ describe API::GeoNodes, :geo, api: true do
 
   describe 'PUT /geo_nodes/:id' do
     it_behaves_like '404 response' do
-      let(:request) { get api("/geo_nodes/#{unexisting_node_id}/status", admin) }
+      let(:request) { put api("/geo_nodes/#{unexisting_node_id}", admin), {} }
     end
 
     it 'denies access if not admin' do
@@ -174,6 +202,32 @@ describe API::GeoNodes, :geo, api: true do
     end
   end
 
+  describe 'DELETE /geo_nodes/:id' do
+    it_behaves_like '404 response' do
+      let(:request) { delete api("/geo_nodes/#{unexisting_node_id}", admin) }
+    end
+
+    it 'denies access if not admin' do
+      delete api("/geo_nodes/#{secondary.id}", user)
+
+      expect(response).to have_gitlab_http_status(403)
+    end
+
+    it 'deletes the node' do
+      delete api("/geo_nodes/#{secondary.id}", admin)
+
+      expect(response).to have_gitlab_http_status(204)
+    end
+
+    it 'returns 400 if Geo Node could not be deleted' do
+      allow_any_instance_of(GeoNode).to receive(:destroy!).and_raise(StandardError, 'Something wrong')
+
+      delete api("/geo_nodes/#{secondary.id}", admin)
+
+      expect(response).to have_gitlab_http_status(500)
+    end
+  end
+
   describe 'GET /geo_nodes/current/failures/:type' do
     it 'fetches the current node failures' do
       create(:geo_project_registry, :sync_failed)

spec/javascripts/geo_nodes/components/geo_node_actions_spec.js

@@ -2,6 +2,8 @@ import Vue from 'vue';
 
 import geoNodeActionsComponent from 'ee/geo_nodes/components/geo_node_actions.vue';
 import mountComponent from 'spec/helpers/vue_mount_component_helper';
+import eventHub from 'ee/geo_nodes/event_hub';
+import { NODE_ACTIONS } from 'ee/geo_nodes/constants';
 import { mockNodes } from '../mock_data';
 
 const createComponent = (node = mockNodes[0], nodeEditAllowed = true, nodeMissingOauth = false) => {
@@ -25,14 +27,6 @@ describe('GeoNodeActionsComponent', () => {
     vm.$destroy();
   });
 
-  describe('data', () => {
-    it('returns default data props', () => {
-      const vmX = createComponent();
-      expect(vmX.isNodeToggleInProgress).toBeFalsy();
-      vmX.$destroy();
-    });
-  });
-
   describe('computed', () => {
     describe('isToggleAllowed', () => {
       it('returns boolean value representing if toggle on node can be allowed', () => {
@@ -59,49 +53,48 @@ describe('GeoNodeActionsComponent', () => {
         vmX.$destroy();
       });
     });
+  });
 
-    describe('nodeDisableMessage', () => {
-      it('returns node toggle message', () => {
-        let mockNode = Object.assign({}, mockNodes[1]);
-        let vmX = createComponent(mockNode);
-        expect(vmX.nodeDisableMessage).toBe('Disabling a node stops the sync process. Are you sure?');
-        vmX.$destroy();
-
-        mockNode = Object.assign({}, mockNodes[1], { enabled: false });
-        vmX = createComponent(mockNode);
-        expect(vmX.nodeDisableMessage).toBe('');
-        vmX.$destroy();
-      });
-    });
-
-    describe('nodePath', () => {
-      it('returns node path', () => {
-        expect(vm.nodePath).toBe('/admin/geo_nodes/1');
-      });
-    });
-
-    describe('nodeRepairAuthPath', () => {
-      it('returns node repair authentication path', () => {
-        expect(vm.nodeRepairAuthPath).toBe('/admin/geo_nodes/1/repair');
+  describe('methods', () => {
+    describe('onToggleNode', () => {
+      it('emits showNodeActionModal with actionType `toggle`, node reference, modalMessage and modalActionLabel', () => {
+        spyOn(eventHub, '$emit');
+        vm.onToggleNode();
+        expect(eventHub.$emit).toHaveBeenCalledWith('showNodeActionModal', {
+          actionType: NODE_ACTIONS.TOGGLE,
+          node: vm.node,
+          modalMessage: 'Disabling a node stops the sync process. Are you sure?',
+          modalActionLabel: vm.nodeToggleLabel,
+        });
       });
     });
 
-    describe('nodeTogglePath', () => {
-      it('returns node toggle path', () => {
-        expect(vm.nodeTogglePath).toBe('/admin/geo_nodes/1/toggle');
+    describe('onRemoveNode', () => {
+      it('emits showNodeActionModal with actionType `remove`, node reference, modalKind, modalMessage and modalActionLabel', () => {
+        spyOn(eventHub, '$emit');
+        vm.onRemoveNode();
+        expect(eventHub.$emit).toHaveBeenCalledWith('showNodeActionModal', {
+          actionType: NODE_ACTIONS.REMOVE,
+          node: vm.node,
+          modalKind: 'danger',
+          modalMessage: 'Removing a node stops the sync process. Are you sure?',
+          modalActionLabel: 'Remove',
+        });
       });
     });
 
-    describe('nodeEditPath', () => {
-      it('returns node edit path', () => {
-        expect(vm.nodeEditPath).toBe('/admin/geo_nodes/1/edit');
+    describe('onRepairNode', () => {
+      it('emits `repairNode` event with node reference', () => {
+        spyOn(eventHub, '$emit');
+        vm.onRepairNode();
+        expect(eventHub.$emit).toHaveBeenCalledWith('repairNode', vm.node);
       });
     });
   });
 
   describe('template', () => {
     it('renders container elements correctly', () => {
-      expect(vm.$el.classList.contains('geo-node-actions')).toBeTruthy();
+      expect(vm.$el.classList.contains('geo-node-actions')).toBe(true);
       expect(vm.$el.querySelectorAll('.node-action-container').length).not.toBe(0);
       expect(vm.$el.querySelectorAll('.btn-node-action').length).not.toBe(0);
     });

spec/javascripts/geo_nodes/components/geo_node_details_spec.js

@@ -77,6 +77,16 @@ describe('GeoNodeDetailsComponent', () => {
     });
 
     describe('nodeVersion', () => {
+      it('returns `Unknown` when `version` and `revision` are null', () => {
+        const nodeDetailsVersionNull = Object.assign({}, mockNodeDetails, {
+          version: null,
+          revision: null,
+        });
+        const vmVersionNull = createComponent(nodeDetailsVersionNull);
+        expect(vmVersionNull.nodeVersion).toBe('Unknown');
+        vmVersionNull.$destroy();
+      });
+
       it('returns version string', () => {
         expect(vm.nodeVersion).toBe('10.4.0-pre (b93c51849b)');
       });
@@ -92,6 +102,15 @@ describe('GeoNodeDetailsComponent', () => {
       it('returns DB replication lag time duration', () => {
         expect(vm.dbReplicationLag).toBe('0m');
       });
+
+      it('returns `Unknown` when `dbReplicationLag` is null', () => {
+        const nodeDetailsLagNull = Object.assign({}, mockNodeDetails, {
+          dbReplicationLag: null,
+        });
+        const vmLagNull = createComponent(nodeDetailsLagNull);
+        expect(vmLagNull.dbReplicationLag).toBe('Unknown');
+        vmLagNull.$destroy();
+      });
     });
 
     describe('lastEventStatus', () => {
@@ -158,10 +177,17 @@ describe('GeoNodeDetailsComponent', () => {
 
     describe('syncSettings', () => {
       it('returns sync settings object', () => {
-        const syncSettings = vm.syncSettings();
+        const nodeDetailsUnknownSync = Object.assign({}, mockNodeDetails, {
+          syncStatusUnavailable: true,
+        });
+        const vmUnknownSync = createComponent(nodeDetailsUnknownSync);
+
+        const syncSettings = vmUnknownSync.syncSettings();
+        expect(syncSettings.syncStatusUnavailable).toBe(true);
         expect(syncSettings.namespaces).toBe(mockNodeDetails.namespaces);
         expect(syncSettings.lastEvent).toBe(mockNodeDetails.lastEvent);
         expect(syncSettings.cursorLastEvent).toBe(mockNodeDetails.cursorLastEvent);
+        vmUnknownSync.$destroy();
       });
     });
 

spec/javascripts/geo_nodes/components/geo_node_item_spec.js

@@ -184,7 +184,7 @@ describe('GeoNodeItemComponent', () => {
         spyOn(eventHub, '$emit');
 
         vm.handleMounted();
-        expect(eventHub.$emit).toHaveBeenCalledWith('pollNodeDetails', mockNodes[0].id);
+        expect(eventHub.$emit).toHaveBeenCalledWith('pollNodeDetails', vm.node);
       });
     });
   });

spec/javascripts/geo_nodes/components/geo_node_sync_settings_spec.js

@@ -5,12 +5,14 @@ import mountComponent from 'spec/helpers/vue_mount_component_helper';
 import { mockNodeDetails } from '../mock_data';
 
 const createComponent = (
+  syncStatusUnavailable = false,
   selectiveSyncType = mockNodeDetails.selectiveSyncType,
   lastEvent = mockNodeDetails.lastEvent,
   cursorLastEvent = mockNodeDetails.cursorLastEvent) => {
   const Component = Vue.extend(geoNodeSyncSettingsComponent);
 
   return mountComponent(Component, {
+    syncStatusUnavailable,
     selectiveSyncType,
     lastEvent,
     cursorLastEvent,
@@ -29,7 +31,7 @@ describe('GeoNodeSyncSettingsComponent', () => {
 
     describe('eventTimestampEmpty', () => {
       it('returns `true` if one of the event timestamp is empty', () => {
-        const vmEmptyTimestamp = createComponent(mockNodeDetails.namespaces, {
+        const vmEmptyTimestamp = createComponent(false, mockNodeDetails.namespaces, {
           id: 0,
           timeStamp: 0,
         }, {
@@ -87,4 +89,12 @@ describe('GeoNodeSyncSettingsComponent', () => {
       });
     });
   });
+
+  describe('template', () => {
+    it('renders `Unknown` when `syncStatusUnavailable` prop is true', () => {
+      const vmSyncUnavailable = createComponent(true);
+      expect(vmSyncUnavailable.$el.innerText.trim()).toBe('Unknown');
+      vmSyncUnavailable.$destroy();
+    });
+  });
 });

spec/javascripts/geo_nodes/mock_data.js

@@ -15,6 +15,12 @@ export const mockNodes = [
     files_max_capacity: 10,
     repos_max_capacity: 25,
     clone_protocol: 'http',
+    _links: {
+      self: 'http://127.0.0.1:3001/api/v4/geo_nodes/1',
+      repair: 'http://127.0.0.1:3001/api/v4/geo_nodes/1/repair',
+      status: 'http://127.0.0.1:3001/api/v4/geo_nodes/1/status',
+      web_edit: 'http://127.0.0.1:3001/admin/geo_nodes/1/edit',
+    },
   },
   {
     id: 2,
@@ -25,9 +31,28 @@ export const mockNodes = [
     files_max_capacity: 10,
     repos_max_capacity: 25,
     clone_protocol: 'http',
+    _links: {
+      self: 'http://127.0.0.1:3001/api/v4/geo_nodes/2',
+      repair: 'http://127.0.0.1:3001/api/v4/geo_nodes/2/repair',
+      status: 'http://127.0.0.1:3001/api/v4/geo_nodes/2/status',
+      web_edit: 'http://127.0.0.1:3001/admin/geo_nodes/2/edit',
+    },
   },
 ];
 
+export const mockNode = {
+  id: 1,
+  url: 'http://127.0.0.1:3001/',
+  primary: true,
+  current: true,
+  enabled: true,
+  nodeActionActive: false,
+  basePath: 'http://127.0.0.1:3001/api/v4/geo_nodes/1',
+  repairPath: 'http://127.0.0.1:3001/api/v4/geo_nodes/1/repair',
+  statusPath: 'http://127.0.0.1:3001/api/v4/geo_nodes/1/status?refresh=true',
+  editPath: 'http://127.0.0.1:3001/admin/geo_nodes/1/edit',
+};
+
 export const rawMockNodeDetails = {
   geo_node_id: 2,
   healthy: true,

spec/javascripts/geo_nodes/store/geo_nodes_store_spec.js

@@ -22,7 +22,7 @@ describe('GeoNodesStore', () => {
   describe('setNodes', () => {
     it('sets nodes list to state', () => {
       store.setNodes(mockNodes);
-      expect(store.getNodes()).toBe(mockNodes);
+      expect(store.getNodes().length).toBe(mockNodes.length);
     });
   });
 
@@ -33,6 +33,28 @@ describe('GeoNodesStore', () => {
     });
   });
 
+  describe('removeNode', () => {
+    it('removes node from store state', () => {
+      store.setNodes(mockNodes);
+      const nodeToBeRemoved = store.getNodes()[1];
+      store.removeNode(nodeToBeRemoved);
+      store.getNodes().forEach((node) => {
+        expect(node.id).not.toBe(nodeToBeRemoved);
+      });
+    });
+  });
+
+  describe('formatNode', () => {
+    it('returns formatted raw node object', () => {
+      const node = GeoNodesStore.formatNode(mockNodes[0]);
+      expect(node.id).toBe(mockNodes[0].id);
+      expect(node.url).toBe(mockNodes[0].url);
+      expect(node.basePath).toBe(mockNodes[0]._links.self);
+      expect(node.repairPath).toBe(mockNodes[0]._links.repair);
+      expect(node.nodeActionActive).toBe(false);
+    });
+  });
+
   describe('formatNodeDetails', () => {
     it('returns formatted raw node details object', () => {
       const nodeDetails = GeoNodesStore.formatNodeDetails(rawMockNodeDetails);