Skip to content

Simulator: Better VR Redundant Status Behaviour#3313

Merged
andrijapanicsb merged 2 commits intoapache:masterfrom
richardlawley:simulator-ha-routers
Jan 3, 2020
Merged

Simulator: Better VR Redundant Status Behaviour#3313
andrijapanicsb merged 2 commits intoapache:masterfrom
richardlawley:simulator-ha-routers

Conversation

@richardlawley
Copy link
Copy Markdown
Contributor

Description

Before this change, when creating a HA network in the Simulator, both Routers would be in BACKUP mode. Many years ago in f0c3b4c, logic was added to allow the line "router_pr=100" to be added to the bootargs field of mockvm in the Simulator database to allow a VR to become MASTER. This doesn't work automatically (nowhere sets it) and actually fails because the field is too short, and no other statuses can be simulated.

With this PR, Redundant State now comes from the "redundate_state=MASTER" part in bootargs, which is already populated.

Additionally, the Start/Reboot/Stop VM code of the simulator now incldues logic to change the redundant state:

  • When Starting a VR, if no other VRs are Running and MASTER on this network, become MASTER
  • When Stopping a VR, if we are MASTER become BACKUP. If another VR is running and BACKUP, make it MASTER.
  • When Rebooting a VR, combine this logic (Stop/Start)

This will allow integration tests to easily change the state of a VR.

With this PR, the simulator will change the redundant state of a VR when it is started or stopped. When a VR is started, if there are no other running VRs that are MASTER then

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Screenshots (if appropriate):

How Has This Been Tested?

  • Deploy Simulator with advanced config
  • Create & Enable HA Network Offering
  • Deploy Network and a VM, check Network > Virtual Appliances page and observe that 2 VRs are present, one MASTER and the other BACKUP
  • Stop MASTER VR. Observe that other VR becomes MASTER
  • Stop other VR. Both will internally become BACKUP, but this isn't displayed when VRs are stopped
  • Start original VR. Observe that it becomes MASTER
  • Start second VR. Observe that it becomes BACKUP
  • Reboot MASTER VR. Observe that the states swap.

@yadvr
Copy link
Copy Markdown
Member

yadvr commented May 27, 2019

Closed-reopend PR to rekick Travis
@blueorangutan package

@yadvr yadvr added this to the 4.13.0.0 milestone May 27, 2019
@blueorangutan
Copy link
Copy Markdown

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2798

@yadvr
Copy link
Copy Markdown
Member

yadvr commented May 31, 2019

Smoketests are not necessary as changes are purely in simulator, Travis tests should be enough to merge this.

@yadvr
Copy link
Copy Markdown
Member

yadvr commented Jun 25, 2019

Pinging for review - @anuragaw @shwstppr

@yadvr
Copy link
Copy Markdown
Member

yadvr commented Jun 25, 2019

Checked LGTM, no explicit smoketests needed as changes are in simulator code that is tested by Travis.

Copy link
Copy Markdown
Contributor

@shwstppr shwstppr left a comment

Choose a reason for hiding this comment

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

LGTM based on changes

@yadvr
Copy link
Copy Markdown
Member

yadvr commented Jun 25, 2019

@richardlawley can you look at the failing marvin tests in Travis and fix or advise if they are related to this PR:
https://travis-ci.org/apache/cloudstack/builds/537778478

Copy link
Copy Markdown
Contributor

@anuragaw anuragaw left a comment

Choose a reason for hiding this comment

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

LGTM
(based on code)

@yadvr yadvr closed this Jun 26, 2019
@yadvr yadvr reopened this Jun 26, 2019
@yadvr
Copy link
Copy Markdown
Member

yadvr commented Jun 26, 2019

Closed reopen the PR to rekick Travis tests. I believe few failures seen in the travis run were caused by PR changes, can you test/confirm @richardlawley ?

@yadvr
Copy link
Copy Markdown
Member

yadvr commented Jun 28, 2019

ping @richardlawley

@richardlawley
Copy link
Copy Markdown
Contributor Author

I will get to this, but my other PR is higher on my priority list as this doesn't offer any functionality for 4.13 - it was there so that I could add tests for the password reset only working for one VR issue.

@yadvr
Copy link
Copy Markdown
Member

yadvr commented Jun 28, 2019

@richardlawley Okay, I'll move this away from 4.13. If you manage to fix the issues before 4.13 RC1 ping me to change the milestone. Thanks.

@yadvr yadvr modified the milestones: 4.13.0.0, 4.14.0.0 Jun 28, 2019
@yadvr
Copy link
Copy Markdown
Member

yadvr commented Dec 7, 2019

I'll close+reopen the PR to rekick Travis and see if it passes with the simulator related changes

@yadvr yadvr closed this Dec 7, 2019
@yadvr yadvr reopened this Dec 7, 2019
@andrijapanicsb
Copy link
Copy Markdown
Contributor

ping @richardlawley can you do any needed changes/fixes if you need this in 4.14 please - or shall we move it to 4.15 milestone?

@DaanHoogland
Copy link
Copy Markdown
Contributor

@andrijapanicsb , this is just simulator code, no production code. I think we can merge this and forward fix if issues are revealed.

@andrijapanicsb
Copy link
Copy Markdown
Contributor

You know better @DaanHoogland - if you feel so, then please move forward with it. (the reason I asked is due to some Travis tests failing, per other guys words due to those changes?)

@DaanHoogland
Copy link
Copy Markdown
Contributor

ok, investigating quickly @andrijapanicsb

@DaanHoogland
Copy link
Copy Markdown
Contributor

All jobs failed due to log length, try insanity to see if that helps.

@andrijapanicsb
Copy link
Copy Markdown
Contributor

OK, merging based on 3 x LGTM

@yadvr
Copy link
Copy Markdown
Member

yadvr commented Jan 7, 2020

@richardlawley can you have a look at it again and re-submit a new PR. It seem this PR breaks Travis/simulator for some of the tests you can see https://travis-ci.org/apache/cloudstack/builds/622114507?utm_source=github_status&utm_medium=notification
cc @andrijapanicsb @DaanHoogland

andrijapanicsb pushed a commit that referenced this pull request Jan 8, 2020
ustcweizhou pushed a commit to ustcweizhou/cloudstack that referenced this pull request Feb 28, 2020
* Make VRs change redundant status in simulator with start/stop/reboot

* Prevent getMockRouters returning null elements
ustcweizhou pushed a commit to ustcweizhou/cloudstack that referenced this pull request Feb 28, 2020
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.

8 participants