Closed Bug 415970 Opened 13 years ago Closed 13 years ago

automation should not need slave/ftp on master server

Categories

(Release Engineering :: General, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: rhelmer, Assigned: rhelmer)

References

Details

Attachments

(8 files, 13 obsolete files)

4.84 KB, patch
bhearsum
: review+
Details | Diff | Splinter Review
1.59 KB, patch
nthomas
: review+
Details | Diff | Splinter Review
473 bytes, patch
nthomas
: review+
Details | Diff | Splinter Review
6.35 KB, patch
Details | Diff | Splinter Review
25.86 KB, patch
Details | Diff | Splinter Review
5.78 KB, patch
Details | Diff | Splinter Review
658 bytes, patch
nthomas
: review+
Details | Diff | Splinter Review
3.17 KB, patch
nthomas
: review+
Details | Diff | Splinter Review
We should not need to run a slave on the master server, and the master server should not need to serve as FTP. More specifically:

1) tag, source, stage steps should be runnable on slaves - they assume that they are running on the stagingServer and have direct access to /home/ftp and /data/cltbld
2) build, repack, update steps push to stagingServer then sync to externalStagingServer - they should just push once to what is now externalStagingServer
Assignee: nobody → rhelmer
We should check that the chroot jail on the new ftp server doesn't cause any problems for this plan.
(In reply to comment #1)
> We should check that the chroot jail on the new ftp server doesn't cause any
> problems for this plan.

Is this because there's a delay between the time builds are uploaded and the time they are available via FTP/HTTP?
Status: NEW → ASSIGNED
(In reply to comment #0)
> 1) tag, source, stage steps should be runnable on slaves - they assume that
> they are running on the stagingServer and have direct access to /home/ftp and
> /data/cltbld

Hmm.. actually, I don't see any reason these shouldn't be runnable right now.. there's really no reason that I can tell to create batch-source in stageHome, we push the final product into the candidates area, and that's what's used for staging.

We do need to remove the SyncToStaging() call, though.

> 2) build, repack, update steps push to stagingServer then sync to
> externalStagingServer - they should just push once to what is now
> externalStagingServer

Again, these would be fine if we removed the SyncToStaging() call.

The only problem I can find is that Stage.pm expects to be able to get the builds directly from /data/ftp/.../, a simple change would be to make it just rsync (or whatever) down the builds, do the staging thing, push the result to private staging area on stage.

comment #1 might have an effect here, though, the main problem I can think of is that en-US pushes to stage.m.o, then l10n expects the builds to be immediately available to download. This seems like something we would want to fix anyway, esp. seeing as how en-US and l10n are done on the same boxes :)

Even if they weren't, I think we could e.g. use the private staging area and rsync instead, or wait for the builds to get virus scanned/published before l10n repack, or give the build machines some kind of direct access to the pre-published area. We need to solve this for nightlies, anyway, right?

Attached patch move most builders off of master (obsolete) — Splinter Review
* remove externalStagingServer in config and PatcherConfig/Updates
* remove SyncToStaging calls in Build, Repack
* move builders to staging-prometheus-vm

I've left the SyncToStaging() call in Source, although it seems like we should just not leave that batch-source dir laying around in the private staging area.

Also, I've left Stage alone, as well as the cleanup and Sign builders. We need to move the CVS mirror, stage.m.o staging box, etc. somewhere else, and teach Stage to download, do it's thing, and push back, so that's for a later patch after more setup :)
Attachment #309192 - Flags: review?(nrthomas)
Attachment #309201 - Flags: review?(nrthomas)
Hopefully this makes these two patches make more sense!

Also, note that I haven't done Stage yet, it would not work as-is. If we manually rsync'd stuff down after signing, it would, but there's no reason not to just fix that in this bug as well.
Attachment #309259 - Flags: review?(nrthomas)
Whiteboard: waiting for revie
Whiteboard: waiting for revie → waiting for review
Comment on attachment 309201 [details] [diff] [review]
add sourceDir var and remove SyncToStaging() from Source step

>Index: release/Bootstrap/Step/Source.pm
>===================================================================
>-    my $stageHome = $config->Get(var => 'stageHome');
>+    my $sourceDir = $config->Get(var => 'sourceDir');
>     my $mozillaCvsroot = $config->Get(var => 'mozillaCvsroot');
> 
>     # create staging area
>-    my $stageDir = catfile($stageHome, $product . '-' . $version, 
>+    my $stageDir = catfile($sourceDir, $product . '-' . $version, 
>                            'batch-source', 'rc' . $rc);

I would prefer a var named versionedSourceDir instead of stageDir, to be similar to the other Steps.

>@@ -114,8 +113,6 @@
>       logFile => catfile($logDir, 'source.log'),
>       dir => catfile($stageDir),
>     );
>-
>-    SyncToStaging();
> }

The rsync just prior to this removal needs to be reworked to publish the tarball to the candidates dir of the $stagingServer (r- for this only).

Looks like all other users of shipped-locales already check it out, so there shouldn't be any fallout from not having a /data/cltbld/$app-$version/$rc/ full of source.
Attachment #309201 - Flags: review?(nrthomas) → review-
Comment on attachment 309192 [details] [diff] [review]
move most builders off of master

Looks good, r+, but see review of the configs before landing.
Attachment #309192 - Flags: review?(nrthomas) → review+
Comment on attachment 309259 [details] [diff] [review]
config changes to go with moving builders off master

>Index: fx-moz18-bootstrap.cfg
>===================================================================
> # username and server to push builds
> sshUser         = cltbld
>-sshServer       = production-1.8-master.build.mozilla.org
>+sshServer       = stage.mozilla.org

You didn't change stagingServer here, so with the (eg) Build.pm changes there will be an attempt to sync between two empty locations on the master. Will all the production (and 1.8 nightly) have stage-old.m.o for both stagingServer and sshServer, while staging and (the 1.9 nightly) point to their master still ? Or are the staging builds going to get buried somewhere on ftp.m.o ?

Looks like we only use sshUser and sshServer in the CONFIG lines of tinder-config.pl. Can we just modify the tinder-config.pl to use stagingUser & stagingServer ?

externalStagingUser/Server is only removed from some configs. Is that because Fx & Tb 20013 are in play ?
Attachment #309259 - Flags: review?(nrthomas) → review-
Attachment #309981 - Flags: review?(nrthomas)
Comment on attachment 309981 [details] [diff] [review]
add sourceVar and remove SyncToStaging from Source take 2

>Index: Bootstrap/Step/Source.pm
>===================================================================
>@@ -110,12 +111,10 @@
>       cmd => 'rsync',
>       cmdArgs => ['-av', catfile('batch-source', 'rc' . $rc, 
>                             $product . '-' . $version . '-source.tar.bz2'),
>-                  $candidateDir],
>+                  $stagingServer . ':' . $candidateDir],
>       logFile => catfile($logDir, 'source.log'),
>-      dir => catfile($stageDir),
>+      dir => catfile($versionedSourceDir),
>     );
>-
>-    SyncToStaging();
> }

r+ if you look up $stagingUser and push to (something like)
   $stagingUser . '@' . $stagingServer . ':' . $candidateDir]
Attachment #309981 - Flags: review?(nrthomas) → review+
Whiteboard: waiting for review
I think this is the final piece here; since SyncToStaging() isn't used anywhere else now, I've renamed it SyncStage() and it does an actual synchronize operation (downloads from stage->local and uploads local->stage, using rsync).

So, Stage calls SyncStage() before it starts, and after it's finished. This is needed because all of the other steps are pushing to stage.mozilla.org now, not the machine that Stage is running on.
Attachment #310292 - Flags: review?(nrthomas)
Whiteboard: waiting for review
Attachment #310292 - Attachment is obsolete: true
Attachment #310292 - Flags: review?(nrthomas)
Comment on attachment 310292 [details] [diff] [review]
sync to/from staging before running Stage step

Nick points out in irc that:

1) SyncStaging doesn't need to ssh to stageServer and rsync from externalStagingServer; just nix the externalStagingServer part

2) this function is only called in Stage.pm, and we don't need a full sync every time. I'll just move the rsync code straight into Stage.pm and remove the function.
Attached patch all together now (obsolete) — Splinter Review
Here are all the changes together, probably easier to read like this as they are all related (removing SyncToStaging(), etc.)
Attachment #309192 - Attachment is obsolete: true
Attachment #309201 - Attachment is obsolete: true
Attachment #309981 - Attachment is obsolete: true
Attachment #310384 - Flags: review?(nrthomas)
The only outstanding issue that I know of is that we use a slave on the buildbot master to update the bootstrap-configs dir (which is then downloaded by the slaves from the master).

Any ideas on what to do here? Maybe we could make the slaves check this out straight from CVS, instead?
This goes with the changes in attachment 310384 [details] [diff] [review]. I still left the slave that's running on the master for:

* prestage - (the bootstrap-config issue I mentioned before specifically, and the fact that it's still the "stage.m.o" staging site)

* sign - not sure what to do about this either, actually. Maybe we should just ping $stagingServer over HTTP, looking for this logfile? Right now it looks on the local filesystem to know that signing is completed.
Attachment #309259 - Attachment is obsolete: true
Attachment #310570 - Flags: review?(nrthomas)
(In reply to comment #16)
> * sign - not sure what to do about this either, actually. Maybe we should just
> ping $stagingServer over HTTP, looking for this logfile? Right now it looks on
> the local filesystem to know that signing is completed.

To elaborate, by "ping" I mean "do HTTP HEAD and look for OK result", or something along those lines.

Assignee: rhelmer → nobody
Status: ASSIGNED → NEW
Component: Build & Release → Release Engineering
QA Contact: build → release
Assignee: nobody → rhelmer
Status: NEW → ASSIGNED
Comment on attachment 310384 [details] [diff] [review]
all together now

>Index: Bootstrap/Util.pm
>===================================================================
OK

>Index: Bootstrap/Step/Build.pm
>===================================================================
>@@ -184,8 +184,6 @@
>                   $candidateDir],
>       logFile => $pushLog,
>     );
>-
>-    SyncToStaging(); 
> }

Before this snippet is code to create the candidates dir and set appropriate perms, which is duplicated in Repack and should be in Source now too. Would probably be worth factoring it out elsewhere.

>Index: Bootstrap/Step/PatcherConfig.pm
>===================================================================
OK

>Index: Bootstrap/Step/Repack.pm
>===================================================================
OK (but see refactor comment in Build.pm)

>Index: Bootstrap/Step/Sign.pm
>===================================================================
OK

>Index: Bootstrap/Step/Source.pm
>===================================================================
>@@ -94,9 +94,10 @@ Push()
> 
>-    my $stageDir =  catfile($stageHome, $product . '-' . $version);
>+    my $versionedSourceDir =  catfile($sourceDir, $product . '-' . $version);
>     my $candidateDir = catfile('/home', 'ftp', 'pub', $product, 'nightly',
>                             $version . '-candidates', 'rc' . $rc ) . '/';

Could use GetFtpCandidateDir here with bitsUnsigned => 0.

>@@ -110,12 +111,10 @@
>       cmd => 'rsync',

Right before this there is a local mkdir, instead of ssh'ing onto stagingServer to do it.

>       cmdArgs => ['-av', catfile('batch-source', 'rc' . $rc, 
>                             $product . '-' . $version . '-source.tar.bz2'),
>-                  $candidateDir],
>+                  $stagingServer . ':' . $candidateDir],

Need to use stagingUser here.

>Index: Bootstrap/Step/Stage.pm
>===================================================================
>+    # download public and private staging areas
>+    my $ftpNightlyDir = $config->GetFtpNightlyDir();

You'd need GetFtpCandidateDir(bitsUnsigned=>0) here, otherwise it'll sync waaaay too much. And sub below.

>+    my $versionedStageHome = CvsCatfile($stageHome, $product.'-'.$version.'/');
>+
>+    $this->Shell(
>+      cmd => 'rsync',
>+      cmdArgs => ['-av', 
>+                  $stagingUser . '@' .  $stagingServer . ':' . $ftpNightlyDir,
>+                  $ftpNightlyDir],
>+      logFile => catfile($logDir, 'download_stage_public.log'),
>+    );
>+
>+    $this->Shell(
>+      cmd => 'rsync',
>+      cmdArgs => ['-av', $stagingUser . '@' .  $stagingServer . ':' . 
>+                  $versionedStageHome, $versionedStageHome],
>+      logFile => catfile($logDir, 'download_stage_private.log'),
>+    );
>+

What's this second rsync for ? The changes to Source mean the private area should be empty at this point, right ?

>     ## Prepare the staging directory for the release.
>     # Create the staging directory.

The rsync should follow this directory creation block. GetStageDir() also saves on constructing $versionedStageHome.
 
>@@ -419,7 +441,37 @@
>+sub Push {

Nit, Push should follow Verify ?

>+    # upload public and private staging areas
>+    my $ftpNightlyDir = $config->GetFtpNightlyDir();
>+    my $versionedStageHome = CvsCatfile($stageHome, $product.'-'.$version.'/');

See comments above.

>+    $this->Shell(
>+      cmd => 'rsync',
>+      cmdArgs => ['-av', $ftpNightlyDir,
>+                  $stagingUser . '@' .  $stagingServer . ':' . $ftpNightlyDir],
>+      logFile => catfile($logDir, 'download_stage_public.log'),
>+    );

I don't grok this one ? There's nothing to push back to the candidates dir after staging.

>+    $this->Shell(
>+      cmd => 'rsync',
>+      cmdArgs => ['-av', $versionedStageHome,
>+                  $stagingUser . '@' .  $stagingServer . ':' . 
>+                  versionedStageHome],
>+      logFile => catfile($logDir, 'download_stage_private.log'),
>+    );
> }

s/download/upload/

>Index: Bootstrap/Step/Updates.pm
>===================================================================
OK

>Index: configs/fx-moz18-staging-bootstrap.cfg
>===================================================================
>Index: configs/fx-moz19-staging-bootstrap.cfg
>===================================================================

Need to define sourceDir in both.
Attachment #310384 - Flags: review?(nrthomas) → review-
You know, with candidates dirs being ~4GB, it might make sense to just rsync from
   stagingServer:$candidatesDir ----> ../prestage-trimmed
and go from there. That'd avoid two useless copies (the candidates dir, and prestage/) if the slaves are not overburdened with disk space.



Comment on attachment 310570 [details] [diff] [review]
master.cfg slave changes corresponding to boostrap patch

Looks good, r+.

With these changes, seems likely the 15GB disk on fx-linux-1.9-slave2 will be too small. Hopefully the 50GB on fx-linux-1.9-slave1 will be alright.
Attachment #310570 - Flags: review?(nrthomas) → review+
(In reply to comment #17)
> (In reply to comment #16)
> > * sign - not sure what to do about this either, actually. Maybe we should just
> > ping $stagingServer over HTTP, looking for this logfile? Right now it looks on
> > the local filesystem to know that signing is completed.
> 
> To elaborate, by "ping" I mean "do HTTP HEAD and look for OK result", or
> something along those lines.

Sounds fine to me.

Whiteboard: waiting for review
Agreed on all counts, thanks! Pretty sure this one addresses everything.

I'll start testing this, it's a pretty big patch so I wouldn't be surprised if there are a couple fiddly bits to sort out.
Attachment #310384 - Attachment is obsolete: true
Attachment #310786 - Flags: review?(nrthomas)
(In reply to comment #19)
> You know, with candidates dirs being ~4GB, it might make sense to just rsync
> from
>    stagingServer:$candidatesDir ----> ../prestage-trimmed
> and go from there. That'd avoid two useless copies (the candidates dir, and
> prestage/) if the slaves are not overburdened with disk space.

I tried to address this in the latest patch too.

(In reply to comment #15)
> The only outstanding issue that I know of is that we use a slave on the
> buildbot master to update the bootstrap-configs dir (which is then downloaded
> by the slaves from the master).
> 
> Any ideas on what to do here? Maybe we could make the slaves check this out
> straight from CVS, instead?

Er, so it occurred to me, we're checking out these files on each slave anyway, in mozilla/tools/release/configs .. we could just tell BootstrapFactory to refer to a file in there instead of uploading one from the master.
Whiteboard: waiting for review
What do you think about this? I kind of like it better than the way we're doing it anyway..
Attachment #311044 - Flags: review?(bhearsum)
Blocks: 408958
Comment on attachment 311044 [details] [diff] [review]
[checked in] use local bootstrap.cfg

We'll have to update the other masters too.
Attachment #311044 - Flags: review?(bhearsum) → review+
Comment on attachment 311044 [details] [diff] [review]
[checked in] use local bootstrap.cfg

Checking in buildbot-configs/automation/staging/master.cfg;
/cvsroot/mozilla/tools/buildbot-configs/automation/staging/master.cfg,v  <--  master.cfg
new revision: 1.33; previous revision: 1.32
done
Checking in buildbotcustom/process/factory.py;
/cvsroot/mozilla/tools/buildbotcustom/process/factory.py,v  <--  factory.py
new revision: 1.3; previous revision: 1.2
done
Attachment #311044 - Attachment description: use local bootstrap.cfg → [checked in] use local bootstrap.cfg
Comment on attachment 310786 [details] [diff] [review]
address nick's comments on attachment 310384 [details] [diff] [review]

>Index: Bootstrap/Step/Build.pm
>===================================================================
...
>-    my $candidateDir = $config->GetFtpCandidateDir(bitsUnsigned => 1);
>-
>-    my $osFileMatch = $config->SystemInfo(var => 'osname');    
>-
>-    # TODO - use a more generic function for this kind of remapping
>-    if ($osFileMatch eq 'win32')  {
>-      $osFileMatch = 'win';
>-    } elsif ($osFileMatch eq 'macosx') {
>-      $osFileMatch = 'mac';
>-    }

This section is needed for the rsync on stagingServer, so it should stay in Build.pm. 

>Index: Bootstrap/Step/Repack.pm
>===================================================================
>-    my $candidateDir = $config->GetFtpCandidateDir(bitsUnsigned => 1);
>-
>-    my $osFileMatch = $config->SystemInfo(var => 'osname');
>-    if ($osFileMatch eq 'win32')  {
>-      $osFileMatch = 'win';
>-    } elsif ($osFileMatch eq 'macosx') {
>-      $osFileMatch = 'mac';
>-    }

Same again.

>Index: Bootstrap/Step/Source.pm
>===================================================================
>     my $version = $config->GetVersion(longName => 0);
>     my $rc = $config->Get(var => 'rc');
>     my $logDir = $config->Get(sysvar => 'logDir');
>-    my $stageHome = $config->Get(var => 'stageHome');
>+    my $sourceDir = $config->Get(var => 'sourceDir');
>+    my $stagingUser = $config->Get(var => 'stagingUser');
>+    my $stagingServer = $config->Get(var => 'stagingServer');
> 
>-    my $stageDir =  catfile($stageHome, $product . '-' . $version);
>-    my $candidateDir = catfile('/home', 'ftp', 'pub', $product, 'nightly',
>-                            $version . '-candidates', 'rc' . $rc ) . '/';

Still need $candidateDir for the upload that follows.

>Index: Bootstrap/Step/Stage.pm
>===================================================================
>+    # download public and private staging areas
>+    my $ftpNightlyDir = $config->GetFtpCandidateDir(bitsUnsigned => 0);
> 
>     $this->Shell(
>       cmd => 'rsync',
>-      cmdArgs => ['-Lav', catfile('/home', 'ftp', 'pub', $product, 'nightly',
>-                                    $version . '-candidates', 'rc' . $rc ) . 
>-                                 '/',
>-                    './'],
>-      logFile => catfile($logDir, 'stage_collect.log'),
>-      dir => $prestageDir
>+      cmdArgs => ['-av', 
>+                  $stagingUser . '@' .  $stagingServer . ':' . $ftpNightlyDir,
>+                  $ftpNightlyDir],
>+      logFile => catfile($logDir, 'download_stage_public.log'),
>     );

Do we have anything in place to make sure all but the last child directory already exists on the slave ? eg /home/ftp/pub/firefox/nightly/3.0b4-candidates/. Seems like rsync will barf otherwise.

>-    # Create a pruning/"trimmed" area; this area will be used to remove
>+    # Collect the release files from the candidates directory into 
>+    # a pruning/"trimmed" area; this area will be used to remove
>     # locales and deliverables we don't ship. 
>     $this->Shell(
>       cmd => 'rsync',
>-      cmdArgs => ['-av', 'prestage/', 'prestage-trimmed/'],
>-      logFile => catfile($logDir, 'stage_collect_trimmed.log'),
>+      cmdArgs => ['-Lav', catfile('/home', 'ftp', 'pub', $product, 'nightly',
>+                                    $version . '-candidates', 'rc' . $rc ) . 
>+                                 '/',

Isn't the catfile in cmdArgs the same as ftpNightlyDir ? 

I was originally proposing combining this rsync with the previous one, so that we don't have a copy of /home/ftp/... on the slave - it syncs from stagingServer directly into .../batch1/prestage-trimmed. Not set on this idea if we have space to play with, but it'd resolve both my points here.

>+sub Push {
>+    my $this = shift;
>+
>+    my $config = new Bootstrap::Config();
>+    my $version = $config->GetVersion(longName => 0);
>+    my $product = $config->Get(var => 'product');
>+    my $productTag = $config->Get(var => 'productTag');
>+    my $rc = $config->Get(var => 'rc');
>+    my $logDir = $config->Get(sysvar => 'logDir');
>+    my $stageHome = $config->Get(var => 'stageHome');
>+    my $stagingUser = $config->Get(var => 'stagingUser');
>+    my $stagingServer = $config->Get(var => 'stagingServer');
>+
>+    # upload public and private staging areas
>+    my $ftpNightlyDir = $config->GetFtpCandidateDir(bitsUnsigned => 0);

Nit - Several of these vars aren't used.
Attachment #310786 - Flags: review?(nrthomas) → review-
Whiteboard: waiting for review
Whiteboard: testing on staging
This addresses everything in comment 28, and is being tested on staging. I'll either add a new patch or set r? on this once it's working there.
Attachment #310786 - Attachment is obsolete: true
Priority: P3 → P2
Comment on attachment 312436 [details] [diff] [review]
remove dependency on "master slave" take 4

Tested on stage, almost the same as before. Mostly little typos, the only major change is I made the stage process skip "prestage" and "prestage-trimmed" and just do everything from "stage-unsigned".

This way we end up with identical stage-signed and stage-unsigned but nothing else in batch1. I had to move the MAR generation/removal sooner in order to accommodate this.

Nick, do you think it's even necessary to have the separate stage-signed and stage-unsigned dirs? Do you find any practical use to it?
Attachment #312436 - Flags: review?(nrthomas)
Attachment #311640 - Attachment is obsolete: true
* fix log settings (got set to "logs.nightly" for prod)
* move almost all staging-1.8-master builders to staging-prometheus-vm
* split out cvsmirror to separate builder as that's still on staging-1.9-master slave
Attachment #310570 - Attachment is obsolete: true
Attachment #312437 - Flags: review?(nrthomas)
Whiteboard: testing on staging → waiting for review
Blocks: 415180
Attachment #312494 - Flags: review? → review?(nrthomas)
Sorry, missed one last SyncToStaging() call in Updates::Push :) Should be the same as take 4 besides.
Attachment #312436 - Attachment is obsolete: true
Attachment #312566 - Flags: review?
Attachment #312436 - Flags: review?(nrthomas)
Attachment #312566 - Flags: review? → review?(nrthomas)
Blocks: 417779
Comment on attachment 312437 [details] [diff] [review]
master.cfg slave changes corresponding to boostrap patch

>Index: master.cfg
>===================================================================
...
>cvsroot = ":ext:cltbld@cvs.mozilla.org:/cvsroot"
>-cvsroot = ":ext:stgbld@cvs.mozilla.org:/cvsroot"
>+##cvsroot = ":ext:stgbld@cvs.mozilla.org:/cvsroot"
> ##cvsroot = ":pserver:anonymous@cvs-mirror.mozilla.org:/cvsroot"
>+cvsroot = ":ext:cltbld@staging-1.8-master:/cvs/cvsmirror/cvsroot"

Apparently this is for testing only and won't be checked in.

>-prestageFactory = BootstrapFactory(
>+cvsmirrorFactory = BootstrapFactory(
>  cvsroot=cvsroot, 
>  cvsmodule=cvsmodule, 

Would be good to preserve the scheduler order when defining the factories, so move the prestageFactory above cvsmirrorFactory on checkin.

We'll need matching changes to 1.9 staging, and the two release configs prior to landing the Bootstrap changes ? Fx2.0.0.14 firedrill is imminent :-(
Attachment #312437 - Flags: review?(nrthomas) → review+
Comment on attachment 312494 [details] [diff] [review]
[checked in] 1.8 staging bootstrap changes

r+
Attachment #312494 - Flags: review?(nrthomas) → review+
Comment on attachment 312566 [details] [diff] [review]
remove dependency on "master slave" take 5

>Index: Bootstrap/Step.pm
>===================================================================
...
>+sub CreateCandidatesDir() {
>+    my $this = shift;
>+
>+    my $config = new Bootstrap::Config();
>+
>+    my $stagingUser = $config->Get(var => 'stagingUser');    
>+    my $stagingServer = $config->Get(var => 'stagingServer');    
X >+    my $osFileMatch = $config->SystemInfo(var => 'osname');    
>+    my $candidateDir = $config->GetFtpCandidateDir(bitsUnsigned => 1);
>+
X >+    # TODO - use a more generic function for this kind of remapping
X >+    if ($osFileMatch eq 'win32')  {
X >+      $osFileMatch = 'win';
X >+    } elsif ($osFileMatch eq 'macosx') {
X >+      $osFileMatch = 'mac';
X >+    }

Lines marked X look surplus, please remove on checkin if you agree.

>Index: Bootstrap/Step/Source.pm
>===================================================================
>@@ -94,28 +94,25 @@ Push
>     my $version = $config->GetVersion(longName => 0);
>     my $rc = $config->Get(var => 'rc');
>     my $logDir = $config->Get(sysvar => 'logDir');
>-    my $stageHome = $config->Get(var => 'stageHome');
>+    my $sourceDir = $config->Get(var => 'sourceDir');
>+    my $stagingUser = $config->Get(var => 'stagingUser');
>+    my $stagingServer = $config->Get(var => 'stagingServer');
> 
>-    my $stageDir =  catfile($stageHome, $product . '-' . $version);
>     my $candidateDir = catfile('/home', 'ftp', 'pub', $product, 'nightly',
>-                            $version . '-candidates', 'rc' . $rc ) . '/';
>+                               $version . '-candidates', 'rc' . $rc ) . '/';

Nit: |my $candidateDir = $config->GetFtpCandidateDir(bitsUnsigned => 0);| works here, right ?

>Index: Bootstrap/Step/Stage.pm
>===================================================================
>+sub Push {
>+    my $this = shift;
>+
>+    my $config = new Bootstrap::Config();
>+    my $logDir = $config->Get(sysvar => 'logDir');
>+    my $stageHome = $config->Get(var => 'stageHome');
>+    my $stagingUser = $config->Get(var => 'stagingUser');
>+    my $stagingServer = $config->Get(var => 'stagingServer');
>+
>+    # upload public and private staging areas
>+    my $ftpNightlyDir = $config->GetFtpCandidateDir(bitsUnsigned => 0);

Nit: var isn't used.

r+ with appropriate changes on checkin.
Attachment #312566 - Flags: review?(nrthomas) → review+
Whiteboard: waiting for review
1.9 bootstrap and master.cfg changes
Attachment #313497 - Flags: review?(nrthomas)
(In reply to comment #35)
> (From update of attachment 312437 [details] [diff] [review])
> >Index: master.cfg
> >==============================================
> We'll need matching changes to 1.9 staging, and the two release configs prior
> to landing the Bootstrap changes ? Fx2.0.0.14 firedrill is imminent :-(

BTW we discussed in irc, 2.0.0.14 should probably use the existing bootstrap tag, master.cfg etc. to be on the safe side, so shouldn't be a factor here.

Whiteboard: waiting for review
Comment on attachment 313497 [details] [diff] [review]
bootstrap and buildbot changes for 1.9

>Index: release/configs/fx-moz19-staging-bootstrap.cfg
>===================================================================

Need to define sourceDir too.

>-stagingServer   = staging-1.9-master.build.mozilla.org
>+stagingServer   = fx-linux-1.9-slave1.build.mozilla.org
> externalStagingUser     = cltbld
> externalStagingServer   = fx-linux-1.9-slave1.build.mozilla.org

externalStaging* got removed in the 1.8 config.

>Index: buildbot-configs/automation/staging-1.9/master.cfg
>===================================================================
>+cvsmirror_depscheduler = Dependent(
>+ name="cvsmirror_dep",
>+ upstream=prestage_depscheduler,
>+ builderNames=["cvsmirror"],
>+)
>+build_depscheduler = Dependent(

Need to remove the build_depscheduler line, looks like a paste-o.

>+c['builders'].append(
>+ {
>+  'name': 'prestage',
>+  'slavename': 'staging-1.9-master',
>+  'category': 'release',
>+  'locks': [linux_lock],
>+  'builddir': 'prestage',
>+  'factory': prestageFactory,
>+ },

This is happening on staging-prometheus-vm for 1.8, and you have the linux lock, so should be fx-linux-1.9-slave1 here ? I see make stage pulls in the previous release.

>-prestageFactory.addStep(ShellCommand,
>+
>+cvsmirrorFactory = BootstrapFactory(
>+ cvsroot=cvsroot, 
>+ cvsmodule=cvsmodule, 
>+ automation_tag=automation_tag,
>+ logdir='/builds/logs', 
>+ bootstrap_config='configs/fx-moz18-staging-bootstrap.cfg',

...-moz19-...

Seems fine otherwise, r+ with changes on checkin.
Attachment #313497 - Flags: review?(nrthomas) → review+
Checking in master.cfg;
/cvsroot/mozilla/tools/buildbot-configs/automation/staging/master.cfg,v  <--  master.cfg
new revision: 1.34; previous revision: 1.33
done
Attachment #312437 - Attachment is obsolete: true
Comment on attachment 312494 [details] [diff] [review]
[checked in] 1.8 staging bootstrap changes

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.23; previous revision: 1.22
done
Attachment #312494 - Attachment description: 1.8 staging bootstrap changes → [checked in] 1.8 staging bootstrap changes
Checking in Bootstrap/Step.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step.pm,v  <--  Step.pm
new revision: 1.17; previous revision: 1.16
done
Checking in Bootstrap/Util.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Util.pm,v  <--  Util.pm
new revision: 1.15; previous revision: 1.14
done
Checking in Bootstrap/Step/Build.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Build.pm,v  <--  Build.pm
new revision: 1.28; previous revision: 1.27
done
Checking in Bootstrap/Step/PatcherConfig.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/PatcherConfig.pm,v  <--  PatcherConfig.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.25; previous revision: 1.24
done
Checking in Bootstrap/Step/Sign.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Sign.pm,v  <--  Sign.pm
new revision: 1.11; previous revision: 1.10
done
Checking in Bootstrap/Step/Source.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Source.pm,v  <--  Source.pm
new revision: 1.14; previous revision: 1.13
done
Checking in Bootstrap/Step/Stage.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Stage.pm,v  <--  Stage.pm
new revision: 1.36; previous revision: 1.35
done
Checking in Bootstrap/Step/Updates.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Updates.pm,v  <--  Updates.pm
new revision: 1.42; previous revision: 1.41
done
Attachment #312566 - Attachment is obsolete: true
Attachment #313520 - Attachment is patch: true
Attachment #313520 - Attachment mime type: application/octet-stream → text/plain
Attachment #313497 - Attachment is obsolete: true
Checking in release/configs/fx-moz19-staging-bootstrap.cfg;
/cvsroot/mozilla/tools/release/configs/fx-moz19-staging-bootstrap.cfg,v  <--  fx-moz19-staging-bootstrap.cfg
new revision: 1.18; previous revision: 1.17
done
Checking in buildbot-configs/automation/staging-1.9/master.cfg;
/cvsroot/mozilla/tools/buildbot-configs/automation/staging-1.9/master.cfg,v  <--  master.cfg
new revision: 1.33; previous revision: 1.32
done
Attachment #313520 - Attachment description: remove dependency on "master slave" as landed → [checked in] remove dependency on "master slave" [as landed]
Attachment #313518 - Attachment description: [checked in] 1.8 master.cfg changes, as landed → [checked in] 1.8 master.cfg changes [as landed]
Attachment #313499 - Flags: review?(nrthomas) → review+
Comment on attachment 313499 [details] [diff] [review]
[checked in] make clean_stage should clean sourceDir too

Checking in Makefile;
/cvsroot/mozilla/tools/release/Makefile,v  <--  Makefile
new revision: 1.29; previous revision: 1.28
done
Attachment #313499 - Attachment description: make clean_stage should clean sourceDir too → [checked in] make clean_stage should clean sourceDir too
Blocks: 427166
Whiteboard: waiting for review
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Nick noticed this on staging; because there is no trailing slash, if /data/cltbld/firefox-2.0.0.13 already exists on the FTP server, then the "Stage" slave will push to /data/cltbld/firefox-2.0.0.13/firefox-2.0.0.13
Attachment #314143 - Flags: review?(nrthomas)
Comment on attachment 314143 [details] [diff] [review]
[checked in] do not create a new dir if $product-$version dir already exists

r+. For kicks, could you fix the comment which immediately precedes this diff,
   # upload public and private staging areas
to just say private.
Attachment #314143 - Flags: review?(nrthomas) → review+
Comment on attachment 314143 [details] [diff] [review]
[checked in] do not create a new dir if $product-$version dir already exists

Checking in Bootstrap/Step/Stage.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Stage.pm,v  <--  Stage.pm
new revision: 1.37; previous revision: 1.36
done
Attachment #314143 - Attachment description: do not create a new dir if $product-$version dir already exists → [checked in] do not create a new dir if $product-$version dir already exists
(In reply to comment #48)
> (From update of attachment 314143 [details] [diff] [review])
> r+. For kicks, could you fix the comment which immediately precedes this diff,
>    # upload public and private staging areas
> to just say private.
> 

done and done, thanks!
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Attachment #320762 - Flags: review? → review?(nrthomas)
Comment on attachment 320762 [details] [diff] [review]
[checked in] 1.8 tinder-config.pl fixed %externalStagingServer% to %ftpServer%

Checking in linux/tinder-config.pl;
/cvsroot/mozilla/tools/tinderbox-configs/firefox/linux/tinder-config.pl,v  <--  tinder-config.pl
new revision: 1.1.12.27; previous revision: 1.1.12.26
done
Checking in macosx/tinder-config.pl;
/cvsroot/mozilla/tools/tinderbox-configs/firefox/macosx/tinder-config.pl,v  <--  tinder-config.pl
new revision: 1.8.4.26; previous revision: 1.8.4.25
done
Checking in win32/tinder-config.pl;
/cvsroot/mozilla/tools/tinderbox-configs/firefox/win32/tinder-config.pl,v  <--  tinder-config.pl
new revision: 1.2.10.28; previous revision: 1.2.10.27
done
Attachment #320762 - Attachment description: 1.8 tinder-config.pl fixed %externalStagingServer% to %ftpServer% → [checked in] 1.8 tinder-config.pl fixed %externalStagingServer% to %ftpServer%
Attachment #320762 - Flags: review?(nrthomas) → review+
Component: Release Engineering: Talos → Release Engineering
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.