Merge branch '28781-migrate-pages-metadata-on-demand' into 'master'

Migrate pages metadata when asked to serve virtual domain

See merge request gitlab-org/gitlab!17204

app/models/namespace.rb

@@ -319,6 +319,12 @@ class Namespace < ApplicationRecord
   private
 
   def all_projects_with_pages
+    if all_projects.pages_metadata_not_migrated.exists?
+      Gitlab::BackgroundMigration::MigratePagesMetadata.new.perform_on_relation(
+        all_projects.pages_metadata_not_migrated
+      )
+    end
+
     all_projects.with_pages_deployed
   end
 

app/models/pages_domain.rb

@@ -194,6 +194,16 @@ class PagesDomain < ApplicationRecord
   private
 
   def pages_deployed?
+    # TODO: remove once `pages_metadatum` is migrated
+    # https://gitlab.com/gitlab-org/gitlab/issues/33106
+    unless project.pages_metadatum
+      Gitlab::BackgroundMigration::MigratePagesMetadata
+        .new
+        .perform_on_relation(Project.where(id: project_id))
+
+      project.reset
+    end
+
     project.pages_metadatum&.deployed?
   end
 

app/models/project.rb

@@ -433,6 +433,11 @@ class Project < ApplicationRecord
     joins(:pages_metadatum).merge(ProjectPagesMetadatum.deployed)
   end
 
+  scope :pages_metadata_not_migrated, -> do
+    left_outer_joins(:pages_metadatum)
+      .where(project_pages_metadata: { project_id: nil })
+  end
+
   enum auto_cancel_pending_pipelines: { disabled: 0, enabled: 1 }
 
   chronic_duration_attr :build_timeout_human_readable, :build_timeout,

changelogs/unreleased/28781-migrate-pages-metadata-on-demand.yml

+---
+title: Add index on ci_builds for successful Pages deploys
+merge_request: 17204
+author:
+type: added

db/migrate/20190919040324_add_successfull_pages_deploy_partial_index_on_ci_builds.rb

+# frozen_string_literal: true
+
+class AddSuccessfullPagesDeployPartialIndexOnCiBuilds < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'index_ci_builds_on_project_id_for_successfull_pages_deploy'
+
+  def up
+    add_concurrent_index(
+      :ci_builds, :project_id,
+      name: INDEX_NAME,
+      where: "type='GenericCommitStatus' AND stage='deploy' AND name='pages:deploy' AND status = 'success'"
+    )
+  end
+
+  def down
+    remove_concurrent_index_by_name :ci_builds, INDEX_NAME
+  end
+end

db/schema.rb

@@ -643,6 +643,7 @@ ActiveRecord::Schema.define(version: 2019_09_27_074328) do
     t.index ["name"], name: "index_ci_builds_on_name_for_security_products_values", where: "((name)::text = ANY (ARRAY[('container_scanning'::character varying)::text, ('dast'::character varying)::text, ('dependency_scanning'::character varying)::text, ('license_management'::character varying)::text, ('sast'::character varying)::text]))"
     t.index ["project_id", "id"], name: "index_ci_builds_on_project_id_and_id"
     t.index ["project_id", "status"], name: "index_ci_builds_project_id_and_status_for_live_jobs_partial2", where: "(((type)::text = 'Ci::Build'::text) AND ((status)::text = ANY (ARRAY[('running'::character varying)::text, ('pending'::character varying)::text, ('created'::character varying)::text])))"
+    t.index ["project_id"], name: "index_ci_builds_on_project_id_for_successfull_pages_deploy", where: "(((type)::text = 'GenericCommitStatus'::text) AND ((stage)::text = 'deploy'::text) AND ((name)::text = 'pages:deploy'::text) AND ((status)::text = 'success'::text))"
     t.index ["protected"], name: "index_ci_builds_on_protected"
     t.index ["queued_at"], name: "index_ci_builds_on_queued_at"
     t.index ["runner_id"], name: "index_ci_builds_on_runner_id"

lib/gitlab/background_migration/migrate_pages_metadata.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # Class that will insert record into project_pages_metadata
+    # for each existing project
+    class MigratePagesMetadata
+      def perform(start_id, stop_id)
+        perform_on_relation(Project.where(id: start_id..stop_id))
+      end
+
+      def perform_on_relation(relation)
+        successful_pages_deploy = <<~SQL
+          SELECT TRUE
+          FROM ci_builds
+          WHERE ci_builds.type = 'GenericCommitStatus'
+            AND ci_builds.status = 'success'
+            AND ci_builds.stage = 'deploy'
+            AND ci_builds.name = 'pages:deploy'
+            AND ci_builds.project_id = projects.id
+          LIMIT 1
+        SQL
+
+        select_from = relation
+          .select("projects.id", "COALESCE((#{successful_pages_deploy}), FALSE)")
+          .to_sql
+
+        ActiveRecord::Base.connection_pool.with_connection do |connection|
+          connection.execute <<~SQL
+            INSERT INTO project_pages_metadata (project_id, deployed)
+          #{select_from}
+            ON CONFLICT (project_id) DO NOTHING
+          SQL
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/background_migration/migrate_pages_metadata_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::MigratePagesMetadata, :migration, schema: 20190919040324 do
+  let(:projects) { table(:projects) }
+
+  subject(:migrate_pages_metadata) { described_class.new }
+
+  describe '#perform_on_relation' do
+    let(:namespaces) { table(:namespaces) }
+    let(:builds) { table(:ci_builds) }
+    let(:pages_metadata) { table(:project_pages_metadata) }
+
+    it 'marks specified projects with successful pages deployment' do
+      namespace = namespaces.create!(name: 'gitlab', path: 'gitlab-org')
+      not_migrated_with_pages = projects.create!(namespace_id: namespace.id, name: 'Not Migrated With Pages')
+      builds.create!(project_id: not_migrated_with_pages.id, type: 'GenericCommitStatus', status: 'success', stage: 'deploy', name: 'pages:deploy')
+
+      migrated = projects.create!(namespace_id: namespace.id, name: 'Migrated')
+      pages_metadata.create!(project_id: migrated.id, deployed: true)
+
+      not_migrated_no_pages = projects.create!(namespace_id: namespace.id, name: 'Not Migrated No Pages')
+      project_not_in_relation_scope = projects.create!(namespace_id: namespace.id, name: 'Other')
+
+      projects_relation = projects.where(id: [not_migrated_with_pages, not_migrated_no_pages, migrated])
+
+      migrate_pages_metadata.perform_on_relation(projects_relation)
+
+      expect(pages_metadata.find_by_project_id(not_migrated_with_pages.id).deployed).to eq(true)
+      expect(pages_metadata.find_by_project_id(not_migrated_no_pages.id).deployed).to eq(false)
+      expect(pages_metadata.find_by_project_id(migrated.id).deployed).to eq(true)
+      expect(pages_metadata.find_by_project_id(project_not_in_relation_scope.id)).to be_nil
+    end
+  end
+
+  describe '#perform' do
+    it 'creates relation and delegates to #perform_on_relation' do
+      expect(migrate_pages_metadata).to receive(:perform_on_relation).with(projects.where(id: 3..5))
+
+      migrate_pages_metadata.perform(3, 5)
+    end
+  end
+end

spec/models/namespace_spec.rb

@@ -928,12 +928,34 @@ describe Namespace do
     let(:project) { create(:project, namespace: namespace) }
 
     context 'when there are pages deployed for the project' do
-      before do
-        project.mark_pages_as_deployed
+      context 'but pages metadata is not migrated' do
+        before do
+          generic_commit_status = create(:generic_commit_status, :success, stage: 'deploy', name: 'pages:deploy')
+          generic_commit_status.update!(project: project)
+          project.pages_metadatum.destroy!
+        end
+
+        it 'migrates pages metadata and returns the virual domain' do
+          virtual_domain = namespace.pages_virtual_domain
+
+          expect(project.reload.pages_metadatum.deployed).to eq(true)
+
+          expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain)
+          expect(virtual_domain.lookup_paths).not_to be_empty
+        end
       end
 
-      it 'returns the virual domain' do
-        expect(namespace.pages_virtual_domain).to be_an_instance_of(Pages::VirtualDomain)
+      context 'and pages metadata is migrated' do
+        before do
+          project.mark_pages_as_deployed
+        end
+
+        it 'returns the virual domain' do
+          virtual_domain = namespace.pages_virtual_domain
+
+          expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain)
+          expect(virtual_domain.lookup_paths).not_to be_empty
+        end
       end
     end
   end

spec/models/pages_domain_spec.rb

@@ -569,7 +569,9 @@ describe PagesDomain do
 
     context 'when there are pages deployed for the project' do
       before do
-        project.mark_pages_as_deployed
+        generic_commit_status = create(:generic_commit_status, :success, stage: 'deploy', name: 'pages:deploy')
+        generic_commit_status.update!(project: project)
+        project.pages_metadatum.destroy!
         project.reload
       end
 
@@ -578,6 +580,12 @@ describe PagesDomain do
 
         expect(pages_domain.pages_virtual_domain).to be_an_instance_of(Pages::VirtualDomain)
       end
+
+      it 'migrates project pages metadata' do
+        expect { pages_domain.pages_virtual_domain }.to change {
+          project.reload.pages_metadatum&.deployed
+        }.from(nil).to(true)
+      end
     end
   end
 end

spec/models/project_spec.rb

@@ -5107,6 +5107,16 @@ describe Project do
     end
   end
 
+  describe '.pages_metadata_not_migrated' do
+    it 'returns only projects that have pages deployed' do
+      _project_with_pages_metadata_migrated = create(:project)
+      project_with_pages_metadata_not_migrated = create(:project)
+      project_with_pages_metadata_not_migrated.pages_metadatum.destroy!
+
+      expect(described_class.pages_metadata_not_migrated).to contain_exactly(project_with_pages_metadata_not_migrated)
+    end
+  end
+
   describe '#pages_group_root?' do
     it 'returns returns true if pages_url is same as pages_group_url' do
       project = build(:project)