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

Update compute_health_check.html.markdown #300

Closed
wants to merge 2 commits into from

Conversation

wjimenez5271
Copy link

Mention limitation of non HTTP health checks with TargetPools to hopefully save other people getting stuck like I did :)

Mention limitation of non HTTP health checks with TargetPools to hopefully save other people getting stuck like I did :)
@@ -10,7 +10,7 @@ description: |-

Manages a health check within GCE. This is used to monitor instances
behind load balancers. Timeouts or HTTP errors cause the instance to be
removed from the pool. For more information, see [the official
removed from the pool. This cannot be used with GCE `TargetPools` as they only support `HttpHealthChecks`. For more information, see [the official
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that this behavior is confusing.

However it would be nicer to make the documentation match the public API closer. Maybe something like:

Be aware that if you're managing a [Network Load Balancer](https://cloud.google.com/compute/docs/load-balancing/network/), you must use one of `google_compute_http_health_check` or `google_compute_https_health_check` instead of this resource. See [Legacy Health Checks](https://cloud.google.com/compute/docs/load-balancing/health-checks#legacy_health_checks) for more information.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, even better!

better explanation, more in line with official GCE docs
@selmanj
Copy link
Contributor

selmanj commented Aug 14, 2017

Sorry, I know you made the requested changes, but after talking to others it makes more sense to document this on the TargetPool resource documentation itself.

Specifically in compute_target_pool.html.markdown, under Argument Reference at the health_checks parameter.

@rosbo
Copy link
Contributor

rosbo commented Nov 7, 2017

I will update the target pool documentation in a separate PR.

@rosbo rosbo closed this Nov 7, 2017
rosbo added a commit that referenced this pull request Nov 8, 2017
…#702)

- Accepts self_link in addition of health check name
- Removes the need for an API call to generate the self link
- Improves the documentation to mention that only the legacy google_compute_http_health_check is supported. This will prevent our user from being stuck like mentioned here: #300.
- Adds a MaxItems:1 in the schema. You can't have more than one. The API will fail. The official docs also says so.
- Adds a check to the acceptance test to ensure the health checks are properly setup.
chrisst pushed a commit to chrisst/terraform-provider-google that referenced this pull request Nov 9, 2018
…hashicorp#702)

- Accepts self_link in addition of health check name
- Removes the need for an API call to generate the self link
- Improves the documentation to mention that only the legacy google_compute_http_health_check is supported. This will prevent our user from being stuck like mentioned here: hashicorp#300.
- Adds a MaxItems:1 in the schema. You can't have more than one. The API will fail. The official docs also says so.
- Adds a check to the acceptance test to ensure the health checks are properly setup.
luis-silva pushed a commit to luis-silva/terraform-provider-google that referenced this pull request May 21, 2019
@ghost
Copy link

ghost commented Mar 30, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants