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/10/21 18:17:29 UTC

[GitHub] [cloudstack] GabrielBrascher opened a new pull request #4418: Create EVENT in case of oobm failure

GabrielBrascher opened a new pull request #4418:
URL: https://github.com/apache/cloudstack/pull/4418


   ## Description
   <!--- Describe your changes in detail -->
   Out of band power operations are constantly issued to assess the host power status. However, if such actions fail the Admins receive a wanr messafge on log. In order to help Admins to easily track such failures, this PR creates an Event each time an Out-of-band power operation fails.
   
   In addition, this PR allows admins to silence such events via global settings by setting `create.event.on.oob.failure` to _false_; default value is _true_.
   
   ## 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)
   - [ ] 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)


----------------------------------------------------------------
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

[GitHub] [cloudstack] GabrielBrascher commented on pull request #4418: Create Event in case of OOBM failure

Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4418:
URL: https://github.com/apache/cloudstack/pull/4418#issuecomment-714644852


   @DaanHoogland I took the liberty to aim this one to 4.15.
   Please let me know if this one is getting too late then I move it to 4.16 ;)


----------------------------------------------------------------
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

[GitHub] [cloudstack] blueorangutan commented on pull request #4418: Create Event in case of OOBM failure

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4418:
URL: https://github.com/apache/cloudstack/pull/4418#issuecomment-714716824


   @DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


----------------------------------------------------------------
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

[GitHub] [cloudstack] DaanHoogland commented on pull request #4418: Create Event in case of OOBM failure

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4418:
URL: https://github.com/apache/cloudstack/pull/4418#issuecomment-724957540


   @PaulAngus very small and meets all criteria, merging


----------------------------------------------------------------
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

[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #4418: Create Event in case of OOBM failure

Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #4418:
URL: https://github.com/apache/cloudstack/pull/4418#discussion_r510305873



##########
File path: server/src/main/java/org/apache/cloudstack/outofbandmanagement/PowerOperationTask.java
##########
@@ -17,16 +17,26 @@
 
 package org.apache.cloudstack.outofbandmanagement;
 
+import com.cloud.event.ActionEventUtils;
+import com.cloud.event.EventTypes;
+import com.cloud.event.EventVO;
 import com.cloud.host.Host;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
 import org.apache.log4j.Logger;
 
-public class PowerOperationTask implements Runnable {
+public class PowerOperationTask implements Runnable, Configurable {
     public static final Logger LOG = Logger.getLogger(PowerOperationTask.class);
 
     final private OutOfBandManagementService service;
     final private Host host;
     final private OutOfBandManagement.PowerOperation powerOperation;
 
+    public static final ConfigKey<Boolean> CREATE_EVENT_ON_OOBM_FAILURE = new ConfigKey<Boolean>("Advanced", Boolean.class, "create.event.on.oob.failure", "true",

Review comment:
       Thanks for the suggestion, @wido.
   I am removing that global settings config key. There are too many config keys already, I am leaving to add a new one only if it is really necessary.




----------------------------------------------------------------
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

[GitHub] [cloudstack] DaanHoogland commented on pull request #4418: Create Event in case of OOBM failure

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4418:
URL: https://github.com/apache/cloudstack/pull/4418#issuecomment-721583824


   @PaulAngus can we put this in for 4.15?


----------------------------------------------------------------
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

[GitHub] [cloudstack] wido commented on a change in pull request #4418: Create Event in case of OOBM failure

Posted by GitBox <gi...@apache.org>.
wido commented on a change in pull request #4418:
URL: https://github.com/apache/cloudstack/pull/4418#discussion_r509924874



##########
File path: server/src/main/java/org/apache/cloudstack/outofbandmanagement/PowerOperationTask.java
##########
@@ -43,8 +53,26 @@ public void run() {
         try {
             service.executePowerOperation(host, powerOperation, null);
         } catch (Exception e) {
-            LOG.warn(String.format("Out-of-band management background task operation=%s for host id=%d failed with: %s",
-                    powerOperation.name(), host.getId(), e.getMessage()));
+            LOG.warn(String.format("Out-of-band management background task operation=%s for host id=%d name=%s failed with: %s",
+                    powerOperation.name(), host.getId(), host.getName(), e.getMessage()));
+
+            if(CREATE_EVENT_ON_OOBM_FAILURE.value()) {
+                String eventMessage = String.format("Error while issuing out-of-band management action (%s) for host (id: %d, name: %s)",
+                        powerOperation.name(), host.getId(), host.getName());

Review comment:
       Same applies here

##########
File path: server/src/main/java/org/apache/cloudstack/outofbandmanagement/PowerOperationTask.java
##########
@@ -43,8 +53,26 @@ public void run() {
         try {
             service.executePowerOperation(host, powerOperation, null);
         } catch (Exception e) {
-            LOG.warn(String.format("Out-of-band management background task operation=%s for host id=%d failed with: %s",
-                    powerOperation.name(), host.getId(), e.getMessage()));
+            LOG.warn(String.format("Out-of-band management background task operation=%s for host id=%d name=%s failed with: %s",
+                    powerOperation.name(), host.getId(), host.getName(), e.getMessage()));

Review comment:
       In general in CloudStack we try to not log the 'hostid' as it doesn't say anything. The ID can only be fetched from the DB and not from the API and thus UI.

##########
File path: server/src/main/java/org/apache/cloudstack/outofbandmanagement/PowerOperationTask.java
##########
@@ -17,16 +17,26 @@
 
 package org.apache.cloudstack.outofbandmanagement;
 
+import com.cloud.event.ActionEventUtils;
+import com.cloud.event.EventTypes;
+import com.cloud.event.EventVO;
 import com.cloud.host.Host;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
 import org.apache.log4j.Logger;
 
-public class PowerOperationTask implements Runnable {
+public class PowerOperationTask implements Runnable, Configurable {
     public static final Logger LOG = Logger.getLogger(PowerOperationTask.class);
 
     final private OutOfBandManagementService service;
     final private Host host;
     final private OutOfBandManagement.PowerOperation powerOperation;
 
+    public static final ConfigKey<Boolean> CREATE_EVENT_ON_OOBM_FAILURE = new ConfigKey<Boolean>("Advanced", Boolean.class, "create.event.on.oob.failure", "true",

Review comment:
       I know there is no naming scheme here, but isn't it easier if OOB related config keys are prefixed with *oob*?
   
   <pre>oob.create.event.on.failure</pre>
   
   For example




----------------------------------------------------------------
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

[GitHub] [cloudstack] DaanHoogland merged pull request #4418: Create Event in case of OOBM failure

Posted by GitBox <gi...@apache.org>.
DaanHoogland merged pull request #4418:
URL: https://github.com/apache/cloudstack/pull/4418


   


----------------------------------------------------------------
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

[GitHub] [cloudstack] DaanHoogland commented on pull request #4418: Create Event in case of OOBM failure

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4418:
URL: https://github.com/apache/cloudstack/pull/4418#issuecomment-714716342


   no tests added but for regression,
   @blueorangutan test


----------------------------------------------------------------
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

[GitHub] [cloudstack] GabrielBrascher commented on pull request #4418: Create Event in case of OOBM failure

Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4418:
URL: https://github.com/apache/cloudstack/pull/4418#issuecomment-720435310


   @rhtyd @DaanHoogland @wido @PaulAngus here follows a screenshot with the warning events:
   ![image](https://user-images.githubusercontent.com/5025148/97866825-74f46c80-1ceb-11eb-8627-c71400df95a9.png)
   


----------------------------------------------------------------
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

[GitHub] [cloudstack] blueorangutan commented on pull request #4418: Create Event in case of OOBM failure

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4418:
URL: https://github.com/apache/cloudstack/pull/4418#issuecomment-714983974


   <b>Trillian test result (tid-3058)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 38046 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4418-t3058-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Smoke tests completed. 84 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 315.93 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 302.26 | test_vpc_redundant.py
   


----------------------------------------------------------------
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

[GitHub] [cloudstack] blueorangutan commented on pull request #4418: Create Event in case of OOBM failure

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4418:
URL: https://github.com/apache/cloudstack/pull/4418#issuecomment-714657731


   Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2265


----------------------------------------------------------------
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

[GitHub] [cloudstack] GabrielBrascher commented on pull request #4418: Create Event in case of OOBM failure

Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4418:
URL: https://github.com/apache/cloudstack/pull/4418#issuecomment-714640014


   @blueorangutan package


----------------------------------------------------------------
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

[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #4418: Create Event in case of OOBM failure

Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #4418:
URL: https://github.com/apache/cloudstack/pull/4418#discussion_r510164323



##########
File path: server/src/main/java/org/apache/cloudstack/outofbandmanagement/PowerOperationTask.java
##########
@@ -43,8 +53,26 @@ public void run() {
         try {
             service.executePowerOperation(host, powerOperation, null);
         } catch (Exception e) {
-            LOG.warn(String.format("Out-of-band management background task operation=%s for host id=%d failed with: %s",
-                    powerOperation.name(), host.getId(), e.getMessage()));
+            LOG.warn(String.format("Out-of-band management background task operation=%s for host id=%d name=%s failed with: %s",
+                    powerOperation.name(), host.getId(), host.getName(), e.getMessage()));

Review comment:
       Makes sense, I am going to keep only the host name then. 




----------------------------------------------------------------
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

[GitHub] [cloudstack] blueorangutan commented on pull request #4418: Create Event in case of OOBM failure

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4418:
URL: https://github.com/apache/cloudstack/pull/4418#issuecomment-714640415


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


----------------------------------------------------------------
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