Skip to content

Update finding radosnamespace and cephblockpoolradosnamespace#15145

Open
jilju wants to merge 4 commits into
red-hat-storage:masterfrom
jilju:cephblockpoolradosnamespace_and_radosnamespace_on_ec_cluster
Open

Update finding radosnamespace and cephblockpoolradosnamespace#15145
jilju wants to merge 4 commits into
red-hat-storage:masterfrom
jilju:cephblockpoolradosnamespace_and_radosnamespace_on_ec_cluster

Conversation

@jilju
Copy link
Copy Markdown
Contributor

@jilju jilju commented May 13, 2026

Update the functions that find cephblockpoolradosnamespace CR and radosnamespace based on the new naming format in erasure coded cluster.
This change fixes #15126 as well

Summary by CodeRabbit

  • Bug Fixes
    • Improved storage namespace resolution in distributed storage setups
    • Enhanced disaster recovery verification for mirroring status checks
    • Strengthened storage consumer configuration handling in provider/consumer scenarios

Review Change Stack

@jilju jilju requested a review from a team as a code owner May 13, 2026 20:27
@pull-request-size pull-request-size Bot added the size/M PR that changes 30-99 lines label May 13, 2026
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 13, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jilju

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jilju jilju added DR Metro and Regional DR related PRs Squad/Turquoise labels May 13, 2026
@jilju
Copy link
Copy Markdown
Contributor Author

jilju commented May 14, 2026

@jilju jilju added the Verified Mark when PR was verified and log provided label May 14, 2026
jilju added 3 commits May 14, 2026 14:41
Signed-off-by: Jilju Joy <jijoy@redhat.com>
Signed-off-by: Jilju Joy <jijoy@redhat.com>
Signed-off-by: Jilju Joy <jijoy@redhat.com>
@jilju jilju force-pushed the cephblockpoolradosnamespace_and_radosnamespace_on_ec_cluster branch from 3e66e17 to cc3155c Compare May 14, 2026 09:11
Signed-off-by: Jilju Joy <jijoy@redhat.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 14, 2026

📝 Walkthrough

Walkthrough

This PR refactors CephBlockPoolRadosNamespace (RNS) resolution logic in DR and helper utilities to dynamically discover RNS names from Kubernetes CRs instead of relying on hardcoded naming patterns. A new find_radosnamespace function wraps the improved discovery logic, and DR helper functions now use dynamic lookups to resolve RNS identifiers in provider/consumer mirroring and backend volume deletion scenarios.

Changes

Radosnamespace Discovery and Resolution

Layer / File(s) Summary
Core RNS resolution helpers
ocs_ci/helpers/helpers.py
Refactored find_cephblockpoolradosnamespace switches into provider config context and enumerates CephBlockPoolRadosNamespace CRs to select the correct RNS based on consumer type. New find_radosnamespace(storageclient_uid=None) returns the derived RADOS namespace name from the resolved RNS CR's spec.name, unless the RNS is "-builtin-implicit".
DR helpers migration to dynamic discovery
ocs_ci/helpers/dr_helpers.py
check_mirroring_status_ok HCI and non-HCI branches replace hardcoded RNS names with dynamic discovery queries over CephBlockPoolRadosNamespace objects. verify_backend_volume_deletion switches from find_cephblockpoolradosnamespace to find_radosnamespace for ceph-radosnamespace resolution. Import list updated to include find_radosnamespace.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A namespace so clever, no longer astray,
Dynamic discovery lights up the way,
No hardcoded names to lead us awry,
RNS resolves true—radosnamespaces fly! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly reflects the main changes: updating functions for finding radosnamespace and cephblockpoolradosnamespace to handle new naming formats in erasure-coded clusters.
Linked Issues check ✅ Passed The code changes address all key requirements from issue #15126: properly resolving CephBlockPoolRadosNamespace names by using dynamic lookup instead of hardcoding, adding the find_radosnamespace helper, and handling provider/consumer context correctly.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the cephblockpoolradosnamespace and radosnamespace resolution logic as specified in the linked issue; no unrelated modifications detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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/helpers/dr_helpers.py`:
- Around line 424-437: The discovery of CephBlockPoolRadosNamespace objects must
run against the provider cluster; wrap the list/get call that builds
cephblockpool_rns_names (the ocp.OCP(kind=constants.CEPHBLOCKPOOLRADOSNS,
namespace=config.ENV_DATA["cluster_namespace"]).get()) and the subsequent
filter/selection of cephbpradosns inside
config.RunWithConfigContext(config.get_provider_index()) so both the list and
the computed cephbpradosns resolve against the provider context (ensure the same
context manager encloses the creation of cephblockpool_rns_names and the
filtering that sets cephbpradosns).
- Around line 428-437: The code builds cephblockpool_rns_names and then does
list(filter(...))[0], which raises IndexError when there is no match; change the
logic in the cephblockpool_rns_names/cephbpradosns handling to capture the
filtered result into a variable, check if it's empty, and if so raise the
expected NotFoundError (or return/handle accordingly) with a clear message
including the search pattern and namespace; apply the same empty-check-and-raise
fix to the similar occurrence around the cephblockpool_rns_names usage at the
later block (the second filter that currently indexes [0]).

In `@ocs_ci/helpers/helpers.py`:
- Around line 6965-6986: The current logic in find_radosnamespace() relies on
substring filters over cephblockpool_rns_names and then indexing [0], which can
IndexError when metadata names don't include -{storageconsumer}; instead, load
the resourceNameMappingConfigMap (if present) to map storageconsumer ->
radosnamespace metadata name, otherwise iterate the CEPHBLOCKPOOLRADOSNS items
(from ocp.OCP(...).get()["items"]) and match either item["spec"]["name"] or
config.ENV_DATA.get("radosnamespace_name") against item["spec"]["name"] (or
compare the mapping value to item["metadata"]["name"]) to find the correct CR
without using substring matching; if no CR is found, raise a clear, controlled
exception indicating the missing radosnamespace for the given storageconsumer.
🪄 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: 5d8faf15-d90d-4021-8cf1-886f6df69ffb

📥 Commits

Reviewing files that changed from the base of the PR and between 51b29d0 and c69efaa.

📒 Files selected for processing (3)
  • .secrets.baseline
  • ocs_ci/helpers/dr_helpers.py
  • ocs_ci/helpers/helpers.py

Comment thread ocs_ci/helpers/dr_helpers.py
Comment thread ocs_ci/helpers/dr_helpers.py
Comment thread ocs_ci/helpers/helpers.py
@jilju
Copy link
Copy Markdown
Contributor Author

jilju commented May 15, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DR Metro and Regional DR related PRs size/M PR that changes 30-99 lines Squad/Turquoise Verified Mark when PR was verified and log provided

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Invalid cephblockpoolradosnamespace prefix added when radosnamespace_name is passed in ENV_DATA

1 participant