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 6 rolling deploys #3373

Conversation

mariash
Copy link
Member

@mariash mariash commented Aug 3, 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

  • Rolling deploy respects readiness health check.
  • Updated ActualLRP protobuf definition to include routable parameter. Diego sets it to true when readiness health checks are not provided or set to process.
  • In case if routable is not provided in ActualLRP protobuf message Diego instance reporter sets it to true for backwards compatibility.
  • Please note. We also changed the response of GET /v2/apps/:guid/instances to include routable parameter since the controller for this endpoint is using the same method all_instances_for_app that is used in deployment updater.

Acceptance Setup

  1. Deploy capi-release with this PR
  2. Deploy diego-release from develop
  3. Update an existing app with rolling strategy cf push --strategy=rolling that has eventually successful readiness check. In parallel window see that curling application does not fail.
    • Use this proxy app with the following readiness checks configuration:
    readiness-health-check-type: http
    readiness-health-check-http-endpoint: "/eventuallysucceed"
    
  4. Update an existing app with rolling strategy cf push --strategy=rolling with failing readiness check. New application should return different response. See that new code is not being deployed and curling application does not fail.
    • Use this proxy app with the following readiness checks configuration:
    env:
      EVENTUALLY_FAIL_AFTER_COUNT: 0
    processes:
    - name: web
      ...
      readiness-health-check-type: http
      readiness-health-check-http-endpoint: "/eventuallyfail"
    
  5. Update an existing app with rolling strategy cf push --strategy=rolling that that does not have readiness check. See that application is updated and curling application does not fail.
    • Use this proxy app with the following readiness checks configuration:
      readiness-health-check-type: process
    

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.

Signed-off-by: Josh Russett <[email protected]>
@mariash mariash force-pushed the readiness-health-checks-story-6-rolling branch from 17dba77 to 72ae081 Compare August 9, 2023 18:55
@sethboyles sethboyles merged commit 31a01d2 into cloudfoundry:readiness-health-checks Aug 10, 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.

3 participants