launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29963
[Merge] ~jugmac00/launchpad:sync-oval-data into launchpad:master
Jürgen Gmach has proposed merging ~jugmac00/launchpad:sync-oval-data into launchpad:master.
Commit message:
Simplify implementation of OVAL sync
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jugmac00/launchpad/+git/launchpad/+merge/441693
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~jugmac00/launchpad:sync-oval-data into launchpad:master.
diff --git a/lib/lp/archivepublisher/scripts/publishdistro.py b/lib/lp/archivepublisher/scripts/publishdistro.py
index 87e9a7d..754c0bf 100644
--- a/lib/lp/archivepublisher/scripts/publishdistro.py
+++ b/lib/lp/archivepublisher/scripts/publishdistro.py
@@ -11,6 +11,12 @@ import os
from filecmp import dircmp
from optparse import OptionValueError
from pathlib import Path
+
+# XXX 2023-04-21 jugmac00: prefer `import shutil` as `copy` is so common
+# This will break a lot of tests which mocked this import, so let's do this
+# together with removing mocks from e.g.
+# `test_syncOVALDataFilesForSuite_oval_data_missing_in_destination`
+from shutil import copy
from subprocess import CalledProcessError, check_call
from storm.store import Store
@@ -418,6 +424,52 @@ class PublishDistro(PublisherScript):
)
return False
+ def synchronizeSecondDirectoryWithFirst(self, first_dir, second_dir):
+ """Synchronize the contents of the second directory with the first."""
+ comparison = dircmp(str(first_dir), str(second_dir))
+ files_to_copy = (
+ comparison.diff_files
+ + comparison.left_only
+ + comparison.funny_files
+ )
+ files_to_delete = comparison.right_only
+
+ for file in files_to_copy:
+ copy(str(first_dir / file), str(second_dir))
+
+ for file in files_to_delete:
+ os.unlink(str(second_dir / file))
+
+ return bool(files_to_copy) or bool(files_to_delete)
+
+ def syncOVALDataFilesForSuite(self, archive, suite):
+ """Synchronize the OVAL data from the staging to the PPA directory."""
+ updated = False
+ staged_oval_data_for_suite = (
+ Path(config.archivepublisher.oval_data_root)
+ / archive.reference
+ / suite
+ )
+ if staged_oval_data_for_suite.exists():
+ for item in staged_oval_data_for_suite.iterdir():
+ if not item.is_dir():
+ continue
+ component = item
+ staged_oval_data_dir = staged_oval_data_for_suite / component
+ dest_dir = (
+ Path(getPubConfig(archive).distsroot)
+ / suite
+ / component.name
+ / "oval"
+ )
+ dest_dir.mkdir(parents=True, exist_ok=True)
+ files_modified = self.synchronizeSecondDirectoryWithFirst(
+ staged_oval_data_dir, dest_dir
+ )
+ if files_modified:
+ updated = True
+ return updated
+
def publishArchive(self, archive, publisher):
"""Ask `publisher` to publish `archive`.
@@ -428,10 +480,13 @@ class PublishDistro(PublisherScript):
for distroseries, pocket in self.findExplicitlyDirtySuites(archive):
if not cannot_modify_suite(archive, distroseries, pocket):
publisher.markSuiteDirty(distroseries, pocket)
+
+ dirty_suites = None
if archive.dirty_suites is not None:
# Clear the explicit dirt indicator before we start doing
# time-consuming publishing, which might race with an
# Archive.markSuiteDirty call.
+ dirty_suites = archive.dirty_suites
archive.dirty_suites = None
self.txn.commit()
@@ -475,6 +530,23 @@ class PublishDistro(PublisherScript):
self.options.enable_release
and publishing_method == ArchivePublishingMethod.LOCAL
):
+ # XXX 2023-04-21 jugmac00: add test for the non-ppa case
+ if (
+ config.archivepublisher.oval_data_rsync_endpoint
+ and archive.is_ppa
+ and dirty_suites
+ ):
+ for dirty_suite in dirty_suites:
+ updated = self.syncOVALDataFilesForSuite(
+ archive, dirty_suite
+ )
+ if updated:
+ self.logger.info(
+ "Synchronized the OVAL data for %s",
+ archive.reference,
+ )
+ # XXX 2023-04-21 jugmac00: evaluate whether the above code
+ # would better fit into the `Publisher` class
publisher.D_writeReleaseFiles(
self.isCareful(
self.options.careful_apt or self.options.careful_release
diff --git a/lib/lp/archivepublisher/tests/test_publishdistro.py b/lib/lp/archivepublisher/tests/test_publishdistro.py
index 049dccb..dba384e 100644
--- a/lib/lp/archivepublisher/tests/test_publishdistro.py
+++ b/lib/lp/archivepublisher/tests/test_publishdistro.py
@@ -8,6 +8,7 @@ import shutil
import subprocess
from optparse import OptionValueError
from pathlib import Path
+from unittest.mock import call
from fixtures import MockPatch
from storm.store import Store
@@ -921,6 +922,7 @@ class FakePublisher:
self.C_writeIndexes = FakeMethod()
self.D_writeReleaseFiles = FakeMethod()
self.createSeriesAliases = FakeMethod()
+ self.markSuiteDirty = FakeMethod()
class TestPublishDistroMethods(TestCaseWithFactory):
@@ -1479,3 +1481,284 @@ class TestPublishDistroMethods(TestCaseWithFactory):
self.assertEqual(
[((archive, publisher), {})], script.publishArchive.calls
)
+
+ def setUpOVALDataRsync(self):
+ self.oval_data_root = self.makeTemporaryDirectory()
+ self.pushConfig(
+ "archivepublisher",
+ oval_data_rsync_endpoint="oval.internal::oval/",
+ oval_data_root=self.oval_data_root,
+ oval_data_rsync_timeout=90,
+ )
+ self.ppa_root = self.makeTemporaryDirectory()
+ self.pushConfig(
+ "personalpackagearchive",
+ root=self.ppa_root,
+ )
+
+ def test_syncOVALDataFilesForSuite_oval_data_missing_in_destination(self):
+ self.setUpOVALDataRsync()
+ self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+ )
+ mock_copy = self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.copy")
+ ).mock
+ # XXX 2023-04-21 jugmac00: as we create temporary directories anyway,
+ # we can avoid to use mocks, and directly assert on the content of the
+ # target directory on the filesystem
+ # This also applies for similar tests here.
+ mock_unlink = self.useFixture(MockPatch("pathlib.Path.unlink")).mock
+ archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+ incoming_dir = (
+ Path(self.oval_data_root)
+ / archive.reference
+ / "breezy-autotest"
+ / "main"
+ )
+ write_file(str(incoming_dir / "test"), b"test")
+ script = self.makeScript()
+ script.txn = FakeTransaction()
+ script.findDistros = FakeMethod([archive.distribution])
+ script.getTargetArchives = FakeMethod([archive])
+ publisher = FakePublisher()
+ script.getPublisher = FakeMethod(publisher)
+ script.main()
+ mock_copy.assert_has_calls(
+ [
+ call(
+ str(incoming_dir / "test"),
+ "{}/breezy-autotest/main/oval".format(
+ getPubConfig(archive).distsroot
+ ),
+ )
+ ]
+ )
+ mock_unlink.assert_not_called()
+
+ def test_syncOVALDataFilesForSuite_oval_data_missing_in_source(self):
+ self.setUpOVALDataRsync()
+ self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+ )
+ mock_copy = self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.copy")
+ ).mock
+ mock_unlink = self.useFixture(MockPatch("os.unlink")).mock
+ archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+ incoming_dir = (
+ Path(self.oval_data_root)
+ / archive.reference
+ / "breezy-autotest"
+ / "main"
+ )
+ incoming_dir.mkdir(parents=True, exist_ok=True)
+ published_dir = (
+ Path(getPubConfig(archive).distsroot)
+ / "breezy-autotest"
+ / "main"
+ / "oval"
+ )
+ write_file(str(published_dir / "foo.oval.xml.bz2"), b"test")
+ write_file(str(published_dir / "foo2.oval.xml.bz2"), b"test")
+
+ script = self.makeScript()
+ script.txn = FakeTransaction()
+ script.findDistros = FakeMethod([archive.distribution])
+ script.getTargetArchives = FakeMethod([archive])
+ publisher = FakePublisher()
+ script.getPublisher = FakeMethod(publisher)
+ script.main()
+ mock_copy.assert_not_called()
+ mock_unlink.assert_has_calls(
+ [
+ call(str(published_dir / "foo.oval.xml.bz2")),
+ call(str(published_dir / "foo2.oval.xml.bz2")),
+ ],
+ any_order=True,
+ )
+
+ def test_syncOVALDataFilesForSuite_oval_data_unchanged(self):
+ self.setUpOVALDataRsync()
+ self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+ )
+ mock_copy = self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.copy")
+ ).mock
+ mock_unlink = self.useFixture(MockPatch("os.unlink")).mock
+ archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+ incoming_dir = (
+ Path(self.oval_data_root)
+ / archive.reference
+ / "breezy-autotest"
+ / "main"
+ )
+ incoming_dir.mkdir(parents=True, exist_ok=True)
+ write_file(str(incoming_dir / "foo.oval.xml.bz2"), b"test")
+ write_file(str(incoming_dir / "foo2.oval.xml.bz2"), b"test")
+ published_dir = (
+ Path(getPubConfig(archive).distsroot)
+ / "breezy-autotest"
+ / "main"
+ / "oval"
+ )
+ write_file(str(published_dir / "foo.oval.xml.bz2"), b"test")
+ write_file(str(published_dir / "foo2.oval.xml.bz2"), b"test")
+
+ script = self.makeScript()
+ script.txn = FakeTransaction()
+ script.findDistros = FakeMethod([archive.distribution])
+ script.getTargetArchives = FakeMethod([archive])
+ publisher = FakePublisher()
+ script.getPublisher = FakeMethod(publisher)
+ script.main()
+ mock_copy.assert_not_called()
+ mock_unlink.assert_not_called()
+
+ def test_syncOVALDataFilesForSuite_oval_data_updated(self):
+ self.setUpOVALDataRsync()
+ self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+ )
+ mock_copy = self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.copy")
+ ).mock
+ mock_unlink = self.useFixture(MockPatch("os.unlink")).mock
+ archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+ incoming_dir = (
+ Path(self.oval_data_root)
+ / archive.reference
+ / "breezy-autotest"
+ / "main"
+ )
+ incoming_dir.mkdir(parents=True, exist_ok=True)
+ write_file(str(incoming_dir / "foo.oval.xml.bz2"), b"test2")
+ write_file(str(incoming_dir / "foo2.oval.xml.bz2"), b"test2")
+ published_dir = (
+ Path(getPubConfig(archive).distsroot)
+ / "breezy-autotest"
+ / "main"
+ / "oval"
+ )
+ write_file(str(published_dir / "foo.oval.xml.bz2"), b"test")
+ write_file(str(published_dir / "foo2.oval.xml.bz2"), b"test")
+
+ script = self.makeScript()
+ script.txn = FakeTransaction()
+ script.findDistros = FakeMethod([archive.distribution])
+ script.getTargetArchives = FakeMethod([archive])
+ publisher = FakePublisher()
+ script.getPublisher = FakeMethod(publisher)
+ script.main()
+ mock_copy.assert_has_calls(
+ [
+ call(
+ str(incoming_dir / "foo.oval.xml.bz2"),
+ "{}/breezy-autotest/main/oval".format(
+ getPubConfig(archive).distsroot
+ ),
+ ),
+ call(
+ str(incoming_dir / "foo2.oval.xml.bz2"),
+ "{}/breezy-autotest/main/oval".format(
+ getPubConfig(archive).distsroot
+ ),
+ ),
+ ],
+ any_order=True,
+ )
+ mock_unlink.assert_not_called()
+
+ def test_syncOVALDataFilesForSuite_oval_data_new_files(self):
+ self.setUpOVALDataRsync()
+ self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+ )
+ mock_copy = self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.copy")
+ ).mock
+ mock_unlink = self.useFixture(MockPatch("os.unlink")).mock
+ archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+ incoming_dir = (
+ Path(self.oval_data_root)
+ / archive.reference
+ / "breezy-autotest"
+ / "main"
+ )
+ incoming_dir.mkdir(parents=True, exist_ok=True)
+ write_file(str(incoming_dir / "foo.oval.xml.bz2"), b"test")
+ write_file(str(incoming_dir / "foo2.oval.xml.bz2"), b"test")
+ write_file(str(incoming_dir / "foo3.oval.xml.bz2"), b"test")
+ published_dir = (
+ Path(getPubConfig(archive).distsroot)
+ / "breezy-autotest"
+ / "main"
+ / "oval"
+ )
+ write_file(str(published_dir / "foo.oval.xml.bz2"), b"test")
+ write_file(str(published_dir / "foo2.oval.xml.bz2"), b"test")
+
+ script = self.makeScript()
+ script.txn = FakeTransaction()
+ script.findDistros = FakeMethod([archive.distribution])
+ script.getTargetArchives = FakeMethod([archive])
+ publisher = FakePublisher()
+ script.getPublisher = FakeMethod(publisher)
+ script.main()
+ mock_copy.assert_has_calls(
+ [
+ call(
+ str(incoming_dir / "foo3.oval.xml.bz2"),
+ "{}/breezy-autotest/main/oval".format(
+ getPubConfig(archive).distsroot
+ ),
+ ),
+ ]
+ )
+ mock_unlink.assert_not_called()
+
+ def test_syncOVALDataFilesForSuite_oval_data_some_files_removed(self):
+ self.setUpOVALDataRsync()
+ self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+ )
+ mock_copy = self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.copy")
+ ).mock
+ mock_unlink = self.useFixture(MockPatch("os.unlink")).mock
+ archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+ incoming_dir = (
+ Path(self.oval_data_root)
+ / archive.reference
+ / "breezy-autotest"
+ / "main"
+ )
+ incoming_dir.mkdir(parents=True, exist_ok=True)
+ write_file(str(incoming_dir / "foo.oval.xml.bz2"), b"test")
+ published_dir = (
+ Path(getPubConfig(archive).distsroot)
+ / "breezy-autotest"
+ / "main"
+ / "oval"
+ )
+ write_file(str(published_dir / "foo.oval.xml.bz2"), b"test")
+ write_file(str(published_dir / "foo2.oval.xml.bz2"), b"test")
+
+ script = self.makeScript()
+ script.txn = FakeTransaction()
+ script.findDistros = FakeMethod([archive.distribution])
+ script.getTargetArchives = FakeMethod([archive])
+ publisher = FakePublisher()
+ script.getPublisher = FakeMethod(publisher)
+ script.main()
+ mock_copy.assert_not_called()
+ mock_unlink.assert_has_calls(
+ [
+ call(
+ "{}/breezy-autotest/main/oval/foo2.oval.xml.bz2".format(
+ getPubConfig(archive).distsroot
+ ),
+ ),
+ ]
+ )
