launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/db-bug-751054 into lp:launchpad/db-devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #751054 in Launchpad itself: "Wrong PPA path in updated cron.publish-ftpmaster"
  https://bugs.launchpad.net/launchpad/+bug/751054

For more details, see:
https://code.launchpad.net/~jtv/launchpad/db-bug-751054/+merge/56298

= Summary =

In rewriting the cron.publish-ftpmaster script in python, I made some changes to the original script.  The changes were needed to make the script testable, so that the tests could then be run against the new code.  They eliminated some lint and such, but were not otherwise needed.  Unfortunately I just found a mistake in a path change for the partner archive on non-production configs.

Meanwhile, a change to the list of master mirrors was also landed by someone else as a change to the script.


== Proposed fix ==

In this change I restore the script to its old form, but with the added master mirror.  I also forward-port the mirror change to the new plugin script that replaces that part of cron.publish-ftpmaster.


== Pre-implementation notes ==

We found some path problems when combining the old script (but including my changes) with the python version.  Part of those may have been due to configuration changes, since all configuration now omes out of the database whereas the old script duplicates settings and special-cases the production config.

Lamont (who made the mirror change) and Julian (who is in charge of this work) are not available so I emailed them about the situation.


== Implementation details ==

There's not much to see.  I really just copied the devel version of the script into a db-devel branch.  Nobody's done any meaningful maintenance to this script for ages; it's pure bad luck that this work coincides with a mirrors change and pure good luck that I noticed.  The content of MASTERMIRRORS is the only difference between the version of the script that I'm proposing here and what we had in db-devel before I touched it.


== Tests ==

{{{
./bin/test -vvc lp.archivepublisher.tests.test_publish_ftpmaster
}}}


== Demo and Q/A ==

We'll try this on dogfood and see whether it fixes the path problem.  There is a chance that we may have to fix the new python version of the script as well, but that's not a rollout blocker since the python version is not in use yet.


= Launchpad lint =

Yes, this re-introduces some lint that I had previously eliminated.  But this is all for the greater good of eliminating the script itself.

Checking for conflicts and issues in changed files.

Linting changed files:
  cronscripts/publishing/cron.publish-ftpmaster
  cronscripts/publishing/distro-parts/ubuntu/finalize.d/90-trigger-mirrors

./cronscripts/publishing/cron.publish-ftpmaster
      56: Line exceeds 78 characters.
      97: Line exceeds 78 characters.
     185: Line exceeds 78 characters.
     186: Line exceeds 78 characters.
     237: Line exceeds 78 characters.
     238: Line exceeds 78 characters.
     249: Line exceeds 78 characters.
-- 
https://code.launchpad.net/~jtv/launchpad/db-bug-751054/+merge/56298
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/db-bug-751054 into lp:launchpad/db-devel.

=== modified file 'cronscripts/publishing/cron.publish-ftpmaster'
--- cronscripts/publishing/cron.publish-ftpmaster	2011-04-01 16:30:57 +0000
+++ cronscripts/publishing/cron.publish-ftpmaster	2011-04-05 07:39:00 +0000
@@ -9,15 +9,6 @@
     echo LPCONFIG must be set to run this script.
     exit 1
 fi
-
-# Injection points for testability.
-DISTRONAME="${DISTRONAME:-ubuntu}"
-ARCHIVEPARENT="${ARCHIVEPARENT:-/srv/launchpad.net/$DISTRONAME-archive}"
-LOCKFILE_CMD="${LOCKFILE_CMD:-lockfile}"
-DSYNC_FLIST_CMD="${DSYNC_FLIST_CMD:-dsync-flist}"
-COMMERCIAL_COMPAT_CMD="${COMMERCIAL_COMPAT_CMD:-commercial-compat.sh}"
-INHIBIT_GPG_SIGNING="${INHIBIT_GPG_SIGNING:-no}"
-
 SECURITY_PUBLISHER="no"
 if [ "$1" = "security" ]; then
     # We are running a security publisher run, which skips some steps.
@@ -31,27 +22,28 @@
 # Launchpad cron.daily (currently just for Ubuntu).
 
 # Informational -- this *MUST* match the database.
-ARCHIVEROOT=$ARCHIVEPARENT/$DISTRONAME
+ARCHIVEROOT=/srv/launchpad.net/ubuntu-archive/ubuntu
 DISTSROOT=$ARCHIVEROOT/dists
-OVERRIDEROOT=$ARCHIVEROOT-overrides
-CACHEROOT=$ARCHIVEROOT-cache
-DISTSCOPYROOT=$ARCHIVEROOT-distscopy
+OVERRIDEROOT=$ARCHIVEROOT/../ubuntu-overrides
+CACHEROOT=$ARCHIVEROOT/../ubuntu-cache
+DISTSCOPYROOT=$ARCHIVEROOT/../ubuntu-distscopy
 INDICES=$ARCHIVEROOT/indices
 PRODUCTION_CONFIG=ftpmaster-publish
 
 if [ "$LPCONFIG" = "$PRODUCTION_CONFIG" ]; then
-    ARCHIVEROOT_PARTNER=$ARCHIVEROOT-partner
-    GNUPGHOME=$ARCHIVEROOT/gnupg-home
+    ARCHIVEROOT_PARTNER=/srv/launchpad.net/ubuntu-archive/ubuntu-partner
+    GNUPGHOME=/srv/launchpad.net/ubuntu-archive/gnupg-home
 else
     # GNUPGHOME does not need to be set, keys can come from ~/.gnupg.
-    ARCHIVEROOT_PARTNER=$ARCHIVEPARENT/ppa/$DISTRONAME-partner
+    ARCHIVEROOT_PARTNER=/srv/launchpad.net/ppa/ubuntu-partner
 fi
 DISTSROOT_PARTNER=$ARCHIVEROOT_PARTNER/dists
-DISTSCOPYROOT_PARTNER=$ARCHIVEROOT-partner-distscopy
+DISTSCOPYROOT_PARTNER=$ARCHIVEROOT_PARTNER/../ubuntu-partner-distscopy
 
 # Configuration options.
 LAUNCHPADROOT=/srv/launchpad.net/codelines/current
 LOCKFILE=/srv/launchpad.net/ubuntu-archive/cron.daily.lock
+DISTRONAME=ubuntu
 TRACEFILE=$ARCHIVEROOT/project/trace/$(hostname --fqdn)
 DSYNCLIST=$CACHEROOT/dsync.list
 MD5LIST=$INDICES/md5sums.gz
@@ -64,7 +56,7 @@
 PATH=$PATH:$LAUNCHPADROOT/scripts:$LAUNCHPADROOT/cronscripts:$LAUNCHPADROOT/cronscripts/publishing:$LAUNCHPADROOT/scripts/ftpmaster-tools
 
 # Claim the lock.
-if ! ${LOCKFILE_CMD} -r1 $LOCKFILE; then
+if ! lockfile -r1 $LOCKFILE; then
   echo "Could not claim lock file."
   exit 1
 fi
@@ -161,34 +153,30 @@
 publish-distro.py -v -v -d $DISTRONAME $SUITEOPTS -R ${DISTSROOT}.new
 
 set +x
-if [ "${INHIBIT_GPG_SIGNING}" != yes ]; then
-  # Find all the Release files for which the Release.GPG is missing/too-old
-  # We use -maxdepth 2 to only sign Release files for distroreleases,
-  # not distroarchreleases/distrosourcereleases.
-  # Also we sign the dist-upgrader tarballs because they're handy too.
-  RELEASE_FILES=`find ${DISTSROOT}.new -maxdepth 2 -name Release`
-  DIST_UPGRADER_TARBALLS=`find ${DISTSROOT}.new/*/*/dist-upgrader* -name "*.tar.gz" || /bin/true`
-  for CANDIDATE in $RELEASE_FILES $DIST_UPGRADER_TARBALLS
-  do
-    #  [ Release.gpg missing   ] or [ Release is newer than Release.gpg ]
-    if [ ! -f $CANDIDATE.gpg ] || [ $CANDIDATE -nt $CANDIDATE.gpg ]; then
-      echo "$(date -R): (re-)signing $CANDIDATE"
-      gpg --yes --detach-sign --armor -o $CANDIDATE.gpg --sign $CANDIDATE
-    else
-      echo "$(date -R): Not re-signing $CANDIDATE"
-    fi
-  done
-  SIGNLIST_PARTNER=$(find ${DISTSROOT_PARTNER}.new -maxdepth 2 -name Release)
-  for CANDIDATE in $SIGNLIST_PARTNER; do
-    #  [ Release.gpg missing   ] or [ Release is newer than Release.gpg ].
-    if [ ! -f $CANDIDATE.gpg ] || [ $CANDIDATE -nt $CANDIDATE.gpg ]; then
-      echo "$(date -R): (re-)signing $CANDIDATE"
-      gpg --yes --detach-sign --armor -o $CANDIDATE.gpg --sign $CANDIDATE
-    else
-      echo "$(date -R): Not re-signing $CANDIDATE"
-    fi
-  done
-fi
+# Find all the Release files for which the Release.GPG is missing/too-old
+# We use -maxdepth 2 to only sign Release files for distroreleases,
+# not distroarchreleases/distrosourcereleases.
+# Also we sign the dist-upgrader tarballs because they're handy too.
+for CANDIDATE in $(find ${DISTSROOT}.new -maxdepth 2 -name Release) \
+       $(find ${DISTSROOT}.new/*/*/dist-upgrader* -name "*.tar.gz"); do
+  #  [ Release.gpg missing   ] or [ Release is newer than Release.gpg ]
+  if [ ! -f $CANDIDATE.gpg ] || [ $CANDIDATE -nt $CANDIDATE.gpg ]; then
+    echo "$(date -R): (re-)signing $CANDIDATE"
+    gpg --yes --detach-sign --armor -o $CANDIDATE.gpg --sign $CANDIDATE
+  else
+    echo "$(date -R): Not re-signing $CANDIDATE"
+  fi
+done
+SIGNLIST_PARTNER=$(find ${DISTSROOT_PARTNER}.new -maxdepth 2 -name Release)
+for CANDIDATE in $SIGNLIST_PARTNER; do
+  #  [ Release.gpg missing   ] or [ Release is newer than Release.gpg ].
+  if [ ! -f $CANDIDATE.gpg ] || [ $CANDIDATE -nt $CANDIDATE.gpg ]; then
+    echo "$(date -R): (re-)signing $CANDIDATE"
+    gpg --yes --detach-sign --armor -o $CANDIDATE.gpg --sign $CANDIDATE
+  else
+    echo "$(date -R): Not re-signing $CANDIDATE"
+  fi
+done
 
 # The Packages and Sources files are very large and would cripple our
 # mirrors, so we remove them now that the uncompressed MD5SUMS are in the
@@ -219,7 +207,7 @@
 # dapper, edgy and feisty releases.  Don't fail the whole script if it
 # fails.
 echo "$(date -R): Generating -commerical pocket..."
-${COMMERCIAL_COMPAT_CMD} || true
+commercial-compat.sh || true
 
 # Timestamp our trace file to track when the last archive publisher run took
 # place.
@@ -246,9 +234,9 @@
     # Run dsync over primary archive only.
     echo "$(date -R): Running dsync over primary archive..."
     ( cd $ARCHIVEROOT ; \
-      $DSYNC_FLIST_CMD -q generate $DSYNCLIST -e 'Packages*' -e 'Sources*' -e 'Release*' --md5 ; \
-      ($DSYNC_FLIST_CMD -q md5sums $DSYNCLIST; find dists '(' -name 'Packages*' -o -name 'Sources*' -o -name 'Release*' ')' -print | xargs -r md5sum) | gzip -9n > ${MD5LIST} ; \
-      $DSYNC_FLIST_CMD -q link-dups $DSYNCLIST || true )
+      dsync-flist -q generate $DSYNCLIST -e 'Packages*' -e 'Sources*' -e 'Release*' --md5 ; \
+      (dsync-flist -q md5sums $DSYNCLIST; find dists '(' -name 'Packages*' -o -name 'Sources*' -o -name 'Release*' ')' -print | xargs -r md5sum) | gzip -9n > ${MD5LIST} ; \
+      dsync-flist -q link-dups $DSYNCLIST || true )
 
     # Clear out empty and thus redundant dirs.
     echo "$(date -R): Clearing out empty directories..."

=== modified file 'cronscripts/publishing/distro-parts/ubuntu/finalize.d/90-trigger-mirrors'
--- cronscripts/publishing/distro-parts/ubuntu/finalize.d/90-trigger-mirrors	2011-03-28 09:18:42 +0000
+++ cronscripts/publishing/distro-parts/ubuntu/finalize.d/90-trigger-mirrors	2011-04-05 07:39:00 +0000
@@ -2,7 +2,7 @@
 
 # Prod the master mirrors to propagate the update.
 
-MASTERMIRRORS="syowa frei scandium"
+MASTERMIRRORS="syowa frei wahoo scandium"
 echo "$(date -R): Triggering master mirrors..."
 
 for HOST in $MASTERMIRRORS