containers / psgo Goto Github PK

proc.readStat simply splits the fields by whitespace, which is not robust if the process name contains whitespace.

The correct algorithm for parsing /proc/.../stat is:

1. find the location of the first (.
2. find the location of the first ) starting from the end.
3. the text between these locations is COMM.
4. the integer before the first ( is the PID.
5. split the rest and assign them sequently.

Currently, we are using github.com/shirou/gopsutil for most of the /proc parsing, which does a great job and is a powerful library, but it does much more than we need. To be a bit more independent, let's parse /proc directly in containers/psgo.

Note that parsing /proc is racy and that we should have a consistent way of dealing with that (e.g., an ErrNoPID error that would yield to a given process being skipped).

Another big advantage of parsing /proc in containers/psgo is that we can do a thorough unit testing, see #4.

This could help users understand what is going on within the container when using something like podman top.

Concurrent accesses will yield surprising results. We can get the library thread-safe by passing a context around, which should be preferred over locking.

Moved over from a Podman issue

The groups command sees:
grep Groups /proc/self/status
Groups: 65534 65534 65534 65534 65534 65534 65534 65534 65534 65534 65534 65534 5829 65534

And then reports them as
5829 65534

If you look at that process from outside of the container you would see that the process still has the groups you expect.

grep Groups /proc/$PID/status
Groups: 25 160 162 998 1039 1072 1101 1109 1489 1501 1606 1608 5829 9250

@vrothberg we probably should add a podman top -o hgroups

Originally posted by @rhatdan in containers/podman#4185 (comment)

podman 2.2.1 fails to build on MIPS (both little and big endian, 32-bit as well as 64-bit) due to some type incompatibility:

# github.com/containers/psgo/internal/dev
../../../../../dl/go-mod-cache/github.com/containers/[email protected]/internal/dev/tty.go:116:22: cannot use s.Rdev (type uint32) as type uint64 in argument to minDevNum
../../../../../dl/go-mod-cache/github.com/containers/[email protected]/internal/dev/tty.go:117:22: cannot use s.Rdev (type uint32) as type uint64 in argument to majDevNum

Please see the links below for full build logs:

https://downloads.openwrt.org/snapshots/faillogs/mips64_octeonplus/packages/podman/compile.txt

https://downloads.openwrt.org/snapshots/faillogs/mips_24kc/packages/podman/compile.txt

https://downloads.openwrt.org/snapshots/faillogs/mipsel_24kc/packages/podman/compile.txt

We need to thorough tests. Currently, make test is implemented in make but should be done in something less limited and easier to extend (e.g., bats).

We should as unit tests as well. One way to make the deterministic is to create process objects on our own and then compare the output based on different format strings.

grep -i "^seccomp:" /proc/PID/status
Seccomp: 0

Hi Valentin, it would be great to have an extra psgo API where you could limit the scope of ProcessInfo to a specified set of pids.

One use case I found is being able to get the the full 64-bit process capabilities bitmask from procfs.
Something like this would probably suffice:

// ProcessInfoByPids is like ProcessInfo but limited to a set of pids
func ProcessInfoByPids(pids []string, descriptors []string) ([][]string, error) {
       aixDescriptors, err := translateDescriptors(descriptors)
       if err != nil {
               return nil, err
       }

       processes, err := process.FromPIDs(pids)
       if err != nil {
               return nil, err
       }

       return processDescriptors(aixDescriptors, processes)
}

ps (1) returns the elapsed time and cpu time in the [[DD-]hh:]mm:ss form. Currently, we are returning the time.Duration format which is something like 9h49m21.457862147s.

We should expose a function to convert the times from the time.Duration format to the ps1 [[DD-]hh:]mm:ss, so that users could change that at wish. Maybe, the API could also be extended with some context or option struct that controls such details in an extensible way.

Most people have no idea what capabilities their processes or containers are running with.
Extracting the data out of /proc/PID/status you get
CapEff
CapBind
These are bit maps, need to translate these bitmaps into something humans can understand.
pscap does this now.

psgo's COMM also parses /proc/.../cmdline, but ps(1) uses the 2nd field from /proc/.../stat.
This makes a difference when a program modifies argv[0].

Dependabot couldn't parse the go.mod found at /go.mod.

The error Dependabot encountered was:

go: github.com/stretchr/[email protected] requires
	gopkg.in/[email protected]: unrecognized import path "gopkg.in/yaml.v2" (https fetch: Get https://gopkg.in/yaml.v2?go-get=1: EOF)

View the update logs.

We should have a more verbose README and mention all descriptors also in the docs to make them visible for godocs.

Some background:

I'm working on a conmon monitoring loop in CRI-O to make sure we catch conmon OOMs in cri-o/cri-o#3022. We want to be absolutely sure that the conmon PID we have saved is that which is attached to the container. We have a couple of heuristics that do so, and psgo is a great way to filter those heuristics.

One thing I wish psgo already supported is exposing starttime. etime would require some calculation and depends on a consistent string format to compare (last-saved-etime + elapsed time == etime).

I thought about using the Process structure directly, but it's internal/ so I can't import it.

Left with no other choice, I come here, on my knees, begging for the start time to be exposed via the API, even though it's not techincally in the AIX format.

WDYT?

ps -eZ or ps -auxwwZ extracts the SELinux label, or other MAC Lables (SMACK)
cat /proc/PID/attr/current
has the data.

The project is Apache 2.0 licensed but there's no actual copyright line anywhere in the project. Can this be added (whether it's to all files -- GNU style -- or just to the README this would be quite invaluable from a licensing perspective).

I want to use this issue to discuss if and to which extent we aim to be compatible with Docker and ps (1). There are several issues that we might need to sort out depending on the decision (all mentioned below).

docker-top

The output of docker-top uses more space than necessary (because the way text/tabwriter is used):

# docker run -d alpine sleep 100
196ced4826d9d4a4e9f8f307878c9362a2c0cf85a6033db7a77d33ece9e1b082
# docker top 196ced4826d9d4a4e9f8f307878c9362a2c0cf85a6033db7a77d33ece9e1b082
UID                 PID                 PPID                C                   STIME               TTY                 TIME                CMD      
root                19247               19230               0                   12:36               ?                   00:00:00            sleep 100

Do we want to keep the same configuration of the tabular out?

(my personal opinion: no, we should compress it a bit more so it's readable on a normal-sized terminal)

time format

ps (1) returns the elapsed time and cpu time in the [[DD-]hh:]mm:ss form. Currently, we are returning the time.Duration format which is something like 9h49m21.457862147s.

Shall we change the format? It would require some additional logic to calcuate it.

(my personal opinion: no, we're in golang and could remain compatible to the time package of the stdlib)

/proc parsing

Currenlty, we are using github.com/shirou/gopsutil for most of the /proc parsing, which does a great job and allows to do much more than we need. However, one thing is currently missing, namely to extract the process group ID (ppid) forcing processPGID(...) in ps/ps.go to parse /proc/$pid/stat. This is a race condition since the process could already be dead (and already led to a failing Travis job).

Hence the question: Shall we do the /proc parsing here or keep using github.com/shirou/gopsutil?

(my opinion: I have a slight tendency to do the parsing on our own, so we can deal with consistently with race conditions. It also would avoid some hops in case we want to extend the library. Some fields in /proc/$pid/stat, for instance, are only visible by certain versions of Linux.)

Cc: @rhatdan @mheon @baude

When running a container with
podman top CID pid, hpid

Would be nice to describe the mapping between the PIDs.

sh-4.4# podman run -d --uidmap 0:100000:5000 fedora sleep 1000
7d54a703ca278cbef43c5072f44d8c39622b4216e9d8b64bf04842038aff61b3
sh-4.4# podman top --latest user huser
USER HUSER
100000 100000

You need to go into the User Namespace and the mount namespace of the container before collecting this information.

HUSER needs to be collected on the outside obviously.