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

Readiness health checks story 5 processes #3370

Conversation

mariash
Copy link
Member

@mariash mariash commented Aug 2, 2023

This PR is part of a larger effort detailed here in this Cloud Controller PR and here in this CFF RFC.

In this story

  • readiness health check parameters returned by:
    • GET /v3/processes
    • GET /v3/apps/:guid/processes
    • GET /v3/processes/:guid
    • GET /v3/apps/:guid/processes/:type
    • POST /v3/apps/:guid/processes/:type/actions/scale
  • readiness health check parameters can be updated by:
    • PATCH /v3/processes/:guid
    • PATCH /v3/apps/:guid/processes/:type
  • API docs are updated to include readiness health check in examples and object definitions.
  • We removed readiness health check params from the Process #export_atrributes and #import_attributes methods since they only affect /v2 summary endpoints and we don't need to change them for /v3 endpoints.

Acceptance Setup

  1. Deploy capi-release with this PR.

  2. Push an app.

  3. See that readiness health check type is set to process:

    "readiness_health_check": {
      "type": "process",
       "data": {
        "invocation_timeout": null
       }
    }

    by checking endpoints:

    cf curl /v3/processes
    cf curl /v3/apps/:guid/processes
    cf curl /v3/processes/:guid
    cf curl /v3/apps/:guid/processes/:type
    cf curl -X POST /v3/apps/:guid/processes/:type/actions/scale
    
  4. Update readiness health check using process type endpoint:

    cf curl -X PATCH -d '{"readiness_health_check": {"type": "http", "data": {"endpoint": "/ready"}}' /v3/apps/:app-guid/processes/web
    
  5. See that readiness health check type is set to http:

    "readiness_health_check": {
      "type": "http",
       "data": {
       "endpoint":  "/ready",
        "invocation_timeout": null
       }
    }

    by checking endpoints:

    cf curl /v3/processes
    cf curl /v3/apps/:guid/processes
    cf curl /v3/processes/:guid
    cf curl /v3/apps/:guid/processes/:type
    cf curl -X POST /v3/apps/:guid/processes/:type/actions/scale
    
  6. Update readiness health check using process endpoint:

    cf curl /v3/processes/710eeb42-cb61-4761-bda0-a28e7a0c0d07 -X PATCH -d '{"readiness_health_check": {"type": "port"}}'
    
  7. See that readiness health check type is set to port (notice that endpoint is not displayed:

    "readiness_health_check": {
      "type": "port",
       "data": {
       "endpoint":  "/ready",
        "invocation_timeout": null
       }
    }

    by checking endpoints:

    cf curl /v3/processes
    cf curl /v3/apps/:guid/processes
    cf curl /v3/processes/:guid
    cf curl /v3/apps/:guid/processes/:type
    cf curl -X POST /v3/apps/:guid/processes/:type/actions/scale
    

@mariash mariash force-pushed the readiness-health-checks-story-5-processes branch 3 times, most recently from 9e635bc to c150f3b Compare August 7, 2023 18:26
mariash added 2 commits August 7, 2023 18:37
GET /v3/processes
GET /v3/processes/:guid
PATCH /v3/processes/:guid
GET /v3/apps/:guid/processes
GET /v3/apps/:guid/processes/:type
PATCH /v3/apps/:guid/processes/:type
POST /v3/apps/:guid/processes/:type/actions/scale
@mariash mariash force-pushed the readiness-health-checks-story-5-processes branch from c150f3b to 3c008b0 Compare August 7, 2023 18:39
@@ -18,6 +18,9 @@ def associated_resources
def to_hash
health_check_data = { timeout: process.health_check_timeout, invocation_timeout: process.health_check_invocation_timeout }
health_check_data[:endpoint] = process.health_check_http_endpoint if process.health_check_type == HealthCheckTypes::HTTP

readiness_health_check_data = { invocation_timeout: process.readiness_health_check_invocation_timeout }
Copy link
Member

Choose a reason for hiding this comment

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

why do readiness health checks not have timeouts like the original health check?

Copy link
Member Author

Choose a reason for hiding this comment

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

@sethboyles health-check-timeout represents start up timeout. When app starts up we run liveness health check for this period to mark it as Running. If it never succeeds for the first time during that timeout period we mark app as crashed and we don't event restart it since it was never running. Readiness health checks don't have that behavior. They start running after app instance is started and marked as Running.

@moleske moleske merged commit 5dff948 into cloudfoundry:readiness-health-checks Aug 9, 2023
moleske pushed a commit that referenced this pull request Aug 17, 2023
* Add readiness health check properties to app manifest
* Set readiness health check properties on process

Now when a user supplies the readiness health check properties via the
app manifest, the values are stored on the process, and saved in the
database.

* correctly default to process for readiness_health_check_type
* Refactor: add base health check policy class
* Set readiness health check properties on LRP
* Refactor: app recipe builder

now with more ruby style!

* Fix: readiness hcs default to process not empty
* Add readiness health check properties to /v3/apps/:guid/manifest
* Return diff for readiness check properties in /v3/spaces/:space_guid/manifest_diff
* Readiness health checks story 5 processes (#3370)
* Add readiness health check properties to process endpoints

GET /v3/processes
GET /v3/processes/:guid
PATCH /v3/processes/:guid
GET /v3/apps/:guid/processes
GET /v3/apps/:guid/processes/:type
PATCH /v3/apps/:guid/processes/:type
POST /v3/apps/:guid/processes/:type/actions/scale

* Add readiness health check examples to API docs
* Rolling deploy respects readiness health check (#3373)

ActualLRP has routable set to true once readiness health check succeeds.
In case if routable is not provided instance reporter sets it to true
for backwards compatibility.

* Readiness health checks story 7 respect defined interval (#3385)
* Validate health_check_interval/readiness_health_check_interval in the manifest
* Save health_check_interval/readiness_health_check_interval in database
* Regenerate bbs protobuf models
* Pass health_check_interval/readiness_health_check_interval in LRP
* Display health_check_interval/readiness_health_check_interval in manifest
* Add health-check-interval/readiness-health-check-interval to manifest diff
* Add health-check-interval/readiness-health-check-interval to manifest docs
* Add health_check_interval/readiness_health_check_interval to /v3/processes
* Add health_check_interval/readiness_health_check_interval to API docs
* Copy health_check_interval/readiness_health_check_interval during rolling deploy

* add need readiness check doc to index
* Disable rubocop cyclomaticcomplexity for some functions

- these functions are idomatic ruby with no nested if statements
- rubocop is just counting if statements so no current concerns
- please review if this changes

* Do not use text:true on readiness health check to process

- see https://github.com/cloudfoundry/cloud_controller_ng/wiki/CAPI-Migration-Style-Guide#always-specify-a-size-for-a-string-and-dont-use-text

Signed-off-by: Marc Paquette <[email protected]>
Signed-off-by: Amelia Downs <[email protected]>
Signed-off-by: Renee Chu <[email protected]>
Signed-off-by: Maria Shaldybin <[email protected]>
Signed-off-by: Josh Russett <[email protected]>
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.

4 participants