708798 - buildbot-wrangler.py shouldn't fail when twistd.log is absent

Status: RESOLVED FIXED

(Release Engineering :: General, enhancement, P5)

Description

[John Ford [:jhford] CET/CEST Berlin Time]

If i try to use our fabric tools to start a master that has never been started, it fails with

[buildbot-master19.build.mtv1.mozilla.com] run: python buildbot-wrangler.py start /builds/buildbot/tests1-tegra/master
[buildbot-master19.build.mtv1.mozilla.com] out: Traceback (most recent call last):
[buildbot-master19.build.mtv1.mozilla.com] out:   File "buildbot-wrangler.py", line 193, in ?
[buildbot-master19.build.mtv1.mozilla.com] out:     w = ReconfigWatcher(twistd_log)
[buildbot-master19.build.mtv1.mozilla.com] out:   File "buildbot-wrangler.py", line 24, in __init__
[buildbot-master19.build.mtv1.mozilla.com] out:     self.fp = open(fname)
[buildbot-master19.build.mtv1.mozilla.com] out: IOError: [Errno 2] No such file or directory: '/builds/buildbot/tests1-tegra/master/twistd.log'

We should create a size 0 file before trying to open it.  Touching the file then rerunning the above makes the wrangler work

Comment 1

[Edmund Wong (:ewong)]

Attachment: buildbot-wrangler.py shouldn't fail when twistd.log is absent (v1)

But it quits when master\twistd.log is not accessible, i.e when master doesn't exist.

Comment 2

[Chris AtLee [:catlee]]

Comment on attachment 652691 [details] [diff] [review]
buildbot-wrangler.py shouldn't fail when twistd.log is absent (v1)

Review of attachment 652691 [details] [diff] [review]:
-----------------------------------------------------------------

I think the bug here is that buildbot-wrangler.py should be able to handle the file not existing gracefully and continue, rather than exiting with an error status.

One use case would be to use buildbot-wrangler.py to start the master if twistd.log doesn't exist. I think creating an empty twistd.log file and then watching that is a good avenue to explore.

Comment 3

[Edmund Wong (:ewong)]

(In reply to Chris AtLee [:catlee] from comment #2)
> Comment on attachment 652691 [details] [diff] [review]
> buildbot-wrangler.py shouldn't fail when twistd.log is absent (v1)
> 
> Review of attachment 652691 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think the bug here is that buildbot-wrangler.py should be able to handle
> the file not existing gracefully and continue, rather than exiting with an
> error status.
> 
> One use case would be to use buildbot-wrangler.py to start the master if
> twistd.log doesn't exist. I think creating an empty twistd.log file and then
> watching that is a good avenue to explore.

But the patch does allow buildbot-wrangler.py to handle a missing
twistd.log.  It exits with an error status when master/ doesn't exist. 
I can change that as I didn't want to assume that the master/ existed.

Comment 4

[Justin Wood (:Callek)]

(In reply to Chris AtLee [:catlee] from comment #2)
> I think the bug here is that buildbot-wrangler.py should be able to handle
> the file not existing gracefully and continue, rather than exiting with an
> error status.

(In reply to Edmund Wong (:ewong) from comment #3)
> But the patch does allow buildbot-wrangler.py to handle a missing
> twistd.log.  It exits with an error status when master/ doesn't exist. 
> I can change that as I didn't want to assume that the master/ existed.

I'm going to try to clear up confusion here...

the Watcher() class which ewong patched, does indeed exit when twistd.log is missing, *however* we should instead not exit at this point, since we could have some cases that a missing twistd.log file is *ok*, and all the command classes override Watcher()

Example for an OK condition would be buildbot-wrangler "start" (http://mxr.mozilla.org/build/source/tools/buildfarm/maintenance/buildbot-wrangler.py#189) which inits ReconfigWatcher(), but on a missing twistd.log would fail out.

The annoying/confusing part here (ignoring twistd.log) might be trying to start reading it when it doesn't exist...

My *suggestion* add a 0-byte write to twistd.log when trying to use one of the "Missing Ok" commands *IFF* there is no twistd.pid file (such as you don't insert yourself during the twistd log rotation miliseconds)

And continue to exit in a missing twistd.log for the not-ok types.

Chris, does that sound like the best path forward? Ewong does that path sound like something you feel comfortable doing, and understand?

Comment 5

[Edmund Wong (:ewong)]

Catlee,  I think my confusion stems from the fact that the intention of my
code is to touch the twistd.log file (via |open(fname, 'w').close()|) and
if the "master/" folder doesn't exist (meaning anything afterwards is
going to be a moot point since the buildbot isn't set up properly), that
is when it exits.  The code, as I see it, does the following:

    if master/twistd.log exists
      os.utime it.
    otherwise try:
       touch master/twistd.log
    if this fails:  (i.e. when master/ doesn't exist)
       exits

That was my intention with the code.  But my python isn't that strong so
I probably made a mistake somewhere (not clear where it is).  Clarifications
appreciated.

Comment 6

[Edmund Wong (:ewong)]

Attachment: buildbot-wrangler.py shouldn't fail when twistd.log is absent. (v2)

Comment 7

[Armen [:armenzg]]

catlee, what do you think the deployment/staging plan for this should be?

This seems that it could only cause trouble when trying to do reconfigs and seems easy to backout.

I can try landing this for my next reconfig and see how it goes?
What do you think?

Comment 8

[Edmund Wong (:ewong)]

Was told this was pushed to production, so closing.