You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by remibergsma <gi...@git.apache.org> on 2015/08/07 14:07:54 UTC
[GitHub] cloudstack pull request: CLOUDSTACK-8709 No out of band migrate al...
GitHub user remibergsma opened a pull request:
https://github.com/apache/cloudstack/pull/666
CLOUDSTACK-8709 No out of band migrate alert for non-routers
This prevents the out-of-band migrate alerts being generated for non-routers.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/remibergsma/cloudstack fix-CLOUDSTACK-8709
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/666.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #666
----
commit 836fe6a0f55c0e52cb6c972f2c437800cd150d24
Author: Remi Bergsma <gi...@remi.nl>
Date: 2015-08-07T11:55:36Z
CLOUDSTACK-8709 No out of band migrate alert for non-routers
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8709 No out of band migrate al...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/666#discussion_r36527589
--- Diff: server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java ---
@@ -2627,7 +2627,7 @@ public boolean postStateTransitionEvent(final StateMachine2.Transition<State, Vi
s_logger.info("Schedule a router reboot task as router " + vo.getId() + " is powered-on out-of-band. we need to reboot to refresh network rules");
--- End diff --
@DaanHoogland I see that you introduced RouterReprovisionOnOutOfBandMigration as part of commit b9dd67c383874ef7273205e0eac831de497bb02d. Wasn't this config just meant to decide between VR reboot and generating alert? But I see that both 'if' and 'else' conditions are different. Has something changed?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8709 No out of band migrate al...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/666#discussion_r36576681
--- Diff: server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java ---
@@ -2627,7 +2627,7 @@ public boolean postStateTransitionEvent(final StateMachine2.Transition<State, Vi
s_logger.info("Schedule a router reboot task as router " + vo.getId() + " is powered-on out-of-band. we need to reboot to refresh network rules");
_rebootRouterExecutor.execute(new RebootTask(vo.getId()));
} else {
- if (isOutOfBandMigrated(opaque)) {
+ if (isOutOfBandMigrated(opaque) && (vo.getType() == VirtualMachine.Type.DomainRouter)) {
--- End diff --
Yes, see my comment above
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8709 No out of band migrate al...
Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/666#issuecomment-128685143
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8709 No out of band migrate al...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/666#discussion_r36576679
--- Diff: server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java ---
@@ -2627,7 +2627,7 @@ public boolean postStateTransitionEvent(final StateMachine2.Transition<State, Vi
s_logger.info("Schedule a router reboot task as router " + vo.getId() + " is powered-on out-of-band. we need to reboot to refresh network rules");
--- End diff --
@koushik-das The problem is probably not solved in the most elegant way. The bit (vo.getType() == VirtualMachine.Type.DomainRouter) could have been pulled up in an extra containing if block.
In the end the only the reprovision_out_of_band is making the final choice between alerting and rebooting
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8709 No out of band migrate al...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/666
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8709 No out of band migrate al...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/666#discussion_r36580162
--- Diff: server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java ---
@@ -2627,7 +2627,7 @@ public boolean postStateTransitionEvent(final StateMachine2.Transition<State, Vi
s_logger.info("Schedule a router reboot task as router " + vo.getId() + " is powered-on out-of-band. we need to reboot to refresh network rules");
_rebootRouterExecutor.execute(new RebootTask(vo.getId()));
} else {
- if (isOutOfBandMigrated(opaque)) {
+ if (isOutOfBandMigrated(opaque) && (vo.getType() == VirtualMachine.Type.DomainRouter)) {
--- End diff --
@koushik-das A first shot:
final VirtualMachine.Event event = transition.getEvent();
boolean outOfBandMigrated = isOutOfBandMigrated(opaque);
boolean reprovision_out_of_band = RouterReprovisionOnOutOfBandMigration.value() && outOfBandMigrated;
if ( (vo.getType() == VirtualMachine.Type.DomainRouter)) {
if (
(event == VirtualMachine.Event.FollowAgentPowerOnReport) &&
(newState == State.Running) &&
((oldState == State.Stopped) || (reprovision_out_of_band))
) {
s_logger.info("Schedule a router reboot task as router " + vo.getId() + " is powered-on out-of-band. we need to reboot to refresh network rules");
_rebootRouterExecutor.execute(new RebootTask(vo.getId()));
} else {
if (outOfBandMigrated) {
final String title = "Router has been migrated out of band: " + vo.getInstanceName();
final String context =
"An out of band migration of router " + vo.getInstanceName() + "(" + vo.getUuid() + ") was detected. No automated action was performed.";
_alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_DOMAIN_ROUTER, vo.getDataCenterId(), vo.getPodIdToDeployIn(), title, context);
}
}
}
It can probably be more clean up even. I saw none but dummy tests are there for this class so I want to create that as well before committing
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8709 No out of band migrate al...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/666#discussion_r36526622
--- Diff: server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java ---
@@ -2627,7 +2627,7 @@ public boolean postStateTransitionEvent(final StateMachine2.Transition<State, Vi
s_logger.info("Schedule a router reboot task as router " + vo.getId() + " is powered-on out-of-band. we need to reboot to refresh network rules");
_rebootRouterExecutor.execute(new RebootTask(vo.getId()));
} else {
- if (isOutOfBandMigrated(opaque)) {
+ if (isOutOfBandMigrated(opaque) && (vo.getType() == VirtualMachine.Type.DomainRouter)) {
--- End diff --
Both if and else have the same check for DomainRouter, should we clean up the code?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8709 No out of band migrate al...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/666#issuecomment-128717347
LGTM from my super phone ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8709 No out of band migrate al...
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/666#issuecomment-128689198
@borisroman Thx, removed the redundant parentheses
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---