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

Adding a new test case for reclaim space job and cronjob on rwop pvc #10096

Merged
merged 4 commits into from
Jul 16, 2024

Conversation

ypersky1980
Copy link
Contributor

This PR contains the following:

  1. Adding a new test case for creating reclaim space job and reclaim space cron job on RWOP PVC
  2. Two new functions in helpers.py for testing the desired job/cronjob status.

@ypersky1980 ypersky1980 requested review from a team as code owners July 14, 2024 18:51
@pull-request-size pull-request-size bot added the size/M PR that changes 30-99 lines label Jul 14, 2024
Signed-off-by: Yulia Persky <[email protected]>
Copy link

@ocs-ci ocs-ci left a comment

Choose a reason for hiding this comment

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

PR validation

Cluster Name:
Cluster Configuration:
PR Test Suite: tier1
PR Test Path: tests/functional/pv/pv_services/test_rwop_pvc.py
Additional Test Params:
OCP VERSION: 4.16
OCS VERSION: 4.16
tested against branch: master

Job UNSTABLE (some or all tests failed).

Comment on lines +5062 to +5123
def wait_for_reclaim_space_cronjob(reclaim_space_cron_job, schedule):
"""
Wait for reclaim space cronjbo

Args:
reclaim_space_cron_job (obj): The reclaim space cron job
schedule (str): Reclaim space cron job schedule

Raises:
UnexpectedBehaviour: In case reclaim space cron job doesn't reach the desired state
"""

try:
for reclaim_space_cron_job_yaml in TimeoutSampler(
timeout=120, sleep=5, func=reclaim_space_cron_job.get
):
result = reclaim_space_cron_job_yaml["spec"]["schedule"]
if result == f"@{schedule}":
logger.info(
f"ReclaimSpaceCronJob {reclaim_space_cron_job.name} succeeded"
)
break
else:
logger.info(
f"Waiting for the @{schedule} result of the ReclaimSpaceCronJob {reclaim_space_cron_job.name}. "
f"Present value of result is {result}"
)
except TimeoutExpiredError:
raise UnexpectedBehaviour(
f"ReclaimSpaceJob {reclaim_space_cron_job.name} is not successful. "
f"Yaml output: {reclaim_space_cron_job.get()}"
)


def wait_for_reclaim_space_job(reclaim_space_job):
"""
Wait for reclaim space cronjbo

Args:
reclaim_space_job (obj): The reclaim space job

Raises:
UnexpectedBehaviour: In case reclaim space job doesn't reach the Succeeded state
"""

try:
for reclaim_space_job_yaml in TimeoutSampler(
timeout=120, sleep=5, func=reclaim_space_job.get
):
result = reclaim_space_job_yaml.get("status", {}).get("result")
if result == "Succeeded":
logger.info(f"ReclaimSpaceJob {reclaim_space_job.name} succeeded")
break
else:
logger.info(
f"Waiting for the Succeeded result of the ReclaimSpaceJob {reclaim_space_job.name}. "
f"Present value of result is {result}"
)
except TimeoutExpiredError:
raise UnexpectedBehaviour(
f"ReclaimSpaceJob {reclaim_space_job.name} is not successful. Yaml output: {reclaim_space_job.get()}"
)
Copy link
Contributor

Choose a reason for hiding this comment

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

the 2 functions are almost identical. Can you club them to 1 function and call them with a parameter to distinguish between reclaim_space_cron_job_yaml and reclaim_space_job_yaml?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#10111 - a Github issue was opened for addressing your comment later.

Copy link
Contributor

Choose a reason for hiding this comment

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

these 2 methods look visually identical but the content is somewhat different, the same as a subject of "reclaim space" action, since it does not look good, and does not help in readability, I would not suggest merging them.

For reference, the one that is merged can look like:

def wait_for_reclaim_space_job(reclaim_space_job, job_type="cronjob", schedule=None):
    """
    Wait for reclaim space job to reach the desired state
    Args:
        reclaim_space_job (obj): The reclaim space job object
        job_type (str): Type of the job, either "cronjob" or "job"
        schedule (str, optional): Reclaim space cron job schedule if job_type is "cronjob"
    Raises:
        UnexpectedBehaviour: In case reclaim space job doesn't reach the desired state
    """

    try:
        for reclaim_space_job_yaml in TimeoutSampler(
            timeout=120, sleep=5, func=reclaim_space_job.get
        ):
            if job_type == "cronjob":
                if schedule is None:
                    raise ValueError("Schedule must be provided for cronjob type")
                result = reclaim_space_job_yaml["spec"]["schedule"]
                desired_state = f"@{schedule}"
                success_message = f"ReclaimSpaceCronJob {reclaim_space_job.name} succeeded"
            else:
                result = reclaim_space_job_yaml.get("status", {}).get("result")
                desired_state = "Succeeded"
                success_message = f"ReclaimSpaceJob {reclaim_space_job.name} succeeded"
            
            if result == desired_state:
                logger.info(success_message)
                break
            else:
                logger.info(
                    f"Waiting for the {desired_state} result of the {job_type} {reclaim_space_job.name}. "
                    f"Present value of result is {result}"
                )
    except TimeoutExpiredError:
        raise UnexpectedBehaviour(
            f"{job_type} {reclaim_space_job.name} is not successful. "
            f"Yaml output: {reclaim_space_job.get()}"
        ) 

@pull-request-size pull-request-size bot added size/L PR that changes 100-499 lines and removed size/M PR that changes 30-99 lines labels Jul 15, 2024
Signed-off-by: Yulia Persky <[email protected]>
ocs-ci

This comment was marked as outdated.

@ypersky1980
Copy link
Contributor Author

#10112 - Github issue for adding Polarion ID.

Copy link

@ocs-ci ocs-ci left a comment

Choose a reason for hiding this comment

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

PR validation

Cluster Name:
Cluster Configuration:
PR Test Suite: tier2
PR Test Path: tests/functional/pv/pv_services/test_rwop_pvc_reclaim_space.py
Additional Test Params:
OCP VERSION: 4.16
OCS VERSION: 4.16
tested against branch: master

Job PASSED.

@ypersky1980 ypersky1980 added the Verified Mark when PR was verified and log provided label Jul 15, 2024
Copy link

openshift-ci bot commented Jul 16, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: DanielOsypenko, ebenahar, jilju, ramkiperiy, ypersky1980

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

@ramkiperiy ramkiperiy merged commit 06a6627 into master Jul 16, 2024
6 of 7 checks passed
@ypersky1980 ypersky1980 deleted the ypersky_rwop_reclaimspace_job branch July 16, 2024 10:28
amr1ta pushed a commit to amr1ta/ocs-ci that referenced this pull request Jul 25, 2024
…ed-hat-storage#10096)

* adding new test case for reclaim space job and cronjob on rwop pvc
* adding rwop reclaim space as a separate file and only for rbd

Signed-off-by: Yulia Persky <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm size/L PR that changes 100-499 lines Verified Mark when PR was verified and log provided
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants