Skip to content

Commit

Permalink
etcd_docker 2: Add a docker based etcdintegration package
Browse files Browse the repository at this point in the history
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
  • Loading branch information
andrewmains12 committed Aug 30, 2022
1 parent bdd86b8 commit 866cd06
Show file tree
Hide file tree
Showing 17 changed files with 1,264 additions and 16 deletions.
11 changes: 11 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// +build integration
//go:build integration

// Copyright (c) 2016 Uber Technologies, Inc.
//
Expand Down
8 changes: 7 additions & 1 deletion src/cluster/client/etcd/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
24 changes: 19 additions & 5 deletions src/cluster/client/etcd/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down
27 changes: 20 additions & 7 deletions src/cluster/client/etcd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)
}

Expand Down
7 changes: 7 additions & 0 deletions src/cluster/client/etcd/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
5 changes: 5 additions & 0 deletions src/cluster/client/etcd/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 866cd06

Please sign in to comment.