Fix: Retry project wait_for_status due to RBAC updates
#2589
Conversation
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Meni Yakove <441263+myakove@users.noreply.github.com>
WalkthroughRemove GitHub Actions workflow file and introduce configurable exception retry handling in OpenShift resource status polling to handle transient 403 Forbidden errors during RBAC propagation delays. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ocp_resources/resource.py (1)
975-975: Consider expanding the parameter documentation.While the parameter is documented, consider adding a note about when custom exception handling might be needed (e.g., "Use custom exceptions_dict when additional transient errors are expected, such as ForbiddenError during RBAC propagation").
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/code-check.yml.old(0 hunks)ocp_resources/project_request.py(2 hunks)ocp_resources/resource.py(3 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/code-check.yml.old
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-11T16:42:27.596Z
Learnt from: servolkov
Repo: RedHatQE/openshift-python-wrapper PR: 2490
File: ocp_resources/route_advertisements.py:5-5
Timestamp: 2025-08-11T16:42:27.596Z
Learning: In the openshift-python-wrapper project, MissingRequiredArgumentError should be imported from ocp_resources.resource (not ocp_resources.exceptions), as it's re-exported there and this is the established pattern used throughout the codebase.
Applied to files:
ocp_resources/project_request.py
🧬 Code graph analysis (1)
ocp_resources/project_request.py (1)
ocp_resources/resource.py (2)
wait_for_status(959-1009)status(1073-1083)
🔇 Additional comments (4)
ocp_resources/resource.py (1)
959-967: LGTM! Clean parameterization of retry exceptions.The addition of the
exceptions_dictparameter towait_for_statusprovides the flexibility needed to handle RBAC propagation delays while maintaining backward compatibility through sensible defaults. The default value preserves the existing retry behavior for callers that don't specify custom exceptions.ocp_resources/project_request.py (3)
5-5: Import location is correct.The
ForbiddenErrorexception is correctly imported fromkubernetes.dynamic.exceptions, which is the proper source for Kubernetes API exceptions.
61-67: Well-implemented fix for RBAC propagation delays.The changes correctly address the transient 403 Forbidden errors that occur during RBAC propagation after ProjectRequest creation. The implementation is appropriately scoped to only this specific scenario where the delay is expected, rather than globally masking permission errors.
The use of
{ForbiddenError: []}with an empty list means all ForbiddenError instances will be retried regardless of error message, which is appropriate since RBAC propagation delays can manifest with various error messages.
64-67: Thank you for clarifying the question. I'll search for specific RBAC propagation timing data in OpenShift to verify if 4 minutes is adequate.Review comment cannot be verified against published standards; recommend confirming timeout empirically in your environment.
The original review asks whether 4 minutes is sufficient for RBAC propagation. However, public OpenShift and Kubernetes documentation do not specify typical RBAC propagation delays. While the 4-minute timeout is conservative relative to typical cache sync defaults (e.g., 2 minutes in controller-runtime), confirming adequacy requires testing against your specific cluster infrastructure, network conditions, and load patterns. Consider validating this timeout by monitoring actual propagation times during project creation workflows in your environment, particularly under peak load.
Short description:
When a ProjectRequest is created, the project is created and the user is added to the project.
RBAC binding may not have propagated yet, causing transient 403 Forbidden errors, so we need to retry on ForbiddenError as well.
More details:
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for reviewer:
Bug:
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.