Commit 8ba7b602 authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch '10244-ux-improvements-for-group-runners' into 'master'

Improve UX For Group Runners

See merge request gitlab-org/gitlab-ce!18649
parents d0cbef7e d100396d
import initSettingsPanels from '~/settings_panels';
import AjaxVariableList from '~/ci_variable_list/ajax_variable_list';
document.addEventListener('DOMContentLoaded', () => {
// Initialize expandable settings panels
initSettingsPanels();
const variableListEl = document.querySelector('.js-ci-variable-list-section');
// eslint-disable-next-line no-new
new AjaxVariableList({
......
class Groups::RunnersController < Groups::ApplicationController
# Proper policies should be implemented per
# https://gitlab.com/gitlab-org/gitlab-ce/issues/45894
before_action :authorize_admin_pipeline!
before_action :runner, only: [:edit, :update, :destroy, :pause, :resume, :show]
def show
render 'shared/runners/show'
end
def edit
end
def update
if Ci::UpdateRunnerService.new(@runner).update(runner_params)
redirect_to group_runner_path(@group, @runner), notice: 'Runner was successfully updated.'
else
render 'edit'
end
end
def destroy
@runner.destroy
redirect_to group_settings_ci_cd_path(@group, anchor: 'runners-settings'), status: 302
end
def resume
if Ci::UpdateRunnerService.new(@runner).update(active: true)
redirect_to group_settings_ci_cd_path(@group, anchor: 'runners-settings'), notice: 'Runner was successfully updated.'
else
redirect_to group_settings_ci_cd_path(@group, anchor: 'runners-settings'), alert: 'Runner was not updated.'
end
end
def pause
if Ci::UpdateRunnerService.new(@runner).update(active: false)
redirect_to group_settings_ci_cd_path(@group, anchor: 'runners-settings'), notice: 'Runner was successfully updated.'
else
redirect_to group_settings_ci_cd_path(@group, anchor: 'runners-settings'), alert: 'Runner was not updated.'
end
end
private
def runner
@runner ||= @group.runners.find(params[:id])
end
def authorize_admin_pipeline!
return render_404 unless can?(current_user, :admin_pipeline, group)
end
def runner_params
params.require(:runner).permit(Ci::Runner::FORM_EDITABLE)
end
end
......@@ -8,7 +8,7 @@ class Projects::RunnerProjectsController < Projects::ApplicationController
return head(403) unless can?(current_user, :assign_runner, @runner)
path = runners_path(project)
path = project_runners_path(project)
runner_project = @runner.assign_to(project, current_user)
if runner_project.persisted?
......@@ -22,6 +22,6 @@ class Projects::RunnerProjectsController < Projects::ApplicationController
runner_project = project.runner_projects.find(params[:id])
runner_project.destroy
redirect_to runners_path(project), status: 302
redirect_to project_runners_path(project), status: 302
end
end
class Projects::RunnersController < Projects::ApplicationController
before_action :authorize_admin_build!
before_action :set_runner, only: [:edit, :update, :destroy, :pause, :resume, :show]
before_action :runner, only: [:edit, :update, :destroy, :pause, :resume, :show]
layout 'project_settings'
......@@ -13,7 +13,7 @@ class Projects::RunnersController < Projects::ApplicationController
def update
if Ci::UpdateRunnerService.new(@runner).update(runner_params)
redirect_to runner_path(@runner), notice: 'Runner was successfully updated.'
redirect_to project_runner_path(@project, @runner), notice: 'Runner was successfully updated.'
else
render 'edit'
end
......@@ -24,26 +24,27 @@ class Projects::RunnersController < Projects::ApplicationController
@runner.destroy
end
redirect_to runners_path(@project), status: 302
redirect_to project_runners_path(@project), status: 302
end
def resume
if Ci::UpdateRunnerService.new(@runner).update(active: true)
redirect_to runners_path(@project), notice: 'Runner was successfully updated.'
redirect_to project_runners_path(@project), notice: 'Runner was successfully updated.'
else
redirect_to runners_path(@project), alert: 'Runner was not updated.'
redirect_to project_runners_path(@project), alert: 'Runner was not updated.'
end
end
def pause
if Ci::UpdateRunnerService.new(@runner).update(active: false)
redirect_to runners_path(@project), notice: 'Runner was successfully updated.'
redirect_to project_runners_path(@project), notice: 'Runner was successfully updated.'
else
redirect_to runners_path(@project), alert: 'Runner was not updated.'
redirect_to project_runners_path(@project), alert: 'Runner was not updated.'
end
end
def show
render 'shared/runners/show'
end
def toggle_shared_runners
......@@ -60,7 +61,7 @@ class Projects::RunnersController < Projects::ApplicationController
protected
def set_runner
def runner
@runner ||= project.runners.find(params[:id])
end
......
......@@ -19,14 +19,6 @@ module GitlabRoutingHelper
project_commits_path(project, ref_name, *args)
end
def runners_path(project, *args)
project_runners_path(project, *args)
end
def runner_path(runner, *args)
project_runner_path(@project, runner, *args)
end
def environment_path(environment, *args)
project_environment_path(environment.project, environment, *args)
end
......
......@@ -29,7 +29,7 @@
%hr
.append-bottom-20
= render '/projects/runners/form', runner: @runner, runner_form_url: admin_runner_path(@runner)
= render 'shared/runners/form', runner: @runner, runner_form_url: admin_runner_path(@runner)
.row
.col-md-6
......
- link = link_to 'Runners API', help_page_path('api/runners.md')
%h3
= _('Group Runners')
.bs-callout.bs-callout-warning
= _('GitLab Group Runners can execute code for all the projects in this group.')
= _('They can be managed using the %{link}.').html_safe % { link: link }
-# Proper policies should be implemented per
-# https://gitlab.com/gitlab-org/gitlab-ce/issues/45894
- if can?(current_user, :admin_pipeline, @group)
= render partial: 'ci/runner/how_to_setup_runner',
locals: { registration_token: @group.runners_token, type: 'group' }
- if @group.runners.empty?
%h4.underlined-title
= _('This group does not provide any group Runners yet.')
- else
%h4.underlined-title
= _('Available group Runners : %{runners}.').html_safe % { runners: @group.runners.count }
%ul.bordered-list
= render partial: 'groups/runners/runner', collection: @group.runners, as: :runner
= render 'shared/runners/runner_description'
%hr
%p.lead
= _('To start serving your jobs you can add Runners to your group')
.row
.col-sm-6
= render 'groups/runners/group_runners'
%li.runner{ id: dom_id(runner) }
%h4
= runner_status_icon(runner)
= link_to runner.short_sha, group_runner_path(@group, runner), class: 'commit-sha'
%small.edit-runner
= link_to edit_group_runner_path(@group, runner) do
= icon('edit')
.pull-right
- if runner.active?
= link_to _('Pause'), pause_group_runner_path(@group, runner), method: :post, class: 'btn btn-sm btn-danger', data: { confirm: _("Are you sure?") }
- else
= link_to _('Resume'), resume_group_runner_path(@group, runner), method: :post, class: 'btn btn-success btn-sm'
= link_to _('Remove Runner'), group_runner_path(@group, runner), data: { confirm: _("Are you sure?") }, method: :delete, class: 'btn btn-danger btn-sm'
.pull-right
%small.light
\##{runner.id}
- if runner.description.present?
%p.runner-description
= runner.description
- if runner.tag_list.present?
%p
- runner.tag_list.sort.each do |tag|
%span.label.label-primary
= tag
- page_title "Edit", "#{@runner.description} ##{@runner.id}", "Runners"
%h4 Runner ##{@runner.id}
%hr
= render 'shared/runners/form', runner: @runner, runner_form_url: group_runner_path(@group, @runner)
- breadcrumb_title "CI / CD Settings"
- page_title "CI / CD"
%h4
- expanded = Rails.env.test?
%section.settings#secret-variables.no-animate{ class: ('expanded' if expanded) }
.settings-header
%h4
= _('Secret variables')
= link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'secret-variables'), target: '_blank', rel: 'noopener noreferrer'
%p
%button.btn.btn-default.js-settings-toggle{ type: "button" }
= expanded ? _('Collapse') : _('Expand')
%p.append-bottom-0
= render "ci/variables/content"
.settings-content
= render 'ci/variables/index', save_endpoint: group_variables_path
= render 'ci/variables/index', save_endpoint: group_variables_path
%section.settings#runners-settings.no-animate{ class: ('expanded' if expanded) }
.settings-header
%h4
= _('Runners settings')
%button.btn.btn-default.js-settings-toggle{ type: "button" }
= expanded ? _('Collapse') : _('Expand')
%p
= _('Register and see your runners for this group.')
.settings-content
= render 'groups/runners/index'
%h3 Group Runners
- link = link_to 'Runners API', help_page_path('api/runners.md')
%h3
= _('Group Runners')
.bs-callout.bs-callout-warning
GitLab Group Runners can execute code for all the projects in this group.
They can be managed using the #{link_to 'Runners API', help_page_path('api/runners.md')}.
= _('GitLab Group Runners can execute code for all the projects in this group.')
= _('They can be managed using the %{link}.').html_safe % { link: link }
- if @project.group
%hr
- if @project.group_runners_enabled?
= link_to toggle_group_runners_project_runners_path(@project), class: 'btn btn-warning', method: :post do
Disable group Runners
= link_to toggle_group_runners_project_runners_path(@project), class: 'btn btn-close', method: :post do
= _('Disable group Runners')
- else
= link_to toggle_group_runners_project_runners_path(@project), class: 'btn btn-success', method: :post do
Enable group Runners
&nbsp; for this project
= link_to toggle_group_runners_project_runners_path(@project), class: 'btn btn-success btn-inverted', method: :post do
= _('Enable group Runners')
&nbsp;
= _('for this project')
- if !@project.group
This project does not belong to a group and can therefore not make use of group Runners.
= _('This project does not belong to a group and can therefore not make use of group Runners.')
- elsif @group_runners.empty?
This group does not provide any group Runners yet.
= _('This group does not provide any group Runners yet.')
- if can?(current_user, :admin_pipeline, @project.group)
= render partial: 'ci/runner/how_to_setup_runner',
locals: { registration_token: @project.group.runners_token, type: 'group' }
- group_link = link_to 'Group CI/CD settings', group_settings_ci_cd_path(@project.group)
= _('Group masters can register group runners in the %{link}').html_safe % { link: group_link }
- else
Ask your group master to setup a group Runner.
= _('Ask your group master to setup a group Runner.')
- else
%h4.underlined-title Available group Runners : #{@group_runners.count}
%h4.underlined-title
= _('Available group Runners : %{runners}').html_safe % { runners: @group_runners.count }
%ul.bordered-list
= render partial: 'projects/runners/runner', collection: @group_runners, as: :runner
.light.prepend-top-default
%p
A 'Runner' is a process which runs a job.
You can setup as many Runners as you need.
%br
Runners can be placed on separate users, servers, and even on your local machine.
%p Each Runner can be in one of the following states:
%div
%ul
%li
%span.label.label-success active
\- Runner is active and can process any new jobs
%li
%span.label.label-danger paused
\- Runner is paused and will not receive any new jobs
= render 'shared/runners/runner_description'
%hr
......@@ -23,7 +8,4 @@
= render 'projects/runners/specific_runners'
.col-sm-6
= render 'projects/runners/shared_runners'
.row
.col-sm-6
.col-sm-6
= render 'projects/runners/group_runners'
......@@ -3,10 +3,10 @@
= runner_status_icon(runner)
- if @project_runners.include?(runner)
= link_to runner.short_sha, runner_path(runner), class: 'commit-sha'
= link_to runner.short_sha, project_runner_path(@project, runner), class: 'commit-sha'
- if runner.locked?
= icon('lock', class: 'has-tooltip', title: 'Locked to current projects')
= icon('lock', class: 'has-tooltip', title: _('Locked to current projects'))
%small.edit-runner
= link_to edit_project_runner_path(@project, runner) do
......@@ -18,18 +18,18 @@
.pull-right
- if @project_runners.include?(runner)
- if runner.active?
= link_to 'Pause', pause_project_runner_path(@project, runner), method: :post, class: 'btn btn-sm btn-danger', data: { confirm: "Are you sure?" }
= link_to _('Pause'), pause_project_runner_path(@project, runner), method: :post, class: 'btn btn-sm btn-danger', data: { confirm: _("Are you sure?") }
- else
= link_to 'Resume', resume_project_runner_path(@project, runner), method: :post, class: 'btn btn-success btn-sm'
= link_to _('Resume'), resume_project_runner_path(@project, runner), method: :post, class: 'btn btn-success btn-sm'
- if runner.belongs_to_one_project?
= link_to 'Remove Runner', runner_path(runner), data: { confirm: "Are you sure?" }, method: :delete, class: 'btn btn-danger btn-sm'
= link_to _('Remove Runner'), project_runner_path(@project, runner), data: { confirm: _("Are you sure?") }, method: :delete, class: 'btn btn-danger btn-sm'
- else
- runner_project = @project.runner_projects.find_by(runner_id: runner)
= link_to 'Disable for this project', project_runner_project_path(@project, runner_project), data: { confirm: "Are you sure?" }, method: :delete, class: 'btn btn-danger btn-sm'
= link_to _('Disable for this project'), project_runner_project_path(@project, runner_project), data: { confirm: _("Are you sure?") }, method: :delete, class: 'btn btn-danger btn-sm'
- elsif !(runner.is_shared? || runner.group_type?) # We can simplify this to `runner.project_type?` when migrating #runner_type is complete
= form_for [@project.namespace.becomes(Namespace), @project, @project.runner_projects.new] do |f|
= f.hidden_field :runner_id, value: runner.id
= f.submit 'Enable for this project', class: 'btn btn-sm'
= f.submit _('Enable for this project'), class: 'btn btn-sm'
.pull-right
%small.light
\##{runner.id}
......
......@@ -17,8 +17,8 @@
&nbsp; for this project
- if @shared_runners_count.zero?
This GitLab server does not provide any shared Runners yet.
Please use the specific Runners or ask your administrator to create one.
This GitLab instance does not provide any shared Runners yet. Instance
administrators can register shared Runners in the admin area.
- else
%h4.underlined-title Available shared Runners : #{@shared_runners_count}
%ul.bordered-list.available-shared-runners
......
......@@ -3,4 +3,4 @@
%h4 Runner ##{@runner.id}
%hr
= render 'form', runner: @runner, runner_form_url: runner_path(@runner)
= render 'shared/runners/form', runner: @runner, runner_form_url: project_runner_path(@project, @runner)
......@@ -18,6 +18,7 @@
.checkbox
= f.check_box :run_untagged
%span.light Indicates whether this runner can pick jobs without tags
- unless runner.group_type?
.form-group
= label :locked, 'Lock to current projects', class: 'control-label'
.col-sm-10
......
.light.prepend-top-default
%p
= _("A 'Runner' is a process which runs a job. You can setup as many Runners as you need.")
%br
= _('Runners can be placed on separate users, servers, and even on your local machine.')
%p
= _('Each Runner can be in one of the following states:')
%div
%ul
%li
%span.label.label-success active
= _('- Runner is active and can process any new jobs')
%li
%span.label.label-danger paused
= _('- Runner is paused and will not receive any new jobs')
......@@ -6,6 +6,9 @@
- if @runner.shared?
%span.runner-state.runner-state-shared
Shared
- elsif @runner.group_type?
%span.runner-state.runner-state-shared
Group
- else
%span.runner-state.runner-state-specific
Specific
......@@ -25,6 +28,7 @@
%tr
%td Can run untagged jobs
%td= @runner.run_untagged? ? 'Yes' : 'No'
- unless @runner.group_type?
%tr
%td Locked to this project
%td= @runner.locked? ? 'Yes' : 'No'
......
......@@ -58,6 +58,13 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
# On CE only index and show actions are needed
resources :boards, only: [:index, :show]
resources :runners, only: [:index, :edit, :update, :destroy, :show] do
member do
post :resume
post :pause
end
end
end
scope(path: '*id',
......
require 'spec_helper'
describe Groups::RunnersController do
let(:user) { create(:user) }
let(:group) { create(:group) }
let(:runner) { create(:ci_runner) }
let(:params) do
{
group_id: group,
id: runner
}
end
before do
sign_in(user)
group.add_master(user)
group.runners << runner
end
describe '#update' do
it 'updates the runner and ticks the queue' do
new_desc = runner.description.swapcase
expect do
post :update, params.merge(runner: { description: new_desc } )
end.to change { runner.ensure_runner_queue_value }
runner.reload
expect(response).to have_gitlab_http_status(302)
expect(runner.description).to eq(new_desc)
end
end
describe '#destroy' do
it 'destroys the runner' do
delete :destroy, params
expect(response).to have_gitlab_http_status(302)
expect(Ci::Runner.find_by(id: runner.id)).to be_nil
end
end
describe '#resume' do
it 'marks the runner as active and ticks the queue' do
runner.update(active: false)
expect do
post :resume, params
end.to change { runner.ensure_runner_queue_value }
runner.reload
expect(response).to have_gitlab_http_status(302)
expect(runner.active).to eq(true)
end
end
describe '#pause' do
it 'marks the runner as inactive and ticks the queue' do
runner.update(active: true)
expect do
post :pause, params
end.to change { runner.ensure_runner_queue_value }
runner.reload
expect(response).to have_gitlab_http_status(302)
expect(runner.active).to eq(false)
end
end
end
......@@ -15,7 +15,7 @@ feature 'Runners' do
end
scenario 'user can see a button to install runners on kubernetes clusters' do
visit runners_path(project)
visit project_runners_path(project)
expect(page).to have_link('Install Runner on Kubernetes', href: project_clusters_path(project))
end
......@@ -36,7 +36,7 @@ feature 'Runners' do
end
scenario 'user sees the specific runner' do
visit runners_path(project)
visit project_runners_path(project)
within '.activated-specific-runners' do
expect(page).to have_content(specific_runner.display_name)
......@@ -48,7 +48,7 @@ feature 'Runners' do
end
scenario 'user can pause and resume the specific runner' do
visit runners_path(project)
visit project_runners_path(project)
within '.activated-specific-runners' do
expect(page).to have_content('Pause')
......@@ -68,7 +68,7 @@ feature 'Runners' do
end
scenario 'user removes an activated specific runner if this is last project for that runners' do
visit runners_path(project)
visit project_runners_path(project)
within '.activated-specific-runners' do
click_on 'Remove Runner'
......@@ -78,7 +78,7 @@ feature 'Runners' do
end
scenario 'user edits the runner to be protected' do
visit runners_path(project)
visit project_runners_path(project)
within '.activated-specific-runners' do
first('.edit-runner > a').click
......@@ -98,7 +98,7 @@ feature 'Runners' do
end
scenario 'user edits runner not to run untagged jobs' do
visit runners_path(project)
visit project_runners_path(project)
within '.activated-specific-runners' do
first('.edit-runner > a').click
......@@ -117,7 +117,7 @@ feature 'Runners' do
given!(:shared_runner) { create(:ci_runner, :shared) }
scenario 'user sees CI/CD setting page' do
visit runners_path(project)
visit project_runners_path(project)
expect(page.find('.available-shared-runners')).to have_content(shared_runner.display_name)
end
......@@ -134,7 +134,7 @@ feature 'Runners' do
end
scenario 'user enables and disables a specific runner' do
visit runners_path(project)
visit project_runners_path(project)
within '.available-specific-runners' do
click_on 'Enable for this project'
......@@ -159,7 +159,7 @@ feature 'Runners' do
end
scenario 'user sees shared runners description' do
visit runners_path(project)
visit project_runners_path(project)
expect(page.find('.shared-runners-description')).to have_content(shared_runners_html)
end
......@@ -174,7 +174,7 @@ feature 'Runners' do
end
scenario 'user enables shared runners' do
visit runners_path(project)
visit project_runners_path(project)
click_on 'Enable shared Runners'
......@@ -182,7 +182,7 @@ feature 'Runners' do
end
end
context 'group runners' do
context 'group runners in project settings' do
background do
project.add_master(user)
end
......@@ -198,12 +198,12 @@ feature 'Runners' do
given(:project) { create :project, group: group }
scenario 'group runners are not available' do
visit runners_path(project)
visit project_runners_path(project)
expect(page).to have_content 'This group does not provide any group Runners yet.'
expect(page).to have_content 'This group does not provide any group Runners yet'
expect(page).to have_content 'Setup a group Runner manually'
expect(page).not_to have_content 'Ask your group master to setup a group Runner.'
expect(page).to have_content 'Group masters can register group runners in the Group CI/CD settings'
expect(page).not_to have_content 'Ask your group master to setup a group Runner'
end
end
end
......@@ -213,7 +213,7 @@ feature 'Runners' do
given(:project) { create :project }
scenario 'group runners are not available' do
visit runners_path(project)
visit project_runners_path(project)
expect(page).to have_content 'This project does not belong to a group and can therefore not make use of group Runners.'
end
......@@ -224,11 +224,11 @@ feature 'Runners' do
given(:project) { create :project, group: group }
scenario 'group runners are not available' do
visit runners_path(project)
visit project_runners_path(project)
expect(page).to have_content 'This group does not provide any group Runners yet.'
expect(page).not_to have_content 'Setup a group Runner manually'
expect(page).not_to have_content 'Group masters can register group runners in the Group CI/CD settings'
expect(page).to have_content 'Ask your group master to setup a group Runner.'
end
end
......@@ -239,14 +239,14 @@ feature 'Runners' do
given!(:ci_runner) { create :ci_runner, groups: [group], description: 'group-runner' }
scenario 'group runners are available' do
visit runners_path(project)
visit project_runners_path(project)
expect(page).to have_content 'Available group Runners : 1'
expect(page).to have_content 'group-runner'
end
scenario 'group runners may be disabled for a project' do
visit runners_path(project)
visit project_runners_path(project)
click_on 'Disable group Runners'
......@@ -261,4 +261,98 @@ feature 'Runners' do
end
end
end
context 'group runners in group settings' do
given(:group) { create :group }
background do
group.add_master(user)
end
context 'group with no runners' do
scenario 'there are no runners displayed' do
visit group_settings_ci_cd_path(group)
expect(page).to have_content 'This group does not provide any group Runners yet'
end
end
context 'group with a runner' do
let!(:runner) { create :ci_runner, groups: [group], description: 'group-runner' }
scenario 'the runner is visible' do
visit group_settings_ci_cd_path(group)
expect(page).not_to have_content 'This group does not provide any group Runners yet'
expect(page).to have_content 'Available group Runners : 1'
expect(page).to have_content 'group-runner'
end
scenario 'user can pause and resume the group runner' do
visit group_settings_ci_cd_path(group)
expect(page).to have_content('Pause')
expect(page).not_to have_content('Resume')
click_on 'Pause'
expect(page).not_to have_content('Pause')
expect(page).to have_content('Resume')
click_on 'Resume'
expect(page).to have_content('Pause')
expect(page).not_to have_content('Resume')
end
scenario 'user can view runner details' do
visit group_settings_ci_cd_path(group)
expect(page).to have_content(runner.display_name)
click_on runner.short_sha
expect(page).to have_content(runner.platform)
end
scenario 'user can remove a group runner' do
visit group_settings_ci_cd_path(group)
click_on 'Remove Runner'
expect(page).not_to have_content(runner.display_name)
end
scenario 'user edits the runner to be protected' do
visit group_settings_ci_cd_path(group)
first('.edit-runner > a').click
expect(page.find_field('runner[access_level]')).not_to be_checked
check 'runner_access_level'
click_button 'Save changes'
expect(page).to have_content 'Protected Yes'
end
context 'when a runner has a tag' do
background do
runner.update(tag_list: ['tag'])
end
scenario 'user edits runner not to run untagged jobs' do
visit group_settings_ci_cd_path(group)
first('.edit-runner > a').click
expect(page.find_field('runner[run_untagged]')).to be_checked
uncheck 'runner_run_untagged'
click_button 'Save changes'
expect(page).to have_content 'Can run untagged jobs No'
end
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment