Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove deprecated host-discovery and overlay networks with external k/v #42247

Merged
merged 15 commits into from
Jan 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 8 additions & 11 deletions cmd/dockerd/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}
Expand Down Expand Up @@ -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) {
Expand Down
9 changes: 1 addition & 8 deletions cmd/dockerd/daemon_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"]))
}
Expand Down
35 changes: 0 additions & 35 deletions daemon/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
3 changes: 0 additions & 3 deletions daemon/config/config_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
79 changes: 0 additions & 79 deletions daemon/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down
86 changes: 0 additions & 86 deletions daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"path"
"path/filepath"
"runtime"
"strings"
"sync"
"time"

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand All @@ -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))

Expand Down
1 change: 0 additions & 1 deletion daemon/daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down