-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
CLOUDSTACK-9055: fix NPE in updating Redundant State of VPC networks #1073
Conversation
code review done: LGTM |
Hi @ustcweizhou, Okay, I understood. Have you prepared a test to cover it? I mean, we need a test that deploys a DC, creates a redundant VPC, adds 1 tier, deploys 1 VM, destroys/stop/disconnect the host then when the router check runs we don't get a NPE. In case writing an integration test for that is too much, perhaps would be nice to write some unit tests. Do you have a test environment? If so, could you please run the following tests against KVM?
Cheers, |
LGTM, based on a set of tests that I run on this branch (which I rebased myself first):
Result:
And:
Result:
These test may not cover your change, all they do is show you didn't break them. Someone else needs to review the code. Please respond to the comments by @wilderrodrigues. |
@wilderrodrigues @remibergsma @DaanHoogland |
let's get this merged. I need no intergration tests for this improvement. a disect of |
CLOUDSTACK-9055: fix NPE in updating Redundant State of VPC networksThis issue happened when the KVM nodes is down.It might also happen when the cloudstack-agent is killed unexpectedly. * pr/1073: CLOUDSTACK-9055: fix NPE in updating Redundant State of VPC networks Signed-off-by: Remi Bergsma <github@remi.nl>
today one of my nodes was rebooted unexpectedly, I got the same error 2015-12-02 09:59:53,224 ERROR c.c.n.r.VirtualNetworkApplianceManagerImpl Fail to complete the RvRStatusUpdateTask! |
This issue happened when the KVM nodes is down.It might also happen when the cloudstack-agent is killed unexpectedly.