Merge branch '3371-filter-object-storage-backed-lfs-objects' into 'master'

Geo - Ignore S3-backed LFS objects on secondary nodes

Closes #3371

See merge request !2889

app/models/geo_node.rb

@@ -114,11 +114,14 @@ class GeoNode < ActiveRecord::Base
   end
 
   def lfs_objects
-    if restricted_project_ids
-      LfsObject.joins(:projects).where(projects: { id: restricted_project_ids })
-    else
-      LfsObject.all
-    end
+    relation =
+      if restricted_project_ids
+        LfsObject.joins(:projects).where(projects: { id: restricted_project_ids })
+      else
+        LfsObject.all
+      end
+
+    relation.with_files_stored_locally
   end
 
   def projects

changelogs/unreleased-ee/3371-filter-object-storage-backed-lfs-objects.yml

+---
+title: Geo - Ignore S3-backed LFS objects on secondary nodes
+merge_request: 2889
+author:
+type: other

spec/workers/geo/file_download_dispatch_worker_spec.rb

@@ -6,10 +6,9 @@ describe Geo::FileDownloadDispatchWorker do
 
   before do
     allow(Gitlab::Geo).to receive(:secondary?).and_return(true)
-    allow_any_instance_of(Gitlab::ExclusiveLease)
-      .to receive(:try_obtain).and_return(true)
-    allow_any_instance_of(Gitlab::ExclusiveLease)
-      .to receive(:renew).and_return(true)
+    allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(true)
+    allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:renew).and_return(true)
+    allow_any_instance_of(described_class).to receive(:over_time?).and_return(false)
     WebMock.stub_request(:get, /primary-geo-node/).to_return(status: 200, body: "", headers: {})
   end
 
@@ -37,13 +36,21 @@ describe Geo::FileDownloadDispatchWorker do
       subject.perform
     end
 
-    it 'executes GeoFileDownloadWorker for each LFS object' do
-      create_list(:lfs_object, 2, :with_file)
+    context 'with LFS objects' do
+      let!(:lfs_object_local_store) { create(:lfs_object, :with_file) }
+      let!(:lfs_object_remote_store) { create(:lfs_object, :with_file) }
 
-      allow_any_instance_of(described_class).to receive(:over_time?).and_return(false)
-      expect(GeoFileDownloadWorker).to receive(:perform_async).twice.and_call_original
+      before do
+        stub_lfs_object_storage
+        lfs_object_remote_store.file.migrate!(LfsObjectUploader::REMOTE_STORE)
+      end
 
-      subject.perform
+      it 'filters S3-backed files' do
+        expect(GeoFileDownloadWorker).to receive(:perform_async).with(:lfs, lfs_object_local_store.id)
+        expect(GeoFileDownloadWorker).not_to receive(:perform_async).with(:lfs, lfs_object_remote_store.id)
+
+        subject.perform
+      end
     end
 
     # Test the case where we have:
@@ -60,8 +67,6 @@ describe Geo::FileDownloadDispatchWorker do
       create_list(:note, 2, :with_attachment)
       create(:appearance, logo: avatar, header_logo: avatar)
 
-      allow_any_instance_of(described_class).to receive(:over_time?).and_return(false)
-
       expect(GeoFileDownloadWorker).to receive(:perform_async).exactly(8).times.and_call_original
       # For 8 downloads, we expect three database reloads:
       # 1. Load the first batch of 5.
@@ -82,7 +87,6 @@ describe Geo::FileDownloadDispatchWorker do
 
       before do
         allow(ProjectCacheWorker).to receive(:perform_async).and_return(true)
-        allow_any_instance_of(described_class).to receive(:over_time?).and_return(false)
 
         secondary.update_attribute(:namespaces, [synced_group])
       end