浏览代码

api/server: get features from a callback fn

Passing around a bare pointer to the map of configured features in order
to propagate to consumers changes to the configuration across reloads is
dangerous. Map operations are not atomic, so concurrently reading from
the map while it is being updated is a data race as there is no
synchronization. Use a getter function to retrieve the current features
map so the features can be retrieved race-free.

Remove the unused features argument from the build router.

Signed-off-by: Cory Snider <csnider@mirantis.com>
Cory Snider 2 年之前
父节点
当前提交
982e4fb448

+ 6 - 8
api/server/router/build/build.go

@@ -9,18 +9,16 @@ import (
 
 
 // buildRouter is a router to talk with the build controller
 // buildRouter is a router to talk with the build controller
 type buildRouter struct {
 type buildRouter struct {
-	backend  Backend
-	daemon   experimentalProvider
-	routes   []router.Route
-	features *map[string]bool
+	backend Backend
+	daemon  experimentalProvider
+	routes  []router.Route
 }
 }
 
 
 // NewRouter initializes a new build router
 // NewRouter initializes a new build router
-func NewRouter(b Backend, d experimentalProvider, features *map[string]bool) router.Router {
+func NewRouter(b Backend, d experimentalProvider) router.Router {
 	r := &buildRouter{
 	r := &buildRouter{
-		backend:  b,
-		daemon:   d,
-		features: features,
+		backend: b,
+		daemon:  d,
 	}
 	}
 	r.initRoutes()
 	r.initRoutes()
 	return r
 	return r

+ 2 - 2
api/server/router/system/system.go

@@ -12,11 +12,11 @@ type systemRouter struct {
 	cluster  ClusterBackend
 	cluster  ClusterBackend
 	routes   []router.Route
 	routes   []router.Route
 	builder  *buildkit.Builder
 	builder  *buildkit.Builder
-	features *map[string]bool
+	features func() map[string]bool
 }
 }
 
 
 // NewRouter initializes a new system router
 // NewRouter initializes a new system router
-func NewRouter(b Backend, c ClusterBackend, builder *buildkit.Builder, features *map[string]bool) router.Router {
+func NewRouter(b Backend, c ClusterBackend, builder *buildkit.Builder, features func() map[string]bool) router.Router {
 	r := &systemRouter{
 	r := &systemRouter{
 		backend:  b,
 		backend:  b,
 		cluster:  c,
 		cluster:  c,

+ 1 - 1
api/server/router/system/system_routes.go

@@ -31,7 +31,7 @@ func (s *systemRouter) pingHandler(ctx context.Context, w http.ResponseWriter, r
 	w.Header().Add("Cache-Control", "no-cache, no-store, must-revalidate")
 	w.Header().Add("Cache-Control", "no-cache, no-store, must-revalidate")
 	w.Header().Add("Pragma", "no-cache")
 	w.Header().Add("Pragma", "no-cache")
 
 
-	builderVersion := build.BuilderVersion(*s.features)
+	builderVersion := build.BuilderVersion(s.features())
 	if bv := builderVersion; bv != "" {
 	if bv := builderVersion; bv != "" {
 		w.Header().Set("Builder-Version", string(bv))
 		w.Header().Set("Builder-Version", string(bv))
 	}
 	}

+ 4 - 4
cmd/dockerd/daemon.go

@@ -337,7 +337,7 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
 type routerOptions struct {
 type routerOptions struct {
 	sessionManager *session.Manager
 	sessionManager *session.Manager
 	buildBackend   *buildbackend.Backend
 	buildBackend   *buildbackend.Backend
-	features       *map[string]bool
+	features       func() map[string]bool
 	buildkit       *buildkit.Builder
 	buildkit       *buildkit.Builder
 	daemon         *daemon.Daemon
 	daemon         *daemon.Daemon
 	cluster        *cluster.Cluster
 	cluster        *cluster.Cluster
@@ -357,7 +357,7 @@ func newRouterOptions(ctx context.Context, config *config.Config, d *daemon.Daem
 	cgroupParent := newCgroupParent(config)
 	cgroupParent := newCgroupParent(config)
 	ro := routerOptions{
 	ro := routerOptions{
 		sessionManager: sm,
 		sessionManager: sm,
-		features:       d.Features(),
+		features:       d.Features,
 		daemon:         d,
 		daemon:         d,
 	}
 	}
 
 
@@ -593,9 +593,9 @@ func (opts routerOptions) Build() []router.Router {
 			opts.daemon.ImageService().DistributionServices().ImageStore,
 			opts.daemon.ImageService().DistributionServices().ImageStore,
 			opts.daemon.ImageService().DistributionServices().LayerStore,
 			opts.daemon.ImageService().DistributionServices().LayerStore,
 		),
 		),
-		systemrouter.NewRouter(opts.daemon, opts.cluster, opts.buildkit, opts.features),
+		systemrouter.NewRouter(opts.daemon, opts.cluster, opts.buildkit, opts.daemon.Features),
 		volume.NewRouter(opts.daemon.VolumesService(), opts.cluster),
 		volume.NewRouter(opts.daemon.VolumesService(), opts.cluster),
-		build.NewRouter(opts.buildBackend, opts.daemon, opts.features),
+		build.NewRouter(opts.buildBackend, opts.daemon),
 		sessionrouter.NewRouter(opts.sessionManager),
 		sessionrouter.NewRouter(opts.sessionManager),
 		swarmrouter.NewRouter(opts.cluster),
 		swarmrouter.NewRouter(opts.cluster),
 		pluginrouter.NewRouter(opts.daemon.PluginManager()),
 		pluginrouter.NewRouter(opts.daemon.PluginManager()),

+ 8 - 2
daemon/daemon.go

@@ -154,8 +154,14 @@ func (daemon *Daemon) HasExperimental() bool {
 }
 }
 
 
 // Features returns the features map from configStore
 // Features returns the features map from configStore
-func (daemon *Daemon) Features() *map[string]bool {
-	return &daemon.configStore.Features
+func (daemon *Daemon) Features() map[string]bool {
+	daemon.configStore.Lock()
+	defer daemon.configStore.Unlock()
+	f := make(map[string]bool, len(daemon.configStore.Features))
+	for k, v := range daemon.configStore.Features {
+		f[k] = v
+	}
+	return f
 }
 }
 
 
 // UsesSnapshotter returns true if feature flag to use containerd snapshotter is enabled
 // UsesSnapshotter returns true if feature flag to use containerd snapshotter is enabled