592060 - Create custom hgpoller for shadow-central that can poll https:// pushlog

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P3)

Description

[Lukas Blakk [:lsblakk] use ?needinfo]

This bug is to track the following changes to shadow-central:

Take out the hgHost for shadow-central (which has been throwing errors): http://hg.mozilla.org/build/buildbot-configs/file/5aeb32b3f87e/mozilla/config.py#l729

Create an hg commit hook that will do a sendchange to production-master01:9009 when a new change is submitted to the private repo.

When this is done we should be able to (still) do force builds on shadow-central repo as well as see builds triggered by shadow-central checkins

Comment 1

[Daniel Veditz [:dveditz]]

Is the one catlee is working on (or at least thinking about), or is that an alternate approach tracked in a different bug?

Comment 2

[Daniel Veditz [:dveditz]]

I can't see "depends on" bug 598681 -- is that an IT request to deploy this? If so, how is it coming?

Comment 3

[Lukas Blakk [:lsblakk] use ?needinfo]

(In reply to comment #2)
> I can't see "depends on" bug 598681 -- is that an IT request to deploy this? If
> so, how is it coming?

Catlee can only get so far in staging trying to test an https custom poller, he needs an account with credentials so we can poll https://hgpvt.m.o
repositories to see if there are new changes that require building and test that we're actually getting the builds to trigger properly.

Comment 4

[Lukas Blakk [:lsblakk] use ?needinfo]

(In reply to comment #1)
> Is the one catlee is working on (or at least thinking about), or is that an
> alternate approach tracked in a different bug?

And yes, it's an IT request to get that LDAP account set up.

Comment 5

[Chris AtLee [:catlee]]

Attachment: external buildbot poller that supports checking certs for https urls

Comment 6

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

Comment on attachment 482964 [details] [diff] [review]
external buildbot poller that supports checking certs for https urls

>diff --git a/bin/hgpoller.py b/bin/hgpoller.py
>+    if branch_state['last_run'] + interval > time.time():
>+        log.debug("Skipping %s, too soon since last run", branch)
>+        return

I think this is a comparison between a smallish integer (300) and an epoch timestamp. Otherwise looks good.

Comment 7

[[e-mail] (:bhearsum)]

Comment on attachment 482964 [details] [diff] [review]
external buildbot poller that supports checking certs for https urls

>+def sendchange(master, branch, change):
>+    log.info("Sendchange %s to %s on branch %s", change['changeset'], master, branch)
>+    cmd = ['buildbot', 'sendchange',
>+            '--master', master,
>+            '--branch', branch,
>+            '--comments', change['comments'],
>+            '--revision', change['changeset'],
>+            '--user', change['author'],
>+            '--when', str(change['updated']),
>+            ]
>+    cmd.extend(change['files'])
>+    subprocess.check_call(cmd)

Might be better to run this through retry_command for better chance of success.


Just want to make sure I understand how this will be run, too. It looks like we'll be providing an initial config file with basic information, branch information, etc. The poller is then going to use this file to store state? And is it running through cron?

Comment 8

[Chris AtLee [:catlee]]

(In reply to comment #6)
> Comment on attachment 482964 [details] [diff] [review]
> external buildbot poller that supports checking certs for https urls
> 
> >diff --git a/bin/hgpoller.py b/bin/hgpoller.py
> >+    if branch_state['last_run'] + interval > time.time():
> >+        log.debug("Skipping %s, too soon since last run", branch)
> >+        return
> 
> I think this is a comparison between a smallish integer (300) and an epoch
> timestamp. Otherwise looks good.

Not sure what you mean here...That the operator precedence is wrong?

Comment 9

[Chris AtLee [:catlee]]

(In reply to comment #7)
> Comment on attachment 482964 [details] [diff] [review]
> external buildbot poller that supports checking certs for https urls
> 
> >+def sendchange(master, branch, change):
> >+    log.info("Sendchange %s to %s on branch %s", change['changeset'], master, branch)
> >+    cmd = ['buildbot', 'sendchange',
> >+            '--master', master,
> >+            '--branch', branch,
> >+            '--comments', change['comments'],
> >+            '--revision', change['changeset'],
> >+            '--user', change['author'],
> >+            '--when', str(change['updated']),
> >+            ]
> >+    cmd.extend(change['files'])
> >+    subprocess.check_call(cmd)
> 
> Might be better to run this through retry_command for better chance of success

Good idea

> Just want to make sure I understand how this will be run, too. It looks like
> we'll be providing an initial config file with basic information, branch
> information, etc. The poller is then going to use this file to store state? And
> is it running through cron?

The state is stored in an auxiliary file, specified by the 'state_file' option in the config (defaults to 'state.json').  This file gets updated by the script, and the original config file is untouched.  The state file stores the times we last polled for data, as well as the last known changeset for the repo.

Comment 10

[Chris AtLee [:catlee]]

And yes, running through cron for now.  We could set something up in buildbot to run this command too.  SubprocessPoller?

Comment 11

[[e-mail] (:bhearsum)]

(In reply to comment #10)
> And yes, running through cron for now.  We could set something up in buildbot
> to run this command too.  SubprocessPoller?

I'm not really picky, was just wondering.

Comment 12

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

(In reply to comment #6)
> >+    if branch_state['last_run'] + interval > time.time():
> I think this is a comparison between a smallish integer (300) and an epoch
> timestamp. Otherwise looks good.

Gah, I just parsed it wrong in my head. Maybe it's easier to read if written as 
  if time.time() > branch_state['last_run'] + interval
though.

Comment 13

[Chris AtLee [:catlee]]

Attachment: external buildbot poller that supports checking certs for https urls

Addresses your comments.

'retry.py' needs to be in $PATH somewhere.  I was thinking of symlinking it from ~/bin to the tools checkout the masters have?

Comment 14

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

Comment on attachment 485303 [details] [diff] [review]
external buildbot poller that supports checking certs for https urls

>+    cmd = ['retry.py', '-r', '5', '-s', '5', '-t', '1800',
>+            '--stdout-regexp', 'change sent successfully']
...
>+    subprocess.check_call(cmd)

This script looks OK for polling shadow-central, but do you think this blocking call will be OK if we end up polling multiple repositories ? I'm concerned that a 30 minute timeout on each sendchange attempt will delay changes from other branches. Perhaps it's better to fail quickly and not update the tip in the state, then try again next time the poller runs.

Comment 15

[Chris AtLee [:catlee]]

(In reply to comment #14)
> Comment on attachment 485303 [details] [diff] [review]
> external buildbot poller that supports checking certs for https urls
> 
> >+    cmd = ['retry.py', '-r', '5', '-s', '5', '-t', '1800',
> >+            '--stdout-regexp', 'change sent successfully']
> ...
> >+    subprocess.check_call(cmd)
> 
> This script looks OK for polling shadow-central, but do you think this blocking
> call will be OK if we end up polling multiple repositories ? I'm concerned that
> a 30 minute timeout on each sendchange attempt will delay changes from other
> branches. Perhaps it's better to fail quickly and not update the tip in the
> state, then try again next time the poller runs.

Sure, I can change it to what...30 seconds instead?

Comment 16

[Chris AtLee [:catlee]]

Attachment: external buildbot poller that supports checking certs for https urls

Differences from previous:
* change sendchange timeout to 30s
* fix unicode encoding problems on comments and authors
* change log levels so it's less noisy by default
* add branch configuration parameter, default_branch_only, which defaults to true.  If true, only pushes on the default branch will be used.  If false, all pushes will be used.  We could use this for the try branch.
* actually ignore off-default pushes when default_branch_only is set

Comment 17

[[e-mail] (:bhearsum)]

Comment on attachment 485756 [details] [diff] [review]
external buildbot poller that supports checking certs for https urls

Thanks for the summary of the differences, looks good.

Comment 18

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

Comment on attachment 485756 [details] [diff] [review]
external buildbot poller that supports checking certs for https urls

Looks good.

Comment 19

[Chris AtLee [:catlee]]

Comment on attachment 485756 [details] [diff] [review]
external buildbot poller that supports checking certs for https urls

changeset:   1026:140e3c0e22a6

Comment 20

[Chris AtLee [:catlee]]

Set up on pm01 in ~/poller.  The pushlog is empty currently, so haven't been able to verify.

Comment 21

[Chris AtLee [:catlee]]

Attachment: Add the hg url to the comments so buildbot thinks the file is important