diff options
14 files changed, 229 insertions, 2 deletions
diff --git a/app/models/clusters/applications/ingress.rb b/app/models/clusters/applications/ingress.rb index aa5cf97756f..5e9086aecca 100644 --- a/app/models/clusters/applications/ingress.rb +++ b/app/models/clusters/applications/ingress.rb @@ -13,6 +13,8 @@ module Clusters nginx: 1 } + IP_ADDRESS_FETCH_RETRIES = 3 + def chart 'stable/nginx-ingress' end @@ -24,6 +26,11 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new(name, chart: chart, chart_values_file: chart_values_file) end + + def post_install + ClusterWaitForIngressIpAddressWorker.perform_in( + ClusterWaitForIngressIpAddressWorker::INTERVAL, name, id, IP_ADDRESS_FETCH_RETRIES) + end end end end diff --git a/app/models/clusters/concerns/application_core.rb b/app/models/clusters/concerns/application_core.rb index a98fa85a5ff..b047fbce214 100644 --- a/app/models/clusters/concerns/application_core.rb +++ b/app/models/clusters/concerns/application_core.rb @@ -23,6 +23,10 @@ module Clusters def name self.class.application_name end + + def post_install + # Override for any extra work that needs to be done after install + end end end end diff --git a/app/serializers/cluster_application_entity.rb b/app/serializers/cluster_application_entity.rb index 3f9a275ad08..b22a0b666ef 100644 --- a/app/serializers/cluster_application_entity.rb +++ b/app/serializers/cluster_application_entity.rb @@ -2,4 +2,5 @@ class ClusterApplicationEntity < Grape::Entity expose :name expose :status_name, as: :status expose :status_reason + expose :external_ip, if: -> (e, _) { e.respond_to?(:external_ip) } end diff --git a/app/services/clusters/applications/check_ingress_ip_address_service.rb b/app/services/clusters/applications/check_ingress_ip_address_service.rb new file mode 100644 index 00000000000..cf132676aa6 --- /dev/null +++ b/app/services/clusters/applications/check_ingress_ip_address_service.rb @@ -0,0 +1,37 @@ +module Clusters + module Applications + class CheckIngressIpAddressService < BaseHelmService + def execute(retries_remaining) + return if app.external_ip + + service = get_service + + if service.status.loadBalancer.ingress + resolve_external_ip(service) + else + retry_if_necessary(retries_remaining) + end + + rescue KubeException + retry_if_necessary(retries_remaining) + end + + private + + def resolve_external_ip(service) + app.update!( external_ip: service.status.loadBalancer.ingress[0].ip) + end + + def get_service + kubeclient.get_service('ingress-nginx-ingress-controller', Gitlab::Kubernetes::Helm::NAMESPACE) + end + + def retry_if_necessary(retries_remaining) + if retries_remaining > 0 + ClusterWaitForIngressIpAddressWorker.perform_in( + ClusterWaitForIngressIpAddressWorker::INTERVAL, app.name, app.id, retries_remaining - 1) + end + 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 bde090eaeec..7dcddc1c3f7 100644 --- a/app/services/clusters/applications/check_installation_progress_service.rb +++ b/app/services/clusters/applications/check_installation_progress_service.rb @@ -20,6 +20,7 @@ module Clusters def on_success app.make_installed! + app.post_install ensure remove_installation_pod end diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index f2c20114534..35ffa5d5fda 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -23,6 +23,7 @@ - gcp_cluster:cluster_wait_for_app_installation - gcp_cluster:wait_for_cluster_creation - gcp_cluster:check_gcp_project_billing +- gcp_cluster:cluster_wait_for_ingress_ip_address - github_import_advance_stage - github_importer:github_import_import_diff_note diff --git a/app/workers/cluster_wait_for_ingress_ip_address_worker.rb b/app/workers/cluster_wait_for_ingress_ip_address_worker.rb new file mode 100644 index 00000000000..829417484cf --- /dev/null +++ b/app/workers/cluster_wait_for_ingress_ip_address_worker.rb @@ -0,0 +1,14 @@ +class ClusterWaitForIngressIpAddressWorker + include ApplicationWorker + include ClusterQueue + include ClusterApplications + + INTERVAL = 10.seconds + TIMEOUT = 20.minutes + + def perform(app_name, app_id, retries_remaining) + find_application(app_name, app_id) do |app| + Clusters::Applications::CheckIngressIpAddressService.new(app).execute(retries_remaining) + end + end +end diff --git a/db/migrate/20180212030105_add_external_ip_to_clusters_applications_ingress.rb b/db/migrate/20180212030105_add_external_ip_to_clusters_applications_ingress.rb new file mode 100644 index 00000000000..c18d1d7a67b --- /dev/null +++ b/db/migrate/20180212030105_add_external_ip_to_clusters_applications_ingress.rb @@ -0,0 +1,31 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddExternalIpToClustersApplicationsIngress < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index", "remove_concurrent_index" or + # "add_column_with_default" you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + add_column :clusters_applications_ingress, :external_ip, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 6b43fc8403c..e13415926ec 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: 20180208183958) do +ActiveRecord::Schema.define(version: 20180212030105) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -568,6 +568,7 @@ ActiveRecord::Schema.define(version: 20180208183958) do t.string "version", null: false t.string "cluster_ip" t.text "status_reason" + t.string "external_ip" end create_table "clusters_applications_prometheus", force: :cascade do |t| diff --git a/spec/fixtures/api/schemas/cluster_status.json b/spec/fixtures/api/schemas/cluster_status.json index 489d563be2b..9617157ee37 100644 --- a/spec/fixtures/api/schemas/cluster_status.json +++ b/spec/fixtures/api/schemas/cluster_status.json @@ -30,7 +30,8 @@ ] } }, - "status_reason": { "type": ["string", "null"] } + "status_reason": { "type": ["string", "null"] }, + "external_ip": { "type": ["string", null] } }, "required" : [ "name", "status" ] } diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index 619c088b0bf..da9535f9d16 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -5,4 +5,18 @@ describe Clusters::Applications::Ingress do it { is_expected.to validate_presence_of(:cluster) } include_examples 'cluster application specs', described_class + + describe '#post_install' do + let(:application) { create(:clusters_applications_ingress, :installed) } + + before do + allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in) + application.post_install + end + + it 'schedules a ClusterWaitForIngressIpAddressWorker' do + expect(ClusterWaitForIngressIpAddressWorker).to have_received(:perform_in) + .with(ClusterWaitForIngressIpAddressWorker::INTERVAL, 'ingress', application.id, 3) + end + end end diff --git a/spec/serializers/cluster_application_entity_spec.rb b/spec/serializers/cluster_application_entity_spec.rb index b5a55b4ef6e..70b274083b2 100644 --- a/spec/serializers/cluster_application_entity_spec.rb +++ b/spec/serializers/cluster_application_entity_spec.rb @@ -26,5 +26,19 @@ describe ClusterApplicationEntity do expect(subject[:status_reason]).to eq(application.status_reason) end end + + context 'for ingress application' do + let(:application) do + build( + :clusters_applications_ingress, + :installed, + external_ip: '111.222.111.222', + ) + end + + it 'includes external_ip' do + expect(subject[:external_ip]).to eq('111.222.111.222') + end + end end end diff --git a/spec/services/clusters/applications/check_ingress_ip_address_service_spec.rb b/spec/services/clusters/applications/check_ingress_ip_address_service_spec.rb new file mode 100644 index 00000000000..5d99196fbe6 --- /dev/null +++ b/spec/services/clusters/applications/check_ingress_ip_address_service_spec.rb @@ -0,0 +1,81 @@ +require 'spec_helper' + +describe Clusters::Applications::CheckIngressIpAddressService do + let(:application) { create(:clusters_applications_ingress, :installed) } + let(:service) { described_class.new(application) } + let(:kube_service) do + ::Kubeclient::Resource.new( + { + status: { + loadBalancer: { + ingress: ingress + } + } + } + ) + end + let(:kubeclient) { double(::Kubeclient::Client, get_service: kube_service) } + let(:ingress) { [{ ip: '111.222.111.222' }] } + + before do + allow(application.cluster).to receive(:kubeclient).and_return(kubeclient) + end + + describe '#execute' do + context 'when the ingress ip address is available' do + it 'updates the external_ip for the app and does not schedule another worker' do + expect(ClusterWaitForIngressIpAddressWorker).not_to receive(:perform_in) + + service.execute(1) + + expect(application.external_ip).to eq('111.222.111.222') + end + end + + context 'when the ingress ip address is not available' do + let(:ingress) { nil } + + it 'it schedules another worker with 1 less retry' do + expect(ClusterWaitForIngressIpAddressWorker) + .to receive(:perform_in) + .with(ClusterWaitForIngressIpAddressWorker::INTERVAL, 'ingress', application.id, 0) + + service.execute(1) + end + + context 'when no more retries remaining' do + it 'does not schedule another worker' do + expect(ClusterWaitForIngressIpAddressWorker).not_to receive(:perform_in) + + service.execute(0) + end + end + end + + context 'when there is already an external_ip' do + let(:application) { create(:clusters_applications_ingress, :installed, external_ip: '001.111.002.111') } + + it 'does nothing' do + expect(kubeclient).not_to receive(:get_service) + + service.execute(1) + + expect(application.external_ip).to eq('001.111.002.111') + end + end + + context 'when a kubernetes error occurs' do + before do + allow(kubeclient).to receive(:get_service).and_raise(KubeException.new(500, 'something blew up', nil)) + end + + it 'it schedules another worker with 1 less retry' do + expect(ClusterWaitForIngressIpAddressWorker) + .to receive(:perform_in) + .with(ClusterWaitForIngressIpAddressWorker::INTERVAL, 'ingress', application.id, 0) + + service.execute(1) + end + end + end +end diff --git a/spec/workers/cluster_wait_for_ingress_ip_address_worker_spec.rb b/spec/workers/cluster_wait_for_ingress_ip_address_worker_spec.rb new file mode 100644 index 00000000000..9f8bf35f604 --- /dev/null +++ b/spec/workers/cluster_wait_for_ingress_ip_address_worker_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe ClusterWaitForIngressIpAddressWorker do + describe '#perform' do + let(:service) { instance_double(Clusters::Applications::CheckIngressIpAddressService) } + let(:application) { instance_double(Clusters::Applications::Ingress) } + let(:worker) { described_class.new } + + it 'finds the application and calls CheckIngressIpAddressService#execute' do + expect(worker).to receive(:find_application).with('ingress', 117).and_yield(application) + expect(Clusters::Applications::CheckIngressIpAddressService) + .to receive(:new) + .with(application) + .and_return(service) + expect(service).to receive(:execute).with(2) + + worker.perform('ingress', 117, 2) + end + end +end |