You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/02/22 04:18:09 UTC
[GitHub] [cloudstack] Slair1 opened a new pull request #3908: Alert on
certain host status transitions
Slair1 opened a new pull request #3908: Alert on certain host status transitions
URL: https://github.com/apache/cloudstack/pull/3908
## Description
<!--- Describe your changes in detail -->
Add ability to create alert for host status transitions, such as from Alert->Up or Up->Disconnected, etc. Right now some transitions may alert, but we wanted to know about any major host status transition.
We also created a ConfigKey that defaults to the current behavior of not alerting on many of the transitions. But, but changing the setting to True, we can receive the host transitions which we find very helpful. Especially as once in a great while we'll have a host transition out of an Up status to something else. But we never get an alert that it transitioned back to Up (even if it happened 1 second later)....
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [X] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [X] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
## Screenshots (if appropriate):
Our environment has it set to True, but it defaults to False.
## How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
Tested putting hosts through various states. Made sure we received the emails and the Alerts appear in the UI.
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] Slair1 commented on a change in pull request #3908:
Alert on certain host status transitions
Posted by GitBox <gi...@apache.org>.
Slair1 commented on a change in pull request #3908: Alert on certain host status transitions
URL: https://github.com/apache/cloudstack/pull/3908#discussion_r382885098
##########
File path: api/src/main/java/com/cloud/host/Status.java
##########
@@ -119,45 +122,45 @@ public Status getNextStatus(Event e) throws NoTransitionException {
static {
Review comment:
if a true flag is passed at the end, that is the transitionAlertFlag. If it is True then an alert should be raised. If it is False or not passed at all, then an alert should NOT be raised.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] Slair1 commented on a change in pull request #3908:
Alert on certain host status transitions
Posted by GitBox <gi...@apache.org>.
Slair1 commented on a change in pull request #3908: Alert on certain host status transitions
URL: https://github.com/apache/cloudstack/pull/3908#discussion_r382885098
##########
File path: api/src/main/java/com/cloud/host/Status.java
##########
@@ -119,45 +122,45 @@ public Status getNextStatus(Event e) throws NoTransitionException {
static {
Review comment:
if a true flag is passed at the end, that is the transitionAlertFlag. If it is True then an alert should be raised. If it is False or not passed at all, then an alert should NOT be raised.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] Slair1 commented on a change in pull request #3908:
Alert on certain host status transitions
Posted by GitBox <gi...@apache.org>.
Slair1 commented on a change in pull request #3908: Alert on certain host status transitions
URL: https://github.com/apache/cloudstack/pull/3908#discussion_r382885268
##########
File path: utils/src/main/java/com/cloud/utils/fsm/StateMachine2.java
##########
@@ -50,7 +50,11 @@ public void addInitialTransition(E event, S toState) {
}
public void addTransition(S currentState, E event, S toState) {
- addTransition(new Transition<S, E>(currentState, event, toState, null));
+ addTransition(currentState, event, toState, false);
+ }
+
+ public void addTransition(S currentState, E event, S toState, boolean alertFlag) {
+ addTransition(new Transition<S, E>(currentState, event, toState, null, alertFlag));
}
Review comment:
Old constructor just calls new constructor with False for the alertFlag. So we don't break any functionality using the previous constructor
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] Slair1 commented on a change in pull request #3908:
Alert on certain host status transitions
Posted by GitBox <gi...@apache.org>.
Slair1 commented on a change in pull request #3908: Alert on certain host status transitions
URL: https://github.com/apache/cloudstack/pull/3908#discussion_r382885204
##########
File path: engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java
##########
@@ -1432,14 +1436,29 @@ public boolean agentStatusTransitTo(final HostVO host, final Status.Event e, fin
host.setManagementServerId(msId);
try {
- return _statusStateMachine.transitTo(host, e, host.getId(), _hostDao);
+ final Status hostStatus = host.getStatus();
+ final Status nextStatus = hostStatus.getNextStatus(e);
+
+ didTransit = _statusStateMachine.transitTo(host, e, host.getId(), _hostDao);
+ if (status_logger.isDebugEnabled()) {
+ status_logger.debug("agentStatusTransitTo: old status - " + hostStatus + ", event - " + e + ", new status - " + nextStatus + " (" + (didTransit ? "success)" : "failed)"));
+ }
+ if (AlertOnHostTransitions.value() && hostStatus.getTransitionAlertFlag(e)) {
+ final DateTimeFormatter dateFormatter = DateTimeFormatter.ofPattern("yyyy-MM-dd H:m:s,SSS");
+ final String hostShortDesc = host.getName() + " (id:" + host.getId() + ")";
+ _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), host.getName() + " transit from " + hostStatus + " to " + nextStatus + " (" + (didTransit ? "success)" : "failed)"),
+ (dateFormatter.format(LocalDateTime.now())) + "\r\nTransit agent status with event " + e + " for host " + hostShortDesc + " from " + hostStatus + " to " + nextStatus + (didTransit ? " was successful" : " failed"));
+ }
+
Review comment:
Above code is what raises the alert if both the ConfigKey is set and the TransitionAlertFlag is set for the particular Transition
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3908: Alert on
certain host status transitions
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3908: Alert on certain host status transitions
URL: https://github.com/apache/cloudstack/pull/3908#issuecomment-590115905
@Slair1 you marked this as new feature/enhancement. These only go on new releases. can you rebase to master?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] Slair1 commented on a change in pull request #3908:
Alert on certain host status transitions
Posted by GitBox <gi...@apache.org>.
Slair1 commented on a change in pull request #3908: Alert on certain host status transitions
URL: https://github.com/apache/cloudstack/pull/3908#discussion_r382885204
##########
File path: engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java
##########
@@ -1432,14 +1436,29 @@ public boolean agentStatusTransitTo(final HostVO host, final Status.Event e, fin
host.setManagementServerId(msId);
try {
- return _statusStateMachine.transitTo(host, e, host.getId(), _hostDao);
+ final Status hostStatus = host.getStatus();
+ final Status nextStatus = hostStatus.getNextStatus(e);
+
+ didTransit = _statusStateMachine.transitTo(host, e, host.getId(), _hostDao);
+ if (status_logger.isDebugEnabled()) {
+ status_logger.debug("agentStatusTransitTo: old status - " + hostStatus + ", event - " + e + ", new status - " + nextStatus + " (" + (didTransit ? "success)" : "failed)"));
+ }
+ if (AlertOnHostTransitions.value() && hostStatus.getTransitionAlertFlag(e)) {
+ final DateTimeFormatter dateFormatter = DateTimeFormatter.ofPattern("yyyy-MM-dd H:m:s,SSS");
+ final String hostShortDesc = host.getName() + " (id:" + host.getId() + ")";
+ _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), host.getName() + " transit from " + hostStatus + " to " + nextStatus + " (" + (didTransit ? "success)" : "failed)"),
+ (dateFormatter.format(LocalDateTime.now())) + "\r\nTransit agent status with event " + e + " for host " + hostShortDesc + " from " + hostStatus + " to " + nextStatus + (didTransit ? " was successful" : " failed"));
+ }
+
Review comment:
Above code is what raises the alert if both the ConfigKey is set and the TransitionAlertFlag is set for the particular Transition
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] Slair1 closed pull request #3908: Alert on certain
host status transitions
Posted by GitBox <gi...@apache.org>.
Slair1 closed pull request #3908: Alert on certain host status transitions
URL: https://github.com/apache/cloudstack/pull/3908
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] Slair1 commented on a change in pull request #3908:
Alert on certain host status transitions
Posted by GitBox <gi...@apache.org>.
Slair1 commented on a change in pull request #3908: Alert on certain host status transitions
URL: https://github.com/apache/cloudstack/pull/3908#discussion_r382885317
##########
File path: utils/src/main/java/com/cloud/utils/fsm/StateMachine2.java
##########
@@ -171,15 +179,22 @@ public String toString() {
private List<Impact> impacts;
+ private boolean alertFlag;
+
public static enum Impact {
USAGE
}
public Transition(S currentState, E event, S toState, List<Impact> impacts) {
+ this(currentState, event, toState, impacts, false);
+ }
+
+ public Transition(S currentState, E event, S toState, List<Impact> impacts, boolean alertFlag) {
this.currentState = currentState;
Review comment:
Old constructor just calls new constructor with False for the alertFlag. So we don't break any functionality using the previous constructor
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] Slair1 commented on issue #3908: Alert on certain host
status transitions
Posted by GitBox <gi...@apache.org>.
Slair1 commented on issue #3908: Alert on certain host status transitions
URL: https://github.com/apache/cloudstack/pull/3908#issuecomment-590362834
> @Slair1 you marked this as new feature/enhancement. These only go on new releases. can you rebase to master?
yep sure can. we are also getting alerts from this feature for our secondary storage VM in one of our environments. We are tracking down if it is a problem with this PR or if this PR just revealed an issue in that environment that we were previously unaware of.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] Slair1 commented on a change in pull request #3908:
Alert on certain host status transitions
Posted by GitBox <gi...@apache.org>.
Slair1 commented on a change in pull request #3908: Alert on certain host status transitions
URL: https://github.com/apache/cloudstack/pull/3908#discussion_r382885317
##########
File path: utils/src/main/java/com/cloud/utils/fsm/StateMachine2.java
##########
@@ -171,15 +179,22 @@ public String toString() {
private List<Impact> impacts;
+ private boolean alertFlag;
+
public static enum Impact {
USAGE
}
public Transition(S currentState, E event, S toState, List<Impact> impacts) {
+ this(currentState, event, toState, impacts, false);
+ }
+
+ public Transition(S currentState, E event, S toState, List<Impact> impacts, boolean alertFlag) {
this.currentState = currentState;
Review comment:
Old constructor just calls new constructor with False for the alertFlag. So we don't break any functionality using the previous constructor
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] Slair1 commented on a change in pull request #3908:
Alert on certain host status transitions
Posted by GitBox <gi...@apache.org>.
Slair1 commented on a change in pull request #3908: Alert on certain host status transitions
URL: https://github.com/apache/cloudstack/pull/3908#discussion_r382885268
##########
File path: utils/src/main/java/com/cloud/utils/fsm/StateMachine2.java
##########
@@ -50,7 +50,11 @@ public void addInitialTransition(E event, S toState) {
}
public void addTransition(S currentState, E event, S toState) {
- addTransition(new Transition<S, E>(currentState, event, toState, null));
+ addTransition(currentState, event, toState, false);
+ }
+
+ public void addTransition(S currentState, E event, S toState, boolean alertFlag) {
+ addTransition(new Transition<S, E>(currentState, event, toState, null, alertFlag));
}
Review comment:
Old constructor just calls new constructor with False for the alertFlag. So we don't break any functionality using the previous constructor
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services