Merge pull request #42247 from thaJeztah/remove_discovery

Remove deprecated host-discovery and overlay networks with external k/v
moby · Jan 6, 2022 · 0f1d65b · 0f1d65b
2 parents b0806bd + 7b692a4
commit 0f1d65b
Show file tree

Hide file tree

Showing 194 changed files with 46 additions and 26,296 deletions.
diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go
@@ -80,6 +80,9 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
 	if cli.Config, err = loadDaemonCliConfig(opts); err != nil {
 		return err
 	}
+	if err := checkDeprecatedOptions(cli.Config); err != nil {
+		return err
+	}
 
 	if opts.Validate {
 		// If config wasn't OK we wouldn't have made it this far.
@@ -89,8 +92,6 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
 
 	configureProxyEnv(cli.Config)
 
-	warnOnDeprecatedConfigOptions(cli.Config)
-
 	if err := configureDaemonLogs(cli.Config); err != nil {
 		return err
 	}
@@ -465,16 +466,12 @@ func loadDaemonCliConfig(opts *daemonOptions) (*config.Config, error) {
 	return conf, nil
 }
 
-func warnOnDeprecatedConfigOptions(config *config.Config) {
-	if config.ClusterAdvertise != "" {
-		logrus.Warn(`The "cluster-advertise" option is deprecated. To be removed soon.`)
-	}
-	if config.ClusterStore != "" {
-		logrus.Warn(`The "cluster-store" option is deprecated. To be removed soon.`)
-	}
-	if len(config.ClusterOpts) > 0 {
-		logrus.Warn(`The "cluster-store-opt" option is deprecated. To be removed soon.`)
+func checkDeprecatedOptions(config *config.Config) error {
+	// Overlay networks with external k/v stores have been deprecated
+	if config.ClusterAdvertise != "" || len(config.ClusterOpts) > 0 || config.ClusterStore != "" {
+		return errors.New("Host-discovery and overlay networks with external k/v stores are deprecated. The 'cluster-advertise', 'cluster-store', and 'cluster-store-opt' options have been removed")
 	}
+	return nil
 }
 
 func initRouter(opts routerOptions) {

diff --git a/cmd/dockerd/daemon_unix_test.go b/cmd/dockerd/daemon_unix_test.go
@@ -48,21 +48,14 @@ func TestLoadDaemonConfigWithNetwork(t *testing.T) {
 }
 
 func TestLoadDaemonConfigWithMapOptions(t *testing.T) {
-	content := `{
-		"cluster-store-opts": {"kv.cacertfile": "/var/lib/docker/discovery_certs/ca.pem"},
-		"log-opts": {"tag": "test"}
-}`
+	content := `{"log-opts": {"tag": "test"}}`
 	tempFile := fs.NewFile(t, "config", fs.WithContent(content))
 	defer tempFile.Remove()
 
 	opts := defaultOptions(t, tempFile.Path())
 	loadedConfig, err := loadDaemonCliConfig(opts)
 	assert.NilError(t, err)
 	assert.Assert(t, loadedConfig != nil)
-	assert.Check(t, loadedConfig.ClusterOpts != nil)
-
-	expectedPath := "/var/lib/docker/discovery_certs/ca.pem"
-	assert.Check(t, is.Equal(expectedPath, loadedConfig.ClusterOpts["kv.cacertfile"]))
 	assert.Check(t, loadedConfig.LogConfig.Config != nil)
 	assert.Check(t, is.Equal("test", loadedConfig.LogConfig.Config["tag"]))
 }

diff --git a/daemon/config/config.go b/daemon/config/config.go
@@ -7,14 +7,11 @@ import (
 	"net"
 	"net/url"
 	"os"
-	"reflect"
 	"strings"
 	"sync"
 
-	daemondiscovery "github.com/docker/docker/daemon/discovery"
 	"github.com/docker/docker/opts"
 	"github.com/docker/docker/pkg/authorization"
-	"github.com/docker/docker/pkg/discovery"
 	"github.com/docker/docker/registry"
 	"github.com/imdario/mergo"
 	"github.com/pkg/errors"
@@ -301,27 +298,10 @@ func New() *Config {
 			LogConfig: LogConfig{
 				Config: make(map[string]string),
 			},
-			ClusterOpts: make(map[string]string),
 		},
 	}
 }
 
-// ParseClusterAdvertiseSettings parses the specified advertise settings
-func ParseClusterAdvertiseSettings(clusterStore, clusterAdvertise string) (string, error) {
-	if clusterAdvertise == "" {
-		return "", daemondiscovery.ErrDiscoveryDisabled
-	}
-	if clusterStore == "" {
-		return "", errors.New("invalid cluster configuration. --cluster-advertise must be accompanied by --cluster-store configuration")
-	}
-
-	advertise, err := discovery.ParseAdvertise(clusterAdvertise)
-	if err != nil {
-		return "", errors.Wrap(err, "discovery advertise parsing failed")
-	}
-	return advertise, nil
-}
-
 // GetConflictFreeLabels validates Labels for conflict
 // In swarm the duplicates for labels are removed
 // so we only take same values here, no conflict values
@@ -636,21 +616,6 @@ func ValidateMaxDownloadAttempts(config *Config) error {
 	return nil
 }
 
-// ModifiedDiscoverySettings returns whether the discovery configuration has been modified or not.
-func ModifiedDiscoverySettings(config *Config, backendType, advertise string, clusterOpts map[string]string) bool {
-	if config.ClusterStore != backendType || config.ClusterAdvertise != advertise {
-		return true
-	}
-
-	if (config.ClusterOpts == nil && clusterOpts == nil) ||
-		(config.ClusterOpts == nil && len(clusterOpts) == 0) ||
-		(len(config.ClusterOpts) == 0 && clusterOpts == nil) {
-		return false
-	}
-
-	return !reflect.DeepEqual(config.ClusterOpts, clusterOpts)
-}
-
 // GetDefaultRuntimeName returns the current default runtime
 func (conf *Config) GetDefaultRuntimeName() string {
 	conf.Lock()

diff --git a/daemon/config/config_linux.go b/daemon/config/config_linux.go
@@ -120,9 +120,6 @@ func (conf *Config) GetResolvConf() string {
 
 // IsSwarmCompatible defines if swarm mode can be enabled in this config
 func (conf *Config) IsSwarmCompatible() error {
-	if conf.ClusterStore != "" || conf.ClusterAdvertise != "" {
-		return fmt.Errorf("--cluster-store and --cluster-advertise daemon configurations are incompatible with swarm mode")
-	}
 	if conf.LiveRestoreEnabled {
 		return fmt.Errorf("--live-restore daemon configuration is incompatible with swarm mode")
 	}

diff --git a/daemon/config/config_test.go b/daemon/config/config_test.go
@@ -5,7 +5,6 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/docker/docker/daemon/discovery"
 	"github.com/docker/docker/libnetwork/ipamutils"
 	"github.com/docker/docker/opts"
 	"github.com/spf13/pflag"
@@ -38,23 +37,6 @@ func TestDaemonBrokenConfiguration(t *testing.T) {
 	}
 }
 
-func TestParseClusterAdvertiseSettings(t *testing.T) {
-	_, err := ParseClusterAdvertiseSettings("something", "")
-	if err != discovery.ErrDiscoveryDisabled {
-		t.Fatalf("expected discovery disabled error, got %v\n", err)
-	}
-
-	_, err = ParseClusterAdvertiseSettings("", "something")
-	if err == nil {
-		t.Fatalf("expected discovery store error, got %v\n", err)
-	}
-
-	_, err = ParseClusterAdvertiseSettings("etcd", "127.0.0.1:8080")
-	if err != nil {
-		t.Fatal(err)
-	}
-}
-
 func TestFindConfigurationConflicts(t *testing.T) {
 	config := map[string]interface{}{"authorization-plugins": "foobar"}
 	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
@@ -447,67 +429,6 @@ func TestValidateConfiguration(t *testing.T) {
 	}
 }
 
-func TestModifiedDiscoverySettings(t *testing.T) {
-	cases := []struct {
-		current  *Config
-		modified *Config
-		expected bool
-	}{
-		{
-			current:  discoveryConfig("foo", "bar", map[string]string{}),
-			modified: discoveryConfig("foo", "bar", map[string]string{}),
-			expected: false,
-		},
-		{
-			current:  discoveryConfig("foo", "bar", map[string]string{"foo": "bar"}),
-			modified: discoveryConfig("foo", "bar", map[string]string{"foo": "bar"}),
-			expected: false,
-		},
-		{
-			current:  discoveryConfig("foo", "bar", map[string]string{}),
-			modified: discoveryConfig("foo", "bar", nil),
-			expected: false,
-		},
-		{
-			current:  discoveryConfig("foo", "bar", nil),
-			modified: discoveryConfig("foo", "bar", map[string]string{}),
-			expected: false,
-		},
-		{
-			current:  discoveryConfig("foo", "bar", nil),
-			modified: discoveryConfig("baz", "bar", nil),
-			expected: true,
-		},
-		{
-			current:  discoveryConfig("foo", "bar", nil),
-			modified: discoveryConfig("foo", "baz", nil),
-			expected: true,
-		},
-		{
-			current:  discoveryConfig("foo", "bar", nil),
-			modified: discoveryConfig("foo", "bar", map[string]string{"foo": "bar"}),
-			expected: true,
-		},
-	}
-
-	for _, c := range cases {
-		got := ModifiedDiscoverySettings(c.current, c.modified.ClusterStore, c.modified.ClusterAdvertise, c.modified.ClusterOpts)
-		if c.expected != got {
-			t.Fatalf("expected %v, got %v: current config %v, new config %v", c.expected, got, c.current, c.modified)
-		}
-	}
-}
-
-func discoveryConfig(backendAddr, advertiseAddr string, opts map[string]string) *Config {
-	return &Config{
-		CommonConfig: CommonConfig{
-			ClusterStore:     backendAddr,
-			ClusterAdvertise: advertiseAddr,
-			ClusterOpts:      opts,
-		},
-	}
-}
-
 // TestReloadSetConfigFileNotExist tests that when `--config-file` is set
 // and it doesn't exist the `Reload` function returns an error.
 func TestReloadSetConfigFileNotExist(t *testing.T) {

diff --git a/daemon/daemon.go b/daemon/daemon.go
@@ -14,7 +14,6 @@ import (
 	"path"
 	"path/filepath"
 	"runtime"
-	"strings"
 	"sync"
 	"time"
 
@@ -29,7 +28,6 @@ import (
 	"github.com/docker/docker/builder"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/daemon/config"
-	"github.com/docker/docker/daemon/discovery"
 	"github.com/docker/docker/daemon/events"
 	"github.com/docker/docker/daemon/exec"
 	_ "github.com/docker/docker/daemon/graphdriver/register" // register graph drivers
@@ -93,7 +91,6 @@ type Daemon struct {
 	EventsService         *events.Events
 	netController         libnetwork.NetworkController
 	volumes               *volumesservice.VolumesService
-	discoveryWatcher      discovery.Reloader
 	root                  string
 	seccompEnabled        bool
 	apparmorEnabled       bool
@@ -523,8 +520,6 @@ func (daemon *Daemon) restore() error {
 				}
 			}
 
-			// Make sure networks are available before starting
-			daemon.waitForNetworks(c)
 			if err := daemon.containerStart(c, "", "", true); err != nil {
 				log.WithError(err).Error("failed to start container")
 			}
@@ -629,40 +624,6 @@ func (daemon *Daemon) RestartSwarmContainers() {
 	group.Wait()
 }
 
-// waitForNetworks is used during daemon initialization when starting up containers
-// It ensures that all of a container's networks are available before the daemon tries to start the container.
-// In practice it just makes sure the discovery service is available for containers which use a network that require discovery.
-func (daemon *Daemon) waitForNetworks(c *container.Container) {
-	if daemon.discoveryWatcher == nil {
-		return
-	}
-
-	// Make sure if the container has a network that requires discovery that the discovery service is available before starting
-	for netName := range c.NetworkSettings.Networks {
-		// If we get `ErrNoSuchNetwork` here, we can assume that it is due to discovery not being ready
-		// Most likely this is because the K/V store used for discovery is in a container and needs to be started
-		if _, err := daemon.netController.NetworkByName(netName); err != nil {
-			if _, ok := err.(libnetwork.ErrNoSuchNetwork); !ok {
-				continue
-			}
-
-			// use a longish timeout here due to some slowdowns in libnetwork if the k/v store is on anything other than --net=host
-			// FIXME: why is this slow???
-			dur := 60 * time.Second
-			timer := time.NewTimer(dur)
-
-			logrus.WithField("container", c.ID).Debugf("Container %s waiting for network to be ready", c.Name)
-			select {
-			case <-daemon.discoveryWatcher.ReadyCh():
-			case <-timer.C:
-			}
-			timer.Stop()
-
-			return
-		}
-	}
-}
-
 func (daemon *Daemon) children(c *container.Container) map[string]*container.Container {
 	return daemon.linkIndex.children(c)
 }
@@ -1050,12 +1011,6 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 		return nil, err
 	}
 
-	// Discovery is only enabled when the daemon is launched with an address to advertise.  When
-	// initialized, the daemon is registered and we can store the discovery backend as it's read-only
-	if err := d.initDiscovery(config); err != nil {
-		return nil, err
-	}
-
 	sysInfo := d.RawSysInfo()
 	for _, w := range sysInfo.Warnings {
 		logrus.Warn(w)
@@ -1390,26 +1345,6 @@ func (daemon *Daemon) IsShuttingDown() bool {
 	return daemon.shutdown
 }
 
-// initDiscovery initializes the discovery watcher for this daemon.
-func (daemon *Daemon) initDiscovery(conf *config.Config) error {
-	advertise, err := config.ParseClusterAdvertiseSettings(conf.ClusterStore, conf.ClusterAdvertise)
-	if err != nil {
-		if err == discovery.ErrDiscoveryDisabled {
-			return nil
-		}
-		return err
-	}
-
-	conf.ClusterAdvertise = advertise
-	discoveryWatcher, err := discovery.Init(conf.ClusterStore, conf.ClusterAdvertise, conf.ClusterOpts)
-	if err != nil {
-		return fmt.Errorf("discovery initialization failed (%v)", err)
-	}
-
-	daemon.discoveryWatcher = discoveryWatcher
-	return nil
-}
-
 func isBridgeNetworkDisabled(conf *config.Config) bool {
 	return conf.BridgeConfig.Iface == config.DisableNetworkBridge
 }
@@ -1428,27 +1363,6 @@ func (daemon *Daemon) networkOptions(dconfig *config.Config, pg plugingetter.Plu
 	dn := runconfig.DefaultDaemonNetworkMode().NetworkName()
 	options = append(options, nwconfig.OptionDefaultDriver(string(dd)))
 	options = append(options, nwconfig.OptionDefaultNetwork(dn))
-
-	if strings.TrimSpace(dconfig.ClusterStore) != "" {
-		kv := strings.Split(dconfig.ClusterStore, "://")
-		if len(kv) != 2 {
-			return nil, errors.New("kv store daemon config must be of the form KV-PROVIDER://KV-URL")
-		}
-		options = append(options, nwconfig.OptionKVProvider(kv[0]))
-		options = append(options, nwconfig.OptionKVProviderURL(kv[1]))
-	}
-	if len(dconfig.ClusterOpts) > 0 {
-		options = append(options, nwconfig.OptionKVOpts(dconfig.ClusterOpts))
-	}
-
-	if daemon.discoveryWatcher != nil {
-		options = append(options, nwconfig.OptionDiscoveryWatcher(daemon.discoveryWatcher))
-	}
-
-	if dconfig.ClusterAdvertise != "" {
-		options = append(options, nwconfig.OptionDiscoveryAddress(dconfig.ClusterAdvertise))
-	}
-
 	options = append(options, nwconfig.OptionLabels(dconfig.Labels))
 	options = append(options, driverOptions(dconfig))
 

diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go
@@ -10,7 +10,6 @@ import (
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/libnetwork"
-	_ "github.com/docker/docker/pkg/discovery/memory"
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/truncindex"
 	volumesservice "github.com/docker/docker/volume/service"