New issue
Advanced search Search tips

Issue 835064 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 18
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Feature

Blocking:
issue 835065



Sign in to add a comment

skylab_swarming_worker: Support provision

Project Member Reported by pprabhu@chromium.org, Apr 20 2018

Issue description

Scope of this bug:
 - Adding a flag to skylab_swarming_worker to specify the build to install, construct the right lucifer flags.
 - Update autoserv's --test-name flag to obtain the test from the provided build (via devserver)

This bug does not include: Actually persisting the provisioned label on the skylab_bot in any way / reporting the provisioned label back to swarming.
 
Labels: -Pri-3 Pri-1
Blocking: 835065
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/infra/lucifer/+/3867142b4f7d1492ec4cc5794d8eed1f857f695a

commit 3867142b4f7d1492ec4cc5794d8eed1f857f695a
Author: Allen Li <ayatane@google.com>
Date: Fri Apr 27 00:54:35 2018

Remove unnecessary goroutines and defers in runLucifer

The goroutine is pointless, since all the main thread does is wait for
the single goroutine to finish.

The defer for Wait() is subtly wrong.

Golang does not have "exceptions", but it does have panic.  panic can
be caught but is generally intended to be fatal.

The problem here is that if there is a panic before the Wait, the
subprocess is going to be a zombie for the duration of the process
lifetime.  If the panic is not caught, that lifetime is very short.
If the panic is caught, then we will leak one zombie process.

This can be prevented by deferring the Wait, except that the panic
will have interrupted us reading from the subprocess stdout pipe.  We
will wait for the subprocess to exit, and the subprocess will wait for
something to finish reading from its stdout pipe.  Deadlock.

Closing our end of the pipe should cause the subprocess to
exit (probably crash).

BUG= chromium:835064 
TEST=None

Change-Id: Ib30fae4b0c5341a30d67a0342f874ae26e52db24
Reviewed-on: https://chromium-review.googlesource.com/1022498
Commit-Ready: Allen Li <ayatane@chromium.org>
Tested-by: Allen Li <ayatane@chromium.org>
Reviewed-by: Craig Bergstrom <craigb@chromium.org>

[modify] https://crrev.com/3867142b4f7d1492ec4cc5794d8eed1f857f695a/src/lucifer/cmd/skylab_swarming_worker/main.go

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/infra/lucifer/+/e58975fe99a5733a02bde2aaa5a74851b5e78cc5

commit e58975fe99a5733a02bde2aaa5a74851b5e78cc5
Author: Allen Li <ayatane@google.com>
Date: Fri Apr 27 00:54:36 2018

Use constants for various string values

BUG= chromium:835064 
TEST=None

Change-Id: I7a87b4a56f066bbdcf2553dd507620ea4ba31389
Reviewed-on: https://chromium-review.googlesource.com/1022503
Commit-Ready: Allen Li <ayatane@chromium.org>
Tested-by: Allen Li <ayatane@chromium.org>
Reviewed-by: Craig Bergstrom <craigb@chromium.org>

[modify] https://crrev.com/e58975fe99a5733a02bde2aaa5a74851b5e78cc5/src/lucifer/cmd/skylab_swarming_worker/main.go

Labels: -Type-Bug Type-Feature
Status: Fixed (was: Assigned)

Sign in to add a comment