Enhance Function status conditions #977
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances Function status reporting by introducing Kubernetes-native condition tracking for detailed reconciliation diagnostics. It replaces simple error strings with structured condition types that surface specific failure modes like database mismatches and criteria evaluation errors.
Key changes:
- Added comprehensive condition helper methods to Function API types with full test coverage
- Updated ingestor reconciliation logic to set detailed conditions for database matching, criteria evaluation, and Kusto execution outcomes
- Enhanced CRD documentation with troubleshooting guides and condition reference tables
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| api/v1/function_types.go | Added condition constants and helper methods for setting database match, criteria match, and reconciled conditions |
| api/v1/function_types_test.go | Comprehensive test coverage for all new condition helper methods |
| ingestor/adx/tasks.go | Updated reconciliation logic to set detailed conditions and clear stale errors |
| ingestor/adx/tasks_test.go | Added extensive test coverage for condition behavior across all reconciliation scenarios |
| ingestor/storage/kql_functions.go | Added UpdateCondition method and condition logging |
| operator/manifests/crds/functions_crd.yaml | Updated CRD with detailed condition documentation |
| kustomize/bases/functions_crd.yaml | Updated CRD with detailed condition documentation |
| docs/guides.md | Added troubleshooting section with condition-based diagnostic workflow |
| docs/crds.md | Added comprehensive condition reference documentation |
| // as the final variadic parameter to enrich messages for debugging. | ||
| func (f *Function) SetCriteriaMatchCondition(matched bool, expression string, err error, clusterLabels ...map[string]string) { | ||
| labels := map[string]string{} | ||
| if len(clusterLabels) > 0 && clusterLabels[0] != nil { |
There was a problem hiding this comment.
This function has an odd interface IMO - we accept a list of clusterLabels map[string]string but only ever consume the first one if it ever exists and have to do weird conditionals to see if the first element is nil.
I think we only ever call this with a single argument (or maybe none), can we simplify to accept a single map[string]string and have FormatClusterLabels handle a nil map (which should just be len == 0 in Go)?
| if function.Spec.Database != v1.AllDatabases && function.Spec.Database != t.kustoCli.Database() { | ||
| availableDB := t.kustoCli.Database() | ||
| configuredDB := function.Spec.Database | ||
| if configuredDB != v1.AllDatabases && !strings.EqualFold(configuredDB, availableDB) { |
There was a problem hiding this comment.
Is there a way we can set these conditions (database mismatch, criteria mismatch) once instead of n-times for the number of Kusto dbs we have? I think every worker for each database will repeatedly set these every reconcile period for ones that don't have matching configs.
2 participants
Summary
Testing