summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--pkg/api/handlers/compat/containers_attach.go33
-rw-r--r--pkg/api/handlers/compat/exec.go31
-rw-r--r--pkg/api/server/idle/tracker.go10
-rw-r--r--pkg/api/server/server.go62
4 files changed, 79 insertions, 57 deletions
diff --git a/pkg/api/handlers/compat/containers_attach.go b/pkg/api/handlers/compat/containers_attach.go
index 4a1196c89..a4013469b 100644
--- a/pkg/api/handlers/compat/containers_attach.go
+++ b/pkg/api/handlers/compat/containers_attach.go
@@ -92,30 +92,29 @@ func AttachContainer(w http.ResponseWriter, r *http.Request) {
return
}
- idleTracker := r.Context().Value("idletracker").(*idle.Tracker)
- hijackChan := make(chan bool, 1)
+ logErr := func(e error) {
+ logrus.Error(errors.Wrapf(e, "error attaching to container %s", ctr.ID()))
+ }
// Perform HTTP attach.
// HTTPAttach will handle everything about the connection from here on
// (including closing it and writing errors to it).
- if err := ctr.HTTPAttach(r, w, streams, detachKeys, nil, query.Stream, query.Logs, hijackChan); err != nil {
- hijackComplete := <-hijackChan
+ hijackChan := make(chan bool, 1)
+ err = ctr.HTTPAttach(r, w, streams, detachKeys, nil, query.Stream, query.Logs, hijackChan)
- // We can't really do anything about errors anymore. HTTPAttach
- // should be writing them to the connection.
- logrus.Errorf("Error attaching to container %s: %v", ctr.ID(), err)
+ if <-hijackChan {
+ // If connection was Hijacked, we have to signal it's being closed
+ t := r.Context().Value("idletracker").(*idle.Tracker)
+ defer t.Close()
- if hijackComplete {
- // We do need to tell the idle tracker that the
- // connection has been closed, though. We can guarantee
- // that is true after HTTPAttach exits.
- idleTracker.Close()
- } else {
- // A hijack was not successfully completed. We need to
- // report the error normally.
- utils.InternalServerError(w, err)
+ if err != nil {
+ // Cannot report error to client as a 500 as the Upgrade set status to 101
+ logErr(err)
}
+ } else {
+ // If the Hijack failed we are going to assume we can still inform client of failure
+ utils.InternalServerError(w, err)
+ logErr(err)
}
-
logrus.Debugf("Attach for container %s completed successfully", ctr.ID())
}
diff --git a/pkg/api/handlers/compat/exec.go b/pkg/api/handlers/compat/exec.go
index df51293c2..cc5db8efe 100644
--- a/pkg/api/handlers/compat/exec.go
+++ b/pkg/api/handlers/compat/exec.go
@@ -174,25 +174,26 @@ func ExecStartHandler(w http.ResponseWriter, r *http.Request) {
return
}
- idleTracker := r.Context().Value("idletracker").(*idle.Tracker)
- hijackChan := make(chan bool, 1)
+ logErr := func(e error) {
+ logrus.Error(errors.Wrapf(e, "error attaching to container %s exec session %s", sessionCtr.ID(), sessionID))
+ }
- if err := sessionCtr.ExecHTTPStartAndAttach(sessionID, r, w, nil, nil, nil, hijackChan); err != nil {
- hijackComplete := <-hijackChan
+ hijackChan := make(chan bool, 1)
+ err = sessionCtr.ExecHTTPStartAndAttach(sessionID, r, w, nil, nil, nil, hijackChan)
- logrus.Errorf("Error attaching to container %s exec session %s: %v", sessionCtr.ID(), sessionID, err)
+ if <-hijackChan {
+ // If connection was Hijacked, we have to signal it's being closed
+ t := r.Context().Value("idletracker").(*idle.Tracker)
+ defer t.Close()
- if hijackComplete {
- // We do need to tell the idle tracker that the
- // connection has been closed, though. We can guarantee
- // that is true after HTTPAttach exits.
- idleTracker.Close()
- } else {
- // A hijack was not successfully completed. We need to
- // report the error normally.
- utils.InternalServerError(w, err)
+ if err != nil {
+ // Cannot report error to client as a 500 as the Upgrade set status to 101
+ logErr(err)
}
+ } else {
+ // If the Hijack failed we are going to assume we can still inform client of failure
+ utils.InternalServerError(w, err)
+ logErr(err)
}
-
logrus.Debugf("Attach for container %s exec session %s completed successfully", sessionCtr.ID(), sessionID)
}
diff --git a/pkg/api/server/idle/tracker.go b/pkg/api/server/idle/tracker.go
index 1b378c492..50e41b7bf 100644
--- a/pkg/api/server/idle/tracker.go
+++ b/pkg/api/server/idle/tracker.go
@@ -61,10 +61,14 @@ func (t *Tracker) ConnState(conn net.Conn, state http.ConnState) {
oldActive := t.ActiveConnections()
// Either the server or a hijacking handler has closed the http connection to a client
- if _, found := t.managed[conn]; found {
- delete(t.managed, conn)
- } else {
+ if conn == nil {
t.hijacked-- // guarded by t.mux above
+ } else {
+ if _, found := t.managed[conn]; found {
+ delete(t.managed, conn)
+ } else {
+ logrus.Warnf("IdleTracker %p: StateClosed transition by un-managed connection", conn)
+ }
}
// Transitioned from any "active" connection to no connections
diff --git a/pkg/api/server/server.go b/pkg/api/server/server.go
index 09a9f6370..355a46fb7 100644
--- a/pkg/api/server/server.go
+++ b/pkg/api/server/server.go
@@ -8,9 +8,9 @@ import (
"net/http"
"os"
"os/signal"
- "runtime"
goRuntime "runtime"
"strings"
+ "sync"
"syscall"
"time"
@@ -42,13 +42,19 @@ const (
UnlimitedServiceDuration = 0 * time.Second
)
+// shutdownOnce ensures Shutdown() may safely be called from several go routines
+var shutdownOnce sync.Once
+
// NewServer will create and configure a new API server with all defaults
func NewServer(runtime *libpod.Runtime) (*APIServer, error) {
return newServer(runtime, DefaultServiceDuration, nil)
}
// NewServerWithSettings will create and configure a new API server using provided settings
-func NewServerWithSettings(runtime *libpod.Runtime, duration time.Duration, listener *net.Listener) (*APIServer, error) {
+func NewServerWithSettings(runtime *libpod.Runtime, duration time.Duration, listener *net.Listener) (
+ *APIServer,
+ error,
+) {
return newServer(runtime, duration, listener)
}
@@ -180,7 +186,7 @@ func (s *APIServer) Serve() error {
go func() {
<-s.idleTracker.Done()
- logrus.Debugf("API Server idle for %v", s.idleTracker.Duration)
+ logrus.Debugf("API Server idle for %s", s.idleTracker.Duration.Round(time.Second).String())
_ = s.Shutdown()
}()
@@ -224,31 +230,43 @@ func (s *APIServer) Serve() error {
// Shutdown is a clean shutdown waiting on existing clients
func (s *APIServer) Shutdown() error {
if s.idleTracker.Duration == UnlimitedServiceDuration {
- logrus.Debug("APIServer.Shutdown ignored as Duration is UnlimitedService.")
+ logrus.Debug("APIServer.Shutdown ignored as Duration is UnlimitedService")
return nil
}
- // Gracefully shutdown server(s), duration of wait same as idle window
- // TODO: Should we really wait the idle window for shutdown?
- ctx, cancel := context.WithTimeout(context.Background(), s.idleTracker.Duration)
- defer cancel()
+ shutdownOnce.Do(func() {
+ if logrus.IsLevelEnabled(logrus.DebugLevel) {
+ _, file, line, _ := goRuntime.Caller(1)
+ logrus.Debugf("APIServer.Shutdown by %s:%d, %d/%d connection(s)",
+ file, line, s.idleTracker.ActiveConnections(), s.idleTracker.TotalConnections())
- if logrus.IsLevelEnabled(logrus.DebugLevel) {
- _, file, line, _ := runtime.Caller(1)
- logrus.Debugf("APIServer.Shutdown by %s:%d, %d/%d connection(s)",
- file, line, s.idleTracker.ActiveConnections(), s.idleTracker.TotalConnections())
- if err := s.pprof.Shutdown(ctx); err != nil {
- logrus.Warn("Failed to cleanly shutdown pprof Server: " + err.Error())
+ go func() {
+ ctx, cancel := context.WithTimeout(context.Background(), s.idleTracker.Duration)
+ go func() {
+ defer cancel()
+ if err := s.pprof.Shutdown(ctx); err != nil {
+ logrus.Warn(
+ errors.Wrapf(err, "failed to cleanly shutdown pprof Server"))
+ }
+ }()
+ <-ctx.Done()
+ }()
}
- }
- go func() {
- err := s.Server.Shutdown(ctx)
- if err != nil && err != context.Canceled && err != http.ErrServerClosed {
- logrus.Error("Failed to cleanly shutdown APIServer: " + err.Error())
- }
- }()
- <-ctx.Done()
+ // Gracefully shutdown server(s), duration of wait same as idle window
+ ctx, cancel := context.WithTimeout(context.Background(), s.idleTracker.Duration)
+ go func() {
+ defer cancel()
+
+ err := s.Server.Shutdown(ctx)
+ if err != nil && err != context.Canceled && err != http.ErrServerClosed {
+ logrus.Error(
+ errors.Wrapf(err, "failed to cleanly shutdown APIServer"))
+ }
+ }()
+ <-ctx.Done()
+ })
+
return nil
}
generated by cgit v1.2.3-54-g00ecf (git 2.39.0) at 2024-11-01 06:29:52 +0000