520903 - need to be able to turn off nightly builds with a flag in config.py

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Lukas Blakk [:lsblakk] use ?needinfo]

This is mostly for tryserver becoming another branch in config.py - we don't need nightly tryserver builds so generateBranchObjects in buildbotcustom/misc.py should allow for that to be turned off via config.py

Might end up being useful for other branches too.

Comment 1

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Change misc.py to test for enable_nightly in config

Comment 2

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Add ['enable_nightly'] to config.py for each branch

Comment 3

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Change misc.py to test for enable_nightly in config

Comment 4

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 417776 [details] [diff] [review]
Add ['enable_nightly'] to config.py for each branch

Tested on staging and was able to remove nightly builder for Places branch

Comment 5

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Add ['enable_nightly'] to config.py for each branch (staging and production)

Comment 6

[[e-mail] (:bhearsum)]

Comment on attachment 417784 [details] [diff] [review]
Change misc.py to test for enable_nightly in config


>-        nightly_builder = '%s nightly' % pf['base_name']
>+            triggeredSchedulers=None
>+            if platform in ('linux', 'macosx', 'win32') and \
>+                nightly_builder in l10nNightlyBuilders:
>+                triggeredSchedulers=[l10nNightlyBuilders[nightly_builder]['l10n_builder']]

spacing nit: to aid readability, don't intend the continuation of the if condition the same amount as the 'triggeredSchedulers' part. We typically use 2 spaces when continuing conditionals on the next line.

You should also make sure l10n nightlies are turned off when en-US nightly ones are. The l10n nightly builders will never be used without the en-US nightly.

Comment 7

[[e-mail] (:bhearsum)]

Comment on attachment 417789 [details] [diff] [review]
Add ['enable_nightly'] to config.py for each branch (staging and production)

Turning off nightly builds for some branches should be a separate bug IMHO - and needs some wider communication first. Please leave it on for all branches for the purposes of this bug.

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Add ['enable_nightly'] to config.py for each branch (take 2)

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Change misc.py to test for enable_nightly in config (take 2)

Comment 10

[[e-mail] (:bhearsum)]

Comment on attachment 417970 [details] [diff] [review]
Change misc.py to test for enable_nightly in config (take 2)

You need to stop creating the 'mozilla2_l10n_nightly_factory' if nightlies are off, too.

Comment 11

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Change misc.py to test for enable_nightly in config (take 3)

now with more indents!

Comment 12

[[e-mail] (:bhearsum)]

Comment on attachment 418031 [details] [diff] [review]
Change misc.py to test for enable_nightly in config (take 3)

Now you've gone too far - we still want the mozilla2_l10n_dep_builder if nightlies are off.

Comment 13

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Change misc.py to test for enable_nightly in config (take 4)

Comment 14

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Add ['enable_nightly'] to config.py for each branch (take 3)

Found small indent error when testing.

Comment 15

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Change misc.py to test for enable_nightly in config (take 5)

tested in staging, turns off the nightly l10n as well as nightly builds while leaving the l10n dep builds intact.

Comment 16

[Chris Cooper [:coop] (he/him)]

Comment on attachment 418941 [details] [diff] [review]
Change misc.py to test for enable_nightly in config (take 5)

>-                mozilla2_l10n_dep_builder = {
>-                    'name': l10nNightlyBuilders[nightly_builder]['l10n_builder'] + " build",
>+            mozilla2_l10n_dep_builder = {
>+                'name': '%s l10n build' % pf['base_name'],

You changed how this builder gets named, but I don't think it's necessarily equivalent.

Everything else looks good though, so r+ with that fix.

Comment 17

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Change misc.py to test for enable_nightly in config (take 6)

So coop gave an r+ on the last patch with a nit about naming the builders but I've added some new lines to this one in order to make it much clearer where the l10n nightlies vs. dep builds are being created.  I've tested this in staging with l10n turned on 1.9.1 without nightlies, l10n and nightlies on 1.9.2, and with nightlies turned off entirely on electrolysis.

All the waterfall columns appear correctly, with the appropriate names - though now they explicitly say "dep" or "nightly" which I find more useful than "l10n" and "l10n build".

I triggered a dep build on 1.9.1 successfully here: http://staging-master.build.mozilla.org:8011/builders/Firefox%20mozilla-1.9.1%20linux%20l10n%20dep/builds/3

Comment 18

[Nick Thomas [:nthomas] (UTC+12)]

(In reply to comment #17)
> All the waterfall columns appear correctly, with the appropriate names - though
> now they explicitly say "dep" or "nightly" which I find more useful than "l10n"
> and "l10n build".

Hurrah!

Comment 19

[[e-mail] (:bhearsum)]

(In reply to comment #17)
> All the waterfall columns appear correctly, with the appropriate names - though
> now they explicitly say "dep" or "nightly" which I find more useful than "l10n"
> and "l10n build".

As much as I like this, I don't think we should take it as a ride along. If we have to back out the rest of this patch for some reason we'll end up changing the l10n builder names. Please put that in a separate bug/patch.

With that removed I think this patch is the same as last time, but can you repost without it, please?

Comment 20

[[e-mail] (:bhearsum)]

(In reply to comment #19)
> (In reply to comment #17)
> > All the waterfall columns appear correctly, with the appropriate names - though
> > now they explicitly say "dep" or "nightly" which I find more useful than "l10n"
> > and "l10n build".
> 
> As much as I like this, I don't think we should take it as a ride along. If we
> have to back out the rest of this patch for some reason we'll end up changing
> the l10n builder names. Please put that in a separate bug/patch.

Yeargh, I thought this was the try server bug. Given what it _really_ is, this seems fine to do here.

Comment 21

[[e-mail] (:bhearsum)]

Comment on attachment 420422 [details] [diff] [review]
Change misc.py to test for enable_nightly in config (take 6)


>-        for b in l10nNightlyBuilders:
>-            l10n_builders.append(l10nNightlyBuilders[b]['l10n_builder'])
>-            l10n_builders.append(l10nNightlyBuilders[b]['l10n_builder'] + " build")
>+        for b in l10nBuilders:
>+            l10n_builders.append(l10nBuilders[b]['l10n_builder'])
>+            if config['enable_nightly']:
>+                l10n_builders.append(l10nNightlyBuilders['%s nightly' % b]['l10n_builder'])
>         l10n_binaryURL = config['enUS_binaryURL']
>         if l10n_binaryURL.endswith('/'):
>             l10n_binaryURL = l10n_binaryURL[:-1]
>         l10n_binaryURL += "-l10n"
> 
>         # This notifies all l10n related build objects to Mozilla-l10n
>         branchObjects['status'].append(TinderboxMailNotifier(
>             fromaddr="[e-mail]",
>@@ -360,17 +373,17 @@ def generateBranchObjects(config, name):
>             name=builder,
>             branch=config['repo_path'],
>             # bug 482123 - keep the minute to avoid problems with DST changes
>             hour=config['start_hour'], minute=config['start_minute'],
>             builderNames=[builder]
>         )
>         branchObjects['schedulers'].append(nightly_scheduler)
> 
>-        if config['enable_l10n'] and builder in l10nNightlyBuilders:
>+        if config['enable_l10n'] and config['enable_nightly'] and builder in l10nNightlyBuilders:

You don't need to check config['enable_nightly'] here - l10nNightlyBuilders is only populated when enable_nightly is on.


>-        if config['enable_l10n']:
>-            if platform in config['l10n_platforms']:
>-                # TODO Linux and mac are not working with mozconfig at this point
>-                # and this will disable it for now. We will fix this in bug 518359.
>-                if platform is 'wince':
>-                    env = pf['env']
>-                    mozconfig = pf['mozconfig']
>-                else:
>-                    env = {}
>-                    mozconfig = None
>+            if config['enable_l10n']:
>+                if platform in config['l10n_platforms']:
>+                    # TODO Linux and mac are not working with mozconfig at this point
>+                    # and this will disable it for now. We will fix this in bug 518359.
>+                    if platform is 'wince':
>+                        env = pf['env']
>+                        mozconfig = pf['mozconfig']
>+                    else:
>+                        env = {}
>+                        mozconfig = None
> 
>-                mozilla2_l10n_nightly_factory = NightlyRepackFactory(
>-                    env=env,
>-                    platform=platform,
>-                    hgHost=config['hghost'],
>-                    tree=config['l10n_tree'],
>-                    project=config['product_name'],
>-                    appName=config['app_name'],
>-                    enUSBinaryURL=config['enUS_binaryURL'],
>-                    nightly=True,
>-                    configRepoPath=config['config_repo_path'],
>-                    configSubDir=config['config_subdir'],
>-                    mozconfig=mozconfig,
>-                    l10nNightlyUpdate=config['l10nNightlyUpdate'],
>-                    l10nDatedDirs=config['l10nDatedDirs'],
>-                    ausBaseUploadDir=config['aus2_base_upload_dir'],
>-                    updatePlatform=pf['update_platform'],
>-                    downloadBaseURL=config['download_base_url'],
>-                    ausUser=config['aus2_user'],
>-                    ausHost=config['aus2_host'],
>-                    stageServer=config['stage_server'],
>-                    stageUsername=config['stage_username'],
>-                    stageSshKey=config['stage_ssh_key'],
>-                    repoPath=config['repo_path'],
>-                    l10nRepoPath=config['l10n_repo_path'],
>-                    buildToolsRepoPath=config['build_tools_repo_path'],
>-                    compareLocalesRepoPath=config['compare_locales_repo_path'],
>-                    compareLocalesTag=config['compare_locales_tag'],
>-                    buildSpace=2,
>-                    clobberURL=config['base_clobber_url'],
>-                    clobberTime=clobberTime,
>-                )
>-                mozilla2_l10n_nightly_builder = {
>-                    'name': l10nNightlyBuilders[nightly_builder]['l10n_builder'],
>-                    'slavenames': config['l10n_slaves'][platform],
>-                    'builddir': '%s-%s-l10n-nightly' % (name, platform),
>-                    'factory': mozilla2_l10n_nightly_factory,
>-                    'category': name,
>-                }
>-                branchObjects['builders'].append(mozilla2_l10n_nightly_builder)
>-                mozilla2_l10n_dep_factory = NightlyRepackFactory(
>-                    hgHost=config['hghost'],
>-                    tree=config['l10n_tree'],
>-                    project=config['product_name'],
>-                    appName=config['app_name'],
>-                    enUSBinaryURL=config['enUS_binaryURL'],
>-                    nightly=False,
>-                    l10nDatedDirs=config['l10nDatedDirs'],
>-                    stageServer=config['stage_server'],
>-                    stageUsername=config['stage_username'],
>-                    stageSshKey=config['stage_ssh_key'],
>-                    repoPath=config['repo_path'],
>-                    l10nRepoPath=config['l10n_repo_path'],
>-                    buildToolsRepoPath=config['build_tools_repo_path'],
>-                    compareLocalesRepoPath=config['compare_locales_repo_path'],
>-                    compareLocalesTag=config['compare_locales_tag'],
>-                    buildSpace=2,
>-                    clobberURL=config['base_clobber_url'],
>-                    clobberTime=clobberTime,
>-                )
>-                mozilla2_l10n_dep_builder = {
>-                    'name': l10nNightlyBuilders[nightly_builder]['l10n_builder'] + " build",
>-                    'slavenames': config['l10n_slaves'][platform],
>-                    'builddir': '%s-%s-l10n-dep' % (name, platform),
>-                    'factory': mozilla2_l10n_dep_factory,
>-                    'category': name,
>-                }
>-                branchObjects['builders'].append(mozilla2_l10n_dep_builder)
>+                    mozilla2_l10n_nightly_factory = NightlyRepackFactory(
>+                        env=env,
>+                        platform=platform,
>+                        hgHost=config['hghost'],
>+                        tree=config['l10n_tree'],
>+                        project=config['product_name'],
>+                        appName=config['app_name'],
>+                        enUSBinaryURL=config['enUS_binaryURL'],
>+                        nightly=True,
>+                        configRepoPath=config['config_repo_path'],
>+                        configSubDir=config['config_subdir'],
>+                        mozconfig=mozconfig,
>+                        l10nNightlyUpdate=config['l10nNightlyUpdate'],
>+                        l10nDatedDirs=config['l10nDatedDirs'],
>+                        ausBaseUploadDir=config['aus2_base_upload_dir'],
>+                        updatePlatform=pf['update_platform'],
>+                        downloadBaseURL=config['download_base_url'],
>+                        ausUser=config['aus2_user'],
>+                        ausHost=config['aus2_host'],
>+                        stageServer=config['stage_server'],
>+                        stageUsername=config['stage_username'],
>+                        stageSshKey=config['stage_ssh_key'],
>+                        repoPath=config['repo_path'],
>+                        l10nRepoPath=config['l10n_repo_path'],
>+                        buildToolsRepoPath=config['build_tools_repo_path'],
>+                        compareLocalesRepoPath=config['compare_locales_repo_path'],
>+                        compareLocalesTag=config['compare_locales_tag'],
>+                        buildSpace=2,
>+                        clobberURL=config['base_clobber_url'],
>+                        clobberTime=clobberTime,
>+                    )
>+                    mozilla2_l10n_nightly_builder = {
>+                        'name': l10nNightlyBuilders[nightly_builder]['l10n_builder'],
>+                        'slavenames': config['l10n_slaves'][platform],
>+                        'builddir': '%s-%s-l10n-nightly' % (name, platform),
>+                        'factory': mozilla2_l10n_nightly_factory,
>+                        'category': name,
>+                    }
>+                    branchObjects['builders'].append(mozilla2_l10n_nightly_builder)
>+        # We still want l10n_dep builds if nightlies are off
>+        if config['enable_l10n'] and platform in ('linux', 'macosx', 'win32'):

I think you want to check platform against config['l10n_platforms'] rather than this hardcoded list.



r=me with those changes.

Comment 22

[[e-mail] (:bhearsum)]

Oh, and don't forget that you need to update l10nbuilds{1,2}.ini to match the new builder names. Dep builds will break (but the configs will still pass) if you don't.

Comment 23

[Armen [:armenzg]]

Comment on attachment 420422 [details] [diff] [review]
Change misc.py to test for enable_nightly in config (take 6)

Ben caught everything that I saw so I won't repeat it.

Really good job at this splitting and I am really happy to take the naming changes.

When we land this what is the master going to do with the logs of the non-existent builders? If we leave them there would they be deleted at some point? Would it confuse the master?

This patch should not go in without the l10nbuilds{1,2}.ini changes. My other concern is if SchedulerL10n it gets refreshed about the builder names after a reconfig or if it needs a restart of the master.

Axel and Coop do you know the answer to this?

Comment 24

[Axel Hecht [:Pike]]

I suspect that the SchedulerL10n would pick that up on reconfig, but that's not based on practical experience, just theory.

Please head over to the l10n tinderboxens and switch scraping on on the new columns if you change the builder names.

Comment 25

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Change misc.py to test for enable_nightly in config (take 7)

with changes from comment 21 addressed.

Comment 26

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: l10nbuilds.ini patch for new builder names

Comment 27

[Lukas Blakk [:lsblakk] use ?needinfo]

http://hg.mozilla.org/build/buildbot-configs/rev/dad1f57d9018
http://hg.mozilla.org/build/buildbotcustom/rev/42eb39ce7871