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/01/08 21:23:24 UTC
[GitHub] [cloudstack] GabrielBrascher opened a new pull request #4575: Enhance log messages with hostName
GabrielBrascher opened a new pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575
### Description
<!--- Describe your changes in DETAIL - And how has behavior functionally changed. -->
Numbers are great for computers, however, we (humans) deal better with names. Logging host's ID is good in terms of short messages; however, admins must know all hosts by their ID or look via DB/API queries to figure out which host the log message is pointing to.
This PR proposes an admin friendly approach for log messages by logging the host name instead of its ID.
An example of how it works currently log messages can be painful in large environments:
`Operation Failed vm's original host id: 334 new host id: 300 host id before state transition: 127`
Vs.
`Operation Failed vm's original host: node23.clusterA new host: node09.clusterA host before state transition: node01.clusterA`
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviors, 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: # -->
<!--- ********************************************************************************* -->
<!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
<!--- PLEASE PUT AN 'X' in only **ONE** box -->
<!--- ********************************************************************************* -->
### 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 functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [X] Minor
----------------------------------------------------------------
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] nvazquez merged pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
nvazquez merged pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r628266981
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2400,16 +2402,17 @@ public void deleteRoutingHost(final HostVO host, final boolean isForced, final b
// Restart HA enabled vms
for (final VMInstanceVO vm : vms) {
if (!vm.isHaEnabled() || vm.getState() == State.Stopping) {
- s_logger.debug("Stopping vm: " + vm + " as a part of deleteHost id=" + host.getId());
+ s_logger.debug("Stopping vm: " + vm + " as a part of hostDelete: " + host);
try {
_vmMgr.advanceStop(vm.getUuid(), false);
} catch (final Exception e) {
- final String errorMsg = "There was an error stopping the vm: " + vm + " as a part of hostDelete id=" + host.getId();
+ final String errorMsg = String.format("There was an error stopping the vm: %s as a part of hostDelete: %s", vm, host);
Review comment:
Lowercase `vm`.
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r628257170
##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -4339,13 +4345,13 @@ private void orchestrateMigrateForScale(final String vmUuid, final long srcHostI
try {
_agentMgr.send(srcHostId, new Commands(cleanup(vm.getInstanceName())), null);
} catch (final AgentUnavailableException e) {
- s_logger.error("AgentUnavailableException while cleanup on source host: " + srcHostId);
+ s_logger.error(String.format("AgentUnavailableException while cleanup on source %s: %s", srcHost), e);
Review comment:
There are 2 params passed to format, but only one value.
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2385,7 +2387,7 @@ public void deleteRoutingHost(final HostVO host, final boolean isForced, final b
try {
_vmMgr.destroy(vm.getUuid(), false);
} catch (final Exception e) {
- final String errorMsg = "There was an error Destory the vm: " + vm + " as a part of hostDelete id=" + host.getId();
+ final String errorMsg = String.format("There was an error when destroying the vm: %s as a part of hostDelete: %s", vm, host);
Review comment:
Lowercase `vm`.
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -1498,7 +1500,7 @@ public boolean checkAndMaintain(final long hostId) {
hostInMaintenance = attemptMaintain(host);
}
} catch (final NoTransitionException e) {
- s_logger.debug("Cannot transmit host " + host.getId() + " to Maintenance state", e);
+ s_logger.debug(String.format("Cannot transit %s to Maintenance state", host), e);
Review comment:
Question / Topic for discussion:
Could exceptions be logged in `debug` instead of `warn` or `error`?
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -903,7 +904,8 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
try {
resourceStateTransitTo(host, ResourceState.Event.DeleteHost, _nodeId);
} catch (final NoTransitionException e) {
- s_logger.debug("Cannot transmit host " + host.getId() + " to Enabled state", e);
+ s_logger.debug("Cannot transit " + host + " to Enabled state", e);
+ s_logger.debug(String.format("Cannot transit %s to Enabled state", host), e);
Review comment:
The same message twice.
##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -4354,8 +4360,8 @@ private void orchestrateMigrateForScale(final String vmUuid, final long srcHostI
s_logger.info("Migration was unsuccessful. Cleaning up: " + vm);
_alertMgr.sendAlert(alertType, fromHost.getDataCenterId(), fromHost.getPodId(),
- "Unable to migrate vm " + vm.getInstanceName() + " from host " + fromHost.getName() + " in zone " + dest.getDataCenter().getName() + " and pod " +
- dest.getPod().getName(), "Migrate Command failed. Please check logs.");
+ "Unable to migrate vm " + vm.getInstanceName() + " from " + fromHost + " in zone " + dest.getDataCenter().getName() + " and pod " +
Review comment:
We can use `String.format` here too.
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2400,16 +2402,17 @@ public void deleteRoutingHost(final HostVO host, final boolean isForced, final b
// Restart HA enabled vms
for (final VMInstanceVO vm : vms) {
if (!vm.isHaEnabled() || vm.getState() == State.Stopping) {
- s_logger.debug("Stopping vm: " + vm + " as a part of deleteHost id=" + host.getId());
+ s_logger.debug("Stopping vm: " + vm + " as a part of hostDelete: " + host);
try {
_vmMgr.advanceStop(vm.getUuid(), false);
} catch (final Exception e) {
- final String errorMsg = "There was an error stopping the vm: " + vm + " as a part of hostDelete id=" + host.getId();
+ final String errorMsg = String.format("There was an error stopping the vm: %s as a part of hostDelete: %s", vm, host);
Review comment:
Lowercase `vm`.
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2400,16 +2402,17 @@ public void deleteRoutingHost(final HostVO host, final boolean isForced, final b
// Restart HA enabled vms
for (final VMInstanceVO vm : vms) {
if (!vm.isHaEnabled() || vm.getState() == State.Stopping) {
- s_logger.debug("Stopping vm: " + vm + " as a part of deleteHost id=" + host.getId());
+ s_logger.debug("Stopping vm: " + vm + " as a part of hostDelete: " + host);
Review comment:
Lowercase `vm`.
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2400,16 +2402,17 @@ public void deleteRoutingHost(final HostVO host, final boolean isForced, final b
// Restart HA enabled vms
for (final VMInstanceVO vm : vms) {
if (!vm.isHaEnabled() || vm.getState() == State.Stopping) {
- s_logger.debug("Stopping vm: " + vm + " as a part of deleteHost id=" + host.getId());
+ s_logger.debug("Stopping vm: " + vm + " as a part of hostDelete: " + host);
try {
_vmMgr.advanceStop(vm.getUuid(), false);
} catch (final Exception e) {
- final String errorMsg = "There was an error stopping the vm: " + vm + " as a part of hostDelete id=" + host.getId();
+ final String errorMsg = String.format("There was an error stopping the vm: %s as a part of hostDelete: %s", vm, host);
s_logger.debug(errorMsg, e);
throw new UnableDeleteHostException(errorMsg + "," + e.getMessage());
}
} else if (vm.isHaEnabled() && (vm.getState() == State.Running || vm.getState() == State.Starting)) {
- s_logger.debug("Scheduling restart for vm: " + vm + " " + vm.getState() + " on the host id=" + host.getId());
+ s_logger.debug("Scheduling restart for vm: " + vm + " " + vm.getState() + " on host: " + host);
+ s_logger.debug(String.format("Scheduling restart for vm: %s, state: %s on host: %s.", vm, vm.getState(), host));
Review comment:
The same message twice;
Lowercase `vm`;
##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -5165,7 +5165,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
}
});
} catch (Exception e) {
- s_logger.warn("Unable to update vm disk statistics for vm: " + userVm.getId() + " from host: " + hostId, e);
+ s_logger.warn(String.format("Unable to update vm disk statistics for vm %s from %s", userVm.getInstanceName(), host), e);
Review comment:
Lowercase `vm`.
##########
File path: server/src/main/java/com/cloud/resource/RollingMaintenanceManagerImpl.java
##########
@@ -537,7 +539,7 @@ private boolean isMaintenanceScriptDefinedOnHost(Host host, List<HostSkipped> ho
answer = agentManager.send(host.getId(), cmd);
} catch (AgentUnavailableException | OperationTimedoutException e) {
// Agent may be restarted on the scripts - continue polling until it is up
- String msg = "Cannot send command to host: " + host.getId() + ", waiting " + pingInterval + "ms - " + e.getMessage();
+ String msg = String.format("Cannot send command to %s, waiting %sms - %s", host, pingInterval, e.getMessage());
s_logger.warn(msg);
Review comment:
Pass exception as parameter.
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-763496488
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2553
----------------------------------------------------------------
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] rafaelweingartner commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
rafaelweingartner commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-781397751
What about adding all of them? I mean, ID, Name, and UUID. This would for sure help debugging. It would have the ID of the DB already there, and the UUID of the element that is used in the API. Moreover, the name to make the log entry more friendly.
Something such as:
```
s_logger.debug(String.format("Create ClusteredDirectAgentAttache for host [id=%s, uuid=%s, name=%s] ", host.getId(), host.getUuid(), host.getName()));
```
----------------------------------------------------------------
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-834317951
THanks for the review @RodrigoDLopez @rafaelweingartner @shwstppr @weizhouapache @sureshanaparti; is there anything missing on the PR?
All the changes aplied recently were made according to the review from @GutoVeronezi.
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-833877536
Thanks for the review, @GutoVeronezi! Updated code in order to give prefference for `String.format`; additionally,
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-871583083
@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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-868954410
<b>Trillian test result (tid-1111)</b>
Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
Total time taken: 51309 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4575-t1111-vmware-65u2.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_outofbandmanagement_nestedplugin.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Smoke tests completed. 86 look OK, 2 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_oobm_issue_power_on | `Error` | 2.34 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_reset | `Error` | 4.15 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_soft | `Error` | 4.96 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_status | `Error` | 3.76 | test_outofbandmanagement_nestedplugin.py
test_01_migrate_VM_and_root_volume | `Error` | 116.45 | test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks | `Error` | 107.61 | test_vm_life_cycle.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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r662515339
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2451,11 +2452,11 @@ public HostVO fillRoutingHostVO(final HostVO host, final StartupRoutingCommand s
@Override
public void deleteRoutingHost(final HostVO host, final boolean isForced, final boolean forceDestroyStorage) throws UnableDeleteHostException {
if (host.getType() != Host.Type.Routing) {
- throw new CloudRuntimeException("Non-Routing host gets in deleteRoutingHost, id is " + host.getId());
+ throw new CloudRuntimeException(String.format("Non-Routing host gets in deleteRoutingHost, id is %s", host.getId()));
}
if (s_logger.isDebugEnabled()) {
- s_logger.debug("Deleting Host: " + host.getId() + " Guid:" + host.getGuid());
+ s_logger.debug(String.format("Deleting Host: %s", host));
Review comment:
Updated
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r628353103
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -1498,7 +1500,7 @@ public boolean checkAndMaintain(final long hostId) {
hostInMaintenance = attemptMaintain(host);
}
} catch (final NoTransitionException e) {
- s_logger.debug("Cannot transmit host " + host.getId() + " to Maintenance state", e);
+ s_logger.debug(String.format("Cannot transit %s to Maintenance state", host), e);
Review comment:
I think that `debug` is not ideal indeed at that context, `warn` looks like a good option. `Error` might be good, but I think that it can bring unnecessary alarms for operators.
I think that the current resource state would also be nice to add on the log. I will update it to something as:
```
s_logger.warn(String.format("Cannot transit %s from %s to Maintenance state", host, host.getResourceState()), e);
```
--
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] nvazquez commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-872270813
@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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-878209863
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-834639208
@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
[GitHub] [cloudstack] blueorangutan commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-868935696
<b>Trillian test result (tid-1110)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 41715 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4575-t1110-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_outofbandmanagement_nestedplugin.py
Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 85 look OK, 3 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 3608.43 | test_kubernetes_clusters.py
test_02_deploy_and_upgrade_kubernetes_cluster | `Failure` | 3609.25 | test_kubernetes_clusters.py
test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 0.06 | test_kubernetes_clusters.py
test_04_basic_lifecycle_kubernetes_cluster | `Failure` | 0.06 | test_kubernetes_clusters.py
test_05_delete_kubernetes_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 97.40 | test_kubernetes_clusters.py
test_oobm_issue_power_off | `Error` | 3.63 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_on | `Error` | 3.72 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_reset | `Error` | 4.62 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_soft | `Error` | 3.78 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_status | `Error` | 3.43 | test_outofbandmanagement_nestedplugin.py
test_hostha_kvm_host_fencing | `Error` | 178.67 | 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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-836361562
travis results are a real mess @GabrielBrascher , can you 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] GabrielBrascher commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-760443690
@shwstppr That is something that I can add. Maybe this is something that we can discuss (on the mailing list or a Github issue) ion order to find conventions that best fit CloudStack log messages.
Nowadays we do not have any convention.
----------------------------------------------------------------
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r628269339
##########
File path: server/src/main/java/com/cloud/resource/RollingMaintenanceManagerImpl.java
##########
@@ -537,7 +539,7 @@ private boolean isMaintenanceScriptDefinedOnHost(Host host, List<HostSkipped> ho
answer = agentManager.send(host.getId(), cmd);
} catch (AgentUnavailableException | OperationTimedoutException e) {
// Agent may be restarted on the scripts - continue polling until it is up
- String msg = "Cannot send command to host: " + host.getId() + ", waiting " + pingInterval + "ms - " + e.getMessage();
+ String msg = String.format("Cannot send command to %s, waiting %sms - %s", host, pingInterval, e.getMessage());
s_logger.warn(msg);
Review comment:
Pass exception as parameter.
--
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] nvazquez commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-871923063
@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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-839321410
<b>Trillian test result (tid-657)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 59218 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4575-t657-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_outofbandmanagement_nestedplugin.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 84 look OK, 4 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 3607.07 | test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 73.84 | test_kubernetes_clusters.py
test_oobm_issue_power_cycle | `Error` | 3.23 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_off | `Error` | 3.43 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_reset | `Error` | 2.21 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_status | `Error` | 1.15 | test_outofbandmanagement_nestedplugin.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 466.22 | test_vpc_redundant.py
test_05_rvpc_multi_tiers | `Failure` | 501.47 | test_vpc_redundant.py
test_hostha_kvm_host_fencing | `Error` | 199.81 | 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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-871954014
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 442
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-762599607
@RodrigoDLopez Hi, I don't have any issue in using just the names of host if we have consensus on that.
I liked the idea of having a discussion on ML as that will allow forming a convention project wide
----------------------------------------------------------------
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r628267398
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -903,7 +904,8 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
try {
resourceStateTransitTo(host, ResourceState.Event.DeleteHost, _nodeId);
} catch (final NoTransitionException e) {
- s_logger.debug("Cannot transmit host " + host.getId() + " to Enabled state", e);
+ s_logger.debug("Cannot transit " + host + " to Enabled state", e);
+ s_logger.debug(String.format("Cannot transit %s to Enabled state", host), e);
Review comment:
The same message twice.
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-834596143
@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] blueorangutan commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-834597052
@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
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-791612263
@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] blueorangutan commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-871599667
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 435
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r626882785
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2105,7 +2106,7 @@ private Host createHostAndAgent(final ServerResource resource, final Map<String,
host = findHostByGuid(firstCmd.getGuidWithoutResource());
}
if (host != null && host.getRemoved() == null) { // host already added, no need to add again
- s_logger.debug("Found the host " + host.getId() + " by guid: " + firstCmd.getGuid() + ", old host reconnected as new");
+ s_logger.debug(String.format("Found host [id: %d, name: %s] by guid: %s, old host reconnected as new", host.getId(), host.getName(), firstCmd.getGuid()));
Review comment:
@GabrielBrascher We can log the entire object here, as it has `toString()` implemented.
##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -4339,13 +4343,13 @@ private void orchestrateMigrateForScale(final String vmUuid, final long srcHostI
try {
_agentMgr.send(srcHostId, new Commands(cleanup(vm.getInstanceName())), null);
} catch (final AgentUnavailableException e) {
- s_logger.error("AgentUnavailableException while cleanup on source host: " + srcHostId);
+ s_logger.error(String.format("AgentUnavailableException while cleanup on source host: %s", srcHost));
Review comment:
@GabrielBrascher We can pass the exception as param to `s_logger.error` here.
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2178,7 +2179,7 @@ private Host createHostAndAgentDeferred(final ServerResource resource, final Map
// added, no
// need to add
// again
- s_logger.debug("Found the host " + host.getId() + " by guid: " + firstCmd.getGuid() + ", old host reconnected as new");
+ s_logger.debug(String.format("Found host [id: %d, name: %s] by guid %s, old host reconnected as new", host.getId(), host.getName(), firstCmd.getGuid()));
Review comment:
@GabrielBrascher We can log the entire object here, as it has toString() implemented.
##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -4219,14 +4219,18 @@ private void orchestrateMigrateForScale(final String vmUuid, final long srcHostI
vm.getServiceOfferingId();
final long dstHostId = dest.getHost().getId();
final Host fromHost = _hostDao.findById(srcHostId);
+ Host srcHost = _hostDao.findById(srcHostId);
if (fromHost == null) {
- s_logger.info("Unable to find the host to migrate from: " + srcHostId);
- throw new CloudRuntimeException("Unable to find the host to migrate from: " + srcHostId);
+ String logMessageUnableToFindHost = String.format("Unable to find host to migrate from %s.", srcHost);
+ s_logger.info(logMessageUnableToFindHost);
+ throw new CloudRuntimeException(logMessageUnableToFindHost);
}
+ Host dstHost = _hostDao.findById(dstHostId);
if (fromHost.getClusterId().longValue() != dest.getCluster().getId()) {
Review comment:
@GabrielBrascher As this class has already been touched, what do you think of doing a few refactors?
Like removing the explicit unboxing...
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-832995364
@GabrielBrascher I noticed that in some changes you added `String.format` but in anothers no, is there any reason to it? Can we use `String.format` in all of them?
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-836770159
@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] blueorangutan commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-763474534
@ravening 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
[GitHub] [cloudstack] blueorangutan commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-871583560
@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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-819059747
@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] blueorangutan commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-837307704
@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
[GitHub] [cloudstack] blueorangutan commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-865416113
<b>Trillian test result (tid-1023)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 56119 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4575-t1023-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_nic.py
Intermittent failure detected: /marvin/tests/smoke/test_outofbandmanagement_nestedplugin.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 81 look OK, 7 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_nic | `Error` | 161.70 | test_nic.py
test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 213.73 | test_internal_lb.py
test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 213.74 | test_internal_lb.py
test_oobm_issue_power_cycle | `Error` | 2.38 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_off | `Error` | 3.52 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_on | `Error` | 3.60 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_reset | `Error` | 4.09 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_soft | `Error` | 4.60 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_status | `Error` | 3.58 | test_outofbandmanagement_nestedplugin.py
test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 344.04 | test_routers_network_ops.py
test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Failure` | 379.93 | test_routers_network_ops.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 533.25 | test_vpc_redundant.py
test_01_redundant_vpc_site2site_vpn | `Failure` | 377.51 | test_vpc_vpn.py
test_hostha_kvm_host_fencing | `Error` | 103.81 | 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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-872960735
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 457
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] RodrigoDLopez commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-762318256
Hey @shwstppr what do you suggest?
I totally agree with @GabrielBrascher, normally when we are talking about a host we use their names to refer to it, I never met someone who uses UUID for this purpose.
Maybe, in some cases, the UUID turn easy to find these hosts on the database, but with a simple change on the query, we have the same result.
And for me, something like
> Operation Failed vm's original host: node23.clusterA (ID: 98c37930-599f-11eb-ae93-0242ac130002) new host: node09.clusterA (ID: bdb22ea8-599f-11eb-ae93-0242ac130002) host before state transition: node01.clusterA (ID: bdb22598-599f-11eb-ae93-0242ac130002)
it is too verbose and does not add anything to the debug factor.
Presenting only the hostname in the log makes it easy to identify, as it is the way we use it to reference it daily. And with the name, we can find the UUID if necessary.
Don't you agree?
----------------------------------------------------------------
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-837550374
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 576
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-871923446
@nvazquez 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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher closed pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher closed pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-878725841
<b>Trillian test result (tid-1238)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42963 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4575-t1238-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Intermittent failure detected: /marvin/tests/smoke/test_host_maintenance.py
Smoke tests completed. 87 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` | 536.06 | test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 464.38 | 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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-872271178
@nvazquez 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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher edited a comment on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher edited a comment on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-760443690
@shwstppr That is something that I can add. Maybe this is something that we can discuss (on the mailing list or a Github issue) in order to find conventions that best fit CloudStack log messages.
Nowadays we do not have any convention.
----------------------------------------------------------------
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-834638646
@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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r627773900
##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -4219,14 +4219,18 @@ private void orchestrateMigrateForScale(final String vmUuid, final long srcHostI
vm.getServiceOfferingId();
final long dstHostId = dest.getHost().getId();
final Host fromHost = _hostDao.findById(srcHostId);
+ Host srcHost = _hostDao.findById(srcHostId);
if (fromHost == null) {
- s_logger.info("Unable to find the host to migrate from: " + srcHostId);
- throw new CloudRuntimeException("Unable to find the host to migrate from: " + srcHostId);
+ String logMessageUnableToFindHost = String.format("Unable to find host to migrate from %s.", srcHost);
+ s_logger.info(logMessageUnableToFindHost);
+ throw new CloudRuntimeException(logMessageUnableToFindHost);
}
+ Host dstHost = _hostDao.findById(dstHostId);
if (fromHost.getClusterId().longValue() != dest.getCluster().getId()) {
Review comment:
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] GabrielBrascher commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-819062707
@GutoVeronezi this PR takes into account the Host to string added by you at PR #4706. I would appreciate any feedback on this one?
@shwstppr the changes on this PR, which takes into account #4706 (merged), should fit with your review by adding the Host **Name**, **ID**, as well as its **UUID**. Can you please review this PR with the proposed approach? Thanks!
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-834664371
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 538
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-868554808
@sureshanaparti a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) 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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r662492455
##########
File path: server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java
##########
@@ -926,8 +926,11 @@ public boolean postStateTransitionEvent(StateMachine2.Transition<State, Event> t
State oldState = transition.getCurrentState();
State newState = transition.getToState();
Event event = transition.getEvent();
- s_logger.debug("VM state transitted from :" + oldState + " to " + newState + " with event: " + event + "vm's original host id: " + vm.getLastHostId() +
- " new host id: " + vm.getHostId() + " host id before state transition: " + oldHostId);
+ Host lastHost = _hostDao.findById(vm.getLastHostId());
+ Host oldHost = _hostDao.findById(oldHostId);
+ Host newHost = _hostDao.findById(vm.getHostId());
+ s_logger.debug(String.format("VM state transited from [%s] to [%s] with event [%s]. VM's original host: %s, new host: %s, host before state transition: %s", oldState,
Review comment:
We could log VM's info (id, name...) here.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-781564472
@rafaelweingartner @sureshanaparti having ID, UUID, and the name sounds like a good way. Might be a bit verbose, but it serves well all the preferences.
----------------------------------------------------------------
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-836454230
<b>Trillian Build Failed (tid-631)<b/>
--
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 closed pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher closed pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575
----------------------------------------------------------------
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r628321847
##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -5165,7 +5165,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
}
});
} catch (Exception e) {
- s_logger.warn("Unable to update vm disk statistics for vm: " + userVm.getId() + " from host: " + hostId, e);
+ s_logger.warn(String.format("Unable to update vm disk statistics for vm %s from %s", userVm.getInstanceName(), host), e);
Review comment:
```suggestion
s_logger.warn(String.format("Unable to update VM disk statistics for vm %s from %s", userVm.getInstanceName(), host), e);
```
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -903,7 +904,8 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
try {
resourceStateTransitTo(host, ResourceState.Event.DeleteHost, _nodeId);
} catch (final NoTransitionException e) {
- s_logger.debug("Cannot transmit host " + host.getId() + " to Enabled state", e);
+ s_logger.debug("Cannot transit " + host + " to Enabled state", e);
+ s_logger.debug(String.format("Cannot transit %s to Enabled state", host), e);
Review comment:
```suggestion
s_logger.debug(String.format("Cannot transit %s to Enabled state", host), e);
```
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2400,16 +2402,17 @@ public void deleteRoutingHost(final HostVO host, final boolean isForced, final b
// Restart HA enabled vms
for (final VMInstanceVO vm : vms) {
if (!vm.isHaEnabled() || vm.getState() == State.Stopping) {
- s_logger.debug("Stopping vm: " + vm + " as a part of deleteHost id=" + host.getId());
+ s_logger.debug("Stopping vm: " + vm + " as a part of hostDelete: " + host);
try {
_vmMgr.advanceStop(vm.getUuid(), false);
} catch (final Exception e) {
- final String errorMsg = "There was an error stopping the vm: " + vm + " as a part of hostDelete id=" + host.getId();
+ final String errorMsg = String.format("There was an error stopping the vm: %s as a part of hostDelete: %s", vm, host);
s_logger.debug(errorMsg, e);
throw new UnableDeleteHostException(errorMsg + "," + e.getMessage());
}
} else if (vm.isHaEnabled() && (vm.getState() == State.Running || vm.getState() == State.Starting)) {
- s_logger.debug("Scheduling restart for vm: " + vm + " " + vm.getState() + " on the host id=" + host.getId());
+ s_logger.debug("Scheduling restart for vm: " + vm + " " + vm.getState() + " on host: " + host);
+ s_logger.debug(String.format("Scheduling restart for vm: %s, state: %s on host: %s.", vm, vm.getState(), host));
Review comment:
```suggestion
s_logger.debug(String.format("Scheduling restart for VM: %s, state: %s on host: %s.", vm, vm.getState(), host));
```
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2400,16 +2402,17 @@ public void deleteRoutingHost(final HostVO host, final boolean isForced, final b
// Restart HA enabled vms
for (final VMInstanceVO vm : vms) {
if (!vm.isHaEnabled() || vm.getState() == State.Stopping) {
- s_logger.debug("Stopping vm: " + vm + " as a part of deleteHost id=" + host.getId());
+ s_logger.debug("Stopping vm: " + vm + " as a part of hostDelete: " + host);
Review comment:
```suggestion
s_logger.debug("Stopping VM: " + vm + " as a part of hostDelete: " + host);
```
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2400,16 +2402,17 @@ public void deleteRoutingHost(final HostVO host, final boolean isForced, final b
// Restart HA enabled vms
for (final VMInstanceVO vm : vms) {
if (!vm.isHaEnabled() || vm.getState() == State.Stopping) {
- s_logger.debug("Stopping vm: " + vm + " as a part of deleteHost id=" + host.getId());
+ s_logger.debug("Stopping vm: " + vm + " as a part of hostDelete: " + host);
try {
_vmMgr.advanceStop(vm.getUuid(), false);
} catch (final Exception e) {
- final String errorMsg = "There was an error stopping the vm: " + vm + " as a part of hostDelete id=" + host.getId();
+ final String errorMsg = String.format("There was an error stopping the vm: %s as a part of hostDelete: %s", vm, host);
Review comment:
```suggestion
final String errorMsg = String.format("There was an error stopping the VM: %s as a part of hostDelete: %s", vm, host);
```
##########
File path: server/src/main/java/com/cloud/resource/RollingMaintenanceManagerImpl.java
##########
@@ -537,7 +539,7 @@ private boolean isMaintenanceScriptDefinedOnHost(Host host, List<HostSkipped> ho
answer = agentManager.send(host.getId(), cmd);
} catch (AgentUnavailableException | OperationTimedoutException e) {
// Agent may be restarted on the scripts - continue polling until it is up
- String msg = "Cannot send command to host: " + host.getId() + ", waiting " + pingInterval + "ms - " + e.getMessage();
+ String msg = String.format("Cannot send command to %s, waiting %sms - %s", host, pingInterval, e.getMessage());
s_logger.warn(msg);
Review comment:
```suggestion
s_logger.warn(msg, e);
```
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2385,7 +2387,7 @@ public void deleteRoutingHost(final HostVO host, final boolean isForced, final b
try {
_vmMgr.destroy(vm.getUuid(), false);
} catch (final Exception e) {
- final String errorMsg = "There was an error Destory the vm: " + vm + " as a part of hostDelete id=" + host.getId();
+ final String errorMsg = String.format("There was an error when destroying the vm: %s as a part of hostDelete: %s", vm, host);
Review comment:
```suggestion
String errorMsg = String.format("There was an error when destroying the VM: %s as a part of hostDelete: %s", vm, host);
```
--
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] weizhouapache commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-782105463
@GabrielBrascher it would be good to add host name in the output.
ip is not very important as we normally ssh into server using hostname, not ip.
```
diff --git a/engine/schema/src/main/java/com/cloud/host/HostVO.java b/engine/schema/src/main/java/com/cloud/host/HostVO.java
index 1c3d7c4267..a78b940142 100644
--- a/engine/schema/src/main/java/com/cloud/host/HostVO.java
+++ b/engine/schema/src/main/java/com/cloud/host/HostVO.java
@@ -677,7 +677,7 @@ public class HostVO implements Host {
@Override
public String toString() {
- return new StringBuilder("Host[").append("-").append(id).append("-").append(type).append("]").toString();
+ return new StringBuilder("Host[").append(name).append("-").append(id).append("-").append(type).append("]").toString();
}
public void setHypervisorType(HypervisorType hypervisorType) {
```
----------------------------------------------------------------
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r628266956
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2400,16 +2402,17 @@ public void deleteRoutingHost(final HostVO host, final boolean isForced, final b
// Restart HA enabled vms
for (final VMInstanceVO vm : vms) {
if (!vm.isHaEnabled() || vm.getState() == State.Stopping) {
- s_logger.debug("Stopping vm: " + vm + " as a part of deleteHost id=" + host.getId());
+ s_logger.debug("Stopping vm: " + vm + " as a part of hostDelete: " + host);
Review comment:
Lowercase `vm`.
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r628266703
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2385,7 +2387,7 @@ public void deleteRoutingHost(final HostVO host, final boolean isForced, final b
try {
_vmMgr.destroy(vm.getUuid(), false);
} catch (final Exception e) {
- final String errorMsg = "There was an error Destory the vm: " + vm + " as a part of hostDelete id=" + host.getId();
+ final String errorMsg = String.format("There was an error when destroying the vm: %s as a part of hostDelete: %s", vm, host);
Review comment:
Lowercase `vm`.
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-872766207
<b>Trillian test result (tid-1188)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 58897 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4575-t1188-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_storage_policy.py
Intermittent failure detected: /marvin/tests/smoke/test_templates.py
Intermittent failure detected: /marvin/tests/smoke/test_usage.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
Smoke tests completed. 81 look OK, 7 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
ContextSuite context=TestVMWareStoragePolicies>:setup | `Error` | 0.00 | test_storage_policy.py
test_02_create_template_with_checksum_sha1 | `Error` | 65.36 | test_templates.py
test_03_create_template_with_checksum_sha256 | `Error` | 65.36 | test_templates.py
test_04_create_template_with_checksum_md5 | `Error` | 65.38 | test_templates.py
test_05_create_template_with_no_checksum | `Error` | 65.38 | test_templates.py
test_04_extract_template | `Failure` | 128.34 | test_templates.py
ContextSuite context=TestISOUsage>:setup | `Error` | 0.00 | test_usage.py
test_01_volume_usage | `Failure` | 786.45 | test_usage.py
test_10_attachAndDetach_iso | `Failure` | 1510.13 | test_vm_life_cycle.py
test_06_download_detached_volume | `Failure` | 426.80 | test_volumes.py
ContextSuite context=TestVPCRedundancy>:setup | `Error` | 0.00 | test_vpc_redundant.py
ContextSuite context=TestVPCNics>:setup | `Error` | 0.00 | test_vpc_router_nics.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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-782134336
@weizhouapache Sorry if I was not clear, but PR #4706 is adding the host's name indeed, IP address would not be interesting indeed.
The idea is exactly what you mentioned. We will then log host ID, UUID, and Name (Type will also be logged the host.toString is used for non-routing hosts as well).
This PR also uses the `toString` in order to have a standard Host log. As PR #4706 already proposes an interesting approach, I added it on this proposal as well.
----------------------------------------------------------------
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-868987383
<b>Trillian test result (tid-1109)</b>
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 72337 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4575-t1109-xenserver-71.zip
Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_outofbandmanagement_nestedplugin.py
Intermittent failure detected: /marvin/tests/smoke/test_outofbandmanagement.py
Intermittent failure detected: /marvin/tests/smoke/test_primary_storage.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_pvlan.py
Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermittent failure detected: /marvin/tests/smoke/test_routers.py
Intermittent failure detected: /marvin/tests/smoke/test_scale_vm.py
Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
Intermittent failure detected: /marvin/tests/smoke/test_usage.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 78 look OK, 10 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 3600.95 | test_kubernetes_clusters.py
test_02_deploy_and_upgrade_kubernetes_cluster | `Failure` | 145.24 | test_kubernetes_clusters.py
test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
test_04_basic_lifecycle_kubernetes_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
test_05_delete_kubernetes_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.06 | test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 121.32 | test_kubernetes_clusters.py
test_oobm_issue_power_on | `Error` | 3.60 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_reset | `Error` | 4.18 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_status | `Error` | 1.15 | test_outofbandmanagement_nestedplugin.py
test_create_pvlan_network | `Error` | 0.06 | test_pvlan.py
ContextSuite context=TestRAMCPUResourceAccounting>:setup | `Error` | 0.00 | test_resource_accounting.py
ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
ContextSuite context=TestRouterIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
ContextSuite context=TestVPCIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
ContextSuite context=TestIsolatedNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
ContextSuite context=TestRedundantIsolateNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
test_06_stop_cpvm | `Error` | 968.35 | test_ssvm.py
test_01_redundant_vpc_site2site_vpn | `Failure` | 445.25 | test_vpc_vpn.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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-819070881
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 409
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-871851690
Thanks for the review, @GutoVeronezi! I've updated the code addressing your comments.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r662502262
##########
File path: server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java
##########
@@ -926,8 +926,11 @@ public boolean postStateTransitionEvent(StateMachine2.Transition<State, Event> t
State oldState = transition.getCurrentState();
State newState = transition.getToState();
Event event = transition.getEvent();
- s_logger.debug("VM state transitted from :" + oldState + " to " + newState + " with event: " + event + "vm's original host id: " + vm.getLastHostId() +
- " new host id: " + vm.getHostId() + " host id before state transition: " + oldHostId);
+ Host lastHost = _hostDao.findById(vm.getLastHostId());
+ Host oldHost = _hostDao.findById(oldHostId);
+ Host newHost = _hostDao.findById(vm.getHostId());
+ s_logger.debug(String.format("VM state transited from [%s] to [%s] with event [%s]. VM's original host: %s, new host: %s, host before state transition: %s", oldState,
Review comment:
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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r628259402
##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -4354,8 +4360,8 @@ private void orchestrateMigrateForScale(final String vmUuid, final long srcHostI
s_logger.info("Migration was unsuccessful. Cleaning up: " + vm);
_alertMgr.sendAlert(alertType, fromHost.getDataCenterId(), fromHost.getPodId(),
- "Unable to migrate vm " + vm.getInstanceName() + " from host " + fromHost.getName() + " in zone " + dest.getDataCenter().getName() + " and pod " +
- dest.getPod().getName(), "Migrate Command failed. Please check logs.");
+ "Unable to migrate vm " + vm.getInstanceName() + " from " + fromHost + " in zone " + dest.getDataCenter().getName() + " and pod " +
Review comment:
We can use `String.format` here too.
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r661920247
##########
File path: server/src/main/java/com/cloud/resource/RollingMaintenanceManagerImpl.java
##########
@@ -571,7 +571,7 @@ private void performPreFlightChecks(List<Host> hosts, int timeout, String payloa
for (Host host : hosts) {
Ternary<Boolean, String, Boolean> result = performStageOnHost(host, Stage.PreFlight, timeout, payload, forced);
if (result.third() && !hostsToAvoidMaintenance.containsKey(host.getId())) {
- s_logger.debug("Host " + host.getId() + " added to the avoid maintenance set");
+ s_logger.debug(String.format("%s added to the avoid maintenance set", host));
hostsToAvoidMaintenance.put(host.getId(), "Pre-flight stage set to avoid maintenance");
}
Review comment:
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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-837308602
@DaanHoogland fixed, travis looks good now, despite one job that keeps reaching maximum logs.
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r627774195
##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -4339,13 +4343,13 @@ private void orchestrateMigrateForScale(final String vmUuid, final long srcHostI
try {
_agentMgr.send(srcHostId, new Commands(cleanup(vm.getInstanceName())), null);
} catch (final AgentUnavailableException e) {
- s_logger.error("AgentUnavailableException while cleanup on source host: " + srcHostId);
+ s_logger.error(String.format("AgentUnavailableException while cleanup on source host: %s", srcHost));
Review comment:
@GutoVeronezi that's a good option indeed, thanks for the attention. I've just updated it.
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r628267824
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2400,16 +2402,17 @@ public void deleteRoutingHost(final HostVO host, final boolean isForced, final b
// Restart HA enabled vms
for (final VMInstanceVO vm : vms) {
if (!vm.isHaEnabled() || vm.getState() == State.Stopping) {
- s_logger.debug("Stopping vm: " + vm + " as a part of deleteHost id=" + host.getId());
+ s_logger.debug("Stopping vm: " + vm + " as a part of hostDelete: " + host);
try {
_vmMgr.advanceStop(vm.getUuid(), false);
} catch (final Exception e) {
- final String errorMsg = "There was an error stopping the vm: " + vm + " as a part of hostDelete id=" + host.getId();
+ final String errorMsg = String.format("There was an error stopping the vm: %s as a part of hostDelete: %s", vm, host);
s_logger.debug(errorMsg, e);
throw new UnableDeleteHostException(errorMsg + "," + e.getMessage());
}
} else if (vm.isHaEnabled() && (vm.getState() == State.Running || vm.getState() == State.Starting)) {
- s_logger.debug("Scheduling restart for vm: " + vm + " " + vm.getState() + " on the host id=" + host.getId());
+ s_logger.debug("Scheduling restart for vm: " + vm + " " + vm.getState() + " on host: " + host);
+ s_logger.debug(String.format("Scheduling restart for vm: %s, state: %s on host: %s.", vm, vm.getState(), host));
Review comment:
The same message twice;
Lowercase `vm`;
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-866454747
<b>Trillian test result (tid-1035)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 64027 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4575-t1035-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_outofbandmanagement_nestedplugin.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
Intermittent failure detected: /marvin/tests/smoke/test_storage_policy.py
Intermittent failure detected: /marvin/tests/smoke/test_templates.py
Intermittent failure detected: /marvin/tests/smoke/test_usage.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 76 look OK, 12 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 231.51 | test_internal_lb.py
test_oobm_issue_power_cycle | `Error` | 3.29 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_off | `Error` | 3.34 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_reset | `Error` | 4.40 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_soft | `Error` | 3.93 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_status | `Error` | 1.17 | test_outofbandmanagement_nestedplugin.py
test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Failure` | 343.97 | test_routers_network_ops.py
ContextSuite context=TestVMWareStoragePolicies>:setup | `Error` | 0.00 | test_storage_policy.py
test_02_create_template_with_checksum_sha1 | `Error` | 65.38 | test_templates.py
test_03_create_template_with_checksum_sha256 | `Error` | 65.38 | test_templates.py
test_04_create_template_with_checksum_md5 | `Error` | 65.40 | test_templates.py
test_05_create_template_with_no_checksum | `Error` | 65.53 | test_templates.py
test_04_extract_template | `Failure` | 128.34 | test_templates.py
ContextSuite context=TestISOUsage>:setup | `Error` | 0.00 | test_usage.py
test_01_volume_usage | `Error` | 186.00 | test_usage.py
test_10_attachAndDetach_iso | `Failure` | 1510.14 | test_vm_life_cycle.py
test_06_download_detached_volume | `Failure` | 432.88 | test_volumes.py
ContextSuite context=TestVPCRedundancy>:setup | `Error` | 0.00 | test_vpc_redundant.py
ContextSuite context=TestVPCNics>:setup | `Error` | 0.00 | test_vpc_router_nics.py
ContextSuite context=TestRVPCSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
ContextSuite context=TestVPCSite2SiteVPNMultipleOptions>:setup | `Error` | 0.00 | test_vpc_vpn.py
ContextSuite context=TestVpcRemoteAccessVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
ContextSuite context=TestVpcSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
test_hostha_kvm_host_fencing | `Error` | 102.84 | 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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-864006798
--
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] nvazquez commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-879384589
Merging based on approvals and test results
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-878210201
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-873292586
<b>Trillian test result (tid-1194)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33154 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4575-t1194-kvm-centos7.zip
Smoke tests completed. 88 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-836771085
@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
[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r661714750
##########
File path: server/src/main/java/com/cloud/network/element/VirtualRouterElement.java
##########
@@ -411,12 +411,12 @@ public boolean stopVpn(final RemoteAccessVpn vpn) throws ResourceUnavailableExce
if (canHandle(network, Service.Vpn)) {
final List<DomainRouterVO> routers = _routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER);
if (routers == null || routers.isEmpty()) {
- s_logger.debug("Virtual router elemnt doesn't need stop vpn on the backend; virtual router doesn't " + "exist in the network " + network.getId());
+ s_logger.debug(String.format("Virtual router element doesn't need stop VPN on the backend; virtual router doesn't exist in the network %s", network.getId()));
Review comment:
We could simplify this message, e.g.:
`There is no virtual router in network [%s], it is not necessary to stop the VPN on backend.`
##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -4489,8 +4495,8 @@ private void orchestrateMigrateForScale(final String vmUuid, final long srcHostI
s_logger.info("Migration was unsuccessful. Cleaning up: " + vm);
_alertMgr.sendAlert(alertType, fromHost.getDataCenterId(), fromHost.getPodId(),
- "Unable to migrate vm " + vm.getInstanceName() + " from host " + fromHost.getName() + " in zone " + dest.getDataCenter().getName() + " and pod " +
- dest.getPod().getName(), "Migrate Command failed. Please check logs.");
+ "Unable to migrate vm " + vm.getInstanceName() + " from " + fromHost + " in zone " + dest.getDataCenter().getName() + " and pod " +
Review comment:
We could use `String.format` here.
##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -4474,13 +4480,13 @@ private void orchestrateMigrateForScale(final String vmUuid, final long srcHostI
try {
_agentMgr.send(srcHostId, new Commands(cleanup(vm.getInstanceName())), null);
} catch (final AgentUnavailableException e) {
- s_logger.error("AgentUnavailableException while cleanup on source host: " + srcHostId);
+ s_logger.error(String.format("AgentUnavailableException while cleanup on source %s: ", srcHost), e);
Review comment:
As `AgentUnavailableException` will appears in stacktrace, we could improve this message with something more clean, e.g `Unable to cleanup source %s...`.
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2481,7 +2482,7 @@ public void deleteRoutingHost(final HostVO host, final boolean isForced, final b
try {
_vmMgr.destroy(vm.getUuid(), false);
} catch (final Exception e) {
- final String errorMsg = "There was an error Destory the vm: " + vm + " as a part of hostDelete id=" + host.getId();
+ String errorMsg = String.format("There was an error when destroying the VM: %s as a part of hostDelete: %s", vm, host);
Review comment:
Could you to take a look into the VM's `toString` implementation?
https://github.com/apache/cloudstack/blob/7835c0812062381c7e5dd5ab9d719297cd3187dc/engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java#L508-L510
Seems like there will be duplicated words:
`There was an error when destroying the VM: VM Instance...`
It would apply to every time we log the VM.
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2607,13 +2608,13 @@ protected void connectAndRestartAgentOnHost(HostVO host, String username, String
final com.trilead.ssh2.Connection connection = SSHCmdHelper.acquireAuthorizedConnection(
host.getPrivateIpAddress(), 22, username, password);
if (connection == null) {
- throw new CloudRuntimeException("SSH to agent is enabled, but failed to connect to host: " + host.getPrivateIpAddress());
+ throw new CloudRuntimeException(String.format("SSH to agent is enabled, but failed to connect to %s via IP address %s.", host, host.getPrivateIpAddress()));
Review comment:
I would rather use `[]` as var delimiter in some cases; IMHO, it is easier to recognize what is a variable and what is fixed in the final message, e.g.:
`SSH to agent is enabled, but failed to connect to %s via IP address [%s].`.
##########
File path: server/src/main/java/com/cloud/resource/RollingMaintenanceManagerImpl.java
##########
@@ -571,7 +571,7 @@ private void performPreFlightChecks(List<Host> hosts, int timeout, String payloa
for (Host host : hosts) {
Ternary<Boolean, String, Boolean> result = performStageOnHost(host, Stage.PreFlight, timeout, payload, forced);
if (result.third() && !hostsToAvoidMaintenance.containsKey(host.getId())) {
- s_logger.debug("Host " + host.getId() + " added to the avoid maintenance set");
+ s_logger.debug(String.format("%s added to the avoid maintenance set", host));
hostsToAvoidMaintenance.put(host.getId(), "Pre-flight stage set to avoid maintenance");
}
Review comment:
This code seems repeated, but changing parameters; We could extract it to a method.
##########
File path: server/src/main/java/com/cloud/resource/RollingMaintenanceManagerImpl.java
##########
@@ -439,7 +439,7 @@ private void enableClusterIfDisabled(Cluster cluster, Set<Long> disabledClusters
*/
private boolean isMaintenanceStageAvoided(Host host, Map<Long, String> hostsToAvoidMaintenance, List<HostSkipped> hostsSkipped) {
if (hostsToAvoidMaintenance.containsKey(host.getId())) {
- s_logger.debug("Host " + host.getId() + " is not being put into maintenance, skipping it");
+ s_logger.debug(String.format("%s is not being put into maintenance, skipping it", host));
Review comment:
I think this message could be more clear, e.g.:
`%s is in avoid maintenance list [%s], skipping its maintenance...`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-819060284
@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
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-837306345
@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] GutoVeronezi commented on a change in pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r662507006
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2451,11 +2452,11 @@ public HostVO fillRoutingHostVO(final HostVO host, final StartupRoutingCommand s
@Override
public void deleteRoutingHost(final HostVO host, final boolean isForced, final boolean forceDestroyStorage) throws UnableDeleteHostException {
if (host.getType() != Host.Type.Routing) {
- throw new CloudRuntimeException("Non-Routing host gets in deleteRoutingHost, id is " + host.getId());
+ throw new CloudRuntimeException(String.format("Non-Routing host gets in deleteRoutingHost, id is %s", host.getId()));
}
if (s_logger.isDebugEnabled()) {
- s_logger.debug("Deleting Host: " + host.getId() + " Guid:" + host.getGuid());
+ s_logger.debug(String.format("Deleting Host: %s", host));
Review comment:
Similar to VM's `toString` would occur here.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-837266814
Packaging result: :heavy_check_mark: centos7 :heavy_multiplication_x: centos8 :heavy_multiplication_x: debian. SL-JID 568
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-836518036
<b>Trillian Build Failed (tid-634)<b/>
--
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-834602744
Packaging result: :heavy_multiplication_x: centos7 :heavy_multiplication_x: centos8 :heavy_multiplication_x: debian. SL-JID 537
--
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] ravening commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-763473714
> @GabrielBrascher can we use something like `host-name(ID: host_uuid)`? Personally I found it easier at times to debug issues with uuid in logs
>
> @blueorangutan package
But this will display uuid to users also right which doesnt make sense to them
----------------------------------------------------------------
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-782089537
Applied changes in order to use the `toString()` method from HostVO. These changes consider the proposal from PR #4706 where `toString()` would present a "_JSON like"_ String.
- How it is today: `Host[-123-Routing]` (yep ... nowadays logs that use the toString have `-` before the ID)
- Proposal from #4706: `Host [{id: "123", name: "hostName", uuid: "1e374e50-g231-4345-d6h3-123abcdrfas143", type="Routing"}]`
The idea is that it would be good for:
1. Debug via DB filtering with the host ID
2. Run commands on API/cloudmonkey by copying the uuid presented at the log
3. Identify the host quickly as name makes logs human-friendly
4. Name also tends to be resolved to the host address; therefore, admins can jump into the host (SSH) by copying the name from the log.
Ping for discussion/review: @rafaelweingartner @sureshanaparti @shwstppr @RodrigoDLopez @DaanHoogland @rhtyd @wido @ustcweizhou @svenvogel @andrijapanicsb and others ...
----------------------------------------------------------------
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 #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r628269791
##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -5165,7 +5165,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
}
});
} catch (Exception e) {
- s_logger.warn("Unable to update vm disk statistics for vm: " + userVm.getId() + " from host: " + hostId, e);
+ s_logger.warn(String.format("Unable to update vm disk statistics for vm %s from %s", userVm.getInstanceName(), host), e);
Review comment:
Lowercase `vm`.
--
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 pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-868554297
@blueorangutan test matrix
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4575: Enhance log messages with host name
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#issuecomment-865416113
<b>Trillian test result (tid-1023)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 56119 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4575-t1023-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_nic.py
Intermittent failure detected: /marvin/tests/smoke/test_outofbandmanagement_nestedplugin.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 81 look OK, 7 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_nic | `Error` | 161.70 | test_nic.py
test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 213.73 | test_internal_lb.py
test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 213.74 | test_internal_lb.py
test_oobm_issue_power_cycle | `Error` | 2.38 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_off | `Error` | 3.52 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_on | `Error` | 3.60 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_reset | `Error` | 4.09 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_soft | `Error` | 4.60 | test_outofbandmanagement_nestedplugin.py
test_oobm_issue_power_status | `Error` | 3.58 | test_outofbandmanagement_nestedplugin.py
test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 344.04 | test_routers_network_ops.py
test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Failure` | 379.93 | test_routers_network_ops.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 533.25 | test_vpc_redundant.py
test_01_redundant_vpc_site2site_vpn | `Failure` | 377.51 | test_vpc_vpn.py
test_hostha_kvm_host_fencing | `Error` | 103.81 | 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