Persist external IP of ingress controller created for GKE (#42643)

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

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

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

app/services/clusters/applications/check_ingress_ip_address_service.rb

+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

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

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

app/workers/cluster_wait_for_ingress_ip_address_worker.rb

+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

db/migrate/20180212030105_add_external_ip_to_clusters_applications_ingress.rb

+# 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

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|

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" ]
     }

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

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

spec/services/clusters/applications/check_ingress_ip_address_service_spec.rb

+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

spec/workers/cluster_wait_for_ingress_ip_address_worker_spec.rb

+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