launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29937
[Merge] ~cjwatson/launchpad:oval-check-changes into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:oval-check-changes into launchpad:master.
Commit message:
Compare incoming OVAL data with already published one
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/441468
I adopted https://code.launchpad.net/~jugmac00/launchpad/+git/launchpad/+merge/441101 and fixed up a number of issues I found while writing tests for it. Normally I'd squash all this before proposing it, but in this case I've kept the commits separate to make it clear what Jürgen did and what I did.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:oval-check-changes into launchpad:master.
diff --git a/lib/lp/archivepublisher/scripts/publishdistro.py b/lib/lp/archivepublisher/scripts/publishdistro.py
index 419b1ef..f827ccf 100644
--- a/lib/lp/archivepublisher/scripts/publishdistro.py
+++ b/lib/lp/archivepublisher/scripts/publishdistro.py
@@ -8,13 +8,16 @@ __all__ = [
]
import os
+from filecmp import dircmp
from optparse import OptionValueError
+from pathlib import Path
from subprocess import CalledProcessError, check_call
from storm.store import Store
from zope.component import getUtility
from lp.app.errors import NotFoundError
+from lp.archivepublisher.config import getPubConfig
from lp.archivepublisher.publishing import (
GLOBAL_PUBLISHER_LOCK,
cannot_modify_suite,
@@ -46,6 +49,17 @@ def is_ppa_public(ppa):
return not ppa.private
+def has_oval_data_changed(incoming_dir, published_dir):
+ """Compare the incoming data with the already published one."""
+ compared = dircmp(str(incoming_dir), str(published_dir))
+ return (
+ bool(compared.left_only)
+ or bool(compared.right_only)
+ or bool(compared.diff_files)
+ or bool(compared.funny_files)
+ )
+
+
class PublishDistro(PublisherScript):
"""Distro publisher."""
@@ -507,56 +521,108 @@ class PublishDistro(PublisherScript):
# store and cause performance problems.
Store.of(archive).reset()
- def rsync_oval_data(self):
- if config.archivepublisher.oval_data_rsync_endpoint:
- # Ensure that the rsync paths have a trailing slash.
- rsync_src = os.path.join(
- config.archivepublisher.oval_data_rsync_endpoint, ""
- )
- rsync_dest = os.path.join(
- config.archivepublisher.oval_data_root, ""
+ def rsyncOVALData(self):
+ # Ensure that the rsync paths have a trailing slash.
+ rsync_src = os.path.join(
+ config.archivepublisher.oval_data_rsync_endpoint, ""
+ )
+ rsync_dest = os.path.join(config.archivepublisher.oval_data_root, "")
+ rsync_command = [
+ "/usr/bin/rsync",
+ "-a",
+ "-q",
+ "--timeout={}".format(
+ config.archivepublisher.oval_data_rsync_timeout
+ ),
+ "--delete",
+ "--delete-after",
+ rsync_src,
+ rsync_dest,
+ ]
+ try:
+ self.logger.info(
+ "Attempting to rsync the OVAL data from '%s' to '%s'",
+ rsync_src,
+ rsync_dest,
)
- rsync_command = [
- "/usr/bin/rsync",
- "-a",
- "-q",
- "--timeout={}".format(
- config.archivepublisher.oval_data_rsync_timeout
- ),
- "--delete",
- "--delete-after",
+ check_call(rsync_command)
+ except CalledProcessError:
+ self.logger.exception(
+ "Failed to rsync OVAL data from '%s' to '%s'",
rsync_src,
rsync_dest,
- ]
- try:
- self.logger.info(
- "Attempting to rsync the OVAL data from '%s' to '%s'",
- rsync_src,
- rsync_dest,
- )
- check_call(rsync_command)
- except CalledProcessError:
- self.logger.exception(
- "Failed to rsync OVAL data from '%s' to '%s'",
- rsync_src,
- rsync_dest,
- )
- raise
- else:
- self.logger.info(
- "Skipping the OVAL data sync as no rsync endpoint"
- " has been configured."
)
+ raise
+
+ def checkForUpdatedOVALData(self, distribution):
+ """Compare the published OVAL files with the incoming one."""
+ start_dir = Path(config.archivepublisher.oval_data_root)
+ archive_set = getUtility(IArchiveSet)
+ for owner_path in start_dir.iterdir():
+ if not owner_path.name.startswith("~"):
+ continue
+ distribution_path = owner_path / distribution.name
+ if not distribution_path.is_dir():
+ continue
+ for archive_path in distribution_path.iterdir():
+ archive = archive_set.getPPAByDistributionAndOwnerName(
+ distribution, owner_path.name[1:], archive_path.name
+ )
+ if archive is None:
+ self.logger.info(
+ "Skipping OVAL data for '~%s/%s/%s' "
+ "(no such archive).",
+ owner_path.name[1:],
+ distribution.name,
+ archive_path.name,
+ )
+ continue
+ for suite_path in archive_path.iterdir():
+ try:
+ series, pocket = distribution.getDistroSeriesAndPocket(
+ suite_path.name
+ )
+ except NotFoundError:
+ self.logger.info(
+ "Skipping OVAL data for '%s:%s' (no such suite).",
+ archive.reference,
+ suite_path.name,
+ )
+ continue
+ for component in archive.getComponentsForSeries(series):
+ incoming_dir = suite_path / component.name
+ published_dir = (
+ Path(getPubConfig(archive).distsroot)
+ / series.name
+ / component.name
+ / "oval"
+ )
+ if not published_dir.is_dir() or has_oval_data_changed(
+ incoming_dir=incoming_dir,
+ published_dir=published_dir,
+ ):
+ archive.markSuiteDirty(
+ distroseries=series, pocket=pocket
+ )
+ break
def main(self, reset_store_between_archives=True):
"""See `LaunchpadScript`."""
self.validateOptions()
self.logOptions()
- self.rsync_oval_data()
+ if config.archivepublisher.oval_data_rsync_endpoint:
+ self.rsyncOVALData()
+ else:
+ self.logger.info(
+ "Skipping the OVAL data sync as no rsync endpoint"
+ " has been configured."
+ )
archive_ids = []
for distribution in self.findDistros():
+ if config.archivepublisher.oval_data_rsync_endpoint:
+ self.checkForUpdatedOVALData(distribution)
for archive in self.getTargetArchives(distribution):
if archive.distribution != distribution:
raise AssertionError(
diff --git a/lib/lp/archivepublisher/tests/test_publishdistro.py b/lib/lp/archivepublisher/tests/test_publishdistro.py
index 3d9e0cc..049dccb 100644
--- a/lib/lp/archivepublisher/tests/test_publishdistro.py
+++ b/lib/lp/archivepublisher/tests/test_publishdistro.py
@@ -7,6 +7,7 @@ import os
import shutil
import subprocess
from optparse import OptionValueError
+from pathlib import Path
from fixtures import MockPatch
from storm.store import Store
@@ -33,6 +34,7 @@ from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.services.config import config
from lp.services.database.interfaces import IStore
from lp.services.log.logger import BufferLogger, DevNullLogger
+from lp.services.osutils import write_file
from lp.services.scripts.base import LaunchpadScriptFailure
from lp.soyuz.enums import (
ArchivePublishingMethod,
@@ -258,10 +260,11 @@ class TestPublishDistro(TestNativePublishingBase):
self.assertEqual(PackagePublishingStatus.PENDING, pub_source.status)
def setUpOVALDataRsync(self):
+ self.oval_data_root = self.makeTemporaryDirectory()
self.pushConfig(
"archivepublisher",
oval_data_rsync_endpoint="oval.internal::oval/",
- oval_data_root="/tmp/oval-data",
+ oval_data_root=self.oval_data_root,
oval_data_rsync_timeout=90,
)
@@ -299,7 +302,7 @@ class TestPublishDistro(TestNativePublishingBase):
"--delete",
"--delete-after",
"oval.internal::oval/",
- "/tmp/oval-data/",
+ self.oval_data_root + "/",
]
mock_subprocess_check_call.assert_called_once_with(call_args)
@@ -322,15 +325,167 @@ class TestPublishDistro(TestNativePublishingBase):
"--delete",
"--delete-after",
"oval.internal::oval/",
- "/tmp/oval-data/",
+ self.oval_data_root + "/",
]
mock_subprocess_check_call.assert_called_once_with(call_args)
expected_log_line = (
"ERROR Failed to rsync OVAL data from "
- "'oval.internal::oval/' to '/tmp/oval-data/'"
+ "'oval.internal::oval/' to '%s/'" % self.oval_data_root
)
self.assertTrue(expected_log_line in self.logger.getLogBuffer())
+ def test_checkForUpdatedOVALData_new(self):
+ self.setUpOVALDataRsync()
+ self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+ )
+ archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+ # Disable normal publication so that dirty_suites isn't cleared.
+ archive.publish = False
+ incoming_dir = (
+ Path(self.oval_data_root)
+ / archive.reference
+ / "breezy-autotest"
+ / "main"
+ )
+ write_file(str(incoming_dir), b"test")
+ self.runPublishDistro(
+ extra_args=["--ppa"], distribution=archive.distribution.name
+ )
+ self.assertEqual(["breezy-autotest"], archive.dirty_suites)
+
+ def test_checkForUpdatedOVALData_unchanged(self):
+ self.setUpOVALDataRsync()
+ self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+ )
+ archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+ # Disable normal publication so that dirty_suites isn't cleared.
+ archive.publish = False
+ incoming_dir = (
+ Path(self.oval_data_root)
+ / archive.reference
+ / "breezy-autotest"
+ / "main"
+ )
+ published_dir = (
+ Path(getPubConfig(archive).distsroot)
+ / "breezy-autotest"
+ / "main"
+ / "oval"
+ )
+ write_file(str(incoming_dir / "foo.oval.xml.bz2"), b"test")
+ shutil.copytree(str(incoming_dir), str(published_dir))
+ self.runPublishDistro(
+ extra_args=["--ppa"], distribution=archive.distribution.name
+ )
+ self.assertIsNone(archive.dirty_suites)
+
+ def test_checkForUpdatedOVALData_updated(self):
+ self.setUpOVALDataRsync()
+ self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+ )
+ archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+ # Disable normal publication so that dirty_suites isn't cleared.
+ archive.publish = False
+ incoming_dir = (
+ Path(self.oval_data_root)
+ / archive.reference
+ / "breezy-autotest"
+ / "main"
+ )
+ published_dir = (
+ Path(getPubConfig(archive).distsroot)
+ / "breezy-autotest"
+ / "main"
+ / "oval"
+ )
+ write_file(str(published_dir / "foo.oval.xml.bz2"), b"old")
+ mtime = (published_dir / "foo.oval.xml.bz2").stat().st_mtime
+ os.utime(
+ str(published_dir / "foo.oval.xml.bz2"), (mtime - 1, mtime - 1)
+ )
+ write_file(str(incoming_dir / "foo.oval.xml.bz2"), b"new")
+ self.runPublishDistro(
+ extra_args=["--ppa"], distribution=archive.distribution.name
+ )
+ self.assertEqual(["breezy-autotest"], archive.dirty_suites)
+
+ def test_checkForUpdatedOVALData_new_files(self):
+ self.setUpOVALDataRsync()
+ self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+ )
+ archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+ # Disable normal publication so that dirty_suites isn't cleared.
+ archive.publish = False
+ incoming_dir = (
+ Path(self.oval_data_root)
+ / archive.reference
+ / "breezy-autotest"
+ / "main"
+ )
+ published_dir = (
+ Path(getPubConfig(archive).distsroot)
+ / "breezy-autotest"
+ / "main"
+ / "oval"
+ )
+ write_file(str(incoming_dir / "foo.oval.xml.bz2"), b"test")
+ shutil.copytree(str(incoming_dir), str(published_dir))
+ write_file(str(incoming_dir / "bar.oval.xml.bz2"), b"test")
+ self.runPublishDistro(
+ extra_args=["--ppa"], distribution=archive.distribution.name
+ )
+ self.assertEqual(["breezy-autotest"], archive.dirty_suites)
+
+ def test_checkForUpdatedOVALData_nonexistent_archive(self):
+ self.setUpOVALDataRsync()
+ self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+ )
+ incoming_dir = (
+ Path(self.oval_data_root)
+ / "~nonexistent"
+ / "ubuntutest"
+ / "archive"
+ / "breezy-autotest"
+ / "main"
+ )
+ write_file(str(incoming_dir / "foo.oval.xml.bz2"), b"test")
+ self.runPublishDistro(extra_args=["--ppa"], distribution="ubuntutest")
+ self.assertIn(
+ "INFO Skipping OVAL data for '~nonexistent/ubuntutest/archive' "
+ "(no such archive).",
+ self.logger.getLogBuffer(),
+ )
+
+ def test_checkForUpdatedOVALData_nonexistent_suite(self):
+ self.setUpOVALDataRsync()
+ self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+ )
+ archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+ # Disable normal publication so that dirty_suites isn't cleared.
+ archive.publish = False
+ incoming_dir = (
+ Path(self.oval_data_root)
+ / archive.reference
+ / "nonexistent"
+ / "main"
+ )
+ write_file(str(incoming_dir / "foo.oval.xml.bz2"), b"test")
+ self.runPublishDistro(
+ extra_args=["--ppa"], distribution=archive.distribution.name
+ )
+ self.assertIn(
+ "INFO Skipping OVAL data for '%s:nonexistent' (no such suite)."
+ % archive.reference,
+ self.logger.getLogBuffer(),
+ )
+ self.assertIsNone(archive.dirty_suites)
+
@defer.inlineCallbacks
def testForPPA(self):
"""Try to run publish-distro in PPA mode.
