From 34ecf20467a5395235760bc30470066f0eee30aa Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sun, 23 Jan 2022 14:13:49 -0800 Subject: [PATCH] Re-implement ContainerInspect call in Wings to use more performant json encoder (#119) * First pass at re-implementing the Docker inspect call to use more efficient json parser * Improve logic --- config/config_docker.go | 2 + environment/docker/api.go | 116 ++++++++++++++++++++++++++++++ environment/docker/container.go | 4 +- environment/docker/environment.go | 7 +- environment/docker/power.go | 4 +- environment/docker/stats.go | 2 +- 6 files changed, 126 insertions(+), 9 deletions(-) create mode 100644 environment/docker/api.go diff --git a/config/config_docker.go b/config/config_docker.go index 8ae11f6..f79872a 100644 --- a/config/config_docker.go +++ b/config/config_docker.go @@ -75,6 +75,8 @@ type DockerConfiguration struct { // Overhead controls the memory overhead given to all containers to circumvent certain // software such as the JVM not staying below the maximum memory limit. Overhead Overhead `json:"overhead" yaml:"overhead"` + + UsePerformantInspect bool `default:"true" json:"use_performant_inspect" yaml:"use_performant_inspect"` } // RegistryConfiguration defines the authentication credentials for a given diff --git a/environment/docker/api.go b/environment/docker/api.go new file mode 100644 index 0000000..2000627 --- /dev/null +++ b/environment/docker/api.go @@ -0,0 +1,116 @@ +package docker + +import ( + "context" + "io" + "net/http" + "reflect" + "strings" + "sync" + + "emperror.dev/errors" + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/versions" + "github.com/docker/docker/client" + "github.com/docker/docker/errdefs" + "github.com/goccy/go-json" + "github.com/pterodactyl/wings/config" +) + +var ( + o sync.Once + cli cliSettings + fastEnabled bool +) + +type cliSettings struct { + enabled bool + proto string + host string + scheme string + version string +} + +func configure(c *client.Client) { + o.Do(func() { + fastEnabled = config.Get().Docker.UsePerformantInspect + + r := reflect.ValueOf(c).Elem() + cli.proto = r.FieldByName("proto").String() + cli.host = r.FieldByName("addr").String() + cli.scheme = r.FieldByName("scheme").String() + cli.version = r.FieldByName("version").String() + }) +} + +// ContainerInspect is a rough equivalent of Docker's client.ContainerInspect() +// but re-written to use a more performant JSON decoder. This is important since +// a large number of requests to this endpoint are spawned by Wings, and the +// standard "encoding/json" shows its performance woes badly even with single +// containers running. +func (e *Environment) ContainerInspect(ctx context.Context) (types.ContainerJSON, error) { + configure(e.client) + + // Support feature flagging of this functionality so that if something goes + // wrong for now it is easy enough for people to switch back to the older method + // of fetching stats. + if !fastEnabled { + return e.client.ContainerInspect(ctx, e.Id) + } + + var st types.ContainerJSON + req, err := http.NewRequestWithContext(ctx, http.MethodGet, "/containers/"+e.Id+"/json", nil) + if err != nil { + return st, errors.WithStack(err) + } + + if cli.proto == "unix" || cli.proto == "npipe" { + req.Host = "docker" + } + + req.URL.Host = cli.host + req.URL.Scheme = cli.scheme + + res, err := e.client.HTTPClient().Do(req) + if err != nil { + return st, errdefs.FromStatusCode(err, res.StatusCode) + } + + body, err := io.ReadAll(res.Body) + if err != nil { + return st, errors.Wrap(err, "failed to read response body from Docker") + } + if err := parseErrorFromResponse(res, body); err != nil { + return st, errdefs.FromStatusCode(err, res.StatusCode) + } + if err := json.Unmarshal(body, &st); err != nil { + return st, errors.WithStack(err) + } + return st, nil +} + +// parseErrorFromResponse is a re-implementation of Docker's +// client.checkResponseErr() function. +func parseErrorFromResponse(res *http.Response, body []byte) error { + if res.StatusCode >= 200 && res.StatusCode < 400 { + return nil + } + + var ct string + if res.Header != nil { + ct = res.Header.Get("Content-Type") + } + + var emsg string + if (cli.version == "" || versions.GreaterThan(cli.version, "1.23")) && ct == "application/json" { + var errResp types.ErrorResponse + if err := json.Unmarshal(body, &errResp); err != nil { + return errors.WithStack(err) + } + emsg = strings.TrimSpace(errResp.Message) + } else { + emsg = strings.TrimSpace(string(body)) + } + + return errors.Wrap(errors.New(emsg), "Error response from daemon") +} \ No newline at end of file diff --git a/environment/docker/container.go b/environment/docker/container.go index 69db61e..858a082 100644 --- a/environment/docker/container.go +++ b/environment/docker/container.go @@ -118,7 +118,7 @@ func (e *Environment) InSituUpdate() error { ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) defer cancel() - if _, err := e.client.ContainerInspect(ctx, e.Id); err != nil { + if _, err := e.ContainerInspect(ctx); err != nil { // If the container doesn't exist for some reason there really isn't anything // we can do to fix that in this process (it doesn't make sense at least). In those // cases just return without doing anything since we still want to save the configuration @@ -150,7 +150,7 @@ func (e *Environment) Create() error { // If the container already exists don't hit the user with an error, just return // the current information about it which is what we would do when creating the // container anyways. - if _, err := e.client.ContainerInspect(context.Background(), e.Id); err == nil { + if _, err := e.ContainerInspect(context.Background()); err == nil { return nil } else if !client.IsErrNotFound(err) { return errors.Wrap(err, "environment/docker: failed to inspect container") diff --git a/environment/docker/environment.go b/environment/docker/environment.go index 40bc571..4142f09 100644 --- a/environment/docker/environment.go +++ b/environment/docker/environment.go @@ -10,7 +10,6 @@ import ( "github.com/apex/log" "github.com/docker/docker/api/types" "github.com/docker/docker/client" - "github.com/pterodactyl/wings/environment" "github.com/pterodactyl/wings/events" "github.com/pterodactyl/wings/remote" @@ -116,7 +115,7 @@ func (e *Environment) Events() *events.Bus { // will work fine when using the container name as the lookup parameter in addition to the longer // ID auto-assigned when the container is created. func (e *Environment) Exists() (bool, error) { - _, err := e.client.ContainerInspect(context.Background(), e.Id) + _, err := e.ContainerInspect(context.Background()) if err != nil { // If this error is because the container instance wasn't found via Docker we // can safely ignore the error and just return false. @@ -140,7 +139,7 @@ func (e *Environment) Exists() (bool, error) { // // @see docker/client/errors.go func (e *Environment) IsRunning(ctx context.Context) (bool, error) { - c, err := e.client.ContainerInspect(ctx, e.Id) + c, err := e.ContainerInspect(ctx) if err != nil { return false, err } @@ -150,7 +149,7 @@ func (e *Environment) IsRunning(ctx context.Context) (bool, error) { // Determine the container exit state and return the exit code and whether or not // the container was killed by the OOM killer. func (e *Environment) ExitState() (uint32, bool, error) { - c, err := e.client.ContainerInspect(context.Background(), e.Id) + c, err := e.ContainerInspect(context.Background()) if err != nil { // I'm not entirely sure how this can happen to be honest. I tried deleting a // container _while_ a server was running and wings gracefully saw the crash and diff --git a/environment/docker/power.go b/environment/docker/power.go index 756edc7..69fd30e 100644 --- a/environment/docker/power.go +++ b/environment/docker/power.go @@ -66,7 +66,7 @@ func (e *Environment) Start(ctx context.Context) error { } }() - if c, err := e.client.ContainerInspect(ctx, e.Id); err != nil { + if c, err := e.ContainerInspect(ctx); err != nil { // Do nothing if the container is not found, we just don't want to continue // to the next block of code here. This check was inlined here to guard against // a nil-pointer when checking c.State below. @@ -235,7 +235,7 @@ func (e *Environment) WaitForStop(seconds uint, terminate bool) error { // Terminate forcefully terminates the container using the signal provided. func (e *Environment) Terminate(signal os.Signal) error { - c, err := e.client.ContainerInspect(context.Background(), e.Id) + c, err := e.ContainerInspect(context.Background()) if err != nil { // Treat missing containers as an okay error state, means it is obviously // already terminated at this point. diff --git a/environment/docker/stats.go b/environment/docker/stats.go index 2815799..54f97da 100644 --- a/environment/docker/stats.go +++ b/environment/docker/stats.go @@ -16,7 +16,7 @@ import ( // Uptime returns the current uptime of the container in milliseconds. If the // container is not currently running this will return 0. func (e *Environment) Uptime(ctx context.Context) (int64, error) { - ins, err := e.client.ContainerInspect(ctx, e.Id) + ins, err := e.ContainerInspect(ctx) if err != nil { return 0, errors.Wrap(err, "environment: could not inspect container") }