🌱 fix(catalogd): follow-up fixes after HA PR merge#2679
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Follow-up fixes to the catalogd HA work to improve diagnosability of server start failures and to make the @CatalogdHA e2e gate reflect actual catalogd replica count (instead of a coarse multi-node heuristic).
Changes:
- Wrap
http.Server.Serveerrors with the configured catalog listen address for better aggregated log messages. - Refine
CatalogdHAe2e gating by reading the catalogd Deployment’s.spec.replicasonceolmNamespaceis known.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
test/e2e/steps/hooks.go |
Overrides CatalogdHA gate based on catalogd Deployment replica count. |
internal/catalogd/serverutil/serverutil.go |
Improves error context when the catalog server fails to serve. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tmshort
force-pushed
the
fix-catalogd-ha-readiness-more
branch
from
d13c3ac to
1384f29
Compare
|
|
||
| // Refine CatalogdHA based on actual catalogd replica count now that | ||
| // olmNamespace is known. The node-count check above can fire on any | ||
| // multi-node cluster (e.g. OpenShift standard e2e AWS) even when catalogd |
There was a problem hiding this comment.
nit: please remove the e.g.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2679 +/- ##
=======================================
Coverage 67.98% 67.98%
=======================================
Files 144 144
Lines 10595 10595
=======================================
Hits 7203 7203
Misses 2870 2870
Partials 522 522
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:
|
Two fixes on top of the catalogd HA work (operator-framework#2674): 1. serverutil: wrap http.Server.Serve error with the catalog listen address so the failure message is self-diagnosing in aggregated manager logs. 2. e2e: fix CatalogdHA gate to override node-count check with actual catalogd replica count. The node-count heuristic fires on any multi-node cluster (e.g. OpenShift standard e2e on AWS), causing the @CatalogdHA scenario to run even when catalogd has only 1 replica, which makes the step "catalogd is ready to reconcile resources" fail with exit status 1. After olmNamespace is populated, query the catalogd deployment's spec.replicas and set the gate unconditionally to (replicas >= 2), overriding the earlier node-count result. For upgrade scenarios the detectOLMDeployment early-return keeps the node-count value intact. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Todd Short <tshort@redhat.com>
tmshort
force-pushed
the
fix-catalogd-ha-readiness-more
branch
from
1384f29 to
eaa55aa
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pedjak, rashmigottipati 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 |
openshift-ci
Bot
added
the
approved
|
/override codecov/patch |
|
@tmshort: Overrode contexts on behalf of tmshort: codecov/patch 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 kubernetes-sigs/prow repository. |
openshift-merge-bot
Bot
merged commit b177be3
into
operator-framework:main
Two fixes on top of the catalogd HA work (#2674) that was merged prematurely:
serverutil: wrap http.Server.Serve error with the catalog listen address so the failure message is self-diagnosing in aggregated manager logs.
e2e: fix CatalogdHA gate to override node-count check with actual catalogd replica count. The node-count heuristic fires on any multi-node cluster (e.g. OpenShift standard e2e on AWS), causing the @CatalogdHA scenario to run even when catalogd has only 1 replica, which makes the step "catalogd is ready to reconcile resources" fail with exit status 1. After olmNamespace is populated, query the catalogd deployment's spec.replicas and set the gate unconditionally to (replicas >= 2), overriding the earlier node-count result. For upgrade scenarios the detectOLMDeployment early-return keeps the node-count value intact.
Description
Reviewer Checklist