Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Socket check type #1130

Merged
merged 2 commits into from
Jul 27, 2015
Merged

Add Socket check type #1130

merged 2 commits into from
Jul 27, 2015

Conversation

pdf
Copy link
Contributor

@pdf pdf commented Jul 23, 2015

Adds the ability to simply check whether a socket accepts connections to
determine if it is healthy. This is a light-weight - though less
comprehensive than scripting - method of checking network service
health.

Two check parameters are used:

  • Socket should be set to the location of the socket to be tested, for
    example, example.com:port, or /path/to/unix.sock

  • Network accepts any network type that the Go net library accepts,
    as documented here: https://golang.org/pkg/net/#Dial

    Network defaults to tcp

Example check:

{
  "check": {
    "id": "dns-udp",
    "name": "DNS (UDP)",
    "socket": "ns0.example.com:53",
    "network": "udp",
    "interval": "10s"
  }
}

No documentation updates yet for this PR, if you're keen to accept this, I'll
take a pass at updating them too.

@armon
Copy link
Member

armon commented Jul 23, 2015

This is interesting. I think it's worth probably just reducing the scope to tcp only. A UDP health check doesn't really make much sense in any case.

/cc: @ryanuber @slackpad

@ryanuber
Copy link
Member

Agreed, although we are just passing parameters to net.Dial, so not sure if there is any harm. Nonetheless I'd probably accept TCP only as well. We could also then remove the network field altogether to make the definition more simple.

Fixes #976.

@armon
Copy link
Member

armon commented Jul 23, 2015

I think renaming the check type from "socket" to "tcp" is also more clear (does it support unix domain sockets for example?)

@pdf
Copy link
Contributor Author

pdf commented Jul 23, 2015

I'm particularly not in love with the naming either. @armon can I ask why a UDP health check doesn't make sense? It does support UNIX sockets too BTW.

@pdf
Copy link
Contributor Author

pdf commented Jul 23, 2015

Also, I figured that people may wish to check that a service is available explicitly on IPv4 or IPv6 - if we limit it to tcp, this can't be done via hostname.

@ryanuber
Copy link
Member

Hey @pdf,

UDP checks wouldn't entirely make sense because of the connectionless nature of the protocol. You would need to send packets and wait to receive packets in order to determine a successful transmission, which would require specialized logic on the remote end.

We should still be able to check IP4/IP6. Looking at the net.Dial function, you can see how passing either format of address is accepted.

@pdf
Copy link
Contributor Author

pdf commented Jul 24, 2015

@ryanbreen of course you're correct on UDP, I wasn't thinking. We could send a packet with no payload though. WRT IPv6/IPv4, there's no way to check explicitly for one or the other if you also want to check via hostname, without specifying the protocol..

@pdf
Copy link
Contributor Author

pdf commented Jul 24, 2015

Okay, UDP is not going to be possible cross-platform due to lack of datagram ICMP support on Windows, so if we restrict this to TCP, do we care about supporting explicit IPv4 vs IPv6 checks?

If not, I'll change this to use the tcp key and probably enable DualStack on the Dial so that it will try both IPv4 and IPv6 and return success on any result. If we wanted to support explicit checks by IP proto version, we could add tcp (with the aforementioned behaviour), tcp4 and tcp6 keys.

The reason I raise the IPv4/IPv6 issue is that it may be important for people to check that a service is available on both IPv6 and IPv4, rather than 'any'.

Adds the ability to simply check whether a TCP socket accepts
connections to determine if it is healthy.  This is a light-weight -
though less comprehensive than scripting - method of checking network
service health.

The check parameter `tcp` should be set to the `address:port`
combination for the service to be tested.  Supports both IPv6 and IPv4,
in the case of a hostname that resolves to both, connections will be
attempted via both protocol versions, with the first successful
connection returning a successful check result.

Example check:

```json
{
  "check": {
    "id": "ssh",
    "name": "SSH (TCP)",
    "tcp": "example.com:22",
    "interval": "10s"
  }
}
```
@pdf
Copy link
Contributor Author

pdf commented Jul 24, 2015

Current version is tcp only.

@armon
Copy link
Member

armon commented Jul 24, 2015

@pdf This is looking great. Would you mind updating the documentation as well? I think that is all that is missing.

@pdf
Copy link
Contributor Author

pdf commented Jul 27, 2015

Righteo, I think I hit all the documentation points.

@pdf
Copy link
Contributor Author

pdf commented Jul 27, 2015

Eh, don't think my documentation changes broke the tests 😉

@Kosta-Github
Copy link

+1

armon added a commit that referenced this pull request Jul 27, 2015
@armon armon merged commit 4a9b91f into hashicorp:master Jul 27, 2015
@armon
Copy link
Member

armon commented Jul 27, 2015

Looks great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants