slaveapi shouldn't assume that a slave has a master

RESOLVED FIXED

Status

RESOLVED FIXED
5 years ago
a year ago

People

(Reporter: bhearsum, Assigned: bhearsum)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 8348842 [details] [diff] [review]
don't assume a master exists

I hit a few slaves like this while testing out bug 914764.
Attachment #8348842 - Flags: review?(jhopkins)
Comment on attachment 8348842 [details] [diff] [review]
don't assume a master exists

+    if not slave.master_url:
+        return FAILURE, "%s - No master set, can't initiate graceful shutdown."
+

If a build slave isn't connected to a master, it can't take any jobs, right?  I'm not sure bailing out is the right thing to do.
Flags: needinfo?(bhearsum)
(Assignee)

Comment 2

5 years ago
(In reply to John Hopkins (:jhopkins) from comment #1)
> Comment on attachment 8348842 [details] [diff] [review]
> don't assume a master exists
> 
> +    if not slave.master_url:
> +        return FAILURE, "%s - No master set, can't initiate graceful
> shutdown."
> +
> 
> If a build slave isn't connected to a master, it can't take any jobs, right?
> I'm not sure bailing out is the right thing to do.

If a buildslave doesn't have a master set, it's impossible to initiate a graceful shutdown. Ignoring bug 914764 for a moment, what else could we do here? I suppose we could set SUCCESS rather than FAILURE, but I can't think of anything else that could be done inside of the shutdown_buildslave action.
Flags: needinfo?(bhearsum)
Comment on attachment 8348842 [details] [diff] [review]
don't assume a master exists

r+ with the following FAILURE changed to SUCCESS, as discussed in IRC:

+    if not slave.master_url:
+        return FAILURE, "%s - No master set, can't initiate graceful shutdown."
+
Attachment #8348842 - Flags: review?(jhopkins) → review+
(Assignee)

Comment 5

5 years ago
Deployed.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Component: Tools → General
Product: Release Engineering → Release Engineering
You need to log in before you can comment on or make changes to this bug.