etcd_docker 2: Add a docker based etcdintegration package

docker-compose.yml

@@ -8,6 +8,17 @@ services:
     volumes:
       - .:/go/src/github.com/m3db/m3
       - /usr/bin/buildkite-agent:/usr/bin/buildkite-agent
+      # Support running docker within docker. That is, buildkite jobs themselves run in a container; that container
+      # needs to be able to spin up functioning docker containers.
+      - /var/run/docker.sock:/var/run/docker.sock
+    extra_hosts:
+      # Allow routing from the buildkite container to the host machine, as host.docker.internal. This allows us to do
+      # the following:
+      #  - Spin up an etcd container with ports published to the host machine
+      #  - Connect to the etcd container from the buildkite test process using host.docker.internal
+      # See
+      # https://medium.com/@TimvanBaarsen/how-to-connect-to-the-docker-host-from-inside-a-docker-container-112b4c71bc66
+      - "host.docker.internal:host-gateway"
     environment:
       - CI
       - BUILDKITE

src/aggregator/integration/one_client_multi_type_untimed_test.go

@@ -1,4 +1,4 @@
-// +build integration
+//go:build integration
 
 // Copyright (c) 2016 Uber Technologies, Inc.
 //

src/cluster/client/etcd/client.go

@@ -339,8 +339,14 @@ func newConfigFromCluster(rnd randInt63N, cluster Cluster) (clientv3.Config, err
 	if err != nil {
 		return clientv3.Config{}, err
 	}
+
+	// Support disabling autosync if a user very explicitly requests it (via negative duration).
+	autoSyncInterval := cluster.AutoSyncInterval()
+	if autoSyncInterval < 0 {
+		autoSyncInterval = 0
+	}
 	cfg := clientv3.Config{
-		AutoSyncInterval:   cluster.AutoSyncInterval(),
+		AutoSyncInterval:   autoSyncInterval,
 		DialTimeout:        cluster.DialTimeout(),
 		DialOptions:        cluster.DialOptions(),
 		Endpoints:          cluster.Endpoints(),

src/cluster/client/etcd/client_test.go

@@ -25,18 +25,23 @@ import (
 	"testing"
 	"time"
 
+	"github.com/m3db/m3/src/cluster/kv"
+	"github.com/m3db/m3/src/cluster/services"
+	integration "github.com/m3db/m3/src/integration/resources/docker/dockerexternal/etcdintegration"
+	"github.com/m3db/m3/src/x/retry"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	clientv3 "go.etcd.io/etcd/client/v3"
-	"go.etcd.io/etcd/tests/v3/framework/integration"
 	"google.golang.org/grpc"
-
-	"github.com/m3db/m3/src/cluster/kv"
-	"github.com/m3db/m3/src/cluster/services"
 )
 
 func TestETCDClientGen(t *testing.T) {
-	cs, err := NewConfigServiceClient(testOptions())
+	cs, err := NewConfigServiceClient(
+		testOptions().
+			// These are error cases; don't retry for no reason.
+			SetRetryOptions(retry.NewOptions().SetMaxRetries(0)),
+	)
 	require.NoError(t, err)
 
 	c := cs.(*csclient)
@@ -414,6 +419,15 @@ func Test_newConfigFromCluster(t *testing.T) {
 		)
 	})
 
+	t.Run("negative autosync on M3 disables autosync for etcd", func(t *testing.T) {
+		inputCfg := newFullConfig()
+		inputCfg.AutoSyncInterval = -1
+		etcdCfg, err := newConfigFromCluster(testRnd, inputCfg.NewCluster())
+		require.NoError(t, err)
+
+		assert.Equal(t, time.Duration(0), etcdCfg.AutoSyncInterval)
+	})
+
 	// Separate test just because the assert.Equal won't work for functions.
 	t.Run("passes through dial options", func(t *testing.T) {
 		clusterCfg := newFullConfig()

src/cluster/client/etcd/config.go

@@ -35,12 +35,22 @@ import (
 
 // ClusterConfig is the config for a zoned etcd cluster.
 type ClusterConfig struct {
-	Zone             string           `yaml:"zone"`
-	Endpoints        []string         `yaml:"endpoints"`
-	KeepAlive        *KeepAliveConfig `yaml:"keepAlive"`
-	TLS              *TLSConfig       `yaml:"tls"`
-	AutoSyncInterval time.Duration    `yaml:"autoSyncInterval"`
-	DialTimeout      time.Duration    `yaml:"dialTimeout"`
+	Zone      string           `yaml:"zone"`
+	Endpoints []string         `yaml:"endpoints"`
+	KeepAlive *KeepAliveConfig `yaml:"keepAlive"`
+	TLS       *TLSConfig       `yaml:"tls"`
+	// AutoSyncInterval configures the etcd client's AutoSyncInterval
+	// (go.etcd.io/etcd/client/v3@v3.6.0-alpha.0/config.go:32).
+	// By default, it is 1m.
+	//
+	// Advanced:
+	//
+	// One important difference from the etcd config: we have autosync *on* by default (unlike etcd), meaning that
+	// the zero value here doesn't indicate autosync off.
+	// Instead, users should pass in a negative value to indicate "disable autosync"
+	// Only do this if you truly have a good reason for it! Most production use cases want autosync on.
+	AutoSyncInterval time.Duration `yaml:"autoSyncInterval"`
+	DialTimeout      time.Duration `yaml:"dialTimeout"`
 
 	DialOptions []grpc.DialOption `yaml:"-"` // nonserializable
 }
@@ -59,7 +69,10 @@ func (c ClusterConfig) NewCluster() Cluster {
 		SetKeepAliveOptions(keepAliveOpts).
 		SetTLSOptions(c.TLS.newOptions())
 
-	if c.AutoSyncInterval > 0 {
+	// Autosync should *always* be on, unless the user very explicitly requests it to be off. They can do this via a
+	// negative value (in which case we can assume they know what they're doing).
+	// Therefore, only update if it's nonzero, on the assumption that zero is just the empty value.
+	if c.AutoSyncInterval != 0 {
 		cluster = cluster.SetAutoSyncInterval(c.AutoSyncInterval)
 	}
 

src/cluster/client/etcd/config_test.go

@@ -181,3 +181,10 @@ func TestDefaultConfig(t *testing.T) {
 	require.Equal(t, defaultDialTimeout, cluster.DialTimeout())
 	require.Equal(t, defaultAutoSyncInterval, cluster.AutoSyncInterval())
 }
+
+func TestConfig_negativeAutosync(t *testing.T) {
+	cluster := ClusterConfig{
+		AutoSyncInterval: -5,
+	}.NewCluster()
+	require.Equal(t, time.Duration(-5), cluster.AutoSyncInterval())
+}

src/cluster/client/etcd/types.go

@@ -159,6 +159,11 @@ type Cluster interface {
 	SetTLSOptions(TLSOptions) Cluster
 
 	AutoSyncInterval() time.Duration
+
+	// SetAutoSyncInterval sets the etcd client to autosync cluster endpoints periodically. This defaults to
+	// 1 minute (defaultAutoSyncInterval). If negative or zero, it will disable autosync. This differs slightly
+	// from the underlying etcd configuration its setting, which only supports 0 for disabling. We do this because
+	// there's otherwise no good way to specify "disable" in our configs (which default to SetAutoSyncInterval(1m)).
 	SetAutoSyncInterval(value time.Duration) Cluster
 
 	DialTimeout() time.Duration