Thanks!

Is the same change needed for other endpoints that have a similar "raw" stream? (attach, perhaps logs? (not sure)); I see a couple more occurrences of application/vnd.docker.raw-stream in the swagger

We should probably document the additions to the headers in the Swagger file; at least a note in the version history; https://github.com/moby/moby/blob/53430f5/docs/api/version-history.md#v141-api-changes

Possibly in the section describing the stream format https://github.com/moby/moby/blob/9c94e82/api/swagger.yaml#L5864-L5896

And possibly

containers/{id}/logs do not set a Content-type header, will fix this at same time.

the commit description needs to be updated, as this is no longer doc-only change

by document I was considering the http header as documentation for the http payload, admitedly confusing.

Do we need an API version switch on this?

Otherwise, very nice. I think this would also make it so the CLI doesn't need to do an inspect (IIRC) to check for TTY

Do we need an API version switch on this?

We may not need to, given that the content-type itself doesn't change, we're only adding parameters (which are optional, and can be ignored unless a client is looking for it specifically). So unless a client takes the raw Content-Type header and does an exact string-match, we should be good (famous last words);

RFC7231;

The type/subtype MAY be followed by parameters in the form of name=value pairs.

https://play.golang.org/p/my367gBiJ5w

package main

import (
	"fmt"
	"mime"
)

func main() {
	mediaType, params, _ := mime.ParseMediaType("application/vnd.docker.raw-stream; stream=raw; something=else; lang=en_US")
	fmt.Println("mediatype:", mediaType)
	fmt.Printf("params: %v\n", params)
}

We could add more documentation around it (if the stream parameter is not present, the client should fall back to calling the GET /containers/{id}/json endpoint to determine if the container has a TTY attached?

updated commit message and PR description
rebased on origin/master

Not sure how to address buid failure
docker-logger-128371389\container.log.1: The process cannot access the file because it is being used by another process.
I tried to rebase and force-push to trigger a fresh new build but other errors still happen on Windows tests.

afaict most of the recent commits on master just failed on CI.
so one one hand make me feel I'm (probably?) not responsible for test failure on this PR, but also don't make me confident this should be merged and we won't get side effects in near future

@cpuguy83 updated: new multiplexed mime type is used for version >= 1.42 and client's Accept header is checked so we don't break any client that would be strict with HTTP.

We discussed this PR in the maintainers meeting.

We had some discussion about whether 406 was trying to be "too perfect", and if a plain 400 (hey, something's invalid) would do. We went back-and-forth a bit, but @corhere brought up some valid points;

• if a client explicitly only wants to use (say) application/vnd.docker.multiplexed-strea, but because of the container-settings, it cannot be, then having a 409 could be useful for it to know "why".
• similar; possibly we'd want to add different mediatype for (e.g.) log-streams, in which case it's useful to have the correct status for this situation (instead of a plain 400)

So even though the use-cases are currently limited; it was considered good to do it "right", and to already use this status code.

There were some other points discussed, but the consensus was that we're ok with the current approach; I asked some maintainers to have a look at the implementation, and to accept/merge this PR unless there are really major issues (minor issues / improvements / cleanups for follow-ups if needed).

We're going back and forth on this topic, trying to find a good "HTTP-friendly" way to manage this, but actually we are not negociating content, there's a single stream type we can return, and this PR is all about documenting this. Could have ben a custom header added to the HTTP response. I'm a favor to give up with client's Accept header and just set Content-Type for API >= 1.42.

Continuing to ignore the client's Accept header while we have nothing to negotiate sounds good to me. It wouldn't impact our ability to add new stream formats to the API in the future since the current formats would generally need to remain the default and we could allow clients to opt in... by content negotiation using an Accept header! Generic clients would need to support the existing stream formats anyway. And any specialized clients with more finicky requirements would need to inspect the Content-Type response header anyway to handle negotiation-unaware daemons regardless of whether awareness of the Accept header was added today.

Continuing to ignore the client's Accept header while we have nothing to negotiate sounds good to me

This is the option I selected on my last push, reverting content negociation and attempts to rely on Accept header to detect client expectations. I don't want this to become a complex beast just to avoid client to remember or inspect container config before it collects logs.

Can we have a test for this? There may be some API related tests already that can check the response header.