support insecure registry in configuration reload

Signed-off-by: allencloud <allen.sun@daocloud.io>

daemon/daemon.go

@@ -992,6 +992,7 @@ func (daemon *Daemon) initDiscovery(config *Config) error {
 // These are the settings that Reload changes:
 // - Daemon labels.
 // - Daemon debug log level.
+// - Daemon insecure registries.
 // - Daemon max concurrent downloads
 // - Daemon max concurrent uploads
 // - Cluster discovery (reconfigure and restart).
@@ -1023,6 +1024,12 @@ func (daemon *Daemon) Reload(config *Config) (err error) {
 	if config.IsValueSet("debug") {
 		daemon.configStore.Debug = config.Debug
 	}
+	if config.IsValueSet("insecure-registries") {
+		daemon.configStore.InsecureRegistries = config.InsecureRegistries
+		if err := daemon.RegistryService.LoadInsecureRegistries(config.InsecureRegistries); err != nil {
+			return err
+		}
+	}
 	if config.IsValueSet("live-restore") {
 		daemon.configStore.LiveRestoreEnabled = config.LiveRestoreEnabled
 		if err := daemon.containerdRemote.UpdateOptions(libcontainerd.WithLiveRestore(config.LiveRestoreEnabled)); err != nil {
@@ -1065,20 +1072,39 @@ func (daemon *Daemon) Reload(config *Config) (err error) {
 	// We emit daemon reload event here with updatable configurations
 	attributes["debug"] = fmt.Sprintf("%t", daemon.configStore.Debug)
 	attributes["live-restore"] = fmt.Sprintf("%t", daemon.configStore.LiveRestoreEnabled)
+
+	if daemon.configStore.InsecureRegistries != nil {
+		insecureRegistries, err := json.Marshal(daemon.configStore.InsecureRegistries)
+		if err != nil {
+			return err
+		}
+		attributes["insecure-registries"] = string(insecureRegistries)
+	} else {
+		attributes["insecure-registries"] = "[]"
+	}
+
 	attributes["cluster-store"] = daemon.configStore.ClusterStore
 	if daemon.configStore.ClusterOpts != nil {
-		opts, _ := json.Marshal(daemon.configStore.ClusterOpts)
+		opts, err := json.Marshal(daemon.configStore.ClusterOpts)
+		if err != nil {
+			return err
+		}
 		attributes["cluster-store-opts"] = string(opts)
 	} else {
 		attributes["cluster-store-opts"] = "{}"
 	}
 	attributes["cluster-advertise"] = daemon.configStore.ClusterAdvertise
+
 	if daemon.configStore.Labels != nil {
-		labels, _ := json.Marshal(daemon.configStore.Labels)
+		labels, err := json.Marshal(daemon.configStore.Labels)
+		if err != nil {
+			return err
+		}
 		attributes["labels"] = string(labels)
 	} else {
 		attributes["labels"] = "[]"
 	}
+
 	attributes["max-concurrent-downloads"] = fmt.Sprintf("%d", *daemon.configStore.MaxConcurrentDownloads)
 	attributes["max-concurrent-uploads"] = fmt.Sprintf("%d", *daemon.configStore.MaxConcurrentUploads)
 	attributes["shutdown-timeout"] = fmt.Sprintf("%d", daemon.configStore.ShutdownTimeout)

daemon/daemon_test.go

@@ -14,6 +14,7 @@ import (
 	_ "github.com/docker/docker/pkg/discovery/memory"
 	"github.com/docker/docker/pkg/registrar"
 	"github.com/docker/docker/pkg/truncindex"
+	"github.com/docker/docker/registry"
 	"github.com/docker/docker/volume"
 	volumedrivers "github.com/docker/docker/volume/drivers"
 	"github.com/docker/docker/volume/local"
@@ -328,13 +329,102 @@ func TestDaemonReloadLabels(t *testing.T) {
 		},
 	}
 
-	daemon.Reload(newConfig)
+	if err := daemon.Reload(newConfig); err != nil {
+		t.Fatal(err)
+	}
+
 	label := daemon.configStore.Labels[0]
 	if label != "foo:baz" {
 		t.Fatalf("Expected daemon label `foo:baz`, got %s", label)
 	}
 }
 
+func TestDaemonReloadInsecureRegistries(t *testing.T) {
+	daemon := &Daemon{}
+	// initialize daemon with existing insecure registries: "127.0.0.0/8", "10.10.1.11:5000", "10.10.1.22:5000"
+	daemon.RegistryService = registry.NewService(registry.ServiceOptions{
+		InsecureRegistries: []string{
+			"127.0.0.0/8",
+			"10.10.1.11:5000",
+			"10.10.1.22:5000", // this will be removed when reloading
+			"docker1.com",
+			"docker2.com", // this will be removed when reloading
+		},
+	})
+
+	daemon.configStore = &Config{}
+
+	insecureRegistries := []string{
+		"127.0.0.0/8",     // this will be kept
+		"10.10.1.11:5000", // this will be kept
+		"10.10.1.33:5000", // this will be newly added
+		"docker1.com",     // this will be kept
+		"docker3.com",     // this will be newly added
+	}
+
+	valuesSets := make(map[string]interface{})
+	valuesSets["insecure-registries"] = insecureRegistries
+
+	newConfig := &Config{
+		CommonConfig: CommonConfig{
+			ServiceOptions: registry.ServiceOptions{
+				InsecureRegistries: insecureRegistries,
+			},
+			valuesSet: valuesSets,
+		},
+	}
+
+	if err := daemon.Reload(newConfig); err != nil {
+		t.Fatal(err)
+	}
+
+	// After Reload, daemon.RegistryService will be changed which is useful
+	// for registry communication in daemon.
+	registries := daemon.RegistryService.ServiceConfig()
+
+	// After Reload(), newConfig has come to registries.InsecureRegistryCIDRs and registries.IndexConfigs in daemon.
+	// Then collect registries.InsecureRegistryCIDRs in dataMap.
+	// When collecting, we need to convert CIDRS into string as a key,
+	// while the times of key appears as value.
+	dataMap := map[string]int{}
+	for _, value := range registries.InsecureRegistryCIDRs {
+		if _, ok := dataMap[value.String()]; !ok {
+			dataMap[value.String()] = 1
+		} else {
+			dataMap[value.String()]++
+		}
+	}
+
+	for _, value := range registries.IndexConfigs {
+		if _, ok := dataMap[value.Name]; !ok {
+			dataMap[value.Name] = 1
+		} else {
+			dataMap[value.Name]++
+		}
+	}
+
+	// Finally compare dataMap with the original insecureRegistries.
+	// Each value in insecureRegistries should appear in daemon's insecure registries,
+	// and each can only appear exactly ONCE.
+	for _, r := range insecureRegistries {
+		if value, ok := dataMap[r]; !ok {
+			t.Fatalf("Expected daemon insecure registry %s, got none", r)
+		} else if value != 1 {
+			t.Fatalf("Expected only 1 daemon insecure registry %s, got %d", r, value)
+		}
+	}
+
+	// assert if "10.10.1.22:5000" is removed when reloading
+	if value, ok := dataMap["10.10.1.22:5000"]; ok {
+		t.Fatalf("Expected no insecure registry of 10.10.1.22:5000, got %d", value)
+	}
+
+	// assert if "docker2.com" is removed when reloading
+	if value, ok := dataMap["docker2.com"]; ok {
+		t.Fatalf("Expected no insecure registry of docker2.com, got %d", value)
+	}
+}
+
 func TestDaemonReloadNotAffectOthers(t *testing.T) {
 	daemon := &Daemon{}
 	daemon.configStore = &Config{
@@ -353,7 +443,10 @@ func TestDaemonReloadNotAffectOthers(t *testing.T) {
 		},
 	}
 
-	daemon.Reload(newConfig)
+	if err := daemon.Reload(newConfig); err != nil {
+		t.Fatal(err)
+	}
+
 	label := daemon.configStore.Labels[0]
 	if label != "foo:baz" {
 		t.Fatalf("Expected daemon label `foo:baz`, got %s", label)

docs/reference/commandline/dockerd.md

@@ -1241,12 +1241,13 @@ The list of currently supported options that can be reconfigured is this:
 - `runtimes`: it updates the list of available OCI runtimes that can
   be used to run containers
 - `authorization-plugin`: specifies the authorization plugins to use.
+- `insecure-registries`: it replaces the daemon insecure registries with a new set of insecure registries. If some existing insecure registries in daemon's configuration are not in newly reloaded insecure resgitries, these existing ones will be removed from daemon's config.
 
 Updating and reloading the cluster configurations such as `--cluster-store`,
 `--cluster-advertise` and `--cluster-store-opts` will take effect only if
 these configurations were not previously configured. If `--cluster-store`
 has been provided in flags and `cluster-advertise` not, `cluster-advertise`
-can be added in the configuration file without accompanied by `--cluster-store`
+can be added in the configuration file without accompanied by `--cluster-store`.
 Configuration reload will log a warning message if it detects a change in
 previously configured cluster configurations.
 

integration-cli/docker_cli_events_unix_test.go

@@ -429,7 +429,7 @@ func (s *DockerDaemonSuite) TestDaemonEvents(c *check.C) {
 	out, err = s.d.Cmd("events", "--since=0", "--until", daemonUnixTime(c))
 	c.Assert(err, checker.IsNil)
 
-	c.Assert(out, checker.Contains, fmt.Sprintf("daemon reload %s (cluster-advertise=, cluster-store=, cluster-store-opts={}, debug=true, default-runtime=runc, labels=[\"bar=foo\"], live-restore=false, max-concurrent-downloads=1, max-concurrent-uploads=5, name=%s, runtimes=runc:{docker-runc []}, shutdown-timeout=10)", daemonID, daemonName))
+	c.Assert(out, checker.Contains, fmt.Sprintf("daemon reload %s (cluster-advertise=, cluster-store=, cluster-store-opts={}, debug=true, default-runtime=runc, insecure-registries=[], labels=[\"bar=foo\"], live-restore=false, max-concurrent-downloads=1, max-concurrent-uploads=5, name=%s, runtimes=runc:{docker-runc []}, shutdown-timeout=10)", daemonID, daemonName))
 }
 
 func (s *DockerDaemonSuite) TestDaemonEventsWithFilters(c *check.C) {

registry/config.go

@@ -82,13 +82,6 @@ func (options *ServiceOptions) InstallCliFlags(flags *pflag.FlagSet) {
 
 // newServiceConfig returns a new instance of ServiceConfig
 func newServiceConfig(options ServiceOptions) *serviceConfig {
-	// Localhost is by default considered as an insecure registry
-	// This is a stop-gap for people who are running a private registry on localhost (especially on Boot2docker).
-	//
-	// TODO: should we deprecate this once it is easier for people to set up a TLS registry or change
-	// daemon flags on boot2docker?
-	options.InsecureRegistries = append(options.InsecureRegistries, "127.0.0.0/8")
-
 	config := &serviceConfig{
 		ServiceConfig: registrytypes.ServiceConfig{
 			InsecureRegistryCIDRs: make([]*registrytypes.NetIPNet, 0),
@@ -99,13 +92,51 @@ func newServiceConfig(options ServiceOptions) *serviceConfig {
 		},
 		V2Only: options.V2Only,
 	}
-	// Split --insecure-registry into CIDR and registry-specific settings.
-	for _, r := range options.InsecureRegistries {
+
+	config.LoadInsecureRegistries(options.InsecureRegistries)
+
+	return config
+}
+
+// LoadInsecureRegistries loads insecure registries to config
+func (config *serviceConfig) LoadInsecureRegistries(registries []string) error {
+	// Localhost is by default considered as an insecure registry
+	// This is a stop-gap for people who are running a private registry on localhost (especially on Boot2docker).
+	//
+	// TODO: should we deprecate this once it is easier for people to set up a TLS registry or change
+	// daemon flags on boot2docker?
+	registries = append(registries, "127.0.0.0/8")
+
+	// Store original InsecureRegistryCIDRs and IndexConfigs
+	// Clean InsecureRegistryCIDRs and IndexConfigs in config, as passed registries has all insecure registry info.
+	originalCIDRs := config.ServiceConfig.InsecureRegistryCIDRs
+	originalIndexInfos := config.ServiceConfig.IndexConfigs
+
+	config.ServiceConfig.InsecureRegistryCIDRs = make([]*registrytypes.NetIPNet, 0)
+	config.ServiceConfig.IndexConfigs = make(map[string]*registrytypes.IndexInfo, 0)
+
+skip:
+	for _, r := range registries {
+		// validate insecure registry
+		if _, err := ValidateIndexName(r); err != nil {
+			// before returning err, roll back to original data
+			config.ServiceConfig.InsecureRegistryCIDRs = originalCIDRs
+			config.ServiceConfig.IndexConfigs = originalIndexInfos
+			return err
+		}
 		// Check if CIDR was passed to --insecure-registry
 		_, ipnet, err := net.ParseCIDR(r)
 		if err == nil {
-			// Valid CIDR.
-			config.InsecureRegistryCIDRs = append(config.InsecureRegistryCIDRs, (*registrytypes.NetIPNet)(ipnet))
+			// Valid CIDR. If ipnet is already in config.InsecureRegistryCIDRs, skip.
+			data := (*registrytypes.NetIPNet)(ipnet)
+			for _, value := range config.InsecureRegistryCIDRs {
+				if value.IP.String() == data.IP.String() && value.Mask.String() == data.Mask.String() {
+					continue skip
+				}
+			}
+			// ipnet is not found, add it in config.InsecureRegistryCIDRs
+			config.InsecureRegistryCIDRs = append(config.InsecureRegistryCIDRs, data)
+
 		} else {
 			// Assume `host:port` if not CIDR.
 			config.IndexConfigs[r] = &registrytypes.IndexInfo{
@@ -125,7 +156,7 @@ func newServiceConfig(options ServiceOptions) *serviceConfig {
 		Official: true,
 	}
 
-	return config
+	return nil
 }
 
 // isSecureIndex returns false if the provided indexName is part of the list of insecure registries

registry/service.go

@@ -6,6 +6,7 @@ import (
 	"net/http"
 	"net/url"
 	"strings"
+	"sync"
 
 	"golang.org/x/net/context"
 
@@ -30,12 +31,14 @@ type Service interface {
 	Search(ctx context.Context, term string, limit int, authConfig *types.AuthConfig, userAgent string, headers map[string][]string) (*registrytypes.SearchResults, error)
 	ServiceConfig() *registrytypes.ServiceConfig
 	TLSConfig(hostname string) (*tls.Config, error)
+	LoadInsecureRegistries([]string) error
 }
 
 // DefaultService is a registry service. It tracks configuration data such as a list
 // of mirrors.
 type DefaultService struct {
 	config *serviceConfig
+	mu     sync.Mutex
 }
 
 // NewService returns a new instance of DefaultService ready to be
@@ -48,7 +51,34 @@ func NewService(options ServiceOptions) *DefaultService {
 
 // ServiceConfig returns the public registry service configuration.
 func (s *DefaultService) ServiceConfig() *registrytypes.ServiceConfig {
-	return &s.config.ServiceConfig
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	servConfig := registrytypes.ServiceConfig{
+		InsecureRegistryCIDRs: make([]*(registrytypes.NetIPNet), 0),
+		IndexConfigs:          make(map[string]*(registrytypes.IndexInfo)),
+		Mirrors:               make([]string, 0),
+	}
+
+	// construct a new ServiceConfig which will not retrieve s.Config directly,
+	// and look up items in s.config with mu locked
+	servConfig.InsecureRegistryCIDRs = append(servConfig.InsecureRegistryCIDRs, s.config.ServiceConfig.InsecureRegistryCIDRs...)
+
+	for key, value := range s.config.ServiceConfig.IndexConfigs {
+		servConfig.IndexConfigs[key] = value
+	}
+
+	servConfig.Mirrors = append(servConfig.Mirrors, s.config.ServiceConfig.Mirrors...)
+
+	return &servConfig
+}
+
+// LoadInsecureRegistries loads insecure registries for Service
+func (s *DefaultService) LoadInsecureRegistries(registries []string) error {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	return s.config.LoadInsecureRegistries(registries)
 }
 
 // Auth contacts the public registry with the provided credentials,
@@ -121,7 +151,11 @@ func (s *DefaultService) Search(ctx context.Context, term string, limit int, aut
 
 	indexName, remoteName := splitReposSearchTerm(term)
 
+	// Search is a long-running operation, just lock s.config to avoid block others.
+	s.mu.Lock()
 	index, err := newIndexInfo(s.config, indexName)
+	s.mu.Unlock()
+
 	if err != nil {
 		return nil, err
 	}
@@ -185,6 +219,8 @@ func (s *DefaultService) Search(ctx context.Context, term string, limit int, aut
 // ResolveRepository splits a repository name into its components
 // and configuration of the associated registry.
 func (s *DefaultService) ResolveRepository(name reference.Named) (*RepositoryInfo, error) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
 	return newRepositoryInfo(s.config, name)
 }
 
@@ -205,17 +241,28 @@ func (e APIEndpoint) ToV1Endpoint(userAgent string, metaHeaders http.Header) (*V
 
 // TLSConfig constructs a client TLS configuration based on server defaults
 func (s *DefaultService) TLSConfig(hostname string) (*tls.Config, error) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	return newTLSConfig(hostname, isSecureIndex(s.config, hostname))
+}
+
+// tlsConfig constructs a client TLS configuration based on server defaults
+func (s *DefaultService) tlsConfig(hostname string) (*tls.Config, error) {
 	return newTLSConfig(hostname, isSecureIndex(s.config, hostname))
 }
 
 func (s *DefaultService) tlsConfigForMirror(mirrorURL *url.URL) (*tls.Config, error) {
-	return s.TLSConfig(mirrorURL.Host)
+	return s.tlsConfig(mirrorURL.Host)
 }
 
 // LookupPullEndpoints creates a list of endpoints to try to pull from, in order of preference.
 // It gives preference to v2 endpoints over v1, mirrors over the actual
 // registry, and HTTPS over plain HTTP.
 func (s *DefaultService) LookupPullEndpoints(hostname string) (endpoints []APIEndpoint, err error) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
 	return s.lookupEndpoints(hostname)
 }
 
@@ -223,6 +270,9 @@ func (s *DefaultService) LookupPullEndpoints(hostname string) (endpoints []APIEn
 // It gives preference to v2 endpoints over v1, and HTTPS over plain HTTP.
 // Mirrors are not included.
 func (s *DefaultService) LookupPushEndpoints(hostname string) (endpoints []APIEndpoint, err error) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
 	allEndpoints, err := s.lookupEndpoints(hostname)
 	if err == nil {
 		for _, endpoint := range allEndpoints {

registry/service_v1.go

@@ -19,7 +19,7 @@ func (s *DefaultService) lookupV1Endpoints(hostname string) (endpoints []APIEndp
 		return endpoints, nil
 	}
 
-	tlsConfig, err = s.TLSConfig(hostname)
+	tlsConfig, err = s.tlsConfig(hostname)
 	if err != nil {
 		return nil, err
 	}

registry/service_v2.go

@@ -44,7 +44,7 @@ func (s *DefaultService) lookupV2Endpoints(hostname string) (endpoints []APIEndp
 		return endpoints, nil
 	}
 
-	tlsConfig, err = s.TLSConfig(hostname)
+	tlsConfig, err = s.tlsConfig(hostname)
 	if err != nil {
 		return nil, err
 	}