[GitHub] cloudstack pull request: CLOUDSTACK-9350: KVM-HA- Fix CheckOnHost ...

2016-04-17 Thread alexandrelimassantana 
Github user alexandrelimassantana commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1496#discussion_r60004052
  
--- Diff: server/src/com/cloud/ha/HighAvailabilityManagerImpl.java ---
@@ -264,6 +265,11 @@ public void scheduleRestartForVmsOnHost(final HostVO 
host, boolean investigate)
 "Host [" + hostDesc + "] is down." + ((sb != null) ? 
sb.toString() : ""));
 
 for (VMInstanceVO vm : reorderedVMList) {
+ServiceOfferingVO vmOffering = 
_serviceOfferingDao.findById(vm.getServiceOfferingId());
--- End diff --

I would recommend putting this changed inside the if scope in line 273. The 
additions are related to debug and if the debug is disabled, you can skip it.


---
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: Fixes regarding VOLUME_DELETE events resu...

2016-04-17 Thread alexandrelimassantana 
Github user alexandrelimassantana commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1491#discussion_r60003773
  
--- Diff: server/src/com/cloud/user/AccountManagerImpl.java ---
@@ -677,6 +679,17 @@ public boolean deleteAccount(AccountVO account, long 
callerUserId, Account calle
 return cleanupAccount(account, callerUserId, caller);
 }
 
+protected List getExpungedInstanceRootVolume(long 
instanceId) {
+SearchBuilder sb = _volumeDao.createSearchBuilder();
+sb.and("instanceId", sb.entity().getInstanceId(), 
SearchCriteria.Op.EQ);
+sb.and("vType", sb.entity().getVolumeType(), SearchCriteria.Op.EQ);
+sb.done();
+SearchCriteria c = sb.create();
+c.setParameters("instanceId", instanceId);
+c.setParameters("vType", Volume.Type.ROOT);
+return _volumeDao.customSearchIncludingRemoved(c, null);
+}
+
 protected boolean cleanupAccount(AccountVO account, long callerUserId, 
Account caller) {
--- End diff --

I think it is missing a simple unit tests as a test if the method 
expungedInstaceRootVolume is called when the cleanupAccount is invoked and 
there is a vm in destroyed state. That and the guarantee that the method 
getExpungedInstanceRootVolume works, added to the tests you made would complete 
it.


---
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: Undetected bug correct and refactor of th...

2016-04-17 Thread alexandrelimassantana 
GitHub user alexandrelimassantana opened a pull request:

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

Undetected bug correct and refactor of the class NfsSecondaryStorageResource

Im in process of rewriting the unit test for this class' mount method. 
Before I can do that, the class needed some refactoring, not just because it 
was fuzzy but because it was impossible to use mocks the way it was coded. Most 
of this PR is about transforming big chunks of code into documented methods 
with two exceptions:

1) I inverted the logic when checking for existing mounts within the same 
root location. Previously one would first create a directory if it didn't exist 
and then test if there was already mount there. Now it is first testing if 
there is a mount, if not, it creates the needed directory.

2) A bug was found in the unmount method. The unmount wasn't issuing the 
command "umount" to unmount a resource but instead was trying the "mount" 
command again, resulting in mounted resources never being unmounted.

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

$ git pull https://github.com/rafaelweingartner/cloudstack 
lrg-cs-hackday-046

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

https://github.com/apache/cloudstack/pull/1499.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 #1499


commit d85810d122ec9f5c697ad77f6a5422e50ac718da
Author: Alexandre Limas Santana 
Date:   2016-04-16T20:49:49Z

Refactor of the class NfsSecondaryStorageResource withing the domain of
the mount process. A bug was found and corrected in the function unmount




---
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-9350: KVM-HA- Fix CheckOnHost ...

2016-04-17 Thread cristofolini 
Github user cristofolini commented on the pull request:

https://github.com/apache/cloudstack/pull/1496#issuecomment-211149034
  
@abhinandanprateek How about extracting lines 70-82 in `KVMInvestigator` to 
their own method? I think it would be nice to get a test case going for that 
section.


---
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: Fixes regarding VOLUME_DELETE events resu...

2016-04-17 Thread cristofolini 
Github user cristofolini commented on the pull request:

https://github.com/apache/cloudstack/pull/1491#issuecomment-211140525
  
@ProjectMoon May I suggest extracting lines 778-784 in `AccountManagerImpl` 
to a separate method? That would allow you to write your comment in a proper 
javadoc and set up a test case targeted at that change.


---
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-9322: Support for Internal LB ...

2016-04-17 Thread prashanthvarma 
Github user prashanthvarma commented on the pull request:

https://github.com/apache/cloudstack/pull/1452#issuecomment-26802
  
@DaanHoogland That's a good suggestion. We will discuss on the optimal 
location in the plugin's project directory, and incorporate them in the next PR.

Thank you for the review and suggestions !!


---
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-9322: Support for Internal LB ...

2016-04-17 Thread DaanHoogland 
Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1452#issuecomment-211099345
  
@swill LGTM based on testing and code walkthrough


---
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-9322: Support for Internal LB ...

2016-04-17 Thread DaanHoogland 
Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1452#issuecomment-211099195
  
@prashanthvarma maybe you can incorporate it in the project dir for the 
plugin somehow. It seems to me to be the best place for it. maybe accompanied 
with a readme on how to install/test?


---
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: Strongswan vpn feature

2016-04-17 Thread remibergsma 
Github user remibergsma commented on the pull request:

https://github.com/apache/cloudstack/pull/872#issuecomment-211094168
  
@jayapalu The strongswan service is now started automatically on all 
systemvms (including console proxy and seconday storage vm). I think we should 
start it only on-demand? Currently testing it, will take some more time but 
I'll report back. Thanks!


---
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-9322: Support for Internal LB ...

2016-04-17 Thread prashanthvarma 
Github user prashanthvarma commented on the pull request:

https://github.com/apache/cloudstack/pull/1452#issuecomment-211092880
  
@DaanHoogland you are right, that is the main motive :). Let me know, if 
you want me to publish it elsewhere as well for easier user access.

Thank you !!


---
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-9322: Support for Internal LB ...

2016-04-17 Thread DaanHoogland 
Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1452#issuecomment-211060040
  
thanks @prashanthvarma , mostly usefull for users of your sdn solution. 
Hopefully some of them will test ;)


---
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-9322: Support for Internal LB ...

2016-04-17 Thread prashanthvarma 
Github user prashanthvarma commented on the pull request:

https://github.com/apache/cloudstack/pull/1452#issuecomment-211044426
  
@DaanHoogland Sure, here is an example Nuage VSP SDN plugin specific Marvin 
tests config file contents:

{
"zones": [
{
"name": "MyZone",
"guestcidraddress": "10.1.1.0/24", 
"dns2": "8.8.6.6", 
"dns1": "10.10.0.10", 
"physical_networks": [
{
"broadcastdomainrange": "Zone", 
"isolationmethods": [
"VLAN"
], 
"name": "Management Network", 
"traffictypes": [
{
"kvm": "mgmtbr0", 
"typ": "Management"
}, 
{
"kvm": "mgmtbr0", 
"typ": "Public"
}, 
{
"kvm": "mgmtbr0", 
"typ": "Storage"
}
], 
"providers": [
{
"broadcastdomainrange": "ZONE", 
"name": "VirtualRouter"
}
]
}, 
{
"broadcastdomainrange": "Zone", 
"isolationmethods": [
"VSP"
], 
"name": "Nuage Network", 
"traffictypes": [
{
"kvm": "alubr0", 
"typ": "Guest"
}
], 
"providers": [
{
"broadcastdomainrange": "ZONE", 
"name": "VirtualRouter"
}, 
{
"broadcastdomainrange": "ZONE", 
"name": "NuageVsp", 
"devices": [
{
"username": "csproot", 
"retryinterval": "60", 
"hostname": "10.30.35.129",
"apiversion": "v3_2", 
"retrycount": "4", 
"password": "csproot", 
"port": 8443
}
]
}, 
{
"broadcastdomainrange": "ZONE", 
"name": "VpcVirtualRouter"
}, 
{
"broadcastdomainrange": "ZONE", 
"name": "InternalLbVm"
}
]
}
], 
"securitygroupenabled": "false", 
"ipranges": [
{
"startip": "10.29.1.1", 
"endip": "10.29.9.255", 
"netmask": "255.255.0.0", 
"gateway": "10.29.0.1"
}
], 
"networktype": "Advanced", 
"pods": [
{
"endip": "10.20.15.255", 
"name": "P0", 
"startip": "10.20.8.0", 
"netmask": "255.255.0.0", 
"clusters": [
{
"clustername": "P0C0", 
"hypervisor": "kvm", 
"hosts": [
{
"username": "root", 
"url": "http://test-kvm;, 
"password": "password"
}
], 
"clustertype": "CloudManaged", 
"primaryStorages": [
{
"url": "nfs://10.20.128.14/primary", 
"name": "P0C0-primaryStorage"
}
]
}

[GitHub] cloudstack pull request: CLOUDSTACK-9322: Support for Internal LB ...

2016-04-17 Thread DaanHoogland 
Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1452#issuecomment-211038147
  
@prashanthvarma  can you publish nuage_ant.cfg?


---
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-9322: Support for Internal LB ...

2016-04-17 Thread DaanHoogland 
Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1452#issuecomment-211032986
  
did the tests and again two failures, replay succeeded


[1452.results.network.txt](https://github.com/apache/cloudstack/files/222681/1452.results.network.txt)

[1452.results.vpc_routers.txt](https://github.com/apache/cloudstack/files/222683/1452.results.vpc_routers.txt)


[1452.results.routers_network_ops.txt](https://github.com/apache/cloudstack/files/222682/1452.results.routers_network_ops.txt)

```
# ssh 192.168.23.5
The authenticity of host '192.168.23.5 (192.168.23.5)' can't be established.
ECDSA key fingerprint is a6:38:aa:5b:07:f9:53:8c:c0:57:19:c7:8f:e4:f1:a7.
Are you sure you want to continue connecting (yes/no)? yes
Warning: Permanently added '192.168.23.5' (ECDSA) to the list of known 
hosts.
root@192.168.23.5's password: 
# ping -c 3 8.8.8.8
PING 8.8.8.8 (8.8.8.8): 56 data bytes
64 bytes from 8.8.8.8: seq=0 ttl=46 time=24.347 ms
64 bytes from 8.8.8.8: seq=1 ttl=46 time=23.389 ms
64 bytes from 8.8.8.8: seq=2 ttl=46 time=26.818 ms

--- 8.8.8.8 ping statistics ---
3 packets transmitted, 3 packets received, 0% packet loss
round-trip min/avg/max = 23.389/24.851/26.818 ms
```


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