Skip to content

Commit

Permalink
Changed GetContainerStats to return ErrCtrStateInvalid
Browse files Browse the repository at this point in the history
This results in some functionality changes:

If a ErrCtrStateInvalid is returned to GetPodStats, the container is ommitted from the stats.
As such, if an empty slice of Container stats are returned to GetPodStats in varlink, an error will occur.
GetContainerStats will return the ErrCtrStateInvalid as well.
Finally, if ErrCtrStateInvalid is returned to the podman stats call, the container will be ommitted from the stats.

Signed-off-by: haircommander <pehunt@redhat.com>

Closes: #1319
Approved by: baude
  • Loading branch information
haircommander authored and rh-atomic-bot committed Aug 23, 2018
1 parent 3df6332 commit 63dd200
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 6 deletions.
2 changes: 1 addition & 1 deletion cmd/podman/stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func statsCmd(c *cli.Context) error {
id := ctr.ID()
if _, ok := containerStats[ctr.ID()]; !ok {
initialStats, err := ctr.GetContainerStats(&libpod.ContainerStats{})
if errors.Cause(err) == libpod.ErrCtrRemoved || errors.Cause(err) == libpod.ErrNoSuchCtr {
if errors.Cause(err) == libpod.ErrCtrRemoved || errors.Cause(err) == libpod.ErrNoSuchCtr || errors.Cause(err) == libpod.ErrCtrStateInvalid {
// skip dealing with a container that is gone
continue
}
Expand Down
7 changes: 6 additions & 1 deletion cmd/podman/varlink/io.podman.varlink
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,8 @@ method ExportContainer(name: string, path: string) -> (tarfile: string)

# GetContainerStats takes the name or ID of a container and returns a single ContainerStats structure which
# contains attributes like memory and cpu usage. If the container cannot be found, a
# [ContainerNotFound](#ContainerNotFound) error will be returned.
# [ContainerNotFound](#ContainerNotFound) error will be returned. If the container is not running, a [NoContainerRunning](#NoContainerRunning)
# error will be returned
# #### Example
# ~~~
# $ varlink call -m unix:/run/podman/io.podman/io.podman.GetContainerStats '{"name": "c33e4164f384"}'
Expand Down Expand Up @@ -759,6 +760,7 @@ method TopPod() -> (notimplemented: NotImplemented)

# GetPodStats takes the name or ID of a pod and returns a pod name and slice of ContainerStats structure which
# contains attributes like memory and cpu usage. If the pod cannot be found, a [PodNotFound](#PodNotFound)
# error will be returned. If the pod has no running containers associated with it, a [NoContainerRunning](#NoContainerRunning)
# error will be returned.
# #### Example
# ~~~
Expand Down Expand Up @@ -792,6 +794,9 @@ error ImageNotFound (name: string)
# ContainerNotFound means the container could not be found by the provided name or ID in local storage.
error ContainerNotFound (name: string)

# NoContainerRunning means none of the containers requested are running in a command that requires a running container.
error NoContainerRunning ()

# PodNotFound means the pod could not be found by the provided name or ID in local storage.
error PodNotFound (name: string)

Expand Down
9 changes: 7 additions & 2 deletions libpod/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"time"

"github.com/containers/storage"
"github.com/pkg/errors"
)

// Pod represents a group of containers that are managed together.
Expand Down Expand Up @@ -192,10 +193,14 @@ func (p *Pod) GetPodStats(previousContainerStats map[string]*ContainerStats) (ma
prevStat = &ContainerStats{}
}
newStats, err := c.GetContainerStats(prevStat)
if err != nil {
// If the container wasn't running, don't include it
// but also suppress the error
if err != nil && errors.Cause(err) != ErrCtrStateInvalid {
return nil, err
}
newContainerStats[c.ID()] = newStats
if err == nil {
newContainerStats[c.ID()] = newStats
}
}
return newContainerStats, nil
}
2 changes: 1 addition & 1 deletion libpod/pod_ffjson.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion libpod/stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func (c *Container) GetContainerStats(previousStats *ContainerStats) (*Container
}

if c.state.State != ContainerStateRunning {
return stats, nil
return stats, ErrCtrStateInvalid
}

cgroupPath, err := c.CGroupPath()
Expand Down
3 changes: 3 additions & 0 deletions pkg/varlinkapi/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@ func (i *LibpodAPI) GetContainerStats(call iopodman.VarlinkCall, name string) er
}
containerStats, err := ctr.GetContainerStats(&libpod.ContainerStats{})
if err != nil {
if errors.Cause(err) == libpod.ErrCtrStateInvalid {
return call.ReplyNoContainerRunning()
}
return call.ReplyErrorOccurred(err.Error())
}
cs := iopodman.ContainerStats{
Expand Down
3 changes: 3 additions & 0 deletions pkg/varlinkapi/pods.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ func (i *LibpodAPI) GetPodStats(call iopodman.VarlinkCall, name string) error {
if err != nil {
return call.ReplyErrorOccurred(err.Error())
}
if len(podStats) == 0 {
return call.ReplyNoContainerRunning()
}
containersStats := make([]iopodman.ContainerStats, 0)
for ctrID, containerStats := range podStats {
cs := iopodman.ContainerStats{
Expand Down

0 comments on commit 63dd200

Please sign in to comment.