Reland Wait cloudtail termination in goma module

Reland Wait cloudtail termination in goma module

This is follow up CL for https://codereview.chromium.org/2375843005/

Reland of https://codereview.chromium.org/2386073002/
Fix:
  catch and ignore exception OSError of ECHILD from os.waitpid in windows.

BUG=

Committed: https://chromium.googlesource.com/chromium/tools/build/+/6cd1bbcae072d0125a8aa4b509fb313149773001

[tikuta, 2016-10-11 06:28:42]

The CQ bit was checked by tikuta@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-11 06:28:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tikuta, 2016-10-11 06:30:05]

tikuta@chromium.org changed reviewers:
+ shinyak@chromium.org, ukai@chromium.org, vadimsh@chromium.org,
yyanagisawa@chromium.org

[commit-bot: I haz the power, 2016-10-11 06:32:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-11 06:32:28]

Dry run: This issue passed the CQ dry run.

[shinyak, 2016-10-11 06:47:02]

https://codereview.chromium.org/2404213002/diff/1/scripts/slave/recipe_module...
File scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py (right):

https://codereview.chromium.org/2404213002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py:28: def
is_running_posix(pid):
Two blank lines between top-level definitions

https://google.github.io/styleguide/pyguide.html?showone=Blank_Lines#Blank_Lines

https://codereview.chromium.org/2404213002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py:47: os.kill(pid,
signal.SIGKILL)
Oh, is_running_posix(pid) sends SIGKILL? I feel it's strange...
I believe it's parent function job.

https://codereview.chromium.org/2404213002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py:86: def main():
Two blank lines between top-level definitions

https://codereview.chromium.org/2404213002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py:113: # within 5
seconds when it recieves SIGINT.
5 seconds. Right?

[tikuta, 2016-10-11 08:07:03]

The CQ bit was checked by tikuta@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-11 08:07:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tikuta, 2016-10-11 08:09:01]

https://codereview.chromium.org/2404213002/diff/1/scripts/slave/recipe_module...
File scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py (right):

https://codereview.chromium.org/2404213002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py:28: def
is_running_posix(pid):
On 2016/10/11 06:47:02, shinyak wrote:
> Two blank lines between top-level definitions
> 
>
https://google.github.io/styleguide/pyguide.html?showone=Blank_Lines#Blank_Lines

Done.

https://codereview.chromium.org/2404213002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py:47: os.kill(pid,
signal.SIGKILL)
On 2016/10/11 06:47:02, shinyak wrote:
> Oh, is_running_posix(pid) sends SIGKILL? I feel it's strange...
> I believe it's parent function job.

Done.

https://codereview.chromium.org/2404213002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py:86: def main():
On 2016/10/11 06:47:02, shinyak wrote:
> Two blank lines between top-level definitions

Done.

https://codereview.chromium.org/2404213002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py:113: # within 5
seconds when it recieves SIGINT.
On 2016/10/11 06:47:02, shinyak wrote:
> 5 seconds. Right?

From
https://codereview.chromium.org/2375843005/#msg28

[commit-bot: I haz the power, 2016-10-11 08:10:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-11 08:10:40]

Dry run: This issue passed the CQ dry run.

[Yoshisato Yanagisawa, 2016-10-11 08:11:18]

lgtm

[shinyak, 2016-10-11 08:31:06]

lgtm

[ukai, 2016-10-12 01:48:28]

lgtm

https://codereview.chromium.org/2404213002/diff/20001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py (right):

https://codereview.chromium.org/2404213002/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py:59: OSError:
is_running_posix, os.waitpid and os.kill may throw OSError.
caller should SIGKILL, or this will send SIGKILL before raising exception?

https://codereview.chromium.org/2404213002/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py:65: os.kill(pid,
signal.SIGKILL)
which case it will try send SIGKILL?

https://codereview.chromium.org/2404213002/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py:73: print('ignore
errno.ECHILD %s' % e)
process of pid died before os.waitpid?

[tikuta, 2016-10-12 06:49:30]

The CQ bit was checked by tikuta@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-12 06:49:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tikuta, 2016-10-12 06:51:51]

https://codereview.chromium.org/2404213002/diff/20001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py (right):

https://codereview.chromium.org/2404213002/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py:59: OSError:
is_running_posix, os.waitpid and os.kill may throw OSError.
On 2016/10/12 01:48:28, ukai wrote:
> caller should SIGKILL, or this will send SIGKILL before raising exception?

I stopped to send SIGKILL in this function.
Caller should SIGKILL.

https://codereview.chromium.org/2404213002/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py:65: os.kill(pid,
signal.SIGKILL)
On 2016/10/12 01:48:28, ukai wrote:
> which case it will try send SIGKILL?

Done.

https://codereview.chromium.org/2404213002/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py:73: print('ignore
errno.ECHILD %s' % e)
On 2016/10/12 01:48:28, ukai wrote:
> process of pid died before os.waitpid?

Done.

[ukai, 2016-10-12 07:05:21]

lgtm

https://codereview.chromium.org/2404213002/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py (right):

https://codereview.chromium.org/2404213002/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py:76: print('ignore
errno.ECHILD %s, '
need to print this?

[commit-bot: I haz the power, 2016-10-12 07:10:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-12 07:10:00]

Dry run: Try jobs failed on following builders:
  Recipe Roll Downstream Tester on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/31d0856d9a46fc10)

[tikuta, 2016-10-12 07:12:05]

The CQ bit was checked by tikuta@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-12 07:12:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tikuta, 2016-10-12 07:12:32]

https://codereview.chromium.org/2404213002/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py (right):

https://codereview.chromium.org/2404213002/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py:76: print('ignore
errno.ECHILD %s, '
On 2016/10/12 07:05:21, ukai wrote:
> need to print this?

Done.

[commit-bot: I haz the power, 2016-10-12 07:35:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-12 07:35:47]

Dry run: Try jobs failed on following builders:
  Recipe Roll Downstream Tester on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/31d08da5b2ecb610)

[shinyak, 2016-10-12 07:43:35]

lgtm

https://codereview.chromium.org/2404213002/diff/60001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py (right):

https://codereview.chromium.org/2404213002/diff/60001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py:82: break
return here?

[tikuta, 2016-10-12 07:52:44]

The CQ bit was checked by tikuta@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-12 07:52:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tikuta, 2016-10-12 07:53:17]

https://codereview.chromium.org/2404213002/diff/60001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py (right):

https://codereview.chromium.org/2404213002/diff/60001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py:82: break
On 2016/10/12 07:43:35, shinyak wrote:
> return here?

Done.

[commit-bot: I haz the power, 2016-10-12 08:14:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-12 08:14:02]

Dry run: Try jobs failed on following builders:
  Recipe Roll Downstream Tester on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/31d0c0ee15280310)

[tikuta, 2016-10-13 08:10:40]

The CQ bit was checked by tikuta@chromium.org

[tikuta, 2016-10-13 08:10:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
yyanagisawa@chromium.org, ukai@chromium.org, shinyak@chromium.org
Link to the patchset: https://codereview.chromium.org/2404213002/#ps80001
(title: "break -> return")

[commit-bot: I haz the power, 2016-10-13 08:10:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-13 08:14:47]

Description was changed from

==========
Reland Wait cloudtail termination in goma module

This is follow up CL for https://codereview.chromium.org/2375843005/

Reland of https://codereview.chromium.org/2386073002/
Fix:
  catch and ignore exception OSError of ECHILD from os.waitpid in windows.

BUG=
==========

to

==========
Reland Wait cloudtail termination in goma module

This is follow up CL for https://codereview.chromium.org/2375843005/

Reland of https://codereview.chromium.org/2386073002/
Fix:
  catch and ignore exception OSError of ECHILD from os.waitpid in windows.

BUG=

Committed:
https://chromium.googlesource.com/chromium/tools/build/+/6cd1bbcae072d0125a8a...
==========

[commit-bot: I haz the power, 2016-10-13 08:14:48]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/tools/build/+/6cd1bbcae072d0125a8a...

[tikuta, 2016-10-13 14:17:25]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/2416903002/ by tikuta@chromium.org.

The reason for reverting is:
https://bugs.chromium.org/p/chromium/issues/detail?id=655618


BUG=655618.