From 7461bafabba1dae736081de61baf6f7563af46d2 Mon Sep 17 00:00:00 2001 From: Ashley Cui Date: Tue, 21 Jul 2020 10:03:25 -0400 Subject: Fix Generate API title/description generate kube title and descritopn was same as play kube for apiv2 docs Signed-off-by: Ashley Cui --- pkg/api/server/register_generate.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'pkg/api/server') diff --git a/pkg/api/server/register_generate.go b/pkg/api/server/register_generate.go index 82f1dc680..a1ab3f727 100644 --- a/pkg/api/server/register_generate.go +++ b/pkg/api/server/register_generate.go @@ -13,8 +13,8 @@ func (s *APIServer) registerGenerateHandlers(r *mux.Router) error { // tags: // - containers // - pods - // summary: Play a Kubernetes YAML file. - // description: Create and run pods based on a Kubernetes YAML file (pod or service kind). + // summary: Generate a Kubernetes YAML file. + // description: Generate Kubernetes YAML based on a pod or container. // parameters: // - in: path // name: name:.* -- cgit v1.2.3-54-g00ecf From 6128cbec04fd80b62a9afe75adabd2acfd968909 Mon Sep 17 00:00:00 2001 From: "Korhonen Sami (Samlink)" Date: Thu, 9 Jul 2020 15:11:44 +0300 Subject: Remove hijacked connections from active connections list StateHijacked is a terminal state. If hijacked connection is registered as an active connection, connection will never be unregistered. This causes two issues First issue is that active connection counters are off. Second issue is a resource leak caused by connection object that is stored to a map. After this patch hijacked connections are no longer visible in counters. If a counter for hijacked connections is required, podman must track connections returned by Hijacker.Hijack() It might make sense to develop abstraction layer for hijacking - and move all hijacking related code to a separate package. Hijacking code is prone to resource leaks and it should be thoroughly tested. Signed-off-by: Sami Korhonen --- pkg/api/server/server.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'pkg/api/server') diff --git a/pkg/api/server/server.go b/pkg/api/server/server.go index 8af6d3186..f5b17ab68 100644 --- a/pkg/api/server/server.go +++ b/pkg/api/server/server.go @@ -255,14 +255,14 @@ func (t *IdleTracker) ConnState(conn net.Conn, state http.ConnState) { oldActive := len(t.active) logrus.Debugf("IdleTracker %p:%v %d/%d connection(s)", conn, state, t.ActiveConnections(), t.TotalConnections()) switch state { - case http.StateNew, http.StateActive, http.StateHijacked: + case http.StateNew, http.StateActive: t.active[conn] = struct{}{} // stop the timer if we transitioned from idle if oldActive == 0 { t.timer.Stop() } t.total++ - case http.StateIdle, http.StateClosed: + case http.StateIdle, http.StateClosed, http.StateHijacked: delete(t.active, conn) // Restart the timer if we've become idle if oldActive > 0 && len(t.active) == 0 { -- cgit v1.2.3-54-g00ecf From 79b005e638f110d6e8601816569827dbd0c8fdc2 Mon Sep 17 00:00:00 2001 From: "Korhonen Sami (Samlink)" Date: Thu, 9 Jul 2020 21:42:27 +0300 Subject: Fix: Correct connection counters for hijacked connections This patch fixes connection counters for v2 endpoints Idletracker was moved to a new package to prevent package cycle. Hijacking code still remains in wrong place and should be moved later to isolated package Signed-off-by: Sami Korhonen --- pkg/api/handlers/compat/containers_attach.go | 23 ++++++++- pkg/api/server/handler_api.go | 1 + pkg/api/server/idletracker/idletracker.go | 74 ++++++++++++++++++++++++++++ pkg/api/server/server.go | 72 ++++----------------------- 4 files changed, 106 insertions(+), 64 deletions(-) create mode 100644 pkg/api/server/idletracker/idletracker.go (limited to 'pkg/api/server') diff --git a/pkg/api/handlers/compat/containers_attach.go b/pkg/api/handlers/compat/containers_attach.go index 724b54ac4..71586fca4 100644 --- a/pkg/api/handlers/compat/containers_attach.go +++ b/pkg/api/handlers/compat/containers_attach.go @@ -11,6 +11,7 @@ import ( "github.com/containers/libpod/v2/libpod" "github.com/containers/libpod/v2/libpod/define" "github.com/containers/libpod/v2/pkg/api/handlers/utils" + "github.com/containers/libpod/v2/pkg/api/server/idletracker" "github.com/gorilla/schema" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -115,7 +116,21 @@ func AttachContainer(w http.ResponseWriter, r *http.Request) { logrus.Debugf("Attach for container %s completed successfully", ctr.ID()) } +type HijackedConnection struct { + net.Conn // Connection + idleTracker *idletracker.IdleTracker // Connection tracker +} + +func (c HijackedConnection) Close() error { + logrus.Debugf("Hijacked connection closed") + + c.idleTracker.TrackHijackedClosed() + return c.Conn.Close() +} + func AttachConnection(w http.ResponseWriter, r *http.Request) (net.Conn, *bufio.ReadWriter, error) { + idleTracker := r.Context().Value("idletracker").(*idletracker.IdleTracker) + // Hijack the connection hijacker, ok := w.(http.Hijacker) if !ok { @@ -126,10 +141,14 @@ func AttachConnection(w http.ResponseWriter, r *http.Request) (net.Conn, *bufio. if err != nil { return nil, nil, errors.Wrapf(err, "error hijacking connection") } + trackedConnection := HijackedConnection{ + Conn: connection, + idleTracker: idleTracker, + } - WriteAttachHeaders(r, connection) + WriteAttachHeaders(r, trackedConnection) - return connection, buffer, nil + return trackedConnection, buffer, nil } func WriteAttachHeaders(r *http.Request, connection io.Writer) { diff --git a/pkg/api/server/handler_api.go b/pkg/api/server/handler_api.go index b0fd932ba..53fe8952b 100644 --- a/pkg/api/server/handler_api.go +++ b/pkg/api/server/handler_api.go @@ -37,6 +37,7 @@ func (s *APIServer) APIHandler(h http.HandlerFunc) http.HandlerFunc { c := context.WithValue(r.Context(), "decoder", s.Decoder) //nolint c = context.WithValue(c, "runtime", s.Runtime) //nolint c = context.WithValue(c, "shutdownFunc", s.Shutdown) //nolint + c = context.WithValue(c, "idletracker", s.idleTracker) //nolint r = r.WithContext(c) h(w, r) diff --git a/pkg/api/server/idletracker/idletracker.go b/pkg/api/server/idletracker/idletracker.go new file mode 100644 index 000000000..1ee905a99 --- /dev/null +++ b/pkg/api/server/idletracker/idletracker.go @@ -0,0 +1,74 @@ +package idletracker + +import ( + "net" + "net/http" + "sync" + "time" + + "github.com/sirupsen/logrus" +) + +type IdleTracker struct { + http map[net.Conn]struct{} + hijacked int + total int + mux sync.Mutex + timer *time.Timer + Duration time.Duration +} + +func NewIdleTracker(idle time.Duration) *IdleTracker { + return &IdleTracker{ + http: make(map[net.Conn]struct{}), + Duration: idle, + timer: time.NewTimer(idle), + } +} + +func (t *IdleTracker) ConnState(conn net.Conn, state http.ConnState) { + t.mux.Lock() + defer t.mux.Unlock() + + oldActive := t.ActiveConnections() + logrus.Debugf("IdleTracker %p:%v %d/%d connection(s)", conn, state, oldActive, t.TotalConnections()) + switch state { + case http.StateNew, http.StateActive: + t.http[conn] = struct{}{} + // stop the timer if we transitioned from idle + if oldActive == 0 { + t.timer.Stop() + } + t.total++ + case http.StateHijacked: + // hijacked connections are handled elsewhere + delete(t.http, conn) + t.hijacked++ + case http.StateIdle, http.StateClosed: + delete(t.http, conn) + // Restart the timer if we've become idle + if oldActive > 0 && len(t.http) == 0 { + t.timer.Stop() + t.timer.Reset(t.Duration) + } + } +} + +func (t *IdleTracker) TrackHijackedClosed() { + t.mux.Lock() + defer t.mux.Unlock() + + t.hijacked-- +} + +func (t *IdleTracker) ActiveConnections() int { + return len(t.http) + t.hijacked +} + +func (t *IdleTracker) TotalConnections() int { + return t.total +} + +func (t *IdleTracker) Done() <-chan time.Time { + return t.timer.C +} diff --git a/pkg/api/server/server.go b/pkg/api/server/server.go index f5b17ab68..1c6007745 100644 --- a/pkg/api/server/server.go +++ b/pkg/api/server/server.go @@ -10,12 +10,12 @@ import ( "runtime" goRuntime "runtime" "strings" - "sync" "syscall" "time" "github.com/containers/libpod/v2/libpod" "github.com/containers/libpod/v2/pkg/api/handlers" + "github.com/containers/libpod/v2/pkg/api/server/idletracker" "github.com/coreos/go-systemd/v22/activation" "github.com/gorilla/mux" "github.com/gorilla/schema" @@ -24,14 +24,14 @@ import ( ) type APIServer struct { - http.Server // The HTTP work happens here - *schema.Decoder // Decoder for Query parameters to structs - context.Context // Context to carry objects to handlers - *libpod.Runtime // Where the real work happens - net.Listener // mux for routing HTTP API calls to libpod routines - context.CancelFunc // Stop APIServer - idleTracker *IdleTracker // Track connections to support idle shutdown - pprof *http.Server // Sidecar http server for providing performance data + http.Server // The HTTP work happens here + *schema.Decoder // Decoder for Query parameters to structs + context.Context // Context to carry objects to handlers + *libpod.Runtime // Where the real work happens + net.Listener // mux for routing HTTP API calls to libpod routines + context.CancelFunc // Stop APIServer + idleTracker *idletracker.IdleTracker // Track connections to support idle shutdown + pprof *http.Server // Sidecar http server for providing performance data } // Number of seconds to wait for next request, if exceeded shutdown server @@ -68,7 +68,7 @@ func newServer(runtime *libpod.Runtime, duration time.Duration, listener *net.Li } router := mux.NewRouter().UseEncodedPath() - idle := NewIdleTracker(duration) + idle := idletracker.NewIdleTracker(duration) server := APIServer{ Server: http.Server{ @@ -231,55 +231,3 @@ func (s *APIServer) Shutdown() error { func (s *APIServer) Close() error { return s.Server.Close() } - -type IdleTracker struct { - active map[net.Conn]struct{} - total int - mux sync.Mutex - timer *time.Timer - Duration time.Duration -} - -func NewIdleTracker(idle time.Duration) *IdleTracker { - return &IdleTracker{ - active: make(map[net.Conn]struct{}), - Duration: idle, - timer: time.NewTimer(idle), - } -} - -func (t *IdleTracker) ConnState(conn net.Conn, state http.ConnState) { - t.mux.Lock() - defer t.mux.Unlock() - - oldActive := len(t.active) - logrus.Debugf("IdleTracker %p:%v %d/%d connection(s)", conn, state, t.ActiveConnections(), t.TotalConnections()) - switch state { - case http.StateNew, http.StateActive: - t.active[conn] = struct{}{} - // stop the timer if we transitioned from idle - if oldActive == 0 { - t.timer.Stop() - } - t.total++ - case http.StateIdle, http.StateClosed, http.StateHijacked: - delete(t.active, conn) - // Restart the timer if we've become idle - if oldActive > 0 && len(t.active) == 0 { - t.timer.Stop() - t.timer.Reset(t.Duration) - } - } -} - -func (t *IdleTracker) ActiveConnections() int { - return len(t.active) -} - -func (t *IdleTracker) TotalConnections() int { - return t.total -} - -func (t *IdleTracker) Done() <-chan time.Time { - return t.timer.C -} -- cgit v1.2.3-54-g00ecf