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.

Sign up for GitHub

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

Jump to bottom

changed multi_cnv_workload fixture to create parallel VMs #11131

Merged
merged 5 commits into from
Jan 20, 2025
Merged

changed multi_cnv_workload fixture to create parallel VMs #11131

merged 5 commits into from
Jan 20, 2025

Conversation

avd-sagare
Copy link
Contributor

No description provided.

@avd-sagare avd-sagare requested a review from a team as a code owner January 8, 2025 13:15
@pull-request-size pull-request-size bot added the size/M PR that changes 30-99 lines label Jan 8, 2025
@pull-request-size pull-request-size bot added size/S PR that changes 10-29 lines and removed size/M PR that changes 30-99 lines labels Jan 14, 2025
@pull-request-size pull-request-size bot added size/M PR that changes 30-99 lines and removed size/S PR that changes 10-29 lines labels Jan 15, 2025
@avd-sagare avd-sagare mentioned this pull request Jan 16, 2025
Open
ocs-ci
ocs-ci reviewed Jan 16, 2025
View reviewed changes
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 on existing cluster

Cluster Name: asagarte-cnv15
Cluster Configuration:
PR Test Suite:
PR Test Path: tests/functional/workloads/cnv/test_multi_vm_configurations.py
Additional Test Params:
OCP VERSION: 4.18
OCS VERSION: 4.18
tested against branch: master

Job FAILED (installation failed, tests not executed).

ocs-ci
ocs-ci reviewed Jan 16, 2025
View reviewed changes
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 on existing cluster

Cluster Name: asagare-cnv15
Cluster Configuration:
PR Test Suite:
PR Test Path: tests/functional/workloads/cnv/test_multi_vm_configurations.py
Additional Test Params:
OCP VERSION: 4.18
OCS VERSION: 4.18
tested against branch: master

Job UNSTABLE (some or all tests failed).

ocs-ci
ocs-ci reviewed Jan 16, 2025
View reviewed changes
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 on existing cluster

Cluster Name: asagare-sc
Cluster Configuration:
PR Test Suite:
PR Test Path: tests/functional/workloads/cnv/test_multi_vm_configurations.py
Additional Test Params:
OCP VERSION: 4.18
OCS VERSION: 4.18
tested against branch: master

Job PASSED.

ocs-ci
ocs-ci reviewed Jan 16, 2025
View reviewed changes
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 on existing cluster

Cluster Name: asagare-cnv15
Cluster Configuration:
PR Test Suite:
PR Test Path: tests/functional/workloads/cnv/test_multi_vm_configurations.py
Additional Test Params:
OCP VERSION: 4.18
OCS VERSION: 4.18
tested against branch: master

Job UNSTABLE (some or all tests failed).

@avd-sagare avd-sagare requested a review from a team as a code owner January 17, 2025 03:37
ocs-ci
ocs-ci reviewed Jan 17, 2025
View reviewed changes
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 on existing cluster

Cluster Name: asagare-cnv16
Cluster Configuration:
PR Test Suite:
PR Test Path: tests/functional/workloads/cnv/test_multi_vm_configurations.py
Additional Test Params:
OCP VERSION: 4.18
OCS VERSION: 4.18
tested against branch: master

Job PASSED.

PrasadDesala
PrasadDesala reviewed Jan 17, 2025
View reviewed changes
tests/conftest.py Outdated
Comment on lines 7321 to 7324
if vm_config["sc_compression"] == "default":
storageclass = sc_obj_def_compr.name
elif vm_config["sc_compression"] == "aggressive":
storageclass = sc_obj_aggressive.name
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
if vm_config["sc_compression"] == "default":
storageclass = sc_obj_def_compr.name
elif vm_config["sc_compression"] == "aggressive":
storageclass = sc_obj_aggressive.name
storageclass = sc_obj_def_compr.name if vm_config["sc_compression"] == "default" else sc_obj_aggressive.name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

tests/conftest.py Outdated
else:
vm_list_default_compr.append(vm_obj)
except Exception as e:
print(f"Error occurred while creating VM: {e}")
Copy link
Contributor

Choose a reason for hiding this comment

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

replace print with logging

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

tests/conftest.py Outdated
)

futures.append((future, vm_config["sc_compression"]))
for future, sc_compression in futures:
Copy link
Contributor

Choose a reason for hiding this comment

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

you can use as_completed(futures) to process each completed future as soon as it finishes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

tests/conftest.py Outdated
vm_list_default_compr.append(vm_obj)
# Use ThreadPoolExecutor to create VMs parallel
with ThreadPoolExecutor() as executor:
futures = []
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you are storing some metadata, should dict be used here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

ocs-ci
ocs-ci reviewed Jan 18, 2025
View reviewed changes
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 on existing cluster

Cluster Name: asagare-cnv17
Cluster Configuration:
PR Test Suite:
PR Test Path: tests/functional/workloads/cnv/test_multi_vm_configurations.py
Additional Test Params:
OCP VERSION: 4.18
OCS VERSION: 4.18
tested against branch: master

Job PASSED.

@avd-sagare
Incorporated comments
52292af 
Signed-off-by: Avdhoot <[email protected]>
@openshift-ci openshift-ci bot added the lgtm label Jan 20, 2025
@avd-sagare avd-sagare added team/e2e E2E team related issues/PRs Verified Mark when PR was verified and log provided labels Jan 20, 2025
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Jan 20, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: avd-sagare, ayush-patni, hnallurv, PrasadDesala

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

ocs-ci
ocs-ci reviewed Jan 20, 2025
View reviewed changes
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 on existing cluster

Cluster Name: asagare-cnv17
Cluster Configuration:
PR Test Suite:
PR Test Path: tests/functional/workloads/cnv/test_multi_vm_configurations.py
Additional Test Params:
OCP VERSION: 4.18
OCS VERSION: 4.18
tested against branch: master

Job PASSED.

@PrasadDesala PrasadDesala merged commit ef6c884 into red-hat-storage:master Jan 20, 2025
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ocs-ci ocs-ci ocs-ci left review comments

@ayush-patni ayush-patni ayush-patni approved these changes

@hnallurv hnallurv hnallurv approved these changes

@PrasadDesala PrasadDesala PrasadDesala approved these changes

@parikshithb parikshithb Awaiting requested review from parikshithb

Assignees

@PrasadDesala PrasadDesala

@hnallurv hnallurv

@ayush-patni ayush-patni

Labels
lgtm size/M PR that changes 30-99 lines team/e2e E2E team related issues/PRs Verified Mark when PR was verified and log provided
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@avd-sagare @PrasadDesala @hnallurv @ocs-ci @ayush-patni @openshift-merge-robot