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

Issue 725993 link

Starred by 4 users

Issue metadata

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



Sign in to add a comment

Clear out AUE builds.

Project Member Reported by bhthompson@google.com, May 24 2017

Issue description

Clear out AUE builds we should no longer need.
 
Cc: vapier@chromium.org jamescook@chromium.org
We have a few CLs related to this clean up we need to land in phases with various waterfall restarts:

1. Remove alex-chrome-pfq
https://chromium-review.googlesource.com/#/c/513521/
https://chromium-review.googlesource.com/#/c/514282/

2. Remove regular alex, mario and zgb paladins
https://chromium-review.googlesource.com/#/c/514243/
https://chromium-review.googlesource.com/#/c/514244/

3. Remove x86-mario-nowithdebug-paladin
https://chromium-review.googlesource.com/#/c/417284/
https://chromium-review.googlesource.com/#/c/513615/

4. Remove x86-mario-incremental builder
https://chromium-review.googlesource.com/#/c/417283/
https://chromium-review.googlesource.com/#/c/513499/

The end result is that we should have removed:
x86-alex-chrome-pfq
x86-alex-paladin
x86-zgb-paladin
x86-mario-nowithdebug-paladin
x86-mario-paladin
mario-incremental

And we will have added:
chell-nowithdebug-paladin
chell-incremental

If there are no objections, please feel free to start reviewing the CLs :).
Cc: ihf@chromium.org jrbarnette@chromium.org xixuan@chromium.org chingcodes@chromium.org
 Issue 722909  has been merged into this issue.
N.B. that there's an important ordering dependency, in that
we must remove the paladins before we remove the PFQ builder.
Otherwise, the paladins will start building Chrome from source.
There may be a unit test to enforce this, but unit test or no,
it's a requirement.

That is interesting, my impression was the reverse, that we had a test somewhere that validates that a paladin exists before a PFQ can exist, so I was thinking we had to do the reverse.

In either case we are making everything not important before they get removed so to hopefully avoid any build outages during this, once they are not important, we can remove both PFQ and CQ at once in a single waterfall restart.
In general, the CQ should catch both mistakes, either though binhost test, or through chromeos_config unittests.
So, it seems we might have to do this with some manual intervention, Don is correct.

The failure in https://chromium-review.googlesource.com/#/c/513521/ shows we cannot make a chrome-pfq builder non important as long as it has a paladin builder.

The failure in https://chromium-review.googlesource.com/#/c/514243/ shows we cannot make a paladin builder non important as long as it has a chrome-pfq builder.

So it seems we don't have a way of cleanly deprecating anything that has a unique PFQ builder, as the chrome-pfq stops generating prebuilts when it is not important.

If we cannot set the builds to non important to let them down cleanly, it means we may have to do an atomic deprecation, e.g. we remove the builders from the waterfall we need to turn down at once (chump) in tandem with a waterfall restart. We could use a series of steps like:

1. Schedule a waterfall restart and engage with the sheriff.
2. 30 min before the restart we chump the changes to put the waterfall configuration into the state we want.
3. Restart the waterfall.
4. Verify we come back up in the desired configuration.

In this case the back out plan is a revert and restart again.

Is there a better way?
>The failure in https://chromium-review.googlesource.com/#/c/513521/ shows we cannot make a chrome-pfq builder non important as long as it has a paladin builder.

>The failure in https://chromium-review.googlesource.com/#/c/514243/ shows we cannot make a paladin builder non important as long as it has a chrome-pfq builder.

Can't you just turn them both off in a single CL?
We can have a single CL that does the operation, however we don't seem to be able to have them all set as not important at the same time.

The CL could also just outright remove the builder configs, skipping the not important phase, however that is likely to cause other build breakage, e.g. the live builders get the new chromite with no config and have a bad day. To avoid this we could immediately restart the waterfall as suggested in comment 7, such that we don't have zombie configs on the waterfall. 
Maybe that is ok, if the config still exists in chromite, but it is not allocated on the waterfall, the CLs can land and be ok until the waterfall later does get restarted. 

It does mean we would go directly from being important to not existing upon the restart though.

The only case this breaks is the nowithdebug part which is directly instantiated in the config_dump.json, but that one seems safe to do in a non important then remove sequence.
I think you only need to mark them all as unimportant simultaneously.
If not, then the unit tests are wrong and should be fixed. We should not be asserting that non-important paladins or pfqs are mutually protected.
We can try that, if a non important paladin does not demand Chrome prebuilts then it should work, I was afraid that even a non important paladin would fail binhost test.
We have optimistic results, I think if we also have https://chromium-review.googlesource.com/#/c/417284/ we can pass.
Project Member

Comment 15 by bugdroid1@chromium.org, May 26 2017

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

commit 4d5303ce28286e4b843e20c5444c6cb4db1443dc
Author: Bernie Thompson <bhthompson@google.com>
Date: Fri May 26 21:35:32 2017

Make alex-chrome-pfq not important

BUG= chromium:725993 
TEST=None
CQ-DEPEND=CL:514243
CQ-DEPEND=CL:417284

Change-Id: I5474b5ace678dc672106020996af65e5719f9798
Reviewed-on: https://chromium-review.googlesource.com/513521
Commit-Ready: Bernie Thompson <bhthompson@chromium.org>
Tested-by: Bernie Thompson <bhthompson@chromium.org>
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>

[modify] https://crrev.com/4d5303ce28286e4b843e20c5444c6cb4db1443dc/cbuildbot/config_dump.json
[modify] https://crrev.com/4d5303ce28286e4b843e20c5444c6cb4db1443dc/cbuildbot/prebuilts_unittest.py
[modify] https://crrev.com/4d5303ce28286e4b843e20c5444c6cb4db1443dc/cbuildbot/chromeos_config.py

Project Member

Comment 16 by bugdroid1@chromium.org, May 26 2017

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

commit 285c477980d2682441f4f9ec4145d2f2c6b7fc98
Author: Bernie Thompson <bhthompson@google.com>
Date: Fri May 26 21:35:32 2017

Make alex, mario, and zgb paladins not important

BUG= chromium:725993 
TEST=None
CQ-DEPEND=CL:513521

Change-Id: If4f436cfcaf17a5b0f43b5e7b3d2c5ff7a262a5b
Reviewed-on: https://chromium-review.googlesource.com/514243
Commit-Ready: Bernie Thompson <bhthompson@chromium.org>
Tested-by: Bernie Thompson <bhthompson@chromium.org>
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>

[modify] https://crrev.com/285c477980d2682441f4f9ec4145d2f2c6b7fc98/cbuildbot/config_dump.json
[modify] https://crrev.com/285c477980d2682441f4f9ec4145d2f2c6b7fc98/cbuildbot/chromeos_config.py

Project Member

Comment 18 by bugdroid1@chromium.org, May 28 2017

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

commit f117ecd5621f0c12c869d0ac1471f9e435a4b317
Author: Bernie Thompson <bhthompson@google.com>
Date: Sun May 28 05:16:07 2017

Remove x86-alex-no-vmtest-pre-cq from default pre-cq list

BUG= chromium:725993 
TEST=None

Change-Id: I037f9629932d179d2ff43375d9b5d2842773a0a0
Reviewed-on: https://chromium-review.googlesource.com/517962
Tested-by: Bernie Thompson <bhthompson@chromium.org>
Reviewed-by: Kevin Cernekee <cernekee@chromium.org>

[modify] https://crrev.com/f117ecd5621f0c12c869d0ac1471f9e435a4b317/lib/constants.py

Project Member

Comment 19 by bugdroid1@chromium.org, May 30 2017

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

commit daa45924dafd228740833ebe2ff2af1f0df13e6d
Author: Bernie Thompson <bhthompson@google.com>
Date: Tue May 30 23:29:02 2017

Remove mario-incremental builder

BUG= chromium:725993 
TEST=None

Change-Id: I4c32b641315b8f8c361766d9d99c10039cbb7de6
Reviewed-on: https://chromium-review.googlesource.com/513499
Commit-Ready: Bernie Thompson <bhthompson@chromium.org>
Tested-by: Bernie Thompson <bhthompson@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/daa45924dafd228740833ebe2ff2af1f0df13e6d/cbuildbot/config_dump.json
[modify] https://crrev.com/daa45924dafd228740833ebe2ff2af1f0df13e6d/cbuildbot/waterfall_layout_dump.txt
[modify] https://crrev.com/daa45924dafd228740833ebe2ff2af1f0df13e6d/cbuildbot/chromeos_config.py

Project Member

Comment 20 by bugdroid1@chromium.org, May 30 2017

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

commit 68e7717f59b9655bc41c25030f369d558026a471
Author: Bernie Thompson <bhthompson@google.com>
Date: Tue May 30 23:56:15 2017

Remove alex, mario, and zgb paladins

BUG= chromium:725993 
TEST=None
CQ-DEPEND=CL:514282

Change-Id: I3741378dae37116e432c65d3c7375f5f96e206c3
Reviewed-on: https://chromium-review.googlesource.com/514244
Commit-Queue: Bernie Thompson <bhthompson@chromium.org>
Tested-by: Bernie Thompson <bhthompson@chromium.org>
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>

[modify] https://crrev.com/68e7717f59b9655bc41c25030f369d558026a471/cbuildbot/config_dump.json
[modify] https://crrev.com/68e7717f59b9655bc41c25030f369d558026a471/cbuildbot/waterfall_layout_dump.txt
[modify] https://crrev.com/68e7717f59b9655bc41c25030f369d558026a471/cbuildbot/chromeos_config.py

Project Member

Comment 21 by bugdroid1@chromium.org, May 30 2017

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

commit 1d432a61991b3ed4c7e178a6ce021cec276b3139
Author: Bernie Thompson <bhthompson@google.com>
Date: Tue May 30 23:56:22 2017

Remove alex-chrome-pfq builder

BUG= chromium:725993 
TEST=None
CQ-DEPEND=CL:514244

Change-Id: Idd90a210d42b0165d7802feb3d017170fc2d9d99
Reviewed-on: https://chromium-review.googlesource.com/514282
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
Commit-Queue: Bernie Thompson <bhthompson@chromium.org>
Tested-by: Bernie Thompson <bhthompson@chromium.org>

[modify] https://crrev.com/1d432a61991b3ed4c7e178a6ce021cec276b3139/cbuildbot/config_dump.json
[modify] https://crrev.com/1d432a61991b3ed4c7e178a6ce021cec276b3139/cbuildbot/waterfall_layout_dump.txt
[modify] https://crrev.com/1d432a61991b3ed4c7e178a6ce021cec276b3139/cbuildbot/chromeos_config.py

Status: Fixed (was: Untriaged)

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

Status: Archived (was: Fixed)
Project Member

Comment 24 by bugdroid1@chromium.org, Jun 16 2018

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chromeos/manifest-internal/+/044d8abf0790b1b364d0a59bc5dfe61107c0c2ad

commit 044d8abf0790b1b364d0a59bc5dfe61107c0c2ad
Author: Bernie Thompson <bhthompson@google.com>
Date: Sat Jun 16 18:50:53 2018

Sign in to add a comment