RHOAIENG-20123: Enhanced GuardrailsOrchestrator status messages #541
Conversation
- Added detailed status messages for all reconciliation stages - Implemented proper error handling for missing InferenceServices - Added Available and Degraded condition types - Enhanced progress tracking with component-level status - Added Kubernetes event recording for better observability - Fixed issue where missing InferenceServices showed as 'not ready' instead of error
|
@artemsa223: This pull request references RHOAIENG-20123 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "4.20.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Reviewer's GuideThis PR enriches the GuardrailsOrchestrator status reporting by introducing new condition types and helper methods, supplying detailed messages for each reconciliation stage, refining error handling for missing or failing components, and injecting Kubernetes events for observability. Sequence diagram for enhanced GuardrailsOrchestrator reconciliation and status reportingsequenceDiagram
participant Controller
participant K8sAPI
participant Recorder
participant Status
Controller->>Status: SetProgressingCondition (Initializing)
Controller->>Recorder: Event: Initializing
Controller->>K8sAPI: Get ServiceAccount
alt ServiceAccount not found
Controller->>K8sAPI: Create ServiceAccount
Controller->>Status: SetDegradedCondition (ServiceAccountCreationFailed)
Controller->>Recorder: Event: ServiceAccountCreated
end
Controller->>K8sAPI: Get ConfigMap
alt ConfigMap not found
Controller->>Status: SetDegradedCondition (ConfigMapNotFound)
Controller->>Recorder: Event: ConfigMapNotFound
end
Controller->>K8sAPI: Get Deployment
alt Deployment not found
Controller->>K8sAPI: Create Deployment
Controller->>Status: SetDegradedCondition (DeploymentCreationFailed)
Controller->>Recorder: Event: DeploymentCreated
end
Controller->>K8sAPI: Get Service
alt Service not found
Controller->>K8sAPI: Create Service
Controller->>Status: SetDegradedCondition (ServiceCreationFailed)
Controller->>Recorder: Event: ServiceCreated
end
Controller->>K8sAPI: Get Routes
alt Route not found
Controller->>K8sAPI: Create Route
Controller->>Status: SetDegradedCondition (RouteCreationFailed)
Controller->>Recorder: Event: RouteCreated
end
Controller->>Status: reconcileStatuses (component-level)
alt All components ready
Status->>Status: SetAvailableCondition (true)
Status->>Status: SetDegradedCondition (false)
Status->>Recorder: Event: Ready
else Missing InferenceService
Status->>Status: SetAvailableCondition (false)
Status->>Status: SetDegradedCondition (true)
Status->>Recorder: Event: Failed
else Any component failed
Status->>Status: SetAvailableCondition (false)
Status->>Status: SetDegradedCondition (true)
Status->>Recorder: Event: Failed
else Progressing
Status->>Status: SetAvailableCondition (false)
Status->>Status: SetDegradedCondition (false)
end
Entity relationship diagram for GuardrailsOrchestrator status conditionserDiagram
GUARDRAILS_ORCHESTRATOR ||--o{ CONDITION : has
CONDITION {
string Type
string Status
string Reason
string Message
}
GUARDRAILS_ORCHESTRATOR {
string Name
string Namespace
string Phase
}
Class diagram for updated GuardrailsOrchestrator status conditions and error handlingclassDiagram
class GuardrailsOrchestratorReconciler {
+updateStatus(ctx, orig, updateFn)
+reconcileStatuses(ctx, orchestrator)
+checkGeneratorPresent(ctx, namespace)
}
class NoInferenceServicesError {
+Namespace: string
+Error() string
}
class Condition {
+Type: ConditionType
+Status: ConditionStatus
+Reason: string
+Message: string
}
class ConditionType {
<<enumeration>>
ReconcileComplete
Progressing
Available
Degraded
}
class ConditionStatus {
<<enumeration>>
True
False
}
GuardrailsOrchestratorReconciler --> NoInferenceServicesError
GuardrailsOrchestratorReconciler --> Condition
Condition --> ConditionType
Condition --> ConditionStatus
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Hi @artemsa223. Thanks for your PR. I'm waiting for a trustyai-explainability member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@artemsa223: This pull request references RHOAIENG-20123 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "4.20.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The reconcileStatuses function is quite large and covers multiple concerns; consider extracting component-specific status logic into smaller helper functions to improve readability and maintainability.
- Comparing err.Error() to literal strings (e.g., "not ready") is brittle—introduce sentinel error types or constants to represent known error conditions instead of string matching.
- Calls to updateStatus often ignore the returned error (
orchestrator, _ = r.updateStatus…); make sure to handle or log these errors so status updates don’t fail silently.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The reconcileStatuses function is quite large and covers multiple concerns; consider extracting component-specific status logic into smaller helper functions to improve readability and maintainability.
- Comparing err.Error() to literal strings (e.g., "not ready") is brittle—introduce sentinel error types or constants to represent known error conditions instead of string matching.
- Calls to updateStatus often ignore the returned error (`orchestrator, _ = r.updateStatus…`); make sure to handle or log these errors so status updates don’t fail silently.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
4 participants
Summary by Sourcery
Enhance GuardrailsOrchestrator status reporting by adding detailed status and reason messages, new Available/Degraded conditions, granular phase tracking, custom error handling for missing InferenceServices, and Kubernetes event recording for better observability.
New Features:
Bug Fixes:
Enhancements: