xds: unify xDS client creation APIs meant for testing

[easwars]

Summary of changes:

• API changes
  • We are now down to three APIs for creating an xDS client
    • New(): almost all production code should use this
    • NewWithConfig(): Only the google-c2p resolver uses this since it has to manually construct the bootstrap configuration
    • NewForTesting(): All tests should use this and it accepts a config struct
      • This places all test xDS clients in a map keyed by its bootstrap configuration, and reuses clients if one exists for the same configuration
• Fix a minor data race in the authority type by grabbing the lock early when handling watch timer expiry
• Bootstrap changes
  • Always normalize contents when parsing bootstrap configuration to create the internal representation. This makes it easier to use the normalized configuration as the map key for xDS clients (used for testing purposes).
  • Remove any usages of the bootstrap.Config struct being manually initialized instead of being created using one of the NewXxx functions.
• Testing changes
  • Cleanup the mess around the functions used to trigger resource-not-found errors for testing
  • Move some tests out of test/xds which were using the above functionality into a test only package in xds
• Update all callsites of deleted APIs to use the new API

#a71-xds-fallback

RELEASE NOTES: none

[dfawley]

Looks good overall, just a couple small things.

[dfawley]

Comments should generally say "why" not "what" -- so why are we doing this step now?

[easwars]

Added a comment.

Earlier, we were doing this for some of the code paths involving testing xDS clients, but not in others. Doing this here would ensure that this is done for all testing xDS clients. And for non-testing xDS clients, this really does not matter.

[dfawley]

This is a bit unfortunate...if we're going to have tests for xds living outside of xds/ then is that another reason to move all of xds/internal into internal/xds? Because we could just export this function directly instead of doing all the dance to trampoline through internal and losing static type safety.

We even still could expose it for use from inside xds/...

[easwars]

We have some tests in test/xds which is probably why we need this. Also, this is not something new that I added in this PR. Instead there were two vars in the internal package that I reduced to one. But I agree with your sentiment and I've filed #7290 to ensure that this gets done. I will get to this once I'm done with the changes that are currently in flight.

[dfawley]

Shouldn't this embed the Unimplemented server?

Also, it might be nice to (*always) use the stub server instead, since that allows you to define the RPC behavior inside the test case. Otherwise your test logic ends up decoupled.

[easwars]

Switched to the unimplemented server.

With regards to using the stub server, this is also an existing test that I moved to a different place. But I looked to see if I could use the stub server. The problem is that the stub server creates a new grpc.Server and registers the test service on it. We need to change it to optionally accept a server (a vanilla grpc.Server or xds.GRPCServer) and register the test service on it, if provided. Filed #7291 to track this.

[arvindbr8]

I think this mostly looks good to me. Had a few comments. PTAL

[arvindbr8]

LGTM, Approved! :)