From 6d3eea7b46d4b363b39a59e1fa17264de33d14d1 Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Wed, 11 Oct 2017 14:15:28 +0200 Subject: if the branch/tag count is over a certain limit, don't execute the long running git query --- app/controllers/projects/commit_controller.rb | 4 ++-- app/models/repository.rb | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index a62f05db7db..893763862ba 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -56,8 +56,8 @@ class Projects::CommitController < Projects::ApplicationController end def branches - @branches = @project.repository.branch_names_contains(commit.id) - @tags = @project.repository.tag_names_contains(commit.id) + @branches = @project.repository.branch_names_contains(commit.id, 1000) + @tags = @project.repository.tag_names_contains(commit.id, 1000) render layout: false end diff --git a/app/models/repository.rb b/app/models/repository.rb index d725c65081d..3a083b76202 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -715,12 +715,12 @@ class Repository end end - def branch_names_contains(sha) - refs_contains_sha('branch', sha) + def branch_names_contains(sha, limit = nil) + limit && branch_count > limit ? [] : refs_contains_sha('branch', sha) end - def tag_names_contains(sha) - refs_contains_sha('tag', sha) + def tag_names_contains(sha, limit = nil) + limit && tag_count > limit ? [] : refs_contains_sha('tag', sha) end def local_branches -- cgit v1.2.1 From 528f9cde0588b0a6e70b1fa971a99eca439d0aa6 Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Thu, 12 Oct 2017 15:31:43 +0200 Subject: moved throttling into the controller. if we hit the throttling threshhold, a message is shown indicating we didn't perform the search --- app/controllers/projects/commit_controller.rb | 7 +++++-- app/helpers/commits_helper.rb | 26 +++++++++++++++++-------- app/models/repository.rb | 8 ++++---- app/views/projects/commit/branches.html.haml | 28 +++++++++++++++------------ 4 files changed, 43 insertions(+), 26 deletions(-) diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 893763862ba..5b8a8159123 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -56,8 +56,11 @@ class Projects::CommitController < Projects::ApplicationController end def branches - @branches = @project.repository.branch_names_contains(commit.id, 1000) - @tags = @project.repository.tag_names_contains(commit.id, 1000) + # branch_names_contains/tag_names_contains can take a long time when there are thousands of + # branches/tags - each `git branch --contains xxx` request can consume a cpu core. + # so only do the query when there are a manageable number of branches/tags + @branches = @project.repository.branch_count > 1000 ? [:limit_exceeded] : @project.repository.branch_names_contains(commit.id) + @tags = @project.repository.tag_count > 1000 ? [:limit_exceeded] : @project.repository.tag_names_contains(commit.id) render layout: false end diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index ef22cafc2e2..8aaf3318f90 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -60,23 +60,33 @@ module CommitsHelper branches.include?(project.default_branch) ? branches.delete(project.default_branch) : branches.pop end + # returns a link formatted as a commit branch link + def commit_branch_link(url, text) + link_to(url, class: 'label label-gray ref-name') do + icon('code-fork') + " #{text}" + end + end + # Returns the sorted alphabetically links to branches, separated by a comma def commit_branches_links(project, branches) branches.sort.map do |branch| - link_to(project_ref_path(project, branch), class: "label label-gray ref-name") do - icon('code-fork') + " #{branch}" - end - end.join(" ").html_safe + commit_branch_link(project_ref_path(project, branch), branch) + end.join(' ').html_safe + end + + # returns a link formatted as a commit tag link + def commit_tag_link(url, text) + link_to(url, class: 'label label-gray ref-name') do + icon('tag') + " #{text}" + end end # Returns the sorted links to tags, separated by a comma def commit_tags_links(project, tags) sorted = VersionSorter.rsort(tags) sorted.map do |tag| - link_to(project_ref_path(project, tag), class: "label label-gray ref-name") do - icon('tag') + " #{tag}" - end - end.join(" ").html_safe + commit_tag_link(project_ref_path(project, tag), tag) + end.join(' ').html_safe end def link_to_browse_code(project, commit) diff --git a/app/models/repository.rb b/app/models/repository.rb index 3a083b76202..d725c65081d 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -715,12 +715,12 @@ class Repository end end - def branch_names_contains(sha, limit = nil) - limit && branch_count > limit ? [] : refs_contains_sha('branch', sha) + def branch_names_contains(sha) + refs_contains_sha('branch', sha) end - def tag_names_contains(sha, limit = nil) - limit && tag_count > limit ? [] : refs_contains_sha('tag', sha) + def tag_names_contains(sha) + refs_contains_sha('tag', sha) end def local_branches diff --git a/app/views/projects/commit/branches.html.haml b/app/views/projects/commit/branches.html.haml index 911c9ddce06..398927fb50d 100644 --- a/app/views/projects/commit/branches.html.haml +++ b/app/views/projects/commit/branches.html.haml @@ -1,15 +1,19 @@ -- if @branches.any? || @tags.any? +- if @branches.any? - branch = commit_default_branch(@project, @branches) - = link_to(project_ref_path(@project, branch), class: "label label-gray ref-name") do - = icon('code-fork') - = branch + - if branch == :limit_exceeded + = commit_branch_link('#', _('Too many branches to search')) + - else + = commit_branch_link(project_ref_path(@project, branch), branch) - -# `commit_default_branch` deletes the default branch from `@branches`, - -# so only render this if we have more branches left - - if @branches.any? || @tags.any? - %span - = link_to "…", "#", class: "js-details-expand label label-gray" +-# `commit_default_branch` deletes the default branch from `@branches`, +-# so only render this if we have more branches or tags left +- if @branches.any? || @tags.any? + %span + = link_to "…", "#", class: "js-details-expand label label-gray" - %span.js-details-content.hide - = commit_branches_links(@project, @branches) if @branches.any? - = commit_tags_links(@project, @tags) if @tags.any? + %span.js-details-content.hide + = commit_branches_links(@project, @branches) if @branches.any? + - if @tags.first == :limit_exceeded + = commit_tag_link('#', _('Too many tags to search')) + - elsif @tags.any? + = commit_tags_links(@project, @tags) -- cgit v1.2.1 From cbdf372eb8e6d38c4f47a1c2f6bff76b4b2c659f Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Fri, 13 Oct 2017 19:11:11 +0200 Subject: implemented using an ivar, and added specs --- app/controllers/projects/commit_controller.rb | 9 ++++- app/views/projects/commit/branches.html.haml | 10 ++--- .../controllers/projects/commit_controller_spec.rb | 26 +++++++++++-- .../projects/commit/branches.html.haml_spec.rb | 44 ++++++++++++++++++++++ 4 files changed, 78 insertions(+), 11 deletions(-) create mode 100644 spec/views/projects/commit/branches.html.haml_spec.rb diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 5b8a8159123..494d412b532 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -16,6 +16,8 @@ class Projects::CommitController < Projects::ApplicationController before_action :define_note_vars, only: [:show, :diff_for_path] before_action :authorize_edit_tree!, only: [:revert, :cherry_pick] + BRANCH_SEARCH_LIMIT = 1000 + def show apply_diff_view_cookie! @@ -59,8 +61,11 @@ class Projects::CommitController < Projects::ApplicationController # branch_names_contains/tag_names_contains can take a long time when there are thousands of # branches/tags - each `git branch --contains xxx` request can consume a cpu core. # so only do the query when there are a manageable number of branches/tags - @branches = @project.repository.branch_count > 1000 ? [:limit_exceeded] : @project.repository.branch_names_contains(commit.id) - @tags = @project.repository.tag_count > 1000 ? [:limit_exceeded] : @project.repository.tag_names_contains(commit.id) + @branches_limit_exceeded = @project.repository.branch_count > BRANCH_SEARCH_LIMIT + @branches = @branches_limit_exceeded ? [] : @project.repository.branch_names_contains(commit.id) + + @tags_limit_exceeded = @project.repository.tag_count > BRANCH_SEARCH_LIMIT + @tags = @tags_limit_exceeded ? [] : @project.repository.tag_names_contains(commit.id) render layout: false end diff --git a/app/views/projects/commit/branches.html.haml b/app/views/projects/commit/branches.html.haml index 398927fb50d..933cb2f73ce 100644 --- a/app/views/projects/commit/branches.html.haml +++ b/app/views/projects/commit/branches.html.haml @@ -1,19 +1,19 @@ -- if @branches.any? - - branch = commit_default_branch(@project, @branches) - - if branch == :limit_exceeded +- if @branches.any? || @branches_limit_exceeded + - if @branches_limit_exceeded = commit_branch_link('#', _('Too many branches to search')) - else + - branch = commit_default_branch(@project, @branches) = commit_branch_link(project_ref_path(@project, branch), branch) -# `commit_default_branch` deletes the default branch from `@branches`, -# so only render this if we have more branches or tags left -- if @branches.any? || @tags.any? +- if @branches.any? || @tags.any? || @tags_limit_exceeded %span = link_to "…", "#", class: "js-details-expand label label-gray" %span.js-details-content.hide = commit_branches_links(@project, @branches) if @branches.any? - - if @tags.first == :limit_exceeded + - if @tags_limit_exceeded = commit_tag_link('#', _('Too many tags to search')) - elsif @tags.any? = commit_tags_links(@project, @tags) diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index df53863482d..f6ecde88b85 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -134,8 +134,8 @@ describe Projects::CommitController do end end - describe "GET branches" do - it "contains branch and tags information" do + describe 'GET branches' do + it 'contains branch and tags information' do commit = project.commit('5937ac0a7beb003549fc5fd26fc247adbce4a52e') get(:branches, @@ -143,8 +143,26 @@ describe Projects::CommitController do project_id: project, id: commit.id) - expect(assigns(:branches)).to include("master", "feature_conflict") - expect(assigns(:tags)).to include("v1.1.0") + expect(assigns(:branches)).to include('master', 'feature_conflict') + expect(assigns(:branches_limit_exceeded)).to be_falsey + expect(assigns(:tags)).to include('v1.1.0') + expect(assigns(:tags_limit_exceeded)).to be_falsey + end + + it 'returns :limit_exceeded when number of branches/tags reach a threshhold' do + commit = project.commit('5937ac0a7beb003549fc5fd26fc247adbce4a52e') + allow_any_instance_of(Repository).to receive(:branch_count).and_return(1001) + allow_any_instance_of(Repository).to receive(:tag_count).and_return(1001) + + get(:branches, + namespace_id: project.namespace, + project_id: project, + id: commit.id) + + expect(assigns(:branches)).to eq([]) + expect(assigns(:branches_limit_exceeded)).to be_truthy + expect(assigns(:tags)).to eq([]) + expect(assigns(:tags_limit_exceeded)).to be_truthy end end diff --git a/spec/views/projects/commit/branches.html.haml_spec.rb b/spec/views/projects/commit/branches.html.haml_spec.rb new file mode 100644 index 00000000000..4199d2fd962 --- /dev/null +++ b/spec/views/projects/commit/branches.html.haml_spec.rb @@ -0,0 +1,44 @@ +require 'spec_helper' + +describe 'projects/commit/branches.html.haml' do + let(:project) { create(:project, :repository) } + + before do + assign(:project, project) + end + + context 'branches and tags' do + before do + assign(:branches, ['master', 'test-branch']) + assign(:branches_limit_exceeded, false) + assign(:tags, ['tag1']) + assign(:tags_limit_exceeded, false) + + render + end + + it 'shows branch and tag links' do + expect(rendered).to have_selector('.js-details-expand') + expect(rendered).to have_link('master') + expect(rendered).to have_link('test-branch') + expect(rendered).to have_link('tag1') + end + end + + context 'throttled branches and tags' do + before do + assign(:branches, []) + assign(:branches_limit_exceeded, true) + assign(:tags, []) + assign(:tags_limit_exceeded, true) + + render + end + + it 'shows too many to search' do + expect(rendered).to have_selector('.js-details-expand') + expect(rendered).to have_link('Too many branches to search', href: '#') + expect(rendered).to have_link('Too many tags to search', href: '#') + end + end +end -- cgit v1.2.1 From a632440fa91c750bb4a75ee430c3a34c6d98f51c Mon Sep 17 00:00:00 2001 From: Hazel Date: Mon, 16 Oct 2017 15:13:21 +0800 Subject: Draft a guide of modals in UX guide --- doc/development/ux_guide/components.md | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/doc/development/ux_guide/components.md b/doc/development/ux_guide/components.md index fa31c496b30..20230a9bb2c 100644 --- a/doc/development/ux_guide/components.md +++ b/doc/development/ux_guide/components.md @@ -252,6 +252,35 @@ Skeleton loading is a way to convey to the user what kind of content is currentl Skeleton loading can replace any existing UI elements for the period in which they are loaded and should aim for maintaining a similar structure visually. +--- + +## Dialog modals +Dialog modals are only used for having a conversation and confirmation with the user. The user is not able to access the features on the main page until making an action. Using dialog modals sparingly as it is interruptive. + +### Usage + +* When the action is irreversible, we should provide the details and confirm with the user before they take an advanced action. +* When the action will affect privacy or authorization, we should provide the advanced information and confirm with the user. + +### Style +* Dialog modals contain the header title, content, and action bar. +* Having an affirmative action, a dismissive action, and an extra action in action bar. +* The order of actions in the action bar: Affirmative action → Extra action → Dismissive action (Right to left) +* Confirmations regarding labels should keep labeling styling. +* References to commits, branches, and tags should be **monospaced**. + +### Writing +* The header title is a question instead of a descriptive phrase. +* The content should not be ambiguous and unclear. It should provide specific information as a reference for the user to take an advanced action. + +### Placement +* Dialog modals should always be the center of the screen horizontally and have **72px** from the top of the screen. + +| Dialog with 2 actions | Dialog with 3 actions | Special confirmation | +| --------------------- | --------------------- | -------------------- | +| ![two-actions]() | ![three-actions]() | ![spcial-confirmation]() | + + --- ## Panels -- cgit v1.2.1 From aeeefe28fbc6f27b200c5c1b15c5f6b5d946f3e8 Mon Sep 17 00:00:00 2001 From: Hazel Date: Mon, 16 Oct 2017 15:54:48 +0800 Subject: Modified the copy --- doc/development/ux_guide/components.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/development/ux_guide/components.md b/doc/development/ux_guide/components.md index 20230a9bb2c..2bcee69201b 100644 --- a/doc/development/ux_guide/components.md +++ b/doc/development/ux_guide/components.md @@ -265,7 +265,7 @@ Dialog modals are only used for having a conversation and confirmation with the ### Style * Dialog modals contain the header title, content, and action bar. * Having an affirmative action, a dismissive action, and an extra action in action bar. -* The order of actions in the action bar: Affirmative action → Extra action → Dismissive action (Right to left) +* The order of actions in the action bar: (Right to left) Affirmative action → Extra action → Dismissive action * Confirmations regarding labels should keep labeling styling. * References to commits, branches, and tags should be **monospaced**. -- cgit v1.2.1 From 4a289e1168c239b03fd386b5c6ca1aa9963849ae Mon Sep 17 00:00:00 2001 From: Hazel Date: Tue, 17 Oct 2017 14:30:50 +0800 Subject: Updated the content and added the images --- doc/development/ux_guide/components.md | 23 +++++++++++---------- .../img/modals-general-confimation-dialog.png | Bin 0 -> 55006 bytes .../ux_guide/img/modals-layout-for-modals.png | Bin 0 -> 71731 bytes .../img/modals-special-confimation-dialog.png | Bin 0 -> 94366 bytes .../ux_guide/img/modals-three-buttons-special.png | Bin 0 -> 59942 bytes .../ux_guide/img/modals-three-buttons.png | Bin 0 -> 55863 bytes 6 files changed, 12 insertions(+), 11 deletions(-) create mode 100644 doc/development/ux_guide/img/modals-general-confimation-dialog.png create mode 100644 doc/development/ux_guide/img/modals-layout-for-modals.png create mode 100644 doc/development/ux_guide/img/modals-special-confimation-dialog.png create mode 100644 doc/development/ux_guide/img/modals-three-buttons-special.png create mode 100644 doc/development/ux_guide/img/modals-three-buttons.png diff --git a/doc/development/ux_guide/components.md b/doc/development/ux_guide/components.md index 2bcee69201b..a31d8cfb443 100644 --- a/doc/development/ux_guide/components.md +++ b/doc/development/ux_guide/components.md @@ -10,6 +10,7 @@ * [Tables](#tables) * [Blocks](#blocks) * [Panels](#panels) +* [Dialog modals](#dialog-modals) * [Alerts](#alerts) * [Forms](#forms) * [Search box](#search-box) @@ -255,31 +256,31 @@ Skeleton loading can replace any existing UI elements for the period in which th --- ## Dialog modals -Dialog modals are only used for having a conversation and confirmation with the user. The user is not able to access the features on the main page until making an action. Using dialog modals sparingly as it is interruptive. +Dialog modals are only used for having a conversation and confirmation with the user. The user is not able to access the features on the main page until closing the modal. ### Usage -* When the action is irreversible, we should provide the details and confirm with the user before they take an advanced action. -* When the action will affect privacy or authorization, we should provide the advanced information and confirm with the user. +* When the action is irreversible, dialog modals provide the details and confirm with the user before they take an advanced action. +* When the action will affect privacy or authorization, dialog modals provide the advanced information and confirm with the user. ### Style -* Dialog modals contain the header title, content, and action bar. -* Having an affirmative action, a dismissive action, and an extra action in action bar. -* The order of actions in the action bar: (Right to left) Affirmative action → Extra action → Dismissive action + +* Dialog modals contain the header, body, and actions. + * **Actions:** Contains a affirmative action, a dismissive action, and an extra action. The order of actions from right to left: Affirmative action → Extra action → Dismissive action + * **Header:** The header title is a question instead of a descriptive phrase. + * **Body:** The content in body should never be ambiguous and unclear. It provides specific information. * Confirmations regarding labels should keep labeling styling. * References to commits, branches, and tags should be **monospaced**. -### Writing -* The header title is a question instead of a descriptive phrase. -* The content should not be ambiguous and unclear. It should provide specific information as a reference for the user to take an advanced action. +![layout-modal](img/modals-layout-for-modals.png) ### Placement + * Dialog modals should always be the center of the screen horizontally and have **72px** from the top of the screen. | Dialog with 2 actions | Dialog with 3 actions | Special confirmation | | --------------------- | --------------------- | -------------------- | -| ![two-actions]() | ![three-actions]() | ![spcial-confirmation]() | - +| ![two-actions](img/modals-general-confimation-dialog.png) | ![three-actions](img/modals-three-buttons-special.png) | ![spcial-confirmation](img/modals-special-confimation-dialog.png) | --- diff --git a/doc/development/ux_guide/img/modals-general-confimation-dialog.png b/doc/development/ux_guide/img/modals-general-confimation-dialog.png new file mode 100644 index 00000000000..8e51cb70c48 Binary files /dev/null and b/doc/development/ux_guide/img/modals-general-confimation-dialog.png differ diff --git a/doc/development/ux_guide/img/modals-layout-for-modals.png b/doc/development/ux_guide/img/modals-layout-for-modals.png new file mode 100644 index 00000000000..71ea52be124 Binary files /dev/null and b/doc/development/ux_guide/img/modals-layout-for-modals.png differ diff --git a/doc/development/ux_guide/img/modals-special-confimation-dialog.png b/doc/development/ux_guide/img/modals-special-confimation-dialog.png new file mode 100644 index 00000000000..ebdcae25e76 Binary files /dev/null and b/doc/development/ux_guide/img/modals-special-confimation-dialog.png differ diff --git a/doc/development/ux_guide/img/modals-three-buttons-special.png b/doc/development/ux_guide/img/modals-three-buttons-special.png new file mode 100644 index 00000000000..d2d40167e4d Binary files /dev/null and b/doc/development/ux_guide/img/modals-three-buttons-special.png differ diff --git a/doc/development/ux_guide/img/modals-three-buttons.png b/doc/development/ux_guide/img/modals-three-buttons.png new file mode 100644 index 00000000000..3947902c021 Binary files /dev/null and b/doc/development/ux_guide/img/modals-three-buttons.png differ -- cgit v1.2.1 From 26abacf829187aaf566e759085c310f2dc4fa95f Mon Sep 17 00:00:00 2001 From: Hazel Date: Tue, 17 Oct 2017 14:35:06 +0800 Subject: Updated the sytle part - changed the order of header, body, and actions --- doc/development/ux_guide/components.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/development/ux_guide/components.md b/doc/development/ux_guide/components.md index a31d8cfb443..20205d6b398 100644 --- a/doc/development/ux_guide/components.md +++ b/doc/development/ux_guide/components.md @@ -266,9 +266,9 @@ Dialog modals are only used for having a conversation and confirmation with the ### Style * Dialog modals contain the header, body, and actions. - * **Actions:** Contains a affirmative action, a dismissive action, and an extra action. The order of actions from right to left: Affirmative action → Extra action → Dismissive action - * **Header:** The header title is a question instead of a descriptive phrase. - * **Body:** The content in body should never be ambiguous and unclear. It provides specific information. + * **Header(1):** The header title is a question instead of a descriptive phrase. + * **Body(2):** The content in body should never be ambiguous and unclear. It provides specific information. + * **Actions(3):** Contains a affirmative action, a dismissive action, and an extra action. The order of actions from right to left: Affirmative action → Extra action → Dismissive action * Confirmations regarding labels should keep labeling styling. * References to commits, branches, and tags should be **monospaced**. -- cgit v1.2.1 From 6d462cea77508e407995c58b0ae581edefa74d22 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Mon, 16 Oct 2017 17:51:58 +0200 Subject: Add helm CLI wrapper --- lib/gitlab/helm.rb | 57 ++++++++++++++++++++++++++++++++++++ spec/lib/gitlab/helm_spec.rb | 70 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+) create mode 100644 lib/gitlab/helm.rb create mode 100644 spec/lib/gitlab/helm_spec.rb diff --git a/lib/gitlab/helm.rb b/lib/gitlab/helm.rb new file mode 100644 index 00000000000..24f8846f69b --- /dev/null +++ b/lib/gitlab/helm.rb @@ -0,0 +1,57 @@ +module Gitlab + class Helm + Error = Class.new(StandardError) + + attr_accessor :namespace + attr_accessor :logger + + delegate :debug, :debug?, to: :logger, allow_nil: true + + def initialize(namespace, kubeconfig, logger: nil) + self.namespace = namespace + self.logger = logger + @kubeconfig = kubeconfig + end + + def init! + prepare_env do |env| + helm('init', '--upgrade', env: env) + end + end + + def install_or_upgrade!(app_name, chart) + prepare_env do |env| + helm('init', '--client-only', env: env) + helm('upgrade', '--install', '--namespace', namespace, app_name, chart, env: env) + end + end + + private + + def prepare_env(*args, &blk) + Dir.mktmpdir(['helm', namespace]) do |tmpdir| + kubeconfig_path = File.join(tmpdir, 'kubeconfig') + env = { + 'HELM_HOME' => File.join(tmpdir, 'helm'), + 'TILLER_NAMESPACE' => namespace, + 'KUBECONFIG' => kubeconfig_path + } + + File.open(kubeconfig_path, 'w') { |c| c << YAML.dump(@kubeconfig) } + + debug("HELM: Running in tmpdir #{tmpdir}") + yield(env) if block_given? + end + end + + def helm(*args, env: {}) + Open3.popen3(env, 'helm', *args) do |_, stdout, stderr, wait_thr| + exit_status = wait_thr.value + + stdout.readlines.each { |l| debug("HELM: #{l.chomp}") } if debug? + + raise Error, stderr.read.chomp unless exit_status.success? + end + end + end +end diff --git a/spec/lib/gitlab/helm_spec.rb b/spec/lib/gitlab/helm_spec.rb new file mode 100644 index 00000000000..5c8fc935b2c --- /dev/null +++ b/spec/lib/gitlab/helm_spec.rb @@ -0,0 +1,70 @@ +# coding: utf-8 +require 'spec_helper' + +describe Gitlab::Helm do + let(:namespace) { 'rails-spec' } + let(:kubeconfig) {} + let(:logger) {} + subject { described_class.new(namespace, kubeconfig, logger: logger)} + + def mock_helm_invocation(success:, error: '') + stderr = StringIO.new(error) + process_status = double('process_status') + wait_thr = double('wait_thread') + + expect(Open3).to receive(:popen3).at_least(:once).and_yield(StringIO.new, StringIO.new, stderr, wait_thr) + expect(wait_thr).to receive(:value).at_least(:once).and_return(process_status) + expect(process_status).to receive(:success?).at_least(:once).and_return(success) + end + + shared_examples 'invokes helm binary' do |method, args| + context 'when helm binary fails' do + let(:error_text) { 'Something went wrong' } + + before do + mock_helm_invocation(success: false, error: error_text) + end + + it 'throws exception' do + expect { subject.send(method, *args) }.to raise_exception(Gitlab::Helm::Error, error_text) + end + end + + context 'when helm binary exit-code is 0' do + before do + mock_helm_invocation(success: true) + end + + it "doesn't raise exceptions" do + expect { subject.send(method, *args) }.not_to raise_exception + end + end + end + + it { is_expected.to delegate_method(:debug).to(:logger) } + it { is_expected.to delegate_method(:debug?).to(:logger) } + + describe '#init!' do + it 'invokes helm init --upgrade' do + expect(subject).to receive(:helm).with('init', '--upgrade', env: instance_of(Hash)) + + subject.init! + end + + it_should_behave_like 'invokes helm binary', :init!, [] + end + + describe '#install_or_upgrade!' do + let(:app_name) { 'app_name' } + let(:chart) { 'stable/a_chart' } + + it 'invokes helm upgrade --install --namespace namespace app_name chart' do + expect(subject).to receive(:helm).with('init', '--client-only', env: instance_of(Hash)).ordered.once + expect(subject).to receive(:helm).with('upgrade', '--install', '--namespace', namespace, app_name, chart, env: instance_of(Hash)).ordered.once + + subject.install_or_upgrade!(app_name, chart) + end + + it_should_behave_like 'invokes helm binary', :install_or_upgrade!, %w[app_name chart] + end +end -- cgit v1.2.1 From 90baab7e21e6d1f327f2675dcc5516feffb3729f Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Tue, 17 Oct 2017 13:16:42 +0200 Subject: optimize branching logic --- app/views/projects/commit/branches.html.haml | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/app/views/projects/commit/branches.html.haml b/app/views/projects/commit/branches.html.haml index 933cb2f73ce..31d10ff9040 100644 --- a/app/views/projects/commit/branches.html.haml +++ b/app/views/projects/commit/branches.html.haml @@ -1,9 +1,8 @@ -- if @branches.any? || @branches_limit_exceeded - - if @branches_limit_exceeded - = commit_branch_link('#', _('Too many branches to search')) - - else - - branch = commit_default_branch(@project, @branches) - = commit_branch_link(project_ref_path(@project, branch), branch) +- if @branches_limit_exceeded + = commit_branch_link('#', _('Too many branches to search')) +- elsif @branches.any? + - branch = commit_default_branch(@project, @branches) + = commit_branch_link(project_ref_path(@project, branch), branch) -# `commit_default_branch` deletes the default branch from `@branches`, -# so only render this if we have more branches or tags left -- cgit v1.2.1 From 35a4370ec873363400ead5e7d4e939f7793b68fa Mon Sep 17 00:00:00 2001 From: Hazel Date: Wed, 18 Oct 2017 14:56:18 +0800 Subject: Modified the font family in the images --- doc/development/ux_guide/components.md | 2 +- .../img/modals-general-confimation-dialog.png | Bin 55006 -> 51205 bytes .../ux_guide/img/modals-layout-for-modals.png | Bin 71731 -> 68203 bytes .../img/modals-special-confimation-dialog.png | Bin 94366 -> 89978 bytes .../ux_guide/img/modals-three-buttons-special.png | Bin 59942 -> 0 bytes .../ux_guide/img/modals-three-buttons.png | Bin 55863 -> 54927 bytes 6 files changed, 1 insertion(+), 1 deletion(-) delete mode 100644 doc/development/ux_guide/img/modals-three-buttons-special.png diff --git a/doc/development/ux_guide/components.md b/doc/development/ux_guide/components.md index 20205d6b398..1d05d40bac0 100644 --- a/doc/development/ux_guide/components.md +++ b/doc/development/ux_guide/components.md @@ -280,7 +280,7 @@ Dialog modals are only used for having a conversation and confirmation with the | Dialog with 2 actions | Dialog with 3 actions | Special confirmation | | --------------------- | --------------------- | -------------------- | -| ![two-actions](img/modals-general-confimation-dialog.png) | ![three-actions](img/modals-three-buttons-special.png) | ![spcial-confirmation](img/modals-special-confimation-dialog.png) | +| ![two-actions](img/modals-general-confimation-dialog.png) | ![three-actions](img/modals-three-buttons.png) | ![spcial-confirmation](img/modals-special-confimation-dialog.png) | --- diff --git a/doc/development/ux_guide/img/modals-general-confimation-dialog.png b/doc/development/ux_guide/img/modals-general-confimation-dialog.png index 8e51cb70c48..00a17374a0b 100644 Binary files a/doc/development/ux_guide/img/modals-general-confimation-dialog.png and b/doc/development/ux_guide/img/modals-general-confimation-dialog.png differ diff --git a/doc/development/ux_guide/img/modals-layout-for-modals.png b/doc/development/ux_guide/img/modals-layout-for-modals.png index 71ea52be124..6c7bc09e750 100644 Binary files a/doc/development/ux_guide/img/modals-layout-for-modals.png and b/doc/development/ux_guide/img/modals-layout-for-modals.png differ diff --git a/doc/development/ux_guide/img/modals-special-confimation-dialog.png b/doc/development/ux_guide/img/modals-special-confimation-dialog.png index ebdcae25e76..bf1e56326c5 100644 Binary files a/doc/development/ux_guide/img/modals-special-confimation-dialog.png and b/doc/development/ux_guide/img/modals-special-confimation-dialog.png differ diff --git a/doc/development/ux_guide/img/modals-three-buttons-special.png b/doc/development/ux_guide/img/modals-three-buttons-special.png deleted file mode 100644 index d2d40167e4d..00000000000 Binary files a/doc/development/ux_guide/img/modals-three-buttons-special.png and /dev/null differ diff --git a/doc/development/ux_guide/img/modals-three-buttons.png b/doc/development/ux_guide/img/modals-three-buttons.png index 3947902c021..519439e64e4 100644 Binary files a/doc/development/ux_guide/img/modals-three-buttons.png and b/doc/development/ux_guide/img/modals-three-buttons.png differ -- cgit v1.2.1 From 2bdad7964a2c626118b7f2c762c39f3e1908fc9d Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Thu, 19 Oct 2017 17:18:51 +0300 Subject: added changelog entry --- changelogs/unreleased/37824-many-branches-lock-server.yml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelogs/unreleased/37824-many-branches-lock-server.yml diff --git a/changelogs/unreleased/37824-many-branches-lock-server.yml b/changelogs/unreleased/37824-many-branches-lock-server.yml new file mode 100644 index 00000000000..f75f79ec4a0 --- /dev/null +++ b/changelogs/unreleased/37824-many-branches-lock-server.yml @@ -0,0 +1,6 @@ +--- +title: While displaying a commit, do not show list of related branches if there are + thousands of branches +merge_request: 14812 +author: +type: other -- cgit v1.2.1 From 84f5aaa729d6286252602800a1f9e1bf1e5b47d3 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Tue, 31 Oct 2017 11:29:56 +0100 Subject: WIP: mock cluster apps status API --- app/serializers/cluster_entity.rb | 12 +++++++++ app/serializers/cluster_serializer.rb | 2 +- spec/fixtures/api/schemas/cluster_status.json | 35 +++++++++++++++++++++++++-- spec/serializers/cluster_entity_spec.rb | 4 +++ spec/serializers/cluster_serializer_spec.rb | 2 +- 5 files changed, 51 insertions(+), 4 deletions(-) diff --git a/app/serializers/cluster_entity.rb b/app/serializers/cluster_entity.rb index 08a113c4d8a..84ce34afb32 100644 --- a/app/serializers/cluster_entity.rb +++ b/app/serializers/cluster_entity.rb @@ -3,4 +3,16 @@ class ClusterEntity < Grape::Entity expose :status_name, as: :status expose :status_reason + expose :applications do |cluster, options| + if cluster.created? + { + helm: { status: 'installed' }, + ingress: { status: 'error', status_reason: 'Missing namespace' }, + runner: { status: 'installing' }, + prometheus: { status: 'installable' } + } + else + {} + end + end end diff --git a/app/serializers/cluster_serializer.rb b/app/serializers/cluster_serializer.rb index 2c87202a105..2e13c1501e7 100644 --- a/app/serializers/cluster_serializer.rb +++ b/app/serializers/cluster_serializer.rb @@ -2,6 +2,6 @@ class ClusterSerializer < BaseSerializer entity ClusterEntity def represent_status(resource) - represent(resource, { only: [:status, :status_reason] }) + represent(resource, { only: [:status, :status_reason, :applications] }) end end diff --git a/spec/fixtures/api/schemas/cluster_status.json b/spec/fixtures/api/schemas/cluster_status.json index 1f255a17881..451ea50f0f9 100644 --- a/spec/fixtures/api/schemas/cluster_status.json +++ b/spec/fixtures/api/schemas/cluster_status.json @@ -5,7 +5,38 @@ ], "properties" : { "status": { "type": "string" }, - "status_reason": { "type": ["string", "null"] } + "status_reason": { "type": ["string", "null"] }, + "applications": { "$ref": "#/definitions/applications" } }, - "additionalProperties": false + "additionalProperties": false, + "definitions": { + "applications": { + "type": "object", + "additionalProperties": false, + "properties" : { + "helm": { "$ref": "#/definitions/app_status" }, + "runner": { "$ref": "#/definitions/app_status" }, + "ingress": { "$ref": "#/definitions/app_status" }, + "prometheus": { "$ref": "#/definitions/app_status" } + } + }, + "app_status": { + "type": "object", + "additionalProperties": false, + "properties" : { + "status": { + "type": { + "enum": [ + "installable", + "installing", + "installed", + "error" + ] + } + }, + "status_reason": { "type": ["string", "null"] } + }, + "required" : [ "status" ] + } + } } diff --git a/spec/serializers/cluster_entity_spec.rb b/spec/serializers/cluster_entity_spec.rb index 2c7f49974f1..ec02af4b620 100644 --- a/spec/serializers/cluster_entity_spec.rb +++ b/spec/serializers/cluster_entity_spec.rb @@ -18,5 +18,9 @@ describe ClusterEntity do it 'contains status reason' do expect(subject[:status_reason]).to eq('general error') end + + it 'contains applications' do + expect(subject[:applications]).to eq({}) + end end end diff --git a/spec/serializers/cluster_serializer_spec.rb b/spec/serializers/cluster_serializer_spec.rb index 1ac6784d28f..44039327457 100644 --- a/spec/serializers/cluster_serializer_spec.rb +++ b/spec/serializers/cluster_serializer_spec.rb @@ -12,7 +12,7 @@ describe ClusterSerializer do let(:resource) { create(:gcp_cluster, :errored) } it 'serializes only status' do - expect(subject.keys).to contain_exactly(:status, :status_reason) + expect(subject.keys).to contain_exactly(:status, :status_reason, :applications) end end end -- cgit v1.2.1 From 244bec9101ff754291e2c02d9844e0879ff5fa2f Mon Sep 17 00:00:00 2001 From: Mayra Cabrera Date: Wed, 1 Nov 2017 17:09:48 -0600 Subject: Add UI/UX improvements - Leave 'too many tags to search' always visible - Add different message to the branch/tags so it states its unavailable --- app/helpers/commits_helper.rb | 16 ++++++++++++++-- app/views/projects/commit/branches.html.haml | 18 ++++++------------ 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 8aaf3318f90..2ba84e55fe0 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -60,7 +60,7 @@ module CommitsHelper branches.include?(project.default_branch) ? branches.delete(project.default_branch) : branches.pop end - # returns a link formatted as a commit branch link + # Returns a link formatted as a commit branch link def commit_branch_link(url, text) link_to(url, class: 'label label-gray ref-name') do icon('code-fork') + " #{text}" @@ -74,13 +74,25 @@ module CommitsHelper end.join(' ').html_safe end - # returns a link formatted as a commit tag link + # Returns a link formatted as a commit tag link def commit_tag_link(url, text) link_to(url, class: 'label label-gray ref-name') do icon('tag') + " #{text}" end end + def branches_unavailable_message + link_to('#', class: 'label label-gray ref-name', title: 'Project has too many branches to search') do + icon('tag') + ' Branches unavailable' + end + end + + def tags_unavailable_message + link_to('#', class: 'label label-gray ref-name', title: 'Project has too many tags to search') do + icon('tag') + ' Tags unavailable' + end + end + # Returns the sorted links to tags, separated by a comma def commit_tags_links(project, tags) sorted = VersionSorter.rsort(tags) diff --git a/app/views/projects/commit/branches.html.haml b/app/views/projects/commit/branches.html.haml index 31d10ff9040..8c7a303adbf 100644 --- a/app/views/projects/commit/branches.html.haml +++ b/app/views/projects/commit/branches.html.haml @@ -1,18 +1,12 @@ - if @branches_limit_exceeded - = commit_branch_link('#', _('Too many branches to search')) + = branches_unavailable_message - elsif @branches.any? - branch = commit_default_branch(@project, @branches) = commit_branch_link(project_ref_path(@project, branch), branch) -# `commit_default_branch` deletes the default branch from `@branches`, --# so only render this if we have more branches or tags left -- if @branches.any? || @tags.any? || @tags_limit_exceeded - %span - = link_to "…", "#", class: "js-details-expand label label-gray" - - %span.js-details-content.hide - = commit_branches_links(@project, @branches) if @branches.any? - - if @tags_limit_exceeded - = commit_tag_link('#', _('Too many tags to search')) - - elsif @tags.any? - = commit_tags_links(@project, @tags) + -# so only render this if we have more branches or tags left +- if @tags_limit_exceeded + = tags_unavailable_message +- elsif @tags.any? + = commit_tags_links(@project, @tags) -- cgit v1.2.1 From ad3853eba2ef3d6ccce7d75ff8ae543595c51735 Mon Sep 17 00:00:00 2001 From: Hazel Date: Thu, 2 Nov 2017 13:18:36 +0800 Subject: Modify the sentences and add Todo --- doc/development/ux_guide/components.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/development/ux_guide/components.md b/doc/development/ux_guide/components.md index 1d05d40bac0..82168b18860 100644 --- a/doc/development/ux_guide/components.md +++ b/doc/development/ux_guide/components.md @@ -261,7 +261,7 @@ Dialog modals are only used for having a conversation and confirmation with the ### Usage * When the action is irreversible, dialog modals provide the details and confirm with the user before they take an advanced action. -* When the action will affect privacy or authorization, dialog modals provide the advanced information and confirm with the user. +* When the action will affect privacy or authorization, dialog modals provide advanced information and confirm with the user. ### Style @@ -276,12 +276,14 @@ Dialog modals are only used for having a conversation and confirmation with the ### Placement -* Dialog modals should always be the center of the screen horizontally and have **72px** from the top of the screen. +* Dialog modals should always be the center of the screen horizontally and be positioned **72px** from the top. | Dialog with 2 actions | Dialog with 3 actions | Special confirmation | | --------------------- | --------------------- | -------------------- | | ![two-actions](img/modals-general-confimation-dialog.png) | ![three-actions](img/modals-three-buttons.png) | ![spcial-confirmation](img/modals-special-confimation-dialog.png) | +> TODO: Special case for dialog modal. + --- ## Panels -- cgit v1.2.1 From 6950f38f830079199c382c974b51ad73048a6939 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Thu, 2 Nov 2017 11:14:10 +0100 Subject: Install k8s application with helm running inside the cluster --- app/models/clusters/concerns.rb | 4 + app/models/clusters/concerns/app_status.rb | 33 +++++++ app/models/clusters/kubernetes.rb | 16 ++++ app/models/clusters/kubernetes/helm_app.rb | 18 ++++ app/models/project_services/kubernetes_service.rb | 6 ++ app/services/clusters/base_helm_service.rb | 17 ++++ .../fetch_app_installation_status_service.rb | 13 +++ .../clusters/finalize_app_installation_service.rb | 15 +++ app/services/clusters/install_app_service.rb | 23 +++++ app/services/clusters/install_tiller_service.rb | 24 +++++ app/workers/cluster_install_app_worker.rb | 11 +++ .../cluster_wait_for_app_installation_worker.rb | 33 +++++++ app/workers/concerns/cluster_app.rb | 10 ++ ...1100710_create_clusters_kubernetes_helm_apps.rb | 14 +++ lib/gitlab/clusters/helm.rb | 104 +++++++++++++++++++++ spec/models/clusters/kubernetes/helm_app_spec.rb | 14 +++ spec/models/clusters/kubernetes_spec.rb | 9 ++ .../project_services/kubernetes_service_spec.rb | 3 +- 18 files changed, 366 insertions(+), 1 deletion(-) create mode 100644 app/models/clusters/concerns.rb create mode 100644 app/models/clusters/concerns/app_status.rb create mode 100644 app/models/clusters/kubernetes.rb create mode 100644 app/models/clusters/kubernetes/helm_app.rb create mode 100644 app/services/clusters/base_helm_service.rb create mode 100644 app/services/clusters/fetch_app_installation_status_service.rb create mode 100644 app/services/clusters/finalize_app_installation_service.rb create mode 100644 app/services/clusters/install_app_service.rb create mode 100644 app/services/clusters/install_tiller_service.rb create mode 100644 app/workers/cluster_install_app_worker.rb create mode 100644 app/workers/cluster_wait_for_app_installation_worker.rb create mode 100644 app/workers/concerns/cluster_app.rb create mode 100644 db/migrate/20171031100710_create_clusters_kubernetes_helm_apps.rb create mode 100644 lib/gitlab/clusters/helm.rb create mode 100644 spec/models/clusters/kubernetes/helm_app_spec.rb create mode 100644 spec/models/clusters/kubernetes_spec.rb diff --git a/app/models/clusters/concerns.rb b/app/models/clusters/concerns.rb new file mode 100644 index 00000000000..cd09863bcfc --- /dev/null +++ b/app/models/clusters/concerns.rb @@ -0,0 +1,4 @@ +module Clusters + module Concerns + end +end diff --git a/app/models/clusters/concerns/app_status.rb b/app/models/clusters/concerns/app_status.rb new file mode 100644 index 00000000000..f6b817e9ce7 --- /dev/null +++ b/app/models/clusters/concerns/app_status.rb @@ -0,0 +1,33 @@ +module Clusters + module Concerns + module AppStatus + extend ActiveSupport::Concern + + included do + state_machine :status, initial: :scheduled do + state :errored, value: -1 + state :scheduled, value: 0 + state :installing, value: 1 + state :installed, value: 2 + + event :make_installing do + transition any - [:installing] => :installing + end + + event :make_installed do + transition any - [:installed] => :installed + end + + event :make_errored do + transition any - [:errored] => :errored + end + + before_transition any => [:errored] do |app_status, transition| + status_reason = transition.args.first + app_status.status_reason = status_reason if status_reason + end + end + end + end + end +end diff --git a/app/models/clusters/kubernetes.rb b/app/models/clusters/kubernetes.rb new file mode 100644 index 00000000000..b68e2ae401e --- /dev/null +++ b/app/models/clusters/kubernetes.rb @@ -0,0 +1,16 @@ +module Clusters + module Kubernetes + def self.table_name_prefix + 'clusters_kubernetes_' + end + + def self.app(app_name) + case app_name + when HelmApp::NAME + HelmApp + else + raise ArgumentError, "Unknown app #{app_name}" + end + end + end +end diff --git a/app/models/clusters/kubernetes/helm_app.rb b/app/models/clusters/kubernetes/helm_app.rb new file mode 100644 index 00000000000..32c9e13a469 --- /dev/null +++ b/app/models/clusters/kubernetes/helm_app.rb @@ -0,0 +1,18 @@ +module Clusters + module Kubernetes + class HelmApp < ActiveRecord::Base + NAME = 'helm'.freeze + + include ::Clusters::Concerns::AppStatus + belongs_to :kubernetes_service, class_name: 'KubernetesService', foreign_key: :service_id + + default_value_for :version, Gitlab::Clusters::Helm::HELM_VERSION + + alias_method :cluster, :kubernetes_service + + def name + NAME + end + end + end +end diff --git a/app/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb index 8ba07173c74..4b754b4d3ce 100644 --- a/app/models/project_services/kubernetes_service.rb +++ b/app/models/project_services/kubernetes_service.rb @@ -3,6 +3,8 @@ class KubernetesService < DeploymentService include Gitlab::Kubernetes include ReactiveCaching + has_one :helm_app, class_name: 'Clusters::Kubernetes::HelmApp', foreign_key: :service_id + self.reactive_cache_key = ->(service) { [service.class.model_name.singular, service.project_id] } # Namespace defaults to the project path, but can be overridden in case that @@ -136,6 +138,10 @@ class KubernetesService < DeploymentService { pods: read_pods } end + def helm + Gitlab::Clusters::Helm.new(build_kubeclient!) + end + TEMPLATE_PLACEHOLDER = 'Kubernetes namespace'.freeze private diff --git a/app/services/clusters/base_helm_service.rb b/app/services/clusters/base_helm_service.rb new file mode 100644 index 00000000000..b8ed52bf376 --- /dev/null +++ b/app/services/clusters/base_helm_service.rb @@ -0,0 +1,17 @@ +module Clusters + class BaseHelmService + attr_accessor :app + + def initialize(app) + @app = app + end + + protected + + def helm + return @helm if defined?(@helm) + + @helm = @app.cluster.helm + end + end +end diff --git a/app/services/clusters/fetch_app_installation_status_service.rb b/app/services/clusters/fetch_app_installation_status_service.rb new file mode 100644 index 00000000000..e21aa49bb43 --- /dev/null +++ b/app/services/clusters/fetch_app_installation_status_service.rb @@ -0,0 +1,13 @@ +module Clusters + class FetchAppInstallationStatusService < BaseHelmService + def execute + return unless app.installing? + + phase = helm.installation_status(app) + log = helm.installation_log(app) if phase == 'Failed' + yield(phase, log) if block_given? + rescue KubeException => ke + app.make_errored!("Kubernetes error: #{ke.message}") unless app.errored? + end + end +end diff --git a/app/services/clusters/finalize_app_installation_service.rb b/app/services/clusters/finalize_app_installation_service.rb new file mode 100644 index 00000000000..c921747febc --- /dev/null +++ b/app/services/clusters/finalize_app_installation_service.rb @@ -0,0 +1,15 @@ +module Clusters + class FinalizeAppInstallationService < BaseHelmService + def execute + helm.delete_installation_pod!(app) + + app.make_errored!('Installation aborted') if aborted? + end + + private + + def aborted? + app.installing? || app.scheduled? + end + end +end diff --git a/app/services/clusters/install_app_service.rb b/app/services/clusters/install_app_service.rb new file mode 100644 index 00000000000..dd8556108d4 --- /dev/null +++ b/app/services/clusters/install_app_service.rb @@ -0,0 +1,23 @@ +module Clusters + class InstallAppService < BaseHelmService + def execute + return unless app.scheduled? + + begin + helm.install(app) + if app.make_installing + ClusterWaitForAppInstallationWorker.perform_in( + ClusterWaitForAppInstallationWorker::INITIAL_INTERVAL, app.name, app.id) + else + app.make_errored!("Failed to update app record; #{app.errors}") + end + + rescue KubeException => ke + app.make_errored!("Kubernetes error: #{ke.message}") + rescue StandardError => e + Rails.logger.warn(e.message) + app.make_errored!("Can't start installation process") + end + end + end +end diff --git a/app/services/clusters/install_tiller_service.rb b/app/services/clusters/install_tiller_service.rb new file mode 100644 index 00000000000..ac77a7ea3c2 --- /dev/null +++ b/app/services/clusters/install_tiller_service.rb @@ -0,0 +1,24 @@ +module Clusters + class InstallTillerService < BaseService + def execute + ensure_namespace + install + end + + private + + def kubernetes_service + return @kubernetes_service if defined?(@kubernetes_service) + + @kubernetes_service = project&.kubernetes_service + end + + def ensure_namespace + kubernetes_service&.ensure_namespace! + end + + def install + kubernetes_service&.helm_client&.init! + end + end +end diff --git a/app/workers/cluster_install_app_worker.rb b/app/workers/cluster_install_app_worker.rb new file mode 100644 index 00000000000..4993b2b7349 --- /dev/null +++ b/app/workers/cluster_install_app_worker.rb @@ -0,0 +1,11 @@ +class ClusterInstallAppWorker + include Sidekiq::Worker + include ClusterQueue + include ClusterApp + + def perform(app_name, app_id) + find_app(app_name, app_id) do |app| + Clusters::InstallAppService.new(app).execute + end + end +end diff --git a/app/workers/cluster_wait_for_app_installation_worker.rb b/app/workers/cluster_wait_for_app_installation_worker.rb new file mode 100644 index 00000000000..21149cf2d19 --- /dev/null +++ b/app/workers/cluster_wait_for_app_installation_worker.rb @@ -0,0 +1,33 @@ +class ClusterWaitForAppInstallationWorker + include Sidekiq::Worker + include ClusterQueue + include ClusterApp + + INITIAL_INTERVAL = 30.seconds + EAGER_INTERVAL = 10.seconds + TIMEOUT = 20.minutes + + def perform(app_name, app_id) + find_app(app_name, app_id) do |app| + Clusters::FetchAppInstallationStatusService.new(app).execute do |phase, log| + case phase + when 'Succeeded' + if app.make_installed + Clusters::FinalizeAppInstallationService.new(app).execute + else + app.make_errored!("Failed to update app record; #{app.errors}") + end + when 'Failed' + app.make_errored!(log || 'Installation silently failed') + Clusters::FinalizeAppInstallationService.new(app).execute + else + if Time.now.utc - app.updated_at.to_time.utc > TIMEOUT + app.make_errored!('App installation timeouted') + else + ClusterWaitForAppInstallationWorker.perform_in(EAGER_INTERVAL, app.name, app.id) + end + end + end + end + end +end diff --git a/app/workers/concerns/cluster_app.rb b/app/workers/concerns/cluster_app.rb new file mode 100644 index 00000000000..2170f8be6f6 --- /dev/null +++ b/app/workers/concerns/cluster_app.rb @@ -0,0 +1,10 @@ +module ClusterApp + extend ActiveSupport::Concern + + included do + def find_app(app_name, id) + app = Clusters::Kubernetes.app(app_name).find(id) + yield(app) if block_given? + end + end +end diff --git a/db/migrate/20171031100710_create_clusters_kubernetes_helm_apps.rb b/db/migrate/20171031100710_create_clusters_kubernetes_helm_apps.rb new file mode 100644 index 00000000000..93611bf8a12 --- /dev/null +++ b/db/migrate/20171031100710_create_clusters_kubernetes_helm_apps.rb @@ -0,0 +1,14 @@ +class CreateClustersKubernetesHelmApps < ActiveRecord::Migration + def change + create_table :clusters_kubernetes_helm_apps do |t| + t.integer :status, null: false + + t.datetime_with_timezone :created_at, null: false + t.datetime_with_timezone :updated_at, null: false + + t.references :service, index: true, null: false, foreign_key: { on_delete: :cascade } + t.string :version, null: false + t.text :status_reason + end + end +end diff --git a/lib/gitlab/clusters/helm.rb b/lib/gitlab/clusters/helm.rb new file mode 100644 index 00000000000..9c75fe2be96 --- /dev/null +++ b/lib/gitlab/clusters/helm.rb @@ -0,0 +1,104 @@ +module Gitlab + module Clusters + class Helm + Error = Class.new(StandardError) + HELM_VERSION = '2.7.0'.freeze + NAMESPACE = 'gitlab-managed-apps'.freeze + COMMAND_SCRIPT = <<-EOS.freeze + set -eo pipefail + apk add -U ca-certificates openssl >/dev/null + wget -q -O - https://kubernetes-helm.storage.googleapis.com/helm-v${HELM_VERSION}-linux-amd64.tar.gz | tar zxC /tmp >/dev/null + mv /tmp/linux-amd64/helm /usr/bin/ + helm init ${HELM_INIT_OPTS} >/dev/null + [[ -z "${HELM_COMMAND+x}" ]] || helm ${HELM_COMMAND} >/dev/null + EOS + + def initialize(kubeclient) + @kubeclient = kubeclient + end + + def init! + ensure_namespace! + @kubeclient.create_pod(pod_resource(OpenStruct.new(name: 'helm'))) + end + + def install(app) + ensure_namespace! + @kubeclient.create_pod(pod_resource(app)) + end + + ## + # Returns Pod phase + # + # https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#pod-phase + # + # values: "Pending", "Running", "Succeeded", "Failed", "Unknown" + # + def installation_status(app) + @kubeclient.get_pod(pod_name(app), NAMESPACE).status.phase + end + + def installation_log(app) + @kubeclient.get_pod_log(pod_name(app), NAMESPACE).body + end + + def delete_installation_pod!(app) + @kubeclient.delete_pod(pod_name(app), NAMESPACE) + end + + private + + def pod_name(app) + "install-#{app.name}" + end + + def pod_resource(app) + labels = { 'gitlab.org/action': 'install', 'gitlab.org/application': app.name } + metadata = { name: pod_name(app), namespace: NAMESPACE, labels: labels } + container = { + name: 'helm', + image: 'alpine:3.6', + env: generate_pod_env(app), + command: %w(/bin/sh), + args: %w(-c $(COMMAND_SCRIPT)) + } + spec = { containers: [container], restartPolicy: 'Never' } + + ::Kubeclient::Resource.new(metadata: metadata, spec: spec) + end + + def generate_pod_env(app) + env = { + HELM_VERSION: HELM_VERSION, + TILLER_NAMESPACE: NAMESPACE, + COMMAND_SCRIPT: COMMAND_SCRIPT + } + + if app.name != 'helm' + env[:HELM_INIT_OPTS] = '--client-only' + env[:HELM_COMMAND] = helm_install_comand(app) + end + + env.map { |key, value| { name: key, value: value } } + end + + def helm_install_comand(app) + "install #{app.chart} --name #{app.name} --namespace #{NAMESPACE}" + end + + def ensure_namespace! + begin + @kubeclient.get_namespace(NAMESPACE) + rescue KubeException => ke + raise ke unless ke.error_code == 404 + + namespace_resource = ::Kubeclient::Resource.new + namespace_resource.metadata = {} + namespace_resource.metadata.name = NAMESPACE + + @kubeclient.create_namespace(namespace_resource) + end + end + end + end +end diff --git a/spec/models/clusters/kubernetes/helm_app_spec.rb b/spec/models/clusters/kubernetes/helm_app_spec.rb new file mode 100644 index 00000000000..27a1561ce6c --- /dev/null +++ b/spec/models/clusters/kubernetes/helm_app_spec.rb @@ -0,0 +1,14 @@ +require 'rails_helper' +require_relative '../kubernetes_spec' + +RSpec.describe Clusters::Kubernetes::HelmApp, type: :model do + it_behaves_like 'a registered kubernetes app' + + it { is_expected.to belong_to(:kubernetes_service) } + + describe '#cluster' do + it 'is an alias to #kubernetes_service' do + expect(subject.method(:cluster).original_name).to eq(:kubernetes_service) + end + end +end diff --git a/spec/models/clusters/kubernetes_spec.rb b/spec/models/clusters/kubernetes_spec.rb new file mode 100644 index 00000000000..5876f08250f --- /dev/null +++ b/spec/models/clusters/kubernetes_spec.rb @@ -0,0 +1,9 @@ +require 'rails_helper' + +RSpec.shared_examples 'a registered kubernetes app' do + let(:name) { described_class::NAME } + + it 'can be retrieved with Clusters::Kubernetes.app' do + expect(Clusters::Kubernetes.app(name)).to eq(described_class) + end +end diff --git a/spec/models/project_services/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb index 2298dcab55f..1084f42ffa7 100644 --- a/spec/models/project_services/kubernetes_service_spec.rb +++ b/spec/models/project_services/kubernetes_service_spec.rb @@ -7,8 +7,9 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do let(:project) { build_stubbed(:kubernetes_project) } let(:service) { project.kubernetes_service } - describe "Associations" do + describe 'Associations' do it { is_expected.to belong_to :project } + it { is_expected.to have_one(:helm_app) } end describe 'Validations' do -- cgit v1.2.1 From d87078520c4c8bf89b9afd77b0fee7075635824e Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Thu, 2 Nov 2017 11:15:39 +0100 Subject: Revert "Add helm CLI wrapper" This reverts commit 6d462cea77508e407995c58b0ae581edefa74d22. --- lib/gitlab/helm.rb | 57 ------------------------------------ spec/lib/gitlab/helm_spec.rb | 70 -------------------------------------------- 2 files changed, 127 deletions(-) delete mode 100644 lib/gitlab/helm.rb delete mode 100644 spec/lib/gitlab/helm_spec.rb diff --git a/lib/gitlab/helm.rb b/lib/gitlab/helm.rb deleted file mode 100644 index 24f8846f69b..00000000000 --- a/lib/gitlab/helm.rb +++ /dev/null @@ -1,57 +0,0 @@ -module Gitlab - class Helm - Error = Class.new(StandardError) - - attr_accessor :namespace - attr_accessor :logger - - delegate :debug, :debug?, to: :logger, allow_nil: true - - def initialize(namespace, kubeconfig, logger: nil) - self.namespace = namespace - self.logger = logger - @kubeconfig = kubeconfig - end - - def init! - prepare_env do |env| - helm('init', '--upgrade', env: env) - end - end - - def install_or_upgrade!(app_name, chart) - prepare_env do |env| - helm('init', '--client-only', env: env) - helm('upgrade', '--install', '--namespace', namespace, app_name, chart, env: env) - end - end - - private - - def prepare_env(*args, &blk) - Dir.mktmpdir(['helm', namespace]) do |tmpdir| - kubeconfig_path = File.join(tmpdir, 'kubeconfig') - env = { - 'HELM_HOME' => File.join(tmpdir, 'helm'), - 'TILLER_NAMESPACE' => namespace, - 'KUBECONFIG' => kubeconfig_path - } - - File.open(kubeconfig_path, 'w') { |c| c << YAML.dump(@kubeconfig) } - - debug("HELM: Running in tmpdir #{tmpdir}") - yield(env) if block_given? - end - end - - def helm(*args, env: {}) - Open3.popen3(env, 'helm', *args) do |_, stdout, stderr, wait_thr| - exit_status = wait_thr.value - - stdout.readlines.each { |l| debug("HELM: #{l.chomp}") } if debug? - - raise Error, stderr.read.chomp unless exit_status.success? - end - end - end -end diff --git a/spec/lib/gitlab/helm_spec.rb b/spec/lib/gitlab/helm_spec.rb deleted file mode 100644 index 5c8fc935b2c..00000000000 --- a/spec/lib/gitlab/helm_spec.rb +++ /dev/null @@ -1,70 +0,0 @@ -# coding: utf-8 -require 'spec_helper' - -describe Gitlab::Helm do - let(:namespace) { 'rails-spec' } - let(:kubeconfig) {} - let(:logger) {} - subject { described_class.new(namespace, kubeconfig, logger: logger)} - - def mock_helm_invocation(success:, error: '') - stderr = StringIO.new(error) - process_status = double('process_status') - wait_thr = double('wait_thread') - - expect(Open3).to receive(:popen3).at_least(:once).and_yield(StringIO.new, StringIO.new, stderr, wait_thr) - expect(wait_thr).to receive(:value).at_least(:once).and_return(process_status) - expect(process_status).to receive(:success?).at_least(:once).and_return(success) - end - - shared_examples 'invokes helm binary' do |method, args| - context 'when helm binary fails' do - let(:error_text) { 'Something went wrong' } - - before do - mock_helm_invocation(success: false, error: error_text) - end - - it 'throws exception' do - expect { subject.send(method, *args) }.to raise_exception(Gitlab::Helm::Error, error_text) - end - end - - context 'when helm binary exit-code is 0' do - before do - mock_helm_invocation(success: true) - end - - it "doesn't raise exceptions" do - expect { subject.send(method, *args) }.not_to raise_exception - end - end - end - - it { is_expected.to delegate_method(:debug).to(:logger) } - it { is_expected.to delegate_method(:debug?).to(:logger) } - - describe '#init!' do - it 'invokes helm init --upgrade' do - expect(subject).to receive(:helm).with('init', '--upgrade', env: instance_of(Hash)) - - subject.init! - end - - it_should_behave_like 'invokes helm binary', :init!, [] - end - - describe '#install_or_upgrade!' do - let(:app_name) { 'app_name' } - let(:chart) { 'stable/a_chart' } - - it 'invokes helm upgrade --install --namespace namespace app_name chart' do - expect(subject).to receive(:helm).with('init', '--client-only', env: instance_of(Hash)).ordered.once - expect(subject).to receive(:helm).with('upgrade', '--install', '--namespace', namespace, app_name, chart, env: instance_of(Hash)).ordered.once - - subject.install_or_upgrade!(app_name, chart) - end - - it_should_behave_like 'invokes helm binary', :install_or_upgrade!, %w[app_name chart] - end -end -- cgit v1.2.1 From 64be8d70ae20928df351e495a3442bb6036bc3e7 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 2 Nov 2017 15:10:46 +0100 Subject: Improve backend structure of data --- .../projects/clusters/applications_controller.rb | 39 ++++++++++++++++++++++ app/models/clusters/applications/helm.rb | 19 +++++++++++ app/models/clusters/cluster.rb | 19 ++++++++++- app/models/clusters/kubernetes.rb | 16 --------- app/models/clusters/kubernetes/helm_app.rb | 18 ---------- app/models/clusters/platforms/kubernetes.rb | 4 +++ app/models/project.rb | 1 + app/models/project_services/kubernetes_service.rb | 6 ++-- app/presenters/clusters/cluster_presenter.rb | 6 ++++ app/serializers/cluster_app_entity.rb | 5 +++ app/serializers/cluster_entity.rb | 13 +------- app/services/clusters/base_helm_service.rb | 12 +++++-- .../fetch_app_installation_status_service.rb | 4 +-- .../clusters/finalize_app_installation_service.rb | 2 +- app/services/clusters/install_app_service.rb | 4 +-- app/workers/concerns/cluster_app.rb | 5 +-- config/routes/project.rb | 4 +++ ...1100710_create_clusters_kubernetes_helm_apps.rb | 8 ++--- db/schema.rb | 35 +++++++++++-------- spec/models/clusters/kubernetes/helm_app_spec.rb | 2 +- spec/models/clusters/kubernetes_spec.rb | 9 ----- .../project_services/kubernetes_service_spec.rb | 1 - 22 files changed, 142 insertions(+), 90 deletions(-) create mode 100644 app/controllers/projects/clusters/applications_controller.rb create mode 100644 app/models/clusters/applications/helm.rb delete mode 100644 app/models/clusters/kubernetes.rb delete mode 100644 app/models/clusters/kubernetes/helm_app.rb create mode 100644 app/serializers/cluster_app_entity.rb delete mode 100644 spec/models/clusters/kubernetes_spec.rb diff --git a/app/controllers/projects/clusters/applications_controller.rb b/app/controllers/projects/clusters/applications_controller.rb new file mode 100644 index 00000000000..438e1853435 --- /dev/null +++ b/app/controllers/projects/clusters/applications_controller.rb @@ -0,0 +1,39 @@ +class Projects::Clusters::ApplicationsController < Projects::ApplicationController + before_action :cluster + before_action :application_class, only: [:create] + before_action :authorize_read_cluster! + before_action :authorize_create_cluster!, only: [:create] + + def new + end + + def create + return render_404 if application + + new_application = application_class.create(cluster: cluster) + + respond_to do |format| + format.json do + if new_application.persisted? + head :ok + else + head :bad_request + end + end + end + end + + private + + def cluster + @cluster ||= project.clusters.find_by(cluster_id: params[:cluster_id]).present(current_user: current_user) + end + + def application_class + Clusters::Cluster::Applications.find(params[:application]) + end + + def application + application_class.find_by(cluster: cluster) + end +end diff --git a/app/models/clusters/applications/helm.rb b/app/models/clusters/applications/helm.rb new file mode 100644 index 00000000000..59e0076c8df --- /dev/null +++ b/app/models/clusters/applications/helm.rb @@ -0,0 +1,19 @@ +module Clusters + module Applications + class Helm < ActiveRecord::Base + self.table_name = 'clusters_applications_helm' + + NAME = 'helm'.freeze + + include ::Clusters::Concerns::AppStatus + + belongs_to :cluser, class_name: 'Clusters::Cluster', foreign_key: :cluster_id + + default_value_for :version, Gitlab::Clusters::Helm::HELM_VERSION + + def name + NAME + end + end + end +end diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index ca09b939f34..c814f475adf 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -4,6 +4,10 @@ module Clusters self.table_name = 'clusters' + APPLICATIONS = { + Clusters::Applications::Helm::NAME => Clusters::Applications::Helm + } + belongs_to :user has_many :cluster_projects, class_name: 'Clusters::Project' @@ -15,13 +19,14 @@ module Clusters # We have to ":destroy" it today to ensure that we clean also the Kubernetes Integration has_one :platform_kubernetes, class_name: 'Clusters::Platforms::Kubernetes', autosave: true, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + has_one :application_helm, class_name: 'Clusters::Applications::Helm' + accepts_nested_attributes_for :provider_gcp, update_only: true accepts_nested_attributes_for :platform_kubernetes, update_only: true validates :name, cluster_name: true validate :restrict_modification, on: :update - delegate :status, to: :provider, allow_nil: true delegate :status_reason, to: :provider, allow_nil: true delegate :status_name, to: :provider, allow_nil: true delegate :on_creation?, to: :provider, allow_nil: true @@ -38,6 +43,14 @@ module Clusters scope :enabled, -> { where(enabled: true) } scope :disabled, -> { where(enabled: false) } + def status_name + if provider + provider.status_name + else + :created + end + end + def provider return provider_gcp if gcp? end @@ -53,6 +66,10 @@ module Clusters end alias_method :project, :first_project + def kubeclient + platform_kubernetes.kubeclient if kubernetes? + end + private def restrict_modification diff --git a/app/models/clusters/kubernetes.rb b/app/models/clusters/kubernetes.rb deleted file mode 100644 index b68e2ae401e..00000000000 --- a/app/models/clusters/kubernetes.rb +++ /dev/null @@ -1,16 +0,0 @@ -module Clusters - module Kubernetes - def self.table_name_prefix - 'clusters_kubernetes_' - end - - def self.app(app_name) - case app_name - when HelmApp::NAME - HelmApp - else - raise ArgumentError, "Unknown app #{app_name}" - end - end - end -end diff --git a/app/models/clusters/kubernetes/helm_app.rb b/app/models/clusters/kubernetes/helm_app.rb deleted file mode 100644 index 32c9e13a469..00000000000 --- a/app/models/clusters/kubernetes/helm_app.rb +++ /dev/null @@ -1,18 +0,0 @@ -module Clusters - module Kubernetes - class HelmApp < ActiveRecord::Base - NAME = 'helm'.freeze - - include ::Clusters::Concerns::AppStatus - belongs_to :kubernetes_service, class_name: 'KubernetesService', foreign_key: :service_id - - default_value_for :version, Gitlab::Clusters::Helm::HELM_VERSION - - alias_method :cluster, :kubernetes_service - - def name - NAME - end - end - end -end diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb index 3ad2ffe531d..1197dfaefcb 100644 --- a/app/models/clusters/platforms/kubernetes.rb +++ b/app/models/clusters/platforms/kubernetes.rb @@ -60,6 +60,10 @@ module Clusters self.class.namespace_for_project(project) if project end + def kubeclient + @kubeclient ||= kubernetes_service.kubeclient if manages_kubernetes_service? + end + private def enforce_namespace_to_lower_case diff --git a/app/models/project.rb b/app/models/project.rb index d9d9e12c947..a42e2553935 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -189,6 +189,7 @@ class Project < ActiveRecord::Base has_one :cluster_project, class_name: 'Clusters::Project' has_one :cluster, through: :cluster_project, class_name: 'Clusters::Cluster' + has_many :clusters, through: :cluster_project, class_name: 'Clusters::Cluster' # Container repositories need to remove data from the container registry, # which is not managed by the DB. Hence we're still using dependent: :destroy diff --git a/app/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb index b4654e8d1ea..5080acffb3c 100644 --- a/app/models/project_services/kubernetes_service.rb +++ b/app/models/project_services/kubernetes_service.rb @@ -3,8 +3,6 @@ class KubernetesService < DeploymentService include Gitlab::Kubernetes include ReactiveCaching - has_one :helm_app, class_name: 'Clusters::Kubernetes::HelmApp', foreign_key: :service_id - self.reactive_cache_key = ->(service) { [service.class.model_name.singular, service.project_id] } # Namespace defaults to the project path, but can be overridden in case that @@ -138,8 +136,8 @@ class KubernetesService < DeploymentService { pods: read_pods } end - def helm - Gitlab::Clusters::Helm.new(build_kubeclient!) + def kubeclient + @kubeclient ||= build_kubeclient! end TEMPLATE_PLACEHOLDER = 'Kubernetes namespace'.freeze diff --git a/app/presenters/clusters/cluster_presenter.rb b/app/presenters/clusters/cluster_presenter.rb index 01cb59d0d44..5e11dac3bd2 100644 --- a/app/presenters/clusters/cluster_presenter.rb +++ b/app/presenters/clusters/cluster_presenter.rb @@ -5,5 +5,11 @@ module Clusters def gke_cluster_url "https://console.cloud.google.com/kubernetes/clusters/details/#{provider.zone}/#{name}" if gcp? end + + def applications + Clusters::Cluster::APPLICATIONS.map do |key, value| + value.find_by(cluster_id: id) + end.compact + end end end diff --git a/app/serializers/cluster_app_entity.rb b/app/serializers/cluster_app_entity.rb new file mode 100644 index 00000000000..7da2d4921a2 --- /dev/null +++ b/app/serializers/cluster_app_entity.rb @@ -0,0 +1,5 @@ +class ClusterAppEntity < Grape::Entity + expose :name + expose :status_name, as: :status + expose :status_reason +end diff --git a/app/serializers/cluster_entity.rb b/app/serializers/cluster_entity.rb index 84ce34afb32..e775c68eb6b 100644 --- a/app/serializers/cluster_entity.rb +++ b/app/serializers/cluster_entity.rb @@ -3,16 +3,5 @@ class ClusterEntity < Grape::Entity expose :status_name, as: :status expose :status_reason - expose :applications do |cluster, options| - if cluster.created? - { - helm: { status: 'installed' }, - ingress: { status: 'error', status_reason: 'Missing namespace' }, - runner: { status: 'installing' }, - prometheus: { status: 'installable' } - } - else - {} - end - end + expose :applications, using: ClusterAppEntity end diff --git a/app/services/clusters/base_helm_service.rb b/app/services/clusters/base_helm_service.rb index b8ed52bf376..c7f7e2d0877 100644 --- a/app/services/clusters/base_helm_service.rb +++ b/app/services/clusters/base_helm_service.rb @@ -8,10 +8,16 @@ module Clusters protected - def helm - return @helm if defined?(@helm) + def cluster + app.cluster + end + + def kubeclient + cluster.kubeclient + end - @helm = @app.cluster.helm + def helm_api + @helm ||= Gitlab::Clusters::Helm.new(kubeclient) end end end diff --git a/app/services/clusters/fetch_app_installation_status_service.rb b/app/services/clusters/fetch_app_installation_status_service.rb index e21aa49bb43..9b281c77c49 100644 --- a/app/services/clusters/fetch_app_installation_status_service.rb +++ b/app/services/clusters/fetch_app_installation_status_service.rb @@ -3,8 +3,8 @@ module Clusters def execute return unless app.installing? - phase = helm.installation_status(app) - log = helm.installation_log(app) if phase == 'Failed' + phase = helm_api.installation_status(app) + log = helm_api.installation_log(app) if phase == 'Failed' yield(phase, log) if block_given? rescue KubeException => ke app.make_errored!("Kubernetes error: #{ke.message}") unless app.errored? diff --git a/app/services/clusters/finalize_app_installation_service.rb b/app/services/clusters/finalize_app_installation_service.rb index c921747febc..b9d5da063eb 100644 --- a/app/services/clusters/finalize_app_installation_service.rb +++ b/app/services/clusters/finalize_app_installation_service.rb @@ -1,7 +1,7 @@ module Clusters class FinalizeAppInstallationService < BaseHelmService def execute - helm.delete_installation_pod!(app) + helm_api.delete_installation_pod!(app) app.make_errored!('Installation aborted') if aborted? end diff --git a/app/services/clusters/install_app_service.rb b/app/services/clusters/install_app_service.rb index dd8556108d4..496af2495fd 100644 --- a/app/services/clusters/install_app_service.rb +++ b/app/services/clusters/install_app_service.rb @@ -4,14 +4,14 @@ module Clusters return unless app.scheduled? begin - helm.install(app) + helm_api.install(app) + if app.make_installing ClusterWaitForAppInstallationWorker.perform_in( ClusterWaitForAppInstallationWorker::INITIAL_INTERVAL, app.name, app.id) else app.make_errored!("Failed to update app record; #{app.errors}") end - rescue KubeException => ke app.make_errored!("Kubernetes error: #{ke.message}") rescue StandardError => e diff --git a/app/workers/concerns/cluster_app.rb b/app/workers/concerns/cluster_app.rb index 2170f8be6f6..a0202901f15 100644 --- a/app/workers/concerns/cluster_app.rb +++ b/app/workers/concerns/cluster_app.rb @@ -3,8 +3,9 @@ module ClusterApp included do def find_app(app_name, id) - app = Clusters::Kubernetes.app(app_name).find(id) - yield(app) if block_given? + Clusters::Applications.const_get(app_name.classify).find(id).try do |app| + yield(app) if block_given? + end end end end diff --git a/config/routes/project.rb b/config/routes/project.rb index 746c0c46677..a6a7c5e7482 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -190,6 +190,10 @@ constraints(ProjectUrlConstrainer.new) do member do get :status, format: :json + + scope '*application' do + resource :applications, only: [:create] + end end end diff --git a/db/migrate/20171031100710_create_clusters_kubernetes_helm_apps.rb b/db/migrate/20171031100710_create_clusters_kubernetes_helm_apps.rb index 93611bf8a12..1035adfe2b5 100644 --- a/db/migrate/20171031100710_create_clusters_kubernetes_helm_apps.rb +++ b/db/migrate/20171031100710_create_clusters_kubernetes_helm_apps.rb @@ -1,12 +1,12 @@ class CreateClustersKubernetesHelmApps < ActiveRecord::Migration def change - create_table :clusters_kubernetes_helm_apps do |t| - t.integer :status, null: false + create_table :clusters_applications_helm do |t| + t.references :cluster, null: false, unique: true, foreign_key: { on_delete: :cascade } t.datetime_with_timezone :created_at, null: false t.datetime_with_timezone :updated_at, null: false - - t.references :service, index: true, null: false, foreign_key: { on_delete: :cascade } + + t.integer :status, null: false t.string :version, null: false t.text :status_reason end diff --git a/db/schema.rb b/db/schema.rb index d76977d45f2..02df408b45e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20171017145932) do +ActiveRecord::Schema.define(version: 20171031100710) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -464,9 +464,7 @@ ActiveRecord::Schema.define(version: 20171017145932) do create_table "cluster_platforms_kubernetes", force: :cascade do |t| t.integer "cluster_id", null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.text "api_url" + t.string "api_url" t.text "ca_cert" t.string "namespace" t.string "username" @@ -474,6 +472,8 @@ ActiveRecord::Schema.define(version: 20171017145932) do t.string "encrypted_password_iv" t.text "encrypted_token" t.string "encrypted_token_iv" + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false end add_index "cluster_platforms_kubernetes", ["cluster_id"], name: "index_cluster_platforms_kubernetes_on_cluster_id", unique: true, using: :btree @@ -491,33 +491,39 @@ ActiveRecord::Schema.define(version: 20171017145932) do create_table "cluster_providers_gcp", force: :cascade do |t| t.integer "cluster_id", null: false t.integer "status" - t.integer "num_nodes", null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false t.text "status_reason" t.string "gcp_project_id", null: false t.string "zone", null: false + t.integer "num_nodes", null: false t.string "machine_type" t.string "operation_id" t.string "endpoint" t.text "encrypted_access_token" t.string "encrypted_access_token_iv" + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false end add_index "cluster_providers_gcp", ["cluster_id"], name: "index_cluster_providers_gcp_on_cluster_id", unique: true, using: :btree create_table "clusters", force: :cascade do |t| - t.integer "user_id", null: false - t.integer "provider_type" - t.integer "platform_type" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.integer "user_id" t.boolean "enabled", default: true t.string "name", null: false + t.integer "provider_type" + t.integer "platform_type" + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false end - add_index "clusters", ["enabled"], name: "index_clusters_on_enabled", using: :btree - add_index "clusters", ["user_id"], name: "index_clusters_on_user_id", using: :btree + create_table "clusters_applications_helm", force: :cascade do |t| + t.integer "cluster_id", null: false + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false + t.integer "status", null: false + t.string "version", null: false + t.text "status_reason" + end create_table "container_repositories", force: :cascade do |t| t.integer "project_id", null: false @@ -1872,6 +1878,7 @@ ActiveRecord::Schema.define(version: 20171017145932) do add_foreign_key "cluster_projects", "projects", on_delete: :cascade add_foreign_key "cluster_providers_gcp", "clusters", on_delete: :cascade add_foreign_key "clusters", "users", on_delete: :nullify + add_foreign_key "clusters_applications_helm", "clusters", on_delete: :cascade add_foreign_key "container_repositories", "projects" add_foreign_key "deploy_keys_projects", "projects", name: "fk_58a901ca7e", on_delete: :cascade add_foreign_key "deployments", "projects", name: "fk_b9a3851b82", on_delete: :cascade diff --git a/spec/models/clusters/kubernetes/helm_app_spec.rb b/spec/models/clusters/kubernetes/helm_app_spec.rb index 27a1561ce6c..6e32e2e7037 100644 --- a/spec/models/clusters/kubernetes/helm_app_spec.rb +++ b/spec/models/clusters/kubernetes/helm_app_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' require_relative '../kubernetes_spec' -RSpec.describe Clusters::Kubernetes::HelmApp, type: :model do +RSpec.describe Clusters::Applications::Helm, type: :model do it_behaves_like 'a registered kubernetes app' it { is_expected.to belong_to(:kubernetes_service) } diff --git a/spec/models/clusters/kubernetes_spec.rb b/spec/models/clusters/kubernetes_spec.rb deleted file mode 100644 index 5876f08250f..00000000000 --- a/spec/models/clusters/kubernetes_spec.rb +++ /dev/null @@ -1,9 +0,0 @@ -require 'rails_helper' - -RSpec.shared_examples 'a registered kubernetes app' do - let(:name) { described_class::NAME } - - it 'can be retrieved with Clusters::Kubernetes.app' do - expect(Clusters::Kubernetes.app(name)).to eq(described_class) - end -end diff --git a/spec/models/project_services/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb index 6d9fc28bf58..1c629155e1e 100644 --- a/spec/models/project_services/kubernetes_service_spec.rb +++ b/spec/models/project_services/kubernetes_service_spec.rb @@ -9,7 +9,6 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do describe 'Associations' do it { is_expected.to belong_to :project } - it { is_expected.to have_one(:helm_app) } end describe 'Validations' do -- cgit v1.2.1 From 30938b898c5415d8ec5cdf6c9851c45c1464c1a0 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 2 Nov 2017 15:31:25 +0100 Subject: Fix and add applications controller --- .../projects/clusters/applications_controller.rb | 14 +++++--------- app/models/clusters/applications/helm.rb | 2 +- app/workers/concerns/cluster_app.rb | 2 +- config/routes/project.rb | 4 ++-- 4 files changed, 9 insertions(+), 13 deletions(-) diff --git a/app/controllers/projects/clusters/applications_controller.rb b/app/controllers/projects/clusters/applications_controller.rb index 438e1853435..cdebbdefb3f 100644 --- a/app/controllers/projects/clusters/applications_controller.rb +++ b/app/controllers/projects/clusters/applications_controller.rb @@ -4,18 +4,14 @@ class Projects::Clusters::ApplicationsController < Projects::ApplicationControll before_action :authorize_read_cluster! before_action :authorize_create_cluster!, only: [:create] - def new - end - def create return render_404 if application - new_application = application_class.create(cluster: cluster) - respond_to do |format| format.json do - if new_application.persisted? - head :ok + # TODO: Do that via Service + if application_class.create(cluster: cluster).persisted? + head :no_data else head :bad_request end @@ -26,11 +22,11 @@ class Projects::Clusters::ApplicationsController < Projects::ApplicationControll private def cluster - @cluster ||= project.clusters.find_by(cluster_id: params[:cluster_id]).present(current_user: current_user) + @cluster ||= project.clusters.find(params[:id]) || render_404 end def application_class - Clusters::Cluster::Applications.find(params[:application]) + Clusters::Cluster::APPLICATIONS[params[:application]] || render_404 end def application diff --git a/app/models/clusters/applications/helm.rb b/app/models/clusters/applications/helm.rb index 59e0076c8df..a18a3f87bc4 100644 --- a/app/models/clusters/applications/helm.rb +++ b/app/models/clusters/applications/helm.rb @@ -7,7 +7,7 @@ module Clusters include ::Clusters::Concerns::AppStatus - belongs_to :cluser, class_name: 'Clusters::Cluster', foreign_key: :cluster_id + belongs_to :cluster, class_name: 'Clusters::Cluster', foreign_key: :cluster_id default_value_for :version, Gitlab::Clusters::Helm::HELM_VERSION diff --git a/app/workers/concerns/cluster_app.rb b/app/workers/concerns/cluster_app.rb index a0202901f15..947cdaefcb7 100644 --- a/app/workers/concerns/cluster_app.rb +++ b/app/workers/concerns/cluster_app.rb @@ -3,7 +3,7 @@ module ClusterApp included do def find_app(app_name, id) - Clusters::Applications.const_get(app_name.classify).find(id).try do |app| + Clusters::Cluster::APPLICATIONS[app_name].find(id).try do |app| yield(app) if block_given? end end diff --git a/config/routes/project.rb b/config/routes/project.rb index a6a7c5e7482..55dab840dca 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -191,8 +191,8 @@ constraints(ProjectUrlConstrainer.new) do member do get :status, format: :json - scope '*application' do - resource :applications, only: [:create] + scope :applications do + get '/*application', to: 'clusters/applications#create' end end end -- cgit v1.2.1 From 31c256c154e7a8727de9e91b1353b9740f380dd8 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Thu, 2 Nov 2017 16:57:18 +0100 Subject: General cleanup --- app/models/clusters/applications/helm.rb | 2 +- app/services/clusters/base_helm_service.rb | 2 +- app/services/clusters/install_app_service.rb | 1 - app/services/clusters/install_tiller_service.rb | 24 ----- ...1100710_create_clusters_kubernetes_helm_apps.rb | 4 + lib/gitlab/clusters/helm.rb | 104 -------------------- lib/gitlab/kubernetes/helm.rb | 108 +++++++++++++++++++++ 7 files changed, 114 insertions(+), 131 deletions(-) delete mode 100644 app/services/clusters/install_tiller_service.rb delete mode 100644 lib/gitlab/clusters/helm.rb create mode 100644 lib/gitlab/kubernetes/helm.rb diff --git a/app/models/clusters/applications/helm.rb b/app/models/clusters/applications/helm.rb index a18a3f87bc4..c35db143205 100644 --- a/app/models/clusters/applications/helm.rb +++ b/app/models/clusters/applications/helm.rb @@ -9,7 +9,7 @@ module Clusters belongs_to :cluster, class_name: 'Clusters::Cluster', foreign_key: :cluster_id - default_value_for :version, Gitlab::Clusters::Helm::HELM_VERSION + default_value_for :version, Gitlab::Kubernetes::Helm::HELM_VERSION def name NAME diff --git a/app/services/clusters/base_helm_service.rb b/app/services/clusters/base_helm_service.rb index c7f7e2d0877..dd717a0bb58 100644 --- a/app/services/clusters/base_helm_service.rb +++ b/app/services/clusters/base_helm_service.rb @@ -17,7 +17,7 @@ module Clusters end def helm_api - @helm ||= Gitlab::Clusters::Helm.new(kubeclient) + @helm ||= Gitlab::Kubernetes::Helm.new(kubeclient) end end end diff --git a/app/services/clusters/install_app_service.rb b/app/services/clusters/install_app_service.rb index 496af2495fd..a72cfa0a17f 100644 --- a/app/services/clusters/install_app_service.rb +++ b/app/services/clusters/install_app_service.rb @@ -15,7 +15,6 @@ module Clusters rescue KubeException => ke app.make_errored!("Kubernetes error: #{ke.message}") rescue StandardError => e - Rails.logger.warn(e.message) app.make_errored!("Can't start installation process") end end diff --git a/app/services/clusters/install_tiller_service.rb b/app/services/clusters/install_tiller_service.rb deleted file mode 100644 index ac77a7ea3c2..00000000000 --- a/app/services/clusters/install_tiller_service.rb +++ /dev/null @@ -1,24 +0,0 @@ -module Clusters - class InstallTillerService < BaseService - def execute - ensure_namespace - install - end - - private - - def kubernetes_service - return @kubernetes_service if defined?(@kubernetes_service) - - @kubernetes_service = project&.kubernetes_service - end - - def ensure_namespace - kubernetes_service&.ensure_namespace! - end - - def install - kubernetes_service&.helm_client&.init! - end - end -end diff --git a/db/migrate/20171031100710_create_clusters_kubernetes_helm_apps.rb b/db/migrate/20171031100710_create_clusters_kubernetes_helm_apps.rb index 1035adfe2b5..ef2e7e07a41 100644 --- a/db/migrate/20171031100710_create_clusters_kubernetes_helm_apps.rb +++ b/db/migrate/20171031100710_create_clusters_kubernetes_helm_apps.rb @@ -1,4 +1,8 @@ class CreateClustersKubernetesHelmApps < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + def change create_table :clusters_applications_helm do |t| t.references :cluster, null: false, unique: true, foreign_key: { on_delete: :cascade } diff --git a/lib/gitlab/clusters/helm.rb b/lib/gitlab/clusters/helm.rb deleted file mode 100644 index 9c75fe2be96..00000000000 --- a/lib/gitlab/clusters/helm.rb +++ /dev/null @@ -1,104 +0,0 @@ -module Gitlab - module Clusters - class Helm - Error = Class.new(StandardError) - HELM_VERSION = '2.7.0'.freeze - NAMESPACE = 'gitlab-managed-apps'.freeze - COMMAND_SCRIPT = <<-EOS.freeze - set -eo pipefail - apk add -U ca-certificates openssl >/dev/null - wget -q -O - https://kubernetes-helm.storage.googleapis.com/helm-v${HELM_VERSION}-linux-amd64.tar.gz | tar zxC /tmp >/dev/null - mv /tmp/linux-amd64/helm /usr/bin/ - helm init ${HELM_INIT_OPTS} >/dev/null - [[ -z "${HELM_COMMAND+x}" ]] || helm ${HELM_COMMAND} >/dev/null - EOS - - def initialize(kubeclient) - @kubeclient = kubeclient - end - - def init! - ensure_namespace! - @kubeclient.create_pod(pod_resource(OpenStruct.new(name: 'helm'))) - end - - def install(app) - ensure_namespace! - @kubeclient.create_pod(pod_resource(app)) - end - - ## - # Returns Pod phase - # - # https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#pod-phase - # - # values: "Pending", "Running", "Succeeded", "Failed", "Unknown" - # - def installation_status(app) - @kubeclient.get_pod(pod_name(app), NAMESPACE).status.phase - end - - def installation_log(app) - @kubeclient.get_pod_log(pod_name(app), NAMESPACE).body - end - - def delete_installation_pod!(app) - @kubeclient.delete_pod(pod_name(app), NAMESPACE) - end - - private - - def pod_name(app) - "install-#{app.name}" - end - - def pod_resource(app) - labels = { 'gitlab.org/action': 'install', 'gitlab.org/application': app.name } - metadata = { name: pod_name(app), namespace: NAMESPACE, labels: labels } - container = { - name: 'helm', - image: 'alpine:3.6', - env: generate_pod_env(app), - command: %w(/bin/sh), - args: %w(-c $(COMMAND_SCRIPT)) - } - spec = { containers: [container], restartPolicy: 'Never' } - - ::Kubeclient::Resource.new(metadata: metadata, spec: spec) - end - - def generate_pod_env(app) - env = { - HELM_VERSION: HELM_VERSION, - TILLER_NAMESPACE: NAMESPACE, - COMMAND_SCRIPT: COMMAND_SCRIPT - } - - if app.name != 'helm' - env[:HELM_INIT_OPTS] = '--client-only' - env[:HELM_COMMAND] = helm_install_comand(app) - end - - env.map { |key, value| { name: key, value: value } } - end - - def helm_install_comand(app) - "install #{app.chart} --name #{app.name} --namespace #{NAMESPACE}" - end - - def ensure_namespace! - begin - @kubeclient.get_namespace(NAMESPACE) - rescue KubeException => ke - raise ke unless ke.error_code == 404 - - namespace_resource = ::Kubeclient::Resource.new - namespace_resource.metadata = {} - namespace_resource.metadata.name = NAMESPACE - - @kubeclient.create_namespace(namespace_resource) - end - end - end - end -end diff --git a/lib/gitlab/kubernetes/helm.rb b/lib/gitlab/kubernetes/helm.rb new file mode 100644 index 00000000000..76bb14a0609 --- /dev/null +++ b/lib/gitlab/kubernetes/helm.rb @@ -0,0 +1,108 @@ +module Gitlab + module Kubernetes + class Helm + HELM_VERSION = '2.7.0'.freeze + NAMESPACE = 'gitlab-managed-apps'.freeze + COMMAND_SCRIPT = <<-EOS.freeze + set -eo pipefail + apk add -U ca-certificates openssl >/dev/null + wget -q -O - https://kubernetes-helm.storage.googleapis.com/helm-v${HELM_VERSION}-linux-amd64.tar.gz | tar zxC /tmp >/dev/null + mv /tmp/linux-amd64/helm /usr/bin/ + helm init ${HELM_INIT_OPTS} >/dev/null + [[ -z "${HELM_COMMAND+x}" ]] || helm ${HELM_COMMAND} >/dev/null + EOS + + def initialize(kubeclient) + @kubeclient = kubeclient + end + + def init! + install(OpenStruct.new(name: 'helm')) + end + + def install(app) + create_namespace! unless has_namespace? + @kubeclient.create_pod(pod_resource(app)) + end + + ## + # Returns Pod phase + # + # https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#pod-phase + # + # values: "Pending", "Running", "Succeeded", "Failed", "Unknown" + # + def installation_status(app) + @kubeclient.get_pod(pod_name(app), NAMESPACE).status.phase + end + + def installation_log(app) + @kubeclient.get_pod_log(pod_name(app), NAMESPACE).body + end + + def delete_installation_pod!(app) + @kubeclient.delete_pod(pod_name(app), NAMESPACE) + end + + private + + def pod_name(app) + "install-#{app.name}" + end + + def pod_resource(app) + labels = { 'gitlab.org/action': 'install', 'gitlab.org/application': app.name } + metadata = { name: pod_name(app), namespace: NAMESPACE, labels: labels } + container = { + name: 'helm', + image: 'alpine:3.6', + env: generate_pod_env(app), + command: %w(/bin/sh), + args: %w(-c $(COMMAND_SCRIPT)) + } + spec = { containers: [container], restartPolicy: 'Never' } + + ::Kubeclient::Resource.new(metadata: metadata, spec: spec) + end + + def generate_pod_env(app) + env = { + HELM_VERSION: HELM_VERSION, + TILLER_NAMESPACE: NAMESPACE, + COMMAND_SCRIPT: COMMAND_SCRIPT + } + + if app.name != 'helm' + env[:HELM_INIT_OPTS] = '--client-only' + env[:HELM_COMMAND] = helm_install_comand(app) + end + + env.map { |key, value| { name: key, value: value } } + end + + def helm_install_comand(app) + "install #{app.chart} --name #{app.name} --namespace #{NAMESPACE}" + end + + def has_namespace? + return @has_namespace if defined?(@has_namespace) + + begin + @kubeclient.get_namespace(NAMESPACE) + @has_namespace = true + rescue KubeException => ke + raise ke unless ke.error_code == 404 + false + end + end + + def create_namespace! + namespace_resource = ::Kubeclient::Resource.new + namespace_resource.metadata = {} + namespace_resource.metadata.name = NAMESPACE + + @kubeclient.create_namespace(namespace_resource) + end + end + end +end -- cgit v1.2.1 From a8d7e4bcb13e24426a531ef37d573e24d2547b81 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Thu, 2 Nov 2017 17:08:56 +0100 Subject: Fix rubocop offenses --- app/models/clusters/cluster.rb | 2 +- app/models/clusters/platforms/kubernetes.rb | 2 +- .../20171031100710_create_clusters_kubernetes_helm_apps.rb | 2 +- spec/models/clusters/applications/helm_spec.rb | 14 ++++++++++++++ spec/models/clusters/kubernetes/helm_app_spec.rb | 14 -------------- 5 files changed, 17 insertions(+), 17 deletions(-) create mode 100644 spec/models/clusters/applications/helm_spec.rb delete mode 100644 spec/models/clusters/kubernetes/helm_app_spec.rb diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index c814f475adf..4513dd426e2 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -6,7 +6,7 @@ module Clusters APPLICATIONS = { Clusters::Applications::Helm::NAME => Clusters::Applications::Helm - } + }.freeze belongs_to :user diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb index 1197dfaefcb..0bb2972f7b7 100644 --- a/app/models/clusters/platforms/kubernetes.rb +++ b/app/models/clusters/platforms/kubernetes.rb @@ -91,7 +91,7 @@ module Clusters api_url: api_url, namespace: namespace, token: token, - ca_pem: ca_cert, + ca_pem: ca_cert ) end diff --git a/db/migrate/20171031100710_create_clusters_kubernetes_helm_apps.rb b/db/migrate/20171031100710_create_clusters_kubernetes_helm_apps.rb index ef2e7e07a41..a2ce37127ea 100644 --- a/db/migrate/20171031100710_create_clusters_kubernetes_helm_apps.rb +++ b/db/migrate/20171031100710_create_clusters_kubernetes_helm_apps.rb @@ -9,7 +9,7 @@ class CreateClustersKubernetesHelmApps < ActiveRecord::Migration t.datetime_with_timezone :created_at, null: false t.datetime_with_timezone :updated_at, null: false - + t.integer :status, null: false t.string :version, null: false t.text :status_reason diff --git a/spec/models/clusters/applications/helm_spec.rb b/spec/models/clusters/applications/helm_spec.rb new file mode 100644 index 00000000000..6e32e2e7037 --- /dev/null +++ b/spec/models/clusters/applications/helm_spec.rb @@ -0,0 +1,14 @@ +require 'rails_helper' +require_relative '../kubernetes_spec' + +RSpec.describe Clusters::Applications::Helm, type: :model do + it_behaves_like 'a registered kubernetes app' + + it { is_expected.to belong_to(:kubernetes_service) } + + describe '#cluster' do + it 'is an alias to #kubernetes_service' do + expect(subject.method(:cluster).original_name).to eq(:kubernetes_service) + end + end +end diff --git a/spec/models/clusters/kubernetes/helm_app_spec.rb b/spec/models/clusters/kubernetes/helm_app_spec.rb deleted file mode 100644 index 6e32e2e7037..00000000000 --- a/spec/models/clusters/kubernetes/helm_app_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -require 'rails_helper' -require_relative '../kubernetes_spec' - -RSpec.describe Clusters::Applications::Helm, type: :model do - it_behaves_like 'a registered kubernetes app' - - it { is_expected.to belong_to(:kubernetes_service) } - - describe '#cluster' do - it 'is an alias to #kubernetes_service' do - expect(subject.method(:cluster).original_name).to eq(:kubernetes_service) - end - end -end -- cgit v1.2.1 From 94d5f5680e398ddfe0cc621bde00dfb5c8a39d03 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Thu, 2 Nov 2017 17:52:17 +0100 Subject: Extract ClusterWaitForAppInstallationWorker logic into a service --- .../check_app_installation_progress_service.rb | 28 ++++++++++++++++++++++ app/services/clusters/install_app_service.rb | 2 +- .../cluster_wait_for_app_installation_worker.rb | 20 +--------------- 3 files changed, 30 insertions(+), 20 deletions(-) create mode 100644 app/services/clusters/check_app_installation_progress_service.rb diff --git a/app/services/clusters/check_app_installation_progress_service.rb b/app/services/clusters/check_app_installation_progress_service.rb new file mode 100644 index 00000000000..ff3398bbd63 --- /dev/null +++ b/app/services/clusters/check_app_installation_progress_service.rb @@ -0,0 +1,28 @@ +module Clusters + class CheckAppInstallationProgressService < BaseHelmService + def execute + return unless app.installing? + + FetchAppInstallationStatusService.new(app).execute do |phase, log| + case phase + when 'Succeeded' + if app.make_installed + FinalizeAppInstallationService.new(app).execute + else + app.make_errored!("Failed to update app record; #{app.errors}") + end + when 'Failed' + app.make_errored!(log || 'Installation silently failed') + FinalizeAppInstallationService.new(app).execute + else + if Time.now.utc - app.updated_at.to_time.utc > ClusterWaitForAppInstallationWorker::TIMEOUT + app.make_errored!('App installation timeouted') + else + ClusterWaitForAppInstallationWorker.perform_in( + ClusterWaitForAppInstallationWorker::EAGER_INTERVAL, app.name, app.id) + end + end + end + end + end +end diff --git a/app/services/clusters/install_app_service.rb b/app/services/clusters/install_app_service.rb index a72cfa0a17f..63f15abaa6d 100644 --- a/app/services/clusters/install_app_service.rb +++ b/app/services/clusters/install_app_service.rb @@ -14,7 +14,7 @@ module Clusters end rescue KubeException => ke app.make_errored!("Kubernetes error: #{ke.message}") - rescue StandardError => e + rescue StandardError app.make_errored!("Can't start installation process") end end diff --git a/app/workers/cluster_wait_for_app_installation_worker.rb b/app/workers/cluster_wait_for_app_installation_worker.rb index 21149cf2d19..7e480c42fd4 100644 --- a/app/workers/cluster_wait_for_app_installation_worker.rb +++ b/app/workers/cluster_wait_for_app_installation_worker.rb @@ -9,25 +9,7 @@ class ClusterWaitForAppInstallationWorker def perform(app_name, app_id) find_app(app_name, app_id) do |app| - Clusters::FetchAppInstallationStatusService.new(app).execute do |phase, log| - case phase - when 'Succeeded' - if app.make_installed - Clusters::FinalizeAppInstallationService.new(app).execute - else - app.make_errored!("Failed to update app record; #{app.errors}") - end - when 'Failed' - app.make_errored!(log || 'Installation silently failed') - Clusters::FinalizeAppInstallationService.new(app).execute - else - if Time.now.utc - app.updated_at.to_time.utc > TIMEOUT - app.make_errored!('App installation timeouted') - else - ClusterWaitForAppInstallationWorker.perform_in(EAGER_INTERVAL, app.name, app.id) - end - end - end + Clusters::CheckAppInstallationProgressService.new(app).execute end end end -- cgit v1.2.1 From 4d0a700da09be50291e40a11975b56f74051405b Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 2 Nov 2017 17:57:50 +0100 Subject: Expose applications as array via API --- app/models/clusters/applications/helm.rb | 2 +- app/models/clusters/cluster.rb | 6 ++++ app/models/clusters/concerns/app_status.rb | 33 --------------------- app/models/clusters/concerns/application_status.rb | 34 ++++++++++++++++++++++ app/presenters/clusters/cluster_presenter.rb | 6 ---- 5 files changed, 41 insertions(+), 40 deletions(-) delete mode 100644 app/models/clusters/concerns/app_status.rb create mode 100644 app/models/clusters/concerns/application_status.rb diff --git a/app/models/clusters/applications/helm.rb b/app/models/clusters/applications/helm.rb index c35db143205..77c0c61d968 100644 --- a/app/models/clusters/applications/helm.rb +++ b/app/models/clusters/applications/helm.rb @@ -5,7 +5,7 @@ module Clusters NAME = 'helm'.freeze - include ::Clusters::Concerns::AppStatus + include ::Clusters::Concerns::ApplicationStatus belongs_to :cluster, class_name: 'Clusters::Cluster', foreign_key: :cluster_id diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 4513dd426e2..3fd8ffb6e92 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -51,6 +51,12 @@ module Clusters end end + def applications + [ + application_helm || build_application_helm + ] + end + def provider return provider_gcp if gcp? end diff --git a/app/models/clusters/concerns/app_status.rb b/app/models/clusters/concerns/app_status.rb deleted file mode 100644 index f6b817e9ce7..00000000000 --- a/app/models/clusters/concerns/app_status.rb +++ /dev/null @@ -1,33 +0,0 @@ -module Clusters - module Concerns - module AppStatus - extend ActiveSupport::Concern - - included do - state_machine :status, initial: :scheduled do - state :errored, value: -1 - state :scheduled, value: 0 - state :installing, value: 1 - state :installed, value: 2 - - event :make_installing do - transition any - [:installing] => :installing - end - - event :make_installed do - transition any - [:installed] => :installed - end - - event :make_errored do - transition any - [:errored] => :errored - end - - before_transition any => [:errored] do |app_status, transition| - status_reason = transition.args.first - app_status.status_reason = status_reason if status_reason - end - end - end - end - end -end diff --git a/app/models/clusters/concerns/application_status.rb b/app/models/clusters/concerns/application_status.rb new file mode 100644 index 00000000000..e1f4c7fdda8 --- /dev/null +++ b/app/models/clusters/concerns/application_status.rb @@ -0,0 +1,34 @@ +module Clusters + module Concerns + module ApplicationStatus + extend ActiveSupport::Concern + + included do + state_machine :status, initial: :installable do + state :errored, value: -1 + state :installable, value: 0 + state :scheduled, value: 1 + state :installing, value: 2 + state :installed, value: 3 + + event :make_installing do + transition any - [:installing] => :installing + end + + event :make_installed do + transition any - [:installed] => :installed + end + + event :make_errored do + transition any - [:errored] => :errored + end + + before_transition any => [:errored] do |app_status, transition| + status_reason = transition.args.first + app_status.status_reason = status_reason if status_reason + end + end + end + end + end +end diff --git a/app/presenters/clusters/cluster_presenter.rb b/app/presenters/clusters/cluster_presenter.rb index 5e11dac3bd2..01cb59d0d44 100644 --- a/app/presenters/clusters/cluster_presenter.rb +++ b/app/presenters/clusters/cluster_presenter.rb @@ -5,11 +5,5 @@ module Clusters def gke_cluster_url "https://console.cloud.google.com/kubernetes/clusters/details/#{provider.zone}/#{name}" if gcp? end - - def applications - Clusters::Cluster::APPLICATIONS.map do |key, value| - value.find_by(cluster_id: id) - end.compact - end end end -- cgit v1.2.1 From 4585f358b923bb9b4ab36a0c3e4682b060e088a7 Mon Sep 17 00:00:00 2001 From: Mayra Cabrera Date: Thu, 2 Nov 2017 15:08:10 -0600 Subject: Add styles for branch & tags boxes - Create a partial for limit exceeded message - Modifies specs to adjust new scenarios --- app/assets/stylesheets/framework/wells.scss | 41 ++++++++++++++++++++++ app/helpers/commits_helper.rb | 12 ------- app/views/projects/commit/_commit_box.html.haml | 2 +- .../commit/_limit_exceeded_message.html.haml | 8 +++++ app/views/projects/commit/branches.html.haml | 9 ++--- .../projects/commit/branches.html.haml_spec.rb | 6 ++-- 6 files changed, 55 insertions(+), 23 deletions(-) create mode 100644 app/views/projects/commit/_limit_exceeded_message.html.haml diff --git a/app/assets/stylesheets/framework/wells.scss b/app/assets/stylesheets/framework/wells.scss index 5f9756bf58a..556485c2f3b 100644 --- a/app/assets/stylesheets/framework/wells.scss +++ b/app/assets/stylesheets/framework/wells.scss @@ -52,6 +52,47 @@ .label.label-gray { background-color: $well-expand-item; } + + .limit-box-branch { + width: 160px; + } + + .limit-box-tag { + width: 125px; + } + + .branches { + display: inline; + } + + .limit-box { + background: $blue-100; + border-radius: 3px; + display: inline-block; + margin-left: 5px; + padding: 1px 1px 0.5px 0; + text-align: center; + vertical-align: bottom; + + &:hover { + background: $blue-200; + } + + .limit-icon { + float: left; + width: 15%; + } + + .limit-message { + background: $white-light; + border-radius: 0 3px 3px 0; + font-family: $regular_font; + font-size: 12px; + float: right; + margin-top: 1px; + width: 85%; + } + } } .light-well { diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 2ba84e55fe0..95fc50a476a 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -81,18 +81,6 @@ module CommitsHelper end end - def branches_unavailable_message - link_to('#', class: 'label label-gray ref-name', title: 'Project has too many branches to search') do - icon('tag') + ' Branches unavailable' - end - end - - def tags_unavailable_message - link_to('#', class: 'label label-gray ref-name', title: 'Project has too many tags to search') do - icon('tag') + ' Tags unavailable' - end - end - # Returns the sorted links to tags, separated by a comma def commit_tags_links(project, tags) sorted = VersionSorter.rsort(tags) diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index 09bcd187e59..cd93886bb24 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -61,7 +61,7 @@ %span.cgray= n_('parent', 'parents', @commit.parents.count) - @commit.parents.each do |parent| = link_to parent.short_id, project_commit_path(@project, parent), class: "commit-sha" - %span.commit-info.branches + %div.commit-info.branches %i.fa.fa-spinner.fa-spin - if @commit.last_pipeline diff --git a/app/views/projects/commit/_limit_exceeded_message.html.haml b/app/views/projects/commit/_limit_exceeded_message.html.haml new file mode 100644 index 00000000000..b3372108321 --- /dev/null +++ b/app/views/projects/commit/_limit_exceeded_message.html.haml @@ -0,0 +1,8 @@ +.has-tooltip{class: "limit-box limit-box-#{objects.singularize}", data: { title: "Project has too many #{objects} to search"} } + .limit-icon + - if objects == "branches" + = icon('code-fork') + - else + = icon('tag') + .limit-message + %span= "#{objects.capitalize} unavailable" diff --git a/app/views/projects/commit/branches.html.haml b/app/views/projects/commit/branches.html.haml index 8c7a303adbf..d282eb47914 100644 --- a/app/views/projects/commit/branches.html.haml +++ b/app/views/projects/commit/branches.html.haml @@ -1,12 +1,9 @@ - if @branches_limit_exceeded - = branches_unavailable_message + = render 'limit_exceeded_message', objects: 'branches' - elsif @branches.any? - - branch = commit_default_branch(@project, @branches) - = commit_branch_link(project_ref_path(@project, branch), branch) + = commit_branches_links(@project, @branches) --# `commit_default_branch` deletes the default branch from `@branches`, - -# so only render this if we have more branches or tags left - if @tags_limit_exceeded - = tags_unavailable_message + = render 'limit_exceeded_message', objects: 'tags' - elsif @tags.any? = commit_tags_links(@project, @tags) diff --git a/spec/views/projects/commit/branches.html.haml_spec.rb b/spec/views/projects/commit/branches.html.haml_spec.rb index 4199d2fd962..c9112ede334 100644 --- a/spec/views/projects/commit/branches.html.haml_spec.rb +++ b/spec/views/projects/commit/branches.html.haml_spec.rb @@ -18,7 +18,6 @@ describe 'projects/commit/branches.html.haml' do end it 'shows branch and tag links' do - expect(rendered).to have_selector('.js-details-expand') expect(rendered).to have_link('master') expect(rendered).to have_link('test-branch') expect(rendered).to have_link('tag1') @@ -36,9 +35,8 @@ describe 'projects/commit/branches.html.haml' do end it 'shows too many to search' do - expect(rendered).to have_selector('.js-details-expand') - expect(rendered).to have_link('Too many branches to search', href: '#') - expect(rendered).to have_link('Too many tags to search', href: '#') + expect(rendered).to have_text('Branches unavailable') + expect(rendered).to have_text('Tags unavailable') end end end -- cgit v1.2.1 From 4477f7bb5925d8d720e3e8272bd882fffcc04b28 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 3 Nov 2017 09:22:11 +0100 Subject: Fix nitpicks --- app/models/clusters/cluster.rb | 2 +- app/services/clusters/base_helm_service.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 3fd8ffb6e92..7dae2234998 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -27,8 +27,8 @@ module Clusters validates :name, cluster_name: true validate :restrict_modification, on: :update + delegate :status, to: :provider, allow_nil: true delegate :status_reason, to: :provider, allow_nil: true - delegate :status_name, to: :provider, allow_nil: true delegate :on_creation?, to: :provider, allow_nil: true enum platform_type: { diff --git a/app/services/clusters/base_helm_service.rb b/app/services/clusters/base_helm_service.rb index dd717a0bb58..0a95955a204 100644 --- a/app/services/clusters/base_helm_service.rb +++ b/app/services/clusters/base_helm_service.rb @@ -17,7 +17,7 @@ module Clusters end def helm_api - @helm ||= Gitlab::Kubernetes::Helm.new(kubeclient) + @helm_api ||= Gitlab::Kubernetes::Helm.new(kubeclient) end end end -- cgit v1.2.1 From c46417c5064867d72debb15c4e280db24c6ab73c Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Thu, 2 Nov 2017 18:55:02 +0100 Subject: Rename App to Applications --- app/models/clusters/concerns.rb | 4 --- .../clusters/applications/base_helm_service.rb | 25 ++++++++++++++++++ .../check_installation_progress_service.rb | 30 ++++++++++++++++++++++ .../fetch_installation_status_service.rb | 15 +++++++++++ .../applications/finalize_installation_service.rb | 17 ++++++++++++ .../clusters/applications/install_service.rb | 24 +++++++++++++++++ app/services/clusters/base_helm_service.rb | 23 ----------------- .../check_app_installation_progress_service.rb | 28 -------------------- .../fetch_app_installation_status_service.rb | 13 ---------- .../clusters/finalize_app_installation_service.rb | 15 ----------- app/services/clusters/install_app_service.rb | 22 ---------------- app/workers/cluster_install_app_worker.rb | 6 ++--- .../cluster_wait_for_app_installation_worker.rb | 6 ++--- app/workers/concerns/cluster_app.rb | 11 -------- app/workers/concerns/cluster_applications.rb | 9 +++++++ 15 files changed, 126 insertions(+), 122 deletions(-) delete mode 100644 app/models/clusters/concerns.rb create mode 100644 app/services/clusters/applications/base_helm_service.rb create mode 100644 app/services/clusters/applications/check_installation_progress_service.rb create mode 100644 app/services/clusters/applications/fetch_installation_status_service.rb create mode 100644 app/services/clusters/applications/finalize_installation_service.rb create mode 100644 app/services/clusters/applications/install_service.rb delete mode 100644 app/services/clusters/base_helm_service.rb delete mode 100644 app/services/clusters/check_app_installation_progress_service.rb delete mode 100644 app/services/clusters/fetch_app_installation_status_service.rb delete mode 100644 app/services/clusters/finalize_app_installation_service.rb delete mode 100644 app/services/clusters/install_app_service.rb delete mode 100644 app/workers/concerns/cluster_app.rb create mode 100644 app/workers/concerns/cluster_applications.rb diff --git a/app/models/clusters/concerns.rb b/app/models/clusters/concerns.rb deleted file mode 100644 index cd09863bcfc..00000000000 --- a/app/models/clusters/concerns.rb +++ /dev/null @@ -1,4 +0,0 @@ -module Clusters - module Concerns - end -end diff --git a/app/services/clusters/applications/base_helm_service.rb b/app/services/clusters/applications/base_helm_service.rb new file mode 100644 index 00000000000..68320a3b267 --- /dev/null +++ b/app/services/clusters/applications/base_helm_service.rb @@ -0,0 +1,25 @@ +module Clusters + module Applications + class BaseHelmService + attr_accessor :app + + def initialize(app) + @app = app + end + + protected + + def cluster + app.cluster + end + + def kubeclient + cluster.kubeclient + end + + def helm_api + @helm_api ||= Gitlab::Kubernetes::Helm.new(kubeclient) + end + end + end +end diff --git a/app/services/clusters/applications/check_installation_progress_service.rb b/app/services/clusters/applications/check_installation_progress_service.rb new file mode 100644 index 00000000000..4e8fd9baaf4 --- /dev/null +++ b/app/services/clusters/applications/check_installation_progress_service.rb @@ -0,0 +1,30 @@ +module Clusters + module Applications + class CheckInstallationProgressService < BaseHelmService + def execute + return unless app.installing? + + FetchInstallationStatusService.new(app).execute do |phase, log| + case phase + when 'Succeeded' + if app.make_installed + FinalizeInstallationService.new(app).execute + else + app.make_errored!("Failed to update app record; #{app.errors}") + end + when 'Failed' + app.make_errored!(log || 'Installation silently failed') + FinalizeInstallationService.new(app).execute + else + if Time.now.utc - app.updated_at.to_time.utc > ClusterWaitForAppInstallationWorker::TIMEOUT + app.make_errored!('App installation timeouted') + else + ClusterWaitForAppInstallationWorker.perform_in( + ClusterWaitForAppInstallationWorker::EAGER_INTERVAL, app.name, app.id) + end + end + end + end + end + end +end diff --git a/app/services/clusters/applications/fetch_installation_status_service.rb b/app/services/clusters/applications/fetch_installation_status_service.rb new file mode 100644 index 00000000000..3d082485532 --- /dev/null +++ b/app/services/clusters/applications/fetch_installation_status_service.rb @@ -0,0 +1,15 @@ +module Clusters + module Applications + class FetchInstallationStatusService < BaseHelmService + def execute + return unless app.installing? + + phase = helm_api.installation_status(app) + log = helm_api.installation_log(app) if phase == 'Failed' + yield(phase, log) if block_given? + rescue KubeException => ke + app.make_errored!("Kubernetes error: #{ke.message}") unless app.errored? + end + end + end +end diff --git a/app/services/clusters/applications/finalize_installation_service.rb b/app/services/clusters/applications/finalize_installation_service.rb new file mode 100644 index 00000000000..339d671c091 --- /dev/null +++ b/app/services/clusters/applications/finalize_installation_service.rb @@ -0,0 +1,17 @@ +module Clusters + module Applications + class FinalizeInstallationService < BaseHelmService + def execute + helm_api.delete_installation_pod!(app) + + app.make_errored!('Installation aborted') if aborted? + end + + private + + def aborted? + app.installing? || app.scheduled? + end + end + end +end diff --git a/app/services/clusters/applications/install_service.rb b/app/services/clusters/applications/install_service.rb new file mode 100644 index 00000000000..7fcccb5e78c --- /dev/null +++ b/app/services/clusters/applications/install_service.rb @@ -0,0 +1,24 @@ +module Clusters + module Applications + class InstallService < BaseHelmService + def execute + return unless app.scheduled? + + begin + helm_api.install(app) + + if app.make_installing + ClusterWaitForAppInstallationWorker.perform_in( + ClusterWaitForAppInstallationWorker::INITIAL_INTERVAL, app.name, app.id) + else + app.make_errored!("Failed to update app record; #{app.errors}") + end + rescue KubeException => ke + app.make_errored!("Kubernetes error: #{ke.message}") + rescue StandardError + app.make_errored!("Can't start installation process") + end + end + end + end +end diff --git a/app/services/clusters/base_helm_service.rb b/app/services/clusters/base_helm_service.rb deleted file mode 100644 index 0a95955a204..00000000000 --- a/app/services/clusters/base_helm_service.rb +++ /dev/null @@ -1,23 +0,0 @@ -module Clusters - class BaseHelmService - attr_accessor :app - - def initialize(app) - @app = app - end - - protected - - def cluster - app.cluster - end - - def kubeclient - cluster.kubeclient - end - - def helm_api - @helm_api ||= Gitlab::Kubernetes::Helm.new(kubeclient) - end - end -end diff --git a/app/services/clusters/check_app_installation_progress_service.rb b/app/services/clusters/check_app_installation_progress_service.rb deleted file mode 100644 index ff3398bbd63..00000000000 --- a/app/services/clusters/check_app_installation_progress_service.rb +++ /dev/null @@ -1,28 +0,0 @@ -module Clusters - class CheckAppInstallationProgressService < BaseHelmService - def execute - return unless app.installing? - - FetchAppInstallationStatusService.new(app).execute do |phase, log| - case phase - when 'Succeeded' - if app.make_installed - FinalizeAppInstallationService.new(app).execute - else - app.make_errored!("Failed to update app record; #{app.errors}") - end - when 'Failed' - app.make_errored!(log || 'Installation silently failed') - FinalizeAppInstallationService.new(app).execute - else - if Time.now.utc - app.updated_at.to_time.utc > ClusterWaitForAppInstallationWorker::TIMEOUT - app.make_errored!('App installation timeouted') - else - ClusterWaitForAppInstallationWorker.perform_in( - ClusterWaitForAppInstallationWorker::EAGER_INTERVAL, app.name, app.id) - end - end - end - end - end -end diff --git a/app/services/clusters/fetch_app_installation_status_service.rb b/app/services/clusters/fetch_app_installation_status_service.rb deleted file mode 100644 index 9b281c77c49..00000000000 --- a/app/services/clusters/fetch_app_installation_status_service.rb +++ /dev/null @@ -1,13 +0,0 @@ -module Clusters - class FetchAppInstallationStatusService < BaseHelmService - def execute - return unless app.installing? - - phase = helm_api.installation_status(app) - log = helm_api.installation_log(app) if phase == 'Failed' - yield(phase, log) if block_given? - rescue KubeException => ke - app.make_errored!("Kubernetes error: #{ke.message}") unless app.errored? - end - end -end diff --git a/app/services/clusters/finalize_app_installation_service.rb b/app/services/clusters/finalize_app_installation_service.rb deleted file mode 100644 index b9d5da063eb..00000000000 --- a/app/services/clusters/finalize_app_installation_service.rb +++ /dev/null @@ -1,15 +0,0 @@ -module Clusters - class FinalizeAppInstallationService < BaseHelmService - def execute - helm_api.delete_installation_pod!(app) - - app.make_errored!('Installation aborted') if aborted? - end - - private - - def aborted? - app.installing? || app.scheduled? - end - end -end diff --git a/app/services/clusters/install_app_service.rb b/app/services/clusters/install_app_service.rb deleted file mode 100644 index 63f15abaa6d..00000000000 --- a/app/services/clusters/install_app_service.rb +++ /dev/null @@ -1,22 +0,0 @@ -module Clusters - class InstallAppService < BaseHelmService - def execute - return unless app.scheduled? - - begin - helm_api.install(app) - - if app.make_installing - ClusterWaitForAppInstallationWorker.perform_in( - ClusterWaitForAppInstallationWorker::INITIAL_INTERVAL, app.name, app.id) - else - app.make_errored!("Failed to update app record; #{app.errors}") - end - rescue KubeException => ke - app.make_errored!("Kubernetes error: #{ke.message}") - rescue StandardError - app.make_errored!("Can't start installation process") - end - end - end -end diff --git a/app/workers/cluster_install_app_worker.rb b/app/workers/cluster_install_app_worker.rb index 4993b2b7349..899aed904e4 100644 --- a/app/workers/cluster_install_app_worker.rb +++ b/app/workers/cluster_install_app_worker.rb @@ -1,11 +1,11 @@ class ClusterInstallAppWorker include Sidekiq::Worker include ClusterQueue - include ClusterApp + include ClusterApplications def perform(app_name, app_id) - find_app(app_name, app_id) do |app| - Clusters::InstallAppService.new(app).execute + find_application(app_name, app_id) do |app| + Clusters::Applications::InstallService.new(app).execute end end end diff --git a/app/workers/cluster_wait_for_app_installation_worker.rb b/app/workers/cluster_wait_for_app_installation_worker.rb index 7e480c42fd4..d5974c467c4 100644 --- a/app/workers/cluster_wait_for_app_installation_worker.rb +++ b/app/workers/cluster_wait_for_app_installation_worker.rb @@ -1,15 +1,15 @@ class ClusterWaitForAppInstallationWorker include Sidekiq::Worker include ClusterQueue - include ClusterApp + include ClusterApplications INITIAL_INTERVAL = 30.seconds EAGER_INTERVAL = 10.seconds TIMEOUT = 20.minutes def perform(app_name, app_id) - find_app(app_name, app_id) do |app| - Clusters::CheckAppInstallationProgressService.new(app).execute + find_application(app_name, app_id) do |app| + Clusters::Applications::CheckInstallationProgressService.new(app).execute end end end diff --git a/app/workers/concerns/cluster_app.rb b/app/workers/concerns/cluster_app.rb deleted file mode 100644 index 947cdaefcb7..00000000000 --- a/app/workers/concerns/cluster_app.rb +++ /dev/null @@ -1,11 +0,0 @@ -module ClusterApp - extend ActiveSupport::Concern - - included do - def find_app(app_name, id) - Clusters::Cluster::APPLICATIONS[app_name].find(id).try do |app| - yield(app) if block_given? - end - end - end -end diff --git a/app/workers/concerns/cluster_applications.rb b/app/workers/concerns/cluster_applications.rb new file mode 100644 index 00000000000..24ecaa0b52f --- /dev/null +++ b/app/workers/concerns/cluster_applications.rb @@ -0,0 +1,9 @@ +module ClusterApplications + extend ActiveSupport::Concern + + included do + def find_application(app_name, id, &blk) + Clusters::Cluster::APPLICATIONS[app_name].find(id).try(&blk) + end + end +end -- cgit v1.2.1 From 08752e5d742a144ffb1ec7c8e07e7a558774fbfc Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Fri, 3 Nov 2017 10:02:30 +0100 Subject: Remove `Clusters::Applications::FetchInstallationStatusService` --- app/models/clusters/cluster.rb | 2 +- .../check_installation_progress_service.rb | 64 +++++++++++++++------- .../fetch_installation_status_service.rb | 15 ----- .../clusters/applications/install_service.rb | 2 +- .../cluster_wait_for_app_installation_worker.rb | 3 +- lib/gitlab/kubernetes/pod.rb | 12 ++++ 6 files changed, 60 insertions(+), 38 deletions(-) delete mode 100644 app/services/clusters/applications/fetch_installation_status_service.rb create mode 100644 lib/gitlab/kubernetes/pod.rb diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 7dae2234998..77b299e46a0 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -52,7 +52,7 @@ module Clusters end def applications - [ + [ application_helm || build_application_helm ] end diff --git a/app/services/clusters/applications/check_installation_progress_service.rb b/app/services/clusters/applications/check_installation_progress_service.rb index 4e8fd9baaf4..7f5a633b749 100644 --- a/app/services/clusters/applications/check_installation_progress_service.rb +++ b/app/services/clusters/applications/check_installation_progress_service.rb @@ -4,26 +4,52 @@ module Clusters def execute return unless app.installing? - FetchInstallationStatusService.new(app).execute do |phase, log| - case phase - when 'Succeeded' - if app.make_installed - FinalizeInstallationService.new(app).execute - else - app.make_errored!("Failed to update app record; #{app.errors}") - end - when 'Failed' - app.make_errored!(log || 'Installation silently failed') - FinalizeInstallationService.new(app).execute - else - if Time.now.utc - app.updated_at.to_time.utc > ClusterWaitForAppInstallationWorker::TIMEOUT - app.make_errored!('App installation timeouted') - else - ClusterWaitForAppInstallationWorker.perform_in( - ClusterWaitForAppInstallationWorker::EAGER_INTERVAL, app.name, app.id) - end - end + case installation_phase + when Gitlab::Kubernetes::Pod::SUCCEEDED + on_succeeded + when Gitlab::Kubernetes::Pod::FAILED + on_failed + else + check_timeout end + rescue KubeException => ke + app.make_errored!("Kubernetes error: #{ke.message}") unless app.errored? + end + + private + + def on_succeeded + if app.make_installed + finalize_installation + else + app.make_errored!("Failed to update app record; #{app.errors}") + end + end + + def on_failed + app.make_errored!(log || 'Installation silently failed') + finalize_installation + end + + def check_timeout + if Time.now.utc - app.updated_at.to_time.utc > ClusterWaitForAppInstallationWorker::TIMEOUT + app.make_errored!('App installation timeouted') + else + ClusterWaitForAppInstallationWorker.perform_in( + ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id) + end + end + + def finilize_installation + FinalizeInstallationService.new(app).execute + end + + def installation_phase + helm_api.installation_status(app) + end + + def installation_errors + helm_api.installation_log(app) end end end diff --git a/app/services/clusters/applications/fetch_installation_status_service.rb b/app/services/clusters/applications/fetch_installation_status_service.rb deleted file mode 100644 index 3d082485532..00000000000 --- a/app/services/clusters/applications/fetch_installation_status_service.rb +++ /dev/null @@ -1,15 +0,0 @@ -module Clusters - module Applications - class FetchInstallationStatusService < BaseHelmService - def execute - return unless app.installing? - - phase = helm_api.installation_status(app) - log = helm_api.installation_log(app) if phase == 'Failed' - yield(phase, log) if block_given? - rescue KubeException => ke - app.make_errored!("Kubernetes error: #{ke.message}") unless app.errored? - end - end - end -end diff --git a/app/services/clusters/applications/install_service.rb b/app/services/clusters/applications/install_service.rb index 7fcccb5e78c..5ed0968a98a 100644 --- a/app/services/clusters/applications/install_service.rb +++ b/app/services/clusters/applications/install_service.rb @@ -9,7 +9,7 @@ module Clusters if app.make_installing ClusterWaitForAppInstallationWorker.perform_in( - ClusterWaitForAppInstallationWorker::INITIAL_INTERVAL, app.name, app.id) + ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id) else app.make_errored!("Failed to update app record; #{app.errors}") end diff --git a/app/workers/cluster_wait_for_app_installation_worker.rb b/app/workers/cluster_wait_for_app_installation_worker.rb index d5974c467c4..548f34fc6a5 100644 --- a/app/workers/cluster_wait_for_app_installation_worker.rb +++ b/app/workers/cluster_wait_for_app_installation_worker.rb @@ -3,8 +3,7 @@ class ClusterWaitForAppInstallationWorker include ClusterQueue include ClusterApplications - INITIAL_INTERVAL = 30.seconds - EAGER_INTERVAL = 10.seconds + INTERVAL = 30.seconds TIMEOUT = 20.minutes def perform(app_name, app_id) diff --git a/lib/gitlab/kubernetes/pod.rb b/lib/gitlab/kubernetes/pod.rb new file mode 100644 index 00000000000..12a6bfcf816 --- /dev/null +++ b/lib/gitlab/kubernetes/pod.rb @@ -0,0 +1,12 @@ +module Gitlab + module Kubernetes + module Pod + PENDING = 'Pending'.freeze + RUNNING = 'Running'.freeze + SUCCEEDED = 'Succeeded'.freeze + FAILED = 'Failed'.freeze + UNKNOWN = 'Unknown'.freeze + PHASES = [PENDING, RUNNING, SUCCEEDED, FAILED, UNKNONW].freeze + end + end +end -- cgit v1.2.1 From 49210dfff12ba0fba5fdbcdc2c485fbbde43f83f Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Fri, 3 Nov 2017 11:10:50 +0100 Subject: Schedule k8s application installation with a service --- .../projects/clusters/applications_controller.rb | 14 ++++------- app/models/clusters/applications/helm.rb | 2 ++ app/models/clusters/concerns/application_status.rb | 8 +++++++ .../applications/schedule_installation_service.rb | 27 ++++++++++++++++++++++ 4 files changed, 42 insertions(+), 9 deletions(-) create mode 100644 app/services/clusters/applications/schedule_installation_service.rb diff --git a/app/controllers/projects/clusters/applications_controller.rb b/app/controllers/projects/clusters/applications_controller.rb index cdebbdefb3f..aa36ccac804 100644 --- a/app/controllers/projects/clusters/applications_controller.rb +++ b/app/controllers/projects/clusters/applications_controller.rb @@ -5,12 +5,12 @@ class Projects::Clusters::ApplicationsController < Projects::ApplicationControll before_action :authorize_create_cluster!, only: [:create] def create - return render_404 if application - respond_to do |format| format.json do - # TODO: Do that via Service - if application_class.create(cluster: cluster).persisted? + scheduled = Clusters::Applications::ScheduleInstallationService.new(project, current_user, + application_class: @application_class, + cluster: @cluster).execute + if scheduled head :no_data else head :bad_request @@ -26,10 +26,6 @@ class Projects::Clusters::ApplicationsController < Projects::ApplicationControll end def application_class - Clusters::Cluster::APPLICATIONS[params[:application]] || render_404 - end - - def application - application_class.find_by(cluster: cluster) + @application_class ||= Clusters::Cluster::APPLICATIONS[params[:application]] || render_404 end end diff --git a/app/models/clusters/applications/helm.rb b/app/models/clusters/applications/helm.rb index 77c0c61d968..54eca613ce3 100644 --- a/app/models/clusters/applications/helm.rb +++ b/app/models/clusters/applications/helm.rb @@ -11,6 +11,8 @@ module Clusters default_value_for :version, Gitlab::Kubernetes::Helm::HELM_VERSION + validates :cluster, presence: true + def name NAME end diff --git a/app/models/clusters/concerns/application_status.rb b/app/models/clusters/concerns/application_status.rb index e1f4c7fdda8..c5711fd0b58 100644 --- a/app/models/clusters/concerns/application_status.rb +++ b/app/models/clusters/concerns/application_status.rb @@ -23,6 +23,14 @@ module Clusters transition any - [:errored] => :errored end + event :make_scheduled do + transition %i(installable errored) => :scheduled + end + + before_transition any => [:scheduled] do |app_status, _| + app_status.status_reason = nil + end + before_transition any => [:errored] do |app_status, transition| status_reason = transition.args.first app_status.status_reason = status_reason if status_reason diff --git a/app/services/clusters/applications/schedule_installation_service.rb b/app/services/clusters/applications/schedule_installation_service.rb new file mode 100644 index 00000000000..17b3a09948d --- /dev/null +++ b/app/services/clusters/applications/schedule_installation_service.rb @@ -0,0 +1,27 @@ +module Clusters + module Applications + class ScheduleInstallationService < ::BaseService + def execute + application = application_class.find_or_create_by!(cluster: cluster) + + application.make_scheduled! + ClusterInstallAppWorker.perform_async(application.name, application.id) + true + rescue ActiveRecord::RecordInvalid + false + rescue StateMachines::InvalidTransition + false + end + + private + + def application_class + params[:application_class] + end + + def cluster + params[:cluster] + end + end + end +end -- cgit v1.2.1 From 44f885eff62cca07b60f8bb210733c4e0a2ea019 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Fri, 3 Nov 2017 12:23:08 +0100 Subject: Fix typo --- lib/gitlab/kubernetes/pod.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/kubernetes/pod.rb b/lib/gitlab/kubernetes/pod.rb index 12a6bfcf816..f3842cdf762 100644 --- a/lib/gitlab/kubernetes/pod.rb +++ b/lib/gitlab/kubernetes/pod.rb @@ -6,7 +6,7 @@ module Gitlab SUCCEEDED = 'Succeeded'.freeze FAILED = 'Failed'.freeze UNKNOWN = 'Unknown'.freeze - PHASES = [PENDING, RUNNING, SUCCEEDED, FAILED, UNKNONW].freeze + PHASES = [PENDING, RUNNING, SUCCEEDED, FAILED, UNKNOWN].freeze end end end -- cgit v1.2.1 From e6616e0468deaf1e37ddddc9332cc3e677567410 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Fri, 3 Nov 2017 13:55:44 +0100 Subject: Fix typos --- .../projects/clusters/applications_controller.rb | 18 +++++++----------- .../check_installation_progress_service.rb | 2 +- .../cluster_wait_for_app_installation_worker.rb | 2 +- config/routes/project.rb | 2 +- 4 files changed, 10 insertions(+), 14 deletions(-) diff --git a/app/controllers/projects/clusters/applications_controller.rb b/app/controllers/projects/clusters/applications_controller.rb index aa36ccac804..fae1ceb04b0 100644 --- a/app/controllers/projects/clusters/applications_controller.rb +++ b/app/controllers/projects/clusters/applications_controller.rb @@ -5,17 +5,13 @@ class Projects::Clusters::ApplicationsController < Projects::ApplicationControll before_action :authorize_create_cluster!, only: [:create] def create - respond_to do |format| - format.json do - scheduled = Clusters::Applications::ScheduleInstallationService.new(project, current_user, - application_class: @application_class, - cluster: @cluster).execute - if scheduled - head :no_data - else - head :bad_request - end - end + scheduled = Clusters::Applications::ScheduleInstallationService.new(project, current_user, + application_class: @application_class, + cluster: @cluster).execute + if scheduled + head :no_data + else + head :bad_request end end diff --git a/app/services/clusters/applications/check_installation_progress_service.rb b/app/services/clusters/applications/check_installation_progress_service.rb index 7f5a633b749..cf96c128c2e 100644 --- a/app/services/clusters/applications/check_installation_progress_service.rb +++ b/app/services/clusters/applications/check_installation_progress_service.rb @@ -40,7 +40,7 @@ module Clusters end end - def finilize_installation + def finalize_installation FinalizeInstallationService.new(app).execute end diff --git a/app/workers/cluster_wait_for_app_installation_worker.rb b/app/workers/cluster_wait_for_app_installation_worker.rb index 548f34fc6a5..4bb8c293e5d 100644 --- a/app/workers/cluster_wait_for_app_installation_worker.rb +++ b/app/workers/cluster_wait_for_app_installation_worker.rb @@ -3,7 +3,7 @@ class ClusterWaitForAppInstallationWorker include ClusterQueue include ClusterApplications - INTERVAL = 30.seconds + INTERVAL = 10.seconds TIMEOUT = 20.minutes def perform(app_name, app_id) diff --git a/config/routes/project.rb b/config/routes/project.rb index 55dab840dca..ee252ee2466 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -192,7 +192,7 @@ constraints(ProjectUrlConstrainer.new) do get :status, format: :json scope :applications do - get '/*application', to: 'clusters/applications#create' + post '/*application', to: 'clusters/applications#create' end end end -- cgit v1.2.1 From c0299ce49406302a26e7fd16ec272bca19715afb Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Fri, 3 Nov 2017 16:40:13 +0100 Subject: Add Projects::Clusters::ApplicationsController tests --- .../projects/clusters/applications_controller.rb | 2 +- config/routes/project.rb | 2 +- .../clusters/applications_controller_spec.rb | 73 ++++++++++++++++++++++ .../matchers/access_matchers_for_controller.rb | 2 +- 4 files changed, 76 insertions(+), 3 deletions(-) create mode 100644 spec/controllers/projects/clusters/applications_controller_spec.rb diff --git a/app/controllers/projects/clusters/applications_controller.rb b/app/controllers/projects/clusters/applications_controller.rb index fae1ceb04b0..4b9d54a8537 100644 --- a/app/controllers/projects/clusters/applications_controller.rb +++ b/app/controllers/projects/clusters/applications_controller.rb @@ -9,7 +9,7 @@ class Projects::Clusters::ApplicationsController < Projects::ApplicationControll application_class: @application_class, cluster: @cluster).execute if scheduled - head :no_data + head :no_content else head :bad_request end diff --git a/config/routes/project.rb b/config/routes/project.rb index ee252ee2466..a1e429e6c20 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -192,7 +192,7 @@ constraints(ProjectUrlConstrainer.new) do get :status, format: :json scope :applications do - post '/*application', to: 'clusters/applications#create' + post '/*application', to: 'clusters/applications#create', as: :install_applications end end end diff --git a/spec/controllers/projects/clusters/applications_controller_spec.rb b/spec/controllers/projects/clusters/applications_controller_spec.rb new file mode 100644 index 00000000000..b8464b713c4 --- /dev/null +++ b/spec/controllers/projects/clusters/applications_controller_spec.rb @@ -0,0 +1,73 @@ +require 'spec_helper' + +describe Projects::Clusters::ApplicationsController do + include AccessMatchersForController + + def current_application + Clusters::Cluster::APPLICATIONS[application] + end + + describe 'POST create' do + let(:cluster) { create(:cluster, :project, :providing_by_gcp) } + let(:project) { cluster.project } + let(:application) { 'helm' } + let(:params) { { application: application, id: cluster.id } } + + describe 'functionality' do + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + end + + it 'schedule an application installation' do + expect(ClusterInstallAppWorker).to receive(:perform_async).with(application, anything).once + + expect { go }.to change { current_application.count } + expect(response).to have_http_status(:no_content) + expect(cluster.application_helm).to be_scheduled + end + + context 'when cluster do not exists' do + before do + cluster.destroy! + end + + it 'return 404' do + expect { go }.not_to change { current_application.count } + expect(response).to have_http_status(:not_found) + end + end + + context 'when application is unknown' do + let(:application) { 'unkwnown-app' } + + it 'return 404' do + go + + expect(response).to have_http_status(:not_found) + end + end + end + + describe 'security' do + before do + allow(ClusterInstallAppWorker).to receive(:perform_async) + end + + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + + def go + post :create, params.merge(namespace_id: project.namespace, project_id: project) + end + end +end diff --git a/spec/support/matchers/access_matchers_for_controller.rb b/spec/support/matchers/access_matchers_for_controller.rb index bb6b7c63ee9..cdb62a5deee 100644 --- a/spec/support/matchers/access_matchers_for_controller.rb +++ b/spec/support/matchers/access_matchers_for_controller.rb @@ -5,7 +5,7 @@ module AccessMatchersForController extend RSpec::Matchers::DSL include Warden::Test::Helpers - EXPECTED_STATUS_CODE_ALLOWED = [200, 201, 302].freeze + EXPECTED_STATUS_CODE_ALLOWED = [200, 201, 204, 302].freeze EXPECTED_STATUS_CODE_DENIED = [401, 404].freeze def emulate_user(role, membership = nil) -- cgit v1.2.1 From 797e758beb882d67f32b87db6453cad41c0b931f Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 24 Oct 2017 07:56:39 +0300 Subject: Add applications section to GKE clusters page --- app/assets/javascripts/clusters.js | 123 ----------- app/assets/javascripts/clusters/clusters_bundle.js | 216 +++++++++++++++++++ .../clusters/components/application_row.vue | 168 +++++++++++++++ .../clusters/components/applications.vue | 105 +++++++++ app/assets/javascripts/clusters/constants.js | 10 + app/assets/javascripts/clusters/event_hub.js | 3 + .../clusters/services/clusters_service.js | 24 +++ .../javascripts/clusters/stores/clusters_store.js | 68 ++++++ app/assets/javascripts/dispatcher.js | 9 +- .../vue_shared/components/loading_button.vue | 7 +- app/assets/stylesheets/framework/buttons.scss | 1 + app/assets/stylesheets/pages/clusters.scss | 5 + app/views/projects/clusters/show.html.haml | 19 +- ...36629-35958-add-cluster-application-section.yml | 6 + .../project/clusters/img/cluster-applications.png | Bin 0 -> 38153 bytes doc/user/project/clusters/index.md | 9 + spec/javascripts/clusters/clusters_bundle_spec.js | 235 +++++++++++++++++++++ .../clusters/components/application_row_spec.js | 213 +++++++++++++++++++ .../clusters/components/applications_spec.js | 42 ++++ spec/javascripts/clusters/services/mock_data.js | 50 +++++ .../clusters/stores/clusters_store_spec.js | 86 ++++++++ spec/javascripts/clusters_spec.js | 79 ------- 22 files changed, 1268 insertions(+), 210 deletions(-) delete mode 100644 app/assets/javascripts/clusters.js create mode 100644 app/assets/javascripts/clusters/clusters_bundle.js create mode 100644 app/assets/javascripts/clusters/components/application_row.vue create mode 100644 app/assets/javascripts/clusters/components/applications.vue create mode 100644 app/assets/javascripts/clusters/constants.js create mode 100644 app/assets/javascripts/clusters/event_hub.js create mode 100644 app/assets/javascripts/clusters/services/clusters_service.js create mode 100644 app/assets/javascripts/clusters/stores/clusters_store.js create mode 100644 changelogs/unreleased/36629-35958-add-cluster-application-section.yml create mode 100644 doc/user/project/clusters/img/cluster-applications.png create mode 100644 spec/javascripts/clusters/clusters_bundle_spec.js create mode 100644 spec/javascripts/clusters/components/application_row_spec.js create mode 100644 spec/javascripts/clusters/components/applications_spec.js create mode 100644 spec/javascripts/clusters/services/mock_data.js create mode 100644 spec/javascripts/clusters/stores/clusters_store_spec.js delete mode 100644 spec/javascripts/clusters_spec.js diff --git a/app/assets/javascripts/clusters.js b/app/assets/javascripts/clusters.js deleted file mode 100644 index c9fef94efea..00000000000 --- a/app/assets/javascripts/clusters.js +++ /dev/null @@ -1,123 +0,0 @@ -/* globals Flash */ -import Visibility from 'visibilityjs'; -import axios from 'axios'; -import setAxiosCsrfToken from './lib/utils/axios_utils'; -import Poll from './lib/utils/poll'; -import { s__ } from './locale'; -import initSettingsPanels from './settings_panels'; -import Flash from './flash'; - -/** - * Cluster page has 2 separate parts: - * Toggle button - * - * - Polling status while creating or scheduled - * -- Update status area with the response result - */ - -class ClusterService { - constructor(options = {}) { - this.options = options; - setAxiosCsrfToken(); - } - fetchData() { - return axios.get(this.options.endpoint); - } -} - -export default class Clusters { - constructor() { - initSettingsPanels(); - - const dataset = document.querySelector('.js-edit-cluster-form').dataset; - - this.state = { - statusPath: dataset.statusPath, - clusterStatus: dataset.clusterStatus, - clusterStatusReason: dataset.clusterStatusReason, - toggleStatus: dataset.toggleStatus, - }; - - this.service = new ClusterService({ endpoint: this.state.statusPath }); - this.toggleButton = document.querySelector('.js-toggle-cluster'); - this.toggleInput = document.querySelector('.js-toggle-input'); - this.errorContainer = document.querySelector('.js-cluster-error'); - this.successContainer = document.querySelector('.js-cluster-success'); - this.creatingContainer = document.querySelector('.js-cluster-creating'); - this.errorReasonContainer = this.errorContainer.querySelector('.js-error-reason'); - - this.toggleButton.addEventListener('click', this.toggle.bind(this)); - - if (this.state.clusterStatus !== 'created') { - this.updateContainer(this.state.clusterStatus, this.state.clusterStatusReason); - } - - if (this.state.statusPath) { - this.initPolling(); - } - } - - toggle() { - this.toggleButton.classList.toggle('checked'); - this.toggleInput.setAttribute('value', this.toggleButton.classList.contains('checked').toString()); - } - - initPolling() { - this.poll = new Poll({ - resource: this.service, - method: 'fetchData', - successCallback: data => this.handleSuccess(data), - errorCallback: () => Clusters.handleError(), - }); - - if (!Visibility.hidden()) { - this.poll.makeRequest(); - } else { - this.service.fetchData() - .then(data => this.handleSuccess(data)) - .catch(() => Clusters.handleError()); - } - - Visibility.change(() => { - if (!Visibility.hidden()) { - this.poll.restart(); - } else { - this.poll.stop(); - } - }); - } - - static handleError() { - Flash(s__('ClusterIntegration|Something went wrong on our end.')); - } - - handleSuccess(data) { - const { status, status_reason } = data.data; - this.updateContainer(status, status_reason); - } - - hideAll() { - this.errorContainer.classList.add('hidden'); - this.successContainer.classList.add('hidden'); - this.creatingContainer.classList.add('hidden'); - } - - updateContainer(status, error) { - this.hideAll(); - switch (status) { - case 'created': - this.successContainer.classList.remove('hidden'); - break; - case 'errored': - this.errorContainer.classList.remove('hidden'); - this.errorReasonContainer.textContent = error; - break; - case 'scheduled': - case 'creating': - this.creatingContainer.classList.remove('hidden'); - break; - default: - this.hideAll(); - } - } -} diff --git a/app/assets/javascripts/clusters/clusters_bundle.js b/app/assets/javascripts/clusters/clusters_bundle.js new file mode 100644 index 00000000000..053f11cc3c4 --- /dev/null +++ b/app/assets/javascripts/clusters/clusters_bundle.js @@ -0,0 +1,216 @@ +import Visibility from 'visibilityjs'; +import Vue from 'vue'; +import { s__, sprintf } from '../locale'; +import Flash from '../flash'; +import Poll from '../lib/utils/poll'; +import initSettingsPanels from '../settings_panels'; +import eventHub from './event_hub'; +import { + APPLICATION_INSTALLED, + REQUEST_LOADING, + REQUEST_SUCCESS, + REQUEST_FAILURE, +} from './constants'; +import ClustersService from './services/clusters_service'; +import ClustersStore from './stores/clusters_store'; +import applications from './components/applications.vue'; + +/** + * Cluster page has 2 separate parts: + * Toggle button and applications section + * + * - Polling status while creating or scheduled + * - Update status area with the response result + */ + +export default class Clusters { + constructor() { + const { + statusPath, + installHelmPath, + installIngressPath, + installRunnerPath, + clusterStatus, + clusterStatusReason, + helpPath, + } = document.querySelector('.js-edit-cluster-form').dataset; + + this.store = new ClustersStore(); + this.store.setHelpPath(helpPath); + this.store.updateStatus(clusterStatus); + this.store.updateStatusReason(clusterStatusReason); + this.service = new ClustersService({ + endpoint: statusPath, + installHelmEndpoint: installHelmPath, + installIngresEndpoint: installIngressPath, + installRunnerEndpoint: installRunnerPath, + }); + + this.toggle = this.toggle.bind(this); + this.installApplication = this.installApplication.bind(this); + + this.toggleButton = document.querySelector('.js-toggle-cluster'); + this.toggleInput = document.querySelector('.js-toggle-input'); + this.errorContainer = document.querySelector('.js-cluster-error'); + this.successContainer = document.querySelector('.js-cluster-success'); + this.creatingContainer = document.querySelector('.js-cluster-creating'); + this.errorReasonContainer = this.errorContainer.querySelector('.js-error-reason'); + this.successApplicationContainer = document.querySelector('.js-cluster-application-notice'); + + initSettingsPanels(); + this.initApplications(); + + if (this.store.state.status !== 'created') { + this.updateContainer(this.store.state.status, this.store.state.statusReason); + } + + this.addListeners(); + if (statusPath) { + this.initPolling(); + } + } + + initApplications() { + const store = this.store; + const el = document.querySelector('#js-cluster-applications'); + + this.applications = new Vue({ + el, + components: { + applications, + }, + data() { + return { + state: store.state, + }; + }, + render(createElement) { + return createElement('applications', { + props: { + applications: this.state.applications, + helpPath: this.state.helpPath, + }, + }); + }, + }); + } + + addListeners() { + this.toggleButton.addEventListener('click', this.toggle); + eventHub.$on('installApplication', this.installApplication); + } + + removeListeners() { + this.toggleButton.removeEventListener('click', this.toggle); + eventHub.$off('installApplication', this.installApplication); + } + + initPolling() { + this.poll = new Poll({ + resource: this.service, + method: 'fetchData', + successCallback: data => this.handleSuccess(data), + errorCallback: () => Clusters.handleError(), + }); + + if (!Visibility.hidden()) { + this.poll.makeRequest(); + } else { + this.service.fetchData() + .then(data => this.handleSuccess(data)) + .catch(() => Clusters.handleError()); + } + + Visibility.change(() => { + if (!Visibility.hidden() && !this.destroyed) { + this.poll.restart(); + } else { + this.poll.stop(); + } + }); + } + + static handleError() { + Flash(s__('ClusterIntegration|Something went wrong on our end.')); + } + + handleSuccess(data) { + const prevApplicationMap = Object.assign({}, this.store.state.applications); + this.store.updateStateFromServer(data.data); + this.checkForNewInstalls(prevApplicationMap, this.store.state.applications); + this.updateContainer(this.store.state.status, this.store.state.statusReason); + } + + toggle() { + this.toggleButton.classList.toggle('checked'); + this.toggleInput.setAttribute('value', this.toggleButton.classList.contains('checked').toString()); + } + + hideAll() { + this.errorContainer.classList.add('hidden'); + this.successContainer.classList.add('hidden'); + this.creatingContainer.classList.add('hidden'); + } + + checkForNewInstalls(prevApplicationMap, newApplicationMap) { + const appTitles = Object.keys(newApplicationMap) + .filter(appId => newApplicationMap[appId].status === APPLICATION_INSTALLED && + prevApplicationMap[appId].status !== APPLICATION_INSTALLED && + prevApplicationMap[appId].status !== null) + .map(appId => newApplicationMap[appId].title); + + if (appTitles.length > 0) { + this.successApplicationContainer.textContent = sprintf(s__('ClusterIntegration|%{appList} was successfully installed on your cluster'), { + appList: appTitles.join(', '), + }); + this.successApplicationContainer.classList.remove('hidden'); + } else { + this.successApplicationContainer.classList.add('hidden'); + } + } + + updateContainer(status, error) { + this.hideAll(); + switch (status) { + case 'created': + this.successContainer.classList.remove('hidden'); + break; + case 'errored': + this.errorContainer.classList.remove('hidden'); + this.errorReasonContainer.textContent = error; + break; + case 'scheduled': + case 'creating': + this.creatingContainer.classList.remove('hidden'); + break; + default: + this.hideAll(); + } + } + + installApplication(appId) { + this.store.updateAppProperty(appId, 'requestStatus', REQUEST_LOADING); + this.store.updateAppProperty(appId, 'requestReason', null); + + this.service.installApplication(appId) + .then(() => { + this.store.updateAppProperty(appId, 'requestStatus', REQUEST_SUCCESS); + }) + .catch(() => { + this.store.updateAppProperty(appId, 'requestStatus', REQUEST_FAILURE); + this.store.updateAppProperty(appId, 'requestReason', s__('ClusterIntegration|Request to begin installing failed')); + }); + } + + destroy() { + this.destroyed = true; + + this.removeListeners(); + + if (this.poll) { + this.poll.stop(); + } + + this.applications.$destroy(); + } +} diff --git a/app/assets/javascripts/clusters/components/application_row.vue b/app/assets/javascripts/clusters/components/application_row.vue new file mode 100644 index 00000000000..f8d53fcc4b7 --- /dev/null +++ b/app/assets/javascripts/clusters/components/application_row.vue @@ -0,0 +1,168 @@ + + + diff --git a/app/assets/javascripts/clusters/components/applications.vue b/app/assets/javascripts/clusters/components/applications.vue new file mode 100644 index 00000000000..865fa4f702e --- /dev/null +++ b/app/assets/javascripts/clusters/components/applications.vue @@ -0,0 +1,105 @@ + + + diff --git a/app/assets/javascripts/clusters/constants.js b/app/assets/javascripts/clusters/constants.js new file mode 100644 index 00000000000..3f202435716 --- /dev/null +++ b/app/assets/javascripts/clusters/constants.js @@ -0,0 +1,10 @@ +// These need to match what is returned from the server +export const APPLICATION_INSTALLABLE = 'installable'; +export const APPLICATION_INSTALLING = 'installing'; +export const APPLICATION_INSTALLED = 'installed'; +export const APPLICATION_ERROR = 'error'; + +// These are only used client-side +export const REQUEST_LOADING = 'request-loading'; +export const REQUEST_SUCCESS = 'request-success'; +export const REQUEST_FAILURE = 'request-failure'; diff --git a/app/assets/javascripts/clusters/event_hub.js b/app/assets/javascripts/clusters/event_hub.js new file mode 100644 index 00000000000..0948c2e5352 --- /dev/null +++ b/app/assets/javascripts/clusters/event_hub.js @@ -0,0 +1,3 @@ +import Vue from 'vue'; + +export default new Vue(); diff --git a/app/assets/javascripts/clusters/services/clusters_service.js b/app/assets/javascripts/clusters/services/clusters_service.js new file mode 100644 index 00000000000..0ac8e68187d --- /dev/null +++ b/app/assets/javascripts/clusters/services/clusters_service.js @@ -0,0 +1,24 @@ +import axios from 'axios'; +import setAxiosCsrfToken from '../../lib/utils/axios_utils'; + +export default class ClusterService { + constructor(options = {}) { + setAxiosCsrfToken(); + + this.options = options; + this.appInstallEndpointMap = { + helm: this.options.installHelmEndpoint, + ingress: this.options.installIngressEndpoint, + runner: this.options.installRunnerEndpoint, + }; + } + + fetchData() { + return axios.get(this.options.endpoint); + } + + installApplication(appId) { + const endpoint = this.appInstallEndpointMap[appId]; + return axios.post(endpoint); + } +} diff --git a/app/assets/javascripts/clusters/stores/clusters_store.js b/app/assets/javascripts/clusters/stores/clusters_store.js new file mode 100644 index 00000000000..e731cdc3042 --- /dev/null +++ b/app/assets/javascripts/clusters/stores/clusters_store.js @@ -0,0 +1,68 @@ +import { s__ } from '../../locale'; + +export default class ClusterStore { + constructor() { + this.state = { + helpPath: null, + status: null, + statusReason: null, + applications: { + helm: { + title: s__('ClusterIntegration|Helm Tiller'), + status: null, + statusReason: null, + requestStatus: null, + requestReason: null, + }, + ingress: { + title: s__('ClusterIntegration|Ingress'), + status: null, + statusReason: null, + requestStatus: null, + requestReason: null, + }, + runner: { + title: s__('ClusterIntegration|GitLab Runner'), + status: null, + statusReason: null, + requestStatus: null, + requestReason: null, + }, + }, + }; + } + + setHelpPath(helpPath) { + this.state.helpPath = helpPath; + } + + updateStatus(status) { + this.state.status = status; + } + + updateStatusReason(reason) { + this.state.statusReason = reason; + } + + updateAppProperty(appId, prop, value) { + this.state.applications[appId][prop] = value; + } + + updateStateFromServer(serverState = {}) { + this.state.status = serverState.status; + this.state.statusReason = serverState.status_reason; + serverState.applications.forEach((serverAppEntry) => { + const { + name: appId, + status, + status_reason: statusReason, + } = serverAppEntry; + + this.state.applications[appId] = { + ...(this.state.applications[appId] || {}), + status, + statusReason, + }; + }); + } +} diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 760fb0cdf67..44606989395 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -1,4 +1,5 @@ /* eslint-disable func-names, space-before-function-paren, no-var, prefer-arrow-callback, wrap-iife, no-shadow, consistent-return, one-var, one-var-declaration-per-line, camelcase, default-case, no-new, quotes, no-duplicate-case, no-case-declarations, no-fallthrough, max-len */ +import { s__ } from './locale'; /* global ProjectSelect */ import IssuableIndex from './issuable_index'; /* global Milestone */ @@ -32,6 +33,7 @@ import Labels from './labels'; import LabelManager from './label_manager'; /* global Sidebar */ +import Flash from './flash'; import CommitsList from './commits'; import Issue from './issue'; import BindInOut from './behaviors/bind_in_out'; @@ -543,9 +545,12 @@ import Diff from './diff'; new DueDateSelectors(); break; case 'projects:clusters:show': - import(/* webpackChunkName: "clusters" */ './clusters') + import(/* webpackChunkName: "clusters" */ './clusters/clusters_bundle') .then(cluster => new cluster.default()) // eslint-disable-line new-cap - .catch(() => {}); + .catch((err) => { + Flash(s__('ClusterIntegration|Problem setting up the cluster JavaScript')); + throw err; + }); break; } switch (path[0]) { diff --git a/app/assets/javascripts/vue_shared/components/loading_button.vue b/app/assets/javascripts/vue_shared/components/loading_button.vue index 6670b554faf..0cc2653761c 100644 --- a/app/assets/javascripts/vue_shared/components/loading_button.vue +++ b/app/assets/javascripts/vue_shared/components/loading_button.vue @@ -26,6 +26,11 @@ export default { required: false, default: false, }, + disabled: { + type: Boolean, + required: false, + default: false, + }, label: { type: String, required: false, @@ -47,7 +52,7 @@ export default { class="btn btn-align-content" @click="onClick" type="button" - :disabled="loading" + :disabled="loading || disabled" > { + let cluster; + preloadFixtures('clusters/show_cluster.html.raw'); + + beforeEach(() => { + loadFixtures('clusters/show_cluster.html.raw'); + cluster = new Clusters(); + }); + + afterEach(() => { + cluster.destroy(); + }); + + describe('toggle', () => { + it('should update the button and the input field on click', () => { + cluster.toggleButton.click(); + + expect( + cluster.toggleButton.classList, + ).not.toContain('checked'); + + expect( + cluster.toggleInput.getAttribute('value'), + ).toEqual('false'); + }); + }); + + describe('checkForNewInstalls', () => { + const INITIAL_APP_MAP = { + helm: { status: null, title: 'Helm Tiller' }, + ingress: { status: null, title: 'Ingress' }, + runner: { status: null, title: 'GitLab Runner' }, + }; + + it('does not show alert when things transition from initial null state to something', () => { + cluster.checkForNewInstalls(INITIAL_APP_MAP, { + ...INITIAL_APP_MAP, + helm: { status: APPLICATION_INSTALLABLE, title: 'Helm Tiller' }, + }); + + expect(document.querySelector('.js-cluster-application-notice.hidden')).toBeDefined(); + }); + + it('shows an alert when something gets newly installed', () => { + cluster.checkForNewInstalls({ + ...INITIAL_APP_MAP, + helm: { status: APPLICATION_INSTALLING, title: 'Helm Tiller' }, + }, { + ...INITIAL_APP_MAP, + helm: { status: APPLICATION_INSTALLED, title: 'Helm Tiller' }, + }); + + expect(document.querySelector('.js-cluster-application-notice:not(.hidden)')).toBeDefined(); + expect(document.querySelector('.js-cluster-application-notice').textContent.trim()).toEqual('Helm Tiller was successfully installed on your cluster'); + }); + + it('shows an alert when multiple things gets newly installed', () => { + cluster.checkForNewInstalls({ + ...INITIAL_APP_MAP, + helm: { status: APPLICATION_INSTALLING, title: 'Helm Tiller' }, + ingress: { status: APPLICATION_INSTALLABLE, title: 'Ingress' }, + }, { + ...INITIAL_APP_MAP, + helm: { status: APPLICATION_INSTALLED, title: 'Helm Tiller' }, + ingress: { status: APPLICATION_INSTALLED, title: 'Ingress' }, + }); + + expect(document.querySelector('.js-cluster-application-notice:not(.hidden)')).toBeDefined(); + expect(document.querySelector('.js-cluster-application-notice').textContent.trim()).toEqual('Helm Tiller, Ingress was successfully installed on your cluster'); + }); + + it('hides existing alert when we call again and nothing is newly installed', () => { + const installedState = { + ...INITIAL_APP_MAP, + helm: { status: APPLICATION_INSTALLED, title: 'Helm Tiller' }, + }; + + // Show the banner + cluster.checkForNewInstalls({ + ...INITIAL_APP_MAP, + helm: { status: APPLICATION_INSTALLING, title: 'Helm Tiller' }, + }, installedState); + + expect(document.querySelector('.js-cluster-application-notice:not(.hidden)')).toBeDefined(); + + // Banner should go back hidden + cluster.checkForNewInstalls(installedState, installedState); + + expect(document.querySelector('.js-cluster-application-notice.hidden')).toBeDefined(); + }); + }); + + describe('updateContainer', () => { + describe('when creating cluster', () => { + it('should show the creating container', () => { + cluster.updateContainer('creating'); + + expect( + cluster.creatingContainer.classList.contains('hidden'), + ).toBeFalsy(); + expect( + cluster.successContainer.classList.contains('hidden'), + ).toBeTruthy(); + expect( + cluster.errorContainer.classList.contains('hidden'), + ).toBeTruthy(); + }); + }); + + describe('when cluster is created', () => { + it('should show the success container', () => { + cluster.updateContainer('created'); + + expect( + cluster.creatingContainer.classList.contains('hidden'), + ).toBeTruthy(); + expect( + cluster.successContainer.classList.contains('hidden'), + ).toBeFalsy(); + expect( + cluster.errorContainer.classList.contains('hidden'), + ).toBeTruthy(); + }); + }); + + describe('when cluster has error', () => { + it('should show the error container', () => { + cluster.updateContainer('errored', 'this is an error'); + + expect( + cluster.creatingContainer.classList.contains('hidden'), + ).toBeTruthy(); + expect( + cluster.successContainer.classList.contains('hidden'), + ).toBeTruthy(); + expect( + cluster.errorContainer.classList.contains('hidden'), + ).toBeFalsy(); + + expect( + cluster.errorReasonContainer.textContent, + ).toContain('this is an error'); + }); + }); + }); + + describe('installApplication', () => { + it('tries to install helm', (done) => { + spyOn(cluster.service, 'installApplication').and.returnValue(Promise.resolve()); + expect(cluster.store.state.applications.helm.requestStatus).toEqual(null); + + cluster.installApplication('helm'); + + expect(cluster.store.state.applications.helm.requestStatus).toEqual(REQUEST_LOADING); + expect(cluster.store.state.applications.helm.requestReason).toEqual(null); + expect(cluster.service.installApplication).toHaveBeenCalledWith('helm'); + + getSetTimeoutPromise() + .then(() => { + expect(cluster.store.state.applications.helm.requestStatus).toEqual(REQUEST_SUCCESS); + expect(cluster.store.state.applications.helm.requestReason).toEqual(null); + }) + .then(done) + .catch(done.fail); + }); + + it('tries to install ingress', (done) => { + spyOn(cluster.service, 'installApplication').and.returnValue(Promise.resolve()); + expect(cluster.store.state.applications.ingress.requestStatus).toEqual(null); + + cluster.installApplication('ingress'); + + expect(cluster.store.state.applications.ingress.requestStatus).toEqual(REQUEST_LOADING); + expect(cluster.store.state.applications.ingress.requestReason).toEqual(null); + expect(cluster.service.installApplication).toHaveBeenCalledWith('ingress'); + + getSetTimeoutPromise() + .then(() => { + expect(cluster.store.state.applications.ingress.requestStatus).toEqual(REQUEST_SUCCESS); + expect(cluster.store.state.applications.ingress.requestReason).toEqual(null); + }) + .then(done) + .catch(done.fail); + }); + + it('tries to install runner', (done) => { + spyOn(cluster.service, 'installApplication').and.returnValue(Promise.resolve()); + expect(cluster.store.state.applications.runner.requestStatus).toEqual(null); + + cluster.installApplication('runner'); + + expect(cluster.store.state.applications.runner.requestStatus).toEqual(REQUEST_LOADING); + expect(cluster.store.state.applications.runner.requestReason).toEqual(null); + expect(cluster.service.installApplication).toHaveBeenCalledWith('runner'); + + getSetTimeoutPromise() + .then(() => { + expect(cluster.store.state.applications.runner.requestStatus).toEqual(REQUEST_SUCCESS); + expect(cluster.store.state.applications.runner.requestReason).toEqual(null); + }) + .then(done) + .catch(done.fail); + }); + + it('sets error request status when the request fails', (done) => { + spyOn(cluster.service, 'installApplication').and.returnValue(Promise.reject(new Error('STUBBED ERROR'))); + expect(cluster.store.state.applications.helm.requestStatus).toEqual(null); + + cluster.installApplication('helm'); + + expect(cluster.store.state.applications.helm.requestStatus).toEqual(REQUEST_LOADING); + expect(cluster.store.state.applications.helm.requestReason).toEqual(null); + expect(cluster.service.installApplication).toHaveBeenCalled(); + + getSetTimeoutPromise() + .then(() => { + expect(cluster.store.state.applications.helm.requestStatus).toEqual(REQUEST_FAILURE); + expect(cluster.store.state.applications.helm.requestReason).toBeDefined(); + }) + .then(done) + .catch(done.fail); + }); + }); +}); diff --git a/spec/javascripts/clusters/components/application_row_spec.js b/spec/javascripts/clusters/components/application_row_spec.js new file mode 100644 index 00000000000..ba38ed6f180 --- /dev/null +++ b/spec/javascripts/clusters/components/application_row_spec.js @@ -0,0 +1,213 @@ +import Vue from 'vue'; +import eventHub from '~/clusters/event_hub'; +import { + APPLICATION_INSTALLABLE, + APPLICATION_INSTALLING, + APPLICATION_INSTALLED, + APPLICATION_ERROR, + REQUEST_LOADING, + REQUEST_SUCCESS, + REQUEST_FAILURE, +} from '~/clusters/constants'; +import applicationRow from '~/clusters/components/application_row.vue'; +import mountComponent from '../../helpers/vue_mount_component_helper'; +import { DEFAULT_APPLICATION_STATE } from '../services/mock_data'; + +describe('Application Row', () => { + let vm; + let ApplicationRow; + + beforeEach(() => { + ApplicationRow = Vue.extend(applicationRow); + }); + + afterEach(() => { + vm.$destroy(); + }); + + describe('Title', () => { + it('shows title', () => { + vm = mountComponent(ApplicationRow, { + ...DEFAULT_APPLICATION_STATE, + titleLink: null, + }); + const title = vm.$el.querySelector('.js-cluster-application-title'); + + expect(title.tagName).toEqual('SPAN'); + expect(title.textContent.trim()).toEqual(DEFAULT_APPLICATION_STATE.title); + }); + + it('shows title link', () => { + expect(DEFAULT_APPLICATION_STATE.titleLink).toBeDefined(); + + vm = mountComponent(ApplicationRow, { + ...DEFAULT_APPLICATION_STATE, + }); + const title = vm.$el.querySelector('.js-cluster-application-title'); + + expect(title.tagName).toEqual('A'); + expect(title.textContent.trim()).toEqual(DEFAULT_APPLICATION_STATE.title); + }); + }); + + describe('Install button', () => { + it('has indeterminate state on page load', () => { + vm = mountComponent(ApplicationRow, { + ...DEFAULT_APPLICATION_STATE, + status: null, + }); + + expect(vm.installButtonLabel).toBeUndefined(); + }); + + it('has enabled "Install" when `status=installable`', () => { + vm = mountComponent(ApplicationRow, { + ...DEFAULT_APPLICATION_STATE, + status: APPLICATION_INSTALLABLE, + }); + + expect(vm.installButtonLabel).toEqual('Install'); + expect(vm.installButtonLoading).toEqual(false); + expect(vm.installButtonDisabled).toEqual(false); + }); + + it('has loading "Installing" when `status=installing`', () => { + vm = mountComponent(ApplicationRow, { + ...DEFAULT_APPLICATION_STATE, + status: APPLICATION_INSTALLING, + }); + + expect(vm.installButtonLabel).toEqual('Installing'); + expect(vm.installButtonLoading).toEqual(true); + expect(vm.installButtonDisabled).toEqual(true); + }); + + it('has disabled "Installed" when `status=installed`', () => { + vm = mountComponent(ApplicationRow, { + ...DEFAULT_APPLICATION_STATE, + status: APPLICATION_INSTALLED, + }); + + expect(vm.installButtonLabel).toEqual('Installed'); + expect(vm.installButtonLoading).toEqual(false); + expect(vm.installButtonDisabled).toEqual(true); + }); + + it('has disabled "Install" when `status=error`', () => { + vm = mountComponent(ApplicationRow, { + ...DEFAULT_APPLICATION_STATE, + status: APPLICATION_ERROR, + }); + + expect(vm.installButtonLabel).toEqual('Install'); + expect(vm.installButtonLoading).toEqual(false); + expect(vm.installButtonDisabled).toEqual(true); + }); + + it('has loading "Install" when `requestStatus=loading`', () => { + vm = mountComponent(ApplicationRow, { + ...DEFAULT_APPLICATION_STATE, + status: APPLICATION_INSTALLABLE, + requestStatus: REQUEST_LOADING, + }); + + expect(vm.installButtonLabel).toEqual('Install'); + expect(vm.installButtonLoading).toEqual(true); + expect(vm.installButtonDisabled).toEqual(true); + }); + + it('has disabled "Install" when `requestStatus=success`', () => { + vm = mountComponent(ApplicationRow, { + ...DEFAULT_APPLICATION_STATE, + status: APPLICATION_INSTALLABLE, + requestStatus: REQUEST_SUCCESS, + }); + + expect(vm.installButtonLabel).toEqual('Install'); + expect(vm.installButtonLoading).toEqual(false); + expect(vm.installButtonDisabled).toEqual(true); + }); + + it('has enabled "Install" when `requestStatus=error` (so you can try installing again)', () => { + vm = mountComponent(ApplicationRow, { + ...DEFAULT_APPLICATION_STATE, + status: APPLICATION_INSTALLABLE, + requestStatus: REQUEST_FAILURE, + }); + + expect(vm.installButtonLabel).toEqual('Install'); + expect(vm.installButtonLoading).toEqual(false); + expect(vm.installButtonDisabled).toEqual(false); + }); + + it('clicking install button emits event', () => { + spyOn(eventHub, '$emit'); + vm = mountComponent(ApplicationRow, { + ...DEFAULT_APPLICATION_STATE, + status: APPLICATION_INSTALLABLE, + }); + const installButton = vm.$el.querySelector('.js-cluster-application-install-button'); + + installButton.click(); + + expect(eventHub.$emit).toHaveBeenCalledWith('installApplication', DEFAULT_APPLICATION_STATE.id); + }); + + it('clicking disabled install button emits nothing', () => { + spyOn(eventHub, '$emit'); + vm = mountComponent(ApplicationRow, { + ...DEFAULT_APPLICATION_STATE, + status: APPLICATION_INSTALLING, + }); + const installButton = vm.$el.querySelector('.js-cluster-application-install-button'); + + expect(vm.installButtonDisabled).toEqual(true); + + installButton.click(); + + expect(eventHub.$emit).not.toHaveBeenCalled(); + }); + }); + + describe('Error block', () => { + it('does not show error block when there is no error', () => { + vm = mountComponent(ApplicationRow, { + ...DEFAULT_APPLICATION_STATE, + status: null, + requestStatus: null, + }); + const generalErrorMessage = vm.$el.querySelector('.js-cluster-application-general-error-message'); + + expect(generalErrorMessage).toBeNull(); + }); + + it('shows status reason when `status=error`', () => { + const statusReason = 'We broke it 0.0'; + vm = mountComponent(ApplicationRow, { + ...DEFAULT_APPLICATION_STATE, + status: APPLICATION_ERROR, + statusReason, + }); + const generalErrorMessage = vm.$el.querySelector('.js-cluster-application-general-error-message'); + const statusErrorMessage = vm.$el.querySelector('.js-cluster-application-status-error-message'); + + expect(generalErrorMessage.textContent.trim()).toEqual(`Something went wrong while installing ${DEFAULT_APPLICATION_STATE.title}`); + expect(statusErrorMessage.textContent.trim()).toEqual(statusReason); + }); + + it('shows request reason when `requestStatus=error`', () => { + const requestReason = 'We broke thre request 0.0'; + vm = mountComponent(ApplicationRow, { + ...DEFAULT_APPLICATION_STATE, + status: APPLICATION_INSTALLABLE, + requestStatus: REQUEST_FAILURE, + requestReason, + }); + const generalErrorMessage = vm.$el.querySelector('.js-cluster-application-general-error-message'); + const requestErrorMessage = vm.$el.querySelector('.js-cluster-application-request-error-message'); + + expect(generalErrorMessage.textContent.trim()).toEqual(`Something went wrong while installing ${DEFAULT_APPLICATION_STATE.title}`); + expect(requestErrorMessage.textContent.trim()).toEqual(requestReason); + }); + }); +}); diff --git a/spec/javascripts/clusters/components/applications_spec.js b/spec/javascripts/clusters/components/applications_spec.js new file mode 100644 index 00000000000..5f59a00dc65 --- /dev/null +++ b/spec/javascripts/clusters/components/applications_spec.js @@ -0,0 +1,42 @@ +import Vue from 'vue'; +import applications from '~/clusters/components/applications.vue'; +import mountComponent from '../../helpers/vue_mount_component_helper'; + +describe('Applications', () => { + let vm; + let Applications; + + beforeEach(() => { + Applications = Vue.extend(applications); + }); + + afterEach(() => { + vm.$destroy(); + }); + + describe('', () => { + beforeEach(() => { + vm = mountComponent(Applications, { + applications: { + helm: { title: 'Helm Tiller' }, + ingress: { title: 'Ingress' }, + runner: { title: 'GitLab Runner' }, + }, + }); + }); + + it('renders a row for Helm Tiller', () => { + expect(vm.$el.querySelector('.js-cluster-application-row-helm')).toBeDefined(); + }); + + /* * / + it('renders a row for Ingress', () => { + expect(vm.$el.querySelector('.js-cluster-application-row-ingress')).toBeDefined(); + }); + + it('renders a row for GitLab Runner', () => { + expect(vm.$el.querySelector('.js-cluster-application-row-runner')).toBeDefined(); + }); + /* */ + }); +}); diff --git a/spec/javascripts/clusters/services/mock_data.js b/spec/javascripts/clusters/services/mock_data.js new file mode 100644 index 00000000000..af6b6a73819 --- /dev/null +++ b/spec/javascripts/clusters/services/mock_data.js @@ -0,0 +1,50 @@ +import { + APPLICATION_INSTALLABLE, + APPLICATION_INSTALLING, + APPLICATION_ERROR, +} from '~/clusters/constants'; + +const CLUSTERS_MOCK_DATA = { + GET: { + '/gitlab-org/gitlab-shell/clusters/1/status.json': { + data: { + status: 'errored', + status_reason: 'Failed to request to CloudPlatform.', + applications: [{ + name: 'helm', + status: APPLICATION_INSTALLABLE, + status_reason: null, + }, { + name: 'ingress', + status: APPLICATION_ERROR, + status_reason: 'Cannot connect', + }, { + name: 'runner', + status: APPLICATION_INSTALLING, + status_reason: null, + }], + }, + }, + }, + POST: { + '/gitlab-org/gitlab-shell/clusters/1/applications/helm': { }, + '/gitlab-org/gitlab-shell/clusters/1/applications/ingress': { }, + '/gitlab-org/gitlab-shell/clusters/1/applications/runner': { }, + }, +}; + +const DEFAULT_APPLICATION_STATE = { + id: 'some-app', + title: 'My App', + titleLink: 'https://about.gitlab.com/', + description: 'Some description about this interesting application!', + status: null, + statusReason: null, + requestStatus: null, + requestReason: null, +}; + +export { + CLUSTERS_MOCK_DATA, + DEFAULT_APPLICATION_STATE, +}; diff --git a/spec/javascripts/clusters/stores/clusters_store_spec.js b/spec/javascripts/clusters/stores/clusters_store_spec.js new file mode 100644 index 00000000000..9f9d63434f7 --- /dev/null +++ b/spec/javascripts/clusters/stores/clusters_store_spec.js @@ -0,0 +1,86 @@ +import ClustersStore from '~/clusters/stores/clusters_store'; +import { APPLICATION_INSTALLING } from '~/clusters/constants'; +import { CLUSTERS_MOCK_DATA } from '../services/mock_data'; + +describe('Clusters Store', () => { + let store; + + beforeEach(() => { + store = new ClustersStore(); + }); + + describe('updateStatus', () => { + it('should store new status', () => { + expect(store.state.status).toEqual(null); + + const newStatus = 'errored'; + store.updateStatus(newStatus); + + expect(store.state.status).toEqual(newStatus); + }); + }); + + describe('updateStatusReason', () => { + it('should store new reason', () => { + expect(store.state.statusReason).toEqual(null); + + const newReason = 'Something went wrong!'; + store.updateStatusReason(newReason); + + expect(store.state.statusReason).toEqual(newReason); + }); + }); + + describe('updateAppProperty', () => { + it('should store new request status', () => { + expect(store.state.applications.helm.requestStatus).toEqual(null); + + const newStatus = APPLICATION_INSTALLING; + store.updateAppProperty('helm', 'requestStatus', newStatus); + + expect(store.state.applications.helm.requestStatus).toEqual(newStatus); + }); + + it('should store new request reason', () => { + expect(store.state.applications.helm.requestReason).toEqual(null); + + const newReason = 'We broke it.'; + store.updateAppProperty('helm', 'requestReason', newReason); + + expect(store.state.applications.helm.requestReason).toEqual(newReason); + }); + }); + + describe('updateStateFromServer', () => { + it('should store new polling data from server', () => { + const mockResponseData = CLUSTERS_MOCK_DATA.GET['/gitlab-org/gitlab-shell/clusters/1/status.json'].data; + store.updateStateFromServer(mockResponseData); + + expect(store.state).toEqual({ + helpPath: null, + status: mockResponseData.status, + statusReason: mockResponseData.status_reason, + applications: { + helm: { + status: mockResponseData.applications[0].status, + statusReason: mockResponseData.applications[0].status_reason, + requestStatus: null, + requestReason: null, + }, + ingress: { + status: mockResponseData.applications[1].status, + statusReason: mockResponseData.applications[1].status_reason, + requestStatus: null, + requestReason: null, + }, + runner: { + status: mockResponseData.applications[2].status, + statusReason: mockResponseData.applications[2].status_reason, + requestStatus: null, + requestReason: null, + }, + }, + }); + }); + }); +}); diff --git a/spec/javascripts/clusters_spec.js b/spec/javascripts/clusters_spec.js deleted file mode 100644 index eb1cd6eb804..00000000000 --- a/spec/javascripts/clusters_spec.js +++ /dev/null @@ -1,79 +0,0 @@ -import Clusters from '~/clusters'; - -describe('Clusters', () => { - let cluster; - preloadFixtures('clusters/show_cluster.html.raw'); - - beforeEach(() => { - loadFixtures('clusters/show_cluster.html.raw'); - cluster = new Clusters(); - }); - - describe('toggle', () => { - it('should update the button and the input field on click', () => { - cluster.toggleButton.click(); - - expect( - cluster.toggleButton.classList, - ).not.toContain('checked'); - - expect( - cluster.toggleInput.getAttribute('value'), - ).toEqual('false'); - }); - }); - - describe('updateContainer', () => { - describe('when creating cluster', () => { - it('should show the creating container', () => { - cluster.updateContainer('creating'); - - expect( - cluster.creatingContainer.classList.contains('hidden'), - ).toBeFalsy(); - expect( - cluster.successContainer.classList.contains('hidden'), - ).toBeTruthy(); - expect( - cluster.errorContainer.classList.contains('hidden'), - ).toBeTruthy(); - }); - }); - - describe('when cluster is created', () => { - it('should show the success container', () => { - cluster.updateContainer('created'); - - expect( - cluster.creatingContainer.classList.contains('hidden'), - ).toBeTruthy(); - expect( - cluster.successContainer.classList.contains('hidden'), - ).toBeFalsy(); - expect( - cluster.errorContainer.classList.contains('hidden'), - ).toBeTruthy(); - }); - }); - - describe('when cluster has error', () => { - it('should show the error container', () => { - cluster.updateContainer('errored', 'this is an error'); - - expect( - cluster.creatingContainer.classList.contains('hidden'), - ).toBeTruthy(); - expect( - cluster.successContainer.classList.contains('hidden'), - ).toBeTruthy(); - expect( - cluster.errorContainer.classList.contains('hidden'), - ).toBeFalsy(); - - expect( - cluster.errorReasonContainer.textContent, - ).toContain('this is an error'); - }); - }); - }); -}); -- cgit v1.2.1 From 6947646d00a7a71984d54087452b0d004429b047 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 3 Nov 2017 11:33:15 -0500 Subject: Add Ingress to cluster applications section --- app/assets/javascripts/clusters/components/applications.vue | 11 ++++++++++- app/assets/stylesheets/pages/clusters.scss | 2 +- spec/javascripts/clusters/components/applications_spec.js | 2 +- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/clusters/components/applications.vue b/app/assets/javascripts/clusters/components/applications.vue index 865fa4f702e..4b2fc069cee 100644 --- a/app/assets/javascripts/clusters/components/applications.vue +++ b/app/assets/javascripts/clusters/components/applications.vue @@ -96,8 +96,17 @@ export default { :request-status="applications.helm.requestStatus" :request-reason="applications.helm.requestReason" /> + - diff --git a/app/assets/stylesheets/pages/clusters.scss b/app/assets/stylesheets/pages/clusters.scss index b9bfae9356c..e5b9e1f2de6 100644 --- a/app/assets/stylesheets/pages/clusters.scss +++ b/app/assets/stylesheets/pages/clusters.scss @@ -6,5 +6,5 @@ .cluster-applications-table { // Wait for the Vue to kick-in and render the applications block - min-height: 179px; + min-height: 302px; } diff --git a/spec/javascripts/clusters/components/applications_spec.js b/spec/javascripts/clusters/components/applications_spec.js index 5f59a00dc65..7460da031c4 100644 --- a/spec/javascripts/clusters/components/applications_spec.js +++ b/spec/javascripts/clusters/components/applications_spec.js @@ -29,11 +29,11 @@ describe('Applications', () => { expect(vm.$el.querySelector('.js-cluster-application-row-helm')).toBeDefined(); }); - /* * / it('renders a row for Ingress', () => { expect(vm.$el.querySelector('.js-cluster-application-row-ingress')).toBeDefined(); }); + /* * / it('renders a row for GitLab Runner', () => { expect(vm.$el.querySelector('.js-cluster-application-row-runner')).toBeDefined(); }); -- cgit v1.2.1 From c6c9b37b1d1c9304b0eef530adb4d32178adae16 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Fri, 3 Nov 2017 19:20:29 +0100 Subject: Add Clusters::Applications::Helm tests --- app/models/clusters/applications/helm.rb | 8 ++- app/models/clusters/cluster.rb | 2 +- app/models/clusters/concerns/application_status.rb | 2 +- spec/factories/clusters/applications/helm.rb | 35 +++++++++ spec/models/clusters/applications/helm_spec.rb | 82 ++++++++++++++++++++-- 5 files changed, 118 insertions(+), 11 deletions(-) create mode 100644 spec/factories/clusters/applications/helm.rb diff --git a/app/models/clusters/applications/helm.rb b/app/models/clusters/applications/helm.rb index 54eca613ce3..42626a50175 100644 --- a/app/models/clusters/applications/helm.rb +++ b/app/models/clusters/applications/helm.rb @@ -3,8 +3,6 @@ module Clusters class Helm < ActiveRecord::Base self.table_name = 'clusters_applications_helm' - NAME = 'helm'.freeze - include ::Clusters::Concerns::ApplicationStatus belongs_to :cluster, class_name: 'Clusters::Cluster', foreign_key: :cluster_id @@ -13,8 +11,12 @@ module Clusters validates :cluster, presence: true + def self.application_name + self.to_s.demodulize.underscore + end + def name - NAME + self.class.application_name end end end diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 242bae4eb3e..7d0be3d3739 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -5,7 +5,7 @@ module Clusters self.table_name = 'clusters' APPLICATIONS = { - Clusters::Applications::Helm::NAME => Clusters::Applications::Helm + Applications::Helm.application_name => Applications::Helm }.freeze belongs_to :user diff --git a/app/models/clusters/concerns/application_status.rb b/app/models/clusters/concerns/application_status.rb index c5711fd0b58..7bb68d75224 100644 --- a/app/models/clusters/concerns/application_status.rb +++ b/app/models/clusters/concerns/application_status.rb @@ -24,7 +24,7 @@ module Clusters end event :make_scheduled do - transition %i(installable errored) => :scheduled + transition any - [:scheduled] => :scheduled end before_transition any => [:scheduled] do |app_status, _| diff --git a/spec/factories/clusters/applications/helm.rb b/spec/factories/clusters/applications/helm.rb new file mode 100644 index 00000000000..968a6a1a007 --- /dev/null +++ b/spec/factories/clusters/applications/helm.rb @@ -0,0 +1,35 @@ +FactoryGirl.define do + factory :applications_helm, class: Clusters::Applications::Helm do + trait :cluster do + before(:create) do |app, _| + app.cluster = create(:cluster) + end + end + + trait :installable do + cluster + status 0 + end + + trait :scheduled do + cluster + status 1 + end + + trait :installing do + cluster + status 2 + end + + trait :installed do + cluster + status 3 + end + + trait :errored do + cluster + status(-1) + status_reason 'something went wrong' + end + end +end diff --git a/spec/models/clusters/applications/helm_spec.rb b/spec/models/clusters/applications/helm_spec.rb index 6e32e2e7037..54fce7d886a 100644 --- a/spec/models/clusters/applications/helm_spec.rb +++ b/spec/models/clusters/applications/helm_spec.rb @@ -1,14 +1,84 @@ require 'rails_helper' -require_relative '../kubernetes_spec' RSpec.describe Clusters::Applications::Helm, type: :model do - it_behaves_like 'a registered kubernetes app' + it { is_expected.to belong_to(:cluster) } + it { is_expected.to validate_presence_of(:cluster) } - it { is_expected.to belong_to(:kubernetes_service) } + describe '#name' do + it 'is .application_name' do + expect(subject.name).to eq(described_class.application_name) + end + + it 'is recorded in Clusters::Cluster::APPLICATIONS' do + expect(Clusters::Cluster::APPLICATIONS[subject.name]).to eq(described_class) + end + end + + describe '#version' do + it 'defaults to Gitlab::Kubernetes::Helm::HELM_VERSION' do + expect(subject.version).to eq(Gitlab::Kubernetes::Helm::HELM_VERSION) + end + end + + describe '#status' do + it 'defaults to :installable' do + expect(subject.status_name).to be(:installable) + end + end + + describe 'status state machine' do + describe '#make_installing' do + subject { create(:applications_helm, :scheduled) } + + it 'is installing' do + subject.make_installing! + + expect(subject).to be_installing + end + end + + describe '#make_installed' do + subject { create(:applications_helm, :installing) } + + it 'is installed' do + subject.make_installed + + expect(subject).to be_installed + end + end + + describe '#make_errored' do + subject { create(:applications_helm, :installing) } + let(:reason) { 'some errors' } + + it 'is errored' do + subject.make_errored(reason) + + expect(subject).to be_errored + expect(subject.status_reason).to eq(reason) + end + end + + describe '#make_scheduled' do + subject { create(:applications_helm, :installable) } + + it 'is scheduled' do + subject.make_scheduled + + expect(subject).to be_scheduled + end + + describe 'when was errored' do + subject { create(:applications_helm, :errored) } + + it 'clears #status_reason' do + expect(subject.status_reason).not_to be_nil + + subject.make_scheduled! - describe '#cluster' do - it 'is an alias to #kubernetes_service' do - expect(subject.method(:cluster).original_name).to eq(:kubernetes_service) + expect(subject.status_reason).to be_nil + end + end end end end -- cgit v1.2.1 From 70b8f421ae6587d2a57db40e7487bf2dd788cbde Mon Sep 17 00:00:00 2001 From: Mayra Cabrera Date: Sat, 4 Nov 2017 14:51:26 -0400 Subject: Modifies commit branches section - Display the default branch (if the limit is not exceeded) - Requires '...' to be clicked before showing the rest of the branches and/or tags - Fixes haml lints --- app/views/projects/commit/_commit_box.html.haml | 2 +- .../commit/_limit_exceeded_message.html.haml | 4 +- app/views/projects/commit/branches.html.haml | 18 ++++-- .../projects/commit/branches.html.haml_spec.rb | 73 +++++++++++++++++++++- 4 files changed, 85 insertions(+), 12 deletions(-) diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index cd93886bb24..b1beb2ff368 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -61,7 +61,7 @@ %span.cgray= n_('parent', 'parents', @commit.parents.count) - @commit.parents.each do |parent| = link_to parent.short_id, project_commit_path(@project, parent), class: "commit-sha" - %div.commit-info.branches + .commit-info.branches %i.fa.fa-spinner.fa-spin - if @commit.last_pipeline diff --git a/app/views/projects/commit/_limit_exceeded_message.html.haml b/app/views/projects/commit/_limit_exceeded_message.html.haml index b3372108321..430ac6df2e4 100644 --- a/app/views/projects/commit/_limit_exceeded_message.html.haml +++ b/app/views/projects/commit/_limit_exceeded_message.html.haml @@ -1,8 +1,8 @@ -.has-tooltip{class: "limit-box limit-box-#{objects.singularize}", data: { title: "Project has too many #{objects} to search"} } +.has-tooltip{ class: "limit-box limit-box-#{objects.singularize}", data: { title: "Project has too many #{objects} to search"} } .limit-icon - if objects == "branches" = icon('code-fork') - else = icon('tag') .limit-message - %span= "#{objects.capitalize} unavailable" + %span #{objects.capitalize} unavailable diff --git a/app/views/projects/commit/branches.html.haml b/app/views/projects/commit/branches.html.haml index d282eb47914..1c4f16b5afa 100644 --- a/app/views/projects/commit/branches.html.haml +++ b/app/views/projects/commit/branches.html.haml @@ -1,9 +1,15 @@ - if @branches_limit_exceeded = render 'limit_exceeded_message', objects: 'branches' -- elsif @branches.any? - = commit_branches_links(@project, @branches) +- else + - branch = commit_default_branch(@project, @branches) + = commit_branch_link(project_ref_path(@project, branch), branch) -- if @tags_limit_exceeded - = render 'limit_exceeded_message', objects: 'tags' -- elsif @tags.any? - = commit_tags_links(@project, @tags) +- if @branches.any? || @tags.any? || @tags_limit_exceeded + %span + = link_to "…", "#", class: "js-details-expand label label-gray" + %span.js-details-content.hide + = commit_branches_links(@project, @branches) + - if @tags_limit_exceeded + = render 'limit_exceeded_message', objects: 'tags' + - else + = commit_tags_links(@project, @tags) diff --git a/spec/views/projects/commit/branches.html.haml_spec.rb b/spec/views/projects/commit/branches.html.haml_spec.rb index c9112ede334..044b73ce419 100644 --- a/spec/views/projects/commit/branches.html.haml_spec.rb +++ b/spec/views/projects/commit/branches.html.haml_spec.rb @@ -7,7 +7,7 @@ describe 'projects/commit/branches.html.haml' do assign(:project, project) end - context 'branches and tags' do + context 'when branches and tags are available' do before do assign(:branches, ['master', 'test-branch']) assign(:branches_limit_exceeded, false) @@ -17,16 +17,75 @@ describe 'projects/commit/branches.html.haml' do render end + it 'shows default branch' do + expect(rendered).to have_link('master') + end + + it 'shows js expand link' do + expect(rendered).to have_selector('.js-details-expand') + end + it 'shows branch and tag links' do + expect(rendered).to have_link('test-branch') + expect(rendered).to have_link('tag1') + end + end + + context 'when branches are available but no tags' do + before do + assign(:branches, ['master', 'test-branch']) + assign(:branches_limit_exceeded, false) + assign(:tags, []) + assign(:tags_limit_exceeded, true) + + render + end + + it 'shows branches' do expect(rendered).to have_link('master') expect(rendered).to have_link('test-branch') + end + + it 'shows js expand link' do + expect(rendered).to have_selector('.js-details-expand') + end + + it 'shows limit exceeded message for tags' do + expect(rendered).to have_text('Tags unavailable') + end + end + + context 'when tags are available but no branches (just default)' do + before do + assign(:branches, ['master']) + assign(:branches_limit_exceeded, true) + assign(:tags, ['tag1', 'tag2']) + assign(:tags_limit_exceeded, false) + + render + end + + it 'shows default branch' do + expect(rendered).to have_text('master') + end + + it 'shows js expand link' do + expect(rendered).to have_selector('.js-details-expand') + end + + it 'shows tags' do expect(rendered).to have_link('tag1') + expect(rendered).to have_link('tag2') + end + + it 'shows limit exceeded for branches' do + expect(rendered).to have_text('Branches unavailable') end end - context 'throttled branches and tags' do + context 'when branches and tags are not available' do before do - assign(:branches, []) + assign(:branches, ['master']) assign(:branches_limit_exceeded, true) assign(:tags, []) assign(:tags_limit_exceeded, true) @@ -34,6 +93,14 @@ describe 'projects/commit/branches.html.haml' do render end + it 'shows default branch' do + expect(rendered).to have_text('master') + end + + it 'shows js expand link' do + expect(rendered).to have_selector('.js-details-expand') + end + it 'shows too many to search' do expect(rendered).to have_text('Branches unavailable') expect(rendered).to have_text('Tags unavailable') -- cgit v1.2.1 From 8efbba33852d57c08507cbe3a1654dd9de9dae73 Mon Sep 17 00:00:00 2001 From: Hazel Date: Mon, 6 Nov 2017 15:07:45 +0800 Subject: Changed the order of buttons from left to right --- doc/development/ux_guide/components.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/development/ux_guide/components.md b/doc/development/ux_guide/components.md index 82168b18860..16a811dbc74 100644 --- a/doc/development/ux_guide/components.md +++ b/doc/development/ux_guide/components.md @@ -256,6 +256,7 @@ Skeleton loading can replace any existing UI elements for the period in which th --- ## Dialog modals + Dialog modals are only used for having a conversation and confirmation with the user. The user is not able to access the features on the main page until closing the modal. ### Usage @@ -268,7 +269,7 @@ Dialog modals are only used for having a conversation and confirmation with the * Dialog modals contain the header, body, and actions. * **Header(1):** The header title is a question instead of a descriptive phrase. * **Body(2):** The content in body should never be ambiguous and unclear. It provides specific information. - * **Actions(3):** Contains a affirmative action, a dismissive action, and an extra action. The order of actions from right to left: Affirmative action → Extra action → Dismissive action + * **Actions(3):** Contains a affirmative action, a dismissive action, and an extra action. The order of actions from left to right: Dismissive action → Extra action → Affirmative action * Confirmations regarding labels should keep labeling styling. * References to commits, branches, and tags should be **monospaced**. -- cgit v1.2.1 From 80b0834ae96480202678d8ca1e19c0ee4abf9001 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Mon, 6 Nov 2017 10:23:15 +0100 Subject: Add Clusters::Appplications::CheckInstallationProgressService tests --- .../check_installation_progress_service.rb | 4 +- spec/factories/clusters/applications/helm.rb | 5 ++ .../check_installation_progress_service_spec.rb | 75 ++++++++++++++++++++++ 3 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 spec/services/clusters/applications/check_installation_progress_service_spec.rb diff --git a/app/services/clusters/applications/check_installation_progress_service.rb b/app/services/clusters/applications/check_installation_progress_service.rb index cf96c128c2e..81306a2ff5b 100644 --- a/app/services/clusters/applications/check_installation_progress_service.rb +++ b/app/services/clusters/applications/check_installation_progress_service.rb @@ -27,13 +27,13 @@ module Clusters end def on_failed - app.make_errored!(log || 'Installation silently failed') + app.make_errored!(installation_errors || 'Installation silently failed') finalize_installation end def check_timeout if Time.now.utc - app.updated_at.to_time.utc > ClusterWaitForAppInstallationWorker::TIMEOUT - app.make_errored!('App installation timeouted') + app.make_errored!('Installation timeouted') else ClusterWaitForAppInstallationWorker.perform_in( ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id) diff --git a/spec/factories/clusters/applications/helm.rb b/spec/factories/clusters/applications/helm.rb index 968a6a1a007..fd956097115 100644 --- a/spec/factories/clusters/applications/helm.rb +++ b/spec/factories/clusters/applications/helm.rb @@ -31,5 +31,10 @@ FactoryGirl.define do status(-1) status_reason 'something went wrong' end + + trait :timeouted do + installing + updated_at ClusterWaitForAppInstallationWorker::TIMEOUT.ago + end end end diff --git a/spec/services/clusters/applications/check_installation_progress_service_spec.rb b/spec/services/clusters/applications/check_installation_progress_service_spec.rb new file mode 100644 index 00000000000..c64c3a0c94c --- /dev/null +++ b/spec/services/clusters/applications/check_installation_progress_service_spec.rb @@ -0,0 +1,75 @@ +require 'spec_helper' + +describe Clusters::Applications::CheckInstallationProgressService do + RESCHEDULE_PHASES = Gitlab::Kubernetes::Pod::PHASES - [Gitlab::Kubernetes::Pod::SUCCEEDED, Gitlab::Kubernetes::Pod::FAILED].freeze + + def mock_helm_api(phase, errors: nil) + expect(service).to receive(:installation_phase).once.and_return(phase) + expect(service).to receive(:installation_errors).once.and_return(errors) if errors.present? + end + + shared_examples 'not yet completed phase' do |phase| + context "when the installation POD phase is #{phase}" do + before do + mock_helm_api(phase) + end + + context 'when not timeouted' do + it 'reschedule a new check' do + expect(ClusterWaitForAppInstallationWorker).to receive(:perform_in).once + + service.execute + + expect(application).to be_installing + expect(application.status_reason).to be_nil + end + end + + context 'when timeouted' do + let(:application) { create(:applications_helm, :timeouted) } + + it 'make the application errored' do + expect(ClusterWaitForAppInstallationWorker).not_to receive(:perform_in) + + service.execute + + expect(application).to be_errored + expect(application.status_reason).to match(/\btimeouted\b/) + end + end + end + end + + describe '#execute' do + let(:application) { create(:applications_helm, :installing) } + let(:service) { described_class.new(application) } + + context 'when installation POD succeeded' do + it 'make the application installed' do + mock_helm_api(Gitlab::Kubernetes::Pod::SUCCEEDED) + expect(service).to receive(:finalize_installation).once + + service.execute + + expect(application).to be_installed + expect(application.status_reason).to be_nil + end + end + + context 'when installation POD failed' do + let(:error_message) { 'test installation failed' } + + it 'make the application errored' do + mock_helm_api(Gitlab::Kubernetes::Pod::FAILED, errors: error_message) + expect(service).to receive(:finalize_installation).once + + service.execute + + expect(application).to be_errored + expect(application.status_reason).to eq(error_message) + end + end + + RESCHEDULE_PHASES.each { |phase| it_behaves_like 'not yet completed phase', phase } + end +end -- cgit v1.2.1 From d8223468ae2ae061020cc26336c51dc93cc75571 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 6 Nov 2017 10:41:27 +0100 Subject: Add ingress application --- app/models/clusters/applications/ingress.rb | 32 ++++++++++++++++++++++ app/models/clusters/cluster.rb | 7 +++-- ...1200_create_clusters_kubernetes_ingress_apps.rb | 21 ++++++++++++++ db/schema.rb | 25 +++++++++-------- 4 files changed, 72 insertions(+), 13 deletions(-) create mode 100644 app/models/clusters/applications/ingress.rb create mode 100644 db/migrate/20171106101200_create_clusters_kubernetes_ingress_apps.rb diff --git a/app/models/clusters/applications/ingress.rb b/app/models/clusters/applications/ingress.rb new file mode 100644 index 00000000000..0554cf84ed7 --- /dev/null +++ b/app/models/clusters/applications/ingress.rb @@ -0,0 +1,32 @@ +module Clusters + module Applications + class Ingress < ActiveRecord::Base + self.table_name = 'clusters_applications_ingress' + + include ::Clusters::Concerns::ApplicationStatus + + belongs_to :cluster, class_name: 'Clusters::Cluster', foreign_key: :cluster_id + + validates :cluster, presence: true + + default_value_for :ingress_type, :nginx + default_value_for :version, :nginx + + enum ingress_type: { + nginx: 1 + } + + def self.application_name + self.to_s.demodulize.underscore + end + + def name + self.class.application_name + end + + def chart + 'stable/nginx-ingress' + end + end + end +end diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 7d0be3d3739..cfed9c52860 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -5,7 +5,8 @@ module Clusters self.table_name = 'clusters' APPLICATIONS = { - Applications::Helm.application_name => Applications::Helm + Applications::Helm.application_name => Applications::Helm, + Applications::Ingress.application_name => Applications::Ingress }.freeze belongs_to :user @@ -20,6 +21,7 @@ module Clusters has_one :platform_kubernetes, class_name: 'Clusters::Platforms::Kubernetes', autosave: true, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_one :application_helm, class_name: 'Clusters::Applications::Helm' + has_one :application_ingress, class_name: 'Clusters::Applications::Ingress' accepts_nested_attributes_for :provider_gcp, update_only: true accepts_nested_attributes_for :platform_kubernetes, update_only: true @@ -59,7 +61,8 @@ module Clusters def applications [ - application_helm || build_application_helm + application_helm || build_application_helm, + application_ingress || build_application_ingress ] end diff --git a/db/migrate/20171106101200_create_clusters_kubernetes_ingress_apps.rb b/db/migrate/20171106101200_create_clusters_kubernetes_ingress_apps.rb new file mode 100644 index 00000000000..21f48b1d1b4 --- /dev/null +++ b/db/migrate/20171106101200_create_clusters_kubernetes_ingress_apps.rb @@ -0,0 +1,21 @@ +class CreateClustersKubernetesIngressApps < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + create_table :clusters_applications_ingress do |t| + t.references :cluster, null: false, unique: true, foreign_key: { on_delete: :cascade } + + t.datetime_with_timezone :created_at, null: false + t.datetime_with_timezone :updated_at, null: false + + t.integer :status, null: false + t.integer :ingress_type, null: false + + t.string :version, null: false + t.string :cluster_ip + t.text :status_reason + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 548f4711339..2e5c98f2541 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20171031100710) do +ActiveRecord::Schema.define(version: 20171106101200) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -464,9 +464,7 @@ ActiveRecord::Schema.define(version: 20171031100710) do create_table "cluster_platforms_kubernetes", force: :cascade do |t| t.integer "cluster_id", null: false - t.datetime_with_timezone "created_at", null: false - t.datetime_with_timezone "updated_at", null: false - t.text "api_url" + t.string "api_url" t.text "ca_cert" t.string "namespace" t.string "username" @@ -493,9 +491,6 @@ ActiveRecord::Schema.define(version: 20171031100710) do create_table "cluster_providers_gcp", force: :cascade do |t| t.integer "cluster_id", null: false t.integer "status" - t.integer "num_nodes", null: false - t.datetime_with_timezone "created_at", null: false - t.datetime_with_timezone "updated_at", null: false t.text "status_reason" t.string "gcp_project_id", null: false t.string "zone", null: false @@ -513,10 +508,6 @@ ActiveRecord::Schema.define(version: 20171031100710) do create_table "clusters", force: :cascade do |t| t.integer "user_id" - t.integer "provider_type" - t.integer "platform_type" - t.datetime_with_timezone "created_at", null: false - t.datetime_with_timezone "updated_at", null: false t.boolean "enabled", default: true t.string "name", null: false t.integer "provider_type" @@ -534,6 +525,17 @@ ActiveRecord::Schema.define(version: 20171031100710) do t.text "status_reason" end + create_table "clusters_applications_ingress", force: :cascade do |t| + t.integer "cluster_id", null: false + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false + t.integer "status", null: false + t.integer "ingress_type", null: false + t.string "version", null: false + t.string "cluster_ip" + t.text "status_reason" + end + create_table "container_repositories", force: :cascade do |t| t.integer "project_id", null: false t.string "name", null: false @@ -1888,6 +1890,7 @@ ActiveRecord::Schema.define(version: 20171031100710) do add_foreign_key "cluster_providers_gcp", "clusters", on_delete: :cascade add_foreign_key "clusters", "users", on_delete: :nullify add_foreign_key "clusters_applications_helm", "clusters", on_delete: :cascade + add_foreign_key "clusters_applications_ingress", "clusters", on_delete: :cascade add_foreign_key "container_repositories", "projects" add_foreign_key "deploy_keys_projects", "projects", name: "fk_58a901ca7e", on_delete: :cascade add_foreign_key "deployments", "projects", name: "fk_b9a3851b82", on_delete: :cascade -- cgit v1.2.1 From 8ef73aaaf461ad7fe2b7e351c28c7dfc150bedf0 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 6 Nov 2017 10:48:38 +0100 Subject: Set installation paths --- app/views/projects/clusters/show.html.haml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/views/projects/clusters/show.html.haml b/app/views/projects/clusters/show.html.haml index 799b07046e5..75cfeb08b34 100644 --- a/app/views/projects/clusters/show.html.haml +++ b/app/views/projects/clusters/show.html.haml @@ -6,9 +6,8 @@ - status_path = status_namespace_project_cluster_path(@cluster.project.namespace, @cluster.project, @cluster.id, format: :json) if can?(current_user, :admin_cluster, @cluster) .edit-cluster-form.js-edit-cluster-form{ data: { status_path: status_path, - install_helm_path: 'TODO', - install_ingress_path: 'TODO', - install_runner_path: 'TODO', + install_helm_path: install_applications_namespace_project_cluster_path(@cluster.project.namespace, @cluster.project, @cluster, :helm), + install_ingress_path: install_applications_namespace_project_cluster_path(@cluster.project.namespace, @cluster.project, @cluster, :ingress), toggle_status: @cluster.enabled? ? 'true': 'false', cluster_status: @cluster.status_name, cluster_status_reason: @cluster.status_reason, -- cgit v1.2.1 From fe0292cfa7266ad5f936e103cdf005a00da4c233 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 6 Nov 2017 10:50:10 +0100 Subject: Remove not used ingress and runner --- app/assets/javascripts/clusters/clusters_bundle.js | 4 ---- app/assets/javascripts/clusters/services/clusters_service.js | 2 -- app/views/projects/clusters/show.html.haml | 4 +--- 3 files changed, 1 insertion(+), 9 deletions(-) diff --git a/app/assets/javascripts/clusters/clusters_bundle.js b/app/assets/javascripts/clusters/clusters_bundle.js index 053f11cc3c4..9a436b3e22d 100644 --- a/app/assets/javascripts/clusters/clusters_bundle.js +++ b/app/assets/javascripts/clusters/clusters_bundle.js @@ -28,8 +28,6 @@ export default class Clusters { const { statusPath, installHelmPath, - installIngressPath, - installRunnerPath, clusterStatus, clusterStatusReason, helpPath, @@ -42,8 +40,6 @@ export default class Clusters { this.service = new ClustersService({ endpoint: statusPath, installHelmEndpoint: installHelmPath, - installIngresEndpoint: installIngressPath, - installRunnerEndpoint: installRunnerPath, }); this.toggle = this.toggle.bind(this); diff --git a/app/assets/javascripts/clusters/services/clusters_service.js b/app/assets/javascripts/clusters/services/clusters_service.js index 0ac8e68187d..a7bb0961c7e 100644 --- a/app/assets/javascripts/clusters/services/clusters_service.js +++ b/app/assets/javascripts/clusters/services/clusters_service.js @@ -8,8 +8,6 @@ export default class ClusterService { this.options = options; this.appInstallEndpointMap = { helm: this.options.installHelmEndpoint, - ingress: this.options.installIngressEndpoint, - runner: this.options.installRunnerEndpoint, }; } diff --git a/app/views/projects/clusters/show.html.haml b/app/views/projects/clusters/show.html.haml index 799b07046e5..f116c4f7dba 100644 --- a/app/views/projects/clusters/show.html.haml +++ b/app/views/projects/clusters/show.html.haml @@ -6,9 +6,7 @@ - status_path = status_namespace_project_cluster_path(@cluster.project.namespace, @cluster.project, @cluster.id, format: :json) if can?(current_user, :admin_cluster, @cluster) .edit-cluster-form.js-edit-cluster-form{ data: { status_path: status_path, - install_helm_path: 'TODO', - install_ingress_path: 'TODO', - install_runner_path: 'TODO', + install_helm_path: install_applications_namespace_project_cluster_path(@cluster.project.namespace, @cluster.project, @cluster, :helm), toggle_status: @cluster.enabled? ? 'true': 'false', cluster_status: @cluster.status_name, cluster_status_reason: @cluster.status_reason, -- cgit v1.2.1 From a85292922d0721b4912e86396ea5a7ce511d0516 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 6 Nov 2017 11:03:25 +0100 Subject: Fix ingress endpoint --- app/assets/javascripts/clusters/clusters_bundle.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/clusters/clusters_bundle.js b/app/assets/javascripts/clusters/clusters_bundle.js index ad376efc3c3..9f92d49f576 100644 --- a/app/assets/javascripts/clusters/clusters_bundle.js +++ b/app/assets/javascripts/clusters/clusters_bundle.js @@ -41,7 +41,7 @@ export default class Clusters { this.service = new ClustersService({ endpoint: statusPath, installHelmEndpoint: installHelmPath, - installIngresEndpoint: installIngressPath, + installIngressEndpoint: installIngressPath, }); this.toggle = this.toggle.bind(this); -- cgit v1.2.1 From 001de85e7c6f86423aca0d245fdc83c57b374630 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 6 Nov 2017 11:03:58 +0100 Subject: Return empty applications if not Kubernetes [ci skip] --- app/models/clusters/cluster.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 7d0be3d3739..5b9bd6e548b 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -58,6 +58,8 @@ module Clusters end def applications + return [] unless kubernetes? + [ application_helm || build_application_helm ] -- cgit v1.2.1 From 61501a07cb9c1fff5a30662b3e3815976f2777cb Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Mon, 6 Nov 2017 15:43:02 +0100 Subject: Add Clusters::Applications services tests --- app/models/clusters/concerns/application_status.rb | 2 +- .../check_installation_progress_service.rb | 11 +---- .../applications/finalize_installation_service.rb | 8 +-- .../clusters/applications/install_service.rb | 9 ++-- .../applications/schedule_installation_service.rb | 13 ++--- .../check_installation_progress_service_spec.rb | 57 ++++++++++++---------- .../finalize_installation_service_spec.rb | 32 ++++++++++++ .../clusters/applications/install_service_spec.rb | 54 ++++++++++++++++++++ .../schedule_installation_service_spec.rb | 55 +++++++++++++++++++++ 9 files changed, 184 insertions(+), 57 deletions(-) create mode 100644 spec/services/clusters/applications/finalize_installation_service_spec.rb create mode 100644 spec/services/clusters/applications/install_service_spec.rb create mode 100644 spec/services/clusters/applications/schedule_installation_service_spec.rb diff --git a/app/models/clusters/concerns/application_status.rb b/app/models/clusters/concerns/application_status.rb index 7bb68d75224..c5711fd0b58 100644 --- a/app/models/clusters/concerns/application_status.rb +++ b/app/models/clusters/concerns/application_status.rb @@ -24,7 +24,7 @@ module Clusters end event :make_scheduled do - transition any - [:scheduled] => :scheduled + transition %i(installable errored) => :scheduled end before_transition any => [:scheduled] do |app_status, _| diff --git a/app/services/clusters/applications/check_installation_progress_service.rb b/app/services/clusters/applications/check_installation_progress_service.rb index 81306a2ff5b..1bd5dae0584 100644 --- a/app/services/clusters/applications/check_installation_progress_service.rb +++ b/app/services/clusters/applications/check_installation_progress_service.rb @@ -6,7 +6,7 @@ module Clusters case installation_phase when Gitlab::Kubernetes::Pod::SUCCEEDED - on_succeeded + finalize_installation when Gitlab::Kubernetes::Pod::FAILED on_failed else @@ -18,14 +18,6 @@ module Clusters private - def on_succeeded - if app.make_installed - finalize_installation - else - app.make_errored!("Failed to update app record; #{app.errors}") - end - end - def on_failed app.make_errored!(installation_errors || 'Installation silently failed') finalize_installation @@ -34,6 +26,7 @@ module Clusters def check_timeout if Time.now.utc - app.updated_at.to_time.utc > ClusterWaitForAppInstallationWorker::TIMEOUT app.make_errored!('Installation timeouted') + finalize_installation else ClusterWaitForAppInstallationWorker.perform_in( ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id) diff --git a/app/services/clusters/applications/finalize_installation_service.rb b/app/services/clusters/applications/finalize_installation_service.rb index 339d671c091..292c789b67b 100644 --- a/app/services/clusters/applications/finalize_installation_service.rb +++ b/app/services/clusters/applications/finalize_installation_service.rb @@ -4,13 +4,7 @@ module Clusters def execute helm_api.delete_installation_pod!(app) - app.make_errored!('Installation aborted') if aborted? - end - - private - - def aborted? - app.installing? || app.scheduled? + app.make_installed! if app.installing? end end end diff --git a/app/services/clusters/applications/install_service.rb b/app/services/clusters/applications/install_service.rb index 5ed0968a98a..4eba19a474e 100644 --- a/app/services/clusters/applications/install_service.rb +++ b/app/services/clusters/applications/install_service.rb @@ -5,14 +5,11 @@ module Clusters return unless app.scheduled? begin + app.make_installing! helm_api.install(app) - if app.make_installing - ClusterWaitForAppInstallationWorker.perform_in( - ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id) - else - app.make_errored!("Failed to update app record; #{app.errors}") - end + ClusterWaitForAppInstallationWorker.perform_in( + ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id) rescue KubeException => ke app.make_errored!("Kubernetes error: #{ke.message}") rescue StandardError diff --git a/app/services/clusters/applications/schedule_installation_service.rb b/app/services/clusters/applications/schedule_installation_service.rb index 17b3a09948d..eb8caa68ef7 100644 --- a/app/services/clusters/applications/schedule_installation_service.rb +++ b/app/services/clusters/applications/schedule_installation_service.rb @@ -2,15 +2,10 @@ module Clusters module Applications class ScheduleInstallationService < ::BaseService def execute - application = application_class.find_or_create_by!(cluster: cluster) - - application.make_scheduled! - ClusterInstallAppWorker.perform_async(application.name, application.id) - true - rescue ActiveRecord::RecordInvalid - false - rescue StateMachines::InvalidTransition - false + application_class.find_or_create_by!(cluster: cluster).try do |application| + application.make_scheduled! + ClusterInstallAppWorker.perform_async(application.name, application.id) + end end private diff --git a/spec/services/clusters/applications/check_installation_progress_service_spec.rb b/spec/services/clusters/applications/check_installation_progress_service_spec.rb index c64c3a0c94c..fe04fac9613 100644 --- a/spec/services/clusters/applications/check_installation_progress_service_spec.rb +++ b/spec/services/clusters/applications/check_installation_progress_service_spec.rb @@ -3,20 +3,27 @@ require 'spec_helper' describe Clusters::Applications::CheckInstallationProgressService do RESCHEDULE_PHASES = Gitlab::Kubernetes::Pod::PHASES - [Gitlab::Kubernetes::Pod::SUCCEEDED, Gitlab::Kubernetes::Pod::FAILED].freeze - def mock_helm_api(phase, errors: nil) - expect(service).to receive(:installation_phase).once.and_return(phase) - expect(service).to receive(:installation_errors).once.and_return(errors) if errors.present? + let(:application) { create(:applications_helm, :installing) } + let(:service) { described_class.new(application) } + let(:phase) { Gitlab::Kubernetes::Pod::UNKNOWN } + let(:errors) { nil } + + shared_examples 'a terminated installation' do + it 'finalize the installation' do + expect(service).to receive(:finalize_installation).once + + service.execute + end end - shared_examples 'not yet completed phase' do |phase| - context "when the installation POD phase is #{phase}" do - before do - mock_helm_api(phase) - end + shared_examples 'a not yet terminated installation' do |a_phase| + let(:phase) { a_phase } + context "when phase is #{a_phase}" do context 'when not timeouted' do it 'reschedule a new check' do expect(ClusterWaitForAppInstallationWorker).to receive(:perform_in).once + expect(service).not_to receive(:finalize_installation) service.execute @@ -28,6 +35,8 @@ describe Clusters::Applications::CheckInstallationProgressService do context 'when timeouted' do let(:application) { create(:applications_helm, :timeouted) } + it_behaves_like 'a terminated installation' + it 'make the application errored' do expect(ClusterWaitForAppInstallationWorker).not_to receive(:perform_in) @@ -40,36 +49,34 @@ describe Clusters::Applications::CheckInstallationProgressService do end end - describe '#execute' do - let(:application) { create(:applications_helm, :installing) } - let(:service) { described_class.new(application) } + before do + expect(service).to receive(:installation_phase).once.and_return(phase) - context 'when installation POD succeeded' do - it 'make the application installed' do - mock_helm_api(Gitlab::Kubernetes::Pod::SUCCEEDED) - expect(service).to receive(:finalize_installation).once + allow(service).to receive(:installation_errors).and_return(errors) + allow(service).to receive(:finalize_installation).and_return(nil) + end - service.execute + describe '#execute' do + context 'when installation POD succeeded' do + let(:phase) { Gitlab::Kubernetes::Pod::SUCCEEDED } - expect(application).to be_installed - expect(application.status_reason).to be_nil - end + it_behaves_like 'a terminated installation' end context 'when installation POD failed' do - let(:error_message) { 'test installation failed' } + let(:phase) { Gitlab::Kubernetes::Pod::FAILED } + let(:errors) { 'test installation failed' } - it 'make the application errored' do - mock_helm_api(Gitlab::Kubernetes::Pod::FAILED, errors: error_message) - expect(service).to receive(:finalize_installation).once + it_behaves_like 'a terminated installation' + it 'make the application errored' do service.execute expect(application).to be_errored - expect(application.status_reason).to eq(error_message) + expect(application.status_reason).to eq(errors) end end - RESCHEDULE_PHASES.each { |phase| it_behaves_like 'not yet completed phase', phase } + RESCHEDULE_PHASES.each { |phase| it_behaves_like 'a not yet terminated installation', phase } end end diff --git a/spec/services/clusters/applications/finalize_installation_service_spec.rb b/spec/services/clusters/applications/finalize_installation_service_spec.rb new file mode 100644 index 00000000000..08b7a80dfcd --- /dev/null +++ b/spec/services/clusters/applications/finalize_installation_service_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Clusters::Applications::FinalizeInstallationService do + describe '#execute' do + let(:application) { create(:applications_helm, :installing) } + let(:service) { described_class.new(application) } + + before do + expect_any_instance_of(Gitlab::Kubernetes::Helm).to receive(:delete_installation_pod!).with(application) + end + + context 'when installation POD succeeded' do + it 'make the application installed' do + service.execute + + expect(application).to be_installed + expect(application.status_reason).to be_nil + end + end + + context 'when installation POD failed' do + let(:application) { create(:applications_helm, :errored) } + + it 'make the application errored' do + service.execute + + expect(application).to be_errored + expect(application.status_reason).not_to be_nil + end + end + end +end diff --git a/spec/services/clusters/applications/install_service_spec.rb b/spec/services/clusters/applications/install_service_spec.rb new file mode 100644 index 00000000000..a646dac1cae --- /dev/null +++ b/spec/services/clusters/applications/install_service_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' + +describe Clusters::Applications::InstallService do + describe '#execute' do + let(:application) { create(:applications_helm, :scheduled) } + let(:service) { described_class.new(application) } + + context 'when there are no errors' do + before do + expect_any_instance_of(Gitlab::Kubernetes::Helm).to receive(:install).with(application) + allow(ClusterWaitForAppInstallationWorker).to receive(:perform_in).and_return(nil) + end + + it 'make the application installing' do + service.execute + + expect(application).to be_installing + end + + it 'schedule async installation status check' do + expect(ClusterWaitForAppInstallationWorker).to receive(:perform_in).once + + service.execute + end + end + + context 'when k8s cluster communication fails' do + before do + error = KubeException.new(500, 'system failure', nil) + expect_any_instance_of(Gitlab::Kubernetes::Helm).to receive(:install).with(application).and_raise(error) + end + + it 'make the application errored' do + service.execute + + expect(application).to be_errored + expect(application.status_reason).to match(/kubernetes error:/i) + end + end + + context 'when application cannot be persisted' do + let(:application) { build(:applications_helm, :scheduled) } + + it 'make the application errored' do + expect(application).to receive(:make_installing!).once.and_raise(ActiveRecord::RecordInvalid) + expect_any_instance_of(Gitlab::Kubernetes::Helm).not_to receive(:install) + + service.execute + + expect(application).to be_errored + end + end + end +end diff --git a/spec/services/clusters/applications/schedule_installation_service_spec.rb b/spec/services/clusters/applications/schedule_installation_service_spec.rb new file mode 100644 index 00000000000..6ba587a41db --- /dev/null +++ b/spec/services/clusters/applications/schedule_installation_service_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe Clusters::Applications::ScheduleInstallationService do + def count_scheduled + application_class&.with_status(:scheduled)&.count || 0 + end + + shared_examples 'a failing service' do + it 'raise an exception' do + expect(ClusterInstallAppWorker).not_to receive(:perform_async) + count_before = count_scheduled + + expect { service.execute }.to raise_error(StandardError) + expect(count_scheduled).to eq(count_before) + end + end + + describe '#execute' do + let(:application_class) { Clusters::Applications::Helm } + let(:cluster) { create(:cluster, :project, :provided_by_gcp) } + let(:project) { cluster.project } + let(:service) { described_class.new(project, nil, cluster: cluster, application_class: application_class) } + + it 'creates a new application' do + expect { service.execute }.to change { application_class.count }.by(1) + end + + it 'make the application scheduled' do + expect(ClusterInstallAppWorker).to receive(:perform_async).with(application_class.application_name, kind_of(Numeric)).once + + expect { service.execute }.to change { application_class.with_status(:scheduled).count }.by(1) + end + + context 'when installation is already in progress' do + let(:application) { create(:applications_helm, :installing) } + let(:cluster) { application.cluster } + + it_behaves_like 'a failing service' + end + + context 'when application_class is nil' do + let(:application_class) { nil } + + it_behaves_like 'a failing service' + end + + context 'when application cannot be persisted' do + before do + expect_any_instance_of(application_class).to receive(:make_scheduled!).once.and_raise(ActiveRecord::RecordInvalid) + end + + it_behaves_like 'a failing service' + end + end +end -- cgit v1.2.1 From ac927462dc1b9578de3a716e9e4ff551f424663b Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 6 Nov 2017 15:48:44 +0100 Subject: Add support for not_installable/scheduled and to not show created banner --- app/assets/javascripts/clusters/clusters_bundle.js | 5 ++++- app/assets/javascripts/clusters/components/application_row.vue | 7 +++++-- app/assets/javascripts/clusters/constants.js | 2 ++ app/models/clusters/applications/helm.rb | 6 ++++++ app/models/clusters/applications/ingress.rb | 6 ++++++ app/models/clusters/cluster.rb | 5 +++-- app/models/clusters/concerns/application_status.rb | 1 + 7 files changed, 27 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/clusters/clusters_bundle.js b/app/assets/javascripts/clusters/clusters_bundle.js index 9f92d49f576..8d0610b23a3 100644 --- a/app/assets/javascripts/clusters/clusters_bundle.js +++ b/app/assets/javascripts/clusters/clusters_bundle.js @@ -134,9 +134,12 @@ export default class Clusters { handleSuccess(data) { const prevApplicationMap = Object.assign({}, this.store.state.applications); + const prevStatus = this.store.state.status; this.store.updateStateFromServer(data.data); this.checkForNewInstalls(prevApplicationMap, this.store.state.applications); - this.updateContainer(this.store.state.status, this.store.state.statusReason); + if (prevStatus.length == 0 || prevStatus !== this.store.state.status) { + this.updateContainer(this.store.state.status, this.store.state.statusReason); + } } toggle() { diff --git a/app/assets/javascripts/clusters/components/application_row.vue b/app/assets/javascripts/clusters/components/application_row.vue index f8d53fcc4b7..9c5ff39534f 100644 --- a/app/assets/javascripts/clusters/components/application_row.vue +++ b/app/assets/javascripts/clusters/components/application_row.vue @@ -3,6 +3,8 @@ import { s__ } from '../../locale'; import eventHub from '../event_hub'; import loadingButton from '../../vue_shared/components/loading_button.vue'; import { + APPLICATION_NOT_INSTALLABLE, + APPLICATION_SCHEDULED, APPLICATION_INSTALLABLE, APPLICATION_INSTALLING, APPLICATION_INSTALLED, @@ -59,6 +61,7 @@ export default { }, installButtonLoading() { return !this.status || + this.status === APPLICATION_SCHEDULED || this.status === APPLICATION_INSTALLING || this.requestStatus === REQUEST_LOADING; }, @@ -72,9 +75,9 @@ export default { }, installButtonLabel() { let label; - if (this.status === APPLICATION_INSTALLABLE || this.status === APPLICATION_ERROR) { + if (this.status === APPLICATION_INSTALLABLE || this.status === APPLICATION_ERROR || this.status === APPLICATION_NOT_INSTALLABLE) { label = s__('ClusterIntegration|Install'); - } else if (this.status === APPLICATION_INSTALLING) { + } else if (this.status === APPLICATION_SCHEDULED || this.status === APPLICATION_INSTALLING) { label = s__('ClusterIntegration|Installing'); } else if (this.status === APPLICATION_INSTALLED) { label = s__('ClusterIntegration|Installed'); diff --git a/app/assets/javascripts/clusters/constants.js b/app/assets/javascripts/clusters/constants.js index 3f202435716..f1894b173b9 100644 --- a/app/assets/javascripts/clusters/constants.js +++ b/app/assets/javascripts/clusters/constants.js @@ -1,5 +1,7 @@ // These need to match what is returned from the server +export const APPLICATION_NOT_INSTALLABLE = 'not_installable'; export const APPLICATION_INSTALLABLE = 'installable'; +export const APPLICATION_SCHEDULED = 'scheduled'; export const APPLICATION_INSTALLING = 'installing'; export const APPLICATION_INSTALLED = 'installed'; export const APPLICATION_ERROR = 'error'; diff --git a/app/models/clusters/applications/helm.rb b/app/models/clusters/applications/helm.rb index 42626a50175..9bc5c026645 100644 --- a/app/models/clusters/applications/helm.rb +++ b/app/models/clusters/applications/helm.rb @@ -11,10 +11,16 @@ module Clusters validates :cluster, presence: true + after_initialize :set_initial_status + def self.application_name self.to_s.demodulize.underscore end + def set_initial_status + self.status = 0 unless cluster.platform_kubernetes_active? + end + def name self.class.application_name end diff --git a/app/models/clusters/applications/ingress.rb b/app/models/clusters/applications/ingress.rb index 0554cf84ed7..8cb1ed68de9 100644 --- a/app/models/clusters/applications/ingress.rb +++ b/app/models/clusters/applications/ingress.rb @@ -9,6 +9,8 @@ module Clusters validates :cluster, presence: true + after_initialize :set_initial_status + default_value_for :ingress_type, :nginx default_value_for :version, :nginx @@ -20,6 +22,10 @@ module Clusters self.to_s.demodulize.underscore end + def set_initial_status + self.status = 0 unless cluster.application_helm_installed? + end + def name self.class.application_name end diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 90508e31e93..185d9473aab 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -39,6 +39,9 @@ module Clusters delegate :on_creation?, to: :provider, allow_nil: true delegate :update_kubernetes_integration!, to: :platform, allow_nil: true + delegate :active?, to: :platform_kubernetes, prefix: true, allow_nil: true + delegate :installed?, to: :application_helm, prefix: true, allow_nil: true + enum platform_type: { kubernetes: 1 } @@ -60,8 +63,6 @@ module Clusters end def applications - return [] unless kubernetes? - [ application_helm || build_application_helm, application_ingress || build_application_ingress diff --git a/app/models/clusters/concerns/application_status.rb b/app/models/clusters/concerns/application_status.rb index 7bb68d75224..7592dd55689 100644 --- a/app/models/clusters/concerns/application_status.rb +++ b/app/models/clusters/concerns/application_status.rb @@ -5,6 +5,7 @@ module Clusters included do state_machine :status, initial: :installable do + state :not_installable, value: -2 state :errored, value: -1 state :installable, value: 0 state :scheduled, value: 1 -- cgit v1.2.1 From 802e6653de7c67def245d86244f9223431618189 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 6 Nov 2017 15:50:55 +0100 Subject: Add active? to Platforms::Kubernetes --- app/models/clusters/platforms/kubernetes.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb index 74f7c9442db..6dc1ee810d3 100644 --- a/app/models/clusters/platforms/kubernetes.rb +++ b/app/models/clusters/platforms/kubernetes.rb @@ -74,6 +74,10 @@ module Clusters ) end + def active? + manages_kubernetes_service? + end + private def enforce_namespace_to_lower_case -- cgit v1.2.1 From f3a3566edc8fe24337b9df163f4699785061bb38 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 6 Nov 2017 15:48:44 +0100 Subject: Add support for not_installable/scheduled and to not show created banner --- app/assets/javascripts/clusters/clusters_bundle.js | 5 ++++- app/assets/javascripts/clusters/components/application_row.vue | 7 +++++-- app/assets/javascripts/clusters/constants.js | 2 ++ app/models/clusters/applications/helm.rb | 6 ++++++ app/models/clusters/cluster.rb | 5 +++-- app/models/clusters/concerns/application_status.rb | 1 + 6 files changed, 21 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/clusters/clusters_bundle.js b/app/assets/javascripts/clusters/clusters_bundle.js index 9a436b3e22d..4d4c90460d2 100644 --- a/app/assets/javascripts/clusters/clusters_bundle.js +++ b/app/assets/javascripts/clusters/clusters_bundle.js @@ -132,9 +132,12 @@ export default class Clusters { handleSuccess(data) { const prevApplicationMap = Object.assign({}, this.store.state.applications); + const prevStatus = this.store.state.status; this.store.updateStateFromServer(data.data); this.checkForNewInstalls(prevApplicationMap, this.store.state.applications); - this.updateContainer(this.store.state.status, this.store.state.statusReason); + if (prevStatus.length == 0 || prevStatus !== this.store.state.status) { + this.updateContainer(this.store.state.status, this.store.state.statusReason); + } } toggle() { diff --git a/app/assets/javascripts/clusters/components/application_row.vue b/app/assets/javascripts/clusters/components/application_row.vue index f8d53fcc4b7..9c5ff39534f 100644 --- a/app/assets/javascripts/clusters/components/application_row.vue +++ b/app/assets/javascripts/clusters/components/application_row.vue @@ -3,6 +3,8 @@ import { s__ } from '../../locale'; import eventHub from '../event_hub'; import loadingButton from '../../vue_shared/components/loading_button.vue'; import { + APPLICATION_NOT_INSTALLABLE, + APPLICATION_SCHEDULED, APPLICATION_INSTALLABLE, APPLICATION_INSTALLING, APPLICATION_INSTALLED, @@ -59,6 +61,7 @@ export default { }, installButtonLoading() { return !this.status || + this.status === APPLICATION_SCHEDULED || this.status === APPLICATION_INSTALLING || this.requestStatus === REQUEST_LOADING; }, @@ -72,9 +75,9 @@ export default { }, installButtonLabel() { let label; - if (this.status === APPLICATION_INSTALLABLE || this.status === APPLICATION_ERROR) { + if (this.status === APPLICATION_INSTALLABLE || this.status === APPLICATION_ERROR || this.status === APPLICATION_NOT_INSTALLABLE) { label = s__('ClusterIntegration|Install'); - } else if (this.status === APPLICATION_INSTALLING) { + } else if (this.status === APPLICATION_SCHEDULED || this.status === APPLICATION_INSTALLING) { label = s__('ClusterIntegration|Installing'); } else if (this.status === APPLICATION_INSTALLED) { label = s__('ClusterIntegration|Installed'); diff --git a/app/assets/javascripts/clusters/constants.js b/app/assets/javascripts/clusters/constants.js index 3f202435716..f1894b173b9 100644 --- a/app/assets/javascripts/clusters/constants.js +++ b/app/assets/javascripts/clusters/constants.js @@ -1,5 +1,7 @@ // These need to match what is returned from the server +export const APPLICATION_NOT_INSTALLABLE = 'not_installable'; export const APPLICATION_INSTALLABLE = 'installable'; +export const APPLICATION_SCHEDULED = 'scheduled'; export const APPLICATION_INSTALLING = 'installing'; export const APPLICATION_INSTALLED = 'installed'; export const APPLICATION_ERROR = 'error'; diff --git a/app/models/clusters/applications/helm.rb b/app/models/clusters/applications/helm.rb index 42626a50175..9bc5c026645 100644 --- a/app/models/clusters/applications/helm.rb +++ b/app/models/clusters/applications/helm.rb @@ -11,10 +11,16 @@ module Clusters validates :cluster, presence: true + after_initialize :set_initial_status + def self.application_name self.to_s.demodulize.underscore end + def set_initial_status + self.status = 0 unless cluster.platform_kubernetes_active? + end + def name self.class.application_name end diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 5b9bd6e548b..68759ebb6df 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -37,6 +37,9 @@ module Clusters delegate :on_creation?, to: :provider, allow_nil: true delegate :update_kubernetes_integration!, to: :platform, allow_nil: true + delegate :active?, to: :platform_kubernetes, prefix: true, allow_nil: true + delegate :installed?, to: :application_helm, prefix: true, allow_nil: true + enum platform_type: { kubernetes: 1 } @@ -58,8 +61,6 @@ module Clusters end def applications - return [] unless kubernetes? - [ application_helm || build_application_helm ] diff --git a/app/models/clusters/concerns/application_status.rb b/app/models/clusters/concerns/application_status.rb index 7bb68d75224..7592dd55689 100644 --- a/app/models/clusters/concerns/application_status.rb +++ b/app/models/clusters/concerns/application_status.rb @@ -5,6 +5,7 @@ module Clusters included do state_machine :status, initial: :installable do + state :not_installable, value: -2 state :errored, value: -1 state :installable, value: 0 state :scheduled, value: 1 -- cgit v1.2.1 From 895b6e5d80397fdd6cb5e1727a410a08f8a5b332 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 6 Nov 2017 15:50:55 +0100 Subject: Add active? to Platforms::Kubernetes --- app/models/clusters/platforms/kubernetes.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb index 74f7c9442db..6dc1ee810d3 100644 --- a/app/models/clusters/platforms/kubernetes.rb +++ b/app/models/clusters/platforms/kubernetes.rb @@ -74,6 +74,10 @@ module Clusters ) end + def active? + manages_kubernetes_service? + end + private def enforce_namespace_to_lower_case -- cgit v1.2.1 From b893a858808f1e2aefca4e3f665d633d31fa5937 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Mon, 6 Nov 2017 16:04:58 +0100 Subject: db/schema.rb cleanup --- db/schema.rb | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 548f4711339..507024c20de 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -474,8 +474,6 @@ ActiveRecord::Schema.define(version: 20171031100710) do t.string "encrypted_password_iv" t.text "encrypted_token" t.string "encrypted_token_iv" - t.datetime_with_timezone "created_at", null: false - t.datetime_with_timezone "updated_at", null: false end add_index "cluster_platforms_kubernetes", ["cluster_id"], name: "index_cluster_platforms_kubernetes_on_cluster_id", unique: true, using: :btree @@ -499,14 +497,11 @@ ActiveRecord::Schema.define(version: 20171031100710) do t.text "status_reason" t.string "gcp_project_id", null: false t.string "zone", null: false - t.integer "num_nodes", null: false t.string "machine_type" t.string "operation_id" t.string "endpoint" t.text "encrypted_access_token" t.string "encrypted_access_token_iv" - t.datetime_with_timezone "created_at", null: false - t.datetime_with_timezone "updated_at", null: false end add_index "cluster_providers_gcp", ["cluster_id"], name: "index_cluster_providers_gcp_on_cluster_id", unique: true, using: :btree @@ -519,12 +514,11 @@ ActiveRecord::Schema.define(version: 20171031100710) do t.datetime_with_timezone "updated_at", null: false t.boolean "enabled", default: true t.string "name", null: false - t.integer "provider_type" - t.integer "platform_type" - t.datetime_with_timezone "created_at", null: false - t.datetime_with_timezone "updated_at", null: false end + add_index "clusters", ["enabled"], name: "index_clusters_on_enabled", using: :btree + add_index "clusters", ["user_id"], name: "index_clusters_on_user_id", using: :btree + create_table "clusters_applications_helm", force: :cascade do |t| t.integer "cluster_id", null: false t.datetime_with_timezone "created_at", null: false -- cgit v1.2.1 From 2802b5bb52b6ba28e6eeb1813f3fd3a79d2c03c4 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Mon, 6 Nov 2017 17:02:28 +0100 Subject: Add ClusterApplicationEntity tests --- app/serializers/cluster_app_entity.rb | 5 ---- app/serializers/cluster_application_entity.rb | 5 ++++ app/serializers/cluster_entity.rb | 2 +- spec/fixtures/api/schemas/cluster_status.json | 26 ++++++++----------- .../serializers/cluster_application_entity_spec.rb | 30 ++++++++++++++++++++++ spec/serializers/cluster_entity_spec.rb | 13 ++++++++-- spec/serializers/cluster_serializer_spec.rb | 2 +- 7 files changed, 59 insertions(+), 24 deletions(-) delete mode 100644 app/serializers/cluster_app_entity.rb create mode 100644 app/serializers/cluster_application_entity.rb create mode 100644 spec/serializers/cluster_application_entity_spec.rb diff --git a/app/serializers/cluster_app_entity.rb b/app/serializers/cluster_app_entity.rb deleted file mode 100644 index 7da2d4921a2..00000000000 --- a/app/serializers/cluster_app_entity.rb +++ /dev/null @@ -1,5 +0,0 @@ -class ClusterAppEntity < Grape::Entity - expose :name - expose :status_name, as: :status - expose :status_reason -end diff --git a/app/serializers/cluster_application_entity.rb b/app/serializers/cluster_application_entity.rb new file mode 100644 index 00000000000..3f9a275ad08 --- /dev/null +++ b/app/serializers/cluster_application_entity.rb @@ -0,0 +1,5 @@ +class ClusterApplicationEntity < Grape::Entity + expose :name + expose :status_name, as: :status + expose :status_reason +end diff --git a/app/serializers/cluster_entity.rb b/app/serializers/cluster_entity.rb index e775c68eb6b..7e5b0997878 100644 --- a/app/serializers/cluster_entity.rb +++ b/app/serializers/cluster_entity.rb @@ -3,5 +3,5 @@ class ClusterEntity < Grape::Entity expose :status_name, as: :status expose :status_reason - expose :applications, using: ClusterAppEntity + expose :applications, using: ClusterApplicationEntity end diff --git a/spec/fixtures/api/schemas/cluster_status.json b/spec/fixtures/api/schemas/cluster_status.json index 451ea50f0f9..489d563be2b 100644 --- a/spec/fixtures/api/schemas/cluster_status.json +++ b/spec/fixtures/api/schemas/cluster_status.json @@ -1,42 +1,38 @@ { "type": "object", "required" : [ - "status" + "status", + "applications" ], "properties" : { "status": { "type": "string" }, "status_reason": { "type": ["string", "null"] }, - "applications": { "$ref": "#/definitions/applications" } + "applications": { + "type": "array", + "items": { "$ref": "#/definitions/application_status" } + } }, "additionalProperties": false, "definitions": { - "applications": { - "type": "object", - "additionalProperties": false, - "properties" : { - "helm": { "$ref": "#/definitions/app_status" }, - "runner": { "$ref": "#/definitions/app_status" }, - "ingress": { "$ref": "#/definitions/app_status" }, - "prometheus": { "$ref": "#/definitions/app_status" } - } - }, - "app_status": { + "application_status": { "type": "object", "additionalProperties": false, "properties" : { + "name": { "type": "string" }, "status": { "type": { "enum": [ "installable", + "scheduled", "installing", "installed", - "error" + "errored" ] } }, "status_reason": { "type": ["string", "null"] } }, - "required" : [ "status" ] + "required" : [ "name", "status" ] } } } diff --git a/spec/serializers/cluster_application_entity_spec.rb b/spec/serializers/cluster_application_entity_spec.rb new file mode 100644 index 00000000000..61cebcefa28 --- /dev/null +++ b/spec/serializers/cluster_application_entity_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +describe ClusterApplicationEntity do + describe '#as_json' do + let(:application) { build(:applications_helm) } + subject { described_class.new(application).as_json } + + it 'has name' do + expect(subject[:name]).to eq(application.name) + end + + it 'has status' do + expect(subject[:status]).to eq(:installable) + end + + it 'has no status_reason' do + expect(subject[:status_reason]).to be_nil + end + + context 'when application is errored' do + let(:application) { build(:applications_helm, :errored) } + + it 'has corresponded data' do + expect(subject[:status]).to eq(:errored) + expect(subject[:status_reason]).not_to be_nil + expect(subject[:status_reason]).to eq(application.status_reason) + end + end + end +end diff --git a/spec/serializers/cluster_entity_spec.rb b/spec/serializers/cluster_entity_spec.rb index abfc3731fb2..c58ee1a1ea6 100644 --- a/spec/serializers/cluster_entity_spec.rb +++ b/spec/serializers/cluster_entity_spec.rb @@ -35,8 +35,17 @@ describe ClusterEntity do end end - it 'contains applications' do - expect(subject[:applications]).to eq({}) + context 'when no application has been installed' do + let(:cluster) { create(:cluster) } + subject { described_class.new(cluster).as_json[:applications]} + + it 'contains helm as installable' do + expect(subject).not_to be_empty + + helm = subject[0] + expect(helm[:name]).to eq('helm') + expect(helm[:status]).to eq(:installable) + end end end end diff --git a/spec/serializers/cluster_serializer_spec.rb b/spec/serializers/cluster_serializer_spec.rb index 04d8728303c..a6dd2309663 100644 --- a/spec/serializers/cluster_serializer_spec.rb +++ b/spec/serializers/cluster_serializer_spec.rb @@ -9,7 +9,7 @@ describe ClusterSerializer do let(:provider) { create(:provider_gcp, :errored) } it 'serializes only status' do - expect(subject.keys).to contain_exactly(:status, :status_reason) + expect(subject.keys).to contain_exactly(:status, :status_reason, :applications) end end -- cgit v1.2.1 From f4fb0340094508106113c0c7c22c865fa7c73f7f Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 6 Nov 2017 10:07:19 -0600 Subject: Add FE tests for not_installable/scheduled and cluster banner rules --- app/assets/javascripts/clusters/clusters_bundle.js | 48 +++++++++++++--------- .../clusters/components/application_row.vue | 6 ++- app/assets/javascripts/clusters/constants.js | 2 +- .../clusters/services/clusters_service.js | 2 + spec/javascripts/clusters/clusters_bundle_spec.js | 48 ++++++++++++++++++++-- .../clusters/components/application_row_spec.js | 42 +++++++++++++++---- .../clusters/stores/clusters_store_spec.js | 3 ++ 7 files changed, 117 insertions(+), 34 deletions(-) diff --git a/app/assets/javascripts/clusters/clusters_bundle.js b/app/assets/javascripts/clusters/clusters_bundle.js index 4d4c90460d2..c486208175f 100644 --- a/app/assets/javascripts/clusters/clusters_bundle.js +++ b/app/assets/javascripts/clusters/clusters_bundle.js @@ -28,6 +28,8 @@ export default class Clusters { const { statusPath, installHelmPath, + installIngressPath, + installRunnerPath, clusterStatus, clusterStatusReason, helpPath, @@ -40,6 +42,8 @@ export default class Clusters { this.service = new ClustersService({ endpoint: statusPath, installHelmEndpoint: installHelmPath, + installIngresEndpoint: installIngressPath, + installRunnerEndpoint: installRunnerPath, }); this.toggle = this.toggle.bind(this); @@ -57,7 +61,7 @@ export default class Clusters { this.initApplications(); if (this.store.state.status !== 'created') { - this.updateContainer(this.store.state.status, this.store.state.statusReason); + this.updateContainer(null, this.store.state.status, this.store.state.statusReason); } this.addListeners(); @@ -131,13 +135,13 @@ export default class Clusters { } handleSuccess(data) { - const prevApplicationMap = Object.assign({}, this.store.state.applications); const prevStatus = this.store.state.status; + const prevApplicationMap = Object.assign({}, this.store.state.applications); + this.store.updateStateFromServer(data.data); + this.checkForNewInstalls(prevApplicationMap, this.store.state.applications); - if (prevStatus.length == 0 || prevStatus !== this.store.state.status) { - this.updateContainer(this.store.state.status, this.store.state.statusReason); - } + this.updateContainer(prevStatus, this.store.state.status, this.store.state.statusReason); } toggle() { @@ -168,22 +172,26 @@ export default class Clusters { } } - updateContainer(status, error) { + updateContainer(prevStatus, status, error) { this.hideAll(); - switch (status) { - case 'created': - this.successContainer.classList.remove('hidden'); - break; - case 'errored': - this.errorContainer.classList.remove('hidden'); - this.errorReasonContainer.textContent = error; - break; - case 'scheduled': - case 'creating': - this.creatingContainer.classList.remove('hidden'); - break; - default: - this.hideAll(); + + // We poll all the time but only want the `created` banner to show when newly created + if (this.store.state.status !== 'created' || prevStatus !== this.store.state.status) { + switch (status) { + case 'created': + this.successContainer.classList.remove('hidden'); + break; + case 'errored': + this.errorContainer.classList.remove('hidden'); + this.errorReasonContainer.textContent = error; + break; + case 'scheduled': + case 'creating': + this.creatingContainer.classList.remove('hidden'); + break; + default: + this.hideAll(); + } } } diff --git a/app/assets/javascripts/clusters/components/application_row.vue b/app/assets/javascripts/clusters/components/application_row.vue index 9c5ff39534f..3dc658d0f1f 100644 --- a/app/assets/javascripts/clusters/components/application_row.vue +++ b/app/assets/javascripts/clusters/components/application_row.vue @@ -75,7 +75,11 @@ export default { }, installButtonLabel() { let label; - if (this.status === APPLICATION_INSTALLABLE || this.status === APPLICATION_ERROR || this.status === APPLICATION_NOT_INSTALLABLE) { + if ( + this.status === APPLICATION_NOT_INSTALLABLE || + this.status === APPLICATION_INSTALLABLE || + this.status === APPLICATION_ERROR + ) { label = s__('ClusterIntegration|Install'); } else if (this.status === APPLICATION_SCHEDULED || this.status === APPLICATION_INSTALLING) { label = s__('ClusterIntegration|Installing'); diff --git a/app/assets/javascripts/clusters/constants.js b/app/assets/javascripts/clusters/constants.js index f1894b173b9..93223aefff8 100644 --- a/app/assets/javascripts/clusters/constants.js +++ b/app/assets/javascripts/clusters/constants.js @@ -4,7 +4,7 @@ export const APPLICATION_INSTALLABLE = 'installable'; export const APPLICATION_SCHEDULED = 'scheduled'; export const APPLICATION_INSTALLING = 'installing'; export const APPLICATION_INSTALLED = 'installed'; -export const APPLICATION_ERROR = 'error'; +export const APPLICATION_ERROR = 'errored'; // These are only used client-side export const REQUEST_LOADING = 'request-loading'; diff --git a/app/assets/javascripts/clusters/services/clusters_service.js b/app/assets/javascripts/clusters/services/clusters_service.js index a7bb0961c7e..0ac8e68187d 100644 --- a/app/assets/javascripts/clusters/services/clusters_service.js +++ b/app/assets/javascripts/clusters/services/clusters_service.js @@ -8,6 +8,8 @@ export default class ClusterService { this.options = options; this.appInstallEndpointMap = { helm: this.options.installHelmEndpoint, + ingress: this.options.installIngressEndpoint, + runner: this.options.installRunnerEndpoint, }; } diff --git a/spec/javascripts/clusters/clusters_bundle_spec.js b/spec/javascripts/clusters/clusters_bundle_spec.js index 26d230ce414..86e9cb22be8 100644 --- a/spec/javascripts/clusters/clusters_bundle_spec.js +++ b/spec/javascripts/clusters/clusters_bundle_spec.js @@ -104,7 +104,21 @@ describe('Clusters', () => { describe('updateContainer', () => { describe('when creating cluster', () => { it('should show the creating container', () => { - cluster.updateContainer('creating'); + cluster.updateContainer(null, 'creating'); + + expect( + cluster.creatingContainer.classList.contains('hidden'), + ).toBeFalsy(); + expect( + cluster.successContainer.classList.contains('hidden'), + ).toBeTruthy(); + expect( + cluster.errorContainer.classList.contains('hidden'), + ).toBeTruthy(); + }); + + it('should continue to show `creating` banner with subsequent updates of the same status', () => { + cluster.updateContainer('creating', 'creating'); expect( cluster.creatingContainer.classList.contains('hidden'), @@ -120,7 +134,7 @@ describe('Clusters', () => { describe('when cluster is created', () => { it('should show the success container', () => { - cluster.updateContainer('created'); + cluster.updateContainer(null, 'created'); expect( cluster.creatingContainer.classList.contains('hidden'), @@ -132,11 +146,25 @@ describe('Clusters', () => { cluster.errorContainer.classList.contains('hidden'), ).toBeTruthy(); }); + + it('should not show a banner when status is already `created`', () => { + cluster.updateContainer('created', 'created'); + + expect( + cluster.creatingContainer.classList.contains('hidden'), + ).toBeTruthy(); + expect( + cluster.successContainer.classList.contains('hidden'), + ).toBeTruthy(); + expect( + cluster.errorContainer.classList.contains('hidden'), + ).toBeTruthy(); + }); }); describe('when cluster has error', () => { it('should show the error container', () => { - cluster.updateContainer('errored', 'this is an error'); + cluster.updateContainer(null, 'errored', 'this is an error'); expect( cluster.creatingContainer.classList.contains('hidden'), @@ -152,6 +180,20 @@ describe('Clusters', () => { cluster.errorReasonContainer.textContent, ).toContain('this is an error'); }); + + it('should show `error` banner when previously `creating`', () => { + cluster.updateContainer('creating', 'errored'); + + expect( + cluster.creatingContainer.classList.contains('hidden'), + ).toBeTruthy(); + expect( + cluster.successContainer.classList.contains('hidden'), + ).toBeTruthy(); + expect( + cluster.errorContainer.classList.contains('hidden'), + ).toBeFalsy(); + }); }); }); diff --git a/spec/javascripts/clusters/components/application_row_spec.js b/spec/javascripts/clusters/components/application_row_spec.js index ba38ed6f180..392cebc5e35 100644 --- a/spec/javascripts/clusters/components/application_row_spec.js +++ b/spec/javascripts/clusters/components/application_row_spec.js @@ -1,6 +1,8 @@ import Vue from 'vue'; import eventHub from '~/clusters/event_hub'; import { + APPLICATION_NOT_INSTALLABLE, + APPLICATION_SCHEDULED, APPLICATION_INSTALLABLE, APPLICATION_INSTALLING, APPLICATION_INSTALLED, @@ -60,7 +62,18 @@ describe('Application Row', () => { expect(vm.installButtonLabel).toBeUndefined(); }); - it('has enabled "Install" when `status=installable`', () => { + it('has disabled "Install" when APPLICATION_NOT_INSTALLABLE', () => { + vm = mountComponent(ApplicationRow, { + ...DEFAULT_APPLICATION_STATE, + status: APPLICATION_NOT_INSTALLABLE, + }); + + expect(vm.installButtonLabel).toEqual('Install'); + expect(vm.installButtonLoading).toEqual(false); + expect(vm.installButtonDisabled).toEqual(true); + }); + + it('has enabled "Install" when APPLICATION_INSTALLABLE', () => { vm = mountComponent(ApplicationRow, { ...DEFAULT_APPLICATION_STATE, status: APPLICATION_INSTALLABLE, @@ -71,7 +84,18 @@ describe('Application Row', () => { expect(vm.installButtonDisabled).toEqual(false); }); - it('has loading "Installing" when `status=installing`', () => { + it('has loading "Installing" when APPLICATION_SCHEDULED', () => { + vm = mountComponent(ApplicationRow, { + ...DEFAULT_APPLICATION_STATE, + status: APPLICATION_SCHEDULED, + }); + + expect(vm.installButtonLabel).toEqual('Installing'); + expect(vm.installButtonLoading).toEqual(true); + expect(vm.installButtonDisabled).toEqual(true); + }); + + it('has loading "Installing" when APPLICATION_INSTALLING', () => { vm = mountComponent(ApplicationRow, { ...DEFAULT_APPLICATION_STATE, status: APPLICATION_INSTALLING, @@ -82,7 +106,7 @@ describe('Application Row', () => { expect(vm.installButtonDisabled).toEqual(true); }); - it('has disabled "Installed" when `status=installed`', () => { + it('has disabled "Installed" when APPLICATION_INSTALLED', () => { vm = mountComponent(ApplicationRow, { ...DEFAULT_APPLICATION_STATE, status: APPLICATION_INSTALLED, @@ -93,7 +117,7 @@ describe('Application Row', () => { expect(vm.installButtonDisabled).toEqual(true); }); - it('has disabled "Install" when `status=error`', () => { + it('has disabled "Install" when APPLICATION_ERROR', () => { vm = mountComponent(ApplicationRow, { ...DEFAULT_APPLICATION_STATE, status: APPLICATION_ERROR, @@ -104,7 +128,7 @@ describe('Application Row', () => { expect(vm.installButtonDisabled).toEqual(true); }); - it('has loading "Install" when `requestStatus=loading`', () => { + it('has loading "Install" when REQUEST_LOADING', () => { vm = mountComponent(ApplicationRow, { ...DEFAULT_APPLICATION_STATE, status: APPLICATION_INSTALLABLE, @@ -116,7 +140,7 @@ describe('Application Row', () => { expect(vm.installButtonDisabled).toEqual(true); }); - it('has disabled "Install" when `requestStatus=success`', () => { + it('has disabled "Install" when REQUEST_SUCCESS', () => { vm = mountComponent(ApplicationRow, { ...DEFAULT_APPLICATION_STATE, status: APPLICATION_INSTALLABLE, @@ -128,7 +152,7 @@ describe('Application Row', () => { expect(vm.installButtonDisabled).toEqual(true); }); - it('has enabled "Install" when `requestStatus=error` (so you can try installing again)', () => { + it('has enabled "Install" when REQUEST_FAILURE (so you can try installing again)', () => { vm = mountComponent(ApplicationRow, { ...DEFAULT_APPLICATION_STATE, status: APPLICATION_INSTALLABLE, @@ -181,7 +205,7 @@ describe('Application Row', () => { expect(generalErrorMessage).toBeNull(); }); - it('shows status reason when `status=error`', () => { + it('shows status reason when APPLICATION_ERROR', () => { const statusReason = 'We broke it 0.0'; vm = mountComponent(ApplicationRow, { ...DEFAULT_APPLICATION_STATE, @@ -195,7 +219,7 @@ describe('Application Row', () => { expect(statusErrorMessage.textContent.trim()).toEqual(statusReason); }); - it('shows request reason when `requestStatus=error`', () => { + it('shows request reason when REQUEST_FAILURE', () => { const requestReason = 'We broke thre request 0.0'; vm = mountComponent(ApplicationRow, { ...DEFAULT_APPLICATION_STATE, diff --git a/spec/javascripts/clusters/stores/clusters_store_spec.js b/spec/javascripts/clusters/stores/clusters_store_spec.js index 9f9d63434f7..cb8b3d38e2e 100644 --- a/spec/javascripts/clusters/stores/clusters_store_spec.js +++ b/spec/javascripts/clusters/stores/clusters_store_spec.js @@ -62,18 +62,21 @@ describe('Clusters Store', () => { statusReason: mockResponseData.status_reason, applications: { helm: { + title: 'Helm Tiller', status: mockResponseData.applications[0].status, statusReason: mockResponseData.applications[0].status_reason, requestStatus: null, requestReason: null, }, ingress: { + title: 'Ingress', status: mockResponseData.applications[1].status, statusReason: mockResponseData.applications[1].status_reason, requestStatus: null, requestReason: null, }, runner: { + title: 'GitLab Runner', status: mockResponseData.applications[2].status, statusReason: mockResponseData.applications[2].status_reason, requestStatus: null, -- cgit v1.2.1 From 2b4fccb7205101480bae2668e681cb3b58fface5 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Mon, 6 Nov 2017 18:06:02 +0100 Subject: Add Helm import/export --- lib/gitlab/import_export/import_export.yml | 3 ++- lib/gitlab/import_export/relation_factory.rb | 2 +- spec/lib/gitlab/import_export/all_models.yml | 8 ++++++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index 561779182bc..06b1035fec6 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -54,7 +54,8 @@ project_tree: - :auto_devops - :triggers - :pipeline_schedules - - :cluster + - clusters: + - :application_helm - :services - :hooks - protected_branches: diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index a790dcfe8a6..679be1b21fa 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -8,8 +8,8 @@ module Gitlab triggers: 'Ci::Trigger', pipeline_schedules: 'Ci::PipelineSchedule', builds: 'Ci::Build', - cluster: 'Clusters::Cluster', clusters: 'Clusters::Cluster', + application_helm: 'Clusters::Applications::Helm', hooks: 'ProjectHook', merge_access_levels: 'ProtectedBranch::MergeAccessLevel', push_access_levels: 'ProtectedBranch::PushAccessLevel', diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 96efdd0949b..6eb266a7b94 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -147,7 +147,8 @@ deploy_keys: - user - deploy_keys_projects - projects -cluster: +clusters: +- application_helm - cluster_projects - projects - user @@ -160,6 +161,8 @@ provider_gcp: - cluster platform_kubernetes: - cluster +application_helm: +- cluster services: - project - service_hook @@ -191,6 +194,7 @@ project: - tags - chat_services - cluster +- clusters - cluster_project - creator - group @@ -299,4 +303,4 @@ push_event_payload: - event issue_assignees: - issue -- assignee \ No newline at end of file +- assignee -- cgit v1.2.1 From f676f388153fc9b5da0a76b2d5c2af9d8ffe832c Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Mon, 6 Nov 2017 18:21:10 +0100 Subject: Add more tests to Projects::Clusters::ApplicationsController --- .../projects/clusters/applications_controller.rb | 14 ++++++-------- .../projects/clusters/applications_controller_spec.rb | 13 +++++++++++++ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/app/controllers/projects/clusters/applications_controller.rb b/app/controllers/projects/clusters/applications_controller.rb index 4b9d54a8537..90c7fa62216 100644 --- a/app/controllers/projects/clusters/applications_controller.rb +++ b/app/controllers/projects/clusters/applications_controller.rb @@ -5,14 +5,12 @@ class Projects::Clusters::ApplicationsController < Projects::ApplicationControll before_action :authorize_create_cluster!, only: [:create] def create - scheduled = Clusters::Applications::ScheduleInstallationService.new(project, current_user, - application_class: @application_class, - cluster: @cluster).execute - if scheduled - head :no_content - else - head :bad_request - end + Clusters::Applications::ScheduleInstallationService.new(project, current_user, + application_class: @application_class, + cluster: @cluster).execute + head :no_content + rescue StandardError + head :bad_request end private diff --git a/spec/controllers/projects/clusters/applications_controller_spec.rb b/spec/controllers/projects/clusters/applications_controller_spec.rb index b8464b713c4..3213a797756 100644 --- a/spec/controllers/projects/clusters/applications_controller_spec.rb +++ b/spec/controllers/projects/clusters/applications_controller_spec.rb @@ -49,6 +49,19 @@ describe Projects::Clusters::ApplicationsController do expect(response).to have_http_status(:not_found) end end + + context 'when application is already installing' do + before do + other = current_application.new(cluster: cluster) + other.make_installing! + end + + it 'returns 400' do + go + + expect(response).to have_http_status(:bad_request) + end + end end describe 'security' do -- cgit v1.2.1 From 02f99feec25491c07777682b20f74f3b0b2b074d Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 6 Nov 2017 12:07:21 -0600 Subject: I18n general app error and update attributes from review --- .../clusters/components/application_row.vue | 26 +++++++++++++++------- .../clusters/components/applications.vue | 4 ++-- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/clusters/components/application_row.vue b/app/assets/javascripts/clusters/components/application_row.vue index 3dc658d0f1f..aee4e86ddac 100644 --- a/app/assets/javascripts/clusters/components/application_row.vue +++ b/app/assets/javascripts/clusters/components/application_row.vue @@ -1,5 +1,5 @@