cloudfoundry-attic / windows_app_lifecycle

Apache License 2.0
8 stars 13 forks source link

[HCFRO-85] Modify healthcheck to verify TCP port by default #4

Closed stefanschneider closed 7 years ago

stefanschneider commented 8 years ago

It can also check for HTTP health if the 'uri' arg is given. This mimics the behavior of the healthcheck for Linux.

Also, change the json parser in healthcheck from JavaScriptSerializer to Json.NET library as recommended by Msdn docs.

A reason behind this change is that some web apps will not return successfully when hitting the root endpoint.

A reason behind this change is that some web apps will not return successfully when hitting the root endpoint.

cfdreddbot commented 8 years ago

Hey stefanschneider!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you've already signed the CLA.

cf-gitbot commented 8 years ago

We have created an issue in Pivotal Tracker to manage this. You can view the current status of your issue at: https://www.pivotaltracker.com/story/show/115029785.

viovanov commented 8 years ago

Hello, this PR has been open for some time, so I thought we would add some more context to it. In the Pivotal tracker I noticed there was a reference to this story. Based on the comments there, here are a few reasons we think are compelling for doing TCP checks:

sbenario commented 8 years ago

Hi @viovanov -- As mentioned in that story, the reason for not accepting this PR at the moment is because it would break existing behavior. IIS would respond to the health check immediately, indicating that the app is "ready for requests" while the app is still starting up.

We wouldn't want to take an app that is working well and then make an underlying change that would start directing traffic to the app before it has started up (thereby breaking it).

We are working on a proposal with Diego and CAPI to change the default healthcheck behavior and provide more options for selecting the right type of health check on a per-app basis, but that hasn't made it through yet. In the meantime, please let me know if you have any other feedback. Thanks!

viovanov commented 8 years ago

Hello @sbenario,

I would agree that the behavior would be different, but I don't think it would be breaking. From your comment it sounds like under no circumstances should an app's endpoint be available if it's still starting, like it may break if it receives requests. If that's the case, I believe that's a miss-behaving application. Do you have a more specific example?

Comparing with other experiences, if you deploy an app on Azure for example (using DotNetNuke will demonstrate this nicely), you'll notice that your app goes through a few stages:

  1. First, a placeholder webpage is made available on your URL
  2. Once all resources are created (like your database service), you're hitting the app itself. At this point you'll see DNN error and wait a bunch of times while it's starting up (and you're seeing this in the browser).
  3. After it starts, you can see the installation page

Having a more flexible healthcheck behavior sounds great, but I have to ask what the default behavior will be, even with those options available?

aarondl commented 8 years ago

@sbenario Have you got time to look at this? We'd really like to get this issue resolved.

mavenraven commented 7 years ago

@stefanschneider how do you feel about windows using https://github.com/cloudfoundry/healthcheck now that it supports http?