Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[APIv2] events?filters return 500 #6899

Closed
skywalker512 opened this issue Jul 8, 2020 · 15 comments · Fixed by #6902
Closed

[APIv2] events?filters return 500 #6899

skywalker512 opened this issue Jul 8, 2020 · 15 comments · Fixed by #6902
Assignees
Labels
locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@skywalker512
Copy link

/kind bug

Description

treafik v2.2.1 will call on /v1.24/events?filters=%7B%22type%22%3A%7B%22container%22%3Atrue%7D%7D but return 500

Steps to reproduce the issue:

  1. podman system service -t 0 tcp:0.0.0.0:9999

  2. start treafik with endpoint = "tcp://127.0.0.1:9999" config

Describe the results you received:

{
    "cause": "runtime error: index out of range [0] with length 0",
    "message": "runtime error: index out of range [0] with length 0",
    "response": 500
}

Describe the results you expected:

{"Type":"container","Action":"stop","Actor":{"ID":"7f6e4f3b81349a57bd253cebb20f7b4fa98597d331f23fe767f862dcc21ece34","Attributes":{"containerExitCode":"0","image":"docker.io/library/traefik:latest","name":"traefik-main"}},"scope":"local","time":1594180142,"timeNano":1594180142023436891}

Additional information you deem important (e.g. issue happens only occasionally):

Output of podman version:

Version:      2.0.2
API Version:  1
Go Version:   go1.14
Git Commit:   201c9505b88f451ca877d29a73ed0f1836bb96c7-dirty
Built:        Thu Jan  1 08:00:01 1970
OS/Arch:      linux/amd64

Output of podman info --debug:

host:
  arch: amd64
  buildahVersion: 1.15.0
  cgroupVersion: v1
  conmon:
    package: Unknown
    path: /usr/libexec/podman/conmon
    version: 'conmon version 2.0.19-dev, commit: ce64ca841e1b12954cb3c972e0c06ec69a7b31ce'
  cpus: 2
  distribution:
    distribution: ubuntu
    version: "20.04"
  eventLogger: file
  hostname: iZ2ze8mxxl9wg4rrytkeuqZ
  idMappings:
    gidmap: null
    uidmap: null
  kernel: 5.4.0-31-generic
  linkmode: static
  memFree: 1043369984
  memTotal: 4127342592
  ociRuntime:
    name: runc
    package: Unknown
    path: /usr/bin/runc
    version: |-
      runc version 1.0.0-rc91
      spec: 1.0.2-dev
  os: linux
  remoteSocket:
    path: /run/podman/podman.sock
  rootless: false
  slirp4netns:
    executable: ""
    package: ""
    version: ""
  swapFree: 0
  swapTotal: 0
  uptime: 23h 47m 10.39s (Approximately 0.96 days)
registries:
  docker.io:
    Blocked: false
    Insecure: false
    Location: docker.io
    MirrorByDigestOnly: false
    Mirrors:
    - Insecure: false
      Location: 1nj0zren.mirror.aliyuncs.com
    Prefix: docker.io
  search:
  - docker.io
store:
  configFile: /etc/containers/storage.conf
  containerStore:
    number: 2
    paused: 0
    running: 1
    stopped: 1
  graphDriverName: overlay
  graphOptions: {}
  graphRoot: /var/lib/containers/storage
  graphStatus:
    Backing Filesystem: extfs
    Native Overlay Diff: "true"
    Supports d_type: "true"
    Using metacopy: "false"
  imageStore:
    number: 4
  runRoot: /var/run/containers/storage
  volumePath: /var/lib/containers/storage/volumes
version:
  APIVersion: 1
  Built: 1
  BuiltTime: Thu Jan  1 08:00:01 1970
  GitCommit: 201c9505b88f451ca877d29a73ed0f1836bb96c7-dirty
  GoVersion: go1.14
  OsArch: linux/amd64
  Version: 2.0.2

Package info (e.g. output of rpm -q podman or apt list podman):

(paste your output here)

Additional environment details (AWS, VirtualBox, physical, etc.):

@vrothberg vrothberg self-assigned this Jul 8, 2020
@vrothberg
Copy link
Member

Thanks for opening the issue!

There's definitely a bug causing the panic but /v1.24/events?filters=%7B%22type%22%3A%7B%22container%22%3Atrue%7D%7D seems to not be marshalled correctly

Failed to Unmarshal {"type":{"container":true}}: json ...

Does the call work with Docker?

@vrothberg
Copy link
Member

Opened #6902 which will fix the panic.

@skywalker512
Copy link
Author

skywalker512 commented Jul 8, 2020

Does the call work with Docker?

Yes, it work fine, and traefik code is here

https://github.com/containous/traefik/blob/master/pkg/provider/docker/docker.go#L271-L299

@vrothberg
Copy link
Member

Thanks, I can reproduce locally when running the docker client against the podman.sock

~ $ sudo docker -H unix:///run/podman/podman.sock events --filter type=container
Error response from daemon: empty value for filter "type"

@vrothberg
Copy link
Member

Fun... the Docker API states that filters are a map[string][]string while they are actually:

// Args stores a mapping of keys to a set of multiple values.
type Args struct {
	fields map[string]map[string]bool
}

@baude, I guess we need to change the wiring on our side to be compatible. WDYT?

@vrothberg
Copy link
Member

This explains the unmarshal error on our side:

Failed to Unmarshal {"type":{"container":true}}: json: cannot unmarshal object into Go value of type []string

@mheon
Copy link
Member

mheon commented Jul 8, 2020

@vrothberg Are you sure the filter typing didn't change between v1.24 and v1.40?

@vrothberg
Copy link
Member

I can have a look at older implementations but the docs of v1.24 and v1.40 didn't change

@baude
Copy link
Member

baude commented Jul 8, 2020

but arent you saying the docs are actually wrong?

@vrothberg
Copy link
Member

but arent you saying the docs are actually wrong?

The docs in v1.40 are wrong. I will look at v1.24 code to check if it has always behaved like that or if the implementation has changed over time.

@mheon
Copy link
Member

mheon commented Jul 8, 2020

Can we verify this with another filter? the Type filter looks unusual enough that it might be handled specially

@vrothberg
Copy link
Member

It's independent of the filter as the types are different (see #6899 (comment)).

We follow the documentation with using a map[string][]string but docker uses map[string]map[string]bool.

@vrothberg
Copy link
Member

but arent you saying the docs are actually wrong?

The docs in v1.40 are wrong. I will look at v1.24 code to check if it has always behaved like that or if the implementation has changed over time.

I just checked. v1.24 (Docker 1.12.) was also using a map[string]map[string]bool so I guess it has always been a bug in the Docker docs.

@vrothberg
Copy link
Member

@baude @mheon ... do you agree to change our docs and implementation to use a map[string]map[string]bool for compat and libpod?

@mheon
Copy link
Member

mheon commented Jul 9, 2020

Sure. Ensuring compatibility with the actual implementation is more important than compatibility with their docs.

vrothberg added a commit to vrothberg/libpod that referenced this issue Jul 20, 2020
Fix a potential panic in the events endpoint when parsing the filters
parameter.  Values of the filters map might be empty, so we need to
account for that instead of uncondtitionally accessing the first item.

Also apply a similar for race conditions as done in commit f4a2d25:

	Fix a race that could cause read errors to be masked.  Masking
	such errors is likely to report red herrings since users don't
	see that reading failed for some reasons but that a given event
	could not be found.

Another race was the handler closing event channel, which could lead to
two kinds of panics: double close, send to close channel.  The backend
takes care of that.  However, make sure that the backend stops working
in case the context has been cancelled.

Fixes: containers#6899
Signed-off-by: Valentin Rothberg <[email protected]>
vrothberg added a commit to vrothberg/libpod that referenced this issue Jul 20, 2020
The versions Docker that the compat endpoints currently support are
using another type for the `filters` parameter than later versions
of Docker, which the libpod/events endpoint is also using.

To prevent existing deplopyments from breaking while still achieving
backward compat, we now support both types for the filters parameter.

Tested manually.

Fixes: containers#6899
Signed-off-by: Valentin Rothberg <[email protected]>
vrothberg added a commit to vrothberg/libpod that referenced this issue Jul 21, 2020
Fix a potential panic in the events endpoint when parsing the filters
parameter.  Values of the filters map might be empty, so we need to
account for that instead of uncondtitionally accessing the first item.

Also apply a similar for race conditions as done in commit f4a2d25:

	Fix a race that could cause read errors to be masked.  Masking
	such errors is likely to report red herrings since users don't
	see that reading failed for some reasons but that a given event
	could not be found.

Another race was the handler closing event channel, which could lead to
two kinds of panics: double close, send to close channel.  The backend
takes care of that.  However, make sure that the backend stops working
in case the context has been cancelled.

Fixes: containers#6899
Signed-off-by: Valentin Rothberg <[email protected]>
vrothberg added a commit to vrothberg/libpod that referenced this issue Jul 21, 2020
The versions Docker that the compat endpoints currently support are
using another type for the `filters` parameter than later versions
of Docker, which the libpod/events endpoint is also using.

To prevent existing deplopyments from breaking while still achieving
backward compat, we now support both types for the filters parameter.

Tested manually.

Fixes: containers#6899
Signed-off-by: Valentin Rothberg <[email protected]>
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants