crossplane-contrib / crossplane-contrib/function-auto-ready
Concerned about healthcheck condition checks
- Dominant language
- Go
- Stars
- 36
- Forks
- 27
- Avg merge
- 2d 9h
- Merged PRs (30d)
- 9
Description
### What happened?
I examined the health checks and noticed some possible problems with the way conditions are checked. Many of the checks loop through the conditions array checking for both valid and invalid conditions and returns the state of the first condition it encounters. If a resource has both valid and invalid conditions in place, the success of the health check will be determined by the order of the conditions. The same exact set of conditions, but in a different order, can return different results.
For example this check in the [HPA check](https://github.com/crossplane-contrib/function-auto-ready/blob/main/healthchecks/horizontalpodautoscaler.go#L27-L47):
```go
for _, condition := range hpa.Status.Conditions {
// Check for degraded conditions
switch condition.Type {
case "FailedGetScale", "FailedUpdateScale", "FailedGetResourceMetric", "InvalidSelector":
if condition.Status == "True" {
return false
}
}
// Check for healthy conditions
switch condition.Type {
case autoscalingv2.ScalingActive:
if condition.Status == "True" {
return true
}
case autoscalingv2.ScalingLimited:
if condition.Status == "True" {
return true
}
}
}
```
The conditions array is sequentially walked through, and if both valid and invalid conditions are in place, the success of the check is determined by the order of the conditions.
### What environment did it happen in?
v0.6.0
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with healthchecks/horizontalpodautoscaler.go, especially lines 27-47, then inspect the other health-check implementations for the same condition-order behavior. Determine the intended precedence when valid and invalid conditions coexist; done means the result is consistent regardless of condition ordering.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100