+deputy fyi

Does this coincide with the recent push?

Related to https://chromium-review.googlesource.com/1128453 ?

This appears to have been part of the recent push (autotest range a022d333f..29868027d), so I'm guessing this is new and possibly outage causing. Upping to P0

Smells like an API compatibility issue. I see this argument being removed on both the server and client side, but perhaps not all clients (in particular, the one used for this test) is up to date.

Yep, any clients from before https://chromium-review.googlesource.com/c/chromiumos/third_party/autotest/+/1128452 will break.
Since both client/server changes were pushed together, that means all current builds as well.

:(

Revert in-flight.

I have a small amount of doubt that we have the root cause right -- this should have broken SSP tests in staging lab, since the server side API changes would have been pushed to the lab, but the client side changes would not have been included in the SSP test.


OTOH, currently there are no changes in master branch of autotest beyond prod.
So an emergency push is relatively safe.

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

commit 5fc1314f94de8f3b6e9bdfc724052e6466e97ffe
Author: Prathmesh Prabhu <pprabhu@chromium.org>
Date: Mon Jul 16 21:15:26 2018

Revert "[autotest] Delete Android from stable version RPC."

This reverts commit 8725202f94b5b0e386d0970d53d527893656fe27.

Reason for revert: This made an RPC change that was not backwards
compatible, breaking callers from before CL:1128452

Original change's description:
> [autotest] Delete Android from stable version RPC.
> 
> This deletes support for getting Android versions via the
> get_stable_version() RPC.
> 
> BUG= chromium:834335 
> TEST=Run the stable_version command on a local instance
> 
> Change-Id: I87aadb6330fc631e3e92c21bd39ddd1c0e9b0f60
> Reviewed-on: https://chromium-review.googlesource.com/1128453
> Commit-Ready: Richard Barnette <jrbarnette@chromium.org>
> Tested-by: Richard Barnette <jrbarnette@chromium.org>
> Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org>

BUG= chromium:864165 

Change-Id: I97d093bf548ebf776804c19c15b416e6b311d8e6
Reviewed-on: https://chromium-review.googlesource.com/1138913
Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org>
Tested-by: Prathmesh Prabhu <pprabhu@chromium.org>

[modify] https://crrev.com/5fc1314f94de8f3b6e9bdfc724052e6466e97ffe/site_utils/stable_version_utils.py
[modify] https://crrev.com/5fc1314f94de8f3b6e9bdfc724052e6466e97ffe/frontend/afe/rpc_interface.py

Emergency push completed. Keeping bug open to collect any prod load fallout.

+Chase-Pending Do we have a hole in staging test that is feasible to fix?

The exact same test (provision_AutoUpdate.double) did pass in staging today: http://chromeos-staging-master2.hot.corp.google.com/afe/#tab_id=view_job&object_id=21122

This is our friendly heisenbug which is a fallout of the push itself. I can't think of a technical fix for this other than staging rollout of the the client/server changes to an API -

- akeshet@'s tests started before the push, off the prod branch. The client code at that time had the old code (and the extra arg).
- I did the push, updating both server and client side code.
- akeshet@'s running test used the old client to call the new server code.

Kaboom.

My emergency push is unlikely to have actually helped, since most clients in this situation would have already hit this pretty soon.
In any case, lets reland and push the server changes in a day or so to let the clients settle.

> In any case, lets reland and push the server changes in a day or so
> to let the clients settle.

<sigh> I knew about this.  The CL that caused the problem said this:
    N.B.  This change can't be committed until the prior change that
    removes client calls is fully rolled out.

What went wrong is that I forgot, and committed the change prematurely.  :-(

That said, I wasn't aware of a code path through 'test_that' that would
break.  So, we'll want to understand that problem, and if necessary, send
a PSA to make sure it doesn't happen again.

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

commit 561c2f5049974f5a2cc0048e8283efb2f1d734a2
Author: Richard Barnette <jrbarnette@chromium.org>
Date: Fri Jul 27 03:45:04 2018

Reland "[autotest] Delete Android from stable version RPC."

This reverts commit 5fc1314f94de8f3b6e9bdfc724052e6466e97ffe.

Reason for revert:  All clients have now been upgraded to use
the new RPC interface.  No extant client should still be using
the `android` flag.  So, it should now be safe to reland this.

Original change's description:
> Revert "[autotest] Delete Android from stable version RPC."
>
> This reverts commit 8725202f94b5b0e386d0970d53d527893656fe27.
>
> Reason for revert: This made an RPC change that was not backwards
> compatible, breaking callers from before CL:1128452
>
> Original change's description:
> > [autotest] Delete Android from stable version RPC.
> >
> > This deletes support for getting Android versions via the
> > get_stable_version() RPC.
> >
> > BUG= chromium:834335 
> > TEST=Run the stable_version command on a local instance
> >
> > Change-Id: I87aadb6330fc631e3e92c21bd39ddd1c0e9b0f60
> > Reviewed-on: https://chromium-review.googlesource.com/1128453
> > Commit-Ready: Richard Barnette <jrbarnette@chromium.org>
> > Tested-by: Richard Barnette <jrbarnette@chromium.org>
> > Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org>
>
> BUG= chromium:864165 
>
> Change-Id: I97d093bf548ebf776804c19c15b416e6b311d8e6
> Reviewed-on: https://chromium-review.googlesource.com/1138913
> Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org>
> Tested-by: Prathmesh Prabhu <pprabhu@chromium.org>

Bug:  chromium:864165 
Change-Id: Iba41cbad43f2af4e2ccfdae71875b064e5b9f0a0
Reviewed-on: https://chromium-review.googlesource.com/1150460
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Richard Barnette <jrbarnette@chromium.org>
Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org>

[modify] https://crrev.com/561c2f5049974f5a2cc0048e8283efb2f1d734a2/site_utils/stable_version_utils.py
[modify] https://crrev.com/561c2f5049974f5a2cc0048e8283efb2f1d734a2/frontend/afe/rpc_interface.py

We see this error in many (but not all) tests on M68 since R68-10718.71.0. Example job: http://cautotest/afe/#tab_id=view_job&object_id=222880716. Not limited to our tests, I see it randomly happening to more general tests as well. Ideas why?

<sigh> SSP, of course.

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

commit 078a3d00f3d112b8c21abd9136028e5a3ffe0dbb
Author: Richard Barnette <jrbarnette@chromium.org>
Date: Fri Aug 03 14:38:35 2018

Revert "Reland "[autotest] Delete Android from stable version RPC.""

This reverts commit 561c2f5049974f5a2cc0048e8283efb2f1d734a2.

Reason for revert:
Clients in SSP containers built from sources that pre-date the
RPC client changes will still make the old call with the
`android` parameter.

Original change's description:
> Reland "[autotest] Delete Android from stable version RPC."
> 
> This reverts commit 5fc1314f94de8f3b6e9bdfc724052e6466e97ffe.
> 
> Reason for revert:  All clients have now been upgraded to use
> the new RPC interface.  No extant client should still be using
> the `android` flag.  So, it should now be safe to reland this.
> 
> Original change's description:
> > Revert "[autotest] Delete Android from stable version RPC."
> >
> > This reverts commit 8725202f94b5b0e386d0970d53d527893656fe27.
> >
> > Reason for revert: This made an RPC change that was not backwards
> > compatible, breaking callers from before CL:1128452
> >
> > Original change's description:
> > > [autotest] Delete Android from stable version RPC.
> > >
> > > This deletes support for getting Android versions via the
> > > get_stable_version() RPC.
> > >
> > > BUG= chromium:834335 
> > > TEST=Run the stable_version command on a local instance
> > >
> > > Change-Id: I87aadb6330fc631e3e92c21bd39ddd1c0e9b0f60
> > > Reviewed-on: https://chromium-review.googlesource.com/1128453
> > > Commit-Ready: Richard Barnette <jrbarnette@chromium.org>
> > > Tested-by: Richard Barnette <jrbarnette@chromium.org>
> > > Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org>
> >
> > BUG= chromium:864165 
> >
> > Change-Id: I97d093bf548ebf776804c19c15b416e6b311d8e6
> > Reviewed-on: https://chromium-review.googlesource.com/1138913
> > Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org>
> > Tested-by: Prathmesh Prabhu <pprabhu@chromium.org>
> 
> Bug:  chromium:864165 
> Change-Id: Iba41cbad43f2af4e2ccfdae71875b064e5b9f0a0
> Reviewed-on: https://chromium-review.googlesource.com/1150460
> Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
> Tested-by: Richard Barnette <jrbarnette@chromium.org>
> Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org>

Bug:  chromium:864165 
Change-Id: I3e8cc7d1153c72e6db414a3c41742caecfaf5d42
Reviewed-on: https://chromium-review.googlesource.com/1162241
Commit-Queue: Richard Barnette <jrbarnette@chromium.org>
Tested-by: Richard Barnette <jrbarnette@chromium.org>
Reviewed-by: Richard Barnette <jrbarnette@google.com>

[modify] https://crrev.com/078a3d00f3d112b8c21abd9136028e5a3ffe0dbb/site_utils/stable_version_utils.py
[modify] https://crrev.com/078a3d00f3d112b8c21abd9136028e5a3ffe0dbb/frontend/afe/rpc_interface.py

> We see this error in many (but not all) tests on M68 since
> R68-10718.71.0. Example job:
> http://cautotest/afe/#tab_id=view_job&object_id=222880716.
> Not limited to our tests, I see it randomly happening to more
> general tests as well. Ideas why?

Bug 870760 explains why this happened to the enterprise_CFM_Perf
test.  Ultimately, that looks like an error in the test.  A spot
check suggests that lots of tests make this mistake.

Here's the complete mechanism that allowed enterprise_CFM_Perf and
others to encounter this the second time.
  * Server-side packaging means that server side tests on branches
    (like the R68 branch) use the branch's version of the test.
  * The R68 tests necessarily depend on the R68 version of other
    Autotest code.
  * The enterprise_CFM_Perf test has a dependency on servo creation
    code (see bug 870760 for how and why).
  * The servo creation code includes code to check whether the servo
    host needs to be updated; that check has client side calls to the
    get_stable_version RPC.
  * The RPC client code for R68 still passes the `android` parameter.

Checking the code, about two dozen tests appear to have the same bogus
dependency on servo as enterprise_CFM_Perf.  However, that mostly doesn't
matter to this bug:
  * Bogus or not, fixing those tests in the branch probably isn't the
    best idea.
  * Some tests (notably FAFT) have legitimate dependencies on servo.

So, the final solution would appear to be to leave the `android` parameter
in the RPC interface, and simply require that the parameter always be false.

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

commit e4f93999a3d544c6448dfd1b31f74bb94b02c16a
Author: Richard Barnette <jrbarnette@chromium.org>
Date: Tue Aug 07 08:50:47 2018

[autotest] Delete Android from stable version RPC.

This deletes server side support for getting Android versions via
the get_stable_version() RPC.

The `android` parameter remains part of the external RPC interface,
in order to support old clients on branches, but the parameter can
no longer be used.

BUG= chromium:834335 ,  chromium:864165 
TEST=None

Change-Id: I45a19e62245963d2a7b7bbe40ac3aa643b68b963
Reviewed-on: https://chromium-review.googlesource.com/1163063
Commit-Ready: Richard Barnette <jrbarnette@chromium.org>
Tested-by: Richard Barnette <jrbarnette@chromium.org>
Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org>

[modify] https://crrev.com/e4f93999a3d544c6448dfd1b31f74bb94b02c16a/site_utils/stable_version_utils.py
[modify] https://crrev.com/e4f93999a3d544c6448dfd1b31f74bb94b02c16a/frontend/afe/rpc_interface.py

There's a TODO in the code that says that the parameter can be
deleted at a future date (roughly, when R69 hits stable).
But, for our purposes, _this_ bug is fixed.