Merge branch '383139-add-runner-registration-feedback' into 'master'

Add runner registration feedback

See merge request !113557



Merged-by: Phil Hughes <me@iamphill.com>
Approved-by: Scott de Jonge <sdejonge@gitlab.com>
Approved-by: Phil Hughes <me@iamphill.com>
Approved-by: Matthew Nearents <mnearents@gitlab.com>
Reviewed-by: Pedro Pombeiro <noreply@pedro.pombei.ro>
Reviewed-by: Miguel Rincon <mrincon@gitlab.com>
Reviewed-by: Scott de Jonge <sdejonge@gitlab.com>
Co-authored-by: Miguel Rincon <mrincon@gitlab.com>

app/assets/javascripts/ci/runner/admin_register_runner/admin_register_runner_app.vue

@@ -5,7 +5,13 @@ import { getParameterByName, updateHistory, mergeUrlParams } from '~/lib/utils/u
 import { convertToGraphQLId } from '~/graphql_shared/utils';
 import { TYPENAME_CI_RUNNER } from '~/graphql_shared/constants';
 import runnerForRegistrationQuery from '../graphql/register/runner_for_registration.query.graphql';
-import { I18N_FETCH_ERROR, PARAM_KEY_PLATFORM, DEFAULT_PLATFORM } from '../constants';
+import {
+  I18N_FETCH_ERROR,
+  PARAM_KEY_PLATFORM,
+  DEFAULT_PLATFORM,
+  STATUS_ONLINE,
+  RUNNER_REGISTRATION_POLLING_INTERVAL_MS,
+} from '../constants';
 import RegistrationInstructions from '../components/registration/registration_instructions.vue';
 import PlatformsDrawer from '../components/registration/platforms_drawer.vue';
 import { captureException } from '../sentry_utils';
@@ -46,6 +52,13 @@ export default {
         createAlert({ message: I18N_FETCH_ERROR });
         captureException({ error, component: this.$options.name });
       },
+      pollInterval() {
+        if (this.runner?.status === STATUS_ONLINE) {
+          // stop polling
+          return 0;
+        }
+        return RUNNER_REGISTRATION_POLLING_INTERVAL_MS;
+      },
     },
   },
   watch: {
@@ -72,7 +85,9 @@ export default {
       :platform="platform"
       :loading="$apollo.queries.runner.loading"
       @toggleDrawer="onToggleDrawer"
-    />
+    >
+      <template #runner-list-name>{{ s__('Runners|Admin area › Runners') }}</template>
+    </registration-instructions>
 
     <platforms-drawer
       :platform="platform"

app/assets/javascripts/ci/runner/components/registration/platforms_drawer.vue

 <script>
-import { GlDrawer, GlFormGroup, GlFormSelect, GlLink, GlSprintf } from '@gitlab/ui';
+import { GlDrawer, GlFormGroup, GlFormSelect, GlIcon, GlLink, GlSprintf } from '@gitlab/ui';
 import { getContentWrapperHeight } from '~/lib/utils/dom_utils';
 import { DRAWER_Z_INDEX } from '~/lib/utils/constants';
 
@@ -19,6 +19,7 @@ export default {
     GlDrawer,
     GlFormGroup,
     GlFormSelect,
+    GlIcon,
     GlLink,
     GlSprintf,
     CliCommand,
@@ -122,7 +123,9 @@ export default {
           "
         >
           <template #link="{ content }">
-            <gl-link :href="$options.INSTALL_HELP_URL">{{ content }}</gl-link>
+            <gl-link :href="$options.INSTALL_HELP_URL">
+              {{ content }} <gl-icon name="external-link" />
+            </gl-link>
           </template>
         </gl-sprintf>
       </p>

app/assets/javascripts/ci/runner/components/registration/registration_instructions.vue

@@ -3,7 +3,12 @@ import { GlIcon, GlLink, GlSprintf, GlSkeletonLoader } from '@gitlab/ui';
 import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
 import { s__, sprintf } from '~/locale';
 
-import { EXECUTORS_HELP_URL, SERVICE_COMMANDS_HELP_URL } from '../../constants';
+import {
+  EXECUTORS_HELP_URL,
+  SERVICE_COMMANDS_HELP_URL,
+  STATUS_ONLINE,
+  I18N_REGISTRATION_SUCCESS,
+} from '../../constants';
 import CliCommand from './cli_command.vue';
 import { commandPrompt, registerCommand, runCommand } from './utils';
 
@@ -51,6 +56,9 @@ export default {
     token() {
       return this.runner?.ephemeralAuthenticationToken;
     },
+    status() {
+      return this.runner?.status;
+    },
     commandPrompt() {
       return commandPrompt({ platform: this.platform });
     },
@@ -72,6 +80,8 @@ export default {
   },
   EXECUTORS_HELP_URL,
   SERVICE_COMMANDS_HELP_URL,
+  STATUS_ONLINE,
+  I18N_REGISTRATION_SUCCESS,
 };
 </script>
 <template>
@@ -144,13 +154,15 @@ export default {
           "
         >
           <template #link="{ content }">
-            <gl-link :href="$options.EXECUTORS_HELP_URL">{{ content }}</gl-link>
+            <gl-link :href="$options.EXECUTORS_HELP_URL" target="_blank">
+              {{ content }} <gl-icon name="external-link" />
+            </gl-link>
           </template>
         </gl-sprintf>
       </p>
     </section>
     <section>
-      <h2 class="gl-font-size-h2">{{ s__('Runners|Optional. Step 3') }}</h2>
+      <h2 class="gl-font-size-h2">{{ s__('Runners|Step 3 (optional)') }}</h2>
       <p>{{ s__('Runners|Manually verify that the runner is available to pick up jobs.') }}</p>
       <cli-command :prompt="commandPrompt" :command="runCommand" />
       <p>
@@ -162,7 +174,20 @@ export default {
           "
         >
           <template #link="{ content }">
-            <gl-link :href="$options.SERVICE_COMMANDS_HELP_URL">{{ content }}</gl-link>
+            <gl-link :href="$options.SERVICE_COMMANDS_HELP_URL" target="_blank">
+              {{ content }} <gl-icon name="external-link" />
+            </gl-link>
+          </template>
+        </gl-sprintf>
+      </p>
+    </section>
+    <section v-if="status == $options.STATUS_ONLINE">
+      <h2 class="gl-font-size-h2">🎉 {{ $options.I18N_REGISTRATION_SUCCESS }}</h2>
+
+      <p class="gl-pl-6">
+        <gl-sprintf :message="s__('Runners|To view the runner, go to %{runnerListName}.')">
+          <template #runnerListName>
+            <span class="gl-font-weight-bold"><slot name="runner-list-name"></slot></span>
           </template>
         </gl-sprintf>
       </p>

app/assets/javascripts/ci/runner/constants.js

@@ -108,6 +108,12 @@ export const I18N_CLEAR_FILTER_PROJECTS = __('Clear');
 export const I18N_NO_JOBS_FOUND = s__('Runners|This runner has not run any jobs.');
 export const I18N_NO_PROJECTS_FOUND = __('No projects found');
 
+// Runner registration
+
+export const I18N_REGISTRATION_SUCCESS = s__("Runners|You've created a new runner!");
+
+export const RUNNER_REGISTRATION_POLLING_INTERVAL_MS = 2000;
+
 // Styles
 
 export const RUNNER_TAG_BADGE_VARIANT = 'info';

app/assets/javascripts/ci/runner/graphql/register/runner_for_registration.query.graphql

@@ -3,5 +3,6 @@ query getRunnerForRegistration($id: CiRunnerID!) {
     id
     description
     ephemeralAuthenticationToken
+    status
   }
 }

locale/gitlab.pot

@@ -37260,6 +37260,9 @@ msgstr ""
 msgid "Runners|Add tags for the types of jobs the runner processes to ensure that the runner only runs jobs that you intend it to. %{helpLinkStart}Learn more.%{helpLinkEnd}"
 msgstr ""
+msgid "Runners|Admin area › Runners"
+msgstr ""
+
 msgid "Runners|Administrator"
 msgstr ""
@@ -37553,9 +37556,6 @@ msgstr ""
 msgid "Runners|Operating systems"
 msgstr ""
-msgid "Runners|Optional. Step 3"
-msgstr ""
-
 msgid "Runners|Owner"
 msgstr ""
@@ -37780,6 +37780,9 @@ msgstr ""
 msgid "Runners|Step 2"
 msgstr ""
+msgid "Runners|Step 3 (optional)"
+msgstr ""
+
 msgid "Runners|Stop the runner from accepting new jobs."
 msgstr ""
@@ -37833,6 +37836,9 @@ msgstr ""
 msgid "Runners|To register them, go to the %{link_start}group's Runners page%{link_end}."
 msgstr ""
+msgid "Runners|To view the runner, go to %{runnerListName}."
+msgstr ""
+
 msgid "Runners|Token expiry"
 msgstr ""
@@ -37902,6 +37908,9 @@ msgstr ""
 msgid "Runners|You have used %{quotaUsed} out of %{quotaLimit} of your shared Runners pipeline minutes."
 msgstr ""
+msgid "Runners|You've created a new runner!"
+msgstr ""
+
 msgid "Runners|active"
 msgstr ""

spec/frontend/ci/runner/admin_register_runner/admin_register_runner_app_spec.js

@@ -11,7 +11,13 @@ import { TEST_HOST } from 'helpers/test_constants';
 
 import { updateHistory } from '~/lib/utils/url_utility';
 import runnerForRegistrationQuery from '~/ci/runner/graphql/register/runner_for_registration.query.graphql';
-import { PARAM_KEY_PLATFORM, DEFAULT_PLATFORM, WINDOWS_PLATFORM } from '~/ci/runner/constants';
+import {
+  PARAM_KEY_PLATFORM,
+  DEFAULT_PLATFORM,
+  WINDOWS_PLATFORM,
+  RUNNER_REGISTRATION_POLLING_INTERVAL_MS,
+  STATUS_ONLINE,
+} from '~/ci/runner/constants';
 import AdminRegisterRunnerApp from '~/ci/runner/admin_register_runner/admin_register_runner_app.vue';
 import RegistrationInstructions from '~/ci/runner/components/registration/registration_instructions.vue';
 import PlatformsDrawer from '~/ci/runner/components/registration/platforms_drawer.vue';
@@ -36,6 +42,11 @@ describe('AdminRegisterRunnerApp', () => {
   const findPlatformsDrawer = () => wrapper.findComponent(PlatformsDrawer);
   const findBtn = () => wrapper.findComponent(GlButton);
 
+  const waitForPolling = async () => {
+    jest.advanceTimersByTime(RUNNER_REGISTRATION_POLLING_INTERVAL_MS);
+    await waitForPromises();
+  };
+
   const createComponent = () => {
     wrapper = shallowMountExtended(AdminRegisterRunnerApp, {
       apolloProvider: createMockApollo([[runnerForRegistrationQuery, mockRunnerQuery]]),
@@ -83,6 +94,34 @@ describe('AdminRegisterRunnerApp', () => {
       expect(findBtn().attributes('href')).toEqual(mockRunnersPath);
       expect(findBtn().props('variant')).toEqual('confirm');
     });
+
+    describe('polling for changes in status', () => {
+      it('fetches data', () => {
+        expect(mockRunnerQuery).toHaveBeenCalledTimes(1);
+      });
+
+      it('polls', async () => {
+        await waitForPolling();
+        expect(mockRunnerQuery).toHaveBeenCalledTimes(2);
+
+        await waitForPolling();
+        expect(mockRunnerQuery).toHaveBeenCalledTimes(3);
+      });
+
+      it('when runner is online, stops polling', async () => {
+        mockRunnerQuery.mockResolvedValue({
+          data: {
+            runner: { ...mockRunner, status: STATUS_ONLINE },
+          },
+        });
+
+        await waitForPolling();
+        expect(mockRunnerQuery).toHaveBeenCalledTimes(2);
+
+        await waitForPolling();
+        expect(mockRunnerQuery).toHaveBeenCalledTimes(2);
+      });
+    });
   });
 
   describe('When another platform has been selected', () => {
@@ -145,11 +184,13 @@ describe('AdminRegisterRunnerApp', () => {
   });
 
   describe('When runner is loading', () => {
-    beforeEach(() => {
-      createComponent();
+    beforeEach(async () => {
+      mockRunnerQuery = jest.fn().mockImplementation(() => new Promise());
     });
 
-    it('shows registration instructions', () => {
+    it('shows registration instructions with no runner', () => {
+      createComponent();
+
       expect(findRegistrationInstructions().props()).toEqual({
         loading: true,
         runner: null,

spec/frontend/ci/runner/components/registration/platforms_drawer_spec.js

 import { nextTick } from 'vue';
-import { GlDrawer } from '@gitlab/ui';
+import { GlDrawer, GlLink, GlIcon, GlSprintf } from '@gitlab/ui';
 import { s__ } from '~/locale';
 import { shallowMountExtended, mountExtended } from 'helpers/vue_test_utils_helper';
 
 import PlatformsDrawer from '~/ci/runner/components/registration/platforms_drawer.vue';
 import CliCommand from '~/ci/runner/components/registration/cli_command.vue';
-import { LINUX_PLATFORM, MACOS_PLATFORM, WINDOWS_PLATFORM } from '~/ci/runner/constants';
+import {
+  LINUX_PLATFORM,
+  MACOS_PLATFORM,
+  WINDOWS_PLATFORM,
+  INSTALL_HELP_URL,
+} from '~/ci/runner/constants';
 import { installScript, platformArchitectures } from '~/ci/runner/components/registration/utils';
 
 const MOCK_WRAPPER_HEIGHT = '99px';
@@ -25,6 +30,7 @@ describe('RegistrationInstructions', () => {
   const findArchitectureOptions = () =>
     wrapper.findByLabelText(s__('Runners|Architecture')).findAll('option');
   const findCliCommand = () => wrapper.findComponent(CliCommand);
+  const findLink = () => wrapper.findComponent(GlLink);
 
   const createComponent = ({ props = {}, mountFn = shallowMountExtended } = {}) => {
     wrapper = mountFn(PlatformsDrawer, {
@@ -32,6 +38,9 @@ describe('RegistrationInstructions', () => {
         open: true,
         ...props,
       },
+      stubs: {
+        GlSprintf,
+      },
     });
   };
 
@@ -89,4 +98,11 @@ describe('RegistrationInstructions', () => {
       installScript({ platform: MACOS_PLATFORM, architecture: MACOS_ARCHS[0] }),
     );
   });
+
+  it('shows external link for more information', () => {
+    createComponent();
+
+    expect(findLink().attributes('href')).toBe(INSTALL_HELP_URL);
+    expect(findLink().findComponent(GlIcon).props('name')).toBe('external-link');
+  });
 });

spec/frontend/ci/runner/components/registration/registration_instructions_spec.js

@@ -10,6 +10,9 @@ import {
   DEFAULT_PLATFORM,
   EXECUTORS_HELP_URL,
   SERVICE_COMMANDS_HELP_URL,
+  STATUS_NEVER_CONTACTED,
+  STATUS_ONLINE,
+  I18N_REGISTRATION_SUCCESS,
 } from '~/ci/runner/constants';
 import { runnerForRegistration } from '../../mock_data';
 
@@ -144,4 +147,37 @@ describe('RegistrationInstructions', () => {
       SERVICE_COMMANDS_HELP_URL,
     );
   });
+
+  describe('success state', () => {
+    describe('when the runner has not been registered', () => {
+      beforeEach(() => {
+        createComponent({
+          runner: {
+            ...mockRunner,
+            status: STATUS_NEVER_CONTACTED,
+          },
+        });
+      });
+
+      it('does not show success message', () => {
+        expect(wrapper.text()).not.toContain(I18N_REGISTRATION_SUCCESS);
+      });
+    });
+
+    describe('when the runner has been registered', () => {
+      beforeEach(() => {
+        createComponent({
+          runner: {
+            ...mockRunner,
+            status: STATUS_ONLINE,
+          },
+        });
+      });
+
+      it('shows success message', () => {
+        expect(wrapper.text()).toContain('🎉');
+        expect(wrapper.text()).toContain(I18N_REGISTRATION_SUCCESS);
+      });
+    });
+  });
 });