396438 - bootstrap needs to automatically sync with stage

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

This is needed for each of the following steps: build, repack, sign, updates, stage

Comment 1

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Attachment: changes to Build.pm, Util.pm, Config.pm

This is a work-in-progress, not complete at all. However, want to get people reviewing this work for Build.pm, to comment if I am on the right track or not. 

If this seems ok, we can try it on the staging environment. If *that* works ok, it would be a similar fix to the other steps that have this problem.

First, lets figure out if this is the correct approach or not.

Comment 2

[Robert Helmer [:rhelmer]]

Comment on attachment 282975 [details] [diff] [review]
changes to Build.pm, Util.pm, Config.pm

>Index: Util.pm
>===================================================================
>RCS file: /cvsroot/mozilla/tools/release/Bootstrap/Util.pm,v
>retrieving revision 1.6
>diff -u -r1.6 Util.pm
>--- Util.pm	27 Sep 2007 17:17:26 -0000	1.6
>+++ Util.pm	1 Oct 2007 07:55:10 -0000
>@@ -61,6 +61,22 @@
>    return keys(%PLATFORM_MAP);
> }
> 
>+##
>+# GetFtpNightlyDir - construct the FTP path for pushing builds & updates to
>+# returns scalar
>+#
>+# no mandatory arguments
>+##
>+
>+sub GetFtpNightlyDir {
>+    my $this = shift;
>+
>+    my $product = $this->Get(var => 'product');
>+
>+    my $nightlyDir = CvsCatfile('/home', 'ftp', 'pub', $product, 'nightly') . '/';
>+    return $nightlyDir;  
>+}
>+



Where is "$product" being set here? You'd need to ask Config for it.
Also, GetFtpNightlyDir() is not exported so no other classes will be able to use it.



>Index: Step/Build.pm
>===================================================================
>RCS file: /cvsroot/mozilla/tools/release/Bootstrap/Step/Build.pm,v
>retrieving revision 1.17
>diff -u -r1.17 Build.pm
>--- Step/Build.pm	6 Sep 2007 18:56:52 -0000	1.17
>+++ Step/Build.pm	1 Oct 2007 07:55:10 -0000
>@@ -7,6 +7,7 @@
> 
> use Bootstrap::Step;
> use Bootstrap::Util qw(CvsCatfile);
>+use Bootstrap::Util GetFtpNightlyDir;
> 
> @ISA = ("Bootstrap::Step");
> 
>@@ -81,6 +82,7 @@
>     my $logDir = $config->Get(sysvar => 'logDir');
>     my $stagingUser = $config->Get(var => 'stagingUser');
>     my $stagingServer = $config->Get(var => 'stagingServer');
>+    my $internalStagingServer = $config->Get(var => 'internalStagingServer');
> 
>     my $rcTag = $productTag . '_RC' . $rc;
>     my $buildLog = catfile($logDir, 'build_' . $rcTag . '-build.log');
>@@ -95,6 +97,7 @@
>     }
>     $pushDir =~ s!^http://ftp.mozilla.org/pub/mozilla.org!/home/ftp/pub!;
> 
>+    my $nightlyDir = $config->GetFtpNightlyDir();
>     my $candidateDir = $config->GetFtpCandidateDir(bitsUnsigned => 1);
> 
>     my $osFileMatch = $config->SystemInfo(var => 'osname');    
>@@ -144,6 +147,17 @@
>                   $candidateDir],
>       logFile => $pushLog,
>     );
>+
>+    $this->Shell(
>+      cmd => 'ssh',
>+      cmdArgs => ['-2', '-l', $stagingUser, $internalStagingServer,
>+                  'rsync', '-av', 
>+                  '--include=*' . $osFileMatch . '*',
>+                  '--exclude=*', 
>+                  $pushDir, 
>+                  $nightlyDir],
>+      logFile => $pushLog,
>+    );
> }
> 
> sub Announce {


I think sshing to the $internalStagingServer first isn't necessary here; the client is running on that server.

You don't want $osFileMatch there, that'll only copy Linux files. There's no reason to include or exclude anything in this sync is there?

Also, that whole Shell call could just be a function in Bootstrap::Util, since you're going to be doing the same exact thing in several classes, something like SyncStaging()?

Comment 3

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Attachment: changes to Build.pm, Util.pm, Config.pm, bootstrap.cfg.example

ok, I think this addresses all the original comments, and everything we covered in IRC since. Fingers crossed! :-)

Comment 4

[Robert Helmer [:rhelmer]]

Comment on attachment 283104 [details] [diff] [review]
changes to Build.pm, Util.pm, Config.pm, bootstrap.cfg.example

this approach is fine. 

note that this won't do anything until we've converted everything to internalStagingServer. This patch could go in now but it'll be a no-op.

the way we are using Bootstrap::Util, and the methods in Bootstrap::Config that should be Util, etc. I filed bug 398223 about that though.

Comment 5

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Attachment: [checked in]Changes to Build.pm, Util.pm, Config.pm, Updates.pm, Source.pm, Repack.pm, PatcherConfig.pm, bootstrap.cfg.example

Comment 6

[Robert Helmer [:rhelmer]]

Comment on attachment 283113 [details] [diff] [review]
[checked in]Changes to Build.pm, Util.pm, Config.pm, Updates.pm, Source.pm, Repack.pm, PatcherConfig.pm, bootstrap.cfg.example  

yeah I think that should do it. I'll go ahead and land this.

Comment 7

[Robert Helmer [:rhelmer]]

Comment on attachment 283113 [details] [diff] [review]
[checked in]Changes to Build.pm, Util.pm, Config.pm, Updates.pm, Source.pm, Repack.pm, PatcherConfig.pm, bootstrap.cfg.example  

Checking in bootstrap.cfg.example;
/cvsroot/mozilla/tools/release/bootstrap.cfg.example,v  <--  bootstrap.cfg.example
new revision: 1.6; previous revision: 1.5
done
Checking in Bootstrap/Config.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Config.pm,v  <--  Config.pm
new revision: 1.14; previous revision: 1.13
done
Checking in Bootstrap/Util.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Util.pm,v  <--  Util.pm
new revision: 1.7; previous revision: 1.6
done
Checking in Bootstrap/Step/Build.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Build.pm,v  <--  Build.pm
new revision: 1.19; previous revision: 1.18
done
Checking in Bootstrap/Step/PatcherConfig.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/PatcherConfig.pm,v  <--  PatcherConfig.pm
new revision: 1.7; previous revision: 1.6
done
Checking in Bootstrap/Step/Repack.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Repack.pm,v  <--  Repack.pm
new revision: 1.18; previous revision: 1.17
done
Checking in Bootstrap/Step/Source.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Source.pm,v  <--  Source.pm
new revision: 1.8; previous revision: 1.7
done
Checking in Bootstrap/Step/Updates.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Updates.pm,v  <--  Updates.pm
new revision: 1.15; previous revision: 1.14
done

Comment 8

[Robert Helmer [:rhelmer]]

Attachment: [checked in] ssh to stagingServer before running rsync

Sorry, not sure what I was thinking earlier :) Of course you need to ssh to $stagingServer before running the rsync, if the command is running on an external slave :/

Also, I added better logging.

Staging run failed on Build, but I tested this patch on one of the failed slaves and it's able to complete now. I'm going to go ahead and land it so I can get another run started:

Checking in Bootstrap/Util.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Util.pm,v  <--  Util.pm
new revision: 1.8; previous revision: 1.7
done

Comment 9

[Robert Helmer [:rhelmer]]

This looks good on staging.

Comment 10

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Comment on attachment 283132 [details] [diff] [review]
[checked in] ssh to stagingServer before running rsync

Looks great. 

...and thanks for catching the redirect stderr, I missed that in my original patch.

Comment 11

[Robert Helmer [:rhelmer]]

Noticed one problem - if SyncNightlyDirToStaging() is running on two steps in parallel, we might see something like:

file has vanished: "/home/ftp/pub/firefox/nightly/2.0.0.7-candidates/rc2/unsigned/.firefox-2.0.0.7.fy-NL.win32.zip.SeoX4O"

This is only something we'd hit on staging, since we have externalStagingServer and stagingServer set the same, so we're basically rsyncing to the same location and seeing tempfiles in the target that we would not normally see.

Let's change externalStagingServer to be staging-prometheus-vm for now; it's a more realistic test, although it'd be ideal to have a staging FTP area at some point.

Comment 12

[Robert Helmer [:rhelmer]]

Attachment: [checked in]set externalServer to staging-prometheus-vm for staging only

Comment 13

[Robert Helmer [:rhelmer]]

Comment on attachment 283548 [details] [diff] [review]
[checked in]set externalServer to staging-prometheus-vm for staging only

Checking in fx-moz18-staging-bootstrap.cfg;
/cvsroot/mozilla/tools/release/configs/fx-moz18-staging-bootstrap.cfg,v  <--  fx-moz18-staging-bootstrap.cfg
new revision: 1.5; previous revision: 1.4
done

Comment 14

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

In https://intranet.mozilla.org/Release:_Firefox_2.0.0.8#Stage we still have to manually rsync files between private and public areas. Need to fix that before we close this bug.

Comment 15

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Attachment: rsync additional dirs missing from previous patches

1) Do rsync of additional directory

2) Change name of function, as its not only dealing with nightlies now.

Comment 16

[Robert Helmer [:rhelmer]]

Comment on attachment 285181 [details] [diff] [review]
rsync additional dirs missing from previous patches

>Index: Util.pm
>===================================================================
>RCS file: /cvsroot/mozilla/tools/release/Bootstrap/Util.pm,v
>retrieving revision 1.8
>diff -u -r1.8 Util.pm
>--- Util.pm	2 Oct 2007 05:21:13 -0000	1.8
>+++ Util.pm	17 Oct 2007 00:51:57 -0000
>+    $dirName = CvsCatfile($config->Get(var => 'stageHome'), $product.'-'.$version);


What's the reason $config->Get() is called here instead of declared at the top of the function (e.g. $logDir, $rc)?

Also, $version isn't defined in this function.

>     my $rv = RunShellCommand(command => $command,
>                              args => \@cmdArgs, 
>                              redirectStderr => 1,
>                              logfile => $pushLog);

$rv is already declared above, drop the "my" from this one.

Looks good to me besides that, and passes "make test" when I make the above changes.

Comment 17

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Attachment: [checked in ]rsync additional dirs missing from previous patches (2nd attempt)

Sorry about that, I forgot to run "make test", which would have caught both of those typos. Fixed, and this time confirmed that "make test" went fine.

Comment 18

[Robert Helmer [:rhelmer]]

Comment on attachment 285187 [details] [diff] [review]
[checked in ]rsync additional dirs missing from previous patches (2nd attempt)

Checking in Bootstrap/Util.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Util.pm,v  <--  Util.pm
new revision: 1.9; previous revision: 1.8
done
Checking in Bootstrap/Step/Build.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Build.pm,v  <--  Build.pm
new revision: 1.20; previous revision: 1.19
done
Checking in Bootstrap/Step/Repack.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Repack.pm,v  <--  Repack.pm
new revision: 1.19; previous revision: 1.18
done
Checking in Bootstrap/Step/Sign.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Sign.pm,v  <--  Sign.pm
new revision: 1.9; previous revision: 1.8
done
Checking in Bootstrap/Step/Source.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Source.pm,v  <--  Source.pm
new revision: 1.11; previous revision: 1.10
done
Checking in Bootstrap/Step/Updates.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Updates.pm,v  <--  Updates.pm
new revision: 1.24; previous revision: 1.23
done

Comment 19

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

This last patch closes out the remaining issues, so I'm closing this bug. We need to include this fix in a newer automation tag, before we start producing the next release.

(thanks to rhelmer for the review, and landing this last patch)

Comment 20

[Robert Helmer [:rhelmer]]

Couple things noticed after staging run:

1) private area is one level too deep e.g.:
/data/cltbld/firefox-2.0.0.7/firefox-2.0.0.7

2) SyncToStaging() needs to be called from Bootstrap::Step::Stage, at the end of Execute() would work.

Comment 21

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Attachment: [checked in]Tweak Util.pm, Stage.pm to resolve review comments

I think this fixes the two problems rhelmer saw in his review at: https://bugzilla.mozilla.org/show_bug.cgi?id=396438#c20

Comment 22

[Robert Helmer [:rhelmer]]

Comment on attachment 285812 [details] [diff] [review]
[checked in]Tweak Util.pm, Stage.pm to resolve review comments

Checking in Bootstrap/Util.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Util.pm,v  <--  Util.pm
new revision: 1.10; previous revision: 1.9
done
Checking in Bootstrap/Step/Stage.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Stage.pm,v  <--  Stage.pm
new revision: 1.23; previous revision: 1.22
done

Comment 23

[Robert Helmer [:rhelmer]]

Started a staging run with this code.

Comment 24

[Robert Helmer [:rhelmer]]

This does the right thing now, thanks!