Skip to content

Fix DR Status issue for discovered apps in Virtual Machine page #2128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

TimothyAsirJeyasing
Copy link
Contributor

@TimothyAsirJeyasing TimothyAsirJeyasing commented Jun 20, 2025

https://issues.redhat.com/browse/DFBUGS-2822
https://issues.redhat.com/browse/DFBUGS-2823

This PR addresses issues related to the above BZs for deploying the CNV workload and protecting it via the Discovered application

@TimothyAsirJeyasing
Copy link
Contributor Author

TimothyAsirJeyasing commented Jun 20, 2025

Screenshot 2025-06-20 at 4 42 00 PM Screenshot 2025-06-20 at 4 43 19 PM Screenshot 2025-06-20 at 4 45 15 PM

@vbnrh
Copy link
Member

vbnrh commented Jun 24, 2025

This needs more UX discussion.
VMs that are protected at namespace level still do not show any DRPC status.

We need to discuss whether it is required as a separate UX to indicate that it protected at ns level

@vbnrh
Copy link
Member

vbnrh commented Jun 24, 2025

Are the screenshots latest ? I can see that the second VM in the first shot is still missing its status

@TimothyAsirJeyasing
Copy link
Contributor Author

Are the screenshots latest ? I can see that the second VM in the first shot is still missing its status

It was created through the ApplicationSet UI, and no protection was applied. I also agree that there may be issues with VM applications created via the UI. We can review and address those separately.

@SanjalKatiyar
Copy link
Collaborator

I think we hit this bug when we DR protect the VM via Data Services --> Disaster recovery --> Protected applications and then check it's status on the Virtual machines page under Infrastructure. 
But if the same VM is protected from the Virtual machines page under Infrastructure, DR status is reflected.
So, we need to figure out what's different in the DR protection flow between these pages?

@TimothyAsirJeyasing what's the reason for this ?? what's the difference ??

@SanjalKatiyar
Copy link
Collaborator

SanjalKatiyar commented Jun 24, 2025

Do we need to do this: https://github.com/red-hat-storage/odf-console/blob/master/packages/mco/components/modals/app-manage-policies/utils/k8s-utils.ts#L474-L476 anymore ?? If not, @TimothyAsirJeyasing plz do the proper/complete cleanup (from constants file as well).

@TimothyAsirJeyasing
Copy link
Contributor Author

what's the reason for this ?? what's the difference ??

When a VM is protected via the Virtual Machines page, it typically gets labeled (e.g., kubevirt.io/instance) and is linked to an ApplicationSet or Subscription, which the parser uses to resolve DR status.

In contrast, when protection is applied via the Protected Applications page, the VM is included in a namespace-level DRPC without necessarily having those labels or application associations.

1st case) : vm created thru applicationset-push/pull and namespace protected using discovered ns
have KUBE_INSTANCE_LABEL which leads to applicationSet then into wrong helper which is applicationsetHelper instead of discoverdHelper
2nd case) : vm application created thru cnv (outside application ui) and protected using discovered namespace
it does't have the KUBE_INSTANCE_LABEL and failed to get details
3rd case)
the vm application protected thru virtualmachine page
has all the labels and shows the drstatus

I think we hit this bug when we DR protect the VM via Data Services --> Disaster recovery --> Protected applications and then check it's status on the Virtual machines page under Infrastructure. 
But if the same VM is protected from the Virtual machines page under Infrastructure, DR status is reflected.
So, we need to figure out what's different in the DR protection flow between these pages?

@TimothyAsirJeyasing what's the reason for this ?? what's the difference ??

@TimothyAsirJeyasing
Copy link
Contributor Author

Previously, the parser determined whether a VM was managed by an ApplicationSet or Subscription first, and only if neither was found, it treated the application as a "Discovered" application.

This led to incorrect behavior when a VM was created via an ApplicationSet but was already protected using a DRPolicy (namespace-based). In such cases, the parser would wrongly try to fetch the app as a GitOps-managed application.

What’s changed and why:
Now, the logic first checks if the VM’s namespace is protected by any DRPlacementControl (DRPC) matching the preferred cluster.

If a matching DRPC is found, the parser immediately renders the DiscoveredParser, as this confirms the VM is already part of a DR-protected (discovered) application.

Only if no DRPC is found, it falls back to checking if the VM belongs to a managed ApplicationSet or Subscription.

@TimothyAsirJeyasing
Copy link
Contributor Author

Screenshot 2025-06-26 at 5 33 58 PM

@TimothyAsirJeyasing
Copy link
Contributor Author

/test odf-console-e2e-aws

1 similar comment
@TimothyAsirJeyasing
Copy link
Contributor Author

/test odf-console-e2e-aws

@alfonsomthd
Copy link
Collaborator

LGTM @SanjalKatiyar to tag.

@vbnrh
Copy link
Member

vbnrh commented Jun 27, 2025

/lgtm

@TimothyAsirJeyasing
Copy link
Contributor Author

/test odf-console-e2e-aws

@SanjalKatiyar
Copy link
Collaborator

/override ci/prow/odf-console-e2e-aws

Copy link
Contributor

openshift-ci bot commented Jun 27, 2025

@SanjalKatiyar: Overrode contexts on behalf of SanjalKatiyar: ci/prow/odf-console-e2e-aws

In response to this:

/override ci/prow/odf-console-e2e-aws

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.

@SanjalKatiyar
Copy link
Collaborator

/approve

Copy link
Contributor

openshift-ci bot commented Jun 27, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SanjalKatiyar, TimothyAsirJeyasing, vbnrh

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

The pull request process is described 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

@openshift-merge-bot openshift-merge-bot bot merged commit 91ec798 into red-hat-storage:master Jun 27, 2025
5 checks passed
@TimothyAsirJeyasing
Copy link
Contributor Author

/cherry-pick release-4.19

@openshift-cherrypick-robot

@TimothyAsirJeyasing: new pull request created: #2135

In response to this:

/cherry-pick release-4.19

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants