[AI] OSAC-1827: Operator treats transient KubeVirt PrintableStatus as fatal provisioning failure - #338
Conversation
…l provisioning failure Co-authored-by: osac-dev-bot <osac-automation@redhat.com>
|
AI Session Costs
|
|
@osac-jira-ai-issue-solver[bot]: This pull request references OSAC-1827 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 bug to target the "5.0.0" version, but no target version was set. DetailsIn 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. |
|
Hi @osac-jira-ai-issue-solver[bot]. Thanks for your PR. I'm waiting for a osac-project member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
In comment, @openshift-ci-robot said:
Acknowledged — the Jira target version warning is a ticket configuration issue, not a code change. No code modifications needed. |
|
In comment, @openshift-ci[bot] said:
Acknowledged — approval status notification. Waiting for human reviewer. |
|
In comment, @openshift-ci[bot] said:
Acknowledged — ok-to-test gate. Waiting for org member to verify the patch. |
|
/ok-to-test |
|
Labeled |
eranco74
left a comment
There was a problem hiding this comment.
Code Review
1. Permanently unschedulable VMs will stay in Starting forever (critical)
The fix correctly handles the transient case (VM booting, scheduler needs a moment), but removes the safety net for the permanent case (VM requests impossible resources like 1TB RAM or a nonexistent GPU type). There's no timeout or escalation — handleKubeVirtVM will keep setting ReasonScheduling on every reconcile, the feedback controller will keep syncing COMPUTE_INSTANCE_STATE_STARTING to fulfillment-service, and the user will see a VM stuck in "Starting" indefinitely with no indication it will never succeed.
Consider adding a timeout or retry counter: after N minutes of continuous Unschedulable, escalate to Failed/ProvisioningFailed so the user knows intervention is needed.
2. Stale doc comments (minor)
Two doc comments still reference ErrorUnschedulable as mapping to Failed:
provisioningErrorMessagedoc (line ~898): says "ErrorUnschedulable, etc." but the case was removed from the switchdeterminePhaseFromPrintableStatusfunction-level doc (line ~974): says "All remaining values (…ErrorUnschedulable…) map to Failed" — the inline comment at the default case was correctly updated, but this one was missed
|
Follow-up task created for the timeout concern: OSAC-2309 — Add timeout for permanently unschedulable ComputeInstances. /lgtm |
## Summary - Fix cost label oscillation observed in production on [osac-operator PR #338](osac-project/osac-operator#338 (comment)): labels cycled between "Feedback (15)" and "Feedback (14) retry 1" because `countFeedbackRounds` excluded error entries from the count - Remove `!strings.Contains(e.Label, "error")` filter — error entries with round numbers (e.g., "Feedback (15) (error)") should count as rounds since `feedbackLabel` always includes the round number - Follow-up to #44 which introduced the error exclusion based on a code review concern about bare "Feedback (error)" labels, which can't occur in practice ## Test plan - [x] Updated `TestCountFeedbackRounds` — "error entries count as rounds" replaces "error entries do not count as rounds" - [x] All executor tests pass with `-race` - [x] Lint clean Assisted-by: Claude <noreply@anthropic.com>
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eranco74, osac-jira-ai-issue-solver[bot] The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
Resolves OSAC-1827
Summary
ReasonSchedulingcondition constant andschedulingWaitMessage()helperTest plan
determinePhaseFromPrintableStatus,provisioningErrorMessage, andhandleKubeVirtVMto reflect new transient status handlingschedulingWaitMessagehelpergolangci-lint run --fixreports 0 issuesFixes: OSAC-1827