[GitHub] cloudstack pull request: CLOUDSTACK-8648: Do not configure the Ima...

[karuturi]

Github user karuturi commented on the pull request:

https://github.com/apache/cloudstack/pull/606#issuecomment-123585585
  
Yes. Copy paste error :)

-Original Message-
From: "Wido den Hollander" 
Sent: ‎21-‎07-‎2015 21:19
To: "apache/cloudstack" 
Cc: "Rajani Karuturi" 
Subject: Re: [cloudstack] CLOUDSTACK-8648: Do not configure the ImageFormat 
Processor when fetc… (#606)

Shouldn't we then pass "params" to the configure method? That seems to be 
missing in your example.
—
Reply to this email directly or view it on GitHub.


---
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-8650: Fix securitygroups ingre...

[resmo]

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

https://github.com/apache/cloudstack/pull/601#discussion_r35187102
  
--- Diff: scripts/vm/network/security_group.py ---
@@ -860,8 +860,10 @@ def add_network_rules(vm_name, vm_id, vm_ip, 
signature, seqno, vmMac, rules, vif
 for ip in ips:
 execute("iptables -I " + vmchain + " -p icmp 
--icmp-type " + range + " " + direction + " " + ip + " -j "+ action)
 
-if allow_any and protocol != 'all':
-if protocol != 'icmp':
+if allow_any
--- End diff --

missing ":"


---
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-8659: Verify presentation of v...

[pritisarap12]

GitHub user pritisarap12 opened a pull request:

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

CLOUDSTACK-8659: Verify presentation of volume ID in events table

Verify if events table records UUID of the volume instead of volume ID in 
description from which snapshot is created for 'SNAPSHOT.CREATE' type .

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

$ git pull https://github.com/pritisarap12/cloudstack 
CLOUDSTACK-8659-Verify-presentation-of-volume-id-in-description-of-events-table-for-SNAPSHOT.CREATE-type

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

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


commit a634ad2e3dcbc1ec668fdc135db9891717bf8a7d
Author: Priti Sarap 
Date:   2015-07-22T07:24:46Z

CLOUDSTACK-8659: Verify presentation of volume id in description of events 
table for 'SNAPSHOT.CREATE' type




---
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-8596 ability to query nested g...

[DaanHoogland]

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

https://github.com/apache/cloudstack/pull/609#discussion_r35188707
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java
 ---
@@ -57,17 +57,22 @@
 private LdapContextFactory _ldapContextFactory;
 
 @Inject
-private LdapUserManager _ldapUserManager;
+private LdapConfiguration _ldapConfiguration;
+
+@Inject LdapUserManagerFactory _ldapUserManagerFactory;
+
 
 public LdapManagerImpl() {
 super();
 }
 
-public LdapManagerImpl(final LdapConfigurationDao 
ldapConfigurationDao, final LdapContextFactory ldapContextFactory, final 
LdapUserManager ldapUserManager) {
+public LdapManagerImpl(final LdapConfigurationDao 
ldapConfigurationDao, final LdapContextFactory ldapContextFactory, final 
LdapUserManagerFactory ldapUserManagerFactory,
+   final LdapConfiguration ldapConfiguration) {
--- End diff --

And why not use injection for that as well? Is this a groovy thing?


---
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-8658: make initializer static ...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/612#issuecomment-123609068
  
LGTM :+1:  merging...

Thanks, @DaanHoogland !!!

Tested with:

KVM + Qemu on Centos7
Management Server on Centos7
Agent 4.6.0 (I built myself)
SystemVM 4.6.0 (latest from jenkins.bac.o

Test advanced zone virtual router ... === TestName: 
test_advZoneVirtualRouter | Status : SUCCESS ===
ok
Test Deploy Virtual Machine ... === TestName: test_deploy_vm | Status : 
SUCCESS ===
ok
Test Multiple Deploy Virtual Machine ... === TestName: 
test_deploy_vm_multiple | Status : SUCCESS ===
ok
Test Stop Virtual Machine ... === TestName: test_01_stop_vm | Status : 
SUCCESS ===
ok
Test Start Virtual Machine ... === TestName: test_02_start_vm | Status : 
SUCCESS ===
ok
Test Reboot Virtual Machine ... === TestName: test_03_reboot_vm | Status : 
SUCCESS ===
ok
Test destroy Virtual Machine ... === TestName: test_06_destroy_vm | Status 
: SUCCESS ===
ok
Test recover Virtual Machine ... === TestName: test_07_restore_vm | Status 
: SUCCESS ===
ok
Test migrate VM ... SKIP: At least two hosts should be present in the zone 
for migration
Test destroy(expunge) Virtual Machine ... === TestName: test_09_expunge_vm 
| Status : SUCCESS ===
ok

--
Ran 10 tests in 2732.895s

OK (SKIP=1)
/tmp//MarvinLogs/test_vm_life_cycle_EW6E00/results.txt (END)


---
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-8658: make initializer static ...

[asfgit]

Github user asfgit closed the pull request at:

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


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

Re: [MASTER] ID RSA pub too open and asking passprrase

[Wilder Rodrigues]

After 3 days we managed to find the problem.

It has been fixed and pushed towards master. KVM is back on business!

Details here: https://github.com/apache/cloudstack/pull/612

Cheers,
Wilder

On 20 Jul 2015, at 15:51, Wilder Rodrigues 
mailto:wrodrig...@schubergphilis.com>> wrote:

Hi Wido,

I’m doing a regression test on it in order to find out when it broke. I started 
getting my PR from 27th May, which introduced the big Libvirt refactor. It 
works fine… like a charm:

[root@kvm1 ~]# ls -lart ~/.ssh/
total 16
dr-xr-x---. 3 root root 4096 Jul 20 09:35 ..
drwx--. 2 root root 4096 Jul 20 09:35 .
-rw-r--r--. 1 root root  389 Jul 20 09:35 id_rsa.pub.cloud
-rw---. 1 root root 1674 Jul 20 09:35 id_rsa.cloud
[root@kvm1 ~]# ssh -i ~/.ssh/id_rsa.cloud -p 3922 169.254.0.191
The authenticity of host '[169.254.0.191]:3922 ([169.254.0.191]:3922)' can't be 
established.
ECDSA key fingerprint is 0a:36:fe:c4:08:ce:2b:46:47:22:ee:f4:1a:fc:e2:88.
Are you sure you want to continue connecting (yes/no)? yes
Warning: Permanently added '[169.254.0.191]:3922' (ECDSA) to the list of known 
hosts.
Linux s-1-VM 3.2.0-4-amd64 #1 SMP Debian 3.2.68-1+deb7u2 x86_64

The programs included with the Debian GNU/Linux system are free software;
the exact distribution terms for each program are described in the
individual files in /usr/share/doc/*/copyright.

Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
permitted by applicable law.
Last login: Sun Jul 19 23:19:41 2015 from 10.0.2.2

The programs included with the Debian GNU/Linux system are free software;
the exact distribution terms for each program are described in the
individual files in /usr/share/doc/*/copyright.

Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
permitted by applicable law.
root@s-1-VM:~#



I will now move on to a PR that was merged 25 days ago. I will keep you in the 
loop.

Btw, I’m taking my PRs first just as a sanity check, because I do remember 
using KVM just fine about 3 weeks ago. Hope I find the root cause today.


Cheers,
Wilder


On 17 Jul 2015, at 17:43, Wilder Rodrigues 
mailto:wrodrig...@schubergphilis.com>>
 wrote:

Could you please ask him, Wido?

I will look into it again tomorrow and get it fixed!

Thanks for the reply!

Cheers,
Wilder

Sent from my iPhone

On 17 Jul 2015, at 15:19, Wido den Hollander 
mailto:w...@widodh.nl>> wrote:



On 17-07-15 13:53, Wilder Rodrigues wrote:
Hi again,

I just cleaned up the whole KVM host, also removing the .ssh/ dir contents and 
deployed a new DC. The private key is not created anymore, only the pub key:

[root@kvm1 ~]# ls -lart .ssh/
total 8
dr-xr-x---. 4 root root 4096 Jul 17 06:08 ..
drwx--. 2 root root 4096 Jul 17 07:38 .
-rw-r--r--. 1 root root0 Jul 17 07:38 id_rsa.pub.cloud
[root@kvm1 ~]# ssh -i ~/.ssh/id_rsa.pub.cloud -p 3922 169.254.0.100
The authenticity of host '[169.254.0.100]:3922 ([169.254.0.100]:3922)' can't be 
established.
ECDSA key fingerprint is 81:be:00:fe:37:8d:3f:99:63:1d:e2:ff:3f:4b:56:73.
Are you sure you want to continue connecting (yes/no)? yes
Warning: Permanently added '[169.254.0.100]:3922' (ECDSA) to the list of known 
hosts.
@@@
@ WARNING: UNPROTECTED PRIVATE KEY FILE!  @
@@@
Permissions 0644 for '/root/.ssh/id_rsa.pub.cloud' are too open.
It is required that your private key files are NOT accessible by others.
This private key will be ignored.
bad permissions: ignore key: /root/.ssh/id_rsa.pub.cloud
Permission denied (publickey).
[root@kvm1 ~]#


Any thoughts?

No, not really. I do know that my colleague Boris faced the same with
deploying from master. Don't know if he actually got it fixed.

Wido

Cheers,
Wilder


On 17 Jul 2015, at 13:33, Wilder Rodrigues 
mailto:wrodrig...@schubergphilis.com>>
 wrote:

Hi all,

I’m nt able to use the id_rsa.pub.cloud on KVM hosts. See snippet bellow:


[root@kvm1 ~]# ssh -i ~/.ssh/id_rsa.pub.cloud -p 3922 169.254.0.136
The authenticity of host '[169.254.0.136]:3922 ([169.254.0.136]:3922)' can't be 
established.
ECDSA key fingerprint is 81:be:00:fe:37:8d:3f:99:63:1d:e2:ff:3f:4b:56:73.
Are you sure you want to continue connecting (yes/no)? yes
Warning: Permanently added '[169.254.0.136]:3922' (ECDSA) to the list of known 
hosts.
@@@
@ WARNING: UNPROTECTED PRIVATE KEY FILE!  @
@@@
Permissions 0644 for '/root/.ssh/id_rsa.pub.cloud' are too open.
It is required that your private key files are NOT accessible by others.
This private key will be ignored.
bad permissions: ignore key: /root/.ssh/id_rsa.pub.cloud
Permission denied (publickey).
[root@kvm1 ~]# chmod 600 /root/.ssh/id_rsa.pub.cloud
[root@kvm1 ~]# ssh -i

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

[karuturi]

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

https://github.com/apache/cloudstack/pull/609#discussion_r35191354
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java
 ---
@@ -57,17 +57,22 @@
 private LdapContextFactory _ldapContextFactory;
 
 @Inject
-private LdapUserManager _ldapUserManager;
+private LdapConfiguration _ldapConfiguration;
+
+@Inject LdapUserManagerFactory _ldapUserManagerFactory;
+
 
 public LdapManagerImpl() {
 super();
 }
 
-public LdapManagerImpl(final LdapConfigurationDao 
ldapConfigurationDao, final LdapContextFactory ldapContextFactory, final 
LdapUserManager ldapUserManager) {
+public LdapManagerImpl(final LdapConfigurationDao 
ldapConfigurationDao, final LdapContextFactory ldapContextFactory, final 
LdapUserManagerFactory ldapUserManagerFactory,
+   final LdapConfiguration ldapConfiguration) {
--- End diff --

It was initially done that way. We could inject as well. I did that in the 
new class.

-Original Message-
From: "Daan Hoogland" 
Sent: ‎22-‎07-‎2015 13:18
To: "apache/cloudstack" 
Cc: "Rajani Karuturi" 
Subject: Re: [cloudstack] CLOUDSTACK-8596 ability to query nested groups 
forMicrosoft AD (#609)

In 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java:
>  
>  public LdapManagerImpl() {
>  super();
>  }
>  
> -public LdapManagerImpl(final LdapConfigurationDao 
ldapConfigurationDao, final LdapContextFactory ldapContextFactory, final 
LdapUserManager ldapUserManager) {
> +public LdapManagerImpl(final LdapConfigurationDao 
ldapConfigurationDao, final LdapContextFactory ldapContextFactory, final 
LdapUserManagerFactory ldapUserManagerFactory,
> +   final LdapConfiguration ldapConfiguration) {
And why not use injection for that as well? Is this a groovy thing?
—
Reply to this email directly or view it on GitHub.


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

[Proposal] CoreOS support

[Kishan Kavala]

I would like to add CoreOS as a supported OS type to cloudstack.
CoreOS vms can be created even now by selecting OS type as "Other". There are 
cloud providers supporting CoreOS offerings in production already.
By making the changes mentioned at [1] I would make it easier to deploy CoreOS 
Vms and also add UI support for providing cloud-config via userdata.
I'll also update the documentation at [2] after making these changes.

[1] https://cwiki.apache.org/confluence/display/CLOUDSTACK/CoreOS
[2] https://coreos.com/os/docs/latest/booting-on-cloudstack.html

[GitHub] cloudstack pull request: CLOUDSTACK-8655: [Browser Based Upload Vo...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/611#issuecomment-123636384
  
LGTM


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

RE: [PROPOSAL] drop old upgrade code

[Koushik Das]

-1 to dropping pre-4.x upgrade code. If possible we should suppress the old 
upgrade files from coverity scan.

Reasons:
There may be users on pre-4.x versions.
Removing a functionality should be associated with proper documentation and an 
advanced notification in some prior releases. This is similar to the way some 
API is deprecated and then eventually removed.

-Koushik  

-Original Message-
From: Daan Hoogland [mailto:daan.hoogl...@gmail.com] 
Sent: Monday, 20 July 2015 17:19
To: dev
Subject: [PROPOSAL] drop old upgrade code

LS,

In coverity the only remaining high impact issues are concerned with upgrade 
code. Some of it is in 4.3 and 4.5 code but most in pre-4 upgrades.

I addressed the file Upgrade218to22.java in a PR [1] and I move that we don't 
pull it but instead drop the file altogether together with all upgrade code 
dating prior to 4.0.0. anybody on older versions can still upgrade to any 
version between 4.0 and 4.5 and move on from there.

My objective is to have no high impact issues remaining so we clearly see when 
we are digressing beit by hand or in  an automated way.

+1?

[1] https://github.com/apache/cloudstack/pull/603
--
Daan

[GitHub] cloudstack pull request: CLOUDSTACK-8649: Fixed unnecessary double...

[borisroman]

GitHub user borisroman opened a pull request:

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

CLOUDSTACK-8649: Fixed unnecessary double url decoding in 
registerSSHKeyPair.

The method cmd.getPublicKey() already returns a decoded string. So when we 
try to decode it again the plus "+" signs are being decoded to white spaces. In 
a next step the key is cut at the spaces and only the first two parts are being 
saved in the DB.

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

$ git pull https://github.com/borisroman/cloudstack CLOUDSTACK-8649

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

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


commit 12a3642df683539fddb3f6be4b223b03744696e2
Author: Boris Schrijver 
Date:   2015-07-22T09:39:19Z

CLOUDSTACK-8649: Fixed unnecessary double url decoding in 
registerSSHKeyPair.




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

RE: [PROPOSAL] drop old upgrade code

[Boris Schrijver]

+1 on dropping the pre-4.x upgrade code, if done in a documented manner. Instead
of voting to drop it now shall we vote to drop it in a future release with
documentation and put it on the roadmap? Like:

At release 4.6: Initial notice to drop pre-4.x upgrade code at release 5.0.
At release 4.6: Suppress pre-4.x upgrade code from coverity scan.
At release 5.0: Drop pre-4.x upgrade code entirely.
At release 5.0: Create documentation to show upgrade path from pre-4.x to 5.0.

Best regards,

Boris Schrijver

> 
> On July 22, 2015 at 11:42 AM Koushik Das  wrote:
> 
> 
> -1 to dropping pre-4.x upgrade code. If possible we should suppress the
> old upgrade files from coverity scan.
> 
> Reasons:
> There may be users on pre-4.x versions.
> Removing a functionality should be associated with proper documentation
> and an advanced notification in some prior releases. This is similar to the
> way some API is deprecated and then eventually removed.
> 
> -Koushik
> 
> -Original Message-
> From: Daan Hoogland [mailto:daan.hoogl...@gmail.com]
> Sent: Monday, 20 July 2015 17:19
> To: dev
> Subject: [PROPOSAL] drop old upgrade code
> 
> LS,
> 
> In coverity the only remaining high impact issues are concerned with
> upgrade code. Some of it is in 4.3 and 4.5 code but most in pre-4 upgrades.
> 
> I addressed the file Upgrade218to22.java in a PR [1] and I move that we
> don't pull it but instead drop the file altogether together with all upgrade
> code dating prior to 4.0.0. anybody on older versions can still upgrade to any
> version between 4.0 and 4.5 and move on from there.
> 
> My objective is to have no high impact issues remaining so we clearly see
> when we are digressing beit by hand or in an automated way.
> 
> +1?
> 
> [1] https://github.com/apache/cloudstack/pull/603
> --
> Daan
> 

[GitHub] cloudstack pull request: CLOUDSTACK-8659: Verify presentation of v...

[nitt10prashant]

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

https://github.com/apache/cloudstack/pull/613#discussion_r35197771
  
--- Diff: test/integration/testpaths/testpath_uuid_event.py ---
@@ -0,0 +1,198 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+""" Test cases to verify presentation of volume id in events table 
+for 'SNAPSHOT.CREATE' type.
+"""
+
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase
+from marvin.lib.utils import (cleanup_resources,
+ validateList)
+from marvin.lib.base import (Account,
+ ServiceOffering,
+ Snapshot,
+ VirtualMachine,
+Configurations
+ )
+from marvin.lib.common import (get_domain,
+   get_zone,
+   get_template,
+   list_volumes,
+  )
+
+from marvin.codes import PASS
+
+
+
+
+class TestVerifyEventsTable(cloudstackTestCase):
+
+@classmethod
+def setUpClass(cls):
+testClient = super(TestVerifyEventsTable, cls).getClsTestClient()
+cls.apiclient = testClient.getApiClient()
+cls.testdata = testClient.getParsedTestDataConfig()
+
+cls.hypervisor = cls.testClient.getHypervisorInfo()
+# Get Zone, Domain and templates
+cls.domain = get_domain(cls.apiclient)
+cls.zone = get_zone(cls.apiclient, testClient.getZoneForTests())
+
+cls.template = get_template(
+cls.apiclient,
+cls.zone.id,
+cls.testdata["ostype"])
+
+cls._cleanup = []
+
+try:
+
+cls.unsupportedHypervisor = False
+if cls.hypervisor.lower() in ['hyperv', 'lxc', 'kvm']:
+   if cls.hypervisor.lower() == 'kvm':
+   configs = Configurations.list(
+   cls.apiclient,
+   name='kvm.snapshot.enabled'
+   )
--- End diff --

use of validateList  required


---
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-8649: Fixed unnecessary double...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/614#issuecomment-123650029
  
Hi @borisroman ,

Should this go into 4.5 first? If so, why?

Cheers,
Wilder


---
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-8649: Fixed unnecessary double...

[borisroman]

Github user borisroman closed the pull request at:

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


---
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-8649: Fixed unnecessary double...

[borisroman]

GitHub user borisroman opened a pull request:

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

CLOUDSTACK-8649: Fixed unnecessary double url decoding in 
registerSSHKeyPair.

The method cmd.getPublicKey() already returns a decoded string. So when we 
try to decode it again the plus "+" signs are being decoded to white spaces. In 
a next step the key is cut at the spaces and only the first two parts are being 
saved in the DB.

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

$ git pull https://github.com/borisroman/cloudstack CLOUDSTACK-8649

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

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


commit b16843fcb9d45184048263bd15d31f175da7def9
Author: Boris Schrijver 
Date:   2015-07-22T10:26:30Z

CLOUDSTACK-8649: Fixed unnecessary double url decoding in 
registerSSHKeyPair.




---
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-8658: make initializer static ...

[borisroman]

Github user borisroman commented on the pull request:

https://github.com/apache/cloudstack/pull/612#issuecomment-123661436
  
The https://builds.apache.org/job/cloudstack-pull-requests fail after 
adding the Default Charset test. I think the localization of the Jenkins server 
needs to be adjusted. Or the test should be removed.


---
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-8596 ability to query nested g...

[karuturi]

Github user karuturi commented on the pull request:

https://github.com/apache/cloudstack/pull/609#issuecomment-123668417
  
```
root@Rajani's MS ~/source/cloudstack/plugins/user-authenticators/ldap 
(CLOUDSTACK-8596)$ mvn clean test
[INFO] Scanning for projects...
[INFO]
[INFO] 

[INFO] Building Apache CloudStack Plugin - User Authenticator LDAP 
4.6.0-SNAPSHOT
[INFO] 

[INFO]
[INFO] --- maven-clean-plugin:2.5:clean (default-clean) @ 
cloud-plugin-user-authenticator-ldap ---
[INFO] Deleting 
/root/source/cloudstack/plugins/user-authenticators/ldap/target (includes = 
[**/*], excludes = [])
[INFO] Deleting /root/source/cloudstack/plugins/user-authenticators/ldap 
(includes = [target, dist], excludes = [])
[INFO]
[INFO] --- maven-checkstyle-plugin:2.11:check (cloudstack-checkstyle) @ 
cloud-plugin-user-authenticator-ldap ---
[INFO] Starting audit...
Audit done.

[INFO]
[INFO] --- maven-remote-resources-plugin:1.3:process (default) @ 
cloud-plugin-user-authenticator-ldap ---
[INFO]
[INFO] --- maven-resources-plugin:2.5:resources (default-resources) @ 
cloud-plugin-user-authenticator-ldap ---
[debug] execute contextualize
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] Copying 2 resources
[INFO] Copying 3 resources
[INFO]
[INFO] --- maven-compiler-plugin:3.2:compile (default-compile) @ 
cloud-plugin-user-authenticator-ldap ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 29 source files to 
/root/source/cloudstack/plugins/user-authenticators/ldap/target/classes
[INFO]
[INFO] --- gmaven-plugin:1.3:compile (default) @ 
cloud-plugin-user-authenticator-ldap ---
[INFO] Compiled 38 Groovy classes
[INFO]
[INFO] --- maven-resources-plugin:2.5:testResources (default-testResources) 
@ cloud-plugin-user-authenticator-ldap ---
[debug] execute contextualize
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] Copying 1 resource
[INFO] Copying 3 resources
[INFO]
[INFO] --- maven-compiler-plugin:3.2:testCompile (default-testCompile) @ 
cloud-plugin-user-authenticator-ldap ---
[INFO] Nothing to compile - all classes are up to date
[INFO]
[INFO] --- gmaven-plugin:1.3:testCompile (default) @ 
cloud-plugin-user-authenticator-ldap ---
[INFO] Compiled 38 Groovy classes
[INFO]
[INFO] --- maven-surefire-plugin:2.18.1:test (default-test) @ 
cloud-plugin-user-authenticator-ldap ---
[INFO] Surefire report directory: 
/root/source/cloudstack/plugins/user-authenticators/ldap/target/surefire-reports

---
 T E S T S
---
Running 
groovy.org.apache.cloudstack.ldap.NoLdapUserMatchingQueryExceptionSpec
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.304 sec - 
in groovy.org.apache.cloudstack.ldap.NoLdapUserMatchingQueryExceptionSpec
Running groovy.org.apache.cloudstack.ldap.LdapManagerImplSpec
log4j:WARN No appenders could be found for logger 
(org.apache.cloudstack.ldap.LdapManagerImpl).
log4j:WARN Please initialize the log4j system properly.
log4j:WARN See http://logging.apache.org/log4j/1.2/faq.html#noconfig for 
more info.
Tests run: 22, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.38 sec - 
in groovy.org.apache.cloudstack.ldap.LdapManagerImplSpec
Running groovy.org.apache.cloudstack.ldap.LdapListUsersCmdSpec
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.046 sec - 
in groovy.org.apache.cloudstack.ldap.LdapListUsersCmdSpec
Running groovy.org.apache.cloudstack.ldap.LdapAddConfigurationCmdSpec
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.028 sec - 
in groovy.org.apache.cloudstack.ldap.LdapAddConfigurationCmdSpec
Running groovy.org.apache.cloudstack.ldap.LdapUserSpec
Tests run: 9, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.018 sec - 
in groovy.org.apache.cloudstack.ldap.LdapUserSpec
Running groovy.org.apache.cloudstack.ldap.LdapAuthenticatorSpec
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.035 sec - 
in groovy.org.apache.cloudstack.ldap.LdapAuthenticatorSpec
Running groovy.org.apache.cloudstack.ldap.LdapConfigurationVOSpec
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.005 sec - 
in groovy.org.apache.cloudstack.ldap.LdapConfigurationVOSpec
Running groovy.org.apache.cloudstack.ldap.OpenLdapUserManagerSpec
Tests run: 12, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.095 sec 
- in groovy.org.apache.cloudstack.ldap.OpenLdapUserManagerSpec
Running groovy.org.apache.cloudstack.ldap.LdapDeleteConfigurationCmdSpec
Tests run: 4, Failures: 0, Errors: 0, Skip

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

[karuturi]

Github user karuturi commented on the pull request:

https://github.com/apache/cloudstack/pull/609#issuecomment-123668544
  
@DaanHoogland now you can run the tests from the command line or ide 


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

Re: Review Request for PR 609

[Rajani Karuturi]

I ran them from intellij (right click on the test package, run tests)

It also used to work from command line (mvn test from the ldap project)
Its not working currently due to the commit
ec32ea30f7b3e5351e661786955d9fa0929047bd which changed gmaven plugin
version.
I will revert that and update the PR.

~Rajani

On Wed, Jul 22, 2015 at 10:45 AM, Daan Hoogland 
wrote:

> Lot of work Rajani, I don't understand the spec files in groovy, yet.
> How can I verify these are run?
>
> On Wed, Jul 22, 2015 at 6:39 AM, Rajani Karuturi 
> wrote:
> > Can someone please review PR 609? Its an enhancement to LDAP import users
> > functionality
> >
> > https://github.com/apache/cloudstack/pull/609
> >
> > Thanks,
> > ~Rajani
>
>
>
> --
> Daan
>

[GitHub] cloudstack pull request: CLOUDSTACK-8660 - StringUtilsTest.testGet...

[wilderrodrigues]

GitHub user wilderrodrigues opened a pull request:

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

CLOUDSTACK-8660 - StringUtilsTest.testGetPrefferedCharset() fails because 
the preferred charset is not always UTF-8

Test was failing due to "expected: but was:"

Build is successful:

[INFO] BUILD SUCCESS
[INFO] 

[INFO] Total time: 11:24 min
[INFO] Finished at: 2015-07-22T13:51:49+02:00
[INFO] Final Memory: 114M/927M
[INFO] 


Test report for test_vm_life_cycle will be sent  in a bit.

@DaanHoogland @bhaisaab , could you please have a look at that?

Cheers,
Wilder

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

$ git pull https://github.com/schubergphilis/cloudstack 
fix/string_utils_test

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

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


commit a45268f2b2e43a5fb27bdc5b5a643a9abfcef84a
Author: wilderrodrigues 
Date:   2015-07-22T11:24:44Z

CLOUDSTACK-8660 - Formatting test and utility classes

commit c6057961bc808a344f2858233598044cc9e4012c
Author: wilderrodrigues 
Date:   2015-07-22T11:37:43Z

CLOUDSTACK-8660 - Adding a method to check if UTF-8 is supported

   - Changing the test to call isUtf8Supported() before checking if the 
preferred charset is actually UTF-8




---
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-8650: Fix securitygroups ingre...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/601#issuecomment-123697692
  
@bhaisaab I was going to ask @franklouwers how did he test his changes, 
since I would like to test them before giving a LGTM.

Actually, I think you were the only one to LGTM it. :)

Cheers,
Wilder


---
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-8649: Fixed unnecessary double...

[kishankavala]

Github user kishankavala commented on the pull request:

https://github.com/apache/cloudstack/pull/615#issuecomment-123699898
  
@DaanHoogland double URL decoding was added during registerSSHKeyPair 
refactor. commit 968e71ad0e49088b5f2f022df29aec02b10a5ede
Can you please check if this fix breaks any of that refactoring?
LGTM otherwise.


---
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-8660 - StringUtilsTest.testGet...

[karuturi]

Github user karuturi commented on the pull request:

https://github.com/apache/cloudstack/pull/616#issuecomment-123701878
  
:+1: 


---
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-8655: [Browser Based Upload Vo...

[kishankavala]

Github user kishankavala commented on the pull request:

https://github.com/apache/cloudstack/pull/611#issuecomment-123703777
  
LGTM


---
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-8650: Fix securitygroups ingre...

[franklouwers]

Github user franklouwers commented on the pull request:

https://github.com/apache/cloudstack/pull/601#issuecomment-123706604
  
All,

I'll check the typo (I know how it happend) later this afternoon. Will also 
provide logs both before (bad behaviour: rule not installed) and after (good 
thing: rule installed) later today. 


---
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-8650: Fix securitygroups ingre...

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/601#issuecomment-123711497
  
@wilderrodrigues yeah, as I said I've not tested it. Just had a glance at 
the code, but good that @resmo pointed out the typo.


---
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-8660 - StringUtilsTest.testGet...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/616#issuecomment-123721624
  
The other test in the string utils test class also failed.

I will have a look at that one as well.

Cheers,
Wilder

Sent from my iPhone

On 22 Jul 2015, at 14:25, asfbot 
mailto:notificati...@github.com>> wrote:


cloudstack-pull-requests 
#800 UNSTABLE
Looks like there's a problem with this pull request

—
Reply to this email directly or view it on 
GitHub.



---
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-8655: [Browser Based Upload Vo...

[asfgit]

Github user asfgit closed the pull request at:

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


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

Re: [Proposal] CoreOS support

[Wido den Hollander]

On 07/22/2015 11:14 AM, Kishan Kavala wrote:
> I would like to add CoreOS as a supported OS type to cloudstack.
> CoreOS vms can be created even now by selecting OS type as "Other". There are 
> cloud providers supporting CoreOS offerings in production already.
> By making the changes mentioned at [1] I would make it easier to deploy 
> CoreOS Vms and also add UI support for providing cloud-config via userdata.
> I'll also update the documentation at [2] after making these changes.
> 

Why not? I'd say, create a Jira issue and file a PR!

Wido

> [1] https://cwiki.apache.org/confluence/display/CLOUDSTACK/CoreOS
> [2] https://coreos.com/os/docs/latest/booting-on-cloudstack.html
> 

Re: [Proposal] CoreOS support

[Koushik Das]

+1

On 22-Jul-2015, at 2:44 PM, Kishan Kavala  wrote:

> I would like to add CoreOS as a supported OS type to cloudstack.
> CoreOS vms can be created even now by selecting OS type as "Other". There are 
> cloud providers supporting CoreOS offerings in production already.
> By making the changes mentioned at [1] I would make it easier to deploy 
> CoreOS Vms and also add UI support for providing cloud-config via userdata.
> I'll also update the documentation at [2] after making these changes.
> 
> [1] https://cwiki.apache.org/confluence/display/CLOUDSTACK/CoreOS
> [2] https://coreos.com/os/docs/latest/booting-on-cloudstack.html

Re: [Proposal] CoreOS support

[Ahmad]

+1 looking forward to this.



> On Jul 22, 2015, at 1:14 PM, Kishan Kavala  wrote:
> 
> I would like to add CoreOS as a supported OS type to cloudstack.
> CoreOS vms can be created even now by selecting OS type as "Other". There are 
> cloud providers supporting CoreOS offerings in production already.
> By making the changes mentioned at [1] I would make it easier to deploy 
> CoreOS Vms and also add UI support for providing cloud-config via userdata.
> I'll also update the documentation at [2] after making these changes.
> 
> [1] https://cwiki.apache.org/confluence/display/CLOUDSTACK/CoreOS
> [2] https://coreos.com/os/docs/latest/booting-on-cloudstack.html

RE: [Proposal] CoreOS support

[Somesh Naidu]

+1

Regards,
Somesh

-Original Message-
From: Ahmad [mailto:aemne...@gmail.com] 
Sent: Wednesday, July 22, 2015 10:28 AM
To: dev@cloudstack.apache.org
Subject: Re: [Proposal] CoreOS support

+1 looking forward to this.



> On Jul 22, 2015, at 1:14 PM, Kishan Kavala  wrote:
> 
> I would like to add CoreOS as a supported OS type to cloudstack.
> CoreOS vms can be created even now by selecting OS type as "Other". There are 
> cloud providers supporting CoreOS offerings in production already.
> By making the changes mentioned at [1] I would make it easier to deploy 
> CoreOS Vms and also add UI support for providing cloud-config via userdata.
> I'll also update the documentation at [2] after making these changes.
> 
> [1] https://cwiki.apache.org/confluence/display/CLOUDSTACK/CoreOS
> [2] https://coreos.com/os/docs/latest/booting-on-cloudstack.html

Re: [Proposal] CoreOS support

[Sebastien Goasguen]

+1

Can you clean that last of OS Type while you are it, last time I checked it was 
really long with some outdated distros…

-sebastien

> On Jul 22, 2015, at 4:35 PM, Somesh Naidu  wrote:
> 
> +1
> 
> Regards,
> Somesh
> 
> -Original Message-
> From: Ahmad [mailto:aemne...@gmail.com] 
> Sent: Wednesday, July 22, 2015 10:28 AM
> To: dev@cloudstack.apache.org
> Subject: Re: [Proposal] CoreOS support
> 
> +1 looking forward to this.
> 
> 
> 
>> On Jul 22, 2015, at 1:14 PM, Kishan Kavala  wrote:
>> 
>> I would like to add CoreOS as a supported OS type to cloudstack.
>> CoreOS vms can be created even now by selecting OS type as "Other". There 
>> are cloud providers supporting CoreOS offerings in production already.
>> By making the changes mentioned at [1] I would make it easier to deploy 
>> CoreOS Vms and also add UI support for providing cloud-config via userdata.
>> I'll also update the documentation at [2] after making these changes.
>> 
>> [1] https://cwiki.apache.org/confluence/display/CLOUDSTACK/CoreOS
>> [2] https://coreos.com/os/docs/latest/booting-on-cloudstack.html

Re: [Proposal] CoreOS support

[Simon Weller]

+1 as well. 


From: Sebastien Goasguen 
Sent: Wednesday, July 22, 2015 9:39 AM
To: dev@cloudstack.apache.org
Subject: Re: [Proposal] CoreOS support

+1

Can you clean that last of OS Type while you are it, last time I checked it was 
really long with some outdated distros…

-sebastien

> On Jul 22, 2015, at 4:35 PM, Somesh Naidu  wrote:
>
> +1
>
> Regards,
> Somesh
>
> -Original Message-
> From: Ahmad [mailto:aemne...@gmail.com]
> Sent: Wednesday, July 22, 2015 10:28 AM
> To: dev@cloudstack.apache.org
> Subject: Re: [Proposal] CoreOS support
>
> +1 looking forward to this.
>
>
>
>> On Jul 22, 2015, at 1:14 PM, Kishan Kavala  wrote:
>>
>> I would like to add CoreOS as a supported OS type to cloudstack.
>> CoreOS vms can be created even now by selecting OS type as "Other". There 
>> are cloud providers supporting CoreOS offerings in production already.
>> By making the changes mentioned at [1] I would make it easier to deploy 
>> CoreOS Vms and also add UI support for providing cloud-config via userdata.
>> I'll also update the documentation at [2] after making these changes.
>>
>> [1] https://cwiki.apache.org/confluence/display/CLOUDSTACK/CoreOS
>> [2] https://coreos.com/os/docs/latest/booting-on-cloudstack.html

Re: [Proposal] CoreOS support

[Keerthiraja SJ]

Will this works even for the RancherOS

On Wed, Jul 22, 2015 at 8:24 PM, Simon Weller  wrote:

>
> +1 as well.
>
> 
> From: Sebastien Goasguen 
> Sent: Wednesday, July 22, 2015 9:39 AM
> To: dev@cloudstack.apache.org
> Subject: Re: [Proposal] CoreOS support
>
> +1
>
> Can you clean that last of OS Type while you are it, last time I checked
> it was really long with some outdated distros…
>
> -sebastien
>
> > On Jul 22, 2015, at 4:35 PM, Somesh Naidu 
> wrote:
> >
> > +1
> >
> > Regards,
> > Somesh
> >
> > -Original Message-
> > From: Ahmad [mailto:aemne...@gmail.com]
> > Sent: Wednesday, July 22, 2015 10:28 AM
> > To: dev@cloudstack.apache.org
> > Subject: Re: [Proposal] CoreOS support
> >
> > +1 looking forward to this.
> >
> >
> >
> >> On Jul 22, 2015, at 1:14 PM, Kishan Kavala 
> wrote:
> >>
> >> I would like to add CoreOS as a supported OS type to cloudstack.
> >> CoreOS vms can be created even now by selecting OS type as "Other".
> There are cloud providers supporting CoreOS offerings in production already.
> >> By making the changes mentioned at [1] I would make it easier to deploy
> CoreOS Vms and also add UI support for providing cloud-config via userdata.
> >> I'll also update the documentation at [2] after making these changes.
> >>
> >> [1] https://cwiki.apache.org/confluence/display/CLOUDSTACK/CoreOS
> >> [2] https://coreos.com/os/docs/latest/booting-on-cloudstack.html
>
>

RE: [Proposal] CoreOS support

[Suresh Sadhu]

Right now No

+1 for core os support

Regards
sadhu

-Original Message-
From: Keerthiraja SJ [mailto:sjkeer...@gmail.com] 
Sent: 22 July 2015 20:32
To: dev@cloudstack.apache.org
Subject: Re: [Proposal] CoreOS support

Will this works even for the RancherOS

On Wed, Jul 22, 2015 at 8:24 PM, Simon Weller  wrote:

>
> +1 as well.
>
> 
> From: Sebastien Goasguen 
> Sent: Wednesday, July 22, 2015 9:39 AM
> To: dev@cloudstack.apache.org
> Subject: Re: [Proposal] CoreOS support
>
> +1
>
> Can you clean that last of OS Type while you are it, last time I 
> checked it was really long with some outdated distros…
>
> -sebastien
>
> > On Jul 22, 2015, at 4:35 PM, Somesh Naidu 
> wrote:
> >
> > +1
> >
> > Regards,
> > Somesh
> >
> > -Original Message-
> > From: Ahmad [mailto:aemne...@gmail.com]
> > Sent: Wednesday, July 22, 2015 10:28 AM
> > To: dev@cloudstack.apache.org
> > Subject: Re: [Proposal] CoreOS support
> >
> > +1 looking forward to this.
> >
> >
> >
> >> On Jul 22, 2015, at 1:14 PM, Kishan Kavala 
> >> 
> wrote:
> >>
> >> I would like to add CoreOS as a supported OS type to cloudstack.
> >> CoreOS vms can be created even now by selecting OS type as "Other".
> There are cloud providers supporting CoreOS offerings in production already.
> >> By making the changes mentioned at [1] I would make it easier to 
> >> deploy
> CoreOS Vms and also add UI support for providing cloud-config via userdata.
> >> I'll also update the documentation at [2] after making these changes.
> >>
> >> [1] https://cwiki.apache.org/confluence/display/CLOUDSTACK/CoreOS
> >> [2] https://coreos.com/os/docs/latest/booting-on-cloudstack.html
>
>

[GitHub] cloudstack pull request: CLOUDSTACK-8650: Fix securitygroups ingre...

[franklouwers]

Github user franklouwers commented on the pull request:

https://github.com/apache/cloudstack/pull/601#issuecomment-123756479
  
See updated commit to fix the missing : .

See also https://gist.github.com/franklouwers/d5061b4ef50e2b4253fe with 
logs of what works, what doesn't work, and how this PR makes it work


---
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-8660 - StringUtilsTest.testGet...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/616#issuecomment-123758576
  
Build is back to normal... keys are being created... and VMs are also fine. 
See details below:

@karuturi @bhaisaab @DaanHoogland: got some time for a second LGTM?

[root@kvm1 ~]# ls -lart ~/.ssh/
total 16
dr-xr-x---. 3 root root 4096 Jul 22 02:37 ..
-rw-r--r--. 1 root root  389 Jul 22 08:21 id_rsa.pub.cloud
drwx--. 2 root root 4096 Jul 22 08:21 .
-rw---. 1 root root 1678 Jul 22 08:21 id_rsa.cloud
[root@kvm1 ~]# 

Test advanced zone virtual router ... === TestName: 
test_advZoneVirtualRouter | Status : SUCCESS ===
ok
Test Deploy Virtual Machine ... === TestName: test_deploy_vm | Status : 
SUCCESS ===
ok
Test Multiple Deploy Virtual Machine ... === TestName: 
test_deploy_vm_multiple | Status : SUCCESS ===
ok
Test Stop Virtual Machine ... === TestName: test_01_stop_vm | Status : 
SUCCESS ===
ok
Test Start Virtual Machine ... === TestName: test_02_start_vm | Status : 
SUCCESS ===
ok
Test Reboot Virtual Machine ... === TestName: test_03_reboot_vm | Status : 
SUCCESS ===
ok
Test destroy Virtual Machine ... === TestName: test_06_destroy_vm | Status 
: SUCCESS ===
ok
Test recover Virtual Machine ... === TestName: test_07_restore_vm | Status 
: SUCCESS ===
ok
Test migrate VM ... SKIP: At least two hosts should be present in the zone 
for migration
Test destroy(expunge) Virtual Machine ... === TestName: test_09_expunge_vm 
| Status : SUCCESS ===
ok

--
Ran 10 tests in 2666.478s

OK (SKIP=1)
/tmp//MarvinLogs/test_vm_life_cycle_1JU23D/results.txt (END)


---
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-8649: Fixed unnecessary double...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/615#issuecomment-123766531
  
Hi guys,

1. RegisterSSHKeyPairCmd.getPublicKey() doesn't do a decoding of the key. 
That's just a simple getter method.
2. However, I don't think the addition of the URLDecoder for the public key 
is really necessary.

I have seen the use of URLDecoder for the cypher text, an URL or form data, 
but not the pub-key itself being decoded using W3C.

""" Decodes a application/x-www-form-urlencoded string using a specific 
encoding scheme. The supplied encoding is used to determine what characters are 
represented by any consecutive sequences of the form "%xy".

Note: The World Wide Web Consortium Recommendation states that UTF-8 should 
be used. Not doing so may introduce incompatibilities."""

@DaanHoogland, is it being done because the data is coming via the API, 
which is a web based? If so, it does make sense.

@borisroman: in order to make Java compatible with the RFC-3986, we need to 
do something like:

* To comply with RFC-3986.
* URLEncoder.encode(original, "UTF-8").replace("+", "%20").replace("*", 
"%2A").replace("%7E", "~");

Perhaps finding where the pub-key gets decoded before the the code Daan 
added is better than just removing it.

In addition, could you let me know which steps you followed to test it? I 
would like to help verifying.

Cheers,
Wilder


---
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-7539: coverity regression dead...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/610#issuecomment-123775323
  
LGTM :+1:  Merging...


---
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-7539: coverity regression dead...

[asfgit]

Github user asfgit closed the pull request at:

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


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

[runseb]

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

https://github.com/apache/cloudstack/pull/605#discussion_r35232143
  
--- Diff: tools/docker/Dockerfile.marvin ---
@@ -0,0 +1,37 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+# 
+#   http://www.apache.org/licenses/LICENSE-2.0
+# 
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+#
+# build for cloudstack_home_dir not this folder
+FROM python:2
+
+MAINTAINER "Apache CloudStack" 
+LABEL Vendor="Apache.org"
+LABEL License=ApacheV2
+LABEL Version=4.6.0
+
+ENV 
PKG_URL=http://jenkins.buildacloud.org/job/cloudstack-marvin-master/lastSuccessfulBuild/artifact/tools/marvin/dist/Marvin-4.6.0-SNAPSHOT.tar.gz
+
+RUN pip install --allow-external mysql-connector-python 
mysql-connector-python
+RUN pip install ${PKG_URL}
+
+RUN mkdir -p /marvin
+COPY setup/dev /marvin/dev
+COPY tools/marvin/marvin /marvin/marvin
+COPY test/integration /marvin/integration
+
+WORKDIR /marvin
--- End diff --

no CMD or ENTRYPOINT here ?


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

[runseb]

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

https://github.com/apache/cloudstack/pull/605#discussion_r35232181
  
--- Diff: tools/docker/README.md ---
@@ -0,0 +1,66 @@
+
+# Docker Files
+
+Dockerfiles used to build CloudStack images available on Docker hub.
+
+
+## Use images from docker.io
+
+### CloudStack Management-server 
+
+```
+docker pull docker.io/mysql:5.5
--- End diff --

remove .io ...


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

[runseb]

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

https://github.com/apache/cloudstack/pull/605#discussion_r35232136
  
--- Diff: tools/docker/Dockerfile.centos6 ---
@@ -0,0 +1,60 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+# 
+#   http://www.apache.org/licenses/LICENSE-2.0
+# 
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+FROM centos:6
+
+MAINTAINER "Apache CloudStack" 
+LABEL Vendor="Apache.org"
+LABEL License=ApacheV2
+LABEL Version=4.6.0
+
+ENV 
PKG_URL=http://jenkins.buildacloud.org/job/build-dockerfile-centos6/lastSuccessfulBuild/artifact/dist/rpmbuild/RPMS/x86_64
+
+# MySQLd
+#RUN yum install mysql-server -y
+#RUN /usr/bin/mysql_install_db --user=mysql
+#RUN /usr/bin/mysqld_safe --user=mysql  >/dev/null 2>&1 &
+#RUN (mysqld_safe &); sleep 5; mysqladmin -u root -p password 'password'
+
--- End diff --

Maybe you can merge some of those RUNS to reduce the number of layers ?
https://docs.docker.com/articles/dockerfile_best-practices/


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

[runseb]

Github user runseb commented on the pull request:

https://github.com/apache/cloudstack/pull/605#issuecomment-123779835
  
LGTM if you address my couple comments.
And really, I would squash this (33 commits), I don't see why we need to 
keep all those tiny commits in our master branch.


---
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-8649: Fixed unnecessary double...

[teulaert]

Github user teulaert commented on the pull request:

https://github.com/apache/cloudstack/pull/615#issuecomment-123818145
  
@wilderrodrigues reproducing is really easy, just try to register your 
public key using the registerSSHKeyPair API call. In the database you will find 
you key, but only a part of it. You can try this on 4.5 but as far we know the 
master contains this same issue. We did not test 4.4.

@borisroman has been looking where the key gets decoded first, but wasn't 
able to yet. If you can help, that would be great.



---
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: Added a null value exception in LibvirtBa...

[manuiiit]

GitHub user manuiiit opened a pull request:

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

Added a null value exception in LibvirtBackupSnapshotCommandWrapper.java



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

$ git pull https://github.com/manuiiit/cloudstack master

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

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


commit 66d7ed3f1a9d06bbdf388cbb7b7e41e945b0a66d
Author: Maneesha P 
Date:   2015-07-22T19:15:00Z

Added a null value exception




---
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-8649: Fixed unnecessary double...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/615#issuecomment-123863689
  
Hi Lennert,

The Command claases use annotations for the fields. The fields values are 
set via reflection. I bet my iPhone the decoding is being done there :)

I will have a look tomorrow, as I might know where the whole thing is 
happening.

For now I would say LGTM and please merge it.

Cheers,
Wilder

Sent from my iPhone

On 22 Jul 2015, at 20:29, Lennert den Teuling 
mailto:notificati...@github.com>> wrote:


@wilderrodrigues reproducing is really 
easy, just try to register your public key using the registerSSHKeyPair API 
call. In the database you will find you key, but only a part of it. You can try 
this on 4.5 but as far we know the master contains this same issue. We did not 
test 4.4.

@borisroman has been looking where the key 
gets decoded first, but wasn't able to yet. If you can help, that would be 
great.

—
Reply to this email directly or view it on 
GitHub.



---
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: Added a null value exception in LibvirtBa...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/617#issuecomment-123879270
  
You added code to a class that was recently refactored and was covered by 
unit tests. Please add a new unit test to cover the new flow.

Cheers,
Wilder

Sent from my iPhone

On 22 Jul 2015, at 22:34, manuiiit 
mailto:notificati...@github.com>> wrote:


You can view, comment on, or merge this pull request online at:

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

Commit Summary

  *   Added a null value exception

File Changes

  *   M 
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtBackupSnapshotCommandWrapper.java
 (9)

Patch Links:

  *   https://github.com/apache/cloudstack/pull/617.patch
  *   https://github.com/apache/cloudstack/pull/617.diff

—
Reply to this email directly or view it on 
GitHub.



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

Fwd: Re: XenServer is disconnected after CS hosts shutdown

[tony_caotong]

copy mail thread to @dev for seeking more help.


 Forwarded Message 
Subject:Re: XenServer is disconnected after CS hosts shutdown
Date:   Wed, 22 Jul 2015 21:03:13 +0800
From:   tony_caot...@163.com
Reply-To:   us...@cloudstack.apache.org
To: us...@cloudstack.apache.org, opsrunb...@gmail.com



Hey!  help please...

some news.
I think the cause is that the ACS host can't communicate with XenServer
host.
ACS continues outputing logs like this

2015-07-22 20:42:13,555 DEBUG [c.c.a.m.ClusteredAgentAttache]
(AgentManager-Handler-7:null) Seq 5-8174877748607582212: Forwarding Seq
5-8174877748607582212:  { Cmd , MgmtId: 279278805451459, via: 5, Ver:
v1, Flags: 100111, [{"com.cloud.agent.api.MaintainCommand":{"wait":0}}]
} to 280345368052992

I am not sure that if the ACS status is wrong or some services on
xenserver are not opend.

on xenserver , I found *xenheartbeat.sh is not running.*
*(/bin/bash /opt/cloud/bin/xenheartbeat.sh
00d8e0d0-8561-4b3d-9044-cbc496ff22cc 120 60)*

As some operations about xenserver was pending， xenserver can not be
deleted from web UI.

I got a temporary solution

1. delete jobs from DB cloud.vm_work_job.
2. delete xenserver from DB cloud.host.
3. add xenserver host back from web UI.

then it works.

Does anyone have a idea for this?

Could anyone tell what things does ACS do on xenserver host when adding
a xenserver ?

Thanks,

---
Cao Tong

On 07/22/2015 04:26 PM, tony_caot...@163.com wrote:


@prashant, following it the answer of you questions

1. Yes, primary storage is connected fine for my xenserver.

2. No, Xenserver's password is not changed.

3. yes, web UI is fine, and I can login.

4.  before reboot, I unmanaged and disabled resources,  and after
reboot I have enabled all of them.

5.  hosts is states is UP.

6. No yum update in anywhere.

7.  system VMs status is fine, i think.

---
Cao Tong

On 07/22/2015 04:13 PM, tony_caot...@163.com wrote:


Hi,

After reinstall, I got the problem again

So, I will describe once again.

WHAT my environment looks like:

I have a ACS server host and a xenserver host, After both reboot, I
can not create a VM on xenserver through ACS.
A KVM and A NFS are running together in ACS manager host.

the status of new VM is always 'staring' on the WEB, but I can create
new VM using xencenter.

- ERR LOGS --
2015-07-22 15:56:56,357 DEBUG [c.c.s.StorageManagerImpl]
(StatsCollector-3:ctx-1aa2e8c9) Unable to send storage pool command
to Pool[4|NetworkFilesystem] via 4
com.cloud.exception.OperationTimedoutException: Commands
2829104990918803478 to Host 4 timed out after 3600

2015-07-22 15:56:56,358 INFO  [c.c.s.StatsCollector]
(StatsCollector-3:ctx-1aa2e8c9) Unable to reach
Pool[4|NetworkFilesystem]
com.cloud.exception.StorageUnavailableException: Resource
[StoragePool:4] is unreachable: Unable to send command to the pool


- and there are lots of DEBUG infos  --- repeat again
and again ---

2015-07-22 15:36:12,887 DEBUG [c.c.a.m.ClusteredAgentAttache]
(AgentManager-Handler-14:null) Seq 4-8064821032713715922: Forwarding
Seq 4-8064821032713715922:  { Cmd , MgmtId: 227448510156211, via: 4,
Ver: v1, Flags: 100111,
[{"com.cloud.agent.api.MaintainCommand":{"wait":0}}] } to
116784073679673
2015-07-22 15:36:12,889 DEBUG [c.c.a.m.ClusteredAgentAttache]
(AgentManager-Handler-10:null) Seq 4-8064821032713715883: Forwarding
Seq 4-8064821032713715883:  { Cmd , MgmtId: 227448510156211, via: 4,
Ver: v1, Flags: 100111,
[{"org.apache.cloudstack.storage.command.CopyCommand":{"srcTO":{"org.apache.cloudstack.storage.to.TemplateObjectTO":{"path":"template/tmpl/1/5/af949612-838f-3a6d-931b-312e612db740.vhd","origUrl":"http://download.cloud.com/templates/builtin/centos56-x86_64.vhd.bz2","uuid":"80b60e46-3017-11e5-8736-00259091a13a","id":5,"format":"VHD","accountId":1,"checksum":"905cec879afd9c9d22ecc8036131a180","hvm":false,"displayText":"CentOS
5.6(64-bit) no GUI
(XenServer)","imageDataStore":{"com.cloud.agent.api.to.NfsTO":{"_url":"nfs://10.0.0.100/storage/secondary","_role":"Image"}},"name":"centos56-x86_64-xen","hypervisorType":"XenServer"}},"destTO":{"org.apache.cloudstack.storage.to.TemplateObjectTO":{"origUrl":"http://download.cloud.com/templates/builtin/centos56-x86_64.vhd.bz2","uuid":"80b60e46-3017-11e5-8736-00259091a13a","id":5,"format":"VHD","accountId":1,"checksum":"905cec879afd9c9d22ecc8036131a180","hvm":false,"displayText":"CentOS
5.6(64-bit) no GUI
(XenServer)","imageDataStore":{"org.apache.cloudstack.storage.to.PrimaryDataStoreTO":{"uuid":"2df26406-31bf-3a95-8a61-f5008defd9a0","id":4,"poolType":"NetworkFilesystem","host":"10.0.0.100","path":"/storage/xen/primary","port":2049,"url":"NetworkFilesystem://10.0.0.100/storage/xen/primary/?ROLE=Primary&STOREUUID=2df26406-31bf-3a95-8a61-f5008defd9a0"}},"name":"centos56-x86_64-xen","hypervisorType":"XenServer"}},"executeInSequence":true,"options":{},"wait":10800}}]
} to 116784073679673

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

[karuturi]

Github user karuturi commented on the pull request:

https://github.com/apache/cloudstack/pull/609#issuecomment-123954109
  
@DaanHoogland @abhinandanprateek @runseb @wilderrodrigues @bhaisaab Can you 
please review this PR?


---
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: Dereference NULL return value

[kansal]

GitHub user kansal opened a pull request:

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

Dereference NULL return value

No check on the null value of metadatafile. If null, the following 
operations failed. Added null check for the metadatafile. 

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

$ git pull https://github.com/kansal/cloudstack master

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

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


commit aa0bd53162bd3f4450e5e7915e3b1f9e898fe55d
Author: Kshitij Kansal 
Date:   2015-07-23T15:55:17Z

Dereference NULL return value




---
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-8659: Verify presentation of v...

[sanju1010]

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

https://github.com/apache/cloudstack/pull/613#discussion_r35288952
  
--- Diff: test/integration/testpaths/testpath_uuid_event.py ---
@@ -0,0 +1,198 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+""" Test cases to verify presentation of volume id in events table 
+for 'SNAPSHOT.CREATE' type.
+"""
+
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase
+from marvin.lib.utils import (cleanup_resources,
+ validateList)
+from marvin.lib.base import (Account,
+ ServiceOffering,
+ Snapshot,
+ VirtualMachine,
+Configurations
+ )
+from marvin.lib.common import (get_domain,
+   get_zone,
+   get_template,
+   list_volumes,
+  )
+
+from marvin.codes import PASS
+
+
+
+
+class TestVerifyEventsTable(cloudstackTestCase):
+
+@classmethod
+def setUpClass(cls):
+testClient = super(TestVerifyEventsTable, cls).getClsTestClient()
+cls.apiclient = testClient.getApiClient()
+cls.testdata = testClient.getParsedTestDataConfig()
+
+cls.hypervisor = cls.testClient.getHypervisorInfo()
+# Get Zone, Domain and templates
+cls.domain = get_domain(cls.apiclient)
+cls.zone = get_zone(cls.apiclient, testClient.getZoneForTests())
+
+cls.template = get_template(
+cls.apiclient,
+cls.zone.id,
+cls.testdata["ostype"])
+
+cls._cleanup = []
+
+try:
+
+cls.unsupportedHypervisor = False
+if cls.hypervisor.lower() in ['hyperv', 'lxc', 'kvm']:
+   if cls.hypervisor.lower() == 'kvm':
+   configs = Configurations.list(
+   cls.apiclient,
+   name='kvm.snapshot.enabled'
+   )
+   if configs[0].value == "false":
+   cls.unsupportedHypervisor = True
+   else:
+   cls.unsupportedHypervisor = True
+
+return
+# Create an account
+cls.account = Account.create(
+cls.apiclient,
+cls.testdata["account"],
+domainid=cls.domain.id
+)
+
+# Create user api client of the account
+cls.userapiclient = testClient.getUserApiClient(
+UserName=cls.account.name,
+DomainName=cls.account.domain
+)
+# Create Service offering
+cls.service_offering = ServiceOffering.create(
+cls.apiclient,
+cls.testdata["service_offering"],
+)
+
+cls._cleanup = [
+cls.account,
+cls.service_offering,
+]
+except Exception as e:
+cls.tearDownClass()
+raise e
+return
+
+@classmethod
+def tearDownClass(cls):
+try:
+cleanup_resources(cls.apiclient, cls._cleanup)
+except Exception as e:
+raise Exception("Warning: Exception during cleanup : %s" % e)
+
+def setUp(self):
+
+self.cleanup = []
+if self.unsupportedHypervisor:
+self.skipTest("snapshots are not supported on %s" % 
self.hypervisor.lower())
+self.apiclient = self.testClient.getApiClient()
+self.dbclient = self.testClient.getDbConnection()
+
+
+
+def tearDown(self):
+try

[GitHub] cloudstack pull request: CLOUDSTACK-8659: Verify presentation of v...

[sanju1010]

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

https://github.com/apache/cloudstack/pull/613#discussion_r35288977
  
--- Diff: test/integration/testpaths/testpath_uuid_event.py ---
@@ -0,0 +1,198 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+""" Test cases to verify presentation of volume id in events table 
+for 'SNAPSHOT.CREATE' type.
+"""
+
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase
+from marvin.lib.utils import (cleanup_resources,
+ validateList)
+from marvin.lib.base import (Account,
+ ServiceOffering,
+ Snapshot,
+ VirtualMachine,
+Configurations
+ )
+from marvin.lib.common import (get_domain,
+   get_zone,
+   get_template,
+   list_volumes,
+  )
+
+from marvin.codes import PASS
+
+
+
+
+class TestVerifyEventsTable(cloudstackTestCase):
+
+@classmethod
+def setUpClass(cls):
+testClient = super(TestVerifyEventsTable, cls).getClsTestClient()
+cls.apiclient = testClient.getApiClient()
+cls.testdata = testClient.getParsedTestDataConfig()
+
+cls.hypervisor = cls.testClient.getHypervisorInfo()
+# Get Zone, Domain and templates
+cls.domain = get_domain(cls.apiclient)
+cls.zone = get_zone(cls.apiclient, testClient.getZoneForTests())
+
+cls.template = get_template(
+cls.apiclient,
+cls.zone.id,
+cls.testdata["ostype"])
+
+cls._cleanup = []
+
+try:
+
+cls.unsupportedHypervisor = False
+if cls.hypervisor.lower() in ['hyperv', 'lxc', 'kvm']:
+   if cls.hypervisor.lower() == 'kvm':
+   configs = Configurations.list(
+   cls.apiclient,
+   name='kvm.snapshot.enabled'
+   )
+   if configs[0].value == "false":
+   cls.unsupportedHypervisor = True
+   else:
+   cls.unsupportedHypervisor = True
+
+return
+# Create an account
+cls.account = Account.create(
+cls.apiclient,
+cls.testdata["account"],
+domainid=cls.domain.id
+)
+
+# Create user api client of the account
+cls.userapiclient = testClient.getUserApiClient(
+UserName=cls.account.name,
+DomainName=cls.account.domain
+)
+# Create Service offering
+cls.service_offering = ServiceOffering.create(
+cls.apiclient,
+cls.testdata["service_offering"],
+)
+
+cls._cleanup = [
+cls.account,
+cls.service_offering,
+]
+except Exception as e:
+cls.tearDownClass()
+raise e
+return
+
+@classmethod
+def tearDownClass(cls):
+try:
+cleanup_resources(cls.apiclient, cls._cleanup)
+except Exception as e:
+raise Exception("Warning: Exception during cleanup : %s" % e)
+
+def setUp(self):
+
+self.cleanup = []
+if self.unsupportedHypervisor:
+self.skipTest("snapshots are not supported on %s" % 
self.hypervisor.lower())
+self.apiclient = self.testClient.getApiClient()
+self.dbclient = self.testClient.getDbConnection()
+
+
+
+def tearDown(self):
+try

Re: [GitHub] cloudstack pull request: CLOUDSTACK-8634: Made changes to test_sec...

[Sanjeev N]

Can somebody please review this PR?

On Tue, Jul 14, 2015 at 5:58 PM, sanju1010  wrote:

> GitHub user sanju1010 opened a pull request:
>
> https://github.com/apache/cloudstack/pull/586
>
> CLOUDSTACK-8634: Made changes to test_security_group.py test suite to
> support EIP
>
>
> Made changes to test_security_group.py test suite to support EIP
>
> 1. In case of a basic zone with EIP/ELB capability vm will have two ip
> addresses one from public ip range and another one from guest ip range.
> vm creation method in base.py returns vm ip address which is part of
> guest ip range as the vm.ssh.ipaddress if we don't pass mode to it. So
> access to vms with that ip address would not be successful. Made changes to
> handle this
> 2.vm public address is associated with Netscaler. So even if we don't
> allow ping traffic in security groups applied to vm, ping will be
> successful. Skipping ping test in case the zone is enabled with EIP/ELB
> 3.Removing default rules from security groups except what is needed
> for that test.
>
>
> Test Results:
> ==
> Test ingress rules for specific IP set ... === TestName:
> test_ingress_rules_specific_IP_set | Status : SUCCESS ===
> ok
> Test revoke ingress rule ... === TestName: test_01_revokeIngressRule |
> Status : SUCCESS ===
> ok
> Test ingress rules for specific IP set and non default security group
> ... === TestName: test_ingress_rules_specific_IP_set_non_def_sec_group |
> Status : SUCCESS ===
> ok
>
> You can merge this pull request into a Git repository by running:
>
> $ git pull https://github.com/sanju1010/cloudstack eip
>
> Alternatively you can review and apply these changes as the patch at:
>
> https://github.com/apache/cloudstack/pull/586.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 #586
>
> 
> commit 3ecf3dc172cc5f60f55d9b389d3e53a6a1e67f05
> Author: sanjeev 
> Date:   2015-07-14T11:54:58Z
>
> CLOUDSTACK-8634: Made changes to test_security_group.py test suite to
> support EIP
>
> 
>
>
> ---
> 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: Added a null value exception in LibvirtBa...

[karuturi]

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

https://github.com/apache/cloudstack/pull/617#discussion_r35289154
  
--- Diff: 
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtBackupSnapshotCommandWrapper.java
 ---
@@ -25,6 +25,8 @@
 import java.io.IOException;
 import java.text.MessageFormat;
 
+import com.thoughtworks.xstream.mapper.Mapper;
+import org.apache.commons.lang.ObjectUtils;
--- End diff --

remove the unused imports and format the code as per coding conventions.


---
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-8660 - StringUtilsTest.testGet...

[karuturi]

Github user karuturi commented on the pull request:

https://github.com/apache/cloudstack/pull/616#issuecomment-123959991
  
looks good :+1: 


---
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: Dereference NULL return value

[kishankavala]

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

https://github.com/apache/cloudstack/pull/618#discussion_r35290636
  
--- Diff: 
plugins/user-authenticators/saml2/src/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java
 ---
@@ -368,8 +368,14 @@ private boolean setup() {
 _idpMetaDataProvider = new HTTPMetadataProvider(_timer, 
client, idpMetaDataUrl);
 } else {
 File metadataFile = 
PropertiesUtil.findConfigFile(idpMetaDataUrl);
-s_logger.debug("Provided Metadata is not a URL, trying to 
read metadata file from local path: " + metadataFile.getAbsolutePath());
-_idpMetaDataProvider = new 
FilesystemMetadataProvider(_timer, metadataFile);
+if (metadataFile == null) {
+s_logger.error("Metadata file returned null");
--- End diff --

Can you please update the error message: "Provided Metadata is not a URL, 
Unable to locate metadata file from local path"


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

[PROPOSAL] Trust LDAP and Auto Import of users

[Rajani Karuturi]

We(Sarath and me) are working on adding support for auto import of LDAP
users to a configured domain in CloudStack.

A WIP FS is available at
https://cwiki.apache.org/confluence/display/CLOUDSTACK/WIP%3A+LDAP%3A+Trust+AD+and+Auto+Import

Please review and let us know if there are any comments/suggestions.


~Rajani

[GitHub] cloudstack pull request: CLOUDSTACK-8634: Made changes to test_sec...

[kishankavala]

Github user kishankavala commented on the pull request:

https://github.com/apache/cloudstack/pull/586#issuecomment-123971770
  
LGTM


---
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-8651: [Browser Based Upload Te...

[karuturi]

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

https://github.com/apache/cloudstack/pull/607#discussion_r35290816
  
--- Diff: test/integration/component/test_browse_templates.py ---
@@ -1674,6 +1674,42 @@ def 
test_09_Browser_Upload_Volume_secondary_storage_resource_limits_after_deleti
 return
 
 
+@attr(tags = ["advanced", "advancedns", "smoke", "basic"], 
required_hardware="false")
+def test_browser_upload_template_incomplete(self):
+"""
+Test browser based incomplete template upload, followed by SSVM 
destroy. Template should go to UploadAbandoned state and get cleaned up.
+"""
+try:
+self.debug("= Test browser based 
incomplete template upload ")
+
+#Only register template, without uploading
+cmd = 
getUploadParamsForTemplate.getUploadParamsForTemplateCmd()
+cmd.zoneid = self.zone.id
+cmd.format = self.uploadtemplateformat
+
cmd.name=self.templatename+self.account.name+(random.choice(string.ascii_uppercase))
+cmd.account=self.account.name
+cmd.domainid=self.domain.id
+cmd.displaytext=cmd.name
+cmd.hypervisor=self.templatehypervisor
+cmd.ostypeid=self.templateostypeid
+
template_response=self.apiclient.getUploadParamsForTemplate(cmd)
+
+#Destroy SSVM, and wait for new one to start
+self.destroy_ssvm()
+
+#Verify that the template is cleaned up as part of sync-up 
during new SSVM start
+list_template_response=Template.list(
+self.apiclient,
+id=template_response.id,
+templatefilter="all",
+zoneid=self.zone.id)
+self.assertEqual(list_template_response, None, "Template is 
not cleaned up, some issue with template sync-up")
+
--- End diff --

Can you add an assert for UploadAbandoned state of the template? Right now 
we are just asserting that the template is not active


---
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-8596 ability to query nested g...

[koushik-das]

Github user koushik-das commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/609#discussion_r35291227
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapConfiguration.java
 ---
@@ -36,6 +36,8 @@
 
 private static final ConfigKey ldapPageSize = new 
ConfigKey(Integer.class, "ldap.request.page.size", "Advanced", "1000",

"page size sent to ldap server on each request to get user", true, 
ConfigKey.Scope.Global, 1);
+private static final ConfigKey ldapProvider = new 
ConfigKey(String.class, "ldap.provider", "Advanced", "openldap", "ldap 
provider ex:openldap, microsoftad",
+   
 true, ConfigKey.Scope.Global, null);
--- End diff --

As part of this change please move all LDAP related config to use the 
ConfigKey/Configurable mechanism of publishing them. 


---
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: Dereference NULL return value

[karuturi]

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

https://github.com/apache/cloudstack/pull/618#discussion_r35291257
  
--- Diff: 
plugins/user-authenticators/saml2/src/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java
 ---
@@ -368,8 +368,14 @@ private boolean setup() {
 _idpMetaDataProvider = new HTTPMetadataProvider(_timer, 
client, idpMetaDataUrl);
 } else {
 File metadataFile = 
PropertiesUtil.findConfigFile(idpMetaDataUrl);
-s_logger.debug("Provided Metadata is not a URL, trying to 
read metadata file from local path: " + metadataFile.getAbsolutePath());
-_idpMetaDataProvider = new 
FilesystemMetadataProvider(_timer, metadataFile);
+if (metadataFile == null) {
+s_logger.error("Metadata file returned null");
--- End diff --

Can you also add the file path its trying to fing (idpMetaDataUrl) to the 
error message?


---
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-8596 ability to query nested g...

[karuturi]

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

https://github.com/apache/cloudstack/pull/609#discussion_r35291594
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapConfiguration.java
 ---
@@ -36,6 +36,8 @@
 
 private static final ConfigKey ldapPageSize = new 
ConfigKey(Integer.class, "ldap.request.page.size", "Advanced", "1000",

"page size sent to ldap server on each request to get user", true, 
ConfigKey.Scope.Global, 1);
+private static final ConfigKey ldapProvider = new 
ConfigKey(String.class, "ldap.provider", "Advanced", "openldap", "ldap 
provider ex:openldap, microsoftad",
+   
 true, ConfigKey.Scope.Global, null);
--- End diff --

no. multiple providers are not possible. 
I havent touched any of the existing configurations as I will be 
refactoring them along with 
https://cwiki.apache.org/confluence/display/CLOUDSTACK/WIP%3A+LDAP%3A+Trust+AD+and+Auto+Import


---
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-8596 ability to query nested g...

[koushik-das]

Github user koushik-das commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/609#discussion_r35291585
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java
 ---
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cloudstack.ldap;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.naming.NamingEnumeration;
+import javax.naming.NamingException;
+import javax.naming.directory.SearchControls;
+import javax.naming.directory.SearchResult;
+import javax.naming.ldap.LdapContext;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.log4j.Logger;
+
+public class ADLdapUserManagerImpl extends OpenLdapUserManagerImpl 
implements LdapUserManager {
+public static final Logger s_logger = 
Logger.getLogger(ADLdapUserManagerImpl.class.getName());
+
+@Override
+public List getUsersInGroup(String groupName, LdapContext 
context) throws NamingException {
+final SearchControls searchControls = new SearchControls();
+searchControls.setSearchScope(_ldapConfiguration.getScope());
+
searchControls.setReturningAttributes(_ldapConfiguration.getReturnAttributes());
+
+String basedn = _ldapConfiguration.getBaseDn();
+if (StringUtils.isBlank(basedn)) {
+throw new IllegalArgumentException("ldap basedn is not 
configured");
--- End diff --

Isn't Basedn a required config? In that case it should be validated while 
configuring the ldap manager and not every time with these operations.  


---
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-8596 ability to query nested g...

[karuturi]

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

https://github.com/apache/cloudstack/pull/609#discussion_r35291761
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java
 ---
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cloudstack.ldap;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.naming.NamingEnumeration;
+import javax.naming.NamingException;
+import javax.naming.directory.SearchControls;
+import javax.naming.directory.SearchResult;
+import javax.naming.ldap.LdapContext;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.log4j.Logger;
+
+public class ADLdapUserManagerImpl extends OpenLdapUserManagerImpl 
implements LdapUserManager {
+public static final Logger s_logger = 
Logger.getLogger(ADLdapUserManagerImpl.class.getName());
+
+@Override
+public List getUsersInGroup(String groupName, LdapContext 
context) throws NamingException {
+final SearchControls searchControls = new SearchControls();
+searchControls.setSearchScope(_ldapConfiguration.getScope());
+
searchControls.setReturningAttributes(_ldapConfiguration.getReturnAttributes());
+
+String basedn = _ldapConfiguration.getBaseDn();
+if (StringUtils.isBlank(basedn)) {
+throw new IllegalArgumentException("ldap basedn is not 
configured");
--- End diff --

basedn comes from global config. Currently, there is no way to ensure that 
this configuration is not null when ldap is enabled. This will be handled once 
I move all the ldap configurations to the ldap_configuration table and not the 
global configuration table.


---
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-8659: Verify presentation of v...

[pritisarap12]

Github user pritisarap12 commented on the pull request:

https://github.com/apache/cloudstack/pull/613#issuecomment-123986839
  
Updated the testcase as per review comments.


---
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: Added a null value exception in LibvirtBa...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/617#issuecomment-123987028
  
Hi @manuiiit 

I checked the LibvirtBackupSnapshotCommandWrapper class and its coverage, 
which is still poor compared to the other classes in the KVM hypervisor plugin.

Please, proceed with the changes @karuturi suggested and I will increase 
the tests on the class in a separate PR.

It is currently 6.9% covered, which is very bad.

Cheers,
Wilder


---
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-8660 - StringUtilsTest.testGet...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/616#issuecomment-123987490
  
Thanks for the LGTM, @karuturi :)

2 LGTM, merging...


---
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-8660 - StringUtilsTest.testGet...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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-8596 ability to query nested g...

[koushik-das]

Github user koushik-das commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/609#discussion_r35292477
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java
 ---
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cloudstack.ldap;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.naming.NamingEnumeration;
+import javax.naming.NamingException;
+import javax.naming.directory.SearchControls;
+import javax.naming.directory.SearchResult;
+import javax.naming.ldap.LdapContext;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.log4j.Logger;
+
+public class ADLdapUserManagerImpl extends OpenLdapUserManagerImpl 
implements LdapUserManager {
+public static final Logger s_logger = 
Logger.getLogger(ADLdapUserManagerImpl.class.getName());
+
+@Override
+public List getUsersInGroup(String groupName, LdapContext 
context) throws NamingException {
+final SearchControls searchControls = new SearchControls();
+searchControls.setSearchScope(_ldapConfiguration.getScope());
+
searchControls.setReturningAttributes(_ldapConfiguration.getReturnAttributes());
+
+String basedn = _ldapConfiguration.getBaseDn();
+if (StringUtils.isBlank(basedn)) {
+throw new IllegalArgumentException("ldap basedn is not 
configured");
+}
+
+if (StringUtils.isBlank(groupName)) {
--- End diff --

This should be at the beginning.


---
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-8596 ability to query nested g...

[koushik-das]

Github user koushik-das commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/609#discussion_r35292541
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java
 ---
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cloudstack.ldap;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.naming.NamingEnumeration;
+import javax.naming.NamingException;
+import javax.naming.directory.SearchControls;
+import javax.naming.directory.SearchResult;
+import javax.naming.ldap.LdapContext;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.log4j.Logger;
+
+public class ADLdapUserManagerImpl extends OpenLdapUserManagerImpl 
implements LdapUserManager {
+public static final Logger s_logger = 
Logger.getLogger(ADLdapUserManagerImpl.class.getName());
+
+@Override
+public List getUsersInGroup(String groupName, LdapContext 
context) throws NamingException {
+final SearchControls searchControls = new SearchControls();
+searchControls.setSearchScope(_ldapConfiguration.getScope());
+
searchControls.setReturningAttributes(_ldapConfiguration.getReturnAttributes());
+
+String basedn = _ldapConfiguration.getBaseDn();
+if (StringUtils.isBlank(basedn)) {
+throw new IllegalArgumentException("ldap basedn is not 
configured");
+}
+
+if (StringUtils.isBlank(groupName)) {
+throw new IllegalArgumentException("ldap group name cannot be 
blank");
+}
+
+NamingEnumeration results = context.search(basedn, 
generateADGroupSearchFilter(groupName), searchControls);
+final List users = new ArrayList();
+while (results.hasMoreElements()) {
+final SearchResult result = results.nextElement();
+users.add(createUser(result));
+}
+return users;
+}
+
+private String generateADGroupSearchFilter(String groupName) {
+final StringBuilder userObjectFilter = new StringBuilder();
+userObjectFilter.append("(objectClass=");
+userObjectFilter.append(_ldapConfiguration.getUserObject());
+userObjectFilter.append(")");
+
+final StringBuilder memberOfFilter = new StringBuilder();
+String groupCnName =  _ldapConfiguration.getCommonNameAttribute() 
+ "=" +groupName + "," +  _ldapConfiguration.getBaseDn();
+memberOfFilter.append("(memberOf:1.2.840.113556.1.4.1941:=");
--- End diff --

What does the numbers mean? Can we have a meaningful name for the constant?


---
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-8596 ability to query nested g...

[koushik-das]

Github user koushik-das commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/609#discussion_r35292608
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java
 ---
@@ -57,17 +57,22 @@
 private LdapContextFactory _ldapContextFactory;
 
 @Inject
-private LdapUserManager _ldapUserManager;
+private LdapConfiguration _ldapConfiguration;
+
+@Inject LdapUserManagerFactory _ldapUserManagerFactory;
+
 
 public LdapManagerImpl() {
 super();
 }
 
-public LdapManagerImpl(final LdapConfigurationDao 
ldapConfigurationDao, final LdapContextFactory ldapContextFactory, final 
LdapUserManager ldapUserManager) {
+public LdapManagerImpl(final LdapConfigurationDao 
ldapConfigurationDao, final LdapContextFactory ldapContextFactory, final 
LdapUserManagerFactory ldapUserManagerFactory,
--- End diff --

Please put a comment that this is test-only


---
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-8649: Fixed unnecessary double...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/615#issuecomment-123990021
  
Okay, we got a LGTM from @kishankavala and 1 from me.

The parameters are being decoded in the 
ApiServlet.processRequestInContext() method.

Cheers,
Wilder


---
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-8649: Fixed unnecessary double...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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-8649: Fixed unnecessary double...

[karuturi]

Github user karuturi commented on the pull request:

https://github.com/apache/cloudstack/pull/615#issuecomment-123990422
  
encoding is done from javascript 
https://github.com/apache/cloudstack/blob/master/ui/scripts/accounts.js#L1816
I think we should either update the apidoc to send an encoded value or 
remove the encoding in js.
This change would break the functionality from UI hence :-1: 


---
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-8649: Fixed unnecessary double...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/615#issuecomment-123991576
  
Hi @karuturi 

I merged just 1min before your comment.

What change do you mean? The one @borisroman is trying to fix?

Please, see my comment above.

Cheers,
Wilder


---
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-8596 ability to query nested g...

[karuturi]

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

https://github.com/apache/cloudstack/pull/609#discussion_r35292815
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java
 ---
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cloudstack.ldap;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.naming.NamingEnumeration;
+import javax.naming.NamingException;
+import javax.naming.directory.SearchControls;
+import javax.naming.directory.SearchResult;
+import javax.naming.ldap.LdapContext;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.log4j.Logger;
+
+public class ADLdapUserManagerImpl extends OpenLdapUserManagerImpl 
implements LdapUserManager {
+public static final Logger s_logger = 
Logger.getLogger(ADLdapUserManagerImpl.class.getName());
+
+@Override
+public List getUsersInGroup(String groupName, LdapContext 
context) throws NamingException {
+final SearchControls searchControls = new SearchControls();
+searchControls.setSearchScope(_ldapConfiguration.getScope());
+
searchControls.setReturningAttributes(_ldapConfiguration.getReturnAttributes());
+
+String basedn = _ldapConfiguration.getBaseDn();
+if (StringUtils.isBlank(basedn)) {
+throw new IllegalArgumentException("ldap basedn is not 
configured");
+}
+
+if (StringUtils.isBlank(groupName)) {
+throw new IllegalArgumentException("ldap group name cannot be 
blank");
+}
+
+NamingEnumeration results = context.search(basedn, 
generateADGroupSearchFilter(groupName), searchControls);
+final List users = new ArrayList();
+while (results.hasMoreElements()) {
+final SearchResult result = results.nextElement();
+users.add(createUser(result));
+}
+return users;
+}
+
+private String generateADGroupSearchFilter(String groupName) {
+final StringBuilder userObjectFilter = new StringBuilder();
+userObjectFilter.append("(objectClass=");
+userObjectFilter.append(_ldapConfiguration.getUserObject());
+userObjectFilter.append(")");
+
+final StringBuilder memberOfFilter = new StringBuilder();
+String groupCnName =  _ldapConfiguration.getCommonNameAttribute() 
+ "=" +groupName + "," +  _ldapConfiguration.getBaseDn();
+memberOfFilter.append("(memberOf:1.2.840.113556.1.4.1941:=");
--- End diff --

thats the AD filter to get nested group users. will move it to a constant.


---
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-8596 ability to query nested g...

[karuturi]

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

https://github.com/apache/cloudstack/pull/609#discussion_r35292860
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java
 ---
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cloudstack.ldap;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.naming.NamingEnumeration;
+import javax.naming.NamingException;
+import javax.naming.directory.SearchControls;
+import javax.naming.directory.SearchResult;
+import javax.naming.ldap.LdapContext;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.log4j.Logger;
+
+public class ADLdapUserManagerImpl extends OpenLdapUserManagerImpl 
implements LdapUserManager {
+public static final Logger s_logger = 
Logger.getLogger(ADLdapUserManagerImpl.class.getName());
+
+@Override
+public List getUsersInGroup(String groupName, LdapContext 
context) throws NamingException {
+final SearchControls searchControls = new SearchControls();
+searchControls.setSearchScope(_ldapConfiguration.getScope());
+
searchControls.setReturningAttributes(_ldapConfiguration.getReturnAttributes());
+
+String basedn = _ldapConfiguration.getBaseDn();
+if (StringUtils.isBlank(basedn)) {
+throw new IllegalArgumentException("ldap basedn is not 
configured");
+}
+
+if (StringUtils.isBlank(groupName)) {
--- End diff --

will do


---
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-8596 ability to query nested g...

[karuturi]

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

https://github.com/apache/cloudstack/pull/609#discussion_r35292915
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java
 ---
@@ -57,17 +57,22 @@
 private LdapContextFactory _ldapContextFactory;
 
 @Inject
-private LdapUserManager _ldapUserManager;
+private LdapConfiguration _ldapConfiguration;
+
+@Inject LdapUserManagerFactory _ldapUserManagerFactory;
+
 
 public LdapManagerImpl() {
 super();
 }
 
-public LdapManagerImpl(final LdapConfigurationDao 
ldapConfigurationDao, final LdapContextFactory ldapContextFactory, final 
LdapUserManager ldapUserManager) {
+public LdapManagerImpl(final LdapConfigurationDao 
ldapConfigurationDao, final LdapContextFactory ldapContextFactory, final 
LdapUserManagerFactory ldapUserManagerFactory,
--- End diff --

this is not added now. its an existing one which is modified. I am planning 
on removing them in the future commits as there are other ways to inject them 
to tests.


---
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-8649: Fixed unnecessary double...

[karuturi]

Github user karuturi commented on the pull request:

https://github.com/apache/cloudstack/pull/615#issuecomment-123995367
  
ok.I thought thought this commit removed decoding and the api now expects a 
decoded value as input. 
I understand now. the issue is with double decoding we removed the second 
layer of it. 
I take back my -1


---
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-8650: Fix securitygroups ingre...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/601#issuecomment-123995530
  
Awesome @franklouwers ! Thanks for the details.

LGTM :+1: Merging...


---
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-8650: Fix securitygroups ingre...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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-8596 ability to query nested g...

[koushik-das]

Github user koushik-das commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/609#discussion_r35293493
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManagerFactory.java
 ---
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cloudstack.ldap;
+
+import org.apache.log4j.Logger;
+import org.springframework.beans.BeansException;
+import org.springframework.beans.factory.config.AutowireCapableBeanFactory;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.ApplicationContextAware;
+
+public class LdapUserManagerFactory implements ApplicationContextAware {
+public static final Logger s_logger = 
Logger.getLogger(LdapUserManagerFactory.class.getName());
+
+private static LdapUserManager adUserManager;
--- End diff --

Looks like both providers can co-exists. In that case it is better to have 
a map .


---
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-8596 ability to query nested g...

[koushik-das]

Github user koushik-das commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/609#discussion_r35293580
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManagerFactory.java
 ---
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cloudstack.ldap;
+
+import org.apache.log4j.Logger;
+import org.springframework.beans.BeansException;
+import org.springframework.beans.factory.config.AutowireCapableBeanFactory;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.ApplicationContextAware;
+
+public class LdapUserManagerFactory implements ApplicationContextAware {
+public static final Logger s_logger = 
Logger.getLogger(LdapUserManagerFactory.class.getName());
+
+private static LdapUserManager adUserManager;
+private static LdapUserManager openLdapUserManager;
+
+static ApplicationContext applicationCtx;
+
+public LdapUserManager getInstance(String id) {
--- End diff --

Can we rewrite like

LdapUserManager manager;
if (microsoft) {}
else {}
applicationCtx.getAutowire…
return manager;


---
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-8580: user can view, expunge a...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/593#issuecomment-123996760
  
@wido @DaanHoogland @borisroman @runseb 

I will test it manually before proceeding with a merge. I will also add an 
integration test for that and send in a new PR.

Cheers,
Wilder


---
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-8596 ability to query nested g...

[koushik-das]

Github user koushik-das commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/609#discussion_r35293652
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java
 ---
@@ -0,0 +1,233 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.ldap;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import javax.inject.Inject;
+import javax.naming.NamingEnumeration;
+import javax.naming.NamingException;
+import javax.naming.directory.Attribute;
+import javax.naming.directory.Attributes;
+import javax.naming.directory.SearchControls;
+import javax.naming.directory.SearchResult;
+import javax.naming.ldap.Control;
+import javax.naming.ldap.LdapContext;
+import javax.naming.ldap.PagedResultsControl;
+import javax.naming.ldap.PagedResultsResponseControl;
+
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang.StringUtils;
+import org.apache.log4j.Logger;
+
+public class OpenLdapUserManagerImpl implements LdapUserManager {
+private static final Logger s_logger = 
Logger.getLogger(OpenLdapUserManagerImpl.class.getName());
+
+@Inject
+protected LdapConfiguration _ldapConfiguration;
+
+public OpenLdapUserManagerImpl() {
+}
+
+public OpenLdapUserManagerImpl(final LdapConfiguration 
ldapConfiguration) {
+_ldapConfiguration = ldapConfiguration;
+}
+
+protected LdapUser createUser(final SearchResult result) throws 
NamingException {
+final Attributes attributes = result.getAttributes();
+
+final String username = LdapUtils.getAttributeValue(attributes, 
_ldapConfiguration.getUsernameAttribute());
+final String email = LdapUtils.getAttributeValue(attributes, 
_ldapConfiguration.getEmailAttribute());
+final String firstname = LdapUtils.getAttributeValue(attributes, 
_ldapConfiguration.getFirstnameAttribute());
+final String lastname = LdapUtils.getAttributeValue(attributes, 
_ldapConfiguration.getLastnameAttribute());
+final String principal = result.getNameInNamespace();
+
+String domain = principal.replace("cn=" + 
LdapUtils.getAttributeValue(attributes, 
_ldapConfiguration.getCommonNameAttribute()) + ",", "");
+domain = domain.replace("," + _ldapConfiguration.getBaseDn(), "");
+domain = domain.replace("ou=", "");
+
+return new LdapUser(username, email, firstname, lastname, 
principal, domain);
+}
+
+private String generateSearchFilter(final String username) {
+final StringBuilder userObjectFilter = new StringBuilder();
+userObjectFilter.append("(objectClass=");
+userObjectFilter.append(_ldapConfiguration.getUserObject());
+userObjectFilter.append(")");
+
+final StringBuilder usernameFilter = new StringBuilder();
+usernameFilter.append("(");
+usernameFilter.append(_ldapConfiguration.getUsernameAttribute());
+usernameFilter.append("=");
+usernameFilter.append((username == null ? "*" : username));
+usernameFilter.append(")");
+
+final StringBuilder memberOfFilter = new StringBuilder();
+if (_ldapConfiguration.getSearchGroupPrinciple() != null) {
+memberOfFilter.append("(memberof=");
+
memberOfFilter.append(_ldapConfiguration.getSearchGroupPrinciple());
+memberOfFilter.append(")");
+}
+
+final StringBuilder result = new StringBuilder();
+result.append("(&");
+result.append(userObjectFilter);
+result.append(usernameFilter);
+result.append(memberOfFilter);
+result.append(")");
+
+return result.toString();
+}
+
+private String generateGroupSearchFilt

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

[karuturi]

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

https://github.com/apache/cloudstack/pull/609#discussion_r35293673
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManagerFactory.java
 ---
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cloudstack.ldap;
+
+import org.apache.log4j.Logger;
+import org.springframework.beans.BeansException;
+import org.springframework.beans.factory.config.AutowireCapableBeanFactory;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.ApplicationContextAware;
+
+public class LdapUserManagerFactory implements ApplicationContextAware {
+public static final Logger s_logger = 
Logger.getLogger(LdapUserManagerFactory.class.getName());
+
+private static LdapUserManager adUserManager;
+private static LdapUserManager openLdapUserManager;
+
+static ApplicationContext applicationCtx;
+
+public LdapUserManager getInstance(String id) {
--- End diff --

The issue with that is, if the provider configuration is changed, 
management server restart is required.


---
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-8596 ability to query nested g...

[koushik-das]

Github user koushik-das commented on the pull request:

https://github.com/apache/cloudstack/pull/609#issuecomment-123998336
  
Once the comments are addressed, LGTM.


---
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-8596 ability to query nested g...

[karuturi]

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

https://github.com/apache/cloudstack/pull/609#discussion_r35294032
  
--- Diff: 
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManagerFactory.java
 ---
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cloudstack.ldap;
+
+import org.apache.log4j.Logger;
+import org.springframework.beans.BeansException;
+import org.springframework.beans.factory.config.AutowireCapableBeanFactory;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.ApplicationContextAware;
+
+public class LdapUserManagerFactory implements ApplicationContextAware {
+public static final Logger s_logger = 
Logger.getLogger(LdapUserManagerFactory.class.getName());
+
+private static LdapUserManager adUserManager;
--- End diff --

I will move them to a map


---
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: Added a null value exception in LibvirtBa...

[manuiiit]

Github user manuiiit commented on the pull request:

https://github.com/apache/cloudstack/pull/617#issuecomment-123998699
  
Hi Wilder,
  The LibvirtBackupSnapshotCommandWrapper class has no unit
tests.Can I add test cases that covers the exception.

Regards,
P.Maneesha.


On Thu, Jul 23, 2015 at 11:35 AM, Wilder Rodrigues  wrote:

> Hi @manuiiit 
>
> I checked the LibvirtBackupSnapshotCommandWrapper class and its coverage,
> which is still poor compared to the other classes in the KVM hypervisor
> plugin.
>
> Please, proceed with the changes @karuturi 
> suggested and I will increase the tests on the class in a separate PR.
>
> It is currently 6.9% covered, which is very bad.
>
> Cheers,
> Wilder
>
> —
> Reply to this email directly or view it on GitHub
> .
>



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