From 8ec618a6ede619d9f75279f03c03b24d106c79c7 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Tue, 7 Nov 2017 15:26:14 +0100 Subject: [PATCH 1/4] Add Helm InstallCommand --- app/models/clusters/applications/helm.rb | 4 + .../applications/base_helm_service.rb | 4 + .../check_installation_progress_service.rb | 6 +- .../clusters/applications/install_service.rb | 2 +- lib/gitlab/kubernetes/helm.rb | 81 +++++++------- spec/lib/gitlab/kubernetes/helm_spec.rb | 100 ++++++++++++++++++ .../models/clusters/applications/helm_spec.rb | 6 ++ .../applications/install_service_spec.rb | 4 +- 8 files changed, 164 insertions(+), 43 deletions(-) create mode 100644 spec/lib/gitlab/kubernetes/helm_spec.rb diff --git a/app/models/clusters/applications/helm.rb b/app/models/clusters/applications/helm.rb index 7ea84841118..c7949d11ef8 100644 --- a/app/models/clusters/applications/helm.rb +++ b/app/models/clusters/applications/helm.rb @@ -26,6 +26,10 @@ module Clusters def name self.class.application_name end + + def install_command + Gitlab::Kubernetes::Helm::InstallCommand.new(name, true) + end end end end diff --git a/app/services/clusters/applications/base_helm_service.rb b/app/services/clusters/applications/base_helm_service.rb index 68320a3b267..9a4ce31cb39 100644 --- a/app/services/clusters/applications/base_helm_service.rb +++ b/app/services/clusters/applications/base_helm_service.rb @@ -20,6 +20,10 @@ module Clusters def helm_api @helm_api ||= Gitlab::Kubernetes::Helm.new(kubeclient) end + + def install_command + @install_command ||= app.install_command + 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 index 69bd3613cce..bde090eaeec 100644 --- a/app/services/clusters/applications/check_installation_progress_service.rb +++ b/app/services/clusters/applications/check_installation_progress_service.rb @@ -48,17 +48,17 @@ module Clusters end def remove_installation_pod - helm_api.delete_installation_pod!(app) + helm_api.delete_installation_pod!(install_command.pod_name) rescue # no-op end def installation_phase - helm_api.installation_status(app) + helm_api.installation_status(install_command.pod_name) end def installation_errors - helm_api.installation_log(app) + helm_api.installation_log(install_command.pod_name) end end end diff --git a/app/services/clusters/applications/install_service.rb b/app/services/clusters/applications/install_service.rb index 4eba19a474e..8ceeec687cd 100644 --- a/app/services/clusters/applications/install_service.rb +++ b/app/services/clusters/applications/install_service.rb @@ -6,7 +6,7 @@ module Clusters begin app.make_installing! - helm_api.install(app) + helm_api.install(install_command) ClusterWaitForAppInstallationWorker.perform_in( ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id) diff --git a/lib/gitlab/kubernetes/helm.rb b/lib/gitlab/kubernetes/helm.rb index 16b2abb7de2..7a50f07f3c5 100644 --- a/lib/gitlab/kubernetes/helm.rb +++ b/lib/gitlab/kubernetes/helm.rb @@ -3,27 +3,27 @@ module Gitlab class Helm HELM_VERSION = '2.7.0'.freeze NAMESPACE = 'gitlab-managed-apps'.freeze - COMMAND_SCRIPT = <<-EOS.freeze + INSTALL_DEPS = <<-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 + InstallCommand = Struct.new(:name, :install_helm, :chart) do + def pod_name + "install-#{name}" + end + end + def initialize(kubeclient) @kubeclient = kubeclient @namespace = Namespace.new(NAMESPACE, kubeclient) end - def init! - install(OpenStruct.new(name: 'helm')) - end - - def install(app) + def install(command) @namespace.ensure_exists! - @kubeclient.create_pod(pod_resource(app)) + @kubeclient.create_pod(pod_resource(command)) end ## @@ -33,31 +33,27 @@ module Gitlab # # values: "Pending", "Running", "Succeeded", "Failed", "Unknown" # - def installation_status(app) - @kubeclient.get_pod(pod_name(app), @namespace.name).status.phase + def installation_status(pod_name) + @kubeclient.get_pod(pod_name, @namespace.name).status.phase end - def installation_log(app) - @kubeclient.get_pod_log(pod_name(app), @namespace.name).body + def installation_log(pod_name) + @kubeclient.get_pod_log(pod_name, @namespace.name).body end - def delete_installation_pod!(app) - @kubeclient.delete_pod(pod_name(app), @namespace.name) + def delete_installation_pod!(pod_name) + @kubeclient.delete_pod(pod_name, @namespace.name) 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.name, labels: labels } + def pod_resource(command) + labels = { 'gitlab.org/action': 'install', 'gitlab.org/application': command.name } + metadata = { name: command.pod_name, namespace: @namespace.name, labels: labels } container = { name: 'helm', image: 'alpine:3.6', - env: generate_pod_env(app), + env: generate_pod_env(command), command: %w(/bin/sh), args: %w(-c $(COMMAND_SCRIPT)) } @@ -66,23 +62,34 @@ module Gitlab ::Kubeclient::Resource.new(metadata: metadata, spec: spec) end - def generate_pod_env(app) - env = { + def generate_pod_env(command) + { 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 } } + TILLER_NAMESPACE: @namespace.name, + COMMAND_SCRIPT: generate_script(command) + }.map { |key, value| { name: key, value: value } } end - def helm_install_comand(app) - "install #{app.chart} --name #{app.name} --namespace #{NAMESPACE}" + def generate_script(command) + [ + INSTALL_DEPS, + helm_init_command(command), + helm_install_command(command) + ].join("\n") + end + + def helm_init_command(command) + if command.install_helm + 'helm init >/dev/null' + else + 'helm init --client-only >/dev/null' + end + end + + def helm_install_command(command) + return if command.chart.nil? + + "helm install #{command.chart} --name #{command.name} --namespace #{@namespace.name} >/dev/null" end end end diff --git a/spec/lib/gitlab/kubernetes/helm_spec.rb b/spec/lib/gitlab/kubernetes/helm_spec.rb new file mode 100644 index 00000000000..15f99b0401f --- /dev/null +++ b/spec/lib/gitlab/kubernetes/helm_spec.rb @@ -0,0 +1,100 @@ +require 'spec_helper' + +describe Gitlab::Kubernetes::Helm do + let(:client) { double('kubernetes client') } + let(:helm) { described_class.new(client) } + let(:namespace) { Gitlab::Kubernetes::Namespace.new(described_class::NAMESPACE, client) } + let(:install_helm) { true } + let(:chart) { 'stable/a_chart' } + let(:application_name) { 'app_name' } + let(:command) { Gitlab::Kubernetes::Helm::InstallCommand.new(application_name, install_helm, chart) } + subject { helm } + + before do + allow(Gitlab::Kubernetes::Namespace).to receive(:new).with(described_class::NAMESPACE, client).and_return(namespace) + end + + describe '#initialize' do + it 'creates a namespace object' do + expect(Gitlab::Kubernetes::Namespace).to receive(:new).with(described_class::NAMESPACE, client) + + subject + end + end + + describe '#install' do + before do + allow(client).to receive(:create_pod).and_return(nil) + allow(namespace).to receive(:ensure_exists!).once + end + + it 'ensures the namespace exists before creating the POD' do + expect(namespace).to receive(:ensure_exists!).once.ordered + expect(client).to receive(:create_pod).once.ordered + + subject.install(command) + end + end + + describe '#installation_status' do + let(:phase) { Gitlab::Kubernetes::Pod::RUNNING } + let(:pod) { Kubeclient::Resource.new(status: { phase: phase }) } # partial representation + + it 'fetches POD phase from kubernetes cluster' do + expect(client).to receive(:get_pod).with(command.pod_name, described_class::NAMESPACE).once.and_return(pod) + + expect(subject.installation_status(command.pod_name)).to eq(phase) + end + end + + describe '#installation_log' do + let(:log) { 'some output' } + let(:response) { RestClient::Response.new(log) } + + it 'fetches POD phase from kubernetes cluster' do + expect(client).to receive(:get_pod_log).with(command.pod_name, described_class::NAMESPACE).once.and_return(response) + + expect(subject.installation_log(command.pod_name)).to eq(log) + end + end + + describe '#delete_installation_pod!' do + it 'deletes the POD from kubernetes cluster' do + expect(client).to receive(:delete_pod).with(command.pod_name, described_class::NAMESPACE).once + + subject.delete_installation_pod!(command.pod_name) + end + end + + describe '#helm_init_command' do + subject { helm.send(:helm_init_command, command) } + + context 'when command.install_helm is true' do + let(:install_helm) { true } + + it { is_expected.to eq('helm init >/dev/null') } + end + + context 'when command.install_helm is false' do + let(:install_helm) { false } + + it { is_expected.to eq('helm init --client-only >/dev/null') } + end + end + + describe '#helm_install_command' do + subject { helm.send(:helm_install_command, command) } + + context 'when command.chart is nil' do + let(:chart) { nil } + + it { is_expected.to be_nil } + end + + context 'when command.chart is set' do + let(:chart) { 'stable/a_chart' } + + it { is_expected.to eq("helm install #{chart} --name #{application_name} --namespace #{namespace.name} >/dev/null")} + end + end +end diff --git a/spec/models/clusters/applications/helm_spec.rb b/spec/models/clusters/applications/helm_spec.rb index beb10bf38c9..f8855079842 100644 --- a/spec/models/clusters/applications/helm_spec.rb +++ b/spec/models/clusters/applications/helm_spec.rb @@ -38,6 +38,12 @@ describe Clusters::Applications::Helm do end end + describe '#install_command' do + it 'has all the needed information' do + expect(subject.install_command).to have_attributes(name: subject.name, install_helm: true, chart: nil) + end + end + describe 'status state machine' do describe '#make_installing' do subject { create(:cluster_applications_helm, :scheduled) } diff --git a/spec/services/clusters/applications/install_service_spec.rb b/spec/services/clusters/applications/install_service_spec.rb index 408f7e4354e..054a49ffedf 100644 --- a/spec/services/clusters/applications/install_service_spec.rb +++ b/spec/services/clusters/applications/install_service_spec.rb @@ -12,7 +12,7 @@ describe Clusters::Applications::InstallService do context 'when there are no errors' do before do - expect(helm_client).to receive(:install).with(application) + expect(helm_client).to receive(:install).with(application.install_command) allow(ClusterWaitForAppInstallationWorker).to receive(:perform_in).and_return(nil) end @@ -33,7 +33,7 @@ describe Clusters::Applications::InstallService do context 'when k8s cluster communication fails' do before do error = KubeException.new(500, 'system failure', nil) - expect(helm_client).to receive(:install).with(application).and_raise(error) + expect(helm_client).to receive(:install).with(application.install_command).and_raise(error) end it 'make the application errored' do From e3b5dfdf20b2617cc706d82e9d52d67ec92d26b6 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Tue, 7 Nov 2017 15:31:49 +0100 Subject: [PATCH 2/4] Fix rubocop warning --- spec/factories/clusters/applications/helm.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/factories/clusters/applications/helm.rb b/spec/factories/clusters/applications/helm.rb index a0c874b103b..fab37195113 100644 --- a/spec/factories/clusters/applications/helm.rb +++ b/spec/factories/clusters/applications/helm.rb @@ -3,7 +3,7 @@ FactoryGirl.define do cluster factory: %i(cluster provided_by_gcp) trait :not_installable do - status -2 + status(-2) end trait :installable do From 93fe65702bfaaa12fd748c57fbbb819e3074ecef Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 8 Nov 2017 01:22:28 +0900 Subject: [PATCH 3/4] Add feature spec for app installation --- spec/features/projects/clusters_spec.rb | 42 +++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/spec/features/projects/clusters_spec.rb b/spec/features/projects/clusters_spec.rb index 368aad860e7..03d866f8bc1 100644 --- a/spec/features/projects/clusters_spec.rb +++ b/spec/features/projects/clusters_spec.rb @@ -50,10 +50,22 @@ feature 'Clusters', :js do it 'user sees a cluster details page and creation status' do expect(page).to have_content('Cluster is being created on Google Container Engine...') + # Application Installation buttons + expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') + expect(page.find(:css, '.js-cluster-application-install-button').text).to eq('Install') + Clusters::Cluster.last.provider.make_created! expect(page).to have_content('Cluster was successfully created on Google Container Engine') end + + it 'user sees a error if something worng during creation' do + expect(page).to have_content('Cluster is being created on Google Container Engine...') + + Clusters::Cluster.last.provider.make_errored!('Something wrong!') + + expect(page).to have_content('Something wrong!') + end end context 'when user filled form with invalid parameters' do @@ -78,6 +90,36 @@ feature 'Clusters', :js do it 'user sees an cluster details page' do expect(page).to have_button('Save') expect(page.find(:css, '.cluster-name').value).to eq(cluster.name) + + # Application Installation buttons + expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to be_nil + expect(page.find(:css, '.js-cluster-application-install-button')).to have_content('Install') + end + + context 'when user installs application: tiller' do + before do + allow(ClusterInstallAppWorker).to receive(:perform_async).and_return(nil) + + page.find(:css, '.js-cluster-application-install-button').click + end + + it 'user sees status transition' do + # FE sends request and gets the responce, then the buttons is "Install" + expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') + expect(page.find(:css, '.js-cluster-application-install-button')).to have_content('Install') + + Clusters::Cluster.last.application_helm.make_installing! + + # FE starts pooling and update the buttons to "Installing" + expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') + expect(page.find(:css, '.js-cluster-application-install-button')).to have_content('Installing') + + Clusters::Cluster.last.application_helm.make_installed! + + expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') + expect(page.find(:css, '.js-cluster-application-install-button')).to have_content('Installed') + expect(page).to have_content('Helm Tiller was successfully installed on your cluster') + end end context 'when user disables the cluster' do From 02878cd958557128cd9c22b27bd2fb97a843266b Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 8 Nov 2017 01:32:12 +0900 Subject: [PATCH 4/4] Fix spec. spec/serializers/cluster_application_entity_spec.rb and spec/serializers/cluster_entity_spec.rb --- spec/serializers/cluster_application_entity_spec.rb | 2 +- spec/serializers/cluster_entity_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/serializers/cluster_application_entity_spec.rb b/spec/serializers/cluster_application_entity_spec.rb index 8a3a081adf8..87c7b2ad36e 100644 --- a/spec/serializers/cluster_application_entity_spec.rb +++ b/spec/serializers/cluster_application_entity_spec.rb @@ -10,7 +10,7 @@ describe ClusterApplicationEntity do end it 'has status' do - expect(subject[:status]).to eq(:installable) + expect(subject[:status]).to eq(:not_installable) end it 'has no status_reason' do diff --git a/spec/serializers/cluster_entity_spec.rb b/spec/serializers/cluster_entity_spec.rb index f50f5999bfc..d6a43fd0f00 100644 --- a/spec/serializers/cluster_entity_spec.rb +++ b/spec/serializers/cluster_entity_spec.rb @@ -39,12 +39,12 @@ describe ClusterEntity do let(:cluster) { create(:cluster) } subject { described_class.new(cluster).as_json[:applications]} - it 'contains helm as installable' do + it 'contains helm as not_installable' do expect(subject).not_to be_empty helm = subject[0] expect(helm[:name]).to eq('helm') - expect(helm[:status]).to eq(:installable) + expect(helm[:status]).to eq(:not_installable) end end end