🐛 Fix: Truncate large error messages in status conditions (OCPBUGS-59518, OCPBUGS-38567) #2169
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
camilamacedo86
changed the title
|
Can you include some details of the messages that are too long? I feel like arbitrarily truncating the message is sort of papering over the underlying issue, which is that 30k-byte messages in conditions are a poor UX, and the real solution would be to make the message shorter to begin with. /hold |
openshift-ci
bot
added
the
do-not-merge/hold
| @@ -160,7 +160,7 @@ func ensureAllConditionsWithReason(ext *ocv1.ClusterExtension, reason v1alpha1.C | |||
| Type: condType, | |||
| Status: metav1.ConditionFalse, | |||
| Reason: string(reason), | |||
| Message: message, | |||
| Message: truncateMessage(message), | |||
There was a problem hiding this comment.
If there's a limit imposed on all condition messages, it seems like we need to make sure that we truncate all condition messages.
This is one of many places where we set condition messages, right?
We may need to implement a wrapper around the meta.SetCondition() that:
- truncates messages
- everything throughout our project uses.
There was a problem hiding this comment.
Yes, I think wrapper will be better as well +1
camilamacedo86
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
This comment was marked as outdated.
This comment was marked as outdated.
camilamacedo86
force-pushed
the
fix-preflight
branch
from
3fd05a8 to
9121120
Compare
camilamacedo86
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
camilamacedo86
force-pushed
the
fix-preflight
branch
2 times, most recently
from
aec2345 to
b30dc47
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2169 +/- ##
==========================================
+ Coverage 72.73% 72.77% +0.03%
==========================================
Files 79 79
Lines 7384 7391 +7
==========================================
+ Hits 5371 5379 +8
+ Misses 1666 1665 -1
Partials 347 347
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
camilamacedo86
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
/hold cancel Thanks Camila! +1 on solving this both ways. Truncate long messages AND try to avoid long messages in the first place! |
openshift-ci
bot
removed
the
do-not-merge/hold
camilamacedo86
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
I've noticed this as well. Not only are they large, but the information density is low. IIRC, the CRD upgrade check output can contain a "diff-style" output that show context around the diff, which isn't really useful. I think we should revisit our message formatting specifically in the case of the CRD upgrade check output to make it easier to understand:
|
camilamacedo86
force-pushed
the
fix-preflight
branch
from
b30dc47 to
f8f409a
Compare
When upgrading operators, CRD validation errors can be very large (50KB+). Kubernetes rejects status updates over 32KB with "Too long: may not be more than 32768 bytes". This causes ClusterExtension upgrades to fail and get stuck. Assisted-by: Cursor
camilamacedo86
force-pushed
the
fix-preflight
branch
from
f8f409a to
87fb307
Compare
|
I think it is good to get merged. I think we should truncate anyway. I updated the last comment in : https://issues.redhat.com/browse/OCPBUGS-59518 The problem here is |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: perdasilva The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
openshift-merge-bot
bot
merged commit 81be2e9
into
operator-framework:main
When upgrading operators, CRD validation errors can be very large (50KB+). Kubernetes rejects status updates over 32KB with "Too long: may not be more than 32768 bytes". This causes ClusterExtension upgrades to fail and get stuck.
Messages keep important info at the start and add "... [message truncated]" suffix. Now upgrades complete successfully even with large CRD validation errors.
Added unit tests for truncation logic and CRD error scenarios.
Reviewer Checklist