listContainers fails with DockerClientDecodeError

[liff]

Environment

• Docker CE 17.09.0-ce-mac35 (19611)

Example code

module Main where

import           Control.Monad.IO.Class
import           Docker.Client

main :: IO ()
main = do
  http <- unixHttpHandler "/var/run/docker.sock"
  runDockerT (defaultClientOpts, http) $ do
    containers <- listContainers defaultListOpts
    liftIO $ print containers

Results in:

Left (DockerClientDecodeError "Error in $[0].Id: ContainerID is not an object.")

From what I can tell, the JSON decoder expects the Container JSON to look something like:

{ "Id": {"Id": "…"}, }

but it's just:

{"Id": "…"}

Relevant lines:

• Types.hs:532
• Types.hs:377

[denibertovic]

Hey @liff Thanks for the bug report. I have written a test and can confirm that this is indeed an issue. I haven't had time to fix the issue yet but my hope is that I will get it done over the weekend.

[jprider63]

@denibertovic Could you share your test/the full JSON response? We should add this to the integration tests.

[jprider63]

I pushed a potential fix to my branch. Can you check if this works for you?

@denibertovic can you add your test to the integration tests?

[denibertovic]

@jprider63 Pushed to branch deni/listcontainers....there is an issue with parsing status as well...

[denibertovic]

@jprider63 I was heading offline now but I will give it a go again tomorrow and check out your fix. Thanks for your help!

[denibertovic]

Yeah this is gonna take a while...The whole API changed a bunch.

[jprider63]

Don't we always call the 1.24 endpoint? How has the API changed?

[pparkkin]

Any update on this? Anything I can do to help?

[denibertovic]

@jprider63 This is what happens when I look at the code without enough coffee in me. You are of course right. And I was looking at the docs for the latest API. However the status parsing is still failing and I haven't gotten to the bottom of it. @pparkkin I will take a crack at it again later tonight and will post back here.

[denibertovic]

Looking at the debug logs again it appears that they did indeed break v1.24 API. :/ The docs are wrong as well. And I'm unable to get the debug output from the docker daemon like I could before. Specifically the API raw output is not logged anymore.

[denibertovic]

I couldn't actually get the docker daemon to print out raw API output, and the docs are wrong, so I had to capture network traffic with Wireshark (this is what it's come to). Just an FYI.

I think I got this working in the bug/listcontainers branch but I'm not happy with the solution. Specifically cause I got rid of the container Status data type and am just passing around text now. I think this needs more work and in general we should add integration tests for each api endpoint otherwise things like this will fail often. Since they changed a versioned API I think we need to take a closer look at how we wanna handle stuff like this in the future.

As things stand now I'm not gonna be able to take a closer look at this til the holidays in a few weeks so if anyone want to take the work from the branch above and bring it across the finish line I'd be happy to accept a PR. What I'm looking for is proper parsing of the container Status data type and adding back the commented out mountMode in the Mount data type (if it's relevant still, I haven't had time to check). Also I'd like to see a integration test for listing the containers (The on I have now doesn't really work cause it relies on my machine having test containers started manually...and even to I've only tested the Exited, Created, and Up Status).

[jprider63]

If the 1.24 API is broken and the documentation is wrong, maybe we should file an issue upstream. It doesn't make sense for every Docker client to potentially break on new releases or have to reverse engineer the API.

[denibertovic]

@jprider63 1.24 is practically ancient now...I'd like to be able to update to a newer API version. I doubt we will get much out of opening an issue upstream. :/

[jprider63]

I agree that we should update to the latest API version. What's the best way to do this? Only officially support one API version? Then we could add documentation stating which docker-hs versions correspond to which API versions. We could try to support multiple API versions at once, but this seems difficult since the data structures seem to be changing between releases and API versions.

I think it's still worth opening an issue, so that they know 1.24 is broken and they can decide whether to deprecate or fix it.

[pparkkin]

From what I can tell from the discussion here, it sounds like the data returned by the API has changed for listContainers, and possibly no longer conforms to the API spec for the version the library is targeting. Is that correct?

Do you have a sample of the changed data?

As far as reporting upstream, not sure if it will do much good, if the API version is very old. That said, the reason the API is versioned is exactly to make sure libraries can target a specific version and have it work even when the API changes in the future. If someone can put together a sample of the changed data, and show how it doesn't conform to the API spec version, it's probably a good idea to report. I can definitely do that if you can get me the sample.

I think a good way forward would be to hold off on any ugly changes to the code until there's a sense of whether upstream is willing to fix the issue. If they are open to fixing it, no changes are necessary. If not, it may make more sense to simply branch off and update the library to support a newer version of the API.

This just from someone who's a complete outsider to the project. Feel free to take it or leave it.

[dschoepe]

According to the documentation, what docker returns is in line with the specification for 1.24. The status parsing errors were a result of trying to parse Status instead of State. This also seems to be in line with the docs on the docker website. I submitted a PR to (hopefully) fix these issues.

[denibertovic]

I believe this is fixed now! Thanks all!