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 incompatibilities with docker-py #5553

Closed
max-arnold opened this issue Mar 19, 2020 · 4 comments · Fixed by #5554
Closed

APIv2 incompatibilities with docker-py #5553

max-arnold opened this issue Mar 19, 2020 · 4 comments · Fixed by #5554
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@max-arnold
Copy link

/kind bug

Description

I'm trying to make the SaltStack Docker driver work with Podman APIv2 (the driver uses docker-py under the hood).

I managed to run simple Salt commands like docker.version, docker.info and docker.pull, but more complex ones like docker.ps or docker.run_container are failing. I traced the failures back to the underlying APIv2 calls that aren't compatible with Docker API:

  1. sudo curl --unix-socket /run/podman/podman.sock http://d/version

It reports "Version":"1.8.1", but the Salt module expects at least 1.9.0 to decide if it should work at all. The problem is arguable and can be fixed on Salt side by checking for Podman Engine component name. However, I decided to report it anyway because other tools in the wild can also make some decisions based on the version number.

  1. sudo curl --unix-socket /run/podman/podman.sock 'http://localhost/v1.40/containers/json?limit=-1&all=1&size=0&trunc_cmd=0'

{"cause":"runtime error: slice bounds out of range","message":"runtime error: slice bounds out of range","response":500}

This problem is caused by the limit=-1 default parameter for containers() in docker-py: https://github.com/docker/docker-py/blob/master/docker/api/container.py#L151-L153

Docker API spec doesn't list -1 as a valid number https://docs.docker.com/engine/api/v1.40/#tag/Container However, other calls use it, so I guess it is a common convention:

curl -s https://docs.docker.com/engine/api/v1.40.yaml | grep -- '-1' | grep limit
        description: "Total memory limit (memory + swap). Set as `-1` to enable unlimited swap."
          Tune a container's PIDs limit. Set `0` or `-1` for unlimited, or `null` to not change.
  1. Creating a container

sudo curl --unix-socket /run/podman/podman.sock --header "Content-Type: application/json" --data '{"Tty": false, "OpenStdin": false, "StdinOnce": false, "AttachStdin": false, "AttachStdout": true, "AttachStderr": true, "Image": "72300a873c2ca11c70d0c8642177ce76ff69ae04d61a5813ef58d40ff66e3e7c", "NetworkDisabled": false, "HostConfig": {"NetworkMode": "default"}}' 'http://localhost/v1.40/containers/create?name=ubuntu'

It returns the following response

{"id":"d3dfc6dcd2b5bff7b6a54dd3fc06a338ab2e28bb4ece1c7aee8ef51a5579f997","Warnings":[]}

The problem here is that the id key is lowercase. I believe it should be Id instead:

curl -s https://docs.docker.com/engine/api/v1.40.yaml | grep -A 10 'OK response to ContainerCreate operation'
            description: "OK response to ContainerCreate operation"
            required: [Id, Warnings]
            properties:
              Id:
                description: "The ID of the created container"
                type: "string"
                x-nullable: false
              Warnings:
                description: "Warnings encountered when creating the container"
                type: "array"
                x-nullable: false

Salt driver uses Id in multiple places and crashes because it can't find the key.

I guess there are other problems, but those are the first roadblocks I noticed. I'm willing to invest more time to test the new APIv2 when/if they are fixed.

Output of podman version:

Version:            1.8.1
RemoteAPI Version:  1
Go Version:         go1.10.1
OS/Arch:            linux/amd64

docker-py version:

docker==4.2.0

Output of podman info --debug:

debug:
  compiler: gc
  git commit: ""
  go version: go1.10.1
  podman version: 1.8.1
host:
  BuildahVersion: 1.14.2
  CgroupVersion: v1
  Conmon:
    package: 'conmon: /usr/libexec/podman/conmon'
    path: /usr/libexec/podman/conmon
    version: 'conmon version 2.0.13, commit: '
  Distribution:
    distribution: ubuntu
    version: "18.04"
  MemFree: 10911744
  MemTotal: 499093504
  OCIRuntime:
    name: runc
    package: 'cri-o-runc: /usr/lib/cri-o-runc/sbin/runc'
    path: /usr/lib/cri-o-runc/sbin/runc
    version: 'runc version spec: 1.0.1-dev'
  SwapFree: 1002172416
  SwapTotal: 1027600384
  arch: amd64
  cpus: 1
  eventlogger: journald
  hostname: minion1
  kernel: 4.15.0-72-generic
  os: linux
  rootless: false
  uptime: 2h 28m 55.25s (Approximately 0.08 days)
registries:
  search:
  - docker.io
  - quay.io
store:
  ConfigFile: /etc/containers/storage.conf
  ContainerStore:
    number: 0
  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: 0
  RunRoot: /var/run/containers/storage
  VolumePath: /var/lib/containers/storage/volumes

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

podman/unknown,now 1.8.1~1 amd64 [installed]
@openshift-ci-robot openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Mar 19, 2020
@baude
Copy link
Member

baude commented Mar 19, 2020

@max-arnold great issue ...

@baude
Copy link
Member

baude commented Mar 19, 2020

@max-arnold i have a patch for the first one ...

@rhatdan
Copy link
Member

rhatdan commented Mar 19, 2020

The next version of podman should fix the first one.

baude added a commit to baude/podman that referenced this issue Mar 19, 2020
honor -1 in in list containers for compatibility mode.  it is commonly used to indicate no limit.

change the json id parameter to Id in container create.

Fixes: containers#5553

Signed-off-by: Brent Baude <[email protected]>
@baude
Copy link
Member

baude commented Mar 19, 2020

thanks for the great report and reproducers. it makes things easier.

snj33v pushed a commit to snj33v/libpod that referenced this issue May 31, 2020
honor -1 in in list containers for compatibility mode.  it is commonly used to indicate no limit.

change the json id parameter to Id in container create.

Fixes: containers#5553

Signed-off-by: Brent Baude <[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
kind/bug Categorizes issue or PR as related to a bug. 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