From a1ff93bdfedc30d707a1230d5d1cb8d987a7360f Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 15 Jan 2020 11:11:30 +0100 Subject: v2: stats: remove windows-specific fields `NumProcs` and `StorageStats` are windows specific and are not popoulated on Linux. Hence, we can safely remove them. Signed-off-by: Valentin Rothberg --- pkg/api/handlers/generic/containers_stats.go | 7 ------- 1 file changed, 7 deletions(-) (limited to 'pkg/api') diff --git a/pkg/api/handlers/generic/containers_stats.go b/pkg/api/handlers/generic/containers_stats.go index 0c4efc1df..538fc6f21 100644 --- a/pkg/api/handlers/generic/containers_stats.go +++ b/pkg/api/handlers/generic/containers_stats.go @@ -127,13 +127,6 @@ func StatsContainer(w http.ResponseWriter, r *http.Request) { IoTimeRecursive: nil, SectorsRecursive: nil, }, - NumProcs: 0, - StorageStats: docker.StorageStats{ - ReadCountNormalized: 0, - ReadSizeBytes: 0, - WriteCountNormalized: 0, - WriteSizeBytes: 0, - }, CPUStats: docker.CPUStats{ CPUUsage: docker.CPUUsage{ TotalUsage: cgroupStat.CPU.Usage.Total, -- cgit v1.2.3-54-g00ecf From 3c7b7761ceafc24f9fa42e5f08178d8c644d9017 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 15 Jan 2020 11:41:38 +0100 Subject: v2: stats: do not ignore errors We must check all errors and handle them properly. Otherwise, we can run into nil dereferences ultimately killing the service. Signed-off-by: Valentin Rothberg --- pkg/api/handlers/generic/containers_stats.go | 37 +++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 6 deletions(-) (limited to 'pkg/api') diff --git a/pkg/api/handlers/generic/containers_stats.go b/pkg/api/handlers/generic/containers_stats.go index 538fc6f21..eb9cfca4a 100644 --- a/pkg/api/handlers/generic/containers_stats.go +++ b/pkg/api/handlers/generic/containers_stats.go @@ -81,19 +81,44 @@ func StatsContainer(w http.ResponseWriter, r *http.Request) { time.Sleep(DefaultStatsPeriod) } - cgroupPath, _ := ctnr.CGroupPath() - cgroup, _ := cgroups.Load(cgroupPath) + cgroupPath, err := ctnr.CGroupPath() + if err != nil { + utils.InternalServerError(w, err) + return + } + + cgroup, err := cgroups.Load(cgroupPath) + if err != nil { + utils.InternalServerError(w, err) + return + } for ok := true; ok; ok = query.Stream { - state, _ := ctnr.State() + state, err := ctnr.State() + if err != nil { + utils.InternalServerError(w, err) + return + } if state != define.ContainerStateRunning { time.Sleep(10 * time.Second) continue } - stats, _ := ctnr.GetContainerStats(stats) - cgroupStat, _ := cgroup.Stat() - inspect, _ := ctnr.Inspect(false) + stats, err := ctnr.GetContainerStats(stats) + if err != nil { + utils.InternalServerError(w, err) + return + } + cgroupStat, err := cgroup.Stat() + if err != nil { + utils.InternalServerError(w, err) + return + } + inspect, err := ctnr.Inspect(false) + if err != nil { + utils.InternalServerError(w, err) + return + } net := make(map[string]docker.NetworkStats) net[inspect.NetworkSettings.EndpointID] = docker.NetworkStats{ -- cgit v1.2.3-54-g00ecf From 0f6955c30e26b5d8bad4eb2360993e7e3cf708d6 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 15 Jan 2020 12:22:56 +0100 Subject: v2: stats: fix errors Also add some comments. Signed-off-by: Valentin Rothberg --- pkg/api/handlers/generic/containers_stats.go | 56 +++++++++++++++++----------- pkg/api/handlers/utils/handler.go | 1 + 2 files changed, 36 insertions(+), 21 deletions(-) (limited to 'pkg/api') diff --git a/pkg/api/handlers/generic/containers_stats.go b/pkg/api/handlers/generic/containers_stats.go index eb9cfca4a..b8021378b 100644 --- a/pkg/api/handlers/generic/containers_stats.go +++ b/pkg/api/handlers/generic/containers_stats.go @@ -43,6 +43,8 @@ func StatsContainer(w http.ResponseWriter, r *http.Request) { return } + // If the container isn't returning, then let's not bother and return + // immediately. state, err := ctnr.State() if err != nil { utils.InternalServerError(w, err) @@ -56,15 +58,14 @@ func StatsContainer(w http.ResponseWriter, r *http.Request) { return } - var preRead time.Time - var preCPUStats docker.CPUStats - stats, err := ctnr.GetContainerStats(&libpod.ContainerStats{}) if err != nil { utils.InternalServerError(w, errors.Wrapf(err, "Failed to obtain Container %s stats", name)) return } + var preRead time.Time + var preCPUStats docker.CPUStats if query.Stream { preRead = time.Now() preCPUStats = docker.CPUStats{ @@ -81,16 +82,19 @@ func StatsContainer(w http.ResponseWriter, r *http.Request) { time.Sleep(DefaultStatsPeriod) } - cgroupPath, err := ctnr.CGroupPath() - if err != nil { - utils.InternalServerError(w, err) - return - } + // Anonymous function to get the cgroup on demand. + getCgroup := func() *cgroups.CgroupControl { + cgroupPath, err := ctnr.CGroupPath() + if err != nil { + return &cgroups.CgroupControl{} + } - cgroup, err := cgroups.Load(cgroupPath) - if err != nil { - utils.InternalServerError(w, err) - return + cgroup, err := cgroups.Load(cgroupPath) + if err != nil { + return &cgroups.CgroupControl{} + } + + return cgroup } for ok := true; ok; ok = query.Stream { @@ -104,12 +108,13 @@ func StatsContainer(w http.ResponseWriter, r *http.Request) { continue } - stats, err := ctnr.GetContainerStats(stats) + cgroup := getCgroup() + cgroupStat, err := cgroup.Stat() if err != nil { - utils.InternalServerError(w, err) - return + cgroupStat = &cgroups.Metrics{} } - cgroupStat, err := cgroup.Stat() + + stats, err := ctnr.GetContainerStats(stats) if err != nil { utils.InternalServerError(w, err) return @@ -120,8 +125,13 @@ func StatsContainer(w http.ResponseWriter, r *http.Request) { return } + // FIXME: network inspection does not yet work entirely net := make(map[string]docker.NetworkStats) - net[inspect.NetworkSettings.EndpointID] = docker.NetworkStats{ + networkName := inspect.NetworkSettings.EndpointID + if networkName == "" { + networkName = "network" + } + net[networkName] = docker.NetworkStats{ RxBytes: stats.NetInput, RxPackets: 0, RxErrors: 0, @@ -192,17 +202,21 @@ func StatsContainer(w http.ResponseWriter, r *http.Request) { preRead = s.Read bits, err := json.Marshal(s.CPUStats) if err != nil { - logrus.Errorf("unable to marshal cpu stats: %q", err) + logrus.Errorf("Unable to marshal cpu stats: %q", err) } if err := json.Unmarshal(bits, &preCPUStats); err != nil { - logrus.Errorf("unable to unmarshal previous stats: %q", err) + logrus.Errorf("Unable to unmarshal previous stats: %q", err) + } + + // Only sleep when we're streaming. + if query.Stream { + time.Sleep(DefaultStatsPeriod) } - time.Sleep(DefaultStatsPeriod) } } func toBlkioStatEntry(entries []cgroups.BlkIOEntry) []docker.BlkioStatEntry { - results := make([]docker.BlkioStatEntry, 0, len(entries)) + results := make([]docker.BlkioStatEntry, len(entries)) for i, e := range entries { bits, err := json.Marshal(e) if err != nil { diff --git a/pkg/api/handlers/utils/handler.go b/pkg/api/handlers/utils/handler.go index 8c2110f97..65698bfd3 100644 --- a/pkg/api/handlers/utils/handler.go +++ b/pkg/api/handlers/utils/handler.go @@ -33,6 +33,7 @@ func WriteResponse(w http.ResponseWriter, code int, value interface{}) { } func WriteJSON(w http.ResponseWriter, code int, value interface{}) { + // FIXME: we don't need to write the header in all/some circumstances. w.Header().Set("Content-Type", "application/json") w.WriteHeader(code) -- cgit v1.2.3-54-g00ecf From fb5e5973027afa7659831a7988208ade4e667c84 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 15 Jan 2020 13:14:56 +0100 Subject: v2: stats: rigorous error checks Also expect the container to be running. Signed-off-by: Valentin Rothberg --- pkg/api/handlers/generic/containers_stats.go | 43 +++++++++------------------- 1 file changed, 13 insertions(+), 30 deletions(-) (limited to 'pkg/api') diff --git a/pkg/api/handlers/generic/containers_stats.go b/pkg/api/handlers/generic/containers_stats.go index b8021378b..4fb856bdb 100644 --- a/pkg/api/handlers/generic/containers_stats.go +++ b/pkg/api/handlers/generic/containers_stats.go @@ -51,10 +51,7 @@ func StatsContainer(w http.ResponseWriter, r *http.Request) { return } if state != define.ContainerStateRunning && !query.Stream { - utils.WriteJSON(w, http.StatusOK, &handlers.Stats{StatsJSON: docker.StatsJSON{ - Name: ctnr.Name(), - ID: ctnr.ID(), - }}) + utils.InternalServerError(w, define.ErrCtrStateInvalid) return } @@ -82,44 +79,30 @@ func StatsContainer(w http.ResponseWriter, r *http.Request) { time.Sleep(DefaultStatsPeriod) } - // Anonymous function to get the cgroup on demand. - getCgroup := func() *cgroups.CgroupControl { - cgroupPath, err := ctnr.CGroupPath() - if err != nil { - return &cgroups.CgroupControl{} - } - - cgroup, err := cgroups.Load(cgroupPath) - if err != nil { - return &cgroups.CgroupControl{} - } - - return cgroup - } - for ok := true; ok; ok = query.Stream { - state, err := ctnr.State() + // Container stats + stats, err := ctnr.GetContainerStats(stats) if err != nil { utils.InternalServerError(w, err) return } - if state != define.ContainerStateRunning { - time.Sleep(10 * time.Second) - continue + inspect, err := ctnr.Inspect(false) + if err != nil { + utils.InternalServerError(w, err) + return } - - cgroup := getCgroup() - cgroupStat, err := cgroup.Stat() + // Cgroup stats + cgroupPath, err := ctnr.CGroupPath() if err != nil { - cgroupStat = &cgroups.Metrics{} + utils.InternalServerError(w, err) + return } - - stats, err := ctnr.GetContainerStats(stats) + cgroup, err := cgroups.Load(cgroupPath) if err != nil { utils.InternalServerError(w, err) return } - inspect, err := ctnr.Inspect(false) + cgroupStat, err := cgroup.Stat() if err != nil { utils.InternalServerError(w, err) return -- cgit v1.2.3-54-g00ecf From b9be9e6170faed90b2673788be08fb0a34da0962 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 15 Jan 2020 13:38:14 +0100 Subject: v2: stats: libpod: use generic handler The docker and libpod endpoints provide the same functionality, so we can use the same handler. Signed-off-by: Valentin Rothberg --- pkg/api/server/register_containers.go | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) (limited to 'pkg/api') diff --git a/pkg/api/server/register_containers.go b/pkg/api/server/register_containers.go index 711aecc84..95b986170 100644 --- a/pkg/api/server/register_containers.go +++ b/pkg/api/server/register_containers.go @@ -629,7 +629,30 @@ func (s *APIServer) RegisterContainersHandlers(r *mux.Router) error { // '500': // "$ref": "#/responses/InternalError" r.HandleFunc(VersionedPath("/libpod/containers/{name:..*}/start"), APIHandler(s.Context, handlers.StartContainer)).Methods(http.MethodPost) - r.HandleFunc(VersionedPath("/libpod/containers/{name:..*}/stats"), APIHandler(s.Context, libpod.StatsContainer)).Methods(http.MethodGet) + // swagger:operation GET /libpod/containers/{nameOrID}/stats containers statsContainer + // --- + // summary: Get stats for a container + // description: This returns a live stream of a container’s resource usage statistics. + // parameters: + // - in: path + // name: nameOrID + // required: true + // description: the name or ID of the container + // - in: query + // name: stream + // type: bool + // default: true + // description: Stream the output + // produces: + // - application/json + // responses: + // '200': + // description: no error + // '404': + // "$ref": "#/responses/NoSuchContainer" + // '500': + // "$ref": "#/responses/InternalError" + r.HandleFunc(VersionedPath("/libpod/containers/{name:..*}/stats"), APIHandler(s.Context, generic.StatsContainer)).Methods(http.MethodGet) r.HandleFunc(VersionedPath("/libpod/containers/{name:..*}/top"), APIHandler(s.Context, handlers.TopContainer)).Methods(http.MethodGet) // swagger:operation POST /libpod/containers/{nameOrID}/unpause containers libpodUnpauseContainer // --- -- cgit v1.2.3-54-g00ecf From d8d42512c15b32a9013938018cdc0f441561944e Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 15 Jan 2020 13:41:15 +0100 Subject: v2: stats: drop redundant sleep when streaming Also remove the redundant stats handler in libpod. Signed-off-by: Valentin Rothberg --- pkg/api/handlers/generic/containers_stats.go | 1 - pkg/api/handlers/libpod/containers.go | 4 +--- 2 files changed, 1 insertion(+), 4 deletions(-) (limited to 'pkg/api') diff --git a/pkg/api/handlers/generic/containers_stats.go b/pkg/api/handlers/generic/containers_stats.go index 4fb856bdb..26c8efa15 100644 --- a/pkg/api/handlers/generic/containers_stats.go +++ b/pkg/api/handlers/generic/containers_stats.go @@ -76,7 +76,6 @@ func StatsContainer(w http.ResponseWriter, r *http.Request) { OnlineCPUs: 0, ThrottlingData: docker.ThrottlingData{}, } - time.Sleep(DefaultStatsPeriod) } for ok := true; ok; ok = query.Stream { diff --git a/pkg/api/handlers/libpod/containers.go b/pkg/api/handlers/libpod/containers.go index bfb028b1b..ceb7460da 100644 --- a/pkg/api/handlers/libpod/containers.go +++ b/pkg/api/handlers/libpod/containers.go @@ -143,9 +143,7 @@ func LogsFromContainer(w http.ResponseWriter, r *http.Request) { // timestamps // tail string } -func StatsContainer(w http.ResponseWriter, r *http.Request) { - //stream -} + func CreateContainer(w http.ResponseWriter, r *http.Request) { } -- cgit v1.2.3-54-g00ecf