Assuming ceph version is >= 19 if not determined for ODF 4.18 and above#15149
Conversation
Signed-off-by: Petr Balogh <pbalogh@redhat.com>
|
I have verified as part of Cherry-pick of Shivam's PR in release-4.21 here: #15148 |
📝 WalkthroughWalkthroughModified ChangesCephadm CA fetch behavior
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dahorak, petr-balogh, vavuthu The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@ocs_ci/deployment/helpers/external_cluster_helpers.py`:
- Around line 1211-1213: The current branch returns True for v is None
unconditionally; instead, when v is None, only return True if the detected ODF
release is 4.18 or newer. Change the `if v is None:` branch to call
`version.get_semantic_version("4.18", only_major_minor=True)` (or your existing
ODF version lookup) and return the result of comparing that ODF semver against
4.18 (i.e., return odf_semver is not None and odf_semver >=
version.get_semantic_version("4.18", only_major_minor=True)); otherwise fall
through to the existing `return v >= version.get_semantic_version("19.0",
only_major_minor=True)` logic. This ties the unknown-version fallback to ODF >=
4.18 rather than applying it unconditionally.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e6333016-017b-4883-90af-3e299d467be9
📒 Files selected for processing (1)
ocs_ci/deployment/helpers/external_cluster_helpers.py
| if v is None: | ||
| return False | ||
| return True | ||
| return v >= version.get_semantic_version("19.0", only_major_minor=True) |
There was a problem hiding this comment.
Gate unknown-version fallback to ODF 4.18+ only.
At Line 1212, None now maps to True unconditionally, which applies the Ceph >=19 path for all ODF versions. The PR objective/docstring scopes this assumption to ODF 4.18 and newer.
🔧 Proposed fix
def external_rgw_ca_should_use_cephadm_fetch():
@@
v = _external_ceph_semantic_version_or_none()
if v is None:
- return True
+ odf_version = version.get_semantic_ocs_version_from_config()
+ return odf_version >= version.VERSION_4_18
return v >= version.get_semantic_version("19.0", only_major_minor=True)🤖 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 `@ocs_ci/deployment/helpers/external_cluster_helpers.py` around lines 1211 -
1213, The current branch returns True for v is None unconditionally; instead,
when v is None, only return True if the detected ODF release is 4.18 or newer.
Change the `if v is None:` branch to call `version.get_semantic_version("4.18",
only_major_minor=True)` (or your existing ODF version lookup) and return the
result of comparing that ODF semver against 4.18 (i.e., return odf_semver is not
None and odf_semver >= version.get_semantic_version("4.18",
only_major_minor=True)); otherwise fall through to the existing `return v >=
version.get_semantic_version("19.0", only_major_minor=True)` logic. This ties
the unknown-version fallback to ODF >= 4.18 rather than applying it
unconditionally.
Summary by CodeRabbit