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 the consul_service_health data source #89

Merged

Conversation

remilapeyre
Copy link
Contributor

This PR adds a new data source consul_service_health that fetch checks information about a given service.

See #87

@remilapeyre
Copy link
Contributor Author

I wish I would be able to export attributes as sub-resources so they would be accessible like so:

${data.consul_service_health.vault.nodes.0.service.id}
${data.consul_service_health.vault.nodes.0.service.name}
${data.consul_service_health.vault.nodes.0.service.tags}
${data.consul_service_health.vault.nodes.0.service.address}

but it seems to me that this is not possible so I had to resort to an ugly _:

${data.consul_service_health.vault.nodes.0.service_id}
${data.consul_service_health.vault.nodes.0.service_name}
${data.consul_service_health.vault.nodes.0.service_tags}
${data.consul_service_health.vault.nodes.0.service_address}

@remilapeyre
Copy link
Contributor Author

To correctly test this PR, it is better to wait for the merge of #64

@ghost ghost added size/XXL dependencies and removed size/XL labels Jan 30, 2019
Rémi Lapeyre added 4 commits March 19, 2019 19:43
When the same service with health-checks is defined on multiple nodes,
all health-checks are read from Consul API no matter which instance of
the service they are associated with. This result on non-empty plans
since each service instance wants to destroy the health-checks of the
others.
@remilapeyre remilapeyre force-pushed the add-service-health-datasource branch from c36c633 to 181c4a4 Compare March 19, 2019 18:46
@ghost ghost added size/XL and removed size/XXL labels Mar 19, 2019
@paultyng
Copy link
Contributor

paultyng commented Mar 20, 2019

Will this let you wait until a service is healthy? For example if you just created a new service in the TF script and wanted to wait for it to be fully available?

Maybe an optional way to use this, with a timeout?

Copy link
Contributor

@pearkes pearkes left a comment

Choose a reason for hiding this comment

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

It looks like this is two changes:

  • Changes to retrieving health checks
  • New consul_service_health data source

On the new data source, I could see certain users getting tripped up on the use-cases here – really this will change so much that it feels a bit strange in the context of Terraform, but I also understand there may be good potential use-cases for then getting Terraform to do other things with the healthy service addresses. I just wonder a bit if we're encouraging strange things by providing this highly mutable endpoint.

On the other hand, this is an opt-in data source and given it is not overwriting / writing data I don't see much downside harm in including it for those users who do have use-cases that are in need of this particular thing.

consul/resource_consul_service.go Show resolved Hide resolved
website/docs/d/service_health.html.markdown Show resolved Hide resolved
@remilapeyre remilapeyre merged commit 084c65c into hashicorp:master May 26, 2019
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.

3 participants