Skip to content

Conversation

@prsurve
Copy link
Contributor

@prsurve prsurve commented Oct 10, 2025

No description provided.

@prsurve prsurve requested a review from a team as a code owner October 10, 2025 18:46
@openshift-ci
Copy link

openshift-ci bot commented Oct 10, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: prsurve

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

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

@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines label Oct 10, 2025
@prsurve prsurve added the DR Metro and Regional DR related PRs label Oct 10, 2025
@prsurve prsurve requested a review from a team October 10, 2025 18:48
"""
if index < 0 or index >= self.nclusters:
raise ClusterNotFoundException(f"Cluster with index {index} not found")
return self.clusters[index].ENV_DATA.get("cluster_path") + "auth/kubeconfig"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return self.clusters[index].ENV_DATA.get("cluster_path") + "auth/kubeconfig"
return os.path.join(
self.clusters[index].ENV_DATA["cluster_path"], self.clusters[index].RUN.get("kubeconfig_location", "auth/kubeconfig")
)

if not cephbpradosns:
raise NotFoundError("Couldn't identify the cephblockpoolradosnamespace")

cephbpradosns = "ocs-storagecluster-cephblockpool-" + cephbpradosns
Copy link
Contributor

Choose a reason for hiding this comment

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

If cephbpradosns is obtained from ocnfigmap the namespace value will be without "ocs-storagecluster-cephblockpool-" and this change works.
If cephbpradosns is obtained from ENV_DATA.radosnamespace_name , we need to ensure that the value is actual namespace name, not the cephblockpoolradosnamespace CR name. No code change change needed for this comment.

for cluster in dr_helpers.get_all_drclusters()
if cluster != self.preferred_primary_cluster
][0]
if get_primary_cluster_config().ENV_DATA["cluster_type"] == "hci_client":
Copy link
Contributor

Choose a reason for hiding this comment

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

HYPERSHIFT_ADDON_DISCOVERYPREFIX is applicable for hosted cluster as clients only.
is_hosted_cluster from ocs_ci/deployment/helpers/hypershift_base.py can be used.

if cluster != self.preferred_primary_cluster
][0]
if get_primary_cluster_config().ENV_DATA["cluster_type"] == "hci_client":
self.preferred_primary_cluster_client = (
Copy link
Contributor

Choose a reason for hiding this comment

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

How about using self.preferred_primary_cluster itself with the updated name ?
self.preferred_primary_cluster = constants.HYPERSHIFT_ADDON_DISCOVERYPREFIX + "-" + self.preferred_primary_cluster

Since this is attribute of a workload related class, it is concerned about the name of the cluster in ACM hub context only.

# load drpc.yaml
drpc_yaml_data = templating.load_yaml(self.drpc_yaml_file)
drpc_yaml_data["spec"]["preferredCluster"] = self.preferred_primary_cluster
drpc_yaml_data["spec"]["preferredCluster"] = (
Copy link
Contributor

Choose a reason for hiding this comment

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

How about using self.preferred_primary_cluster itself with the updated name ?
self.preferred_primary_cluster = constants.HYPERSHIFT_ADDON_DISCOVERYPREFIX + "-" + self.preferred_primary_cluster

Since this is attribute of a workload related class, it is concerned about the name of the cluster in ACM hub context only.

"preferred_primary_cluster"
) or (get_primary_cluster_config().ENV_DATA["cluster_name"])
if get_primary_cluster_config().ENV_DATA["cluster_type"] == "hci_client":
self.preferred_primary_cluster_client = (
Copy link
Contributor

Choose a reason for hiding this comment

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

How about using self.preferred_primary_cluster itself with the updated name ?
self.preferred_primary_cluster = constants.HYPERSHIFT_ADDON_DISCOVERYPREFIX + "-" + self.preferred_primary_cluster

Since this is attribute of a workload related class, it is concerned about the name of the cluster in ACM hub context only.

self.preferred_primary_cluster = config.ENV_DATA.get(
"preferred_primary_cluster"
) or (get_primary_cluster_config().ENV_DATA["cluster_name"])
if get_primary_cluster_config().ENV_DATA["cluster_type"] == "hci_client":
Copy link
Contributor

Choose a reason for hiding this comment

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

HYPERSHIFT_ADDON_DISCOVERYPREFIX is applicable for hosted cluster as clients only.
is_hosted_cluster from ocs_ci/deployment/helpers/hypershift_base.py can be used.

self.preferred_primary_cluster = kwargs.get("preferred_primary_cluster") or (
get_primary_cluster_config().ENV_DATA["cluster_name"]
)
if get_primary_cluster_config().ENV_DATA["cluster_type"] == "hci_client":
Copy link
Contributor

Choose a reason for hiding this comment

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

the two comments given above is applicable here

"""
non_acm_cluster_and_non_provider_cluster_indexes = []
for cluster in get_non_acm_cluster_config():
if not cluster.ENV_DATA["cluster_type"] == "provider":
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use the function config.get_provider_cluster_indexes

@openshift-merge-robot
Copy link
Collaborator

PR needs rebase.

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.

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 needs-rebase size/L PR that changes 100-499 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants