{"action":"created","issue":{"url":"https://api.github.com/repos/docker/docker/issues/9232","labels_url":"https://api.github.com/repos/docker/docker/issues/9232/labels{/name}","comments_url":"https://api.github.com/repos/docker/docker/issues/9232/comments","events_url":"https://api.github.com/repos/docker/docker/issues/9232/events","html_url":"https://github.com/docker/docker/pull/9232","id":49340912,"number":9232,"title":"Proposal: Make API for `/containers/(id)/top` independent of implementation","user":{"login":"joshwilsdon","id":78194,"avatar_url":"https://avatars.githubusercontent.com/u/78194?v=3","gravatar_id":"","url":"https://api.github.com/users/joshwilsdon","html_url":"https://github.com/joshwilsdon","followers_url":"https://api.github.com/users/joshwilsdon/followers","following_url":"https://api.github.com/users/joshwilsdon/following{/other_user}","gists_url":"https://api.github.com/users/joshwilsdon/gists{/gist_id}","starred_url":"https://api.github.com/users/joshwilsdon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/joshwilsdon/subscriptions","organizations_url":"https://api.github.com/users/joshwilsdon/orgs","repos_url":"https://api.github.com/users/joshwilsdon/repos","events_url":"https://api.github.com/users/joshwilsdon/events{/privacy}","received_events_url":"https://api.github.com/users/joshwilsdon/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/docker/docker/labels//project/doc","name":"/project/doc","color":"207de5"},{"url":"https://api.github.com/repos/docker/docker/labels/Proposal","name":"Proposal","color":"009800"}],"state":"closed","locked":false,"assignee":null,"milestone":null,"comments":16,"created_at":"2014-11-19T07:12:17Z","updated_at":"2015-03-18T21:28:36Z","closed_at":"2015-03-17T00:10:04Z","pull_request":{"url":"https://api.github.com/repos/docker/docker/pulls/9232","html_url":"https://github.com/docker/docker/pull/9232","diff_url":"https://github.com/docker/docker/pull/9232.diff","patch_url":"https://github.com/docker/docker/pull/9232.patch"},"body":"# Background and Problem\r\n\r\nIssue #7205 and discussion therein, describes some problems with the current API with regard to the:\r\n\r\n```\r\nGET /containers/(id)/top\r\n```\r\n\r\nAPI endpoint. I think these problems can be boiled down to a single issue which is that\r\nthe API includes ps_args which are not bounded and are not in control of the API itself.\r\nThe result is different output on different systems even when using the same arguments.\r\nThe existing implementation uses `ps` to gather the data but the API does not allow\r\nalternate implementations because it is not fully defined.\r\n\r\nThis proposal also makes it possible to normalize the behavior across existing hosts,\r\nas different docker hosts with slightly different versions of ps already exhibit minor\r\ndifferences in response (see for example #8075).\r\n\r\nAs docker is ported to other platforms which may or may not have the same behavior and\r\nmay in fact not even have a `ps` command host-side, I think this will become more of an\r\nissue. I'm making this proposal in attempt to make something which can be supported by\r\nother platforms and also be implemented differently on Linux hosts in the future (Eg.\r\nthe /proc reader that was suggested in #7205).\r\n\r\n# Existing Behavior\r\n\r\nOn an Ubuntu 14.04 host with docker 1.3.1 the default fields I get when I use\r\n`docker top <id>` with no ps_args are:\r\n\r\n * UID\r\n * PID\r\n * PPID\r\n * C\r\n * STIME\r\n * TTY\r\n * TIME\r\n * CMD\r\n\r\nWhich is the equivalent to running `ps -ef` on the host.\r\n\r\nThe API docs for v1.15 also refer to `aux` as an example for ps_args. With that\r\nthe fields are:\r\n\r\n * USER\r\n * PID\r\n * %CPU\r\n * %MEM\r\n * VSZ\r\n * RSS\r\n * TTY\r\n * STAT\r\n * START\r\n * TIME\r\n * COMMAND\r\n\r\nwhich actually has very little overlap with the default set.\r\n\r\nWhat I'd like to propose is that the `ps_args` parameter go away, to be replaced\r\nby a `fields` parameter which defines the ordered set of fields that should be in\r\nthe output. The possible fields should be clearly defined and a finite set. I've\r\nattempted to err here on the side of including fewer fields as it's much easier\r\nto add them later than to include them now and take them away later.\r\n\r\nI've also attempted to move away from implementation-specific formatting for\r\nthings like 'start_time' and 'cputime' which on Linux are formatted as \"MmmDD\"\r\nor \"HH:MM\" for 'start_time' (depending how old) and '[DD-]hh:mm:ss' for\r\n'cputime'. These are both confusing formats and also lose information that\r\nimplementations may otherwise have. For example, on the same Linux system\r\n/proc/<pid>/sched contains this information with microsecond precision.\r\n\r\n# Proposal\r\n\r\nI propose that initially the set of allowed 'fields' would include:\r\n\r\n| field       | type                        | description                                                                                               |\r\n| ----------- | --------------------------- | --------------------------------------------------------------------------------------------------------- |\r\n| c           | integer                     | processor utilization over process lifetime                                                               |\r\n| comm        | string                      | command name (only the executable name)                                                                   |\r\n| command     | string                      | command with all its arguments                                                                            |\r\n| cputime     | number                      | cumulative CPU time for the process in seconds, with or without decimals                                  |\r\n| gid         | integer                     | effective group ID number of the process as a decimal integer                                             |\r\n| lwp         | integer                     | light weight process (thread) ID of the dispatchable entity                                               |\r\n| nice        | integer                     | nice value. This ranges from 19 (nicest) to -20 (not nice to others)                                      |\r\n| pcpu        | number                      | cpu utilization of the process as a percentage of its run time, with or without decimals                  |\r\n| pid         | integer                     | a number representing the process ID                                                                      |\r\n| pgid        | integer                     | process group ID or, equivalently, the process ID of the process group leader                             |\r\n| pmem        | integer                     | ratio of the process's resident set size to the physical memory on the machine, expressed as a percentage |\r\n| ppid        | integer                     | parent process ID                                                                                         |\r\n| psr         | integer                     | processor that process is currently assigned to                                                           |\r\n| rgid        | integer                     | real group ID                                                                                             |\r\n| rss         | integer                     | resident set size, the non-swapped physical memory that a task has used in KiB                            |\r\n| ruid        | integer                     | real user ID                                                                                              |\r\n| start_time  | number                      | time the process started (seconds since 1970-01-01 00:00:00 UTC), with or without decimals                |\r\n| state       | string (see 'state fields') | normalized state of the process                                                                           |\r\n| state_flags | string (see 'state fields') | implementation-specific process state flags                                                               |\r\n| tty         | string                      | controlling tty (terminal)                                                                                |\r\n| uid         | integer                     | effective user ID                                                                                         |\r\n| vsz         | integer                     | virtual size of the process in KiB                                                                        |\r\n\r\nSo the default for `fields` would be:\r\n\r\n```\r\nfields=uid,pid,ppid,c,start_time,tty,cputime,command\r\n```\r\n\r\nwhich contains all the data the client would require in order to generate the\r\nexisting default v1.15 output of 'docker top', whether the client would want to\r\ndisplay it this way or in some more useful fashion.\r\n\r\nI'd also define a special option:\r\n\r\n```\r\nfields=all\r\n```\r\n\r\nwhich would be identical to all the fields:\r\n\r\n```\r\nfields=c,comm,command,cputime,gid,lwp,nice,pcpu,pid,pgid,pmem,ppid,psr,rgid,rss,ruid,start_time,state,state_flags,tty,uid,vsz\r\n```\r\n\r\nNOTE:\r\n\r\n * The fields list purposefully does not include an analog to the \"USER\" ps\r\n   field, because it doesn't seem likely that most implementations (the current\r\n   one doesn't for example) will display the user names here from the guest's\r\n   context, and would fallback to UID anyway.\r\n\r\n## state fields\r\n\r\nIn order to make the `state` and `state_flags` fields more generic and in\r\nattempt to make them more portable these fields should be defined as follows\r\nfor `state` (`ps state` is included here for reference only):\r\n\r\n| State           |`ps state` | Meaning                                                               |\r\n| --------------- | --------- | --------------------------------------------------------------------- |\r\n| uninterruptible | D         | uninterruptible sleep (usually IO)                                    |\r\n| running         | R         | running or runnable (on run queue)                                    |\r\n| sleep           | S         | interruptible sleep (waiting for an event to complete)                |\r\n| stopped         | T         | stopped, either by a job control signal or because it is being traced |\r\n| zombie          | Z         | defunct (\"zombie\") process, terminated but not reaped by its parent   |\r\n\r\nAnd the `state_flags` should be a comma separated list of strings representing\r\nIt is possible that some implementations will not support these flags. As such,\r\nclients should not fail when this list is empty. Supported values at this point\r\nshould include (`ps stat` here represents the Linux character used to indicate\r\nthis flag with the `stat` field, again only for reference):\r\n\r\n| Flag       | `ps stat` | Meaning                                                       |\r\n| ---------  | --------- | ------------------------------------------------------------- |\r\n| high       | <         | high-priority (not nice to other users)                       |\r\n| low        | N         | low-priority (nice to other users)                            |\r\n| locked     | L         | has pages locked into memory (for real-time and custom IO)    |\r\n| leader     | s         | is a session leader                                           |\r\n| threads    | l         | is multi-threaded (using CLONE_THREAD, like NPTL pthreads do) |\r\n| foreground | +         | is in the foreground process group                            |\r\n\r\nSo an example `ps aux` output that included:\r\n\r\n```\r\nsyslog     948  0.0  1.6 258300 17260 ?        Ssl  Nov06   0:08 rsyslogd\r\nroot      1037  0.0  0.0      0     0 ?        S<   Nov06   0:00 [dm_bufio_cache]\r\nroot      1114  0.0  0.0  14540   956 tty4     Ss+  Nov06   0:00 /sbin/getty -8 38400 tty4\r\n```\r\n\r\nThe `state` field for all three of these problems would be 'sleep' and the\r\n`state_flags` fields would be:\r\n\r\n * leader,threads\r\n * high\r\n * foreground,leader\r\n\r\nrespectively.\r\n\r\n# Backward compatibility (optional)\r\n\r\nFor the initial version (in order to support existing usage) ps_args could still\r\nbe supported but deprecated and defined in the above terms. Eg.\r\n\r\n```\r\nps_args=aux\r\n```\r\n\r\nwould be translated into:\r\n\r\n```\r\nfields=uid,pid,pcpu,pmem,vsz,rss,tty,stat,stime,time,command\r\n```\r\n\r\nhowever a specific limited set of ps_args values would need to be chosen here.\r\n\r\nMy own bias would be toward keeping compatibility only for older clients. Ie. have\r\nthe code use the old behavior for v1.15 and older clients and the new behavior only\r\nfor v1.16 and higher clients.\r\n\r\n# Responses\r\n\r\nCurrent responses look like (copied from the v1.15 API doc):\r\n\r\n```\r\n    {\r\n         \"Titles\":[\r\n                 \"USER\",\r\n                 \"PID\",\r\n                 \"%CPU\",\r\n                 \"%MEM\",\r\n                 \"VSZ\",\r\n                 \"RSS\",\r\n                 \"TTY\",\r\n                 \"STAT\",\r\n                 \"START\",\r\n                 \"TIME\",\r\n                 \"COMMAND\"\r\n                 ],\r\n         \"Processes\":[\r\n                 [\"root\",\"20147\",\"0.0\",\"0.1\",\"18060\",\"1864\",\"pts/4\",\"S\",\"10:06\",\"0:00\",\"bash\"],\r\n                 [\"root\",\"20271\",\"0.0\",\"0.0\",\"4312\",\"352\",\"pts/4\",\"S+\",\"10:07\",\"0:00\",\"sleep\",\"10\"]\r\n         ]\r\n    }\r\n```\r\n\r\nand I propose to change the output to something like:\r\n\r\n```\r\n    {\r\n         \"Processes\":[\r\n                 {\r\n                          \"uid\": 0,\r\n                          \"pid\": 20147,\r\n                          \"pcpu\": 0.0,\r\n                          \"pmem\": 0.1,\r\n                          \"vsz\": 18060,\r\n                          \"rss\": 1864,\r\n                          \"tty\": \"pts/4\",\r\n                          \"state\": \"stopped\",\r\n                          \"state_flags\": \"\",\r\n                          \"start_time\": 1416305160,\r\n                          \"time\": 0.0001,\r\n                          \"command\": \"bash\"\r\n                 }, {\r\n                          \"uid\": 0,\r\n                          \"pid\": 20271,\r\n                          \"pcpu\": 0.0,\r\n                          \"pmem\": 0.0,\r\n                          \"vsz\": 4312,\r\n                          \"rss\": 352,\r\n                          \"tty\": \"pts/4\",\r\n                          \"state\": \"stopped\",\r\n                          \"state_flags\": \"foreground\",\r\n                          \"start_time\": 1416305220,\r\n                          \"time\": 0.0002,\r\n                          \"command\": \"sleep 10\"\r\n                 }\r\n         ]\r\n    }\r\n```\r\n\r\nwhich makes it easier for the client to know the which field is which in the\r\nresponse. Combined with 'fields=all' this also allows a client to determine\r\nwhich fields a given implementation/server-version supports.\r\n\r\nI would also suggest the addition of an additional status code:\r\n\r\n-   **422** – invalid fields\r\n\r\nfor the case where invalid values are requested for `fields`.\r\n\r\n# CLI changes\r\n\r\nIn order to support this new API, I propose the CLI interface to this endpoint be\r\nmodified from:\r\n\r\n```\r\ndocker ps [ps ARGS]\r\n```\r\n\r\nto:\r\n\r\n```\r\ndocker ps [FIELDS]\r\n```\r\n\r\nwith the fields matching those defined by the API. The output here would be the requested\r\nfields in the order they were requested. Eg:\r\n\r\n```\r\n$ sudo docker top 8601afda2b pid tty state state_flags cpu_time command\r\npid      tty       state    state_flags   cpu_time    command\r\n16623    ?         S        leader        0.03        sleep 99999\r\n```\r\n\r\n# Implementation notes\r\n\r\nThe initial implementation here in docker would be straight-forward since all these\r\nfields map to ps fields and the initial implementation could be splitting apart the\r\nfields from:\r\n\r\n```\r\nps -Ho <fields>\r\n```\r\n\r\nWith some translation on the server side. Specifically:\r\n\r\n * cputime would need to be turned into a number from the 'HH:MM:SS' default format of 'cputime'\r\n * start_time would need to be turned into a number from the default format of 'start_time'\r\n * state would need to be a simple table-translation of single-character state -> string\r\n * state_flags would be a simple table-translation of all the special 'stat' characters -> array of strings\r\n\r\nOther implementations however would now be much simpler as we could for\r\nexample read data out of /proc and generate objects with the specified fields\r\nwithout a problem."},"comment":{"url":"https://api.github.com/repos/docker/docker/issues/comments/83186387","html_url":"https://github.com/docker/docker/pull/9232#issuecomment-83186387","issue_url":"https://api.github.com/repos/docker/docker/issues/9232","id":83186387,"user":{"login":"tiborvass","id":827131,"avatar_url":"https://avatars.githubusercontent.com/u/827131?v=3","gravatar_id":"","url":"https://api.github.com/users/tiborvass","html_url":"https://github.com/tiborvass","followers_url":"https://api.github.com/users/tiborvass/followers","following_url":"https://api.github.com/users/tiborvass/following{/other_user}","gists_url":"https://api.github.com/users/tiborvass/gists{/gist_id}","starred_url":"https://api.github.com/users/tiborvass/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tiborvass/subscriptions","organizations_url":"https://api.github.com/users/tiborvass/orgs","repos_url":"https://api.github.com/users/tiborvass/repos","events_url":"https://api.github.com/users/tiborvass/events{/privacy}","received_events_url":"https://api.github.com/users/tiborvass/received_events","type":"User","site_admin":false},"created_at":"2015-03-18T21:28:36Z","updated_at":"2015-03-18T21:28:36Z","body":"@jhowardmsft what do you think of this?"}}