Add support for health-checks to consul_service

[remilapeyre]

As proposed in #54, this patch add the support for health-checks defined inline in Consul services definition.

[pearkes]

Made a few notes about this in the issue: #54 (comment)

[remilapeyre]

I used this version of the code for a few weeks and I think there is some problem for the update, I need to add some tests.

[pearkes]

@remilapeyre do you think this would be cleaner with the addition of hashicorp/consul#1680? I think that'd make it much clearer but would require another upstream change. Thoughts?

[remilapeyre]

This would be better as we could detect changes in checks from other sources.
I think it is not required for the first version of theis PR if we put an information about changes done outside from terraform not being detected. For example, for my needs no other service would have made changes to the checks defined in terraform and it would have still been useful.

Is there a reason hashicorp/consul#1680 (comment) does not use the same fields than https://www.consul.io/api/agent/check.html#register-check? Does his message predates the introduction of https://www.consul.io/api/agent/check.html#register-check?

[pearkes]

@remilapeyre I feel like we should just get this merged in the current approach, and not necessarily wait for that upstream change. Does that sound good to you?

[remilapeyre]

Hi @pearkes, I'm getting confused about the issue, if I understand hashicorp/consul#1680 correctly, the information is already available since Consul v1.0.1.

Health-Checks definitions were first added in a7c42a6c2a3c63331170e2fee6ca1bc7bf6731e9 (Expose SkipNodeUpdate field and some health check info in the http api), then refined in subsequents commits.

Here's what http://localhost:8500/v1/health/checks/example-external gives since v1.0.1:

[
    {
        "Node": "external-example",
        "CheckID": "service:redis1",
        "Name": "Redis health check",
        "Status": "critical",
        "Notes": "Script based health check",
        "Output": "",
        "ServiceID": "example-external",
        "ServiceName": "example-external",
        "ServiceTags": [],
        "Definition": {
            "HTTP": "https://www.google.com",
            "Interval": "5s",
            "Timeout": "1s"
        },
        "CreateIndex": 9,
        "ModifyIndex": 10
    }
]

It seems to me that it is just the documentation at https://www.consul.io/api/health.html#list-checks-for-service that is not up to date.

If this is right, this PR need documentation and to make header support in checks work.

[pearkes]

I'm suggesting we change Consul to return the health checks in the same API response as when querying for service instances. Currently it just returns the health check status, not definition.

But in the interim I'm supportive of this current approach of using the (separate) heath check API to read the health check definitions. Would just be nice if we could do it in one API call, but not a big deal.

this PR need documentation and to make header support in checks work.

That makes sense to me!

[pearkes]

Nice work. Looks good I just think we shouldn't nest under definition.

[pearkes]

Great job 👍