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

Added post validation steps for NS loadbalancing scenarios #4193

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

HaruChebrolu
Copy link
Contributor

@HaruChebrolu HaruChebrolu commented Oct 28, 2024

Description

Added post validation steps for NS loadbalancing scenarios.
Logs: http://magna002.ceph.redhat.com/cephci-jenkins/harika/ns_loadbalancing/cephci-run-NLZQ8M/

click to expand checklist
  • Create a test case in Polarion reviewed and approved.
  • Create a design/automation approach doc. Optional for tests with similar tests already automated.
  • Review the automation design
  • Implement the test script and perform test runs
  • Submit PR for code review and approve
  • Update Polarion Test with Automation script details and update automation fields
  • If automation is part of Close loop, update BZ flag qe-test_coverage “+” and link Polarion test

Copy link
Contributor

@rahullepakshi rahullepakshi left a comment

Choose a reason for hiding this comment

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

gw info logs too many times, if possible lets try to avoid duplication and remove it if its part of GW initialization


# Add listeners and namespaces to newly added GWs
for subsys_args in config["subsystems"]:
LOG.info(f"Adding listeners for {scaleup_nodes}")
Copy link
Contributor

Choose a reason for hiding this comment

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

@HaruChebrolu It is better to add all listeners first and then move to new namespace addition in interest of time and IO getting stuck . Though they are new GWs, it is always advised to add all listeners before adding namespaces

Copy link
Contributor

Choose a reason for hiding this comment

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

The order below would be more beneficial for scale up new Gws

  1. Run IO on old namespaces
  2. ceph orch apply new GW nodes
  3. Add listeners on all subsystems for alll new GW nodes immediately
  4. Then add namespaces
  5. Disconnect, discover and connect again from initiators

Currently it is 2,1,4-3,5

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will update the loop to add listeners to all subsystems and then move to adding namespaces.
And currently I running IO on old namespaces and then I am applying new GW nodes, please let me know if I am missing something.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @HaruChebrolu
I got confused with

ha.scale_up(scaleup_nodes, gw_nodes, old_namespaces)
# Validate IO for old namespaces
ha.validate_scaleup(scaleup_nodes, old_namespaces)

i think you are validating IO for old namespaces after orch apply new nodes? If its case then its fine. thanks
Another suggestionis you can validate IO for old namespaces as well as new namespaces together after listener add and namespace add. We can do this way too

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

@HaruChebrolu
Copy link
Contributor Author

gw info logs too many times, if possible lets try to avoid duplication and remove it if its part of GW initialization

I will try to mitigate as much as I can, but I am little skeptical if I can completely avoid it.

@openshift-ci openshift-ci bot added the lgtm Add this label when the PR is good to be merged label Oct 30, 2024
Copy link
Contributor

@Manohar-Murthy Manohar-Murthy left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

openshift-ci bot commented Oct 30, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: HaruChebrolu, Manohar-Murthy, rahullepakshi

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

@mergify mergify bot merged commit 1a1085f into master Oct 30, 2024
7 of 8 checks passed
@mergify mergify bot deleted the loadbalancing_postfixes branch October 30, 2024 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.0 Squid Automation lgtm Add this label when the PR is good to be merged nvmeof
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants