change pushsnip, so "backup existing updates" is separate from "push new snippets"

RESOLVED FIXED

Status

Release Engineering
General
P2
normal
RESOLVED FIXED
11 years ago
5 years ago

People

(Reporter: joduinn, Assigned: joduinn)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

Currently, pushsnip backs up all updates before pushing new updates live. This is good, in case the newly pushed updates have a problem - we can then quickly revert to a known working state. However,this backup step takes 30+minutes, and happens at a crunch time of the release, when we only want to push the actual snippet live. It would be nice if the backup step was done earlier, so that at crunch time, we only had to push the new snippets (a quick process). It is still good to have a backup, so we can revert if anything goes wrong with the push.

There are a couple of ways we could implement this. For example:
- run pushsnip with special arg to enable backup. No special arg means do not do backup before pushing updates. In this situation, we should probably also have special arg to not push updates, so that we can safely do backups earlier in the day.
- separate out the logic of doing backups into a separate script ("backupsnip"?). We could then run backupsnip at any time, maybe even from automation after stage step, and then run pushsnip to only push snippets live.

(I'm sure there's other ways to do this also, just two examples while brainstorming).

Honestly, I thought there was already a bug filed tracking this, but I cant find it, so filing this... but if I've missed an existing bug, please close this as DUP.
Created attachment 309907 [details]
pushnip (without doing backup)

pushsnip currently backs up all previous snippets before starting to push new snippets. Thats ok. However, the backup takes 30-40mins, and can be done before we get the "formal go" from QA to push snippets. 

This patch is to separate out the "backupsnip" script from the "pushsnip" script.

Also, note that the pushsnip script should be moved from mofo repo to public repo. How about mozilla/tools/release/pushsnip?
Attachment #309907 - Flags: review?(rhelmer)
Created attachment 309908 [details]
backupsnip (without doing push)

The other half of pushsnip changes. In the public repo, how about mozilla/tools/release/backupsnip?
Attachment #309908 - Flags: review?(rhelmer)
Assignee: nobody → joduinn
Priority: -- → P2
It would be great if the pushsnip checked for a backup before proceeding. Maybe a 
  find $BACKUP_DIR -mtime -1 | grep $newSnippetDir.tar.bz2

(only with decent quoting/escaping on the grep)
Comment on attachment 309907 [details]
pushnip (without doing backup)

Can you attach this as a diff to /mofo/release/stage/pushsnip?

>[joduinn@dm-peep01 tools]$ more backupsnip 

Remove this line :)

>#!/bin/bash
>
>set -e
>#set -v
>
>LIVE_SNIPPET_DIR=/opt/aus2/incoming/3
>STAGING_DIR=/opt/aus2/snippets/staging
>
>RSYNC=/usr/bin/rsync
>WC=/usr/bin/wc
>SED=/bin/sed
>
>if test -z $1; then
>   echo Usage: $0 [snippet-directory-to-sync-in from $STAGING_DIR]
>   exit 1
>fi
>
>## Remove trailing /'s, but it turns out that it
>newSnippetDir=`echo $1 | $SED -e 's/\///'g`


I know this is in the original code, but there's something missing in that comment. Can you just remove it?


>if ! test -d $STAGING_DIR/$newSnippetDir; then
>   echo Usage: $0 [snippet-directory-to-sync-in from $STAGING_DIR]
>   exit 1
>fi
>
># publish the new snippets
>pushd $STAGING_DIR > /dev/null
>echo Running $RSYNC -Pa $STAGING_DIR/$1/ $LIVE_SNIPPET_DIR
>$RSYNC -Pa $STAGING_DIR/$1/ $LIVE_SNIPPET_DIR
>popd > /dev/null
>
>exit 0


I think Nick's suggestion of checking for the existence of a backup dir is a good one. It seems like that should be an error.
Attachment #309907 - Flags: review?(rhelmer) → review-
Comment on attachment 309908 [details]
backupsnip (without doing push)

Same deal with the incomplete comment in this one. This seems ok, but there's a lot of code copied duplicated; seems like you could roll these both into one script that supports different modes (e.g. default backup+push, backup-only, push-only).

However, I think separating the backup from the push is more important for right now. We can do this kind of refactoring later.

One thing missing from this bug is the bootstrap support for this, in Bootstrap::Updates::Push(), it only knows to call pushsnip not backupsnip.
Attachment #309908 - Flags: review?(rhelmer) → review+
Created attachment 310191 [details] [diff] [review]
[checked in] pushnip (without doing backup) (retry)

1) Adjusted to handle review comments.

2) The only remaining overlap between these two scripts has to do with how backupsnip requires a runtime parameter to determine the name of the directory to backup into. This seems unusual to me, but its always been done this way. While I think it should be factored out later, for now splitting out these scripts with as few design changes as possible, seems the best approach.

3) This patch is not in normal "cvs diff -u" format, because this patch is also to move the file from mofo repo to moco repo. The new proposed location of the file is: moco cvs:/mozilla/tools/pushsnip, but I am open to other locations.
Attachment #309907 - Attachment is obsolete: true
Attachment #310191 - Flags: review?(rhelmer)
Created attachment 310192 [details] [diff] [review]
[checked in] backupsnip (without doing push)  (retry)

Comments (1) and (2) same as for pushsnip patch above. Comment (3) has different filename in moco repo at:  moco cvs:/mozilla/tools/backupsnip
Attachment #309908 - Attachment is obsolete: true
Attachment #310192 - Flags: review?(rhelmer)
Created attachment 310195 [details] [diff] [review]
[checked in] Bootstrap::Updates::Push() should do backup before doing push

In mozilla/tools/release/Bootstrap/Step/Updates.pm, we now need to call the new backupsnip script as part of automation, immediately before we call the pushsnip script.
Attachment #310195 - Flags: review?(rhelmer)

Comment 9

11 years ago
(In reply to comment #4)
> >## Remove trailing /'s, but it turns out that it
> >newSnippetDir=`echo $1 | $SED -e 's/\///'g`
> 
> I know this is in the original code, but there's something missing in that
> comment. Can you just remove it?

I'm going to bet that if one were to |cvs blame| that line, it will have my name on it.

I'm also going to bet that what I meant to say was "Remove trailing /'s, but it turns out that it [doesn't matter, because rsync only cares about the semantics of slashes on the source argument, not the destination argument.]"

This may or may not be a detail that you want to continue to explicitly mark in the "mo[zilla]co[rp] repo [at cvs.mozilla.org]"-copy, but at least should explain what I was trying to convey... assuming my initial bet paid out anyway. :-)
Attachment #310191 - Flags: review?(rhelmer) → review+
Attachment #310192 - Flags: review?(rhelmer) → review+
Attachment #310195 - Flags: review?(rhelmer) → review+
We'll need to figure out exactly where to put this on staging :) Currently ~/bin/ is a checkout of /mofo/release/stage.

Maybe we should move this and anything else we need from stage out to mozilla/tools/release/stage, and make the checkout be this instead? I don't think there's anything in there that needs to be private.
(In reply to comment #9)
> (In reply to comment #4)
> > >## Remove trailing /'s, but it turns out that it
> > >newSnippetDir=`echo $1 | $SED -e 's/\///'g`
> > 
> > I know this is in the original code, but there's something missing in that
> > comment. Can you just remove it?
> 
> I'm going to bet that if one were to |cvs blame| that line, it will have my
> name on it.
> 
> I'm also going to bet that what I meant to say was "Remove trailing /'s, but it
> turns out that it [doesn't matter, because rsync only cares about the semantics
> of slashes on the source argument, not the destination argument.]"
> 
> This may or may not be a detail that you want to continue to explicitly mark in
> the "mo[zilla]co[rp] repo [at cvs.mozilla.org]"-copy, but at least should
> explain what I was trying to convey... assuming my initial bet paid out anyway.
> :-)


Thanks, makes sense. One of the finer points of rsync :) Maybe it's worth removing that line altogether, if it truly doesn't matter.
(In reply to comment #10)
> We'll need to figure out exactly where to put this on staging :) Currently
> ~/bin/ is a checkout of /mofo/release/stage.
> Maybe we should move this and anything else we need from stage out to
> mozilla/tools/release/stage, and make the checkout be this instead? I don't
> think there's anything in there that needs to be private.

I'm all in favour of moving these from private repo to public repo. Looking on aus2-staging, under /home/cltbld/bin I see a bunch of files that look like they come from cvs mofo repo. I know we need "pushsnip". How many of the rest of these do we actually *need* on aus2-staging?


-bash-3.00$ ls -la
total 128
drwxr-xr-x  3 cltbld users   4096 Oct  5 15:53 .
drwxr-xr-x  5 cltbld cltbld  4096 Oct  5 06:12 ..
-rwxr-xr-x  1 cltbld users  12157 Jun  1  2006 add-file-verifications
-rwxr-xr-x  1 cltbld users    302 Sep 27  2006 check_locales.sh
-rwxr-xr-x  1 cltbld users  12185 Jun  1  2006 checksum-files
-rwxr-xr-x  1 cltbld users   1148 Jun  1  2006 clean_stage.pl
drwxr-xr-x  2 cltbld users   4096 Jul 17  2007 CVS
-rwxr-xr-x  1 cltbld users   2433 Jun  1  2006 firefox-l10n-src-tarball-nobuild
-rwxr-xr-x  1 cltbld users   2172 Dec 12  2005 firefox-src-tarball-nobuild
-rwxr-xr-x  1 cltbld users  11853 Aug 29  2006 groom-files
-rwxr-xr-x  1 cltbld users  12795 Feb 26  2007 groom-files-partners
-rwxr-xr-x  1 cltbld users   5434 Dec 12  2005 modify-installer-url.pl
-rwxr-xr-x  1 cltbld users   2714 Jun  1  2006 nuke-respins.sh
-rwxr-xr-x  1 cltbld users   1410 Jul 17  2007 pushsnip
-rwxr-xr-x  1 cltbld users   1072 Jul 31  2006 remove-aus-locale.sh
-rwxr-xr-x  1 cltbld users   2567 Dec 12  2005 seamonkey-src-tarball-nobuild
-rwxr-xr-x  1 cltbld users   2411 Dec 12  2005 thunderbird-src-tarball-nobuild
-rwxr-xr-x  1 cltbld users    378 Jun  1  2006 update-download-stats.sh
-rwxr-xr-x  1 cltbld users  10147 Sep 25  2006 verify-locales.pl
-rwxr-xr-x  1 cltbld users   2180 Jan 27  2006 xulrunner-src-tarball-nobuild
-bash-3.00$ 
(In reply to comment #12)
> (In reply to comment #10)
> > We'll need to figure out exactly where to put this on staging :) Currently
> > ~/bin/ is a checkout of /mofo/release/stage.
> > Maybe we should move this and anything else we need from stage out to
> > mozilla/tools/release/stage, and make the checkout be this instead? I don't
> > think there's anything in there that needs to be private.
> 
> I'm all in favour of moving these from private repo to public repo. Looking on
> aus2-staging, under /home/cltbld/bin I see a bunch of files that look like they
> come from cvs mofo repo. I know we need "pushsnip". How many of the rest of
> these do we actually *need* on aus2-staging?
> 
> 
> -bash-3.00$ ls -la
> total 128
> drwxr-xr-x  3 cltbld users   4096 Oct  5 15:53 .
> drwxr-xr-x  5 cltbld cltbld  4096 Oct  5 06:12 ..
> -rwxr-xr-x  1 cltbld users  12157 Jun  1  2006 add-file-verifications
> -rwxr-xr-x  1 cltbld users    302 Sep 27  2006 check_locales.sh
> -rwxr-xr-x  1 cltbld users  12185 Jun  1  2006 checksum-files
> -rwxr-xr-x  1 cltbld users   1148 Jun  1  2006 clean_stage.pl
> drwxr-xr-x  2 cltbld users   4096 Jul 17  2007 CVS
> -rwxr-xr-x  1 cltbld users   2433 Jun  1  2006 firefox-l10n-src-tarball-nobuild
> -rwxr-xr-x  1 cltbld users   2172 Dec 12  2005 firefox-src-tarball-nobuild
> -rwxr-xr-x  1 cltbld users  11853 Aug 29  2006 groom-files
> -rwxr-xr-x  1 cltbld users  12795 Feb 26  2007 groom-files-partners
> -rwxr-xr-x  1 cltbld users   5434 Dec 12  2005 modify-installer-url.pl
> -rwxr-xr-x  1 cltbld users   2714 Jun  1  2006 nuke-respins.sh
> -rwxr-xr-x  1 cltbld users   1410 Jul 17  2007 pushsnip
> -rwxr-xr-x  1 cltbld users   1072 Jul 31  2006 remove-aus-locale.sh
> -rwxr-xr-x  1 cltbld users   2567 Dec 12  2005 seamonkey-src-tarball-nobuild
> -rwxr-xr-x  1 cltbld users   2411 Dec 12  2005 thunderbird-src-tarball-nobuild
> -rwxr-xr-x  1 cltbld users    378 Jun  1  2006 update-download-stats.sh
> -rwxr-xr-x  1 cltbld users  10147 Sep 25  2006 verify-locales.pl
> -rwxr-xr-x  1 cltbld users   2180 Jan 27  2006 xulrunner-src-tarball-nobuild
> -bash-3.00$ 
> 

We don't need any of the others, this is just a checkout of /mofo/release/stage.
Ok, settled on "mozilla/tools/release/bin" for these. 

cvs commit: Examining .
RCS file: /cvsroot/mozilla/tools/release/bin/backupsnip,v
done
Checking in backupsnip;
/cvsroot/mozilla/tools/release/bin/backupsnip,v  <--  backupsnip
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/tools/release/bin/pushsnip,v
done
Checking in pushsnip;
/cvsroot/mozilla/tools/release/bin/pushsnip,v  <--  pushsnip
initial revision: 1.1
done
Attachment #310191 - Attachment description: pushnip (without doing backup) (retry) → [checked in] pushnip (without doing backup) (retry)
Attachment #310192 - Attachment description: backupsnip (without doing push) (retry) → [checked in] backupsnip (without doing push) (retry)
Comment on attachment 310195 [details] [diff] [review]
[checked in] Bootstrap::Updates::Push() should do backup before doing push

Checking in Updates.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Updates.pm,v  <--  Updates.pm
new revision: 1.39; previous revision: 1.38
done
Attachment #310195 - Attachment description: Bootstrap::Updates::Push() should do backup before doing push → [checked in] Bootstrap::Updates::Push() should do backup before doing push
1) This bug separates pushsnip into two scripts:
- backupsnip: backs up existing channels; can be run while waiting for formal "go"
- pushsnip: pushes new updates to channel; only run after formal "go".
...as well as update existing build automation to run both steps. These changes are now all checked into cvs and live in staging & production machines.


2) We also moved these scripts from private mofo repo to the normal public repo. 


3) I updated the cltbld account on aus2-staging to now default to: CVSROOT=":pserver:anonymous@cvs-mirror.mozilla.org:/cvsroot"
The contents of /home/cltbld/bin are from the public repo:
$ sudo su - cltbld
$ cd ~
$ cvs get -d bin mozilla/tools/release/bin/ 


4) For now, I've renamed the existing /home/cltbld/bin to /home/cltbld/bin.old. Even though we've already tested this on staging, I wanted to wait until we make it through the next release or two before deleting /home/cltbld/bin.old. I did confirm no local patches to pushsnip.


5) I've now updated the buildnotes for:
http://wiki.mozilla.org/Firefox_2.0.0.13:BuildNotes
http://wiki.mozilla.org/Firefox_3.0b5:BuildNotes
...to reflect the new reality of backupsnip being separate from pushsnip, as well as including instructions for how to ensure that the latest version of the scripts are being used for future releases.

Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.