[GitHub] cloudstack pull request: CLOUDSTACK-8855 Improve Error Message for...

[rhtyd]

Github user rhtyd commented on the pull request:

https://github.com/apache/cloudstack/pull/837#issuecomment-216190239
  
@bvbharat please rebase against latest master and push -f, update on status 
of your PR

also, how do we test your changes


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8855 Improve Error Message for...

[swill]

Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/837#issuecomment-213430064
  
@bvbharatk no problem.  :)  Thanks for the continued effort testing.  I 
just wanted to make sure we understood what was going on.  


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8855 Improve Error Message for...

[bvbharatk]

Github user bvbharatk commented on the pull request:

https://github.com/apache/cloudstack/pull/837#issuecomment-213424108
  
@swill 

This is because of a problem with the CI environment. i will fix this and 
post the results again. Will remove these results for now. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8855 Improve Error Message for...

[swill]

Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/837#issuecomment-213422548
  
I am a bit concerned about this CI run.  There is a LOT failing which does 
not usually fail.  We will need to review the CI results and try to 
understand/fix the problems...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8855 Improve Error Message for...

[bvbharatk]

Github user bvbharatk commented on the pull request:

https://github.com/apache/cloudstack/pull/837#issuecomment-213395917
  
### ACS CI BVT Run
 **Sumarry:**
 Build Number 3
 Hypervisor xenserver
 NetworkType Advanced
 Passed=69
 Failed=19
 Skipped=3

_Link to logs Folder (search by build_no):_ 
https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0


**Failed tests:**
* test_vm_snapshots.py

 * test_01_create_vm_snapshots Failed

 * test_02_revert_vm_snapshots Failed

 * test_03_delete_vm_snapshots Failed

* test_guest_vlan_range.py

 * test_dedicateGuestVlanRange Failed

* test_scale_vm.py

 * test_02_scale_vm_without_hypervisor_specifics Failed

* test_service_offerings.py

 * test_04_change_offering_small Failed

* test_loadbalance.py

 * test_01_create_lb_rule_src_nat Failed

 * test_02_create_lb_rule_non_nat Failed

 * test_assign_and_removal_lb Failed

* test_deploy_vm_iso.py

 * test_deploy_vm_from_iso Failed

* test_volumes.py

 * test_01_create_volume Failed

 * test_02_attach_volume Failed

 * test_06_download_detached_volume Failed

* test_internal_lb.py

 * test_internallb Failed

* test_vm_life_cycle.py

 * test_10_attachAndDetach_iso Failed

* test_templates.py

 * test_01_create_template Failed

 * test_03_delete_template Failed

* test_iso.py

 * test_01_create_iso Failing since 2 runs

 * ContextSuite context=TestISO>:setup Failing since 2 runs


**Skipped tests:**
test_vm_nic_adapter_vmxnet3
test_deploy_vgpu_enabled_vm
test_06_copy_template

**Passed test suits:**
test_deploy_vm_with_userdata.py
test_portable_publicip.py
test_vpc_vpn.py
test_over_provisioning.py
test_global_settings.py
test_privategw_acl.py
test_routers.py
test_reset_vm_on_reboot.py
test_snapshots.py
test_deploy_vms_with_varied_deploymentplanners.py
test_non_contigiousvlan.py
test_public_ip_range.py
test_multipleips_per_nic.py
test_regions.py
test_affinity_groups.py
test_network_acl.py
test_pvlan.py
test_nic.py
test_deploy_vm_root_resize.py
test_resource_detail.py
test_secondary_storage.py
test_disk_offerings.py


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8855 Improve Error Message for...

[bvbharatk]

Github user bvbharatk commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/837#discussion_r60530964
  
--- Diff: server/src/com/cloud/resource/ResourceManagerImpl.java ---
@@ -1148,15 +1148,16 @@ public Host cancelMaintenance(final 
CancelMaintenanceCmd cmd) {
 }
 
 @Override
-public Host reconnectHost(final ReconnectHostCmd cmd) {
-final Long hostId = cmd.getId();
+public Host reconnectHost(ReconnectHostCmd cmd) throws 
CloudRuntimeException, AgentUnavailableException{
--- End diff --

@rodrigo93 

 The scope of this pr is to improve the exception handling and error 
messaging. Changing the method signature is not a immediate concern. I will 
make the suggested changes in another pr if i find time. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8855 Improve Error Message for...

[bvbharatk]

Github user bvbharatk commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/837#discussion_r60529246
  
--- Diff: 
engine/orchestration/src/com/cloud/agent/manager/AgentManagerImpl.java ---
@@ -971,33 +971,28 @@ public Answer easySend(final Long hostId, final 
Command cmd) {
 }
 
 @Override
-public boolean reconnect(final long hostId) {
+public void reconnect(final long hostId) throws CloudRuntimeException, 
AgentUnavailableException{
 HostVO host;
 
 host = _hostDao.findById(hostId);
 if (host == null || host.getRemoved() != null) {
-s_logger.warn("Unable to find host " + hostId);
-return false;
+throw new CloudRuntimeException("Unable to find host " + 
hostId);
 }
 
 if (host.getStatus() == Status.Disconnected) {
-s_logger.info("Host is already disconnected, no work to be 
done");
-return true;
+throw new CloudRuntimeException("Host is already disconnected, 
no work to be done");
 }
 
 if (host.getStatus() != Status.Up && host.getStatus() != 
Status.Alert && host.getStatus() != Status.Rebalancing) {
-s_logger.info("Unable to disconnect host because it is not in 
the correct state: host=" + hostId + "; Status=" + host.getStatus());
-return false;
+throw  new CloudRuntimeException("Unable to disconnect host 
because it is not in the correct state: host=" + hostId + "; Status=" + 
host.getStatus());
 }
 
 final AgentAttache attache = findAttache(hostId);
 if (attache == null) {
-s_logger.info("Unable to disconnect host because it is not 
connected to this server: " + hostId);
-return false;
+throw new CloudRuntimeException("Unable to disconnect host 
because it is not connected to this server: " + hostId);
 }
 
 disconnectWithoutInvestigation(attache, Event.ShutdownRequested);
-return true;
 }
 
 public boolean executeUserRequest(final long hostId, final Event 
event) throws AgentUnavailableException {
--- End diff --

@rodrigo93 
This method is already a void. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8855 Improve Error Message for...

[rodrigo93]

Github user rodrigo93 commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/837#discussion_r52093680
  
--- Diff: 
engine/orchestration/src/com/cloud/agent/manager/AgentManagerImpl.java ---
@@ -971,33 +971,28 @@ public Answer easySend(final Long hostId, final 
Command cmd) {
 }
 
 @Override
-public boolean reconnect(final long hostId) {
+public void reconnect(final long hostId) throws CloudRuntimeException, 
AgentUnavailableException{
 HostVO host;
 
 host = _hostDao.findById(hostId);
 if (host == null || host.getRemoved() != null) {
-s_logger.warn("Unable to find host " + hostId);
-return false;
+throw new CloudRuntimeException("Unable to find host " + 
hostId);
 }
 
 if (host.getStatus() == Status.Disconnected) {
-s_logger.info("Host is already disconnected, no work to be 
done");
-return true;
+throw new CloudRuntimeException("Host is already disconnected, 
no work to be done");
 }
 
 if (host.getStatus() != Status.Up && host.getStatus() != 
Status.Alert && host.getStatus() != Status.Rebalancing) {
-s_logger.info("Unable to disconnect host because it is not in 
the correct state: host=" + hostId + "; Status=" + host.getStatus());
-return false;
+throw  new CloudRuntimeException("Unable to disconnect host 
because it is not in the correct state: host=" + hostId + "; Status=" + 
host.getStatus());
 }
 
 final AgentAttache attache = findAttache(hostId);
 if (attache == null) {
-s_logger.info("Unable to disconnect host because it is not 
connected to this server: " + hostId);
-return false;
+throw new CloudRuntimeException("Unable to disconnect host 
because it is not connected to this server: " + hostId);
 }
 
 disconnectWithoutInvestigation(attache, Event.ShutdownRequested);
-return true;
 }
 
 public boolean executeUserRequest(final long hostId, final Event 
event) throws AgentUnavailableException {
--- End diff --

Couldn't this method be void as well and throw an exception if the 
execution fails?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8855 Improve Error Message for...

[rodrigo93]

Github user rodrigo93 commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/837#discussion_r52094072
  
--- Diff: server/src/com/cloud/resource/ResourceManagerImpl.java ---
@@ -1148,15 +1148,16 @@ public Host cancelMaintenance(final 
CancelMaintenanceCmd cmd) {
 }
 
 @Override
-public Host reconnectHost(final ReconnectHostCmd cmd) {
-final Long hostId = cmd.getId();
+public Host reconnectHost(ReconnectHostCmd cmd) throws 
CloudRuntimeException, AgentUnavailableException{
--- End diff --

Does this method need to return a host? It seems that it just reconnects to 
a host, if not, throws an exception.
If so, it could be void as well.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8855 Improve Error Message for...

[bvbharatk]

GitHub user bvbharatk opened a pull request:

https://github.com/apache/cloudstack/pull/838

CLOUDSTACK-8855 Improve Error Message for Host Alert State and reconnect 
host API.

earlier we were eating up exceptions from the lower layer which resulted in 
improper error messages. fixed partially by throwing exceptions and catching 
them at the appropriate layer.

This also fixes the host alters. Earlier in the host alerts window we are 
listing only the alters, this dose not tell why the host when to alert state. 
This is fixed by adding the alert message to the host alerts.
(changes in AlertManagerImpl.java). 

tested manually.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/bvbharatk/cloudstack CLOUDSTACK-8857

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/838.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #838


commit 00bc60025e578e223613119690fae62da84ffdbf
Author: Bharat Kumar 
Date:   2015-02-19T05:10:16Z

CLOUDSTACK-8857 listProjects doesn't return tags vmstopped or vmrunning 
when their value is zero




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8855 Improve Error Message for...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/838#issuecomment-14064
  
Which steps should we follow to test it, @bvbharatk ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8855 Improve Error Message for...

[bvbharatk]

GitHub user bvbharatk opened a pull request:

https://github.com/apache/cloudstack/pull/837

CLOUDSTACK-8855 Improve Error Message for Host Alert State and reconnect 
host API.

earlier we were eating up exceptions from the lower layer which resulted in 
improper error messages. fixed partially by throwing exceptions and catching 
them at the appropriate layer.

This also fixes the host alters. Earlier in the host alerts window we are 
listing only the alters, this dose not tell why the host when to alert state. 
This is fixed by adding the alert message to the host alerts.
(changes in AlertManagerImpl.java). 

tested manually.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/bvbharatk/cloudstack CLOUDSTACK-8855

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/837.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #837


commit 7920ecfbbb92336bafd0804d1db244d40b9c4852
Author: Bharat Kumar 
Date:   2015-04-07T06:15:13Z

CLOUDSTACK-8855 Improve Error Message for Host Alert State




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8855 Improve Error Message for...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/837#issuecomment-140645828
  
Why do we have 2 PR with the same issue/branch/description, @bvbharatk ? I 
check the PR #838 and that's about something else. Could you please edit the PR 
description?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8855 Improve Error Message for...

[bvbharatk]

Github user bvbharatk commented on the pull request:

https://github.com/apache/cloudstack/pull/837#issuecomment-140646781
  
@wilderrodrigues 
my bad, something went wrong while creating the pr. fixed this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8855 Improve Error Message for...

[remibergsma]

Github user remibergsma commented on the pull request:

https://github.com/apache/cloudstack/pull/837#issuecomment-140745806
  
@bvbharatk Any advise on how to test this?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---