New issue
Advanced search Search tips

Issue 868452 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

In RemoteHost.reboot(), current_boot_id may be used before set

Reported by jrbarnette@chromium.org, Jul 27

Issue description

This file:  server/hosts/remote.py
This method:  RemoteHost.reboot()

The relevant source:
    def reboot(self, timeout=DEFAULT_REBOOT_TIMEOUT, wait=True,
               fastsync=False, reboot_cmd=None, **dargs):
        # ... portions omitted for clarity
        def reboot():
            # pylint: disable=missing-docstring
            self.record("GOOD", None, "reboot.start")
            try:
                current_boot_id = self.get_boot_id()
                # ... portions omitted for clarity
            except error.AutoservRunError:
                # If wait is set, ignore the error here, and rely on the
                # wait_for_restart() for stability, instead.
                # reboot_cmd sometimes causes an error even if reboot is
                # successfully in progress. This is difficult to be avoided,
                # because we have no much control on remote machine after
                # "reboot" starts.
                if not wait:
                    # TODO(b/37652392): Revisit no-wait case, later.
                    self.record("ABORT", None, "reboot.start",
                                "reboot command failed")
                    raise
            if wait:
                self.wait_for_restart(timeout, old_boot_id=current_boot_id,
                                      **dargs)
        # ... portions omitted for clarity

If `wait` is true, and the call to `self.get_boot_id()` fails (e.g.
because the target is now down), then `current_boot_id` will be
unset, and the call to `self.wait_for_restart` will fail.

This problem is seen from time to time during provision tasks.  See
for instance:
    http://cautotest.corp.google.com/tko/retrieve_logs.cgi?job=/results/hosts/chromeos6-row4-rack13-host22/1281474-provision/
    http://cautotest.corp.google.com/tko/retrieve_logs.cgi?job=/results/hosts/chromeos2-row6-rack3-host3/968914-provision/
    http://cautotest.corp.google.com/tko/retrieve_logs.cgi?job=/results/hosts/chromeos6-row3-rack12-host7/1157573-provision/

 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/73b35171a70c166f91d2c03a7a070caf34252c1d

commit 73b35171a70c166f91d2c03a7a070caf34252c1d
Author: Richard Barnette <jrbarnette@chromium.org>
Date: Thu Aug 02 20:53:37 2018

[autotest] Protect from unassigned variable in RemoteHost.reboot()

In certain error paths in RemoteHost.reboot(), a local variable
could wind up unassigned.  This guarantees that the variable will be
assigned, and makes sure that the underlying error gets reported.

BUG=chromium:868452
TEST=Test provision in a local autotest instance

Change-Id: Id10dd185610c9ec2515e3e77787c2190271319f1
Reviewed-on: https://chromium-review.googlesource.com/1153440
Commit-Ready: Richard Barnette <jrbarnette@chromium.org>
Tested-by: Richard Barnette <jrbarnette@chromium.org>
Reviewed-by: Yaakov Shaul <yshaul@google.com>

[modify] https://crrev.com/73b35171a70c166f91d2c03a7a070caf34252c1d/server/hosts/remote.py

Sign in to add a comment