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

KuberhealthyCheck status field instead of separate KuberhealthyState object #1120

Open
LS80 opened this issue Mar 17, 2023 · 5 comments
Open
Labels
do-not-close Prevents issues from being automatically closed if stale feature request A request for a specific feature to be added to Kuberhealthy

Comments

@LS80
Copy link

LS80 commented Mar 17, 2023

Describe the feature you would like and why you want it

Currently the status of a KuberhealthyCheck is stored in a separate KuberhealthyState object of the same name. Is there a good reason why this isn't instead stored in a status field of the KuberhealthyCheck itself, as is customary in Kubernetes?

Additional context

The separate object makes it impossible to create a custom Argo CD health check for the KuberhealthyCheck, which would allow Argo CD to show the application as unhealthy if a check is failing.

@LS80 LS80 added the feature request A request for a specific feature to be added to Kuberhealthy label Mar 17, 2023
ugurzongur added a commit to ugur-zongur/kuberhealthy that referenced this issue May 2, 2023
Fixes kuberhealthy#1120

Signed-off-by: Ugur Zongur <ugur.zongur@thetradedesk.com>
ugurzongur added a commit to ugur-zongur/kuberhealthy that referenced this issue May 2, 2023
Fixes kuberhealthy#1120

Signed-off-by: Ugur Zongur
ugurzongur added a commit to ugur-zongur/kuberhealthy that referenced this issue May 2, 2023
Fixes kuberhealthy#1120

Signed-off-by: Ugur Zongur
ugurzongur added a commit to ugur-zongur/kuberhealthy that referenced this issue May 2, 2023
Fixes kuberhealthy#1120

Signed-off-by: Ugur Zongur
ugurzongur added a commit to ugur-zongur/kuberhealthy that referenced this issue May 2, 2023
ugurzongur added a commit to ugur-zongur/kuberhealthy that referenced this issue May 2, 2023
Fixes kuberhealthy#1120

Signed-off-by: Ugur Zongur <104643878+ugurzongur@users.noreply.github.com>
ugurzongur added a commit to ugur-zongur/kuberhealthy that referenced this issue May 4, 2023
Fixes kuberhealthy#1120

Signed-off-by: Ugur Zongur <104643878+ugurzongur@users.noreply.github.com>
@Hungrylion2019
Copy link
Collaborator

LGTM. But the code heavily depend on the khstate.
I usually see the khstate or the api "healthCheckHandler" to get current concrete check results.

@fungaren
Copy link

See: #1061 (comment)

@sedflix
Copy link

sedflix commented Jul 19, 2023

I feel that following the kstatus spec would also be helpful for flux health check.

For custom resource definitions (CRDs), there currently isn't much guidance on which properties should be exposed in the status object and which conditions should be used. As part of this effort we want to define a set of standard conditions that the library will understand and that we encourage developers to adopt in their CRDs. These standard conditions will be focused on providing the necessary information for understanding status of the reconcile after apply and it is not expected that these will necessarily be the only conditions exposed in a custom resource

It seems like a good thing to follow as per the motivation of the spec.

Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment on the issue or this will be closed in 15 days.

@github-actions github-actions bot added the Stale label Jan 26, 2024
@integrii integrii added the do-not-close Prevents issues from being automatically closed if stale label Feb 10, 2024
@integrii
Copy link
Collaborator

I think there are a few reasons we are considering revising the khcheck resource, and using the status section for khstate sounds like a good idea. When we first implemented Kuberhealthy, CRDs didn't usually have a status field!

If we do a CRD refactor, it seems prudent to think about getting rid of the khstate resource...

@integrii integrii removed the Stale label Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-close Prevents issues from being automatically closed if stale feature request A request for a specific feature to be added to Kuberhealthy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants