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

fix!: Use builtin probe endpoints #216

Merged
merged 2 commits into from
Oct 22, 2024

Conversation

nikolaik
Copy link
Contributor

Description of the change

This starts using the built in probe endpoints.

These are /.backstage/health/v1/readiness and /.backstage/health/v1/liveness and were added in v1.29

Ref: https://backstage.io/docs/releases/v1.29.0#backend-health-service

I made this a breaking change, since these are new defaults

Checklist

  • Chart version bumped in Chart.yaml according to semver.
  • Variables are documented in the values.yaml and added to the README.md. The helm-docs utility can be used to generate the necessary content. Use helm-docs --dry-run to preview the content.
  • JSON Schema generated.
  • List tests pass for Chart using the Chart Testing tool and the ct lint command.

@nikolaik nikolaik requested a review from a team as a code owner August 14, 2024 09:38
@nikolaik nikolaik force-pushed the nikolaik/healthchecks-v1.29 branch from 5acd31b to 9c93fa6 Compare August 14, 2024 09:39
Copy link

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution!

@github-actions github-actions bot added the stale label Aug 22, 2024
@ChrisJBurns
Copy link
Contributor

Hi @nikolaik would you be able to update the branch with the main? That should also reduce the amount of changes in the diff.

@github-actions github-actions bot removed the stale label Aug 23, 2024
Copy link

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution!

@github-actions github-actions bot added the stale label Aug 30, 2024
@nikolaik nikolaik force-pushed the nikolaik/healthchecks-v1.29 branch from 9c93fa6 to 61e3539 Compare August 30, 2024 06:37
@nikolaik
Copy link
Contributor Author

Hi @nikolaik would you be able to update the branch with the main? That should also reduce the amount of changes in the diff.

Rebased now <3

@github-actions github-actions bot removed the stale label Aug 31, 2024
@ChrisJBurns
Copy link
Contributor

@nikolaik Seems to be some test failures in the pipeline, are you able to take a look?

@nikolaik nikolaik force-pushed the nikolaik/healthchecks-v1.29 branch from 61e3539 to 144247f Compare September 6, 2024 08:30
Copy link

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution!

@github-actions github-actions bot added the stale label Sep 14, 2024
@nikolaik nikolaik force-pushed the nikolaik/healthchecks-v1.29 branch from ced9d20 to 1c75a54 Compare September 16, 2024 08:45
@nikolaik
Copy link
Contributor Author

@nikolaik Seems to be some test failures in the pipeline, are you able to take a look?

@ChrisJBurns Sorry for the delay! Found the issue with the failing tests

Copy link
Member

@vinzscam vinzscam left a comment

Choose a reason for hiding this comment

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

Hi @nikolaik!
I've noticed that the PR isn't actually specifying the new values as default:

@github-actions github-actions bot removed the stale label Sep 17, 2024
@nikolaik nikolaik force-pushed the nikolaik/healthchecks-v1.29 branch from 1c75a54 to 156514c Compare September 17, 2024 08:49
@nikolaik
Copy link
Contributor Author

Hi @nikolaik! I've noticed that the PR isn't actually specifying the new values as default:

Fixed now! But hitting another issue with the :latest backstage image, seems to be related to the new default config required for techdocs. I'm thinking of waiting until :latest is pointing to v1.31.0 to see if things get fixed

@nikolaik
Copy link
Contributor Author

@vinzscam This should be good for a re-review

Copy link

github-actions bot commented Oct 2, 2024

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution!

@github-actions github-actions bot added the stale label Oct 2, 2024
@ChrisJBurns
Copy link
Contributor

lgtm

@github-actions github-actions bot removed the stale label Oct 3, 2024
Copy link

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution!

@github-actions github-actions bot added the stale label Oct 10, 2024
charts/backstage/values.yaml Show resolved Hide resolved
@github-actions github-actions bot removed the stale label Oct 11, 2024
Copy link

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution!

@github-actions github-actions bot added the stale label Oct 18, 2024
@nikolaik nikolaik force-pushed the nikolaik/healthchecks-v1.29 branch from 6d92b9f to 7e92b05 Compare October 18, 2024 09:52
@nikolaik nikolaik requested a review from vinzscam October 18, 2024 09:53
These are `/.backstage/health/v1/readiness` and `/.backstage/health/v1/liveness` and were added in v1.29

Ref: https://backstage.io/docs/releases/v1.29.0#backend-health-service

Signed-off-by: Nikolai Røed Kristiansen <[email protected]>
Techdocs needs to be configured in app config.
Point our image-digest test to the v1.31.0 sha

Signed-off-by: Nikolai Røed Kristiansen <[email protected]>
@nikolaik nikolaik force-pushed the nikolaik/healthchecks-v1.29 branch from 7e92b05 to 9e25207 Compare October 18, 2024 13:44
@github-actions github-actions bot removed the stale label Oct 19, 2024
Copy link
Member

@tumido tumido left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@tumido tumido dismissed vinzscam’s stale review October 22, 2024 12:58

Resolved in the latest commit.

@tumido tumido merged commit d77bf20 into backstage:main Oct 22, 2024
3 checks passed
@nikolaik nikolaik deleted the nikolaik/healthchecks-v1.29 branch November 12, 2024 11:47
rm3l added a commit to rm3l/rhdh-chart that referenced this pull request Dec 13, 2024
Startup probe settings seem to have been added in the upstream Backstage Chart
in [1], but the current settings do not allow the RHDH Chart for the
liveness probe to be triggered sufficiently enough for the app to be
considered live.
This adjust such settings by accounting for the worst case scenario
where the application might take a bit long to start.

This also aligns the probe endpoints with the upstream chart.

[1] backstage/charts#216
rm3l added a commit to rm3l/rhdh-chart that referenced this pull request Dec 13, 2024
Startup probe settings seem to have been added in the upstream Backstage Chart
in [1], but the current settings do not allow the RHDH Chart for the
liveness probe to be triggered sufficiently enough for the app to be
considered live.
This adjust such settings by accounting for the worst case scenario
where the application might take a bit long to start.

This also aligns the probe endpoints with the upstream chart.

[1] backstage/charts#216
rm3l added a commit to rm3l/redhat-developer-hub-operator that referenced this pull request Dec 13, 2024
Startup probe settings seem to have been added in the upstream Backstage Chart
in [1], but the current settings do not allow the RHDH Chart for the
liveness probe to be triggered sufficiently enough for the app to be
considered live.
This adjust such settings by accounting for the worst case scenario
where the application might take a bit long to start.

This also aligns the probe endpoints with the upstream chart.

[1] backstage/charts#216
rm3l added a commit to redhat-developer/rhdh-operator that referenced this pull request Dec 13, 2024
* fix: adjust startup, liveness and readiness probes settings

Startup probe settings seem to have been added in the upstream Backstage Chart
in [1], but the current settings do not allow the RHDH Chart for the
liveness probe to be triggered sufficiently enough for the app to be
considered live.
This adjust such settings by accounting for the worst case scenario
where the application might take a bit long to start.

This also aligns the probe endpoints with the upstream chart.

[1] backstage/charts#216

* Update bundle/rhdh/manifests/rhdh-default-config_v1_configmap.yaml

Co-authored-by: Patrick Knight <[email protected]>
Co-authored-by: Gustavo Lira e Silva <[email protected]>

* Regenerate bundle manifests

Co-authored-by: rm3l <[email protected]>

* Apply suggestions from code review

Co-authored-by: Patrick Knight <[email protected]>
Co-authored-by: Gustavo Lira e Silva <[email protected]>
Co-authored-by: Nick Boldt <[email protected]>

* Regenerate bundle manifests

Co-authored-by: rm3l <[email protected]>

---------

Co-authored-by: Patrick Knight <[email protected]>
Co-authored-by: Gustavo Lira e Silva <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: rm3l <[email protected]>
Co-authored-by: Nick Boldt <[email protected]>
Co-authored-by: Tomas Kral <[email protected]>
rm3l added a commit to rm3l/redhat-developer-hub-operator that referenced this pull request Dec 13, 2024
…eveloper#564)

* fix: adjust startup, liveness and readiness probes settings

Startup probe settings seem to have been added in the upstream Backstage Chart
in [1], but the current settings do not allow the RHDH Chart for the
liveness probe to be triggered sufficiently enough for the app to be
considered live.
This adjust such settings by accounting for the worst case scenario
where the application might take a bit long to start.

This also aligns the probe endpoints with the upstream chart.

[1] backstage/charts#216

* Update bundle/rhdh/manifests/rhdh-default-config_v1_configmap.yaml

Co-authored-by: Patrick Knight <[email protected]>
Co-authored-by: Gustavo Lira e Silva <[email protected]>

* Regenerate bundle manifests

Co-authored-by: rm3l <[email protected]>

* Apply suggestions from code review

Co-authored-by: Patrick Knight <[email protected]>
Co-authored-by: Gustavo Lira e Silva <[email protected]>
Co-authored-by: Nick Boldt <[email protected]>

* Regenerate bundle manifests

Co-authored-by: rm3l <[email protected]>

---------

Co-authored-by: Patrick Knight <[email protected]>
Co-authored-by: Gustavo Lira e Silva <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: rm3l <[email protected]>
Co-authored-by: Nick Boldt <[email protected]>
Co-authored-by: Tomas Kral <[email protected]>
rm3l added a commit to redhat-developer/rhdh-operator that referenced this pull request Dec 13, 2024
)

* fix: adjust startup, liveness and readiness probes settings

Startup probe settings seem to have been added in the upstream Backstage Chart
in [1], but the current settings do not allow the RHDH Chart for the
liveness probe to be triggered sufficiently enough for the app to be
considered live.
This adjust such settings by accounting for the worst case scenario
where the application might take a bit long to start.

This also aligns the probe endpoints with the upstream chart.

[1] backstage/charts#216

* Update bundle/rhdh/manifests/rhdh-default-config_v1_configmap.yaml




* Regenerate bundle manifests



* Apply suggestions from code review





* Regenerate bundle manifests



---------

Co-authored-by: Patrick Knight <[email protected]>
Co-authored-by: Gustavo Lira e Silva <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: rm3l <[email protected]>
Co-authored-by: Nick Boldt <[email protected]>
Co-authored-by: Tomas Kral <[email protected]>
nickboldt added a commit to redhat-developer/rhdh-chart that referenced this pull request Dec 13, 2024
* Adjust startup, liveness and readiness probes settings

Startup probe settings seem to have been added in the upstream Backstage Chart
in [1], but the current settings do not allow the RHDH Chart for the
liveness probe to be triggered sufficiently enough for the app to be
considered live.
This adjust such settings by accounting for the worst case scenario
where the application might take a bit long to start.

This also aligns the probe endpoints with the upstream chart.

[1] backstage/charts#216

* Update Chart version

* Update charts/backstage/values.yaml

Co-authored-by: Patrick Knight <[email protected]>
Co-authored-by: Gustavo Lira e Silva <[email protected]>

* Add comment explaining why there is no 'initialDelaySeconds' on both liveness and readiness probes

Co-authored-by: Nick Boldt <[email protected]>

---------

Co-authored-by: Patrick Knight <[email protected]>
Co-authored-by: Gustavo Lira e Silva <[email protected]>
Co-authored-by: Nick Boldt <[email protected]>
rm3l added a commit to rm3l/rhdh-chart that referenced this pull request Dec 13, 2024
…eveloper#50)

* Adjust startup, liveness and readiness probes settings

Startup probe settings seem to have been added in the upstream Backstage Chart
in [1], but the current settings do not allow the RHDH Chart for the
liveness probe to be triggered sufficiently enough for the app to be
considered live.
This adjust such settings by accounting for the worst case scenario
where the application might take a bit long to start.

This also aligns the probe endpoints with the upstream chart.

[1] backstage/charts#216

* Update Chart version

* Update charts/backstage/values.yaml

Co-authored-by: Patrick Knight <[email protected]>
Co-authored-by: Gustavo Lira e Silva <[email protected]>

* Add comment explaining why there is no 'initialDelaySeconds' on both liveness and readiness probes

Co-authored-by: Nick Boldt <[email protected]>

---------

Co-authored-by: Patrick Knight <[email protected]>
Co-authored-by: Gustavo Lira e Silva <[email protected]>
Co-authored-by: Nick Boldt <[email protected]>
nickboldt pushed a commit to redhat-developer/rhdh-chart that referenced this pull request Dec 13, 2024
* Adjust startup, liveness and readiness probes settings

Startup probe settings seem to have been added in the upstream Backstage Chart
in [1], but the current settings do not allow the RHDH Chart for the
liveness probe to be triggered sufficiently enough for the app to be
considered live.
This adjust such settings by accounting for the worst case scenario
where the application might take a bit long to start.

This also aligns the probe endpoints with the upstream chart.

[1] backstage/charts#216

* Update Chart version

* Update charts/backstage/values.yaml

Co-authored-by: Patrick Knight <[email protected]>
Co-authored-by: Gustavo Lira e Silva <[email protected]>

* Add comment explaining why there is no 'initialDelaySeconds' on both liveness and readiness probes

Co-authored-by: Nick Boldt <[email protected]>

---------

Co-authored-by: Patrick Knight <[email protected]>
Co-authored-by: Gustavo Lira e Silva <[email protected]>
Co-authored-by: Nick Boldt <[email protected]>

fix URLs and bump to 2.24.2 in 1.4 branch

Signed-off-by: RHDH Build (rhdh-bot) <[email protected]>
rm3l added a commit to redhat-developer/rhdh-chart that referenced this pull request Dec 13, 2024
* Adjust startup, liveness and readiness probes settings

Startup probe settings seem to have been added in the upstream Backstage Chart
in [1], but the current settings do not allow the RHDH Chart for the
liveness probe to be triggered sufficiently enough for the app to be
considered live.
This adjust such settings by accounting for the worst case scenario
where the application might take a bit long to start.

This also aligns the probe endpoints with the upstream chart.

[1] backstage/charts#216

* Update Chart version

* Update charts/backstage/values.yaml




* Add comment explaining why there is no 'initialDelaySeconds' on both liveness and readiness probes



---------





fix URLs and bump to 2.24.2 in 1.4 branch

Signed-off-by: RHDH Build (rhdh-bot) <[email protected]>
Co-authored-by: Armel Soro <[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