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

RESOLVED FIXED

Status

Release Engineering
General
P5
enhancement
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: jhford, Assigned: ewong)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [simple])

Attachments

(1 attachment, 1 obsolete attachment)

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

Updated

7 years ago
Whiteboard: [simple]
(Assignee)

Comment 1

6 years ago
Created attachment 652691 [details] [diff] [review]
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.
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Attachment #652691 - Flags: review?(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.
Attachment #652691 - Flags: review?(catlee) → review-
(Assignee)

Comment 3

6 years ago
(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.
(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?
(Assignee)

Comment 5

6 years ago
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.
(Assignee)

Comment 6

6 years ago
Created attachment 658469 [details] [diff] [review]
buildbot-wrangler.py shouldn't fail when twistd.log is absent. (v2)
Attachment #652691 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Attachment #658469 - Flags: review?(catlee)

Updated

6 years ago
Attachment #658469 - Flags: review?(catlee) → review+

Comment 7

6 years ago
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?
(Assignee)

Updated

6 years ago
Attachment #658469 - Flags: checked-in+
(Assignee)

Comment 8

6 years ago
Was told this was pushed to production, so closing.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.