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

andrewmains12 · 2022-08-29T17:41:12Z

tl;dr:

etcd server dependencies are problematic
They're brought in mostly for tests (etcd/v3/integration, etcd/v3/server/embed)
We can and should replace them with etcd running in a docker container, and only depend on etcd client packages.

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 #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:

Develop a docker based drop in replacement for the etcd/v3/integration package (e.g. m3/docker/etcdintegration)
Replace all usages of the current etcd integration package with m3/docker/etcdintegration
Replace all test usages of the embed package with a docker based solution (same underlying code as m3/docker/etcdintegration)
Deprecate running etcd as part of a dbnode. Provide easy means for customers to migrate to a docker based quickstart instead.

The text was updated successfully, but these errors were encountered:

andrewmains12 · 2022-08-29T17:41:35Z

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

andrewmains12 · 2022-08-29T17:47:03Z

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 · 2022-08-29T18:42:42Z

cc also @justinjc @SokolAndrey

PR 6 for #4144 Now that all test usages of the embed package had been removed, this PR removes the usage of the embed package from the m3db server itself. N.B.: this PR currently makes no attempt to fix up any "quick start" flows relying on embedded etcd; that will come later. commit-id:2870a75c

#4147) * etcd_docker 2: Add a docker based etcdintegration package PR 2 for #4144 High level approach is as described in #4144 . This PR adds: - Functions to spin up a 1 node etcd cluster using docker (in `dockerexternal`) - A drop in replacement for the etcd/integration package using `dockerexternal` commit-id:e4e80f1d * etcd_docker 3: Incorporate docker based etcd integration package into unittests. PR 3 for #4144 High level approach is as described in #4144 . This PR incorporates the new test package into our unittests. Usage is via the `etcdintegration` package, which makes it transparent to the test code; it simply gets an etcd server started via different means. One piece of weirdness to call out here: the package currently relies on autosync being *disabled* on the client side. This is because the advertise client URL (aka what etcd tells clients to connect to) isn't correct for the open port on the host. That is we have: - etcd: listen on container port 0.0.0.0:2379, advertise 0.0.0.0:2379 - docker: expose etcd port 2379 to 0.0.0.0:0 on host machine (random free port) - client: connect to etcd via host machine. We could probably make this better. commit-id:263fed13

PR 6 for #4144 Now that all test usages of the embed package had been removed, this PR removes the usage of the embed package from the m3db server itself. N.B.: this PR currently makes no attempt to fix up any "quick start" flows relying on embedded etcd; that will come later. commit-id:2870a75c

#4148) PR 4 for #4144 High level approach is as described in #4144 . This PR integrates docker based etcd into our Go integration tests. It removes the need to have the embed package running in m3db for them, but doesn't yet touch that functionality. commit-id:3ae12ffd

PR 6 for #4144 Now that all test usages of the embed package had been removed, this PR removes the usage of the embed package from the m3db server itself. N.B.: this PR currently makes no attempt to fix up any "quick start" flows relying on embedded etcd; that will come later. commit-id:2870a75c

* etcd_docker 5: Incorporate docker based etcd approach into docker integration tests. PR 5 for #4144 This PR makes the docker integration tests use containerized etcd. Previously, these relied on M3DB running an embbeded etcd server. There's no inherent need for this, and it opens us up to dependency issues as described in the linked github issue. Note: there are a handful that require multiple servers; these are currently "skipped" (commented). I intend to bring those back at a later date.. commit-id:e67a5172 * etcd_docker 6: Remove etcd embed from M3DB node PR 6 for #4144 Now that all test usages of the embed package had been removed, this PR removes the usage of the embed package from the m3db server itself. N.B.: this PR currently makes no attempt to fix up any "quick start" flows relying on embedded etcd; that will come later. commit-id:2870a75c

This changes our etcd version *down* to 3.5.4 from 3.6.0-alpha.0. In #4087, we made the choice to go to an alpha version of etcd in order to get around the thorny dependency issues described in that ticket. This was partially on the assumption that etcd would release a full 3.6.0 version in the months following. As of 9/2/2022, that hasn't happened--no progress seems to have been made on etcd 3.6.0. As described in #4144, we've since addressed the dependency issues via other means, removing our dependency on the `embed` and `integration` packages. This allows us to move back to an etcd stable version, 3.5.4. Since stable versions are generally preferable, I'm taking the opportunity to do so. commit-id:720abd40