Skip to content

Add HEAD support for /_ping endpoint#38570

Merged
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:keep_your_head_up
Jan 31, 2019
Merged

Add HEAD support for /_ping endpoint#38570
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:keep_your_head_up

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah commented Jan 14, 2019

this one is created on top of #38569; only the last commit is new

Monitoring systems and load balancers are usually configured to use HEAD
requests for health monitoring. The /_ping endpoint currently does not
support this type of request, which means that those systems have fallback
to GET requests.

This patch adds support for HEAD requests on the /_ping endpoint.

Although optional, this patch also returns Content-Type and Content-Length
headers in case of a HEAD request; Refering to RFC 7231, section 4.3.2:

The HEAD method is identical to GET except that the server MUST NOT
send a message body in the response (i.e., the response terminates at
the end of the header section). The server SHOULD send the same
header fields in response to a HEAD request as it would have sent if
the request had been a GET, except that the payload header fields
(Section 3.3) MAY be omitted. This method can be used for obtaining
metadata about the selected representation without transferring the
representation data and is often used for testing hypertext links for
validity, accessibility, and recent modification.

A payload within a HEAD request message has no defined semantics;
sending a payload body on a HEAD request might cause some existing
implementations to reject the request.

The response to a HEAD request is cacheable; a cache MAY use it to
satisfy subsequent HEAD requests unless otherwise indicated by the
Cache-Control header field (Section 5.2 of [RFC7234]). A HEAD
response might also have an effect on previously cached responses to
GET; see Section 4.3.5 of [RFC7234].

With this patch applied, either GET or HEAD requests work; the only
difference is that the body is empty in case of a HEAD request;

curl -i --unix-socket /var/run/docker.sock http://localhost/_ping
HTTP/1.1 200 OK
Api-Version: 1.40
Cache-Control: no-cache, no-store, must-revalidate
Docker-Experimental: false
Ostype: linux
Pragma: no-cache
Server: Docker/dev (linux)
Date: Mon, 14 Jan 2019 12:35:16 GMT
Content-Length: 2
Content-Type: text/plain; charset=utf-8

OK

curl --head -i --unix-socket /var/run/docker.sock http://localhost/_ping
HTTP/1.1 200 OK
Api-Version: 1.40
Cache-Control: no-cache, no-store, must-revalidate
Content-Length: 0
Content-Type: text/plain; charset=utf-8
Docker-Experimental: false
Ostype: linux
Pragma: no-cache
Server: Docker/dev (linux)
Date: Mon, 14 Jan 2019 12:34:15 GMT

The client is also updated to use HEAD by default, but fallback to GET
if the daemon does not support this method.

- Description for the changelog

+ Add `HEAD` support for the `/_ping` endpoint

@thaJeztah
Copy link
Copy Markdown
Member Author

@cpuguy83 and last ping in this series 👍 😂

@thaJeztah
Copy link
Copy Markdown
Member Author

@thaJeztah
Copy link
Copy Markdown
Member Author

/cc @vsaraswat Also may needs an update to the changes made in docker/docs#4758 if this is accepted, and included in a Docker release 👍

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 15, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@393838c). Click here to learn what that means.
The diff coverage is 82.35%.

@@            Coverage Diff            @@
##             master   #38570   +/-   ##
=========================================
  Coverage          ?   36.64%           
=========================================
  Files             ?      608           
  Lines             ?    45189           
  Branches          ?        0           
=========================================
  Hits              ?    16561           
  Misses            ?    26343           
  Partials          ?     2285

Monitoring systems and load balancers are usually configured to use HEAD
requests for health monitoring. The /_ping endpoint currently does not
support this type of request, which means that those systems have fallback
to GET requests.

This patch adds support for HEAD requests on the /_ping endpoint.

Although optional, this patch also returns `Content-Type` and `Content-Length`
headers in case of a HEAD request; Refering to RFC 7231, section 4.3.2:

    The HEAD method is identical to GET except that the server MUST NOT
    send a message body in the response (i.e., the response terminates at
    the end of the header section).  The server SHOULD send the same
    header fields in response to a HEAD request as it would have sent if
    the request had been a GET, except that the payload header fields
    (Section 3.3) MAY be omitted.  This method can be used for obtaining
    metadata about the selected representation without transferring the
    representation data and is often used for testing hypertext links for
    validity, accessibility, and recent modification.

    A payload within a HEAD request message has no defined semantics;
    sending a payload body on a HEAD request might cause some existing
    implementations to reject the request.

    The response to a HEAD request is cacheable; a cache MAY use it to
    satisfy subsequent HEAD requests unless otherwise indicated by the
    Cache-Control header field (Section 5.2 of [RFC7234]).  A HEAD
    response might also have an effect on previously cached responses to
    GET; see Section 4.3.5 of [RFC7234].

With this patch applied, either `GET` or `HEAD` requests work; the only
difference is that the body is empty in case of a `HEAD` request;

    curl -i --unix-socket /var/run/docker.sock http://localhost/_ping
    HTTP/1.1 200 OK
    Api-Version: 1.40
    Cache-Control: no-cache, no-store, must-revalidate
    Docker-Experimental: false
    Ostype: linux
    Pragma: no-cache
    Server: Docker/dev (linux)
    Date: Mon, 14 Jan 2019 12:35:16 GMT
    Content-Length: 2
    Content-Type: text/plain; charset=utf-8

    OK

    curl --head -i --unix-socket /var/run/docker.sock http://localhost/_ping
    HTTP/1.1 200 OK
    Api-Version: 1.40
    Cache-Control: no-cache, no-store, must-revalidate
    Content-Length: 0
    Content-Type: text/plain; charset=utf-8
    Docker-Experimental: false
    Ostype: linux
    Pragma: no-cache
    Server: Docker/dev (linux)
    Date: Mon, 14 Jan 2019 12:34:15 GMT

The client is also updated to use `HEAD` by default, but fallback to `GET`
if the daemon does not support this method.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Copy Markdown
Member Author

Rebased, to get rid of the commit from #38569

ping @vdemeester @tiborvass @cpuguy83 PTAL 🤗

Copy link
Copy Markdown
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🐯

Copy link
Copy Markdown
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
Copy link
Copy Markdown
Member Author

Experimental failure (DockerSwarmSuite.TestAPISwarmLeaderElection) is unrelated, and a known flaky #32673

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants