Skip to content
This repository has been archived by the owner on Aug 12, 2024. It is now read-only.

Add new "Healthy" type condition and Reasons #711

Merged
merged 6 commits into from
Oct 2, 2023
Merged

Add new "Healthy" type condition and Reasons #711

merged 6 commits into from
Oct 2, 2023

Conversation

jmprusi
Copy link
Member

@jmprusi jmprusi commented Aug 29, 2023 •
edited
Loading

Adds a "built-in" health check function under a feature gate.

It returns true if all resources are healthy with nil error.
It returns false if any of the resources are not healthy; the error contains the GVK + resource name and the error message of
each unhealthy resource.

The current list of supported resources is:

  • Deployments
  • StatefulSets
  • DaemonSets
  • ReplicaSets
  • Pods
  • APIServices
  • CustomResourceDefinitions

If the resource is not supported, it is assumed to be healthy.

closes #704
closes #705
closes #706

@jmprusi jmprusi requested a review from a team as a code owner August 29, 2023 13:22
@codecov
Copy link

codecov bot commented Aug 29, 2023 •
edited
Loading

Codecov Report

Attention: 64 lines in your changes are missing coverage. Please review.

Comparison is base (cdc6839) 19.21% compared to head (7d0dfb2) 24.67%.
Report is 2 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #711      +/-   ##
==========================================
+ Coverage   19.21%   24.67%   +5.45%     
==========================================
  Files          13       14       +1     
  Lines        1046     1143      +97     
==========================================
+ Hits          201      282      +81     
- Misses        801      807       +6     
- Partials       44       54      +10
Files Coverage Δ
internal/healthchecks/builtin.go 74.31% <74.31%> (ø)
...l/controllers/bundledeployment/bundledeployment.go 3.92% <0.00%> (+0.16%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

everettraven
everettraven previously approved these changes Aug 31, 2023
View reviewed changes
@stevekuznetsov
Copy link
Member

Was this PR supposed to include the implementation as well?

@everettraven
Copy link
Contributor

@stevekuznetsov My understanding from looking at the issues was that #706 would be for the implementation

@stevekuznetsov
Copy link
Member

Seems weird but ok

@ncdc
Copy link
Member

ncdc commented Sep 1, 2023

I would prefer that API changes are merged in a single PR with their implementation. Would you be able to do that for this?

@jmprusi
Copy link
Member Author

jmprusi commented Sep 4, 2023

I would prefer that API changes are merged in a single PR with their implementation. Would you be able to do that for this?

Totally! Makes more sense to me.

@ncdc ncdc marked this pull request as draft September 6, 2023 14:47
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 6, 2023
@jmprusi jmprusi force-pushed the jmprusi/new-healthy-condition branch from 21bd3c2 to 7c1b52f Compare September 6, 2023 14:49
@jmprusi jmprusi mentioned this pull request Sep 6, 2023
Closed
ncdc
ncdc reviewed Sep 6, 2023
View reviewed changes
internal/controllers/bundledeployment/bundledeployment.go Outdated Show resolved Hide resolved
internal/healthchecks/generic/generic.go Outdated Show resolved Hide resolved
internal/healthchecks/generic/generic.go Outdated Show resolved Hide resolved
internal/healthchecks/generic/generic.go Outdated Show resolved Hide resolved
@jmprusi jmprusi force-pushed the jmprusi/new-healthy-condition branch 4 times, most recently from 3fbda66 to 0f9a933 Compare September 8, 2023 15:13
@varshaprasad96 varshaprasad96 mentioned this pull request Sep 11, 2023
Closed
@jmprusi jmprusi force-pushed the jmprusi/new-healthy-condition branch from 0f9a933 to 7610a43 Compare September 12, 2023 11:23
@jmprusi jmprusi force-pushed the jmprusi/new-healthy-condition branch 3 times, most recently from a19f94c to 392a52f Compare September 18, 2023 13:26
@jmprusi jmprusi marked this pull request as ready for review September 18, 2023 13:57
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 18, 2023
@jmprusi jmprusi force-pushed the jmprusi/new-healthy-condition branch 4 times, most recently from 677c31f to 253041a Compare September 19, 2023 13:28
@ncdc ncdc added this to the v0.14.0 milestone Sep 19, 2023
everettraven
everettraven reviewed Sep 26, 2023
View reviewed changes
internal/healthchecks/builtin.go Outdated Show resolved Hide resolved
internal/healthchecks/builtin.go Show resolved Hide resolved
internal/healthchecks/builtin.go Show resolved Hide resolved
internal/healthchecks/builtin.go Outdated Show resolved Hide resolved
internal/healthchecks/builtin.go Outdated Show resolved Hide resolved
pkg/features/features.go Outdated Show resolved Hide resolved
@everettraven everettraven dismissed their stale review September 26, 2023 20:09

The approving review I gave was long ago and is stale

@jmprusi jmprusi force-pushed the jmprusi/new-healthy-condition branch from 253041a to 3b55b2e Compare September 28, 2023 10:42
ncdc
ncdc reviewed Sep 28, 2023
View reviewed changes
internal/healthchecks/builtin.go Outdated Show resolved Hide resolved
internal/healthchecks/builtin.go Outdated Show resolved Hide resolved
@jmprusi jmprusi force-pushed the jmprusi/new-healthy-condition branch from 3b55b2e to 1436e98 Compare October 2, 2023 11:31
@jmprusi
Adds new condition and Reasons
8b90547 
Signed-off-by: Joaquim Moreno Prusi <[email protected]>
@jmprusi
Adds feature gates infrastructure
f03ba40 
Signed-off-by: Joaquim Moreno Prusi <[email protected]>
@jmprusi
Adds BundleDeploymentHealth feature gate
fb2cac8 
Signed-off-by: Joaquim Moreno Prusi <[email protected]>
@jmprusi
Feature-gated Builtin healtcheck
bd15448 
Adds a "builtin" healthcheck function.

It returns true if all resources are healthy with nil error.
It returns false if any of the resources are not healthy, the error contains the GVK + resource name, and the error message of
each unhealthy resource.

The current list of supported resources is:
- Deployments
- StatefulSets
- DaemonSets
- ReplicaSets
- Pods
- APIServices
- CustomResourceDefinitions

If the resource is not supported, it is assumed to be healthy.

Signed-off-by: Joaquim Moreno Prusi <[email protected]>
@jmprusi
Adds APIRegistration to the client schema for healthcheck
21777b6 
Signed-off-by: Joaquim Moreno Prusi <[email protected]>
@jmprusi
Update ginkgo cli version to v2.12.0
7d0dfb2 
Signed-off-by: Joaquim Moreno Prusi <[email protected]>
@jmprusi jmprusi force-pushed the jmprusi/new-healthy-condition branch from 1436e98 to 7d0dfb2 Compare October 2, 2023 14:10
@ncdc ncdc enabled auto-merge October 2, 2023 14:20
@ncdc ncdc added this pull request to the merge queue Oct 2, 2023
Merged via the queue into operator-framework:main with commit 05f3afd Oct 2, 2023
11 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@joelanford joelanford joelanford left review comments

@stevekuznetsov stevekuznetsov stevekuznetsov left review comments

@everettraven everettraven everettraven left review comments

@ncdc ncdc ncdc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.14.0
6 participants
@jmprusi @stevekuznetsov @everettraven @ncdc @joelanford @openshift-merge-robot