error creating container when HostConfig.LogConfig.Config is not specified

[ryane]

I have been working on updating the Terraform Docker provider with support for sdc-docker. There is an open PR that fixes a few issues and adds additional support for various container options. However, I just realized that there is still an issue that I believe needs to be fixed on the sdc-docker side. With the PR I submitted to Terraform, you are able to use the docker provider to manage containers on SDC Docker. However, it will only work if you specify valid log_opts. You cannot exclude it. For example, this will work:

resource "docker_container" "foo" {
  name = "tf-test"
  image = "${docker_image.foo.latest}"
  log_driver = "json-file"
  log_opts = {
    max-size = "1m"
  }
}

but this will not:

resource "docker_container" "foo" {
  name = "tf-test"
  image = "${docker_image.foo.latest}"
}

You will get the error:

Unable to create container: API error (422): (Validation) HostConfig.LogConfig: missing Config (801dd910-8345-11e5-a6a0-e97a8f70ea66)

The default logging driver is json-file and the available options are max-size and max-file; the default is that neither of those are set. Setting a default on either of those will break expectations about how container logging will work.

It looks like the fix would be to remove the validation check at https://github.com/joyent/sdc-docker/blob/0a9778c8c65a4ce01cbecdf71762f0f2b88af621/lib/validate.js#L305. The next several lines of code ensure Config is defaulted to an empty object if one isn't passed in.

[joshwilsdon]

Have you found any way to reproduce this on the official docker client?

If I use either of: docker run -it ubuntu or docker run -it --log-driver=json-file ubuntu this doesn't happen.

I will try figure out whether there's a reasonable way to workaround this, but at this point I'm having trouble to reproduce it so that I can confirm it's fixed. A reproduction case with the official client would help quite a bit.

In the meantime would it be possible for you to try with:

ryane/terraform@72c86a6#diff-bf6ca9e5fe91d993d41708aedf87c09cR106

changed so that it always sends the Config: {} even when empty, like the official docker client does?

In the case of the two docker commands I listed above, what I get from the official client at the server for HostConfig.LogConfig is:

        "LogConfig": {
          "Type": "",
          "Config": {}
        },

and:

        "LogConfig": {
          "Type": "json-file",
          "Config": {}
        },

The docker remote API also includes the empty Config in their documented interface: https://docs.docker.com/engine/reference/api/docker_remote_api_v1.21/#create-a-container and it's not clear to me from the description there that we should actually be allowing Config to be missing rather than empty.

[ryane]

Right, I don't think you will be able to duplicate this with the Docker CLI since, as you mentioned, it sends a {} for LogConfig. The problem is that other docker API clients don't (including the one Terraform uses). I've already tried changing the Terraform provider to include {} as you suggested, but an empty {} is still omitted from the JSON request.

In my testing, excluding LogConfig rather than sending an empty object works fine against the official docker remote API but does work not against the sdc-docker implementation.

In fact, the official Docker remote API will happily accept a request with a missing LogConfig object, and even a missing HostConfig object. I asked about that in this issue on the triton-terraform project. I was able to workaround the missing HostConfig and LogConfig issues in the Terraform provider but I cannot workaround a missing LogConfig.Config as there is no sensible, non-empty default that I can include.

Let me know if you want a standalone golang repro case outside of the Terraform provider above.

It does seem that you already have code to handle a missing LogConfig.Config but it is being short-circuited by the validation on line 305.

[joshwilsdon]

A golang repro case won't be very helpful here as the environment our tests run in does not have go available. I'll try to write a test case soon that exercises this the same way as the go-dockerclient. Then I should be able to work around this behavior.

[joshwilsdon]

I've tested this just with CURL as it turned out that writing a test with the current test suite wasn't going to get done in a reasonable time. If someone else is trying to reproduce my test, I just used:

curl http://${DOCKER_IP}:2375/v1.19/containers/create -X POST -H accept:application/json -H content-type:application/json --data-binary @docker.payload

after disabling SSL and auth. The docker.payload looked like:

{
  "Hostname": "",
  "Domainname": "",
  "User": "",
  "AttachStdin": true,
  "AttachStdout": true,
  "AttachStderr": true,
  "PortSpecs": null,
  "ExposedPorts": {},
  "Tty": true,
  "OpenStdin": true,
  "StdinOnce": true,
  "Env": [],
  "Cmd": null,
  "Image": "ubuntu",
  "Volumes": {},
  "VolumeDriver": "",
  "WorkingDir": "",
  "Entrypoint": null,
  "NetworkDisabled": false,
  "MacAddress": "",
  "OnBuild": null,
  "Labels": {},
  "HostConfig": {
    "Binds": null,
    "ContainerIDFile": "",
    "LxcConf": [],
    "Memory": 0,
    "MemorySwap": 0,
    "CpuShares": 0,
    "CpuPeriod": 0,
    "CpusetCpus": "",
    "CpusetMems": "",
    "CpuQuota": 0,
    "BlkioWeight": 0,
    "OomKillDisable": false,
    "Privileged": false,
    "PortBindings": {},
    "Links": null,
    "PublishAllPorts": false,
    "Dns": null,
    "DnsSearch": null,
    "ExtraHosts": null,
    "VolumesFrom": null,
    "Devices": [],
    "NetworkMode": "bridge",
    "IpcMode": "",
    "PidMode": "",
    "UTSMode": "",
    "CapAdd": null,
    "CapDrop": null,
    "RestartPolicy": {
      "Name": "no",
      "MaximumRetryCount": 0
    },
    "SecurityOpt": null,
    "ReadonlyRootfs": false,
    "Ulimits": null,
    "LogConfig": {
      "Type": ""
    },
    "CgroupParent": ""
  },
  "Name": ""
}