commit 43c296f6d2ef2234168c4130374da2b7500c9372
author Vadim Shtayura <vadimsh@chromium.org> Fri Nov 01 20:11:20 2024
committer LUCI CQ <infra-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Nov 01 20:11:20 2024
tree 25f6379b8a997fae51fb5ed987103379d6b029ad
parent 2a5f92ed1087aa2f3b9fea253a98ecd88fb1f5db

[gcemeta] Return AssumeNonGCE hack, but reverse its value.

If InheritFromGCE is true => probe real GCE metadata.
If InheritFromGCE is false => do not probe real GCE metadata.

There are two conflicting concerns.

The first is related to testing code that behaves differently
in the GCE environment. Let's call it the "testing concern".

metadata.OnGCE() caches its value the first time it is called.
There are third party libraries we are importing that check
value of metadata.OnGCE() and do something differently based
on it.

In particular, the Cloud Telemetry has a failure mode where,
with a particular state of dependencies in go.mod (that can
and did happen accidentally via "go mod tidy"), it compiles,
but panics at runtime, but **only on GCE** (see
http://b/356517179#comment16). This caused an outage.

To have a test for this, we need to make sure to run tests
such that metadata.OnGCE() returns true and there's an emulated
metadata server running.

The second concern is that gcemeta.Server itself needs to know
if it runs on **real** GCE to get a **real** instance zone. Thus
it needs to call metadata.OnGCE() at some point. Let's call this
the "real zone concern".

There are 2 approaches:
1) Call metadata.OnGCE() before starting gcemeta.Server. Since
   the emulation is not enabled yet, this solves the "real zone
   concern". But it doesn't address the "testing concern", since
   when running tests not on GCE, metadata.OnGCE will forever be
   cached as "false" and the code under test will just ignore the
   emulated metadata server.
2) Call metadata.OnGCE() as late as possible in ".../zone"
   metadata handler. This solves the "testing concern", but it
   doesn't really solve the "real zone concern". It work only if
   the process that started gcemeta.Server doesn't install it
   into its own process environment, because if it does,
   metadata.OnGCE() will always return true, even when not on
   GCE, because it will be discovering the emulated server itself.

Initially the code used approach 1) and a boolean field
AssumeNonGCE to disable probing of GCE. This was deemed as
a hack since using AssumeNonGCE required understanding all
this subtleties.

The code was changed to use approach 2) to get rid of AssumeNonGCE.
Issues with it were discovered. In particular `luciexe` installs
the emulated metadata server into its own environment, which results
in real zone detection not working. Changing `luciexe` to not do that
looks non-trivial (process environ modification is a part of its public
API). And generally this is as brittle and confusing as AssumeNonGCE.

This CL switches back to use approach 1) except now with
InheritFromGCE field which should be passed explicitly to
**enable** GCE probing. That way everything works more or
less as expected when writing tests. There's only one
non-test usage of gcemeta.Server, InheritFromGCE is enabled
there. Hopefully, this will be less brittle than two previous
approaches.

R=iannucci@chromium.org
CC=​ukai@google.com
BUG=b/376372151

Change-Id: I0b2501d41110ee05e82fe7f7d536ed2dc80d1a70
Reviewed-on: https://chromium-review.googlesource.com/c/infra/luci/luci-go/+/5980912
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>