-
Notifications
You must be signed in to change notification settings - Fork 192
RHOAIENG-31497 - Monitoring namespace is not created on self-managed clusters - display error #2257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RHOAIENG-31497 - Monitoring namespace is not created on self-managed clusters - display error #2257
Conversation
WalkthroughThe changes update error handling within deployment and resource application logic. Specifically, "not found" errors are no longer ignored in the main deployment and resource apply paths, while the status application path retains its previous handling for such errors. No public interfaces or function signatures were modified. Changes
Sequence Diagram(s)sequenceDiagram
participant Controller
participant Resources
Controller->>Resources: Apply(resource)
alt Patch returns error
alt Previous: NotFound error
Resources-->>Controller: Return nil (ignore error)
else Other error
Resources-->>Controller: Return error with context
end
else No error
Resources-->>Controller: Success
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
/test opendatahub-operator-e2e |
7ca74f3
to
a79939b
Compare
/test opendatahub-operator-e2e |
a79939b
to
542723a
Compare
/test opendatahub-operator-e2e |
542723a
to
1e2b647
Compare
/test opendatahub-operator-e2e |
2502066
to
b372c53
Compare
…clusters - display error This commit doesn't fix the issue but allows any NotFound error - e.g. when a namespace if a resource to be created is missing, to be displayed to users.
b372c53
to
77c7819
Compare
/test image |
/test images |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2257 +/- ##
=======================================
Coverage 41.91% 41.92%
=======================================
Files 144 144
Lines 11389 11387 -2
=======================================
Hits 4774 4774
+ Misses 6214 6212 -2
Partials 401 401 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lburgazzoli 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 |
…clusters - display error (opendatahub-io#2257) This commit doesn't fix the issue but allows any NotFound error - e.g. when a namespace if a resource to be created is missing, to be displayed to users.
…clusters - display error (opendatahub-io#2257) This commit doesn't fix the issue but allows any NotFound error - e.g. when a namespace if a resource to be created is missing, to be displayed to users.
…tors are required to be installed despite no observability configured (#2258) * chore: Align otel condition with other observability operators * chore: Align monitoring controller actions workflow * RHOAIENG-31536 - OpenTelemetry and Cluster Observability operators are required to be installed despite no observability configured * fix: enable monitoring namespace for other platform if monitoring is enabled (#2250) * fix: enable moniotoring for other platform if monitoring is enabled - ODH and self-managed : is monitoring is enabled - managed : always Signed-off-by: Wen Zhou <[email protected]> * fix: wrong logic, the one should always create namespace is Managed Signed-off-by: Wen Zhou <[email protected]> --------- Signed-off-by: Wen Zhou <[email protected]> * RHOAIENG-31643 - otel-collector is not deployed when only traces are enabled (#2262) * RHOAIENG-31497 - Monitoring namespace is not created on self-managed clusters - display error (#2257) This commit doesn't fix the issue but allows any NotFound error - e.g. when a namespace if a resource to be created is missing, to be displayed to users. --------- Signed-off-by: Wen Zhou <[email protected]> Co-authored-by: Marián Macik <[email protected]> Co-authored-by: Wen Zhou <[email protected]> Co-authored-by: Marián Macik <[email protected]>
Description
This commit doesn't fix the issue but allows any NotFound error - e.g. when a namespace if a resource to be created is missing, to be displayed to users.
How Has This Been Tested?
Manually on a separate cluster all components were able to deploy, so there doesn't seem to be a reason for ignoring NotFound errors as with patch/apply operation they can only happen if something is wrong with the environment, e.g. a resource is to be created in a non-existing namespace, which users should be informed about.
Screenshot or short clip
N/A
Merge criteria
Summary by CodeRabbit