fix: tolerate transient API latency in leader election - #333
Open
obrienrobert wants to merge 1 commit into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe manager now uses stored Kubernetes configuration and explicit leader-election timings. Liveness and readiness probes now have five-second timeouts. ChangesManager runtime tuning
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The PR adjusts leader-election timing, client rate limits, and probe timeout to tolerate transient API latency. No actionable merge-blocking risk remains based on the supplied evidence. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
obrienrobert
force-pushed
the
leader-election-fix
branch
from
August 7, 2026 21:30
fe950ae to
d96a0fd
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (1)
appconfig/rhplatformtest.yaml (1)
62-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the placeholder Snowflake backend.
The entry is disabled and carries an inline dummy
pat: "fdfdfs". Other environments reference secrets with thefile|prefix. Delete this block, or align it with therhsandboxnaming and thefile|secret reference used inappconfig/default.yaml.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@appconfig/rhplatformtest.yaml` around lines 62 - 67, Remove the disabled placeholder Snowflake backend named test, including its dummy pat and connection settings; do not leave the inline secret in rhplatformtest.yaml.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.gitlab-ci.yml:
- Around line 202-211: Update the image-publishing job rules or push commands so
merge request pipelines never push or overwrite "${IMAGE_URL}:latest". Preserve
latest publishing for the default branch and tag pipelines, while allowing merge
request pipelines to skip that shared-tag push.
- Line 196: Update the buildah login command in the CI configuration to remove
the password command-line argument and pass REGISTRY_PASSWORD through standard
input using --password-stdin, while preserving the existing registry username
and endpoint.
- Around line 251-277: Update the rules for deploy-operator-rhpreprod and
deploy-operator-rhprod so the rule matching CI_COMMIT_TAG explicitly sets when:
manual. Remove the separate job-level when: manual declarations, preserving
manual approval for tagged deployments.
In `@appconfig/rhplatformtest.yaml`:
- Around line 3-4: Update the consolidated Dataverse pattern and output mapping
so the tenant ID is captured and substituted into the group name: either make
the Dataverse-type alternation non-capturing while retaining $1, or keep both
captures and change output to $2_group.
In `@appconfig/rhpreprod.yaml`:
- Line 83: The offboardUserExclusionListConfigPath configuration uses a mutable
main-branch URL in both environments; update appconfig/rhpreprod.yaml lines
83-83 and appconfig/rhprod.yaml lines 101-101 to load from the usernaut-config
ConfigMap or pin the URL to a released tag, applying the same stable source in
both files.
In `@appconfig/rhprod.yaml`:
- Around line 21-25: Update the production Redis configuration under the redis
key so it no longer references the preproduction cache endpoint: replace host
with the correct production endpoint, or remove the entire redis block since the
current driver is memory.
- Around line 97-98: Update the production CORS configuration under
cors.allowed_origins to remove both localhost entries and retain only the
production Route hostname.
In `@config/default/overlays/rhpreprod/patches/manager-deployment-env.yaml`:
- Around line 10-13: Update the base manager Deployment in manager.yaml to set
the container security context field readOnlyRootFilesystem: true, then add
explicit writable volume mounts and corresponding volumes only for runtime paths
the manager requires. Preserve the existing read-only config and secret mounts
and restrictive security settings.
In `@config/manager/manager.yaml`:
- Around line 93-106: Update the manager’s livenessProbe settings to reduce the
failure-recovery window: lower its initialDelaySeconds and/or failureThreshold,
or add a startupProbe to preserve slow-start tolerance while allowing wedged
instances to restart within the availability target. Keep the readinessProbe’s
higher tolerance unchanged unless required otherwise.
In
`@config/mpp-manual/tenant-namespace/overlays/usernaut-rhplatformtest/tenant-egress.yaml`:
- Around line 7-10: Replace the unrestricted cidrSelector in the TenantEgress
egress rule with the specific CIDR ranges required by Usernaut integrations. If
unrestricted access is intentionally allowed for this platform-test environment,
document that exception and scope enforcement exclusively to this environment.
In `@Makefile`:
- Around line 237-240: Update the configure-mpp-all target so its name matches
its behavior: either rename it to configure-mpp-rhprod and update all
references, or make it aggregate the configure-mpp-rhplatformtest,
configure-mpp-rhpreprod, and configure-mpp-rhprod targets so every environment
is configured.
- Around line 248-252: Update the configure-usernaut-rhpreprod target
declaration to include the kustomize prerequisite, matching the prerequisite
pattern used by the configure-mpp-* targets, while leaving its existing recipe
commands unchanged.
In `@scripts/configmap.sh`:
- Line 5: Update the kubectl invocation in the script to quote the
environment-derived --from-file source and -n namespace arguments, preventing
word splitting and glob expansion. Add validation near the script entry point to
reject unset ENVIRONMENT values and unsupported environment names before
constructing the configmap command, while preserving the existing apply flow.
---
Nitpick comments:
In `@appconfig/rhplatformtest.yaml`:
- Around line 62-67: Remove the disabled placeholder Snowflake backend named
test, including its dummy pat and connection settings; do not leave the inline
secret in rhplatformtest.yaml.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ebaca8cd-9317-428a-9fc6-b9ff4e12c47d
📒 Files selected for processing (41)
.gitlab-ci.ymlCODEOWNERSDockerfile.redhatMakefileappconfig/default.yamlappconfig/exclusionlist.yamlappconfig/rhplatformtest.yamlappconfig/rhpreprod.yamlappconfig/rhprod.yamlcmd/main.goconfig/default/base/kustomization.yamlconfig/default/overlays/rhpreprod/kustomization.yamlconfig/default/overlays/rhpreprod/patches/manager-deployment-env.yamlconfig/default/overlays/rhpreprod/patches/route-env.yamlconfig/default/overlays/rhprod/kustomization.yamlconfig/default/overlays/rhprod/patches/route-env.yamlconfig/groups/dataverse-platform-rhplatformtestaccess.yamlconfig/manager/kustomization.yamlconfig/manager/manager.yamlconfig/manager/route.yamlconfig/manager/svc.yamlconfig/manifests/overlays/rhpreprod/kustomization.yamlconfig/manual-rbac/base/secret.yamlconfig/manual-rbac/base/serviceaccount.yamlconfig/manual-rbac/overlays/usernaut-rhplatformtest/admin-rolebinding.yamlconfig/manual-rbac/overlays/usernaut-rhpreprod/admin-rolebinding.yamlconfig/manual-rbac/overlays/usernaut-rhpreprod/kustomization.yamlconfig/manual-rbac/overlays/usernaut-rhprod/admin-rolebinding.yamlconfig/mpp-manual/tenant-namespace/base/kustomization.yamlconfig/mpp-manual/tenant-namespace/base/tenant-namespace.yamlconfig/mpp-manual/tenant-namespace/overlays/usernaut-rhplatformtest/kustomization.yamlconfig/mpp-manual/tenant-namespace/overlays/usernaut-rhplatformtest/tenant-egress.yamlconfig/mpp-manual/tenant-namespace/overlays/usernaut-rhpreprod/kustomization.yamlconfig/mpp-manual/tenant-namespace/overlays/usernaut-rhprod/kustomization.yamlconfig/rbac-admin-sa/base/rolebinding.yamlconfig/rbac-admin-sa/overlays/all/patches/usernaut-admin-role-patch.yamlconfig/redis/kustomization.yamlconfig/redis/redis.yamlconfig/redis/service.yamlconfig/samples/dataverse-platform-preprodusernaut.yamlscripts/configmap.sh
💤 Files with no reviewable changes (5)
- config/redis/service.yaml
- config/redis/kustomization.yaml
- config/redis/redis.yaml
- config/rbac-admin-sa/overlays/all/patches/usernaut-admin-role-patch.yaml
- config/default/base/kustomization.yaml
Author
|
@coderabbitai review |
|
obrienrobert
force-pushed
the
leader-election-fix
branch
2 times, most recently
from
August 7, 2026 22:39
76324fa to
18462d5
Compare
obrienrobert
changed the title
Author
|
@CodeRabbit review |
|
iambibhas
reviewed
Aug 14, 2026
Usernaut runs as a single replica with --leader-elect, but was using controller-runtime defaults (15s/10s/2s) designed for fast multi-replica failover. Brief API server stalls caused lease renewal to timeout, crashing the manager and triggering daily Gatus alerts. - Set LeaseDuration=60s, RenewDeadline=40s, RetryPeriod=5s - Add timeoutSeconds=5 to liveness/readiness probes Ref: DATA-7347 Signed-off-by: Robert O'Brien <robrien@redhat.com>
obrienrobert
force-pushed
the
leader-election-fix
branch
from
August 14, 2026 09:17
18462d5 to
18a297c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
📝 Description
What changed?
Leader election: LeaseDuration=60s, RenewDeadline=40s, RetryPeriod=5s (from defaults 15s/10s/2s)
Probe timeoutSeconds=5 (from default 1s)
Why is this change needed?
Usernaut restarts frequently due to leader election lost after brief API server latency spikes (context deadline exceeded on lease renewal). As a single-replica operator, the aggressive HA defaults provide no failover benefit - they just make the pod crash-prone. Correlated failures across sibling operators on the same node confirm the trigger is cluster-side, but Usernaut's intolerance is the reason it crashes.
Dependencies
🧪 Testing
Test Coverage
Performance Impact
🚀 Deployment
Deploy Steps
Prerequisites
Post-Deployment Monitoring
Rollback Plan
Details:
⚙️ Configuration Changes
✅ Developer Checklist