Skip to content
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

CNV workload with Discovered Apps #10616

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

prsurve
Copy link
Contributor

@prsurve prsurve commented Oct 4, 2024

No description provided.

@prsurve prsurve added DR Metro and Regional DR related PRs Squad/Turquoise labels Oct 4, 2024
@prsurve prsurve requested a review from a team October 4, 2024 08:06
@prsurve prsurve requested a review from a team as a code owner October 4, 2024 08:06
Copy link

openshift-ci bot commented Oct 4, 2024

[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

@prsurve prsurve added the Verified Mark when PR was verified and log provided label Oct 7, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

rdr marker needs to be added

Signed-off-by: prsurve <prsurve@redhat.com>
Signed-off-by: prsurve <prsurve@redhat.com>
Signed-off-by: prsurve <prsurve@redhat.com>
Signed-off-by: prsurve <prsurve@redhat.com>
Signed-off-by: prsurve <prsurve@redhat.com>
Signed-off-by: prsurve <prsurve@redhat.com>
skip_replication_resources=skip_replication_resources,
)

def delete_workload(self, force=False):
Copy link
Contributor

Choose a reason for hiding this comment

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

force parameter is not being used anywhere. Needs to be removed


class CnvWorkloadDiscoveredApps(DRWorkload):
"""
Class handling everything related to CNV workloads covers Discovered Apps`1
Copy link
Contributor

Choose a reason for hiding this comment

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

is '`1' intentional?

logger = logging.getLogger(__name__)


@rdr
Copy link
Contributor

Choose a reason for hiding this comment

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

polarion id missing



@rdr
@acceptance
Copy link
Contributor

Choose a reason for hiding this comment

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

A few questions about acceptance marker usage here:

  1. Is this in the context of including RDR in the build acceptance pipeline?
  2. Do we consider this scenario the basic functionality of RDR? or basic RDR+CNV?
  3. The criteria of the acceptance suite are tests that if fail we do not accept the build. Isi it the case here?
  4. Is the test stable enough to degree that we are sure it's a product bug every time it fails?

def deploy_workload(self):
"""
Deployment specific to busybox workload for Discovered/Imperative Apps
Copy link
Contributor

Choose a reason for hiding this comment

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

Why busybox?


def create_namespace(self):
"""
Create Namespace for Workload's to run
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
Create Namespace for Workload's to run
Create namespace for workloads to run

def factory(pvc_vm=1):
"""
Args:
kubeobject (int): Number if Discovered Apps workload with kube object protection to be created
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
kubeobject (int): Number if Discovered Apps workload with kube object protection to be created
kubeobject (int): Number of Discovered Apps workload with kube object protection to be created

kubeobject (int): Number if Discovered Apps workload with kube object protection to be created
Raises:
ResourceNotDeleted: In case workload resources not deleted properly
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
ResourceNotDeleted: In case workload resources not deleted properly
ResourceNotDeletedException: In case workload resources are not deleted

"""
Tests to verify cnv application failover and Relocate with Discovered Apps
There are two test cases:
1) Failover to secondary cluster when primary cluster is UP
Copy link
Contributor

Choose a reason for hiding this comment

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

With failover, we should test node down scenario for happy path

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/XL Squad/Turquoise Verified Mark when PR was verified and log provided
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants