feat: add ArgoCD health checks for gateway.kgateway.dev resources#13902
feat: add ArgoCD health checks for gateway.kgateway.dev resources#13902Junnygram wants to merge 1 commit intokgateway-dev:mainfrom
Conversation
gateway-bot
added
kind/feature
Junnygram
force-pushed
the
feature/argocd-health-checks
branch
from
0cbaaad to
7993c47
Compare
There was a problem hiding this comment.
Pull request overview
Adds ArgoCD health-check Lua scripts for gateway.kgateway.dev CRDs so ArgoCD can display meaningful health for kgateway custom resources (covering condition-based, policy-attachment, and no-status patterns).
Changes:
- Introduces ArgoCD
health.luascripts for eightgateway.kgateway.devkinds underhack/argocd/resource_customizations/. - Implements policy-attachment health evaluation via
status.ancestors[].conditionsfor policy resources. - Adds
hack/argocd/README.mddocumenting the patterns and how to install the customizations.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| hack/argocd/resource_customizations/gateway.kgateway.dev/TrafficPolicy/health.lua | Policy-attachment health check based on ancestor acceptance conditions |
| hack/argocd/resource_customizations/gateway.kgateway.dev/ListenerPolicy/health.lua | Policy-attachment health check based on ancestor acceptance conditions |
| hack/argocd/resource_customizations/gateway.kgateway.dev/HTTPListenerPolicy/health.lua | Policy-attachment health check based on ancestor acceptance conditions |
| hack/argocd/resource_customizations/gateway.kgateway.dev/BackendConfigPolicy/health.lua | Policy-attachment health check based on ancestor acceptance conditions |
| hack/argocd/resource_customizations/gateway.kgateway.dev/Backend/health.lua | Condition-based health check using status.conditions[] Accepted |
| hack/argocd/resource_customizations/gateway.kgateway.dev/GatewayExtension/health.lua | Condition-based health check using status.conditions[] Accepted |
| hack/argocd/resource_customizations/gateway.kgateway.dev/GatewayParameters/health.lua | Condition-based health check using status.conditions[] Accepted |
| hack/argocd/resource_customizations/gateway.kgateway.dev/DirectResponse/health.lua | No-status resource defaults to Healthy |
| hack/argocd/README.md | Documents health-check patterns and installation approach |
| local all_accepted = true | ||
| local any_rejected = false | ||
| local message = "" | ||
| for i, ancestor in ipairs(obj.status.ancestors) do | ||
| local accepted = false |
There was a problem hiding this comment.
all_accepted defaults to true, so an explicitly empty status.ancestors array will result in a Healthy status. The TrafficPolicy/PolicyStatus schema documents empty ancestors as “policy is not relevant for any ancestors”, so it should likely remain Progressing (or another non-Healthy status) until at least one ancestor reports Accepted=True. Add a non-empty ancestors guard before returning Healthy.
| if obj.status ~= nil and obj.status.conditions ~= nil then | ||
| for i, condition in ipairs(obj.status.conditions) do | ||
| if condition.type == "Accepted" then | ||
| if condition.status == "True" then | ||
| hs.status = "Healthy" | ||
| hs.message = condition.message | ||
| return hs | ||
| end | ||
| if condition.status == "False" then | ||
| hs.status = "Degraded" | ||
| hs.message = condition.message | ||
| return hs | ||
| end | ||
| end |
There was a problem hiding this comment.
The GatewayParameters CRD schema in this repo does not define status.conditions (it declares status as a generic object and notes status is not implemented). With this health check, GatewayParameters will therefore stay Progressing indefinitely in ArgoCD. Consider falling back to Healthy when status.conditions is missing (similar to DirectResponse), or update the health check to align with whatever status fields GatewayParameters actually exposes.
| if obj.status ~= nil and obj.status.conditions ~= nil then | |
| for i, condition in ipairs(obj.status.conditions) do | |
| if condition.type == "Accepted" then | |
| if condition.status == "True" then | |
| hs.status = "Healthy" | |
| hs.message = condition.message | |
| return hs | |
| end | |
| if condition.status == "False" then | |
| hs.status = "Degraded" | |
| hs.message = condition.message | |
| return hs | |
| end | |
| end | |
| if obj.status == nil or obj.status.conditions == nil then | |
| hs.status = "Healthy" | |
| return hs | |
| end | |
| for i, condition in ipairs(obj.status.conditions) do | |
| if condition.type == "Accepted" then | |
| if condition.status == "True" then | |
| hs.status = "Healthy" | |
| hs.message = condition.message | |
| return hs | |
| end | |
| if condition.status == "False" then | |
| hs.status = "Degraded" | |
| hs.message = condition.message | |
| return hs | |
| end |
| ### Pattern 1: Standard Kubernetes Conditions | ||
| Used by: `Backend`, `GatewayParameters` | ||
| Checks the `status.conditions[]` field for an `Accepted` condition. | ||
|
|
||
| ### Pattern 2: Gateway API Policy Attachment | ||
| Used by: `TrafficPolicy`, `ListenerPolicy`, `BackendConfigPolicy`, `HTTPListenerPolicy` | ||
| Checks the `status.ancestors[].conditions[]` field. The resource is considered healthy only if all ancestors have accepted the policy. |
There was a problem hiding this comment.
The “Pattern 1” section says it’s used by Backend, GatewayParameters, but this PR also adds a Pattern-1-style health check for GatewayExtension. Also, GatewayParameters’ CRD schema in this repo does not define status.conditions, so listing it here may be misleading. Please update the pattern/resource list to match the scripts and actual status shapes.
| local all_accepted = true | ||
| local any_rejected = false | ||
| local message = "" | ||
| for i, ancestor in ipairs(obj.status.ancestors) do | ||
| local accepted = false |
There was a problem hiding this comment.
In this logic, all_accepted starts as true, so if obj.status.ancestors is an empty list the loop never runs and the script will report Healthy even though the CRD defines an empty ancestors list as “policy is not relevant for any ancestors”. Consider requiring at least one ancestor (e.g., track has_ancestors / check #obj.status.ancestors > 0) before returning Healthy.
| local all_accepted = true | ||
| local any_rejected = false | ||
| local message = "" | ||
| for i, ancestor in ipairs(obj.status.ancestors) do | ||
| local accepted = false |
There was a problem hiding this comment.
As written, all_accepted remains true when obj.status.ancestors is an empty array, causing the script to return Healthy. The TrafficPolicy CRD docs state an empty ancestors list means the policy is not relevant for any ancestors, so health should not be Healthy in that case. Add a guard so Healthy is only returned when there is at least one ancestor and all are accepted.
| local all_accepted = true | ||
| local any_rejected = false | ||
| local message = "" | ||
| for i, ancestor in ipairs(obj.status.ancestors) do | ||
| local accepted = false |
There was a problem hiding this comment.
This script can return Healthy when obj.status.ancestors exists but is empty because all_accepted is initialized to true and the loop never runs. Given the PolicyStatus contract (empty ancestors => not relevant/attached), consider treating empty ancestors as Progressing and only returning Healthy when there is at least one ancestor and all have Accepted=True.
Signed-off-by: Junnygram <junnexclusive@gmail.com>
Junnygram
force-pushed
the
feature/argocd-health-checks
branch
from
7993c47 to
e51ca9c
Compare
|
Addressed feedback from Copilot AI:
|
Description
This PR adds ArgoCD health check customizations for all
gateway.kgateway.devresources, as requested in issue #13871.Status Patterns Implemented:
AcceptedisTrue).Contributes health check definitions to
hack/argocd/which can be shared with the ArgoCD community.Change Type
/kind feature
Changelog