root
/
gitlab-ce
mirror of https://gitlab.com/free-releases/gitlab-ce.git
1
0
Fork 0
Code Issues Actions Packages Projects Releases Wiki Activity

Creates Clusterss::ApplciationsController update endpoint 

- Creates new route
- Creates new controller action
- Creates call stack:
  Clusterss::ApplciationsController calls -->
  Clusters::Applications::UpdateService calls -->
  Clusters::Applications::ScheduleUpdateService calls -->
  ClusterUpdateAppWorker calls -->
  Clusters::Applications::PatchService -->
  ClusterWaitForAppInstallationWorker

DRY req params

Adds gcp_cluster:cluster_update_app queue

Schedule_update_service is uneeded

Extract common logic to a parent class (UpdateService will need it)

Introduce new UpdateService

Fix rescue class namespace

Fix RuboCop offenses

Adds BaseService for create and update services

Remove request_handler code duplication

Fixes update command

Move update_command to ApplicationCore so all apps can use it

Adds tests for Knative update_command

Adds specs for PatchService

Raise error if update receives an unistalled app

Adds update_service spec

Fix RuboCop offense

Use subject in favor of go

Adds update endpoint specs for project namespace

Adds update endpoint specs for group namespace
This commit is contained in:
João Cunha 2019-02-18 17:06:51 +00:00 committed by jerasmus
parent cf1b85dd72
commit f8234d9a08
16 changed files with 569 additions and 104 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
app/controllers/clusters/applications_controller.rb
View File

@ -3,26 +3,41 @@
class Clusters::ApplicationsController < Clusters::BaseController
before_action :cluster
before_action :authorize_create_cluster!, only: [:create]
before_action :authorize_update_cluster!, only: [:update]
def create
Clusters::Applications::CreateService
.new(@cluster, current_user, create_cluster_application_params)
.execute(request)
request_handler do
Clusters::Applications::CreateService
.new(@cluster, current_user, cluster_application_params)
.execute(request)
end
end
def update
request_handler do
Clusters::Applications::UpdateService
.new(@cluster, current_user, cluster_application_params)
.execute(request)
end
end
private
def request_handler
yield
head :no_content
rescue Clusters::Applications::CreateService::InvalidApplicationError
rescue Clusters::Applications::BaseService::InvalidApplicationError
render_404
rescue StandardError
head :bad_request
end
private
def cluster
@cluster ||= clusterable.clusters.find(params[:id]) || render_404
end
def create_cluster_application_params
def cluster_application_params
params.permit(:application, :hostname, :email)
end
end

6
app/models/clusters/concerns/application_core.rb
View File

@ -30,6 +30,12 @@ module Clusters
# Override if you need extra data synchronized
# from K8s after installation
end
def update_command
command = install_command
command.version = version
command
end
end
end
end

4
app/services/clusters/applications/base_helm_service.rb
View File

@ -46,6 +46,10 @@ module Clusters
@install_command ||= app.install_command
end
def update_command
@update_command ||= app.update_command
end
def upgrade_command(new_values = "")
app.upgrade_command(new_values)
end

76
app/services/clusters/applications/base_service.rb Normal file
View File

@ -0,0 +1,76 @@
# frozen_string_literal: true
module Clusters
module Applications
class BaseService
InvalidApplicationError = Class.new(StandardError)
attr_reader :cluster, :current_user, :params
def initialize(cluster, user, params = {})
@cluster = cluster
@current_user = user
@params = params.dup
end
def execute(request)
instantiate_application.tap do |application|
if application.has_attribute?(:hostname)
application.hostname = params[:hostname]
end
if application.has_attribute?(:email)
application.email = params[:email]
end
if application.respond_to?(:oauth_application)
application.oauth_application = create_oauth_application(application, request)
end
worker = worker_class(application)
application.make_scheduled!
worker.perform_async(application.name, application.id)
end
end
protected
def worker_class(application)
raise NotImplementedError
end
def builders
raise NotImplementedError
end
def project_builders
raise NotImplementedError
end
def instantiate_application
builder.call(@cluster) || raise(InvalidApplicationError, "invalid application: #{application_name}")
end
def builder
builders[application_name] || raise(InvalidApplicationError, "invalid application: #{application_name}")
end
def application_name
params[:application]
end
def create_oauth_application(application, request)
oauth_application_params = {
name: params[:application],
redirect_uri: application.callback_url,
scopes: 'api read_user openid',
owner: current_user
}
::Applications::CreateService.new(current_user, oauth_application_params).execute(request)
end
end
end
end

57
app/services/clusters/applications/create_service.rb
View File

@ -2,47 +2,11 @@
module Clusters
module Applications
class CreateService
InvalidApplicationError = Class.new(StandardError)
attr_reader :cluster, :current_user, :params
def initialize(cluster, user, params = {})
@cluster = cluster
@current_user = user
@params = params.dup
end
def execute(request)
create_application.tap do |application|
if application.has_attribute?(:hostname)
application.hostname = params[:hostname]
end
if application.has_attribute?(:email)
application.email = params[:email]
end
if application.respond_to?(:oauth_application)
application.oauth_application = create_oauth_application(application, request)
end
worker = application.updateable? ? ClusterUpgradeAppWorker : ClusterInstallAppWorker
application.make_scheduled!
worker.perform_async(application.name, application.id)
end
end
class CreateService < Clusters::Applications::BaseService
private
def create_application
builder.call(@cluster)
end
def builder
builders[application_name] || raise(InvalidApplicationError, "invalid application: #{application_name}")
def worker_class(application)
application.updateable? ? ClusterUpgradeAppWorker : ClusterInstallAppWorker
end
def builders
@ -65,21 +29,6 @@ module Clusters
"knative" => -> (cluster) { cluster.application_knative || cluster.build_application_knative }
}
end
def application_name
params[:application]
end
def create_oauth_application(application, request)
oauth_application_params = {
name: params[:application],
redirect_uri: application.callback_url,
scopes: 'api read_user openid',
owner: current_user
}
::Applications::CreateService.new(current_user, oauth_application_params).execute(request)
end
end
end
end

26
app/services/clusters/applications/patch_service.rb Normal file
View File

@ -0,0 +1,26 @@
# frozen_string_literal: true
module Clusters
module Applications
class PatchService < BaseHelmService
def execute
return unless app.scheduled?
begin
app.make_updating!
helm_api.update(update_command)
ClusterWaitForAppInstallationWorker.perform_in(
ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id)
rescue Kubeclient::HttpError => e
log_error(e)
app.make_update_errored!("Kubernetes error: #{e.error_code}")
rescue StandardError => e
log_error(e)
app.make_update_errored!("Can't start update process.")
end
end
end
end
end

34
app/services/clusters/applications/update_service.rb Normal file
View File

@ -0,0 +1,34 @@
# frozen_string_literal: true
module Clusters
module Applications
class UpdateService < Clusters::Applications::BaseService
private
def worker_class(application)
ClusterUpdateAppWorker
end
def builders
{
"helm" => -> (cluster) { cluster.application_helm },
"ingress" => -> (cluster) { cluster.application_ingress },
"cert_manager" => -> (cluster) { cluster.application_cert_manager }
}.tap do |hash|
hash.merge!(project_builders) if cluster.project_type?
end
end
# These applications will need extra configuration to enable them to work
# with groups of projects
def project_builders
{
"prometheus" => -> (cluster) { cluster.application_prometheus },
"runner" => -> (cluster) { cluster.application_runner },
"jupyter" => -> (cluster) { cluster.application_jupyter },
"knative" => -> (cluster) { cluster.application_knative }
}
end
end
end
end

1
app/workers/all_queues.yml
View File

@ -23,6 +23,7 @@
- cronjob:prune_web_hook_logs
- gcp_cluster:cluster_install_app
- gcp_cluster:cluster_update_app
- gcp_cluster:cluster_upgrade_app
- gcp_cluster:cluster_provision
- gcp_cluster:cluster_wait_for_app_installation

13
app/workers/cluster_update_app_worker.rb Normal file
View File

@ -0,0 +1,13 @@
# frozen_string_literal: true
class ClusterUpdateAppWorker
include ApplicationWorker
include ClusterQueue
include ClusterApplications
def perform(app_name, app_id)
find_application(app_name, app_id) do |app|
Clusters::Applications::PatchService.new(app).execute
end
end
end

1
config/routes.rb
View File

@ -101,6 +101,7 @@ Rails.application.routes.draw do
member do
scope :applications do
post '/:application', to: 'clusters/applications#create', as: :install_applications
patch '/:application', to: 'clusters/applications#update', as: :update_applications
end
get :cluster_status, format: :json

3
lib/gitlab/kubernetes/helm/install_command.rb
View File

@ -7,7 +7,8 @@ module Gitlab
include BaseCommand
include ClientCommand
attr_reader :name, :files, :chart, :version, :repository, :preinstall, :postinstall
attr_reader :name, :files, :chart, :repository, :preinstall, :postinstall
attr_accessor :version
def initialize(name:, chart:, files:, rbac:, version: nil, repository: nil, preinstall: nil, postinstall: nil)
@name = name

100
spec/controllers/groups/clusters/applications_controller_spec.rb
View File

@ -9,9 +9,25 @@ describe Groups::Clusters::ApplicationsController do
Clusters::Cluster::APPLICATIONS[application]
end
shared_examples 'a secure endpoint' do
it { expect { subject }.to be_allowed_for(:admin) }
it { expect { subject }.to be_allowed_for(:owner).of(group) }
it { expect { subject }.to be_allowed_for(:maintainer).of(group) }
it { expect { subject }.to be_denied_for(:developer).of(group) }
it { expect { subject }.to be_denied_for(:reporter).of(group) }
it { expect { subject }.to be_denied_for(:guest).of(group) }
it { expect { subject }.to be_denied_for(:user) }
it { expect { subject }.to be_denied_for(:external) }
end
let(:cluster) { create(:cluster, :group, :provided_by_gcp) }
let(:group) { cluster.group }
describe 'POST create' do
let(:cluster) { create(:cluster, :group, :provided_by_gcp) }
let(:group) { cluster.group }
subject do
post :create, params: params.merge(group_id: group)
end
let(:application) { 'helm' }
let(:params) { { application: application, id: cluster.id } }
@ -26,7 +42,7 @@ describe Groups::Clusters::ApplicationsController do
it 'schedule an application installation' do
expect(ClusterInstallAppWorker).to receive(:perform_async).with(application, anything).once
expect { go }.to change { current_application.count }
expect { subject }.to change { current_application.count }
expect(response).to have_http_status(:no_content)
expect(cluster.application_helm).to be_scheduled
end
@ -37,7 +53,7 @@ describe Groups::Clusters::ApplicationsController do
end
it 'return 404' do
expect { go }.not_to change { current_application.count }
expect { subject }.not_to change { current_application.count }
expect(response).to have_http_status(:not_found)
end
end
@ -46,9 +62,7 @@ describe Groups::Clusters::ApplicationsController do
let(:application) { 'unkwnown-app' }
it 'return 404' do
go
expect(response).to have_http_status(:not_found)
is_expected.to have_http_status(:not_found)
end
end
@ -58,9 +72,7 @@ describe Groups::Clusters::ApplicationsController do
end
it 'returns 400' do
go
expect(response).to have_http_status(:bad_request)
is_expected.to have_http_status(:bad_request)
end
end
end
@ -70,18 +82,66 @@ describe Groups::Clusters::ApplicationsController 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(group) }
it { expect { go }.to be_allowed_for(:maintainer).of(group) }
it { expect { go }.to be_denied_for(:developer).of(group) }
it { expect { go }.to be_denied_for(:reporter).of(group) }
it { expect { go }.to be_denied_for(:guest).of(group) }
it { expect { go }.to be_denied_for(:user) }
it { expect { go }.to be_denied_for(:external) }
it_behaves_like 'a secure endpoint'
end
end
describe 'PATCH update' do
subject do
patch :update, params: params.merge(group_id: group)
end
def go
post :create, params: params.merge(group_id: group)
let!(:application) { create(:clusters_applications_cert_managers, :installed, cluster: cluster) }
let(:application_name) { application.name }
let(:params) { { application: application_name, id: cluster.id, email: "new-email@example.com" } }
describe 'functionality' do
let(:user) { create(:user) }
before do
group.add_maintainer(user)
sign_in(user)
end
context "when cluster and app exists" do
it "schedules an application update" do
expect(ClusterUpdateAppWorker).to receive(:perform_async).with(application.name, anything).once
is_expected.to have_http_status(:no_content)
expect(cluster.application_cert_manager).to be_scheduled
end
end
context 'when cluster do not exists' do
before do
cluster.destroy!
end
it { is_expected.to have_http_status(:not_found) }
end
context 'when application is unknown' do
let(:application_name) { 'unkwnown-app' }
it { is_expected.to have_http_status(:not_found) }
end
context 'when application is already scheduled' do
before do
application.make_scheduled!
end
it { is_expected.to have_http_status(:bad_request) }
end
end
describe 'security' do
before do
allow(ClusterUpdateAppWorker).to receive(:perform_async)
end
it_behaves_like 'a secure endpoint'
end
end
end

97
spec/controllers/projects/clusters/applications_controller_spec.rb
View File

@ -9,7 +9,22 @@ describe Projects::Clusters::ApplicationsController do
Clusters::Cluster::APPLICATIONS[application]
end
shared_examples 'a secure endpoint' do
it { expect { subject }.to be_allowed_for(:admin) }
it { expect { subject }.to be_allowed_for(:owner).of(project) }
it { expect { subject }.to be_allowed_for(:maintainer).of(project) }
it { expect { subject }.to be_denied_for(:developer).of(project) }
it { expect { subject }.to be_denied_for(:reporter).of(project) }
it { expect { subject }.to be_denied_for(:guest).of(project) }
it { expect { subject }.to be_denied_for(:user) }
it { expect { subject }.to be_denied_for(:external) }
end
describe 'POST create' do
subject do
post :create, params: params.merge(namespace_id: project.namespace, project_id: project)
end
let(:cluster) { create(:cluster, :project, :provided_by_gcp) }
let(:project) { cluster.project }
let(:application) { 'helm' }
@ -26,7 +41,7 @@ describe Projects::Clusters::ApplicationsController do
it 'schedule an application installation' do
expect(ClusterInstallAppWorker).to receive(:perform_async).with(application, anything).once
expect { go }.to change { current_application.count }
expect { subject }.to change { current_application.count }
expect(response).to have_http_status(:no_content)
expect(cluster.application_helm).to be_scheduled
end
@ -37,7 +52,7 @@ describe Projects::Clusters::ApplicationsController do
end
it 'return 404' do
expect { go }.not_to change { current_application.count }
expect { subject }.not_to change { current_application.count }
expect(response).to have_http_status(:not_found)
end
end
@ -46,9 +61,7 @@ describe Projects::Clusters::ApplicationsController do
let(:application) { 'unkwnown-app' }
it 'return 404' do
go
expect(response).to have_http_status(:not_found)
is_expected.to have_http_status(:not_found)
end
end
@ -58,9 +71,7 @@ describe Projects::Clusters::ApplicationsController do
end
it 'returns 400' do
go
expect(response).to have_http_status(:bad_request)
is_expected.to have_http_status(:bad_request)
end
end
end
@ -70,18 +81,68 @@ describe Projects::Clusters::ApplicationsController 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(:maintainer).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) }
it_behaves_like 'a secure endpoint'
end
end
describe 'PATCH update' do
subject do
patch :update, params: params.merge(namespace_id: project.namespace, project_id: project)
end
def go
post :create, params: params.merge(namespace_id: project.namespace, project_id: project)
let(:cluster) { create(:cluster, :project, :provided_by_gcp) }
let(:project) { cluster.project }
let!(:application) { create(:clusters_applications_knative, :installed, cluster: cluster) }
let(:application_name) { application.name }
let(:params) { { application: application_name, id: cluster.id, hostname: "new.example.com" } }
describe 'functionality' do
let(:user) { create(:user) }
before do
project.add_maintainer(user)
sign_in(user)
end
context "when cluster and app exists" do
it "schedules an application update" do
expect(ClusterUpdateAppWorker).to receive(:perform_async).with(application.name, anything).once
is_expected.to have_http_status(:no_content)
expect(cluster.application_knative).to be_scheduled
end
end
context 'when cluster do not exists' do
before do
cluster.destroy!
end
it { is_expected.to have_http_status(:not_found) }
end
context 'when application is unknown' do
let(:application_name) { 'unkwnown-app' }
it { is_expected.to have_http_status(:not_found) }
end
context 'when application is already scheduled' do
before do
application.make_scheduled!
end
it { is_expected.to have_http_status(:bad_request) }
end
end
describe 'security' do
before do
allow(ClusterUpdateAppWorker).to receive(:perform_async)
end
it_behaves_like 'a secure endpoint'
end
end
end

26
spec/models/clusters/applications/knative_spec.rb
View File

@ -66,9 +66,7 @@ describe Clusters::Applications::Knative do
end
end
describe '#install_command' do
subject { knative.install_command }
shared_examples 'a command' do
it 'should be an instance of Helm::InstallCommand' do
expect(subject).to be_an_instance_of(Gitlab::Kubernetes::Helm::InstallCommand)
end
@ -76,7 +74,6 @@ describe Clusters::Applications::Knative do
it 'should be initialized with knative arguments' do
expect(subject.name).to eq('knative')
expect(subject.chart).to eq('knative/knative')
expect(subject.version).to eq('0.2.2')
expect(subject.files).to eq(knative.files)
end
@ -98,6 +95,27 @@ describe Clusters::Applications::Knative do
end
end
describe '#install_command' do
subject { knative.install_command }
it 'should be initialized with latest version' do
expect(subject.version).to eq('0.2.2')
end
it_behaves_like 'a command'
end
describe '#update_command' do
let!(:current_installed_version) { knative.version = '0.1.0' }
subject { knative.update_command }
it 'should be initialized with current version' do
expect(subject.version).to eq(current_installed_version)
end
it_behaves_like 'a command'
end
describe '#files' do
let(:application) { knative }
let(:values) { subject[:'values.yaml'] }

128
spec/services/clusters/applications/patch_service_spec.rb Normal file
View File

@ -0,0 +1,128 @@
# frozen_string_literal: true
require 'spec_helper'
describe Clusters::Applications::PatchService do
describe '#execute' do
let(:application) { create(:clusters_applications_knative, :scheduled) }
let!(:update_command) { application.update_command }
let(:service) { described_class.new(application) }
let(:helm_client) { instance_double(Gitlab::Kubernetes::Helm::Api) }
before do
allow(service).to receive(:update_command).and_return(update_command)
allow(service).to receive(:helm_api).and_return(helm_client)
end
context 'when there are no errors' do
before do
expect(helm_client).to receive(:update).with(update_command)
allow(ClusterWaitForAppInstallationWorker).to receive(:perform_in).and_return(nil)
end
it 'make the application updating' do
expect(application.cluster).not_to be_nil
service.execute
expect(application).to be_updating
end
it 'schedule async installation status check' do
expect(ClusterWaitForAppInstallationWorker).to receive(:perform_in).once
service.execute
end
end
context 'when kubernetes cluster communication fails' do
let(:error) { Kubeclient::HttpError.new(500, 'system failure', nil) }
before do
expect(helm_client).to receive(:update).with(update_command).and_raise(error)
end
it 'make the application errored' do
service.execute
expect(application).to be_update_errored
expect(application.status_reason).to match('Kubernetes error: 500')
end
it 'logs errors' do
expect(service.send(:logger)).to receive(:error).with(
{
exception: 'Kubeclient::HttpError',
message: 'system failure',
service: 'Clusters::Applications::PatchService',
app_id: application.id,
project_ids: application.cluster.project_ids,
group_ids: [],
error_code: 500
}
)
expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with(
error,
extra: {
exception: 'Kubeclient::HttpError',
message: 'system failure',
service: 'Clusters::Applications::PatchService',
app_id: application.id,
project_ids: application.cluster.project_ids,
group_ids: [],
error_code: 500
}
)
service.execute
end
end
context 'a non kubernetes error happens' do
let(:application) { create(:clusters_applications_knative, :scheduled) }
let(:error) { StandardError.new('something bad happened') }
before do
expect(application).to receive(:make_updating!).once.and_raise(error)
end
it 'make the application errored' do
expect(helm_client).not_to receive(:update)
service.execute
expect(application).to be_update_errored
expect(application.status_reason).to eq("Can't start update process.")
end
it 'logs errors' do
expect(service.send(:logger)).to receive(:error).with(
{
exception: 'StandardError',
error_code: nil,
message: 'something bad happened',
service: 'Clusters::Applications::PatchService',
app_id: application.id,
project_ids: application.cluster.projects.pluck(:id),
group_ids: []
}
)
expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with(
error,
extra: {
exception: 'StandardError',
error_code: nil,
message: 'something bad happened',
service: 'Clusters::Applications::PatchService',
app_id: application.id,
project_ids: application.cluster.projects.pluck(:id),
group_ids: []
}
)
service.execute
end
end
end
end

72
spec/services/clusters/applications/update_service_spec.rb Normal file
View File

@ -0,0 +1,72 @@
# frozen_string_literal: true
require 'spec_helper'
describe Clusters::Applications::UpdateService do
include TestRequestHelpers
let(:cluster) { create(:cluster, :project, :provided_by_gcp) }
let(:user) { create(:user) }
let(:params) { { application: 'knative', hostname: 'udpate.example.com' } }
let(:service) { described_class.new(cluster, user, params) }
subject { service.execute(test_request) }
describe '#execute' do
before do
allow(ClusterUpdateAppWorker).to receive(:perform_async)
end
context 'application is not installed' do
it 'raises Clusters::Applications::BaseService::InvalidApplicationError' do
expect(ClusterUpdateAppWorker).not_to receive(:perform_async)
expect { subject }
.to raise_exception { Clusters::Applications::BaseService::InvalidApplicationError }
.and not_change { Clusters::Applications::Knative.count }
.and not_change { Clusters::Applications::Knative.with_status(:scheduled).count }
end
end
context 'application is installed' do
context 'application is schedulable' do
let!(:application) do
create(:clusters_applications_knative, status: 3, cluster: cluster)
end
it 'updates the application data' do
expect do
subject
end.to change { application.reload.hostname }.to(params[:hostname])
end
it 'makes application scheduled!' do
subject
expect(application.reload).to be_scheduled
end
it 'schedules ClusterUpdateAppWorker' do
expect(ClusterUpdateAppWorker).to receive(:perform_async)
subject
end
end
context 'application is not schedulable' do
let!(:application) do
create(:clusters_applications_knative, status: 4, cluster: cluster)
end
it 'raises StateMachines::InvalidTransition' do
expect(ClusterUpdateAppWorker).not_to receive(:perform_async)
expect { subject }
.to raise_exception { StateMachines::InvalidTransition }
.and not_change { application.reload.hostname }
.and not_change { Clusters::Applications::Knative.with_status(:scheduled).count }
end
end
end
end
end