diff --git a/exodus_gw/settings.py b/exodus_gw/settings.py index ffc4eee2..381a9150 100644 --- a/exodus_gw/settings.py +++ b/exodus_gw/settings.py @@ -377,6 +377,24 @@ class Settings(BaseSettings): ] """List of file names that should be saved for last when publishing.""" + phase2_patterns: list[re.Pattern[str]] = [ + # kickstart repos; note the logic here matches + # the manual workaround RHELDST-27642 + re.compile(r"/kickstart/.*(? bool: + # Return True if item should be handled in phase 2 of commit. + name = basename(item.web_uri) + if ( + name == self.settings.autoindex_filename + or name in self.settings.entry_point_files + ): + # Typical entrypoint + return True + + for pattern in self.settings.phase2_patterns: + # Arbitrary patterns from settings. + # e.g. this is where /kickstart/ is expected to be handled. + if pattern.search(item.web_uri): + LOG.debug( + "%s: phase2 forced via pattern %s", item.web_uri, pattern + ) + return True + + return False + @property def item_select(self): # Returns base of the SELECT query to find all items for commit. @@ -334,9 +355,6 @@ def write_publish_items(self) -> list[Item]: # Save any entry point items to publish last. final_items: list[Item] = [] - final_basenames = self.settings.entry_point_files + [ - self.settings.autoindex_filename - ] wrote_count = 0 @@ -358,7 +376,7 @@ def write_publish_items(self) -> list[Item]: self.check_item(item) - if basename(item.web_uri) in final_basenames: + if self.is_phase2(item): LOG.debug( "Delayed write for %s", item.web_uri, diff --git a/tests/worker/test_publish.py b/tests/worker/test_publish.py index 76487a2c..f01d3041 100644 --- a/tests/worker/test_publish.py +++ b/tests/worker/test_publish.py @@ -9,7 +9,7 @@ from sqlalchemy import not_ from exodus_gw import models, worker -from exodus_gw.models import Publish +from exodus_gw.models import Item, Publish from exodus_gw.models.path import PublishedPath from exodus_gw.settings import load_settings @@ -27,6 +27,32 @@ def _task(publish_id): ) +def add_kickstart(publish: Publish): + """Adds some kickstart items onto a publish.""" + publish.items.extend( + [ + models.Item( + web_uri="/content/testproduct/1/kickstart/extra_files.json", + object_key="cee38a35950f3f9465378b1548c4495882da0bfbe217999add63cb3a8e2c4d75", + publish_id=publish.id, + updated=datetime(2023, 10, 4, 3, 52, 2), + ), + models.Item( + web_uri="/content/testproduct/1/kickstart/EULA", + object_key="6c92384cdbf1a8c448278aaffaf7d8c3f048749e201d504ffaab07d85f6b1a03", + publish_id=publish.id, + updated=datetime(2023, 10, 4, 3, 52, 2), + ), + models.Item( + web_uri="/content/testproduct/1/kickstart/Packages/example.rpm", + object_key="88a2831543aaca1355a725ad2f5969c7a180643beddfe94281343a2ba361c979", + publish_id=publish.id, + updated=datetime(2023, 10, 4, 3, 52, 2), + ), + ] + ) + + @mock.patch("exodus_gw.worker.publish.AutoindexEnricher.run") @mock.patch("exodus_gw.worker.publish.CurrentMessage.get_current_message") @mock.patch("exodus_gw.worker.publish.DynamoDB.write_batch") @@ -49,6 +75,9 @@ def test_commit( # Simulate successful write of items by write_batch. mock_write_batch.return_value = True + # Let this test cover kickstart items. + add_kickstart(fake_publish) + db.add(fake_publish) db.add(task) # Caller would've set publish state to COMMITTING. @@ -86,6 +115,10 @@ def test_commit( # Note that this does not include paths after alias resolution, # because Flusher does alias resolution itself internally # (tested elsewhere) + # Note that the default phase2_patterns cause all non-RPM kickstart + # files to be flushed. + "/content/testproduct/1/kickstart/EULA", + "/content/testproduct/1/kickstart/extra_files.json", "/content/testproduct/1/repo/", "/content/testproduct/1/repo/repomd.xml", ] @@ -104,6 +137,38 @@ def test_commit( [ # Note that both sides of the 1 alias are recorded, including # beyond the RHUI alias. + ( + "test", + "/content/testproduct/1.1.0/kickstart/extra_files.json", + ), + ( + "test", + "/content/testproduct/1/kickstart/extra_files.json", + ), + ( + "test", + "/content/testproduct/rhui/1.1.0/kickstart/extra_files.json", + ), + ( + "test", + "/content/testproduct/rhui/1/kickstart/extra_files.json", + ), + ( + "test", + "/content/testproduct/1.1.0/kickstart/EULA", + ), + ( + "test", + "/content/testproduct/1/kickstart/EULA", + ), + ( + "test", + "/content/testproduct/rhui/1.1.0/kickstart/EULA", + ), + ( + "test", + "/content/testproduct/rhui/1/kickstart/EULA", + ), ("test", "/content/testproduct/1/repo/"), ("test", "/content/testproduct/1.1.0/repo/"), ("test", "/content/testproduct/1/repo/repomd.xml"), @@ -490,6 +555,9 @@ def test_commit_phase1( ) ) + # Let this test cover kickstart items. + add_kickstart(fake_publish) + db.add(fake_publish) db.add(task) @@ -554,6 +622,25 @@ def test_commit_phase1( [ {"dirty": False, "web_uri": "/other/path"}, {"dirty": False, "web_uri": "/some/path"}, + # kickstart content: + # - EULA, though not an entrypoint, was forcibly delayed to phase2 + # via phase2_patterns setting. And therefore is still 'dirty'. + # - RPMs aren't matched by phase2_patterns and can still be + # processed in phase1. + # - extra_files.json is an entrypoint and so is also delayed until + # phase2. + { + "dirty": True, + "web_uri": "/content/testproduct/1/kickstart/EULA", + }, + { + "dirty": False, + "web_uri": "/content/testproduct/1/kickstart/Packages/example.rpm", + }, + { + "dirty": True, + "web_uri": "/content/testproduct/1/kickstart/extra_files.json", + }, # the unresolved link is not yet written and therefore remains dirty {"dirty": True, "web_uri": "/some/path/to/link-src"}, # autoindex and repomd.xml are both entrypoints, not yet written, @@ -572,7 +659,7 @@ def test_commit_phase1( # It should have told us how many it wrote and how many remain assert ( - "Phase 1: committed 2 items, phase 2: 2 items remaining" in caplog.text + "Phase 1: committed 3 items, phase 2: 4 items remaining" in caplog.text ) # Let's do the same commit again... @@ -590,7 +677,7 @@ def test_commit_phase1( # This time there should not have been any phase1 items processed at all, # as none of them were dirty. assert ( - "Phase 1: committed 0 items, phase 2: 2 items remaining" in caplog.text + "Phase 1: committed 0 items, phase 2: 4 items remaining" in caplog.text ) # And it should NOT have invoked the autoindex enricher in either commit