LGTM :)

I don't know how we usually handled API deprecations like this. Looking at this, I wonder if we actually can remove this, without disabling API versions 1.something and older.

The reason for a versioned API is of course that it doesn't change, so wondering if we should disable older API versions altogether. Open to suggestions, just thinking out loud here

@thaJeztah Could you provide some information or previous discussion about deprecating this feature?
Indeed removing this code might break some old apps. I've no idea whether we should keep the current behavior for old API users. Suggestions are welcomed.
Thanks 😃

Found #17799 and #14777

@mountkin yes, I'm trying to think of other examples, i.e., have we done this before? Usually, we keep the old behavior around for older API versions, so perhaps we should change it to be an error for API version 1.24 and above, but keep the old behavior for older versions.

ping @cpuguy83 @calavera wdyt?

I don't think we can just remove it, which means we'll have to still handle older API versions, which means the case must still be handled (by switching on API versions) which will probably make the code more complex and not less, sadly.

I agree with @cpuguy83

Alright, so we should keep this code around (unfortunately), and make it an error if it's used on API 1.24 or above (instead of just a warning).

@mountkin can you update the PR to do that? Sorry for that, I think we all wished we could cleanup this code

I think we can add an HTTP header (for example: Deprecated-Features-Used: 17799, 17799 is the github issue where the deprecation was proposed) to all the HTTP requests to the official registry.
Then the Docker team can analyse the docker hub HTTP log to determine whether the deprecated feature can be completely removed.
We're already sending kernel version and os version information via the User-Agent header.

Code updated.
I also moved the related API tests to a separate file integration-cli/docker_deprecated_api_v124_test.go and renamed all the test cases to TestDeprecatedXXX so that we can run TESTFLAGS='-check.f DockerSuite.TestDeprecated*' make test-integration-cli to test the deprecated APIs.

ping @LK4D4 @cpuguy83 PTAL!

Ping @runcom and @calavera too :-)

🤘 LGTM

It looks like WinTP5 complains in those new tests, btw.

All tests passed. PTAL again. Thanks

LGTM 👍

Putting it to docs-review, @thaJeztah 😝

I think we should add a mention in the API changes;
https://github.com/docker/docker/blob/master/docs/reference/api/docker_remote_api.md#v124-api-changes

something like;

* `POST /containers/(id or name)/start` no longer accepts a `HostConfig`

doc updated

Thanks! LGTM

ping @vdemeester ptal

LGTM 🐮