launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #32159
[Merge] ~tushar5526/launchpad:add-support-for-excluded-ppas into launchpad:master
Tushar Gupta has proposed merging ~tushar5526/launchpad:add-support-for-excluded-ppas into launchpad:master.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~tushar5526/launchpad/+git/launchpad/+merge/480294
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~tushar5526/launchpad:add-support-for-excluded-ppas into launchpad:master.
diff --git a/lib/lp/archivepublisher/scripts/base.py b/lib/lp/archivepublisher/scripts/base.py
index f590791..c00cdb9 100644
--- a/lib/lp/archivepublisher/scripts/base.py
+++ b/lib/lp/archivepublisher/scripts/base.py
@@ -13,7 +13,14 @@ from optparse import OptionValueError
from zope.component import getUtility
from lp.registry.interfaces.distribution import IDistributionSet
+from lp.services.features import getFeatureFlag
from lp.services.scripts.base import LaunchpadCronScript
+from lp.soyuz.enums import ArchivePurpose
+from lp.soyuz.interfaces.archive import IArchiveSet
+
+PUBLISHER_EXCLUDED_PPAS_FEATURE_FLAG = (
+ "archivepublisher.publisher.excluded_ppas"
+)
class PublisherScript(LaunchpadCronScript):
@@ -62,3 +69,76 @@ class PublisherScript(LaunchpadCronScript):
return self.findDerivedDistros()
else:
return [self.findSelectedDistro()]
+
+ def _setExcludedPPAs(self):
+ """
+ Gets the excluded PPAs from the feature flag. Though feature-rules
+ don't change within context, this ensures we get a consistent value
+ within a publisher run.
+ """
+ excluded_ppas_feature_flag = getFeatureFlag(
+ PUBLISHER_EXCLUDED_PPAS_FEATURE_FLAG
+ )
+
+ if not excluded_ppas_feature_flag:
+ self._excluded_ppas = []
+ return
+
+ excluded_ppas = []
+ for reference in excluded_ppas_feature_flag.split(" "):
+ archive = getUtility(IArchiveSet).getByReference(reference)
+ if not archive:
+ self.logger.warning(
+ "Cannot find archive reference: '%s', "
+ "set in 'archive.publisher.excluded_ppas' feature flag"
+ % reference
+ )
+ continue
+ if archive.purpose != ArchivePurpose.PPA:
+ self.logger.warning(
+ "Only specify PPAs to be excluded."
+ "Archive %s is of type %s" % (reference, archive.purpose)
+ )
+ continue
+ excluded_ppas.append(archive)
+
+ self._excluded_ppas = excluded_ppas
+
+ def getExcludedPPAs(self, distribution):
+ """
+ Retrieve a list of excluded PPAs for a given distribution.
+ This function checks if the '_excluded_ppas' attribute exists.
+ If it does not exist, it calls the '_setExcludedPPAs' method
+ to set it. Then, it filters and returns the PPAs that match the
+ specified distribution.
+ """
+ if not hasattr(self, "_excluded_ppas"):
+ self._setExcludedPPAs()
+
+ return [
+ archive
+ for archive in self._excluded_ppas
+ if archive.distribution == distribution and
+ archive.purpose == ArchivePurpose.PPA
+ ]
+
+ def addParallelPublisherOptions(self):
+ """
+ Adds the necessary CLI options needed to enable the parallel publisher
+ runs.
+ """
+ self.parser.add_option(
+ "--run-excluded-ppas",
+ action="store_true",
+ dest="run_excluded_ppas",
+ default=False,
+ help=(
+ "Run the publisher only for the excluded PPAs."
+ " Set this option when running another publisher process"
+ " in parallel with the main publisher process. The"
+ " excluded PPAs are set via the"
+ ' "archive.publisher.excluded_ppas" feature flag and'
+ " these PPAs are excluded from the MAIN publisher"
+ " run."
+ ),
+ )
diff --git a/lib/lp/archivepublisher/scripts/publishdistro.py b/lib/lp/archivepublisher/scripts/publishdistro.py
index 38ee406..f143cc0 100644
--- a/lib/lp/archivepublisher/scripts/publishdistro.py
+++ b/lib/lp/archivepublisher/scripts/publishdistro.py
@@ -75,6 +75,7 @@ class PublishDistro(PublisherScript):
def add_my_options(self):
self.addDistroOptions()
+ self.addParallelPublisherOptions()
self.parser.add_option(
"-C",
@@ -276,6 +277,7 @@ class PublishDistro(PublisherScript):
self.options.private_ppa,
self.options.copy_archive,
self.options.archive,
+ self.options.run_excluded_ppas,
]
return len(list(filter(None, exclusive_options)))
@@ -375,6 +377,7 @@ class PublishDistro(PublisherScript):
def getTargetArchives(self, distribution):
"""Find the archive(s) selected by the script's options."""
+ excluded_ppas = self.getExcludedPPAs(distribution)
if self.options.archive:
archive = getUtility(IArchiveSet).getByReference(
self.options.archive
@@ -383,12 +386,26 @@ class PublishDistro(PublisherScript):
return [archive]
else:
return []
+ elif self.options.run_excluded_ppas:
+ return excluded_ppas
elif self.options.partner:
return [distribution.getArchiveByComponent("partner")]
elif self.options.ppa:
- return filter(is_ppa_public, self.getPPAs(distribution))
+ return [
+ archive
+ for archive in filter(
+ is_ppa_public, self.getPPAs(distribution)
+ )
+ if archive not in excluded_ppas
+ ]
elif self.options.private_ppa:
- return filter(is_ppa_private, self.getPPAs(distribution))
+ return [
+ archive
+ for archive in filter(
+ is_ppa_private, self.getPPAs(distribution)
+ )
+ if archive not in excluded_ppas
+ ]
elif self.options.copy_archive:
return self.getCopyArchives(distribution)
else:
@@ -634,6 +651,7 @@ class PublishDistro(PublisherScript):
"""Compare the published OVAL files with the incoming one."""
start_dir = Path(config.archivepublisher.oval_data_root)
archive_set = getUtility(IArchiveSet)
+ excluded_ppas = self.getExcludedPPAs(distribution)
for owner_path in start_dir.iterdir():
if not owner_path.name.startswith("~"):
continue
@@ -644,6 +662,29 @@ class PublishDistro(PublisherScript):
archive = archive_set.getPPAByDistributionAndOwnerName(
distribution, owner_path.name[1:], archive_path.name
)
+ # Run it only for the excluded archives if the option is
+ # set, else ALWAYS skip the excluded archives.
+ if self.options.run_excluded_ppas:
+ if archive not in excluded_ppas:
+ continue
+ self.logger.info(
+ "Running OVAL data for '~%s/%s/%s' "
+ "(excluded archive for parallel publisher run).",
+ owner_path.name[1:],
+ distribution.name,
+ archive_path.name,
+ )
+ else:
+ if archive in excluded_ppas:
+ self.logger.info(
+ "Skipping OVAL data for '~%s/%s/%s' "
+ "(excluded archive for parallel publisher run).",
+ owner_path.name[1:],
+ distribution.name,
+ archive_path.name,
+ )
+ continue
+
if archive is None:
self.logger.info(
"Skipping OVAL data for '~%s/%s/%s' "
@@ -687,8 +728,18 @@ class PublishDistro(PublisherScript):
self.validateOptions()
self.logOptions()
+ # We only run the global rsyncOVALData in the main publisher run,
+ # and skip it for all other cases. run_excluded_ppas is primarily
+ # meant to be used in a parallel publisher process, so skip the rsync
+ # call when it is True.
if config.archivepublisher.oval_data_rsync_endpoint:
- self.rsyncOVALData()
+ if not self.options.run_excluded_ppas:
+ self.rsyncOVALData()
+ else:
+ self.logger.info(
+ "Skipping the OVAL data sync in excluded PPA publisher"
+ "run."
+ )
else:
self.logger.info(
"Skipping the OVAL data sync as no rsync endpoint"
diff --git a/lib/lp/archivepublisher/tests/test_publishdistro.py b/lib/lp/archivepublisher/tests/test_publishdistro.py
index ad591fa..ef6c629 100644
--- a/lib/lp/archivepublisher/tests/test_publishdistro.py
+++ b/lib/lp/archivepublisher/tests/test_publishdistro.py
@@ -25,6 +25,9 @@ from lp.archivepublisher.interfaces.archivegpgsigningkey import (
)
from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
from lp.archivepublisher.publishing import Publisher
+from lp.archivepublisher.scripts.base import (
+ PUBLISHER_EXCLUDED_PPAS_FEATURE_FLAG,
+)
from lp.archivepublisher.scripts.publishdistro import PublishDistro
from lp.archivepublisher.tests.artifactory_fixture import (
FakeArtifactoryFixture,
@@ -34,6 +37,7 @@ from lp.registry.interfaces.person import IPersonSet
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.services.config import config
from lp.services.database.interfaces import IStore
+from lp.services.features.testing import FeatureFixture
from lp.services.log.logger import BufferLogger, DevNullLogger
from lp.services.osutils import write_file
from lp.services.scripts.base import LaunchpadScriptFailure
@@ -487,6 +491,101 @@ class TestPublishDistro(TestNativePublishingBase):
)
self.assertIsNone(archive.dirty_suites)
+ def test_checkForUpdatedOVALData_skips_excluded_ppas(self):
+ """
+ Skip excluded PPAs in checkForUpdatedOVALData when the option
+ "run_excluded_ppas" is not set
+ """
+
+ self.setUpOVALDataRsync()
+ # XXX 29-01-2025 tushar5526: There appears to be an issue with
+ # FeatureFixture's implementation. It cleans up database objects
+ # when called, causing reference errors in Storm. Therefore we are
+ # calling it here with a fixed ppa reference and creating associated
+ # PPAs with correct owners later.
+ self.useFixture(FeatureFixture({PUBLISHER_EXCLUDED_PPAS_FEATURE_FLAG: '~test-user/ubuntu/ppa'}))
+ self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+ )
+
+ owner = self.factory.makePerson(name='test-user')
+ excluded_ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA, name='ppa',owner=owner)
+ ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA, owner=owner)
+ # Disable normal publication so that dirty_suites isn't cleared.
+ ppa.publish = False
+ excluded_ppa.publish = False
+
+ for archive in [ppa, excluded_ppa]:
+ 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="ubuntu"
+ )
+
+ self.assertEqual(["breezy-autotest"], ppa.dirty_suites)
+ self.assertIsNone(excluded_ppa.dirty_suites)
+ self.assertIn(
+ "INFO Skipping OVAL data for '%s' "
+ "(excluded archive for parallel publisher run)." % (excluded_ppa.reference),
+ self.logger.getLogBuffer(),
+ )
+
+ def test_checkForUpdatedOVALData_runs_excluded_ppas(self):
+ """
+ Run excluded PPAs in checkForUpdatedOVALData when the option
+ "run_excluded_ppas" is set
+ """
+
+ self.setUpOVALDataRsync()
+ # XXX 29-01-2025 tushar5526: There appears to be an issue with
+ # FeatureFixture's implementation. It cleans up database objects
+ # when called, causing reference errors in Storm. Therefore we are
+ # calling it here with a fixed ppa reference and creating associated
+ # PPAs with correct owners later.
+ self.useFixture(FeatureFixture({PUBLISHER_EXCLUDED_PPAS_FEATURE_FLAG: '~test-user/ubuntu/ppa'}))
+ self.useFixture(
+ MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+ )
+
+ owner = self.factory.makePerson(name='test-user')
+ excluded_ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA, name='ppa',owner=owner)
+ ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA, owner=owner)
+ # Disable normal publication so that dirty_suites isn't cleared.
+ ppa.publish = False
+ excluded_ppa.publish = False
+
+ for archive in [ppa, excluded_ppa]:
+ incoming_dir = (
+ Path(self.oval_data_root)
+ / archive.reference
+ / "breezy-autotest"
+ / "main"
+ )
+ write_file(str(incoming_dir), b"test")
+
+ self.runPublishDistro(
+ extra_args=["--run-excluded-ppas"], distribution="ubuntu"
+ )
+
+ self.assertIsNone(ppa.dirty_suites)
+ self.assertEqual(["breezy-autotest"], excluded_ppa.dirty_suites)
+ self.assertIn(
+ "INFO Running OVAL data for '%s' "
+ "(excluded archive for parallel publisher run)." % (excluded_ppa.reference),
+ self.logger.getLogBuffer(),
+ )
+ # Further logs should not have any reference to ppa.reference as we skipped it.
+ self.assertNotIn(
+ ppa.reference,
+ self.logger.getLogBuffer(),
+ )
+
@defer.inlineCallbacks
def testForPPA(self):
"""Try to run publish-distro in PPA mode.
@@ -954,6 +1053,60 @@ class TestPublishDistroMethods(TestCaseWithFactory):
script.logger = DevNullLogger()
return script
+ def makeArchivesForDifferentTypesDistros(self):
+ """
+ Creates a list of PPAs with different
+ types/publishing-methods/distros.
+ """
+ archives = []
+ archives.append(
+ self.factory.makeArchive(purpose=ArchivePurpose.PPA, private=False)
+ )
+ archives.append(
+ self.factory.makeArchive(
+ purpose=ArchivePurpose.PPA,
+ private=True,
+ publishing_method=ArchivePublishingMethod.ARTIFACTORY,
+ )
+ )
+ archives.append(
+ self.factory.makeArchive(
+ purpose=ArchivePurpose.COPY,
+ private=False,
+ publishing_method=ArchivePublishingMethod.ARTIFACTORY,
+ )
+ )
+ archives.append(
+ self.factory.makeArchive(
+ purpose=ArchivePurpose.COPY,
+ private=True,
+ publishing_method=ArchivePublishingMethod.ARTIFACTORY,
+ )
+ )
+ archives.append(
+ self.factory.makeArchive(
+ purpose=ArchivePurpose.PARTNER,
+ private=False,
+ publishing_method=ArchivePublishingMethod.ARTIFACTORY,
+ )
+ )
+ archives.append(
+ self.factory.makeArchive(
+ purpose=ArchivePurpose.PARTNER,
+ private=True,
+ publishing_method=ArchivePublishingMethod.ARTIFACTORY,
+ )
+ )
+ return archives
+
+ def generateExcludedArchivesFeatureFlagValue(self, excluded_archives):
+ """
+ Returns a space separate string of archive references
+ from a list of archives
+ """
+
+ return " ".join([a.reference for a in excluded_archives])
+
def test_isCareful_is_false_if_option_not_set(self):
# isCareful normally returns False for a carefulness option that
# evaluates to False.
@@ -1016,6 +1169,14 @@ class TestPublishDistroMethods(TestCaseWithFactory):
1, self.makeScript(args=["--copy-archive"]).countExclusiveOptions()
)
+ def test_countExclusiveOptions_counts_run_excluded_ppas(self):
+ self.assertEqual(
+ 1,
+ self.makeScript(
+ args=["--run-excluded-ppas"]
+ ).countExclusiveOptions(),
+ )
+
def test_countExclusiveOptions_detects_conflict(self):
# If more than one of the exclusive options has been set, that
# raises the result from countExclusiveOptions above 1.
@@ -1276,40 +1437,138 @@ class TestPublishDistroMethods(TestCaseWithFactory):
script = self.makeScript(distro, ["--partner"])
self.assertContentEqual([partner], script.getTargetArchives(distro))
- def test_getTargetArchives_ignores_public_ppas_if_private(self):
+ def test_getTargetArchives_ignores_public_ppas_if_private_and_ignores_excluded_ppas( # noqa: E501
+ self,
+ ):
# If the selected exclusive option is "private-ppa,"
- # getTargetArchives looks for PPAs but leaves out public ones.
+ # getTargetArchives looks for PPAs but leaves out public ones
+ # and excluded PPAs
distro = self.makeDistro()
+ excluded_archives = self.makeArchivesForDifferentTypesDistros()
+
+ # Add different archives from the same distribution as well
+ excluded_archives.extend([
+ self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.PPA, private=False
+ ),
+ self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.PPA, private=True
+ ),])
+
+ self.useFixture(
+ FeatureFixture(
+ {
+ PUBLISHER_EXCLUDED_PPAS_FEATURE_FLAG: self.generateExcludedArchivesFeatureFlagValue( # noqa: E501
+ excluded_archives
+ )
+ }
+ )
+ )
self.factory.makeArchive(
distro, purpose=ArchivePurpose.PPA, private=False
)
script = self.makeScript(distro, ["--private-ppa"])
self.assertContentEqual([], script.getTargetArchives(distro))
- def test_getTargetArchives_gets_private_ppas_if_private(self):
+ def test_getTargetArchives_gets_private_ppas_if_private_and_ignores_excluded_ppas( # noqa: E501
+ self,
+ ):
# If the selected exclusive option is "private-ppa,"
# getTargetArchives looks for private PPAs.
distro = self.makeDistro()
+ excluded_archives = self.makeArchivesForDifferentTypesDistros()
+
+ # Add different archives from the same distribution as well
+ excluded_archives.append(
+ self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.PPA, private=False
+ )
+ )
+ excluded_archives.append(
+ self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.PPA, private=True
+ )
+ )
+ self.useFixture(
+ FeatureFixture(
+ {
+ PUBLISHER_EXCLUDED_PPAS_FEATURE_FLAG: self.generateExcludedArchivesFeatureFlagValue( # noqa: E501
+ excluded_archives
+ )
+ }
+ )
+ )
+
ppa = self.factory.makeArchive(
distro, purpose=ArchivePurpose.PPA, private=True
)
script = self.makeScript(distro, ["--private-ppa", "--careful"])
self.assertContentEqual([ppa], script.getTargetArchives(distro))
- def test_getTargetArchives_gets_public_ppas_if_not_private(self):
+ def test_getTargetArchives_gets_public_ppas_if_not_private_and_ignores_excluded_ppas( # noqa: E501
+ self,
+ ):
# If the selected exclusive option is "ppa," getTargetArchives
# looks for public PPAs.
distro = self.makeDistro()
+ excluded_archives = self.makeArchivesForDifferentTypesDistros()
+
+ # Add different archives from the same distribution as well
+ excluded_archives.append(
+ self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.PPA, private=False
+ )
+ )
+ excluded_archives.append(
+ self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.PPA, private=True
+ )
+ )
+ self.useFixture(
+ FeatureFixture(
+ {
+ PUBLISHER_EXCLUDED_PPAS_FEATURE_FLAG: self.generateExcludedArchivesFeatureFlagValue( # noqa: E501
+ excluded_archives
+ )
+ }
+ )
+ )
+
ppa = self.factory.makeArchive(
distro, purpose=ArchivePurpose.PPA, private=False
)
script = self.makeScript(distro, ["--ppa", "--careful"])
self.assertContentEqual([ppa], script.getTargetArchives(distro))
- def test_getTargetArchives_ignores_private_ppas_if_not_private(self):
+ def test_getTargetArchives_ignores_private_ppas_if_not_private_and_ignores_excluded_ppas( # noqa: E501
+ self,
+ ):
# If the selected exclusive option is "ppa," getTargetArchives
# leaves out private PPAs.
distro = self.makeDistro()
+ excluded_archives = self.makeArchivesForDifferentTypesDistros()
+
+ # Add different archives from the same distribution as well
+ excluded_archives.append(
+ self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.PPA, private=False
+ )
+ )
+ excluded_archives.append(
+ self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.PPA, private=True
+ )
+ )
+ self.useFixture(
+ FeatureFixture(
+ {
+ PUBLISHER_EXCLUDED_PPAS_FEATURE_FLAG: self.generateExcludedArchivesFeatureFlagValue( # noqa: E501
+ excluded_archives
+ )
+ }
+ )
+ )
+
self.factory.makeArchive(
distro, purpose=ArchivePurpose.PPA, private=True
)
@@ -1324,6 +1583,47 @@ class TestPublishDistroMethods(TestCaseWithFactory):
script = self.makeScript(distro, ["--copy-archive"])
self.assertContentEqual([copy], script.getTargetArchives(distro))
+ def test_getTargetArchives_gets_excluded_archives_only(self):
+ distro = self.makeDistro()
+ excluded_archives = self.makeArchivesForDifferentTypesDistros()
+
+ # Add different archives from the same distribution as well
+ ppa = self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.PPA, private=False
+ )
+ pppa = self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.PPA, private=True
+ )
+ excluded_archives.extend([
+ self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.COPY, private=True
+ ),
+ self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.PARTNER, private=True
+ ),
+ ppa,
+ pppa
+ ])
+ self.useFixture(
+ FeatureFixture(
+ {
+ PUBLISHER_EXCLUDED_PPAS_FEATURE_FLAG: self.generateExcludedArchivesFeatureFlagValue( # noqa: E501
+ excluded_archives
+ )
+ }
+ )
+ )
+
+ self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.PPA, private=True
+ )
+ self.factory.makeArchive(
+ distro, purpose=ArchivePurpose.PPA, private=False
+ )
+
+ script = self.makeScript(distro, ["--run-excluded-ppas"])
+ self.assertContentEqual([ppa, pppa], script.getTargetArchives(distro))
+
def test_getPublisher_returns_publisher(self):
# getPublisher produces a Publisher instance.
distro = self.makeDistro()
@@ -1806,3 +2106,25 @@ class TestPublishDistroMethods(TestCaseWithFactory):
self.assertTrue(by_hash_dir.is_dir())
# and still contains the two test files
self.assertEqual(2, len(list(by_hash_dir.iterdir())))
+
+ def test_main_skips_rsyncOVALData_when_run_excluded_ppas_is_set(self):
+ self.setUpOVALDataRsync()
+
+ script = self.makeScript(args=['--run-excluded-ppas'])
+ rsyncOVALData_mock = script.rsyncOVALData = FakeMethod()
+ script.txn = FakeTransaction()
+ script.findDistros = FakeMethod([])
+ script.main()
+
+ self.assertEqual(rsyncOVALData_mock.call_count, 0)
+
+ def test_main_calls_rsyncOVALData_when_run_excluded_ppas_is_not_set(self):
+ self.setUpOVALDataRsync()
+
+ script = self.makeScript()
+ rsyncOVALData_mock = script.rsyncOVALData = FakeMethod()
+ script.txn = FakeTransaction()
+ script.findDistros = FakeMethod([])
+ script.main()
+
+ self.assertEqual(rsyncOVALData_mock.call_count, 1)
Follow ups
