From 63796a7c48872badb072b5dddf1850eb8b90c2e1 Mon Sep 17 00:00:00 2001 From: Reed Allman Date: Wed, 26 Jul 2017 06:31:09 -0700 Subject: [PATCH] change hot containers when route/app vars change this changes the behavior of hot containers: 1) we are no longer populating a hot container with all of the env vars from the first request to start up that hot container. this will only populate the container with any vars that are defined on the app or route. 2) when env vars are changed on the route or app, we will now start up a new hot container that contains those changes. 3) fixes a bug where we could have a collision if the image and path name created one, e.g. `/yo/foo` & `oze/yo:latest` collides with `/yo/fo` and `ooze/yo:latest` (if all other fields are held constant), since we're name spacing with app name in theory it would happen to the same user (though we were relying on a comma delimiter there, not great). now we use NULL bytes which should be hard to get in through a json api ;) i added a sha1 to keep the size of the (soon to be very large) map down, i don't expect collisions but, well, it's a hash function. a small note that we could add a few things to the hot container that will not change on a request basis, such as `app_name`, `format` and `route` but it's a bit pedantic. ultimately, it's confusing imo that we have a different set of vars in the env and in the request itself for hot, which is unavoidable unless we choose to omit setting env vars entirely, but it seems to be what the people want (lmk, people, if otherwise). --- api/runner/task.go | 9 ++++++--- api/runner/task/task.go | 3 ++- api/runner/worker.go | 27 +++++++++++++++++++++++++-- api/server/runner.go | 12 ++++++++++-- 4 files changed, 43 insertions(+), 8 deletions(-) diff --git a/api/runner/task.go b/api/runner/task.go index e8619c997..03c0ee098 100644 --- a/api/runner/task.go +++ b/api/runner/task.go @@ -11,6 +11,7 @@ import ( "github.com/docker/cli/cli/config/configfile" "github.com/fnproject/fn/api/runner/drivers" + "github.com/fnproject/fn/api/runner/protocol" "github.com/fnproject/fn/api/runner/task" docker "github.com/fsouza/go-dockerclient" ) @@ -62,6 +63,9 @@ type containerTask struct { func (t *containerTask) Command() string { return "" } func (t *containerTask) EnvVars() map[string]string { + if protocol.IsStreamable(protocol.Protocol(t.cfg.Format)) { + return t.cfg.BaseEnv + } return t.cfg.Env } func (t *containerTask) Input() io.Reader { @@ -82,9 +86,8 @@ func (t *containerTask) IdleTimeout() time.Duration { return t.cfg.IdleTimeo func (t *containerTask) Logger() (io.Writer, io.Writer) { return t.cfg.Stdout, t.cfg.Stderr } func (t *containerTask) Volumes() [][2]string { return [][2]string{} } func (t *containerTask) WorkDir() string { return "" } - -func (t *containerTask) Close() {} -func (t *containerTask) WriteStat(drivers.Stat) {} +func (t *containerTask) Close() {} +func (t *containerTask) WriteStat(drivers.Stat) {} // Implementing the docker.AuthConfiguration interface. Pulling in // the docker repo password from environment variables diff --git a/api/runner/task/task.go b/api/runner/task/task.go index 219c586a2..cc568ca38 100644 --- a/api/runner/task/task.go +++ b/api/runner/task/task.go @@ -16,7 +16,8 @@ type Config struct { IdleTimeout time.Duration AppName string Memory uint64 - Env map[string]string + BaseEnv map[string]string // only app & route config vals [for hot] + Env map[string]string // includes BaseEnv Format string ReceivedTime time.Time // Ready is used to await the first pull diff --git a/api/runner/worker.go b/api/runner/worker.go index 70cfe5f24..3964e89be 100644 --- a/api/runner/worker.go +++ b/api/runner/worker.go @@ -2,6 +2,7 @@ package runner import ( "context" + "crypto/sha1" "errors" "fmt" "io" @@ -132,8 +133,8 @@ func (h *htfnmgr) getPipe(ctx context.Context, rnr *Runner, cfg *task.Config) ch h.RLock() } - // TODO(ccirello): re-implement this without memory allocation (fmt.Sprint) - fn := fmt.Sprint(cfg.AppName, ",", cfg.Path, cfg.Image, cfg.Timeout, cfg.Memory, cfg.Format) + fn := key(cfg) + svr, ok := h.hc[fn] h.RUnlock() if !ok { @@ -149,6 +150,28 @@ func (h *htfnmgr) getPipe(ctx context.Context, rnr *Runner, cfg *task.Config) ch return svr.tasksin } +func key(cfg *task.Config) string { + // TODO we should probably colocate this with Config, but it's kind of hot + // specific so it makes sense here, too (just brittle & hidden) + + // return a sha1 hash of a (hopefully) unique string of all the config + // values, to make map lookups quicker [than the giant unique string] + hash := sha1.New() + fmt.Fprint(hash, cfg.AppName, "\x00") + fmt.Fprint(hash, cfg.Path, "\x00") + fmt.Fprint(hash, cfg.Image, "\x00") + for k, v := range cfg.BaseEnv { + fmt.Fprint(hash, k, "\x00", v, "\x00") + } + fmt.Fprint(hash, cfg.Timeout, "\x00") + fmt.Fprint(hash, cfg.IdleTimeout, "\x00") + fmt.Fprint(hash, cfg.Memory, "\x00") + fmt.Fprint(hash, cfg.Format, "\x00") + + var buf [sha1.Size]byte + return string(hash.Sum(buf[:])) +} + // htfnsvr is part of htfnmgr, abstracted apart for simplicity, its only // purpose is to test for hot functions saturation and try starting as many as // needed. In case of absence of workload, it will stop trying to start new hot diff --git a/api/server/runner.go b/api/server/runner.go index 46ce41eab..365ccf124 100644 --- a/api/server/runner.go +++ b/api/server/runner.go @@ -156,12 +156,19 @@ func (s *Server) serve(ctx context.Context, c *gin.Context, appName string, rout "FORMAT": route.Format, } + // TODO we could add... format, app_name, route from above (but nothing from the specific request) + baseVars := make(map[string]string, len(app.Config)+len(route.Config)) + // app config for k, v := range app.Config { - envVars[toEnvName("", k)] = v + k = toEnvName("", k) + envVars[k] = v + baseVars[k] = v } for k, v := range route.Config { - envVars[toEnvName("", k)] = v + k = toEnvName("", k) + envVars[k] = v + baseVars[k] = v } // params @@ -177,6 +184,7 @@ func (s *Server) serve(ctx context.Context, c *gin.Context, appName string, rout cfg := &task.Config{ AppName: appName, Path: route.Path, + BaseEnv: baseVars, Env: envVars, Format: route.Format, ID: reqID,