New issue
Advanced search Search tips

Issue 748153 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

activate_date init script is not working as intended

Project Member Reported by shapiroc@chromium.org, Jul 24 2017

Issue description

There is a dependency on tlsdated, which isn't declared and the first usage causes a silent failure.

The sleep 200 call triggers a timeout since it's 200 seconds and not 200 ms.

For pyro and sentry, the fuddata variable is never set, so fud fails to the battery anyway.
 
As a note, the glimmer and ultima customizations also need to be fixed since they have copied the core script ... or work out a better shared architecture.
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/915330751c3e37fd721d19d14b37630b537b78e2

commit 915330751c3e37fd721d19d14b37630b537b78e2
Author: C Shapiro <shapiroc@chromium.org>
Date: Thu Jul 27 05:42:00 2017

Delete pyro/sentry specific activate_date scripts

The only delta with the pyro/sentry impl is that is attempted to set the First
Use Date (FUD) on the battery; however, the fuddata was empty in the
script anyway, which caused ectool to error out, but the script
passively ignored the error anyway.

So the net of this customization was nothing.  Additionally though, this
script has multiple other bugs that were fixed in the base impl of the
script.

This breaks the delete into 2 steps so that there are no file conflicts with the
incremental paladin builder.  It first puts hardblockers inplace to the old virtual
packages so that they are unmerged and then adds a new dep to the base impl.

On the 2nd step, these virtual impls will be deleted altogether.

BUG= chromium:748153 
TEST=locally on sentry and using the new functional test
platform_ActivateDate

Change-Id: Ia491126753a04c886d353b5f16b61aefb78661ee
Reviewed-on: https://chromium-review.googlesource.com/587400
Commit-Ready: C Shapiro <shapiroc@google.com>
Tested-by: C Shapiro <shapiroc@google.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Jason Clinton <jclinton@chromium.org>

[delete] https://crrev.com/597d38b799568534691ac71052ff913601f9fd1c/overlay-pyro/chromeos-base/chromeos-activate-date-pyro/chromeos-activate-date-pyro-0.0.1.ebuild
[rename] https://crrev.com/915330751c3e37fd721d19d14b37630b537b78e2/overlay-sentry/virtual/chromeos-activate-date/chromeos-activate-date-2-r2.ebuild
[delete] https://crrev.com/597d38b799568534691ac71052ff913601f9fd1c/overlay-pyro/chromeos-base/chromeos-activate-date-pyro/files/activate_date
[modify] https://crrev.com/915330751c3e37fd721d19d14b37630b537b78e2/overlay-sentry/virtual/chromeos-activate-date/chromeos-activate-date-2.ebuild
[delete] https://crrev.com/597d38b799568534691ac71052ff913601f9fd1c/overlay-sentry/chromeos-base/chromeos-activate-date-sentry/files/activate_date
[delete] https://crrev.com/597d38b799568534691ac71052ff913601f9fd1c/overlay-sentry/chromeos-base/chromeos-activate-date-sentry/chromeos-activate-date-sentry-0.0.1-r2.ebuild
[modify] https://crrev.com/915330751c3e37fd721d19d14b37630b537b78e2/overlay-pyro/virtual/chromeos-activate-date/chromeos-activate-date-2.ebuild
[delete] https://crrev.com/597d38b799568534691ac71052ff913601f9fd1c/overlay-sentry/chromeos-base/chromeos-activate-date-sentry/files/activate_date.conf
[rename] https://crrev.com/915330751c3e37fd721d19d14b37630b537b78e2/overlay-pyro/virtual/chromeos-activate-date/chromeos-activate-date-2-r2.ebuild
[delete] https://crrev.com/597d38b799568534691ac71052ff913601f9fd1c/overlay-sentry/chromeos-base/chromeos-activate-date-sentry/chromeos-activate-date-sentry-0.0.1.ebuild
[delete] https://crrev.com/597d38b799568534691ac71052ff913601f9fd1c/overlay-pyro/chromeos-base/chromeos-activate-date-pyro/chromeos-activate-date-pyro-0.0.1-r1.ebuild
[delete] https://crrev.com/597d38b799568534691ac71052ff913601f9fd1c/overlay-pyro/chromeos-base/chromeos-activate-date-pyro/files/activate_date.conf

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 28 2017

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/7ce5f9c6e0b069285a2ccedb4176c9b47daea3ba

commit 7ce5f9c6e0b069285a2ccedb4176c9b47daea3ba
Author: C Shapiro <shapiroc@chromium.org>
Date: Tue Aug 01 17:15:43 2017

Fix activate date with correct deps and exit cond

This script had multiple issues that would cause silent
failures.  However, since there are no tests for this setup and it only
ever needs to succeed once, the issues were never caught.

The first condition was a dependency to tlsdated, which if the service
hadn't started yet, when the dbus message was sent, it would fail and
cause an early termination of the script.

The second was if there was no value set for ActivateDate in vpd, it
would return non-zero, which would cause early termination from the
script.

Functional tests for this are being added in:
https://chromium-review.googlesource.com/c/583939/

BUG= chromium:748153 
TEST=tested locally on pryo

Change-Id: I5126d6b084490d62d9ddaecb53d1c2f0879ca5a7
Reviewed-on: https://chromium-review.googlesource.com/583772
Commit-Ready: C Shapiro <shapiroc@google.com>
Tested-by: C Shapiro <shapiroc@google.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Jason Clinton <jclinton@chromium.org>

[modify] https://crrev.com/7ce5f9c6e0b069285a2ccedb4176c9b47daea3ba/chromeos-base/chromeos-activate-date/files/activate_date.conf
[modify] https://crrev.com/7ce5f9c6e0b069285a2ccedb4176c9b47daea3ba/chromeos-base/chromeos-activate-date/files/activate_date.sh
[modify] https://crrev.com/7ce5f9c6e0b069285a2ccedb4176c9b47daea3ba/chromeos-base/chromeos-activate-date/files/activate_date.service
[rename] https://crrev.com/7ce5f9c6e0b069285a2ccedb4176c9b47daea3ba/chromeos-base/chromeos-activate-date/chromeos-activate-date-0.0.1-r8.ebuild

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 1 2017

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

commit 918ee2da26ebea31951366dca2513315ef355a74
Author: C Shapiro <shapiroc@chromium.org>
Date: Tue Aug 01 17:15:43 2017

Functional test for activate_date init script

The activate date script had a lot of synchronization issues and
failures and this test adds coverage to verify it works every time on
the first attempt.

BUG= chromium:748153 
TEST=test_that

Depends-On: I5126d6b084490d62d9ddaecb53d1c2f0879ca5a7
Change-Id: Ic89d6be7694b9407c427dc470373edc9379864ec
Reviewed-on: https://chromium-review.googlesource.com/583939
Commit-Ready: C Shapiro <shapiroc@google.com>
Tested-by: C Shapiro <shapiroc@google.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Jason Clinton <jclinton@chromium.org>

[add] https://crrev.com/918ee2da26ebea31951366dca2513315ef355a74/server/site_tests/platform_ActivateDate/platform_ActivateDate.py
[add] https://crrev.com/918ee2da26ebea31951366dca2513315ef355a74/server/site_tests/platform_ActivateDate/control

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/6a99607d5d4b02de87a393f20918be6600fff0ac

commit 6a99607d5d4b02de87a393f20918be6600fff0ac
Author: C Shapiro <shapiroc@chromium.org>
Date: Thu Aug 03 17:11:42 2017

Adding test coverage for ActivateDate

The activate_date init script had a lot of issues with no test coverage.
This adds basic functional test coverage to ensure there are no
regressions with the recent fixes to the script.

BUG= chromium:748153 
TEST=tested via test_that

Depends-On: Ic89d6be7694b9407c427dc470373edc9379864ec
Change-Id: I5e23652f6f04f1b361597c1ac58c4139e5df2f0e
Reviewed-on: https://chromium-review.googlesource.com/583992
Commit-Ready: C Shapiro <shapiroc@google.com>
Tested-by: C Shapiro <shapiroc@google.com>
Reviewed-by: Simon Glass <sjg@chromium.org>

[modify] https://crrev.com/6a99607d5d4b02de87a393f20918be6600fff0ac/chromeos-base/autotest-server-tests/autotest-server-tests-9999.ebuild

Status: Fixed (was: Started)

Sign in to add a comment