New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 735191 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Adjust payload generation for new GE data formats.

Project Member Reported by dgarr...@chromium.org, Jun 20 2017

Issue description

As part of increasing the number of delta payloads generated, GE is revising the payload related json files they generate.

We need to revise our code appropriately. The relevant code is chromite/lib/paygen/paygen_build_lib.py.

The proposed changes from GE are here:

https://docs.google.com/a/google.com/document/d/1mDyvYmVPiwlbCTYqNf0Ty55MD_wjFoiHh9aHrVDFgJc
 
Cc: akes...@chromium.org
Labels: -Pri-3 Pri-1
I intend to review the new format for correctness by may/may not be the one to implement the work.
Here an alternative proposed format (different from doc above)

'type' is only used for logging purposes.
'board', 'channel', 'version' identify the source build.
If 'delta' is true, we generate a delta.
If 'delta_test' is true, we schedule a delta test.
If 'full_test' is true, we schedule a full payload test.

The logic used today is:

type OMAHA:
  delta = true
  delta_test = true
  full_test = false

type STEPPING_STONE:
  treat the same as FSI

type FSI:
  delta = is_active && is_delta_supported
  delta_test = delta && is_lab_stable
  full_test = is_lab_stable


{
  "asuka": [
    {
      "type": "OMAHA",
      "board": "asuka",
      "channel": "dev",
      "version": "5.12.0",
      "delta": true,
      "delta_test": true,
      "full_test": true
    },
    <arbitrary number of source versions listed>
  ],
  "never_released_board": []
}

Cc: leecy@chromium.org
leecy@ is going to revise the format in her doc to partially match my suggestion. The important part is that it will move all business logic about deciding what to generate into GE.

One change is that some boards will be emitted with a reason of "NO_DELTA_TYPE" to show that they are valid and need a full payload, but don't need any deltas generated.

paygen_build_lib will only use type fields for logging, other than that particular value.
Also, when this change is landed, it will probably need cherry-picking to at least R60 if not all active branches. That should be easy to do, since paygen_build_lib has been relatively static for a long time.
Owner: leecy@chromium.org
This is currently blocked on GE providing a sample file, which we expect soon.

Please pass back when it's ready.

Comment 6 by leecy@chromium.org, Jun 29 2017

Try the file at gs://chromeos-build-release-console-staging/paygen.json.  I've granted read access to you.  It's missing some entries which I need to track down but this is the format of the file.

Comment 7 by leecy@chromium.org, Jun 29 2017

Owner: dgarr...@chromium.org
Status: Started (was: Untriaged)
Cc: nxia@chromium.org
Christine, I think I screwed up and missed something the other day.

Things not currently specified:
  Generate the full payload.
  Test the full payload for N -> N updates.
  Generate an unsigned (but not a signed) N->N payload.
  Test the N->N delta.

My thinking when we talked was that we always do all of them, so there is no need to specify them. However, I was wrong.

We do always generate full payloads for every build that runs paygen. So... that much is good.

However, we don't run N->N full payload testing if payload tests are disabled.
And we don't generate N->N test payloads if delta generation is disabled.
And we don't test N->N test payloads if payload testing is disabled.

I can continue to use the build option file values to determine this, if needed, but it means we'll still retain partial ownership.

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/dd8ddffc2773af67103857696a4f02845fa92884

commit dd8ddffc2773af67103857696a4f02845fa92884
Author: Don Garrett <dgarrett@google.com>
Date: Fri Jun 30 23:02:48 2017

paygen_build_lib: Remove nplusone (npo) concept.

We haven't generated these types of images in a long time, so remove
support for them. Before adding new features.

BUG= chromium:735191 
TEST=Unittests only.

Change-Id: Ib75ffd745165789d9dd1ad35b23e29f5687a6584
Reviewed-on: https://chromium-review.googlesource.com/557359
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Ningning Xia <nxia@chromium.org>

[modify] https://crrev.com/dd8ddffc2773af67103857696a4f02845fa92884/lib/paygen/paygen_build_lib.py
[modify] https://crrev.com/dd8ddffc2773af67103857696a4f02845fa92884/lib/paygen/paygen_build_lib_unittest.py
[modify] https://crrev.com/dd8ddffc2773af67103857696a4f02845fa92884/lib/paygen/paygen_payload_lib_unittest.py

Comment 11 by leecy@chromium.org, Jun 30 2017

I guess for the immediate short term it makes sense to still retain partial ownership since GE doesn't know about these scenarios at all.  Let's discuss when you get back however if it makes sense to move this portion into the paygen file.
Labels: akeshet-pending-downgrade
ChromeOS Infra P1 Bugscrub.

P1 Bugs in this component should be important enough to get weekly status updates.

Is this already fixed?  -> Fixed
Is this no longer relevant? -> Archived or WontFix
Is this not a P1, based on go/chromeos-infra-bug-slo rubric? -> lower priority.
Is this a Feature Request rather than a bug? Type -> Feature
Is this missing important information or scope needed to decide how to proceed? -> Ask question on bug, possibly reassign.
Does this bug have the wrong owner? -> reassign.

Bugs that remain in this state next week will be downgraded to P2.
Labels: -Type-Bug Type-Feature
Needs to be in production before the next branch in two weeks.
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/47dfc33970e62a29e06cff2dd7153c082925f769

commit 47dfc33970e62a29e06cff2dd7153c082925f769
Author: Don Garrett <dgarrett@google.com>
Date: Wed Jul 19 19:34:33 2017

paygen_build_lib: Add GetPaygenJson().

We are replacing all paygen config files with the new paygen.json.

This CL adds logic to fetch/parse the new file, and uses it to replace
boards.json.

BUG= chromium:735191 
TEST=Unittests

Change-Id: I3e0d91295a9c18ea427c72772433792a7d7ab406
Reviewed-on: https://chromium-review.googlesource.com/557360
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>

[add] https://crrev.com/47dfc33970e62a29e06cff2dd7153c082925f769/lib/paygen/testdata/paygen.json
[modify] https://crrev.com/47dfc33970e62a29e06cff2dd7153c082925f769/lib/paygen/paygen_build_lib.py
[modify] https://crrev.com/47dfc33970e62a29e06cff2dd7153c082925f769/lib/paygen/paygen_build_lib_unittest.py
[modify] https://crrev.com/47dfc33970e62a29e06cff2dd7153c082925f769/cbuildbot/stages/release_stages.py

One other concept that isn't mentioned in the paygen.json file, but that I will deal with is PreMP vs MP signed images.

The rules will be:
  Generate a signed full payload for PreMP if it exists.
  Generate a signed full payload for MP if it exists.
  Generate a delta if source/target both have PreMP signed images.
  Generate a delta if source/target both have MP signed images.
  Generate a test delta if asked for, and ignore signing.

Note that a given build can be signed for PreMP only, MP only, or both PreMP and MP at the same time.

Comment 16 by leecy@chromium.org, Jul 20 2017

FYI, just added the permissions for the builders to access paygen.json, since the CL was reverted due to lack of access.
Project Member

Comment 17 by bugdroid1@chromium.org, Jul 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/10be8824167db8d31bbe933b3764b38ced66596c

commit 10be8824167db8d31bbe933b3764b38ced66596c
Author: Don Garrett <dgarrett@google.com>
Date: Thu Jul 20 23:41:22 2017

paygen_build_lib: Fix --network test.

Since we don't run --network tests, one of them had bitrotted.

BUG= chromium:735191 
TEST=Ran tests with --network.

Change-Id: Ib020f137dd9ba65d342754d814305c6b3a1393ba
Reviewed-on: https://chromium-review.googlesource.com/578235
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Ningning Xia <nxia@chromium.org>

[modify] https://crrev.com/10be8824167db8d31bbe933b3764b38ced66596c/lib/paygen/paygen_build_lib_unittest.py

Project Member

Comment 18 by bugdroid1@chromium.org, Jul 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/1d72eae1a0a5175f6a29d90b91c2ff261b4d217f

commit 1d72eae1a0a5175f6a29d90b91c2ff261b4d217f
Author: Wu-Cheng Li <wuchengli@chromium.org>
Date: Fri Jul 21 15:31:36 2017

Revert "paygen_build_lib: Add GetPaygenJson()."

This reverts commit 47dfc33970e62a29e06cff2dd7153c082925f769.

Reason for revert: The build passes, but it doesn't produce any payloads.

BUG=chromium:747211
TEST=None

Original change's description:
> paygen_build_lib: Add GetPaygenJson().
>
> We are replacing all paygen config files with the new paygen.json.
>
> This CL adds logic to fetch/parse the new file, and uses it to replace
> boards.json.
>
> BUG= chromium:735191 
> TEST=Unittests
>
> Change-Id: I3e0d91295a9c18ea427c72772433792a7d7ab406
> Reviewed-on: https://chromium-review.googlesource.com/557360
> Commit-Ready: Don Garrett <dgarrett@chromium.org>
> Tested-by: Don Garrett <dgarrett@chromium.org>
> Reviewed-by: Don Garrett <dgarrett@chromium.org>

Bug:  chromium:735191 
Change-Id: I5ba892664c38e1cbfe6888f6ff25352ea9a42913
Reviewed-on: https://chromium-review.googlesource.com/580471
Commit-Ready: Wu-Cheng Li <wuchengli@chromium.org>
Tested-by: Wu-Cheng Li <wuchengli@chromium.org>
Reviewed-by: Wu-Cheng Li <wuchengli@chromium.org>

[delete] https://crrev.com/9b4522a4bc6b9ba67b381694c11fe28534193b27/lib/paygen/testdata/paygen.json
[modify] https://crrev.com/1d72eae1a0a5175f6a29d90b91c2ff261b4d217f/lib/paygen/paygen_build_lib.py
[modify] https://crrev.com/1d72eae1a0a5175f6a29d90b91c2ff261b4d217f/lib/paygen/paygen_build_lib_unittest.py
[modify] https://crrev.com/1d72eae1a0a5175f6a29d90b91c2ff261b4d217f/cbuildbot/stages/release_stages.py

Project Member

Comment 19 by bugdroid1@chromium.org, Jul 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/8e3216043b0ae95b99362e743802f7ec613c800b

commit 8e3216043b0ae95b99362e743802f7ec613c800b
Author: Don Garrett <dgarrett@google.com>
Date: Wed Jul 26 23:46:52 2017

paygen_build_lib: Add GetPaygenJson().

We are replacing all paygen config files with the new paygen.json.

This CL adds logic to fetch/parse the new file, and uses it to replace
boards.json.

RELANDING:
  Rev 1 of this CL was previously landed as CL:557360.
  Diff Rev 1 and final Rev to see the fix.

BUG= chromium:735191 
TEST=Unittests

Change-Id: I95d88a36bea70dfce61b95ec08e39d4c7b134227
Reviewed-on: https://chromium-review.googlesource.com/581431
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>

[modify] https://crrev.com/8e3216043b0ae95b99362e743802f7ec613c800b/cbuildbot/stages/release_stages_unittest.py
[add] https://crrev.com/8e3216043b0ae95b99362e743802f7ec613c800b/lib/paygen/testdata/paygen.json
[modify] https://crrev.com/8e3216043b0ae95b99362e743802f7ec613c800b/lib/paygen/paygen_build_lib.py
[modify] https://crrev.com/8e3216043b0ae95b99362e743802f7ec613c800b/lib/paygen/paygen_build_lib_unittest.py
[modify] https://crrev.com/8e3216043b0ae95b99362e743802f7ec613c800b/cbuildbot/stages/release_stages.py

Project Member

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

Labels: merge-merged-release-R61-9765.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/ee5ea4bc6fc71fa82b291da0a6e50de773891103

commit ee5ea4bc6fc71fa82b291da0a6e50de773891103
Author: Wu-Cheng Li <wuchengli@chromium.org>
Date: Thu Jul 27 03:55:34 2017

Revert "paygen_build_lib: Add GetPaygenJson()."

This reverts commit 47dfc33970e62a29e06cff2dd7153c082925f769.

Reason for revert: The build passes, but it doesn't produce any payloads.

BUG=chromium:747211
TEST=None

Original change's description:
> paygen_build_lib: Add GetPaygenJson().
>
> We are replacing all paygen config files with the new paygen.json.
>
> This CL adds logic to fetch/parse the new file, and uses it to replace
> boards.json.
>
> BUG= chromium:735191 
> TEST=Unittests
>
> Change-Id: I3e0d91295a9c18ea427c72772433792a7d7ab406
> Reviewed-on: https://chromium-review.googlesource.com/557360
> Commit-Ready: Don Garrett <dgarrett@chromium.org>
> Tested-by: Don Garrett <dgarrett@chromium.org>
> Reviewed-by: Don Garrett <dgarrett@chromium.org>

Bug:  chromium:735191 
Change-Id: I5ba892664c38e1cbfe6888f6ff25352ea9a42913
Reviewed-on: https://chromium-review.googlesource.com/580471
Commit-Ready: Wu-Cheng Li <wuchengli@chromium.org>
Tested-by: Wu-Cheng Li <wuchengli@chromium.org>
Reviewed-by: Wu-Cheng Li <wuchengli@chromium.org>
(cherry picked from commit 1d72eae1a0a5175f6a29d90b91c2ff261b4d217f)
Reviewed-on: https://chromium-review.googlesource.com/588348
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
Commit-Queue: Bernie Thompson <bhthompson@chromium.org>
Tested-by: Bernie Thompson <bhthompson@chromium.org>

[delete] https://crrev.com/dce9b3d5aceb93653df8a65906f6c3d1ee0f8a0a/lib/paygen/testdata/paygen.json
[modify] https://crrev.com/ee5ea4bc6fc71fa82b291da0a6e50de773891103/lib/paygen/paygen_build_lib.py
[modify] https://crrev.com/ee5ea4bc6fc71fa82b291da0a6e50de773891103/lib/paygen/paygen_build_lib_unittest.py
[modify] https://crrev.com/ee5ea4bc6fc71fa82b291da0a6e50de773891103/cbuildbot/stages/release_stages.py

Project Member

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

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/9a1a7f620923f154228bc1c4bf52bc22a202baa4

commit 9a1a7f620923f154228bc1c4bf52bc22a202baa4
Author: Don Garrett <dgarrett@google.com>
Date: Thu Jul 27 05:41:56 2017

paygen_build_lib: Cleanup of PaygenBuild class.

PaygenBuild class was created to be more flexible than we need it to
be, and that flexibility makes it more complicated than it should be.

  paygen_build_lib._PaygenBuild -> paygen_build_lib.PaygenBuild
  Remove paygen_build_lib.CreatePayloads wrapper.
  Remove PaygenBuild unused ignore_finished argument.
  Remove PaygenBuild unused run_parallel argument.
  Remove PaygenBuild unused output_dir argument.
  Remove duplicate call to ValidateBoardConfig in release_stages.
  Remove self._control_dir.
  gspaths UnsignedImageArchiveUri -> UnsignedImageUri.
  DiscoverImages -> DiscoverSignedImages.
  DiscoverTestImageArchives -> DiscoverTestImages.

This CL includes a series of small incremental changes which will be
uploaded as new revisions for easier review, instead of using a tall
stack of very small CLs.

BUG= chromium:735191 
TEST=Unittests (including --network paygen tests)

Change-Id: I636026656c18174b5edd8290cc8cae09f9e22573
Reviewed-on: https://chromium-review.googlesource.com/576857
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Aseda Aboagye <aaboagye@chromium.org>

[modify] https://crrev.com/9a1a7f620923f154228bc1c4bf52bc22a202baa4/cbuildbot/stages/release_stages.py
[modify] https://crrev.com/9a1a7f620923f154228bc1c4bf52bc22a202baa4/lib/paygen/paygen_build_lib_unittest.py
[modify] https://crrev.com/9a1a7f620923f154228bc1c4bf52bc22a202baa4/lib/paygen/gspaths_unittest.py
[modify] https://crrev.com/9a1a7f620923f154228bc1c4bf52bc22a202baa4/lib/paygen/gspaths.py
[modify] https://crrev.com/9a1a7f620923f154228bc1c4bf52bc22a202baa4/cbuildbot/stages/release_stages_unittest.py
[modify] https://crrev.com/9a1a7f620923f154228bc1c4bf52bc22a202baa4/lib/paygen/paygen_build_lib.py

Project Member

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

Labels: merge-merged-stabilize-9765.7.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/6b481472e1e22276b8b69c391112b4746e483d5c

commit 6b481472e1e22276b8b69c391112b4746e483d5c
Author: Wu-Cheng Li <wuchengli@chromium.org>
Date: Fri Jul 28 00:17:05 2017

Revert "paygen_build_lib: Add GetPaygenJson()."

This reverts commit 47dfc33970e62a29e06cff2dd7153c082925f769.

Reason for revert: The build passes, but it doesn't produce any payloads.

BUG=chromium:747211
TEST=None

Original change's description:
> paygen_build_lib: Add GetPaygenJson().
>
> We are replacing all paygen config files with the new paygen.json.
>
> This CL adds logic to fetch/parse the new file, and uses it to replace
> boards.json.
>
> BUG= chromium:735191 
> TEST=Unittests
>
> Change-Id: I3e0d91295a9c18ea427c72772433792a7d7ab406
> Reviewed-on: https://chromium-review.googlesource.com/557360
> Commit-Ready: Don Garrett <dgarrett@chromium.org>
> Tested-by: Don Garrett <dgarrett@chromium.org>
> Reviewed-by: Don Garrett <dgarrett@chromium.org>

Bug:  chromium:735191 
Change-Id: I5ba892664c38e1cbfe6888f6ff25352ea9a42913
Reviewed-on: https://chromium-review.googlesource.com/580471
Commit-Ready: Wu-Cheng Li <wuchengli@chromium.org>
Tested-by: Wu-Cheng Li <wuchengli@chromium.org>
Reviewed-by: Wu-Cheng Li <wuchengli@chromium.org>
(cherry picked from commit 1d72eae1a0a5175f6a29d90b91c2ff261b4d217f)
Reviewed-on: https://chromium-review.googlesource.com/588348
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
Commit-Queue: Bernie Thompson <bhthompson@chromium.org>
Tested-by: Bernie Thompson <bhthompson@chromium.org>
(cherry picked from commit ee5ea4bc6fc71fa82b291da0a6e50de773891103)
Reviewed-on: https://chromium-review.googlesource.com/590147

[delete] https://crrev.com/fe9bcd63461d06c14fed202fcd9de365e2b96624/lib/paygen/testdata/paygen.json
[modify] https://crrev.com/6b481472e1e22276b8b69c391112b4746e483d5c/lib/paygen/paygen_build_lib.py
[modify] https://crrev.com/6b481472e1e22276b8b69c391112b4746e483d5c/lib/paygen/paygen_build_lib_unittest.py
[modify] https://crrev.com/6b481472e1e22276b8b69c391112b4746e483d5c/cbuildbot/stages/release_stages.py

Project Member

Comment 23 by bugdroid1@chromium.org, Jul 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/5ba690792c80e5dfc95caa7a740306aeeab2c85d

commit 5ba690792c80e5dfc95caa7a740306aeeab2c85d
Author: Don Garrett <dgarrett@google.com>
Date: Mon Jul 31 00:39:47 2017

Use paygen.json to configure payload generation.

This is a heavy rework of paygen_build_lib, and a total rework of
paygen_build_lib_unittest.

This change lets Golden Eye own the business logic of what payloads to
generate and how to test them. This allows them to add proper support
for stepping stones, and to generate increased delta coverage without
needing support from Chromite (and being able to avoid our branching
issues).

Before this change, paygen_build_lib contained a lot of logic to
decide which payloads to generate and how to test them. This meant
detailed knowledge about FSIs, NMO builds, etc.

Afterwards, we chromite still owns knowledge of PreMP vs MP key
signing, full payload generation, and N2N delta
generation/testing. Also, a build configuration setting can still be
used to turn off all delta generation.

As a secondary issue, this CL revamps the paygen_build_lib unittests
to be based on mock instead of mox.

BUG= chromium:735191 
TEST=run_tests && paygen_build_lib_unittest --network
     cbuildbot --remote <board>-payloads

Change-Id: I0c6da4ae403da2ab02a49e221acf926fc95fdc8c
Reviewed-on: https://chromium-review.googlesource.com/585897
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org>
Reviewed-by: Ningning Xia <nxia@chromium.org>

[modify] https://crrev.com/5ba690792c80e5dfc95caa7a740306aeeab2c85d/lib/paygen/testdata/paygen.json
[modify] https://crrev.com/5ba690792c80e5dfc95caa7a740306aeeab2c85d/cbuildbot/stages/release_stages.py
[modify] https://crrev.com/5ba690792c80e5dfc95caa7a740306aeeab2c85d/lib/paygen/paygen_build_lib_unittest.py
[modify] https://crrev.com/5ba690792c80e5dfc95caa7a740306aeeab2c85d/lib/paygen/gspaths_unittest.py
[modify] https://crrev.com/5ba690792c80e5dfc95caa7a740306aeeab2c85d/lib/paygen/gspaths.py
[modify] https://crrev.com/5ba690792c80e5dfc95caa7a740306aeeab2c85d/cbuildbot/stages/release_stages_unittest.py
[modify] https://crrev.com/5ba690792c80e5dfc95caa7a740306aeeab2c85d/lib/paygen/paygen_build_lib.py
[modify] https://crrev.com/5ba690792c80e5dfc95caa7a740306aeeab2c85d/lib/paygen/paygen_payload_lib.py

There were two errors in the CL that showed up in production:

Some boards without any deltas generate an error during .json searching:

https://luci-logdog.appspot.com/v/?s=chromeos%2Fbb%2Fchromeos%2Farkham-release%2F1357%2F%2B%2Frecipes%2Fsteps%2FPaygenBuildCanary%2F0%2Fstdout

The rest of the boards failed to generate actual payloads because a URL value wasn't populated anymore.

https://luci-logdog.appspot.com/v/?s=chromeos%2Fbb%2Fchromeos%2Ffalco_li-release%2F1459%2F%2B%2Frecipes%2Fsteps%2FPaygenBuildDev%2F0%2Fstdout
Project Member

Comment 25 by bugdroid1@chromium.org, Aug 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/2b66b9a1f7d663a2d7d8da2b42b2dfc9633dde4c

commit 2b66b9a1f7d663a2d7d8da2b42b2dfc9633dde4c
Author: Don Garrett <dgarrett@google.com>
Date: Wed Aug 02 19:47:53 2017

Use paygen.json to configure payload generation.

This is a heavy rework of paygen_build_lib, and a total rework of
paygen_build_lib_unittest.

This change lets Golden Eye own the business logic of what payloads to
generate and how to test them. This allows them to add proper support
for stepping stones, and to generate increased delta coverage without
needing support from Chromite (and being able to avoid our branching
issues).

Before this change, paygen_build_lib contained a lot of logic to
decide which payloads to generate and how to test them. This meant
detailed knowledge about FSIs, NMO builds, etc.

Afterwards, we chromite still owns knowledge of PreMP vs MP key
signing, full payload generation, and N2N delta
generation/testing. Also, a build configuration setting can still be
used to turn off all delta generation.

As a secondary issue, this CL revamps the paygen_build_lib unittests
to be based on mock instead of mox.

This CL was landed as CL:585897, reverted as CL:594487. There were two
error cases with the original CL.

A) 'channel' key error for boards with no deltas defined.

Missed in testing because it only applies to a small number of boards,
like 'arkham'.

B) .url is None for payload being generated.

Missed in testing because testing tryjobs were only run against builds
that failed payload testing, but all actual payloads existed.

First revision uploaded is the original CL.
Second revision includes a unittest and fix for A.

BUG= chromium:735191 
TEST=run_tests && paygen_build_lib_unittest --network
     cbuildbot --remote <board>-payloads

Change-Id: I72a20f745d049963790d3e3880e3d6076b8e2324
Reviewed-on: https://chromium-review.googlesource.com/594841
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Ningning Xia <nxia@chromium.org>

[modify] https://crrev.com/2b66b9a1f7d663a2d7d8da2b42b2dfc9633dde4c/lib/paygen/testdata/paygen.json
[modify] https://crrev.com/2b66b9a1f7d663a2d7d8da2b42b2dfc9633dde4c/cbuildbot/stages/release_stages.py
[modify] https://crrev.com/2b66b9a1f7d663a2d7d8da2b42b2dfc9633dde4c/lib/paygen/paygen_build_lib_unittest.py
[modify] https://crrev.com/2b66b9a1f7d663a2d7d8da2b42b2dfc9633dde4c/lib/paygen/gspaths_unittest.py
[modify] https://crrev.com/2b66b9a1f7d663a2d7d8da2b42b2dfc9633dde4c/lib/paygen/gspaths.py
[modify] https://crrev.com/2b66b9a1f7d663a2d7d8da2b42b2dfc9633dde4c/cbuildbot/stages/release_stages_unittest.py
[modify] https://crrev.com/2b66b9a1f7d663a2d7d8da2b42b2dfc9633dde4c/lib/paygen/paygen_build_lib.py
[modify] https://crrev.com/2b66b9a1f7d663a2d7d8da2b42b2dfc9633dde4c/lib/paygen/paygen_payload_lib.py

Labels: -akeshet-pending-downgrade -merge-merged-release-R61-9765.B -merge-merged-stabilize-9765.7.B
Labels: Merge-Request-61
Pls apply appropriate OSs. Thank you.
It's in the component Infra>Client>ChromeOS.
Labels: OS-Chrome
Project Member

Comment 31 by sheriffbot@chromium.org, Aug 4 2017

Labels: -Merge-Request-61 Hotlist-Merge-Reject Merge-Reject-61
The bug is marked as P3 or Feature. It should not be merged as M61 is in beta. 
Please contact the approriate milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Type-Feature -Hotlist-Merge-Reject -Merge-Reject-61 Merge-Request-61 Type-Bug
It's a feature needed in M61. Switched to Bug to get past the automation.
Project Member

Comment 33 by sheriffbot@chromium.org, Aug 4 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: M61 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: keta...@chromium.org kbleicher@chromium.org
Labels: M-61
I think this is ok for merge to 61. Adding Ketaki to review/approve 
Labels: -Merge-Review-61 Merge-Approved-61
Approving merge to M61 Chrome OS.
Project Member

Comment 36 by bugdroid1@chromium.org, Aug 10 2017

Labels: merge-merged-release-R61-9765.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/87db05bf06e87f19facc4190d10dc53abe63cd02

commit 87db05bf06e87f19facc4190d10dc53abe63cd02
Author: Don Garrett <dgarrett@google.com>
Date: Thu Aug 10 21:55:53 2017

paygen_build_lib: Add GetPaygenJson().

We are replacing all paygen config files with the new paygen.json.

This CL adds logic to fetch/parse the new file, and uses it to replace
boards.json.

RELANDING:
  Rev 1 of this CL was previously landed as CL:557360.
  Diff Rev 1 and final Rev to see the fix.

BUG= chromium:735191 
TEST=Unittests

Change-Id: I95d88a36bea70dfce61b95ec08e39d4c7b134227
Previous-Reviewed-on: https://chromium-review.googlesource.com/581431
(cherry picked from commit 53ffd29dc9cb5d2cfa8bdffd74cb903382e372d5)
Reviewed-on: https://chromium-review.googlesource.com/611208
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>

[modify] https://crrev.com/87db05bf06e87f19facc4190d10dc53abe63cd02/cbuildbot/stages/release_stages_unittest.py
[add] https://crrev.com/87db05bf06e87f19facc4190d10dc53abe63cd02/lib/paygen/testdata/paygen.json
[modify] https://crrev.com/87db05bf06e87f19facc4190d10dc53abe63cd02/lib/paygen/paygen_build_lib.py
[modify] https://crrev.com/87db05bf06e87f19facc4190d10dc53abe63cd02/lib/paygen/paygen_build_lib_unittest.py
[modify] https://crrev.com/87db05bf06e87f19facc4190d10dc53abe63cd02/cbuildbot/stages/release_stages.py

Project Member

Comment 37 by bugdroid1@chromium.org, Aug 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/06343119e9855ba23f05928f4fbbb2b0b24f64fb

commit 06343119e9855ba23f05928f4fbbb2b0b24f64fb
Author: Don Garrett <dgarrett@google.com>
Date: Thu Aug 10 21:56:08 2017

paygen_build_lib: Cleanup of PaygenBuild class.

PaygenBuild class was created to be more flexible than we need it to
be, and that flexibility makes it more complicated than it should be.

  paygen_build_lib._PaygenBuild -> paygen_build_lib.PaygenBuild
  Remove paygen_build_lib.CreatePayloads wrapper.
  Remove PaygenBuild unused ignore_finished argument.
  Remove PaygenBuild unused run_parallel argument.
  Remove PaygenBuild unused output_dir argument.
  Remove duplicate call to ValidateBoardConfig in release_stages.
  Remove self._control_dir.
  gspaths UnsignedImageArchiveUri -> UnsignedImageUri.
  DiscoverImages -> DiscoverSignedImages.
  DiscoverTestImageArchives -> DiscoverTestImages.

This CL includes a series of small incremental changes which will be
uploaded as new revisions for easier review, instead of using a tall
stack of very small CLs.

BUG= chromium:735191 
TEST=Unittests (including --network paygen tests)

Change-Id: I636026656c18174b5edd8290cc8cae09f9e22573
Previous-Reviewed-on: https://chromium-review.googlesource.com/576857
(cherry picked from commit 7ee1994141a4829550667d85036dbc804060b15e)
Reviewed-on: https://chromium-review.googlesource.com/611209
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>

[modify] https://crrev.com/06343119e9855ba23f05928f4fbbb2b0b24f64fb/cbuildbot/stages/release_stages.py
[modify] https://crrev.com/06343119e9855ba23f05928f4fbbb2b0b24f64fb/lib/paygen/paygen_build_lib_unittest.py
[modify] https://crrev.com/06343119e9855ba23f05928f4fbbb2b0b24f64fb/lib/paygen/gspaths_unittest.py
[modify] https://crrev.com/06343119e9855ba23f05928f4fbbb2b0b24f64fb/lib/paygen/gspaths.py
[modify] https://crrev.com/06343119e9855ba23f05928f4fbbb2b0b24f64fb/cbuildbot/stages/release_stages_unittest.py
[modify] https://crrev.com/06343119e9855ba23f05928f4fbbb2b0b24f64fb/lib/paygen/paygen_build_lib.py

Project Member

Comment 38 by bugdroid1@chromium.org, Aug 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/4abfb675ae517b3d62197d69d03c71a6125734a7

commit 4abfb675ae517b3d62197d69d03c71a6125734a7
Author: Don Garrett <dgarrett@google.com>
Date: Thu Aug 10 21:56:25 2017

Use paygen.json to configure payload generation.

This is a heavy rework of paygen_build_lib, and a total rework of
paygen_build_lib_unittest.

This change lets Golden Eye own the business logic of what payloads to
generate and how to test them. This allows them to add proper support
for stepping stones, and to generate increased delta coverage without
needing support from Chromite (and being able to avoid our branching
issues).

Before this change, paygen_build_lib contained a lot of logic to
decide which payloads to generate and how to test them. This meant
detailed knowledge about FSIs, NMO builds, etc.

Afterwards, we chromite still owns knowledge of PreMP vs MP key
signing, full payload generation, and N2N delta
generation/testing. Also, a build configuration setting can still be
used to turn off all delta generation.

As a secondary issue, this CL revamps the paygen_build_lib unittests
to be based on mock instead of mox.

This CL was landed as CL:585897, reverted as CL:594487. There were two
error cases with the original CL.

A) 'channel' key error for boards with no deltas defined.

Missed in testing because it only applies to a small number of boards,
like 'arkham'.

B) .url is None for payload being generated.

Missed in testing because testing tryjobs were only run against builds
that failed payload testing, but all actual payloads existed.

First revision uploaded is the original CL.
Second revision includes a unittest and fix for A.

BUG= chromium:735191 
TEST=run_tests && paygen_build_lib_unittest --network
     cbuildbot --remote <board>-payloads

Change-Id: I72a20f745d049963790d3e3880e3d6076b8e2324
Previous-Reviewed-on: https://chromium-review.googlesource.com/594841
(cherry picked from commit 17b7c2ba29ba378842c2948f2b8e45d9a380400a)
Reviewed-on: https://chromium-review.googlesource.com/611186
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>

[modify] https://crrev.com/4abfb675ae517b3d62197d69d03c71a6125734a7/lib/paygen/testdata/paygen.json
[modify] https://crrev.com/4abfb675ae517b3d62197d69d03c71a6125734a7/cbuildbot/stages/release_stages.py
[modify] https://crrev.com/4abfb675ae517b3d62197d69d03c71a6125734a7/lib/paygen/paygen_build_lib_unittest.py
[modify] https://crrev.com/4abfb675ae517b3d62197d69d03c71a6125734a7/lib/paygen/gspaths_unittest.py
[modify] https://crrev.com/4abfb675ae517b3d62197d69d03c71a6125734a7/lib/paygen/gspaths.py
[modify] https://crrev.com/4abfb675ae517b3d62197d69d03c71a6125734a7/cbuildbot/stages/release_stages_unittest.py
[modify] https://crrev.com/4abfb675ae517b3d62197d69d03c71a6125734a7/lib/paygen/paygen_build_lib.py
[modify] https://crrev.com/4abfb675ae517b3d62197d69d03c71a6125734a7/lib/paygen/paygen_payload_lib.py

Project Member

Comment 39 by sheriffbot@chromium.org, Aug 14 2017

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Approved-61
Labels: M-60 Merge-Approved-60
Approved for M60
Project Member

Comment 42 by bugdroid1@chromium.org, Aug 17 2017

Labels: merge-merged-release-R60-9592.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/a52086d1bba13c82cfde3bf87c30fa9bb86e8e70

commit a52086d1bba13c82cfde3bf87c30fa9bb86e8e70
Author: Don Garrett <dgarrett@google.com>
Date: Thu Aug 17 18:07:08 2017

paygen: Rolled up changes to implment paygen.json support.

This is a rolled up collection of many different CLs (and portions of
CLs) in cbuildbot that are required to backport paygen.json support.

BUG= chromium:735191 
TEST=run_tests and tryjobs.

Change-Id: I01e92333bcfb1190ceb03dd7cee41f0b63110383
Reviewed-on: https://chromium-review.googlesource.com/617809
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>

[add] https://crrev.com/a52086d1bba13c82cfde3bf87c30fa9bb86e8e70/lib/paygen/testdata/paygen.json
[modify] https://crrev.com/a52086d1bba13c82cfde3bf87c30fa9bb86e8e70/lib/retry_util.py
[modify] https://crrev.com/a52086d1bba13c82cfde3bf87c30fa9bb86e8e70/cbuildbot/stages/release_stages.py
[add] https://crrev.com/a52086d1bba13c82cfde3bf87c30fa9bb86e8e70/lib/retry_util_unittest.py
[modify] https://crrev.com/a52086d1bba13c82cfde3bf87c30fa9bb86e8e70/lib/paygen/paygen_build_lib_unittest.py
[modify] https://crrev.com/a52086d1bba13c82cfde3bf87c30fa9bb86e8e70/lib/paygen/gspaths_unittest.py
[modify] https://crrev.com/a52086d1bba13c82cfde3bf87c30fa9bb86e8e70/lib/paygen/paygen_payload_lib_unittest.py
[modify] https://crrev.com/a52086d1bba13c82cfde3bf87c30fa9bb86e8e70/lib/paygen/gspaths.py
[modify] https://crrev.com/a52086d1bba13c82cfde3bf87c30fa9bb86e8e70/cbuildbot/stages/release_stages_unittest.py
[modify] https://crrev.com/a52086d1bba13c82cfde3bf87c30fa9bb86e8e70/lib/paygen/paygen_build_lib.py
[modify] https://crrev.com/a52086d1bba13c82cfde3bf87c30fa9bb86e8e70/lib/paygen/paygen_payload_lib.py

Labels: -Merge-Approved-60
Status: Fixed (was: Started)
It's in M60. The merge was messier than I hoped, so we'll see if it works. It's in a single CL to make revert easier.
After CL https://chromium-review.googlesource.com/c/chromiumos/chromite/+/585897 warnings

"WARNING: Previous build image is missing, skipping: Build definition (board=u'guado-moblab', version=u'7358.0.0', channel='canary-channel') has no basic images.[" 

turned into fatal errors 

"INFO: Translating result Build definition (board=u'guado-moblab', version=u'7358.0.0', channel='canary-channel') has no basic images. to fail."

for guado_moblab-release builder. As a result, the builder fails starting with build #2289.

Comment 46 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment