m3db / m3

M3 monorepo - Distributed TSDB, Aggregator and Query Engine, Prometheus Sidecar, Graphite Compatible, Metrics Platform
https://m3db.io/
Apache License 2.0
4.76k stars 453 forks source link

Migrate M3 usages of etcd/v3/integration, etcd/v3/server/embed to docker #4144

Open andrewmains12 opened 2 years ago

andrewmains12 commented 2 years ago

tl;dr:

Problem

M3 currently uses the etcd/v3/integration and etcd/v3/server/embed packages heavily in tests.

This poses problems for M3's use as a library; it introduces dependencies on etcd server side packages, which can conflict with M3 versions of those same packages. Most recently, we hit this particular issue with grpc-go and otel (see https://github.com/m3db/m3/issues/3725):

    // etcd is currently on an alpha version to accomodate a GRPC version upgrade. See
    // https://github.com/m3db/m3/issues/4090 for the followup task to move back to a stable version.
    //  Gory details (why we're doing this):
    //
    //    - We import etcd/server/v3 via etcd/embed and etcd/testing/v3/frameworks/integration.
    //    - etcd/server/v3 in 3.5.2 depends on pre 1.0 opentelemetry. Bleeding edge etcd depends on 1.0 opentelemetry
    //    - M3 depends on 1.0 opentelemetry — this conflicts with etcd 3.5.2, but not bleeding edge etcd

Furthermore, etcd's documentation suggests that the integration package is somewhat misused in tests today, and may eventually go away (or be replaced)

BeforeTestExternal initializes test context and is targeted for external APIs. In general the integration package is not targeted to be used outside of etcd project, but till the dedicated package is developed, this is the best entry point so far (without backward compatibility promise).

Proposal

We can avoid these dependency problems entirely by limiting our usage of etcd to its client dependencies.

This means:

andrewmains12 commented 2 years ago

cc @zmt I'm tracking the work overall here for public consumption.

andrewmains12 commented 2 years ago

Thoughts on this @nbroyles ? cc'ing you since I know you did a lot of work on the Go based integration tests; lmk if there's anyone else relevant to loop in.

andrewmains12 commented 2 years ago

cc also @justinjc @SokolAndrey

andrewmains12 commented 2 years ago

Hey @schallert any thoughts on this, and/or cc anyone who might have knowledge/concerns?

I did a bunch of PR's against an Uber specific branch to unblock things on our side; I think they should be workable against master as well. If the overall approach seems reasonable, I'll get those going.