New issue
Advanced search Search tips

Issue 917474 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 915406



Sign in to add a comment

luci-py proto comparison failure only in recipe run

Project Member Reported by mar...@chromium.org, Dec 21

Issue description

We're in a weird state where appengine/swarming/handlers_prpc_test.py succeeds on macOS, google linux and when run locally on a bot, but fails in the recipe run by the CQ.

I've seen that reverting the last 3 CLS resolves the problem, but I don't know why;
https://chromium-review.googlesource.com/c/1388106
https://chromium-review.googlesource.com/c/1388104
https://chromium-review.googlesource.com/c/1385589

There's a backlog of CL to commit so I'll revert these 3 for now, but we need to investigate and figure out why this happened.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 21

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/8ad11a5bb7cc3cac9ff81fb9549904403181bfc7

commit 8ad11a5bb7cc3cac9ff81fb9549904403181bfc7
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Fri Dec 21 20:42:36 2018

Revert 3 CLs due to problem with proto message comparison

Revert "Fix compile_proto.py so it works as makefiles expect it to."
This reverts commit d3264d9e2d51595de64cb1f81880b631ae83f2dd.
https://chromium-review.googlesource.com/c/1385589

Revert "Recompile protobufs."
This reverts commit 0366e852518eee045ad5e1cc34aea831b0b030a8.
https://chromium-review.googlesource.com/c/1388104

Revert "Return file path in config validation response."
This reverts commit 20c0045691357d51e2047b8f5508fe804fc14776.
https://chromium-review.googlesource.com/c/1388106

TBR=vadimsh@chromium.org
Bug:  917474 
Change-Id: I4f672c596ae703677812f230c86c78eb24235e56
Reviewed-on: https://chromium-review.googlesource.com/c/1387970
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/8ad11a5bb7cc3cac9ff81fb9549904403181bfc7/appengine/components/components/config/endpoint.py
[modify] https://crrev.com/8ad11a5bb7cc3cac9ff81fb9549904403181bfc7/appengine/components/components/config/endpoint_test.py
[modify] https://crrev.com/8ad11a5bb7cc3cac9ff81fb9549904403181bfc7/appengine/components/components/config/proto/Makefile
[modify] https://crrev.com/8ad11a5bb7cc3cac9ff81fb9549904403181bfc7/appengine/components/components/config/proto/project_config_pb2.py
[modify] https://crrev.com/8ad11a5bb7cc3cac9ff81fb9549904403181bfc7/appengine/components/components/config/proto/service_config.proto
[modify] https://crrev.com/8ad11a5bb7cc3cac9ff81fb9549904403181bfc7/appengine/components/components/config/proto/service_config_pb2.py
[modify] https://crrev.com/8ad11a5bb7cc3cac9ff81fb9549904403181bfc7/appengine/components/tools/compile_proto.py
[modify] https://crrev.com/8ad11a5bb7cc3cac9ff81fb9549904403181bfc7/appengine/config_service/api.py
[modify] https://crrev.com/8ad11a5bb7cc3cac9ff81fb9549904403181bfc7/appengine/config_service/api_test.py
[modify] https://crrev.com/8ad11a5bb7cc3cac9ff81fb9549904403181bfc7/appengine/config_service/storage.py
[modify] https://crrev.com/8ad11a5bb7cc3cac9ff81fb9549904403181bfc7/appengine/swarming/handlers_prpc_test.py

Do you have a link to a failure?
Huh... Are you absolutely sure it's not just a flake? Reverted CLs look completely irrelevant to handlers_prpc_test.py...
I know. I spent the day trying to reproduce this. This did unblock the other CLs.
Blocking: 915406
Cc: bpastene@chromium.org
Nothing makes sense;
- The changes I do to Swarming protos all pass fine without any problem.

- This change always fails; https://chromium-review.googlesource.com/c/1387805
To not take any chance, I've re-created the CL fresh, with a new local branch and it still fails the same way: https://chromium-review.googlesource.com/c/1389195

This is annoying because this blocks the fix for issue 915406 but I really don't understand what's happening here.
This doesn't make any sense, this CL passes the CQ;
https://chromium-review.googlesource.com/c/infra/luci/luci-py/+/1389196

I don't understand why the swarming bot one doesn't. Maybe the presubmit checks in swarming_bot mutates the presubmit check sys.path state?
I can reproduce the failure locally with a virtual env on Linux.
Here's the diff (seen by modifying the test to set self.maxDiff = None):
-       version: "bd5fadf7768b3f4965cf1fe8b58c6c4018d1039e40eb253215dac1193372ffeb"
+       version: "8a90698034b4b49f82ec83c82937464937be542c3b435977ead9d7ad35f4ff71"
recipe_engine is using protobuf 3.6.0, as specified in https://chromium.googlesource.com/infra/luci/recipes-py/+/master/.vpython

This only applies if you are importing the _pb2 files in the recipe_engine process. Otherwise you'll get whatever version of protobuf is specified in the .vpython environment above your test script.

Nothing on the bots uses the `-vpython-spec` option.

The version of the proto compiler you use should probably match the version of the protobuf library that you use in your scripts.

If these are in appengine land, they probably are using the dev_appserver copy of protobuf which I have no idea which version that is.

You can see where protobuf comes from (and version) by

  import google.protobuf
  print google.protobuf, google.protobuf.__version__
Cc: garymm@google.com
Owner: mar...@chromium.org
I relanded all 3 of my changes, making sure to trigger swarming/handlers_prpc_test.py each time in CQ, so I don't know what else I can do about this.
Assigning back to maruel.
Let me know if there's something I can do to help though.
Sorry for the trouble, I found the bug. Indeed not related at all.
Please do share. I'm curious.
Project Member

Comment 13 by bugdroid1@chromium.org, Jan 10

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/10138a3917660926a866a972e2a964e3359a48bf

commit 10138a3917660926a866a972e2a964e3359a48bf
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Thu Jan 10 19:05:31 2019

swarming: fix flaky test

Use the calculated bot version.

Bug:  917474 
Change-Id: I3cb4f4a4794a29516d93a17f09c54858ebdb986f
Reviewed-on: https://chromium-review.googlesource.com/c/1403941
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/10138a3917660926a866a972e2a964e3359a48bf/appengine/swarming/handlers_prpc_test.py

Status: Fixed (was: Assigned)
The CL above fixed the problem, which was a incorrect expectation.

I have no clue why reverting your CL had any effect on that. Please accept my apology for the disruption.

Sign in to add a comment