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 2021/02/18 14:11:29 UTC

[GitHub] [cloudstack] GutoVeronezi opened a new pull request #4705: Improve logs on KVMHAVMActivityChecker

GutoVeronezi opened a new pull request #4705:
URL: https://github.com/apache/cloudstack/pull/4705


   ### Description
   This PR intends to improve logging in the class `KVMHAVMActivityChecker` to facilitate troubleshooting.
   
   ### 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)
   - [x] Enhancement (improves an existing feature and functioanlity)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   - [ ] Major
   - [x] Minor
   
   ### How Has This Been Tested?
   It has been tested locally on a test lab.


----------------------------------------------------------------
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] sureshanaparti commented on a change in pull request #4705: Improve logs on KVMHAVMActivityChecker

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



##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAVMActivityChecker.java
##########
@@ -52,11 +52,14 @@ public Boolean checkingHB() {
         cmd.add("-t", String.valueOf(String.valueOf(System.currentTimeMillis() / 1000)));
         cmd.add("-d", String.valueOf(suspectTimeInSeconds));
         OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+

Review comment:
       @GutoVeronezi may be you can also improve this method name,  `checkingHB()` => _checkingHeartBeat()_




-- 
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 #4705: Improve logs on KVMHAVMActivityChecker

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



##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAVMActivityChecker.java
##########
@@ -52,11 +52,14 @@ public Boolean checkingHB() {
         cmd.add("-t", String.valueOf(String.valueOf(System.currentTimeMillis() / 1000)));
         cmd.add("-d", String.valueOf(suspectTimeInSeconds));
         OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+
         String result = cmd.execute(parser);
-        LOG.debug("KVMHAVMActivityChecker pool: " + nfsStoragePool._poolIp);
-        LOG.debug("KVMHAVMActivityChecker result: " + result);
-        LOG.debug("KVMHAVMActivityChecker parser: " + parser.getLine());
-        if (result == null && parser.getLine().contains("DEAD")) {
+        String parsedLine = parser.getLine();
+
+        LOG.debug(String.format("Checking HB with KVMHAVMActivityChecker [{command=\"%s\", result: \"%s\", log: \"%s\", pool: \"%s\"}].", cmd.toString(), result, parsedLine, nfsStoragePool._poolIp));

Review comment:
       I assuming that HB should be HA:
   1. Checking **HB** with KVMHAVMActivityChecker ...
   2. Checking **HA** with ...




-- 
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 #4705: Improve logs on KVMHAVMActivityChecker

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


   @GutoVeronezi on first sight the error in the smoke test run seems related to the changes, though the stack trace in the result file for that test does not. I'm re-running, if it fails again please have a look.


-- 
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 #4705: Improve logs on KVMHAVMActivityChecker

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


   log only, smoke tests are ok >> 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 #4705: Improve logs on KVMHAVMActivityChecker

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



##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAVMActivityChecker.java
##########
@@ -52,11 +52,14 @@ public Boolean checkingHB() {
         cmd.add("-t", String.valueOf(String.valueOf(System.currentTimeMillis() / 1000)));
         cmd.add("-d", String.valueOf(suspectTimeInSeconds));
         OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+
         String result = cmd.execute(parser);
-        LOG.debug("KVMHAVMActivityChecker pool: " + nfsStoragePool._poolIp);
-        LOG.debug("KVMHAVMActivityChecker result: " + result);
-        LOG.debug("KVMHAVMActivityChecker parser: " + parser.getLine());
-        if (result == null && parser.getLine().contains("DEAD")) {
+        String parsedLine = parser.getLine();
+
+        LOG.debug(String.format("Checking HB with KVMHAVMActivityChecker [{command=\"%s\", result: \"%s\", log: \"%s\", pool: \"%s\"}].", cmd.toString(), result, parsedLine, nfsStoragePool._poolIp));

Review comment:
       Never mind, it is the Heart Beat, right?
   
   I would suggest a different name then as HB might not mean much.




-- 
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 #4705: Improve logs on KVMHAVMActivityChecker

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


   <b>Trillian test result (tid-597)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 36593 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4705-t597-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Smoke tests completed. 87 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_disable_oobm_ha_state_ineligible | `Error` | 1512.09 | test_hostha_kvm.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 #4705: Improve logs on KVMHAVMActivityChecker

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


   Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 475


-- 
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 #4705: Improve logs on KVMHAVMActivityChecker

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


   Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 510


-- 
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] GutoVeronezi commented on pull request #4705: Improve logs on KVMHAVMActivityChecker

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


   @DaanHoogland @GabrielBrascher @sureshanaparti is there anything else to do?


-- 
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 #4705: Improve logs on KVMHAVMActivityChecker

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



##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAVMActivityChecker.java
##########
@@ -52,11 +52,14 @@ public Boolean checkingHB() {
         cmd.add("-t", String.valueOf(String.valueOf(System.currentTimeMillis() / 1000)));
         cmd.add("-d", String.valueOf(suspectTimeInSeconds));
         OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+

Review comment:
       **+1** HB is not really telling much




-- 
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] GutoVeronezi commented on a change in pull request #4705: Improve logs on KVMHAVMActivityChecker

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



##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAVMActivityChecker.java
##########
@@ -52,11 +52,14 @@ public Boolean checkingHB() {
         cmd.add("-t", String.valueOf(String.valueOf(System.currentTimeMillis() / 1000)));
         cmd.add("-d", String.valueOf(suspectTimeInSeconds));
         OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+
         String result = cmd.execute(parser);
-        LOG.debug("KVMHAVMActivityChecker pool: " + nfsStoragePool._poolIp);
-        LOG.debug("KVMHAVMActivityChecker result: " + result);
-        LOG.debug("KVMHAVMActivityChecker parser: " + parser.getLine());
-        if (result == null && parser.getLine().contains("DEAD")) {
+        String parsedLine = parser.getLine();
+
+        LOG.debug(String.format("Checking HB with KVMHAVMActivityChecker [{command=\"%s\", result: \"%s\", log: \"%s\", pool: \"%s\"}].", cmd.toString(), result, parsedLine, nfsStoragePool._poolIp));

Review comment:
       @GabrielBrascher done.




-- 
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 #4705: Improve logs on KVMHAVMActivityChecker

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


   <b>Trillian test result (tid-627)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 45918 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4705-t627-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Smoke tests completed. 87 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 3615.38 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.06 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.06 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 79.00 | test_kubernetes_clusters.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] DaanHoogland merged pull request #4705: Improve logs on KVMHAVMActivityChecker

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


   


-- 
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] GutoVeronezi commented on a change in pull request #4705: Improve logs on KVMHAVMActivityChecker

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



##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAVMActivityChecker.java
##########
@@ -52,11 +52,14 @@ public Boolean checkingHB() {
         cmd.add("-t", String.valueOf(String.valueOf(System.currentTimeMillis() / 1000)));
         cmd.add("-d", String.valueOf(suspectTimeInSeconds));
         OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+

Review comment:
       @sureshanaparti done.




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