[GitHub] cloudstack pull request: Remodeling of Nuage VSP Plugin + CLOUDSTA...

[remibergsma]

Github user remibergsma commented on the pull request:

https://github.com/apache/cloudstack/pull/1494#issuecomment-209893539
  
@nlivens Please add a description because this confuses me ;-) You guys go 
the non-oss way? :-(



---
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-9003] Resource Naming Policie...

[ProjectMoon]

Github user ProjectMoon commented on the pull request:

https://github.com/apache/cloudstack/pull/1492#issuecomment-209875708
  
> @ProjectMoon correct resource naming is critical to the proper operation 
of the management server. We have had a significant bugs and production issues 
caused by subtle changes to resource naming strategies between releases where 
CloudStack suddenly can't find a resource on the device it is attempting to 
control. 

> Have you performed any upgrade testing for this PR? If so, what tests 
have you performed in which configurations?

We have not specifically performed any upgrade testing. Our current stable 
version is based on 4.7.1, and essentially our "upgrade testing" has consisted 
of deploying 4.7.1 before and after our development of this feature is 
complete. The configuration has been tested with KVM, VmWare, and the 
simulator. 

> Also, could you please add an FS to the wiki and start a conversation on 
dev@? Given the importance of resource naming, it would be extremely helpful to 
have an explanation of the design and its operation.

As far as I know, I don't have access to the wiki. Otherwise I would add to 
it. Do I need to register a separate account or can I use my Apache JIRA 
account?

> @ProjectMoon we'll need more information on why you're doing this, why we 
should have it, what is fixes and will it guarantee backward resource-name 
compatibility (for example, vmware vms have this strictly tie up with internal 
ACS vm name, such config are set in each vmware's VM's annotations) and upgrade 
paths

* **Why we are doing this**: we implement our own naming scheme for the 
supported resources. On our 4.2 branch we hacked this in, but now we want to 
present a framework that we can extend, and open the possibilities to other.
* **Why should mainline have this**: More flexibility for developers, 
easier testing (static classes notoriously cause problems), a unified way to 
generate names (DRY principle).
* **What it fixes**: It doesn't _fix_ anything per se, but the refactoring 
helps move us towards a cleaner codebase.
* **Backwards compatibility**: The default plugin generates names and UUIDs 
in the exact same way as before, so yes.

VmWare is an interesting case though. Can you describe in more detail/point 
me to where this stuff happens so I can verify that custom naming plugins will 
not break it?


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

[GitHub] cloudstack pull request: CLOUDSTACK-9348: Use non-blocking SSL han...

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/1493#issuecomment-209896417
  
- Tested against KVM, mgmt server - KVM links and clustered management 
server
- NioTest modified to have multiple clients against a server instance with 
just one worker and 10 malicious clients (they simply do a secure connect to 
the server and don't do anything else) trying to connect server per valid client
- Ran Marvin smoke tests successfully against KVM


---
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: utils: Use non-blocking SSL handshake in ...

[bhaisaab]

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

https://github.com/apache/cloudstack/pull/1493#discussion_r59671416
  
--- Diff: utils/src/main/java/com/cloud/utils/nio/Link.java ---
@@ -453,115 +449,150 @@ public static SSLContext initSSLContext(boolean 
isClient) throws GeneralSecurity
 return sslContext;
 }
 
-public static void doHandshake(SocketChannel ch, SSLEngine sslEngine, 
boolean isClient) throws IOException {
-if (s_logger.isTraceEnabled()) {
-s_logger.trace("SSL: begin Handshake, isClient: " + isClient);
+public static ByteBuffer enlargeBuffer(ByteBuffer buffer, final int 
sessionProposedCapacity) {
+if (sessionProposedCapacity > buffer.capacity()) {
+buffer = ByteBuffer.allocate(sessionProposedCapacity);
+} else {
+buffer = ByteBuffer.allocate(buffer.capacity() * 2);
 }
+return buffer;
--- End diff --

Yes


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

Feature proposal: Resource naming policies

[Jeff Hair]

Yesterday, we submitted this pull request:
https://github.com/apache/cloudstack/pull/1492

This originally grew out of making the VirtualMachineName class non-static
(original PR is mentioned in the above link). We're presenting this as a
refactoring of the existing code to enable more extensibility and
flexibility, make unit testing easier, and unify the way CloudStack
generates resource names.

There is an associated JIRA ticket at CLOUDSTACK-9003. I will be writing up
a design document for the CS wiki in the next few days.

jburwell wanted me to open a discussion on the developer list about the PR
and how it is implemented:

There is now a ResourceNamingPolicyManager and a bunch of
ResourceNamingPolicies. The manager exposes a method to get a policy for a
type of resource, and the naming policies generate UUIDs + resource names.

The default naming policies generate names exactly the same way as they are
created now. This is in a new module called default-naming-policies. By
excluding this module and loading our own naming policies, we gain the
ability to change how names are generated.

[GitHub] cloudstack pull request: Remodeling of Nuage VSP Plugin + CLOUDSTA...

[nlivens]

Github user nlivens commented on the pull request:

https://github.com/apache/cloudstack/pull/1494#issuecomment-209912550
  
@remibergsma, I've added description :)


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

[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...

[mike-tutkowski]

Github user mike-tutkowski commented on the pull request:

https://github.com/apache/cloudstack/pull/1403#issuecomment-210081474
  
@swill I just pushed a new commit to rename the SolidFire integration 
testing project (which you saw as failing).

While that should not have anything to do with a failure, I'd be curious to 
have you re-run your build because ApiSolidFireServiceImpl (which was failing) 
should not even be a part of the codebase in that SHA.

Thanks!


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

[GitHub] cloudstack pull request: Fix Sync of template.properties in Swift

[jburwell]

Github user jburwell commented on the pull request:

https://github.com/apache/cloudstack/pull/1331#issuecomment-210081688
  
@rafaelweingartner The issue is not one of style but performance and 
availability.  The garbage collector skips reachability evaluations of ``final 
static`` fields where it must perform those for instance variables.  This cost 
adds up when a large number of object is allocated and deallocated.  Second, 
you cannot log from a static method.  Our standard in the codebase is to have 
``final static`` loggers, and I don't see a compelling reason to step away from 
it.

In terms of the Mockito approach, I wouldn't have an opinion if the unit 
test where not requiring us to change the class.  The test should always be the 
servant of the system not the other way around.  Personally, I find mocks to be 
an obfuscation especially when the thing being mocked provides a standard 
mechanism to observe side-effects.


---
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-8562: Dynamic Role-Based API C...

[bhaisaab]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59773313
  
--- Diff: api/src/org/apache/cloudstack/acl/RoleType.java ---
@@ -16,18 +16,90 @@
 // under the License.
 package org.apache.cloudstack.acl;
 
+import com.cloud.user.Account;
+import com.google.common.base.Enums;
+import com.google.common.base.Strings;
+
 // Enum for default roles in CloudStack
 public enum RoleType {
-Admin(1), ResourceAdmin(2), DomainAdmin(4), User(8), Unknown(0);
+Admin(1L, Account.ACCOUNT_TYPE_ADMIN, 1),
+ResourceAdmin(2L, Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN, 2),
+DomainAdmin(3L, Account.ACCOUNT_TYPE_DOMAIN_ADMIN, 4),
+User(4L, Account.ACCOUNT_TYPE_NORMAL, 8),
+Unknown(-1L, (short) -1, 0);
 
+private long id;
+private short accountType;
 private int mask;
 
-private RoleType(int mask) {
+RoleType(final long id, final short accountType, final int mask) {
+this.id = id;
+this.accountType = accountType;
 this.mask = mask;
 }
 
-public int getValue() {
+public long getId() {
+return id;
+}
+
+public short getAccountType() {
+return accountType;
+}
+
+public int getMask() {
 return mask;
 }
-}
 
+public static RoleType fromString(final String name) {
+if (!Strings.isNullOrEmpty(name)
+&& Enums.getIfPresent(RoleType.class, name).isPresent()) {
+return RoleType.valueOf(name);
+}
+return null;
+}
+
+public static RoleType fromMask(int mask) {
+for (RoleType roleType : RoleType.values()) {
+if (roleType.getMask() == mask) {
+return roleType;
+}
+}
+return Unknown;
+}
+
+public static RoleType getByAccountType(final short accountType) {
+RoleType roleType = RoleType.Unknown;
+switch (accountType) {
+case Account.ACCOUNT_TYPE_ADMIN:
+roleType = RoleType.Admin;
+break;
+case Account.ACCOUNT_TYPE_DOMAIN_ADMIN:
+roleType = RoleType.DomainAdmin;
+break;
+case Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN:
+roleType = RoleType.ResourceAdmin;
+break;
+case Account.ACCOUNT_TYPE_NORMAL:
+roleType = RoleType.User;
+break;
+}
+return roleType;
+}
+
+public static Long getRoleByAccountType(final Long roleId, final Short 
accountType) {
--- End diff --

This is for doing backward compatiblity in create APIs when roleId passed 
is null, but an account type is passed. This does the translation using above 
method.


---
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-9003] Resource Naming Policie...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1492#issuecomment-210081172
  
np, imagined that


---
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-8562: Dynamic Role-Based API C...

[bhaisaab]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59773107
  
--- Diff: api/src/org/apache/cloudstack/acl/RoleType.java ---
@@ -16,18 +16,90 @@
 // under the License.
 package org.apache.cloudstack.acl;
 
+import com.cloud.user.Account;
+import com.google.common.base.Enums;
+import com.google.common.base.Strings;
+
 // Enum for default roles in CloudStack
 public enum RoleType {
-Admin(1), ResourceAdmin(2), DomainAdmin(4), User(8), Unknown(0);
+Admin(1L, Account.ACCOUNT_TYPE_ADMIN, 1),
+ResourceAdmin(2L, Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN, 2),
+DomainAdmin(3L, Account.ACCOUNT_TYPE_DOMAIN_ADMIN, 4),
+User(4L, Account.ACCOUNT_TYPE_NORMAL, 8),
+Unknown(-1L, (short) -1, 0);
 
+private long id;
+private short accountType;
 private int mask;
 
-private RoleType(int mask) {
+RoleType(final long id, final short accountType, final int mask) {
+this.id = id;
+this.accountType = accountType;
 this.mask = mask;
 }
 
-public int getValue() {
+public long getId() {
+return id;
+}
+
+public short getAccountType() {
+return accountType;
+}
+
+public int getMask() {
 return mask;
 }
-}
 
+public static RoleType fromString(final String name) {
+if (!Strings.isNullOrEmpty(name)
+&& Enums.getIfPresent(RoleType.class, name).isPresent()) {
+return RoleType.valueOf(name);
+}
+return null;
+}
+
+public static RoleType fromMask(int mask) {
+for (RoleType roleType : RoleType.values()) {
+if (roleType.getMask() == mask) {
+return roleType;
+}
+}
+return Unknown;
+}
+
+public static RoleType getByAccountType(final short accountType) {
+RoleType roleType = RoleType.Unknown;
+switch (accountType) {
+case Account.ACCOUNT_TYPE_ADMIN:
+roleType = RoleType.Admin;
+break;
+case Account.ACCOUNT_TYPE_DOMAIN_ADMIN:
+roleType = RoleType.DomainAdmin;
+break;
+case Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN:
+roleType = RoleType.ResourceAdmin;
--- End diff --

All four default cloudstack roles -- Admin, DomainAdmin, ResourceAdmin and 
User. It's just an enum class, that maps these role types to account types. 
This method previously existed in account mgr impl, I moved it here.
They've always existed. I could not find a previous FS on this, this is the 
oldest document on wiki referring to RoleTypes 
https://cwiki.apache.org/confluence/display/CLOUDSTACK/Annotations+use+in+the+API


---
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-8562: Dynamic Role-Based API C...

[jburwell]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59788370
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/acl/CreateRoleCmd.java ---
@@ -0,0 +1,104 @@
+// 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.api.command.admin.acl;
+
+import com.cloud.user.Account;
+import com.google.common.base.Strings;
+import org.apache.cloudstack.acl.Role;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.RoleResponse;
+import org.apache.cloudstack.context.CallContext;
+
+@APICommand(name = CreateRoleCmd.APINAME, description = "Creates a role", 
responseObject = RoleResponse.class,
+requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
+since = "4.9.0",
+authorized = {RoleType.Admin})
+public class CreateRoleCmd extends BaseCmd {
+public static final String APINAME = "createRole";
+
+/
+ API parameters /
+/
+
+@Parameter(name = ApiConstants.NAME, type = CommandType.STRING, 
required = true, description = "creates a role with this unique name")
+private String roleName;
+
+@Parameter(name = ApiConstants.TYPE, type = CommandType.STRING, 
required = true, description = "The type of the role, valid options are: Admin, 
ResourceAdmin, DomainAdmin, User")
+private String roleType;
+
+@Parameter(name = ApiConstants.DESCRIPTION, type = CommandType.STRING, 
description = "The description of the role")
+private String roleDescription;
+
+/
+/// Accessors ///
+/
+
+public String getRoleName() {
+return roleName;
+}
+
+public RoleType getRoleType() {
+return RoleType.fromString(roleType);
+}
+
+public String getRoleDescription() {
+return roleDescription;
+}
+
+/
+/// API Implementation///
+/
+
+@Override
+public String getCommandName() {
+return APINAME.toLowerCase() + BaseCmd.RESPONSE_SUFFIX;
+}
+
+@Override
+public long getEntityOwnerId() {
+return Account.ACCOUNT_ID_SYSTEM;
+}
+
+@Override
+public void execute() {
+if (Strings.isNullOrEmpty(getRoleName())) {
+throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Empty 
role name provided");
+}
+if (getRoleType() == null || getRoleType() == RoleType.Unknown) {
+throw new ServerApiException(ApiErrorCode.PARAM_ERROR, 
"Invalid role type provided");
+}
--- End diff --

Consider extracting this validation logic to a private method to make 
things more readable.


---
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: Taking fast and efficient volume snapshot...

[mike-tutkowski]

Github user mike-tutkowski commented on the pull request:

https://github.com/apache/cloudstack/pull/1403#issuecomment-210084608
  
@swill I think I might know what happened.

There used to be a ApiSolidFireServiceImpl.java, but I renamed it in 940d5b.

It seems the file is likely still sitting on your filesystem and causing 
you this trouble.


---
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: Fix Sync of template.properties in Swift

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1331#issuecomment-210086309
  
I see your point in regards to the garbage collector; but, that class is a 
singleton, it is not going to be allocated and deallocated.

About the point about of using the log in static method; well, that class 
is a singleton and I believe that they should not have static method, we are 
using a dependency injection framework, we should wire things up using the 
lifecycle of the framework we choose and not try to work things out with static 
methods.

Actually I see that the need for unit tests reflects on the code. Code with 
50, 100 and even more lines cannot be properly tested. The TDD approach affects 
a lot on the design and writing of code.



---
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-8562: Dynamic Role-Based API C...

[pdube]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59777518
  
--- Diff: api/src/org/apache/cloudstack/acl/RoleType.java ---
@@ -16,18 +16,90 @@
 // under the License.
 package org.apache.cloudstack.acl;
 
+import com.cloud.user.Account;
+import com.google.common.base.Enums;
+import com.google.common.base.Strings;
+
 // Enum for default roles in CloudStack
 public enum RoleType {
-Admin(1), ResourceAdmin(2), DomainAdmin(4), User(8), Unknown(0);
+Admin(1L, Account.ACCOUNT_TYPE_ADMIN, 1),
+ResourceAdmin(2L, Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN, 2),
+DomainAdmin(3L, Account.ACCOUNT_TYPE_DOMAIN_ADMIN, 4),
+User(4L, Account.ACCOUNT_TYPE_NORMAL, 8),
+Unknown(-1L, (short) -1, 0);
 
+private long id;
+private short accountType;
 private int mask;
 
-private RoleType(int mask) {
+RoleType(final long id, final short accountType, final int mask) {
+this.id = id;
+this.accountType = accountType;
 this.mask = mask;
 }
 
-public int getValue() {
+public long getId() {
+return id;
+}
+
+public short getAccountType() {
+return accountType;
+}
+
+public int getMask() {
 return mask;
 }
-}
 
+public static RoleType fromString(final String name) {
+if (!Strings.isNullOrEmpty(name)
+&& Enums.getIfPresent(RoleType.class, name).isPresent()) {
+return RoleType.valueOf(name);
+}
+return null;
+}
+
+public static RoleType fromMask(int mask) {
+for (RoleType roleType : RoleType.values()) {
+if (roleType.getMask() == mask) {
+return roleType;
+}
+}
+return Unknown;
+}
+
+public static RoleType getByAccountType(final short accountType) {
+RoleType roleType = RoleType.Unknown;
+switch (accountType) {
+case Account.ACCOUNT_TYPE_ADMIN:
+roleType = RoleType.Admin;
+break;
+case Account.ACCOUNT_TYPE_DOMAIN_ADMIN:
+roleType = RoleType.DomainAdmin;
+break;
+case Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN:
+roleType = RoleType.ResourceAdmin;
--- End diff --

I meant the ResourceAdmin specifically. What is this role? Is there any 
documentation on it?


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

[GitHub] cloudstack pull request: CLOUDSTACK-8562: Dynamic Role-Based API C...

[jburwell]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59786533
  
--- Diff: api/src/org/apache/cloudstack/acl/Rule.java ---
@@ -0,0 +1,42 @@
+// 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.acl;
+
+import com.cloud.exception.InvalidParameterValueException;
+import com.google.common.base.Strings;
+
+public final class Rule {
+private final String rule;
+private final static String ALLOWED_PATTERN = "^[a-zA-Z0-9*]+$";
+
+public Rule(final String rule) {
+validate(rule);
+this.rule = rule;
+}
+
+public String toString() {
+return rule;
+}
+
+public static boolean validate(final String rule) throws 
InvalidParameterValueException {
+if (Strings.isNullOrEmpty(rule) || !rule.matches(ALLOWED_PATTERN)) 
{
--- End diff --

Extract this validation logic into an immutable value class, ``Rule``, to 
encapsulate this validation logic and provide strong type specification in 
lower layers.


---
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-8562: Dynamic Role-Based API C...

[jburwell]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59787197
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java 
---
@@ -196,5 +204,8 @@ private void validateParams() {
 if(StringUtils.isEmpty(getPassword())) {
 throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Empty 
passwords are not allowed");
 }
+if (getAccountType() == null && getRoleId() == null) {
--- End diff --

Consider splitting these checks to give a more precise error message to the 
end user.  Also, shouldn't check that ``roleId`` is greater than ``0L``?


---
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-8562: Dynamic Role-Based API C...

[jburwell]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59787000
  
--- Diff: api/src/org/apache/cloudstack/acl/Rule.java ---
@@ -0,0 +1,42 @@
+// 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.acl;
+
+import com.cloud.exception.InvalidParameterValueException;
+import com.google.common.base.Strings;
+
+public final class Rule {
+private final String rule;
+private final static String ALLOWED_PATTERN = "^[a-zA-Z0-9*]+$";
--- End diff --

Aren't wildcard characters were only allowed as the first or last character 
in the rule specification?  If so, then this regex needs to be tightened up.


---
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: Taking fast and efficient volume snapshot...

[mike-tutkowski]

Github user mike-tutkowski commented on the pull request:

https://github.com/apache/cloudstack/pull/1403#issuecomment-210087782
  
No problem. :)

I had forgotten that I had renamed that file at one point. So, when I saw 
that you had a file by that name, I was really confused how you would have had 
access to a file that I had not yet checked into the repo (the current file 
with that name is for a project I'm still developing and I haven't opened a PR 
on it yet).


---
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-9348: Use non-blocking SSL han...

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/1493#issuecomment-210103368
  
I've created two commits to show: (1) test to prove denial of service 
behavior due to blocking main IO loop, (2) the fix (as mentioned earlier long 
term fix would require migration to a better framework).


---
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-8562: Dynamic Role-Based API C...

[bhaisaab]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59785020
  
--- Diff: api/src/org/apache/cloudstack/acl/RoleType.java ---
@@ -16,18 +16,90 @@
 // under the License.
 package org.apache.cloudstack.acl;
 
+import com.cloud.user.Account;
+import com.google.common.base.Enums;
+import com.google.common.base.Strings;
+
 // Enum for default roles in CloudStack
 public enum RoleType {
-Admin(1), ResourceAdmin(2), DomainAdmin(4), User(8), Unknown(0);
+Admin(1L, Account.ACCOUNT_TYPE_ADMIN, 1),
+ResourceAdmin(2L, Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN, 2),
+DomainAdmin(3L, Account.ACCOUNT_TYPE_DOMAIN_ADMIN, 4),
+User(4L, Account.ACCOUNT_TYPE_NORMAL, 8),
+Unknown(-1L, (short) -1, 0);
 
+private long id;
+private short accountType;
 private int mask;
 
-private RoleType(int mask) {
+RoleType(final long id, final short accountType, final int mask) {
+this.id = id;
+this.accountType = accountType;
 this.mask = mask;
 }
 
-public int getValue() {
+public long getId() {
+return id;
+}
+
+public short getAccountType() {
+return accountType;
+}
+
+public int getMask() {
 return mask;
 }
-}
 
+public static RoleType fromString(final String name) {
+if (!Strings.isNullOrEmpty(name)
+&& Enums.getIfPresent(RoleType.class, name).isPresent()) {
+return RoleType.valueOf(name);
+}
+return null;
+}
+
+public static RoleType fromMask(int mask) {
+for (RoleType roleType : RoleType.values()) {
+if (roleType.getMask() == mask) {
+return roleType;
+}
+}
+return Unknown;
+}
+
+public static RoleType getByAccountType(final short accountType) {
+RoleType roleType = RoleType.Unknown;
+switch (accountType) {
+case Account.ACCOUNT_TYPE_ADMIN:
+roleType = RoleType.Admin;
+break;
+case Account.ACCOUNT_TYPE_DOMAIN_ADMIN:
+roleType = RoleType.DomainAdmin;
+break;
+case Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN:
+roleType = RoleType.ResourceAdmin;
--- End diff --

@pdube I think this was used to support cpbm-cloudstack integration, I 
don't know any documentation; please google? For backward compatibility 
reasons, all four roles types supported by static-role-based-checked (as in 
commands.properties) need to be supported.


---
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-8562: Dynamic Role-Based API C...

[jburwell]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59800096
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmd.java ---
@@ -117,6 +123,12 @@ public long getEntityOwnerId() {
 
 @Override
 public void execute() {
+if (Strings.isNullOrEmpty(getCfgName())) {
+throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Empty 
configuration name provided");
+}
+if 
(getCfgName().equalsIgnoreCase(RoleService.EnableDynamicApiChecker.key())) {
+throw new ServerApiException(ApiErrorCode.METHOD_NOT_ALLOWED, 
"Restricted configuration update not allowed");
--- End diff --

Method Not Allowed (405) indicates the caller attempted to use an HTTP 
method (i.e. GET, PUT, POST, DELETE) that is not allowed for the resource.  In 
this case, the method is allowed, but the parameters do not all the method to 
be performed.  Seems like ``PARAM_ERROR`` would be more appropriate.


---
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-8562: Dynamic Role-Based API C...

[jburwell]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59801500
  
--- Diff: engine/schema/src/com/cloud/upgrade/dao/Upgrade481to490.java ---
@@ -53,6 +62,139 @@ public boolean supportsRollingUpgrade() {
 
 @Override
 public void performDataMigration(Connection conn) {
+setupRolesAndPermissionsForDynamicRBAC(conn);
+}
+
+private void createDefaultRole(final Connection conn, final Long id, 
final String name, final RoleType roleType) {
+final String insertSql = String.format("INSERT INTO 
`cloud`.`roles` (`id`, `uuid`, `name`, `role_type`, `description`) values (%d, 
UUID(), '%s', '%s', 'Default %s role');",
+id, name, roleType.name(), roleType.name().toLowerCase());
+try ( PreparedStatement updatePstmt = 
conn.prepareStatement(insertSql) ) {
+updatePstmt.executeUpdate();
+} catch (SQLException e) {
+throw new CloudRuntimeException("Unable to create default role 
with id: " + id + " name: " + name, e);
+}
+}
+
+private void createRoleMapping(final Connection conn, final Long 
roleId, final String apiName) {
+final String insertSql = String.format("INSERT INTO 
`cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`) values 
(UUID(), %d, '%s', 'ALLOW') ON DUPLICATE KEY UPDATE rule=rule;",
+roleId, apiName);
+try ( PreparedStatement updatePstmt = 
conn.prepareStatement(insertSql)) {
+updatePstmt.executeUpdate();
+} catch (SQLException ignored) {
+s_logger.debug("Unable to insert mapping for role id:" + 
roleId + " apiName: " + apiName);
+}
+}
+
+private void addRoleColumnAndMigrateAccountTable(final Connection 
conn, final RoleType[] roleTypes) {
+final String alterTableSql = "ALTER TABLE `cloud`.`account` ADD 
COLUMN `role_id` bigint(20) unsigned COMMENT 'role id for this account' AFTER 
`type`, " +
+"ADD KEY `fk_account__role_id` (`role_id`), " +
+"ADD CONSTRAINT `fk_account__role_id` FOREIGN KEY 
(`role_id`) REFERENCES `roles` (`id`);";
+try (PreparedStatement pstmt = 
conn.prepareStatement(alterTableSql)) {
+pstmt.executeUpdate();
+s_logger.info("Altered cloud.account table and added column 
role_id");
+} catch (SQLException e) {
+if (e.getMessage().contains("role_id")) {
+s_logger.warn("cloud.account table already has the role_id 
column, skipping altering table and migration of accounts");
+return;
+} else {
+throw new CloudRuntimeException("Unable to create column 
quota_calculated in table cloud_usage.cloud_usage", e);
+}
+}
+migrateAccountsToDefaultRoles(conn, roleTypes);
+}
+
+private void migrateAccountsToDefaultRoles(final Connection conn, 
final RoleType[] roleTypes) {
+try (PreparedStatement selectStatement = 
conn.prepareStatement("SELECT `id`, `type` FROM `cloud`.`account`;");
+ ResultSet selectResultSet = selectStatement.executeQuery()) {
+while (selectResultSet.next()) {
+Long accountId = selectResultSet.getLong(1);
+Short accountType = selectResultSet.getShort(2);
+Long roleId = null;
+for (RoleType roleType : roleTypes) {
+if (roleType.getAccountType() == accountType) {
+roleId = roleType.getId();
+break;
+}
+}
+if (roleId == null) {
+continue;
+}
+try (PreparedStatement updateStatement = 
conn.prepareStatement("UPDATE `cloud`.`account` SET role_id = ? WHERE id = 
?;")) {
+updateStatement.setLong(1, roleId);
+updateStatement.setLong(2, accountId);
+updateStatement.executeUpdate();
+updateStatement.close();
+
+} catch (SQLException e) {
+s_logger.error("Failed to update cloud.account role_id 
for account id:" + accountId + " with exception: " + e.getMessage());
+throw new CloudRuntimeException("Exception while 
updating cloud.account role_id", e);
+}
+}
+} catch (SQLException e) {
+throw new CloudRuntimeException("Exception while migrating 
existing account table's role_id column to a role based on account type", e);
+}
+s_logger.debug("Done migrating existing accounts to use one of 

[GitHub] cloudstack pull request: CLOUDSTACK-8562: Dynamic Role-Based API C...

[jburwell]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59801445
  
--- Diff: engine/schema/src/com/cloud/upgrade/dao/Upgrade481to490.java ---
@@ -53,6 +62,139 @@ public boolean supportsRollingUpgrade() {
 
 @Override
 public void performDataMigration(Connection conn) {
+setupRolesAndPermissionsForDynamicRBAC(conn);
+}
+
+private void createDefaultRole(final Connection conn, final Long id, 
final String name, final RoleType roleType) {
+final String insertSql = String.format("INSERT INTO 
`cloud`.`roles` (`id`, `uuid`, `name`, `role_type`, `description`) values (%d, 
UUID(), '%s', '%s', 'Default %s role');",
+id, name, roleType.name(), roleType.name().toLowerCase());
+try ( PreparedStatement updatePstmt = 
conn.prepareStatement(insertSql) ) {
+updatePstmt.executeUpdate();
+} catch (SQLException e) {
+throw new CloudRuntimeException("Unable to create default role 
with id: " + id + " name: " + name, e);
+}
+}
+
+private void createRoleMapping(final Connection conn, final Long 
roleId, final String apiName) {
+final String insertSql = String.format("INSERT INTO 
`cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`) values 
(UUID(), %d, '%s', 'ALLOW') ON DUPLICATE KEY UPDATE rule=rule;",
+roleId, apiName);
+try ( PreparedStatement updatePstmt = 
conn.prepareStatement(insertSql)) {
+updatePstmt.executeUpdate();
+} catch (SQLException ignored) {
+s_logger.debug("Unable to insert mapping for role id:" + 
roleId + " apiName: " + apiName);
+}
+}
+
+private void addRoleColumnAndMigrateAccountTable(final Connection 
conn, final RoleType[] roleTypes) {
+final String alterTableSql = "ALTER TABLE `cloud`.`account` ADD 
COLUMN `role_id` bigint(20) unsigned COMMENT 'role id for this account' AFTER 
`type`, " +
+"ADD KEY `fk_account__role_id` (`role_id`), " +
+"ADD CONSTRAINT `fk_account__role_id` FOREIGN KEY 
(`role_id`) REFERENCES `roles` (`id`);";
+try (PreparedStatement pstmt = 
conn.prepareStatement(alterTableSql)) {
+pstmt.executeUpdate();
+s_logger.info("Altered cloud.account table and added column 
role_id");
+} catch (SQLException e) {
+if (e.getMessage().contains("role_id")) {
+s_logger.warn("cloud.account table already has the role_id 
column, skipping altering table and migration of accounts");
+return;
+} else {
+throw new CloudRuntimeException("Unable to create column 
quota_calculated in table cloud_usage.cloud_usage", e);
+}
+}
+migrateAccountsToDefaultRoles(conn, roleTypes);
+}
+
+private void migrateAccountsToDefaultRoles(final Connection conn, 
final RoleType[] roleTypes) {
+try (PreparedStatement selectStatement = 
conn.prepareStatement("SELECT `id`, `type` FROM `cloud`.`account`;");
+ ResultSet selectResultSet = selectStatement.executeQuery()) {
+while (selectResultSet.next()) {
+Long accountId = selectResultSet.getLong(1);
+Short accountType = selectResultSet.getShort(2);
+Long roleId = null;
+for (RoleType roleType : roleTypes) {
+if (roleType.getAccountType() == accountType) {
+roleId = roleType.getId();
+break;
+}
+}
+if (roleId == null) {
+continue;
+}
+try (PreparedStatement updateStatement = 
conn.prepareStatement("UPDATE `cloud`.`account` SET role_id = ? WHERE id = 
?;")) {
+updateStatement.setLong(1, roleId);
+updateStatement.setLong(2, accountId);
+updateStatement.executeUpdate();
+updateStatement.close();
+
+} catch (SQLException e) {
+s_logger.error("Failed to update cloud.account role_id 
for account id:" + accountId + " with exception: " + e.getMessage());
+throw new CloudRuntimeException("Exception while 
updating cloud.account role_id", e);
+}
+}
--- End diff --

Should these updates be wrapped in a transaction to prevent data 
inconsistencies if there is an 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 

[GitHub] cloudstack pull request: CLOUDSTACK-9348: Use non-blocking SSL han...

[swill]

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

https://github.com/apache/cloudstack/pull/1493#discussion_r59799743
  
--- Diff: utils/src/test/java/com/cloud/utils/testcase/NioTest.java ---
@@ -19,146 +19,198 @@
 
 package com.cloud.utils.testcase;
 
-import java.nio.channels.ClosedChannelException;
-import java.util.Random;
-
-import junit.framework.TestCase;
-
-import org.apache.log4j.Logger;
-import org.junit.Assert;
-
+import com.cloud.utils.concurrency.NamedThreadFactory;
 import com.cloud.utils.exception.NioConnectionException;
 import com.cloud.utils.nio.HandlerFactory;
 import com.cloud.utils.nio.Link;
 import com.cloud.utils.nio.NioClient;
 import com.cloud.utils.nio.NioServer;
 import com.cloud.utils.nio.Task;
 import com.cloud.utils.nio.Task.Type;
+import org.apache.log4j.Logger;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
 
-/**
- *
- *
- *
- *
- */
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.nio.channels.ClosedChannelException;
+import java.nio.channels.Selector;
+import java.nio.channels.SocketChannel;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+public class NioTest {
 
-public class NioTest extends TestCase {
+private static final Logger LOGGER = Logger.getLogger(NioTest.class);
 
-private static final Logger s_logger = Logger.getLogger(NioTest.class);
+final private int totalTestCount = 10;
+private int completedTestCount = 0;
 
-private NioServer _server;
-private NioClient _client;
+private NioServer server;
+private List clients = new ArrayList<>();
+private List maliciousClients = new ArrayList<>();
 
-private Link _clientLink;
+private ExecutorService clientExecutor = 
Executors.newFixedThreadPool(totalTestCount, new 
NamedThreadFactory("NioClientHandler"));;
+private ExecutorService maliciousExecutor = 
Executors.newFixedThreadPool(5*totalTestCount, new 
NamedThreadFactory("MaliciousNioClientHandler"));;
 
-private int _testCount;
-private int _completedCount;
+private Random randomGenerator = new Random();
+private byte[] testBytes;
 
 private boolean isTestsDone() {
 boolean result;
 synchronized (this) {
-result = _testCount == _completedCount;
+result = totalTestCount == completedTestCount;
 }
 return result;
 }
 
-private void getOneMoreTest() {
-synchronized (this) {
-_testCount++;
-}
-}
-
 private void oneMoreTestDone() {
 synchronized (this) {
-_completedCount++;
+completedTestCount++;
 }
 }
 
-@Override
+@Before
 public void setUp() {
-s_logger.info("Test");
+LOGGER.info("Setting up Benchmark Test");
 
-_testCount = 0;
-_completedCount = 0;
-
-_server = new NioServer("NioTestServer", , 5, new 
NioTestServer());
-try {
-_server.start();
-} catch (final NioConnectionException e) {
-fail(e.getMessage());
-}
+completedTestCount = 0;
+testBytes = new byte[100];
+randomGenerator.nextBytes(testBytes);
 
-_client = new NioClient("NioTestServer", "127.0.0.1", , 5, new 
NioTestClient());
+// Server configured with one worker
+server = new NioServer("NioTestServer", , 1, new 
NioTestServer());
 try {
-_client.start();
+server.start();
 } catch (final NioConnectionException e) {
-fail(e.getMessage());
+Assert.fail(e.getMessage());
 }
 
-while (_clientLink == null) {
-try {
-s_logger.debug("Link is not up! Waiting ...");
-Thread.sleep(1000);
-} catch (final InterruptedException e) {
-// TODO Auto-generated catch block
-e.printStackTrace();
+// 5 malicious clients per valid client
+for (int i = 0; i < totalTestCount; i++) {
+for (int j = 0; j < 5; j++) {
+final NioClient maliciousClient = new 
NioMaliciousClient("NioMaliciousTestClient-" + i, "127.0.0.1", , 1, new 
NioMaliciousTestClient());
+maliciousClients.add(maliciousClient);
+

[GitHub] cloudstack pull request: CLOUDSTACK-8562: Dynamic Role-Based API C...

[jburwell]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59800725
  
--- Diff: api/test/org/apache/cloudstack/acl/RoleTypeTest.java ---
@@ -0,0 +1,92 @@
+// 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.acl;
+
+import com.cloud.user.Account;
+import org.junit.Assert;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.util.Arrays;
+
+public class RoleTypeTest {
+
+@Test
+public void testRoleTypeFromString() {
+Assert.assertEquals(RoleType.fromString(null), null);
+Assert.assertEquals(RoleType.fromString(""), null);
+Assert.assertEquals(RoleType.fromString("admin"), null);
+Assert.assertEquals(RoleType.fromString("12345%^&*"), null);
+for (RoleType roleType : RoleType.values()) {
+Assert.assertEquals(RoleType.fromString(roleType.name()), 
roleType);
+}
+}
+
+@Test
+public void testDefaultRoleMaskByValue() {
+Assert.assertEquals(RoleType.fromMask(1), RoleType.Admin);
+Assert.assertEquals(RoleType.fromMask(2), RoleType.ResourceAdmin);
+Assert.assertEquals(RoleType.fromMask(4), RoleType.DomainAdmin);
+Assert.assertEquals(RoleType.fromMask(8), RoleType.User);
+Assert.assertEquals(RoleType.fromMask(0), RoleType.Unknown);
+}
+
+@Test
+public void testGetByAccountType() {
+
Assert.assertEquals(RoleType.getByAccountType(Account.ACCOUNT_TYPE_NORMAL), 
RoleType.User);
+
Assert.assertEquals(RoleType.getByAccountType(Account.ACCOUNT_TYPE_ADMIN), 
RoleType.Admin);
+
Assert.assertEquals(RoleType.getByAccountType(Account.ACCOUNT_TYPE_DOMAIN_ADMIN),
 RoleType.DomainAdmin);
+
Assert.assertEquals(RoleType.getByAccountType(Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN),
 RoleType.ResourceAdmin);
+
Assert.assertEquals(RoleType.getByAccountType(Account.ACCOUNT_TYPE_PROJECT), 
RoleType.Unknown);
+}
+
+@Test
+public void testGetRoleByAccountTypeWhenRoleIdIsProvided() {
+Assert.assertEquals(RoleType.getRoleByAccountType(123L, 
Account.ACCOUNT_TYPE_ADMIN), Long.valueOf(123L));
+Assert.assertEquals(RoleType.getRoleByAccountType(1234L, null), 
Long.valueOf(1234L));
+}
+
+@Test
+public void testGetRoleByAccountTypeForDefaultAccountTypes() {
+Assert.assertEquals(RoleType.getRoleByAccountType(null, 
Account.ACCOUNT_TYPE_ADMIN), (Long) RoleType.Admin.getId());
--- End diff --

Passing ``null`` as a parameter to a method as a flag is an anti-pattern.  
Create a two parameter override of ``getRoleByAccountType`` to cover this 
scenario.


---
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-8562: Dynamic Role-Based API C...

[jburwell]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59801354
  
--- Diff: engine/schema/src/com/cloud/upgrade/dao/Upgrade481to490.java ---
@@ -53,6 +62,139 @@ public boolean supportsRollingUpgrade() {
 
 @Override
 public void performDataMigration(Connection conn) {
+setupRolesAndPermissionsForDynamicRBAC(conn);
+}
+
+private void createDefaultRole(final Connection conn, final Long id, 
final String name, final RoleType roleType) {
+final String insertSql = String.format("INSERT INTO 
`cloud`.`roles` (`id`, `uuid`, `name`, `role_type`, `description`) values (%d, 
UUID(), '%s', '%s', 'Default %s role');",
+id, name, roleType.name(), roleType.name().toLowerCase());
+try ( PreparedStatement updatePstmt = 
conn.prepareStatement(insertSql) ) {
+updatePstmt.executeUpdate();
+} catch (SQLException e) {
+throw new CloudRuntimeException("Unable to create default role 
with id: " + id + " name: " + name, e);
+}
+}
+
+private void createRoleMapping(final Connection conn, final Long 
roleId, final String apiName) {
+final String insertSql = String.format("INSERT INTO 
`cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`) values 
(UUID(), %d, '%s', 'ALLOW') ON DUPLICATE KEY UPDATE rule=rule;",
+roleId, apiName);
+try ( PreparedStatement updatePstmt = 
conn.prepareStatement(insertSql)) {
+updatePstmt.executeUpdate();
+} catch (SQLException ignored) {
+s_logger.debug("Unable to insert mapping for role id:" + 
roleId + " apiName: " + apiName);
+}
+}
+
+private void addRoleColumnAndMigrateAccountTable(final Connection 
conn, final RoleType[] roleTypes) {
+final String alterTableSql = "ALTER TABLE `cloud`.`account` ADD 
COLUMN `role_id` bigint(20) unsigned COMMENT 'role id for this account' AFTER 
`type`, " +
+"ADD KEY `fk_account__role_id` (`role_id`), " +
+"ADD CONSTRAINT `fk_account__role_id` FOREIGN KEY 
(`role_id`) REFERENCES `roles` (`id`);";
+try (PreparedStatement pstmt = 
conn.prepareStatement(alterTableSql)) {
+pstmt.executeUpdate();
+s_logger.info("Altered cloud.account table and added column 
role_id");
+} catch (SQLException e) {
+if (e.getMessage().contains("role_id")) {
+s_logger.warn("cloud.account table already has the role_id 
column, skipping altering table and migration of accounts");
+return;
+} else {
+throw new CloudRuntimeException("Unable to create column 
quota_calculated in table cloud_usage.cloud_usage", e);
+}
+}
+migrateAccountsToDefaultRoles(conn, roleTypes);
+}
+
+private void migrateAccountsToDefaultRoles(final Connection conn, 
final RoleType[] roleTypes) {
+try (PreparedStatement selectStatement = 
conn.prepareStatement("SELECT `id`, `type` FROM `cloud`.`account`;");
+ ResultSet selectResultSet = selectStatement.executeQuery()) {
--- End diff --

The ``selectResultSet`` is a resource that needs to be closed.  Please add 
it to enclosing try with resources block.


---
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-8562: Dynamic Role-Based API C...

[jburwell]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59800354
  
--- Diff: api/test/org/apache/cloudstack/acl/RoleTypeTest.java ---
@@ -0,0 +1,92 @@
+// 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.acl;
+
+import com.cloud.user.Account;
+import org.junit.Assert;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.util.Arrays;
+
+public class RoleTypeTest {
+
+@Test
+public void testRoleTypeFromString() {
+Assert.assertEquals(RoleType.fromString(null), null);
+Assert.assertEquals(RoleType.fromString(""), null);
+Assert.assertEquals(RoleType.fromString("admin"), null);
+Assert.assertEquals(RoleType.fromString("12345%^&*"), null);
--- End diff --

Why doesn't ``fromString`` should either return ``Unknown`` or throw an 
``IllegalStateException`` in these scenarios?


---
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-8562: Dynamic Role-Based API C...

[jburwell]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59801544
  
--- Diff: engine/schema/src/com/cloud/upgrade/dao/Upgrade481to490.java ---
@@ -53,6 +62,139 @@ public boolean supportsRollingUpgrade() {
 
 @Override
 public void performDataMigration(Connection conn) {
+setupRolesAndPermissionsForDynamicRBAC(conn);
+}
+
+private void createDefaultRole(final Connection conn, final Long id, 
final String name, final RoleType roleType) {
+final String insertSql = String.format("INSERT INTO 
`cloud`.`roles` (`id`, `uuid`, `name`, `role_type`, `description`) values (%d, 
UUID(), '%s', '%s', 'Default %s role');",
+id, name, roleType.name(), roleType.name().toLowerCase());
+try ( PreparedStatement updatePstmt = 
conn.prepareStatement(insertSql) ) {
+updatePstmt.executeUpdate();
+} catch (SQLException e) {
+throw new CloudRuntimeException("Unable to create default role 
with id: " + id + " name: " + name, e);
+}
+}
+
+private void createRoleMapping(final Connection conn, final Long 
roleId, final String apiName) {
+final String insertSql = String.format("INSERT INTO 
`cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`) values 
(UUID(), %d, '%s', 'ALLOW') ON DUPLICATE KEY UPDATE rule=rule;",
+roleId, apiName);
+try ( PreparedStatement updatePstmt = 
conn.prepareStatement(insertSql)) {
+updatePstmt.executeUpdate();
+} catch (SQLException ignored) {
+s_logger.debug("Unable to insert mapping for role id:" + 
roleId + " apiName: " + apiName);
+}
+}
+
+private void addRoleColumnAndMigrateAccountTable(final Connection 
conn, final RoleType[] roleTypes) {
+final String alterTableSql = "ALTER TABLE `cloud`.`account` ADD 
COLUMN `role_id` bigint(20) unsigned COMMENT 'role id for this account' AFTER 
`type`, " +
+"ADD KEY `fk_account__role_id` (`role_id`), " +
+"ADD CONSTRAINT `fk_account__role_id` FOREIGN KEY 
(`role_id`) REFERENCES `roles` (`id`);";
+try (PreparedStatement pstmt = 
conn.prepareStatement(alterTableSql)) {
+pstmt.executeUpdate();
+s_logger.info("Altered cloud.account table and added column 
role_id");
+} catch (SQLException e) {
+if (e.getMessage().contains("role_id")) {
+s_logger.warn("cloud.account table already has the role_id 
column, skipping altering table and migration of accounts");
+return;
+} else {
+throw new CloudRuntimeException("Unable to create column 
quota_calculated in table cloud_usage.cloud_usage", e);
+}
+}
+migrateAccountsToDefaultRoles(conn, roleTypes);
+}
+
+private void migrateAccountsToDefaultRoles(final Connection conn, 
final RoleType[] roleTypes) {
+try (PreparedStatement selectStatement = 
conn.prepareStatement("SELECT `id`, `type` FROM `cloud`.`account`;");
+ ResultSet selectResultSet = selectStatement.executeQuery()) {
+while (selectResultSet.next()) {
+Long accountId = selectResultSet.getLong(1);
+Short accountType = selectResultSet.getShort(2);
+Long roleId = null;
+for (RoleType roleType : roleTypes) {
+if (roleType.getAccountType() == accountType) {
+roleId = roleType.getId();
+break;
+}
+}
+if (roleId == null) {
+continue;
+}
+try (PreparedStatement updateStatement = 
conn.prepareStatement("UPDATE `cloud`.`account` SET role_id = ? WHERE id = 
?;")) {
+updateStatement.setLong(1, roleId);
+updateStatement.setLong(2, accountId);
+updateStatement.executeUpdate();
+updateStatement.close();
+
+} catch (SQLException e) {
+s_logger.error("Failed to update cloud.account role_id 
for account id:" + accountId + " with exception: " + e.getMessage());
+throw new CloudRuntimeException("Exception while 
updating cloud.account role_id", e);
+}
+}
+} catch (SQLException e) {
+throw new CloudRuntimeException("Exception while migrating 
existing account table's role_id column to a role based on account type", e);
+}
+s_logger.debug("Done migrating existing accounts to use one of 

[GitHub] cloudstack pull request: CLOUDSTACK-8562: Dynamic Role-Based API C...

[jburwell]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59788566
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/acl/DeleteRolePermissionCmd.java
 ---
@@ -0,0 +1,84 @@
+// 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.api.command.admin.acl;
+
+import com.cloud.user.Account;
+import org.apache.cloudstack.acl.RolePermission;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.RolePermissionResponse;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.context.CallContext;
+
+@APICommand(name = DeleteRolePermissionCmd.APINAME, description = "Deletes 
a role permission", responseObject = SuccessResponse.class,
+requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
+since = "4.9.0",
+authorized = {RoleType.Admin})
+public class DeleteRolePermissionCmd extends BaseCmd {
+public static final String APINAME = "deleteRolePermission";
+
+/
+ API parameters /
+/
+
+@Parameter(name = ApiConstants.ID, type = BaseCmd.CommandType.UUID, 
required = true, entityType = RolePermissionResponse.class, description = "ID 
of the role permission")
+private Long rolePermissionId;
+
+/
+/// Accessors ///
+/
+
+public Long getRolePermissionId() {
+return rolePermissionId;
+}
+
+/
+/// API Implementation///
+/
+
+@Override
+public String getCommandName() {
+return APINAME.toLowerCase() + BaseCmd.RESPONSE_SUFFIX;
+}
+
+@Override
+public long getEntityOwnerId() {
+return Account.ACCOUNT_ID_SYSTEM;
+}
+
+@Override
+public void execute() {
+if (getRolePermissionId() == null || getRolePermissionId() < 1L) {
+throw new ServerApiException(ApiErrorCode.PARAM_ERROR, 
"Invalid role permission id provided");
+}
+RolePermission rolePermission = 
roleService.findRolePermission(getRolePermissionId());
+if (rolePermission == null) {
+throw new ServerApiException(ApiErrorCode.PARAM_ERROR, 
"Invalid role permission id provided");
+}
--- End diff --

Consider extracting this validation logic to a private method to make 
things more readable.


---
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-8562: Dynamic Role-Based API C...

[jburwell]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59788517
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/acl/DeleteRoleCmd.java ---
@@ -0,0 +1,84 @@
+// 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.api.command.admin.acl;
+
+import com.cloud.user.Account;
+import org.apache.cloudstack.acl.Role;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.RoleResponse;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.context.CallContext;
+
+@APICommand(name = DeleteRoleCmd.APINAME, description = "Deletes a role", 
responseObject = SuccessResponse.class,
+requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
+since = "4.9.0",
+authorized = {RoleType.Admin})
+public class DeleteRoleCmd extends BaseCmd {
+public static final String APINAME = "deleteRole";
+
+/
+ API parameters /
+/
+
+@Parameter(name = ApiConstants.ID, type = BaseCmd.CommandType.UUID, 
required = true, entityType = RoleResponse.class, description = "ID of the 
role")
+private Long roleId;
+
+/
+/// Accessors ///
+/
+
+public Long getRoleId() {
+return roleId;
+}
+
+/
+/// API Implementation///
+/
+
+@Override
+public String getCommandName() {
+return APINAME.toLowerCase() + BaseCmd.RESPONSE_SUFFIX;
+}
+
+@Override
+public long getEntityOwnerId() {
+return Account.ACCOUNT_ID_SYSTEM;
+}
+
+@Override
+public void execute() {
+if (getRoleId() == null || getRoleId() < 1L) {
+throw new ServerApiException(ApiErrorCode.PARAM_ERROR, 
"Invalid role id provided");
+}
+Role role = roleService.findRole(getRoleId());
+if (role == null) {
+throw new ServerApiException(ApiErrorCode.PARAM_ERROR, 
"Invalid role id provided");
+}
--- End diff --

Consider extracting this validation logic to a private method to make 
things more readable.


---
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-9348: Use non-blocking SSL han...

[GabrielBrascher]

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

https://github.com/apache/cloudstack/pull/1493#discussion_r59790778
  
--- Diff: utils/src/test/java/com/cloud/utils/testcase/NioTest.java ---
@@ -19,146 +19,198 @@
 
 package com.cloud.utils.testcase;
 
-import java.nio.channels.ClosedChannelException;
-import java.util.Random;
-
-import junit.framework.TestCase;
-
-import org.apache.log4j.Logger;
-import org.junit.Assert;
-
+import com.cloud.utils.concurrency.NamedThreadFactory;
 import com.cloud.utils.exception.NioConnectionException;
 import com.cloud.utils.nio.HandlerFactory;
 import com.cloud.utils.nio.Link;
 import com.cloud.utils.nio.NioClient;
 import com.cloud.utils.nio.NioServer;
 import com.cloud.utils.nio.Task;
 import com.cloud.utils.nio.Task.Type;
+import org.apache.log4j.Logger;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
 
-/**
- *
- *
- *
- *
- */
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.nio.channels.ClosedChannelException;
+import java.nio.channels.Selector;
+import java.nio.channels.SocketChannel;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+public class NioTest {
 
-public class NioTest extends TestCase {
+private static final Logger LOGGER = Logger.getLogger(NioTest.class);
 
-private static final Logger s_logger = Logger.getLogger(NioTest.class);
+final private int totalTestCount = 10;
+private int completedTestCount = 0;
 
-private NioServer _server;
-private NioClient _client;
+private NioServer server;
+private List clients = new ArrayList<>();
+private List maliciousClients = new ArrayList<>();
 
-private Link _clientLink;
+private ExecutorService clientExecutor = 
Executors.newFixedThreadPool(totalTestCount, new 
NamedThreadFactory("NioClientHandler"));;
+private ExecutorService maliciousExecutor = 
Executors.newFixedThreadPool(5*totalTestCount, new 
NamedThreadFactory("MaliciousNioClientHandler"));;
 
-private int _testCount;
-private int _completedCount;
+private Random randomGenerator = new Random();
+private byte[] testBytes;
 
 private boolean isTestsDone() {
 boolean result;
 synchronized (this) {
-result = _testCount == _completedCount;
+result = totalTestCount == completedTestCount;
 }
 return result;
 }
 
-private void getOneMoreTest() {
-synchronized (this) {
-_testCount++;
-}
-}
-
 private void oneMoreTestDone() {
 synchronized (this) {
-_completedCount++;
+completedTestCount++;
 }
 }
 
-@Override
+@Before
 public void setUp() {
-s_logger.info("Test");
+LOGGER.info("Setting up Benchmark Test");
 
-_testCount = 0;
-_completedCount = 0;
-
-_server = new NioServer("NioTestServer", , 5, new 
NioTestServer());
-try {
-_server.start();
-} catch (final NioConnectionException e) {
-fail(e.getMessage());
-}
+completedTestCount = 0;
+testBytes = new byte[100];
+randomGenerator.nextBytes(testBytes);
 
-_client = new NioClient("NioTestServer", "127.0.0.1", , 5, new 
NioTestClient());
+// Server configured with one worker
+server = new NioServer("NioTestServer", , 1, new 
NioTestServer());
 try {
-_client.start();
+server.start();
 } catch (final NioConnectionException e) {
-fail(e.getMessage());
+Assert.fail(e.getMessage());
 }
 
-while (_clientLink == null) {
-try {
-s_logger.debug("Link is not up! Waiting ...");
-Thread.sleep(1000);
-} catch (final InterruptedException e) {
-// TODO Auto-generated catch block
-e.printStackTrace();
+// 5 malicious clients per valid client
+for (int i = 0; i < totalTestCount; i++) {
+for (int j = 0; j < 5; j++) {
+final NioClient maliciousClient = new 
NioMaliciousClient("NioMaliciousTestClient-" + i, "127.0.0.1", , 1, new 
NioMaliciousTestClient());
+maliciousClients.add(maliciousClient);
+  

[GitHub] cloudstack pull request: CLOUDSTACK-8562: Dynamic Role-Based API C...

[jburwell]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59800993
  
--- Diff: api/test/org/apache/cloudstack/acl/RuleTest.java ---
@@ -0,0 +1,39 @@
+package org.apache.cloudstack.acl;
+
+import com.cloud.exception.InvalidParameterValueException;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Arrays;
+
+public class RuleTest {
+
+@Test
+public void testToString() throws Exception {
+Rule rule = new Rule("someString");
+Assert.assertEquals(rule.toString(), "someString");
+}
+
+@Test
+public void testValidateRuleWithValidData() throws Exception {
+for (String rule : Arrays.asList("a", "1", "someApi", 
"someApi321", "123SomeApi",
+"prefix*", "*middle*", "*Suffix",
+"*", "**", "f***", "m0nk3yMa**g1c*")) {
+Assert.assertTrue(Rule.validate(rule));
+Assert.assertEquals(new Rule(rule).toString(), rule);
+}
+}
+
+@Test
+public void testValidateRuleWithInvalidData() throws Exception {
+for (String rule : Arrays.asList(null, "", " ", "  ", "\n", "\t", 
"\r", "\"", "\'",
+"^someApi$", "^someApi", "some$", "some-Api;", "some,Api",
+"^", "$", "^$", ".*", "\\w+", "r**l3rd0@Kr3", 
"j@s1n|+|0È·",
+"[a-z0-9-]+", 
"^([a-z0-9_\\.-]+)@([\\da-z\\.-]+)\\.([a-z\\.]{2,6})$")) {
+try {
+new Rule(rule);
+Assert.fail("Invalid rule, exception was expected");
+} catch (InvalidParameterValueException ignored) {}
+}
--- End diff --

This method can be simplified by removing the try-catch and added 
``expected=InvalidParameterValueException.class`` to the ``@Test`` annotation.


---
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-9348: Use non-blocking SSL han...

[bhaisaab]

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

https://github.com/apache/cloudstack/pull/1493#discussion_r59791280
  
--- Diff: utils/src/test/java/com/cloud/utils/testcase/NioTest.java ---
@@ -19,146 +19,198 @@
 
 package com.cloud.utils.testcase;
 
-import java.nio.channels.ClosedChannelException;
-import java.util.Random;
-
-import junit.framework.TestCase;
-
-import org.apache.log4j.Logger;
-import org.junit.Assert;
-
+import com.cloud.utils.concurrency.NamedThreadFactory;
 import com.cloud.utils.exception.NioConnectionException;
 import com.cloud.utils.nio.HandlerFactory;
 import com.cloud.utils.nio.Link;
 import com.cloud.utils.nio.NioClient;
 import com.cloud.utils.nio.NioServer;
 import com.cloud.utils.nio.Task;
 import com.cloud.utils.nio.Task.Type;
+import org.apache.log4j.Logger;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
 
-/**
- *
- *
- *
- *
- */
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.nio.channels.ClosedChannelException;
+import java.nio.channels.Selector;
+import java.nio.channels.SocketChannel;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+public class NioTest {
 
-public class NioTest extends TestCase {
+private static final Logger LOGGER = Logger.getLogger(NioTest.class);
 
-private static final Logger s_logger = Logger.getLogger(NioTest.class);
+final private int totalTestCount = 10;
+private int completedTestCount = 0;
 
-private NioServer _server;
-private NioClient _client;
+private NioServer server;
+private List clients = new ArrayList<>();
+private List maliciousClients = new ArrayList<>();
 
-private Link _clientLink;
+private ExecutorService clientExecutor = 
Executors.newFixedThreadPool(totalTestCount, new 
NamedThreadFactory("NioClientHandler"));;
+private ExecutorService maliciousExecutor = 
Executors.newFixedThreadPool(5*totalTestCount, new 
NamedThreadFactory("MaliciousNioClientHandler"));;
 
-private int _testCount;
-private int _completedCount;
+private Random randomGenerator = new Random();
+private byte[] testBytes;
 
 private boolean isTestsDone() {
 boolean result;
 synchronized (this) {
-result = _testCount == _completedCount;
+result = totalTestCount == completedTestCount;
 }
 return result;
 }
 
-private void getOneMoreTest() {
-synchronized (this) {
-_testCount++;
-}
-}
-
 private void oneMoreTestDone() {
 synchronized (this) {
-_completedCount++;
+completedTestCount++;
 }
 }
 
-@Override
+@Before
 public void setUp() {
-s_logger.info("Test");
+LOGGER.info("Setting up Benchmark Test");
 
-_testCount = 0;
-_completedCount = 0;
-
-_server = new NioServer("NioTestServer", , 5, new 
NioTestServer());
-try {
-_server.start();
-} catch (final NioConnectionException e) {
-fail(e.getMessage());
-}
+completedTestCount = 0;
+testBytes = new byte[100];
+randomGenerator.nextBytes(testBytes);
 
-_client = new NioClient("NioTestServer", "127.0.0.1", , 5, new 
NioTestClient());
+// Server configured with one worker
+server = new NioServer("NioTestServer", , 1, new 
NioTestServer());
 try {
-_client.start();
+server.start();
 } catch (final NioConnectionException e) {
-fail(e.getMessage());
+Assert.fail(e.getMessage());
 }
 
-while (_clientLink == null) {
-try {
-s_logger.debug("Link is not up! Waiting ...");
-Thread.sleep(1000);
-} catch (final InterruptedException e) {
-// TODO Auto-generated catch block
-e.printStackTrace();
+// 5 malicious clients per valid client
+for (int i = 0; i < totalTestCount; i++) {
+for (int j = 0; j < 5; j++) {
+final NioClient maliciousClient = new 
NioMaliciousClient("NioMaliciousTestClient-" + i, "127.0.0.1", , 1, new 
NioMaliciousTestClient());
+maliciousClients.add(maliciousClient);
+ 

[GitHub] cloudstack pull request: CLOUDSTACK-9348: Use non-blocking SSL han...

[swill]

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

https://github.com/apache/cloudstack/pull/1493#discussion_r59792529
  
--- Diff: utils/src/test/java/com/cloud/utils/testcase/NioTest.java ---
@@ -19,146 +19,198 @@
 
 package com.cloud.utils.testcase;
 
-import java.nio.channels.ClosedChannelException;
-import java.util.Random;
-
-import junit.framework.TestCase;
-
-import org.apache.log4j.Logger;
-import org.junit.Assert;
-
+import com.cloud.utils.concurrency.NamedThreadFactory;
 import com.cloud.utils.exception.NioConnectionException;
 import com.cloud.utils.nio.HandlerFactory;
 import com.cloud.utils.nio.Link;
 import com.cloud.utils.nio.NioClient;
 import com.cloud.utils.nio.NioServer;
 import com.cloud.utils.nio.Task;
 import com.cloud.utils.nio.Task.Type;
+import org.apache.log4j.Logger;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
 
-/**
- *
- *
- *
- *
- */
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.nio.channels.ClosedChannelException;
+import java.nio.channels.Selector;
+import java.nio.channels.SocketChannel;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+public class NioTest {
 
-public class NioTest extends TestCase {
+private static final Logger LOGGER = Logger.getLogger(NioTest.class);
 
-private static final Logger s_logger = Logger.getLogger(NioTest.class);
+final private int totalTestCount = 10;
+private int completedTestCount = 0;
 
-private NioServer _server;
-private NioClient _client;
+private NioServer server;
+private List clients = new ArrayList<>();
+private List maliciousClients = new ArrayList<>();
 
-private Link _clientLink;
+private ExecutorService clientExecutor = 
Executors.newFixedThreadPool(totalTestCount, new 
NamedThreadFactory("NioClientHandler"));;
+private ExecutorService maliciousExecutor = 
Executors.newFixedThreadPool(5*totalTestCount, new 
NamedThreadFactory("MaliciousNioClientHandler"));;
 
-private int _testCount;
-private int _completedCount;
+private Random randomGenerator = new Random();
+private byte[] testBytes;
 
 private boolean isTestsDone() {
 boolean result;
 synchronized (this) {
-result = _testCount == _completedCount;
+result = totalTestCount == completedTestCount;
 }
 return result;
 }
 
-private void getOneMoreTest() {
-synchronized (this) {
-_testCount++;
-}
-}
-
 private void oneMoreTestDone() {
 synchronized (this) {
-_completedCount++;
+completedTestCount++;
 }
 }
 
-@Override
+@Before
 public void setUp() {
-s_logger.info("Test");
+LOGGER.info("Setting up Benchmark Test");
 
-_testCount = 0;
-_completedCount = 0;
-
-_server = new NioServer("NioTestServer", , 5, new 
NioTestServer());
-try {
-_server.start();
-} catch (final NioConnectionException e) {
-fail(e.getMessage());
-}
+completedTestCount = 0;
+testBytes = new byte[100];
+randomGenerator.nextBytes(testBytes);
 
-_client = new NioClient("NioTestServer", "127.0.0.1", , 5, new 
NioTestClient());
+// Server configured with one worker
+server = new NioServer("NioTestServer", , 1, new 
NioTestServer());
 try {
-_client.start();
+server.start();
 } catch (final NioConnectionException e) {
-fail(e.getMessage());
+Assert.fail(e.getMessage());
 }
 
-while (_clientLink == null) {
-try {
-s_logger.debug("Link is not up! Waiting ...");
-Thread.sleep(1000);
-} catch (final InterruptedException e) {
-// TODO Auto-generated catch block
-e.printStackTrace();
+// 5 malicious clients per valid client
+for (int i = 0; i < totalTestCount; i++) {
+for (int j = 0; j < 5; j++) {
+final NioClient maliciousClient = new 
NioMaliciousClient("NioMaliciousTestClient-" + i, "127.0.0.1", , 1, new 
NioMaliciousTestClient());
+maliciousClients.add(maliciousClient);
+

[GitHub] cloudstack pull request: CLOUDSTACK-9348: Use non-blocking SSL han...

[bhaisaab]

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

https://github.com/apache/cloudstack/pull/1493#discussion_r59795416
  
--- Diff: utils/src/test/java/com/cloud/utils/testcase/NioTest.java ---
@@ -19,146 +19,198 @@
 
 package com.cloud.utils.testcase;
 
-import java.nio.channels.ClosedChannelException;
-import java.util.Random;
-
-import junit.framework.TestCase;
-
-import org.apache.log4j.Logger;
-import org.junit.Assert;
-
+import com.cloud.utils.concurrency.NamedThreadFactory;
 import com.cloud.utils.exception.NioConnectionException;
 import com.cloud.utils.nio.HandlerFactory;
 import com.cloud.utils.nio.Link;
 import com.cloud.utils.nio.NioClient;
 import com.cloud.utils.nio.NioServer;
 import com.cloud.utils.nio.Task;
 import com.cloud.utils.nio.Task.Type;
+import org.apache.log4j.Logger;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
 
-/**
- *
- *
- *
- *
- */
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.nio.channels.ClosedChannelException;
+import java.nio.channels.Selector;
+import java.nio.channels.SocketChannel;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+public class NioTest {
 
-public class NioTest extends TestCase {
+private static final Logger LOGGER = Logger.getLogger(NioTest.class);
 
-private static final Logger s_logger = Logger.getLogger(NioTest.class);
+final private int totalTestCount = 10;
+private int completedTestCount = 0;
 
-private NioServer _server;
-private NioClient _client;
+private NioServer server;
+private List clients = new ArrayList<>();
+private List maliciousClients = new ArrayList<>();
 
-private Link _clientLink;
+private ExecutorService clientExecutor = 
Executors.newFixedThreadPool(totalTestCount, new 
NamedThreadFactory("NioClientHandler"));;
+private ExecutorService maliciousExecutor = 
Executors.newFixedThreadPool(5*totalTestCount, new 
NamedThreadFactory("MaliciousNioClientHandler"));;
 
-private int _testCount;
-private int _completedCount;
+private Random randomGenerator = new Random();
+private byte[] testBytes;
 
 private boolean isTestsDone() {
 boolean result;
 synchronized (this) {
-result = _testCount == _completedCount;
+result = totalTestCount == completedTestCount;
 }
 return result;
 }
 
-private void getOneMoreTest() {
-synchronized (this) {
-_testCount++;
-}
-}
-
 private void oneMoreTestDone() {
 synchronized (this) {
-_completedCount++;
+completedTestCount++;
 }
 }
 
-@Override
+@Before
 public void setUp() {
-s_logger.info("Test");
+LOGGER.info("Setting up Benchmark Test");
 
-_testCount = 0;
-_completedCount = 0;
-
-_server = new NioServer("NioTestServer", , 5, new 
NioTestServer());
-try {
-_server.start();
-} catch (final NioConnectionException e) {
-fail(e.getMessage());
-}
+completedTestCount = 0;
+testBytes = new byte[100];
+randomGenerator.nextBytes(testBytes);
 
-_client = new NioClient("NioTestServer", "127.0.0.1", , 5, new 
NioTestClient());
+// Server configured with one worker
+server = new NioServer("NioTestServer", , 1, new 
NioTestServer());
 try {
-_client.start();
+server.start();
 } catch (final NioConnectionException e) {
-fail(e.getMessage());
+Assert.fail(e.getMessage());
 }
 
-while (_clientLink == null) {
-try {
-s_logger.debug("Link is not up! Waiting ...");
-Thread.sleep(1000);
-} catch (final InterruptedException e) {
-// TODO Auto-generated catch block
-e.printStackTrace();
+// 5 malicious clients per valid client
+for (int i = 0; i < totalTestCount; i++) {
+for (int j = 0; j < 5; j++) {
+final NioClient maliciousClient = new 
NioMaliciousClient("NioMaliciousTestClient-" + i, "127.0.0.1", , 1, new 
NioMaliciousTestClient());
+maliciousClients.add(maliciousClient);
+ 

[GitHub] cloudstack pull request: CLOUDSTACK-8562: Dynamic Role-Based API C...

[jburwell]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59797686
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/acl/ListRolePermissionsCmd.java 
---
@@ -0,0 +1,104 @@
+// 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.api.command.admin.acl;
+
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.user.Account;
+import org.apache.cloudstack.acl.Role;
+import org.apache.cloudstack.acl.RolePermission;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ListResponse;
+import org.apache.cloudstack.api.response.RolePermissionResponse;
+import org.apache.cloudstack.api.response.RoleResponse;
+
+import java.util.ArrayList;
+import java.util.List;
+
+
+@APICommand(name = ListRolePermissionsCmd.APINAME, description = "Lists 
role permissions", responseObject = RolePermissionResponse.class,
+requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
+since = "4.9.0",
+authorized = {RoleType.Admin})
+public class ListRolePermissionsCmd extends BaseCmd {
+public static final String APINAME = "listRolePermissions";
+
+/
+ API parameters /
+/
+
+@Parameter(name = ApiConstants.ROLE_ID, type = CommandType.UUID, 
entityType = RoleResponse.class, description = "ID of the role")
+private Long roleId;
+
+/
+/// Accessors ///
+/
+
+public Long getRoleId() {
+return roleId;
+}
+
+/
+/// API Implementation///
+/
+
+@Override
+public String getCommandName() {
+return APINAME.toLowerCase() + BaseCmd.RESPONSE_SUFFIX;
+}
+
+@Override
+public long getEntityOwnerId() {
+return Account.ACCOUNT_ID_SYSTEM;
+}
+
+@Override
+public void execute() throws ResourceUnavailableException, 
InsufficientCapacityException, ServerApiException {
+if (getRoleId() != null && getRoleId() < 1L) {
+throw new ServerApiException(ApiErrorCode.PARAM_ERROR, 
"Invalid role id provided");
+}
+List rolePermissions = 
roleService.findAllPermissionsBy(getRoleId());
+
+ListResponse response = new 
ListResponse<>();
+List rolePermissionResponses = new 
ArrayList<>();
+for (RolePermission rolePermission : rolePermissions) {
+Role role = roleService.findRole(rolePermission.getRoleId());
--- End diff --

Am I correct in understanding that all the permissions in 
``rolePerimssions`` will be for ``roleId``?  If so, why do we retrieve the same 
role for every permission evaluated?  Why not retrieve the role once between 
the for loop?  A step further, why not model ``RolePermission`` to have an 
association with ``Role`` and retrieve and associate it in 
``roleService.findAllPermissionsBy``?


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

[GitHub] cloudstack pull request: CLOUDSTACK-8562: Dynamic Role-Based API C...

[jburwell]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59800553
  
--- Diff: api/test/org/apache/cloudstack/acl/RoleTypeTest.java ---
@@ -0,0 +1,92 @@
+// 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.acl;
+
+import com.cloud.user.Account;
+import org.junit.Assert;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.util.Arrays;
+
+public class RoleTypeTest {
+
+@Test
+public void testRoleTypeFromString() {
+Assert.assertEquals(RoleType.fromString(null), null);
+Assert.assertEquals(RoleType.fromString(""), null);
+Assert.assertEquals(RoleType.fromString("admin"), null);
+Assert.assertEquals(RoleType.fromString("12345%^&*"), null);
+for (RoleType roleType : RoleType.values()) {
+Assert.assertEquals(RoleType.fromString(roleType.name()), 
roleType);
+}
+}
+
+@Test
+public void testDefaultRoleMaskByValue() {
+Assert.assertEquals(RoleType.fromMask(1), RoleType.Admin);
+Assert.assertEquals(RoleType.fromMask(2), RoleType.ResourceAdmin);
+Assert.assertEquals(RoleType.fromMask(4), RoleType.DomainAdmin);
+Assert.assertEquals(RoleType.fromMask(8), RoleType.User);
+Assert.assertEquals(RoleType.fromMask(0), RoleType.Unknown);
--- End diff --

What happens if ``fromMask`` is passed unexpected integer values?


---
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-8562: Dynamic Role-Based API C...

[jburwell]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59801226
  
--- Diff: engine/schema/src/com/cloud/upgrade/dao/Upgrade481to490.java ---
@@ -53,6 +62,139 @@ public boolean supportsRollingUpgrade() {
 
 @Override
 public void performDataMigration(Connection conn) {
+setupRolesAndPermissionsForDynamicRBAC(conn);
+}
+
+private void createDefaultRole(final Connection conn, final Long id, 
final String name, final RoleType roleType) {
+final String insertSql = String.format("INSERT INTO 
`cloud`.`roles` (`id`, `uuid`, `name`, `role_type`, `description`) values (%d, 
UUID(), '%s', '%s', 'Default %s role');",
+id, name, roleType.name(), roleType.name().toLowerCase());
+try ( PreparedStatement updatePstmt = 
conn.prepareStatement(insertSql) ) {
+updatePstmt.executeUpdate();
+} catch (SQLException e) {
+throw new CloudRuntimeException("Unable to create default role 
with id: " + id + " name: " + name, e);
+}
+}
+
+private void createRoleMapping(final Connection conn, final Long 
roleId, final String apiName) {
+final String insertSql = String.format("INSERT INTO 
`cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`) values 
(UUID(), %d, '%s', 'ALLOW') ON DUPLICATE KEY UPDATE rule=rule;",
+roleId, apiName);
+try ( PreparedStatement updatePstmt = 
conn.prepareStatement(insertSql)) {
+updatePstmt.executeUpdate();
+} catch (SQLException ignored) {
+s_logger.debug("Unable to insert mapping for role id:" + 
roleId + " apiName: " + apiName);
--- End diff --

Why not log this condition to ``WARN``?


---
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: Feature proposal: Resource naming policies

[ilya]

Awesome and long awaited



On 4/14/16 4:40 AM, Jeff Hair wrote:
> Yesterday, we submitted this pull request:
> https://github.com/apache/cloudstack/pull/1492
> 
> This originally grew out of making the VirtualMachineName class non-static
> (original PR is mentioned in the above link). We're presenting this as a
> refactoring of the existing code to enable more extensibility and
> flexibility, make unit testing easier, and unify the way CloudStack
> generates resource names.
> 
> There is an associated JIRA ticket at CLOUDSTACK-9003. I will be writing up
> a design document for the CS wiki in the next few days.
> 
> jburwell wanted me to open a discussion on the developer list about the PR
> and how it is implemented:
> 
> There is now a ResourceNamingPolicyManager and a bunch of
> ResourceNamingPolicies. The manager exposes a method to get a policy for a
> type of resource, and the naming policies generate UUIDs + resource names.
> 
> The default naming policies generate names exactly the same way as they are
> created now. This is in a new module called default-naming-policies. By
> excluding this module and loading our own naming policies, we gain the
> ability to change how names are generated.
> 

Re: Strange XenServer SR behavior when deploying system VMs in Basic Zone on 4.9

[Tutkowski, Mike]

I'm not sure, Daan.

I plan to keep an eye on this behavior for a while when creating new clouds.


From: Daan Hoogland 
Sent: Thursday, April 14, 2016 2:12 AM
To: dev
Subject: Re: Strange XenServer SR behavior when deploying system VMs in Basic 
Zone on 4.9

Mike, did the iso copy process not complete as expected. Sound like they
are a remanence of some task ending in an exception. Probably a silently
ignored one ;|

On Thu, Apr 14, 2016 at 2:49 AM, Tutkowski, Mike 
wrote:

> Just an FYI, but when I kicked off my first VM in this cloud, the VR
> happened to get deployed to XenServer-6.5-3 (which was one of my XenServer
> hosts that had an un-expected shared SR pointing at secondary storage
> beforehand).
>
> Once the process of copying the system template down to local storage
> completed, the shared SR pointing at secondary storage went away (as you
> would expect).
>
> This leaves me now with one un-expected shared SR pointing at secondary
> storage on XenServer-6.5-1.
>
> 
> From: Tutkowski, Mike 
> Sent: Wednesday, April 13, 2016 5:10 PM
> To: dev@cloudstack.apache.org
> Subject: Strange XenServer SR behavior when deploying system VMs in Basic
> Zone on 4.9
>
> Hi,
>
>
> Has anyone recently observed the following behavior:
>
>
> http://imgur.com/8ALJmWb
>
>
> As you can see in the image, I have three 6.5 XenServer hosts in a
> resource pool.
>
>
> I just used them when creating a basic zone and the system VMs were
> deployed just fine. However, there are SRs pointing to secondary storage on
> my XenServer-6.5-1 and XenServer-6.5-3 hosts still (there used to be one on
> my XenServer-6.5-2 host, but it went away once the system VMs started up on
> that host).
>
>
> Thoughts?
>
>
> Thanks,
>
> Mike
>



--
Daan

[GitHub] cloudstack pull request: CLOUDSTACK-8562: Dynamic Role-Based API C...

[pdube]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59812425
  
--- Diff: api/src/org/apache/cloudstack/acl/RoleType.java ---
@@ -16,18 +16,90 @@
 // under the License.
 package org.apache.cloudstack.acl;
 
+import com.cloud.user.Account;
+import com.google.common.base.Enums;
+import com.google.common.base.Strings;
+
 // Enum for default roles in CloudStack
 public enum RoleType {
-Admin(1), ResourceAdmin(2), DomainAdmin(4), User(8), Unknown(0);
+Admin(1L, Account.ACCOUNT_TYPE_ADMIN, 1),
+ResourceAdmin(2L, Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN, 2),
+DomainAdmin(3L, Account.ACCOUNT_TYPE_DOMAIN_ADMIN, 4),
+User(4L, Account.ACCOUNT_TYPE_NORMAL, 8),
+Unknown(-1L, (short) -1, 0);
 
+private long id;
+private short accountType;
 private int mask;
 
-private RoleType(int mask) {
+RoleType(final long id, final short accountType, final int mask) {
+this.id = id;
+this.accountType = accountType;
 this.mask = mask;
 }
 
-public int getValue() {
+public long getId() {
+return id;
+}
+
+public short getAccountType() {
+return accountType;
+}
+
+public int getMask() {
 return mask;
 }
-}
 
+public static RoleType fromString(final String name) {
+if (!Strings.isNullOrEmpty(name)
+&& Enums.getIfPresent(RoleType.class, name).isPresent()) {
+return RoleType.valueOf(name);
+}
+return null;
+}
+
+public static RoleType fromMask(int mask) {
+for (RoleType roleType : RoleType.values()) {
+if (roleType.getMask() == mask) {
+return roleType;
+}
+}
+return Unknown;
+}
+
+public static RoleType getByAccountType(final short accountType) {
+RoleType roleType = RoleType.Unknown;
+switch (accountType) {
+case Account.ACCOUNT_TYPE_ADMIN:
+roleType = RoleType.Admin;
+break;
+case Account.ACCOUNT_TYPE_DOMAIN_ADMIN:
+roleType = RoleType.DomainAdmin;
+break;
+case Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN:
+roleType = RoleType.ResourceAdmin;
--- End diff --

Thanks. I had seen it in the code and looked for more information about it, 
but didn't find anything conclusive


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

SHA bbe0fc

[Tutkowski, Mike]

Hi,


I noticed an issue in Marvin the other day and I tracked it to this commit:


https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=commit;h=bbe0fc4be9527d51820b067a602886003991db4d


The problem is that it assumes the "ispublic" parameter will be provided. If it 
is not, then an exception is thrown.


I think we want code more like this:


if "ispublic" in services:
cmd.ispublic = services["ispublic"]


I don't think we would want to require the "ispublic" parameter in Marvin. It's 
not required in our API:


http://cloudstack.apache.org/api/apidocs-4.8/root_admin/createTemplate.html?


Unless someone can think of a reason why this part of the code is the way it is 
now, I plan to open a PR to fix this soon.


Thanks,

Mike

[GitHub] cloudstack pull request: Fixes regarding VOLUME_DELETE events resu...

[koushik-das]

Github user koushik-das commented on the pull request:

https://github.com/apache/cloudstack/pull/1491#issuecomment-209830019
  
@ProjectMoon On latest master during VM deployment no VOLUME.CREATE is 
getting generated for ROOT volume. Also during destroy there is no 
VOLUME.DELETE event. On the event table I only see events for VM 
creation/destroy. Is ROOT volume events broken in general?


---
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: Strange XenServer SR behavior when deploying system VMs in Basic Zone on 4.9

[Daan Hoogland]

Mike, did the iso copy process not complete as expected. Sound like they
are a remanence of some task ending in an exception. Probably a silently
ignored one ;|

On Thu, Apr 14, 2016 at 2:49 AM, Tutkowski, Mike 
wrote:

> Just an FYI, but when I kicked off my first VM in this cloud, the VR
> happened to get deployed to XenServer-6.5-3 (which was one of my XenServer
> hosts that had an un-expected shared SR pointing at secondary storage
> beforehand).
>
> Once the process of copying the system template down to local storage
> completed, the shared SR pointing at secondary storage went away (as you
> would expect).
>
> This leaves me now with one un-expected shared SR pointing at secondary
> storage on XenServer-6.5-1.
>
> 
> From: Tutkowski, Mike 
> Sent: Wednesday, April 13, 2016 5:10 PM
> To: dev@cloudstack.apache.org
> Subject: Strange XenServer SR behavior when deploying system VMs in Basic
> Zone on 4.9
>
> Hi,
>
>
> Has anyone recently observed the following behavior:
>
>
> http://imgur.com/8ALJmWb
>
>
> As you can see in the image, I have three 6.5 XenServer hosts in a
> resource pool.
>
>
> I just used them when creating a basic zone and the system VMs were
> deployed just fine. However, there are SRs pointing to secondary storage on
> my XenServer-6.5-1 and XenServer-6.5-3 hosts still (there used to be one on
> my XenServer-6.5-2 host, but it went away once the system VMs started up on
> that host).
>
>
> Thoughts?
>
>
> Thanks,
>
> Mike
>



-- 
Daan

[GitHub] cloudstack pull request: utils: Use non-blocking SSL handshake in ...

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/1493#issuecomment-209809491
  
@jburwell fixed all issues


---
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-9334: Support jenv and pyenv t...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1460#issuecomment-209822016
  
so it was a timeout:
```
[INFO] Apache CloudStack Plugin - Quota Service ... SUCCESS [ 
44.634 s]
[INFO] Apache CloudStack Framework - Spring Module  SUCCESS [ 
38.721 s]
[INFO] Apache CloudStack Secondary Storage Controller . FAILURE [10:04 
min]
[INFO] Apache CloudStack Client UI  SKIPPED
[INFO] Apache CloudStack Console Proxy - RDP Client ... SKIPPED
[INFO] Apache CloudStack Console Proxy  SKIPPED
[INFO] Apache CloudStack Console Proxy - Server ... SKIPPED
[INFO] Apache CloudStack Framework - QuickCloud ... SKIPPED
[INFO] 

[INFO] BUILD FAILURE
[INFO] 

[INFO] Total time: 01:25 h
[INFO] Finished at: 2016-04-13T22:08:13+00:00
[INFO] Final Memory: 249M/4518M
[INFO] 

[ERROR] Failed to execute goal 
org.codehaus.mojo:findbugs-maven-plugin:3.0.1:findbugs (findbugs) on project 
cloud-controller-secondary-storage: Execution findbugs of goal 
org.codehaus.mojo:findbugs-maven-plugin:3.0.1:findbugs failed: Timeout: killed 
the sub-process -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e 
switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, 
please read the following articles:
[ERROR] [Help 1] 
http://cwiki.apache.org/confluence/display/MAVEN/PluginExecutionException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the 
command
[ERROR]   mvn  -rf :cloud-controller-secondary-storage
Build step 'Invoke top-level Maven targets' marked build as failure
[CHECKSTYLE] Skipping publisher since build result is FAILURE
[FINDBUGS] Skipping publisher since build result is FAILURE
[PMD] Skipping publisher since build result is FAILURE
[DRY] Skipping publisher since build result is FAILURE
Archiving artifacts
Skipping Cobertura coverage report as build was not UNSTABLE or better ...
Publishing Clover coverage report...
No Clover report will be published due to a Build Failure
Putting comment on the pull request
Finished: FAILURE
```


---
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: Master nuage

[nlivens]

GitHub user nlivens opened a pull request:

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

Master nuage



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

$ git pull https://github.com/nlivens/cloudstack master_nuage

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

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


commit b738ab0e7a6039e0aec9658bab6a25acf645715f
Author: Nick Livens 
Date:   2016-03-02T12:53:47Z

CLOUDSTACK-9242 : Remodel Nuage VSP plugin

commit 1e98c86cf2f73a49f71443e68f37420c647645df
Author: Nick Livens 
Date:   2016-03-16T11:07:50Z

CLOUDSTACK-9294 : Make sure to remove VR from VSD when removing the VPC




---
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-9003] Resource Naming Policie...

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/1492#issuecomment-209856283
  
@ProjectMoon we'll need more information on why you're doing this, why we 
should have it, what is fixes and will it guarantee backward resource-name 
compatibility (for example, vmware vms have this strictly tie up with internal 
ACS vm name, such config are set in each vmware's VM's annotations) and upgrade 
paths


---
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-8562: Dynamic Role-Based API C...

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/1489#issuecomment-209856388
  
thanks @DaanHoogland 


---
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: [discuss] CloudStack logging

[Paul Angus]

Grabbing a couple of examples - these should be debug - I can't 'do' anything 
with this information.

INFO  [o.a.c.f.j.i.AsyncJobManagerImpl] (AsyncJobMgr-Heartbeat-1:ctx-4119d5bc) 
Begin cleanup expired async-jobs
INFO  [o.a.c.f.j.i.AsyncJobManagerImpl] (AsyncJobMgr-Heartbeat-1:ctx-4119d5bc) 
End cleanup expired async-jobs
INFO  [c.c.h.v.r.VmwareResource] (DirectAgentCronJob-2:ctx-3209f65c) Scan hung 
worker VM to recycle
INFO  [c.c.h.v.r.VmwareResource] (DirectAgentCronJob-113:ctx-56d9f9f2) Scan 
hung worker VM to recycle

Whereas this should be WARN. I wouldn't want to lose this by switching off DEBUG

DEBUG [c.c.c.ClusterManagerImpl] (Cluster-Heartbeat-1:ctx-91226be7) Detected 
management node left, id:2, nodeIP:10.2.0.6
DEBUG [c.c.c.ClusterManagerImpl] (Cluster-Heartbeat-1:ctx-0bccd078) Detected 
management node left, id:2, nodeIP:10.2.0.6



Kind regards,

Paul Angus

Regards,

Paul Angus

paul.an...@shapeblue.com 
www.shapeblue.com
53 Chandos Place, Covent Garden, London  WC2N 4HSUK
@shapeblue

-Original Message-
From: Wido den Hollander [mailto:w...@widodh.nl] 
Sent: 14 April 2016 15:04
To: Paul Angus ; dev@cloudstack.apache.org
Subject: Re: [discuss] CloudStack logging


> Op 14 april 2016 om 14:49 schreef Paul Angus :
> 
> 
> Hi All
> 
> I think that we can all agree that CloudStack logs are very difficult 
> to read, especially for operational staff.
> 
> I believe that the primary reason for this is that a large amount of 
> what should be INFO level events are categorised as DEBUG.  The 
> logging therefore must be set at DEBUG for the logs to capture these 
> events.  By defaulting the logging to DEBUG we include a lot of detail 
> which is counter-productive when using the logs to diagnose operational 
> issues.
> 

Yes!

> I think the situation can be greatly improved by reviewing the 
> categorisation of logged events and where necessary re-categorise then 
> such that INFO, WARN and ERROR logging would be sufficient for an 
> 'operational' admin.
> 
> I think a phased approach where the default logging level remains at 
> DEBUG while re-categorisation occurs would mean that nothing 
> materially changes until we are happy with the categorisations. Then 
> we can default the logging level to INFO.
> 

I created a issue for this a while ago:
https://issues.apache.org/jira/browse/CLOUDSTACK-8645

Short: Yes. Just change this where needed.

Wido

> 
> 
> Thoughts everyone?
> 
> 
> 
> 
> Kind regards,
> 
> Paul Angus
> 
> 
> Regards,
> 
> Paul Angus
> 
> paul.an...@shapeblue.com
> www.shapeblue.com
> 53 Chandos Place, Covent Garden, London  WC2N 4HSUK @shapeblue

Re: [discuss] CloudStack logging

[Patrick Dube]

Really agree with this one. With a toolchain like ELK, it isn't as bad
because you can filter out the things you don't want, but generally
speaking, the log levels need to be reevaluated. I would also add that many
log statements would benefit from additional context, but that is a whole
different story.

Another example:
INFO [com.cloud.agent.transport.Request] (StatsCollector-3:ctx-de473a88)
not building log message for '[{}]', _cmds.length == 1

It even says "not building log message".


On Thu, Apr 14, 2016 at 10:18 AM, Simon Weller  wrote:

> I'm all for this as well.
>
> Training our systems folks on how to work through the logs is a pain.
> Focusing in on the most important items is more useful and also makes it
> easier to automate log fetching and categorization.
>
> - Si
>
> 
> From: Paul Angus 
> Sent: Thursday, April 14, 2016 9:06 AM
> To: Wido den Hollander; dev@cloudstack.apache.org
> Subject: RE: [discuss] CloudStack logging
>
> Grabbing a couple of examples - these should be debug - I can't 'do'
> anything with this information.
>
> INFO  [o.a.c.f.j.i.AsyncJobManagerImpl]
> (AsyncJobMgr-Heartbeat-1:ctx-4119d5bc) Begin cleanup expired async-jobs
> INFO  [o.a.c.f.j.i.AsyncJobManagerImpl]
> (AsyncJobMgr-Heartbeat-1:ctx-4119d5bc) End cleanup expired async-jobs
> INFO  [c.c.h.v.r.VmwareResource] (DirectAgentCronJob-2:ctx-3209f65c) Scan
> hung worker VM to recycle
> INFO  [c.c.h.v.r.VmwareResource] (DirectAgentCronJob-113:ctx-56d9f9f2)
> Scan hung worker VM to recycle
>
> Whereas this should be WARN. I wouldn't want to lose this by switching off
> DEBUG
>
> DEBUG [c.c.c.ClusterManagerImpl] (Cluster-Heartbeat-1:ctx-91226be7)
> Detected management node left, id:2, nodeIP:10.2.0.6
> DEBUG [c.c.c.ClusterManagerImpl] (Cluster-Heartbeat-1:ctx-0bccd078)
> Detected management node left, id:2, nodeIP:10.2.0.6
>
>
>
> Kind regards,
>
> Paul Angus
>
> Regards,
>
> Paul Angus
>
> paul.an...@shapeblue.com
> www.shapeblue.com
> 53 Chandos Place, Covent Garden, London  WC2N 4HSUK
> @shapeblue
>
> -Original Message-
> From: Wido den Hollander [mailto:w...@widodh.nl]
> Sent: 14 April 2016 15:04
> To: Paul Angus ; dev@cloudstack.apache.org
> Subject: Re: [discuss] CloudStack logging
>
>
> > Op 14 april 2016 om 14:49 schreef Paul Angus :
> >
> >
> > Hi All
> >
> > I think that we can all agree that CloudStack logs are very difficult
> > to read, especially for operational staff.
> >
> > I believe that the primary reason for this is that a large amount of
> > what should be INFO level events are categorised as DEBUG.  The
> > logging therefore must be set at DEBUG for the logs to capture these
> > events.  By defaulting the logging to DEBUG we include a lot of detail
> > which is counter-productive when using the logs to diagnose operational
> issues.
> >
>
> Yes!
>
> > I think the situation can be greatly improved by reviewing the
> > categorisation of logged events and where necessary re-categorise then
> > such that INFO, WARN and ERROR logging would be sufficient for an
> 'operational' admin.
> >
> > I think a phased approach where the default logging level remains at
> > DEBUG while re-categorisation occurs would mean that nothing
> > materially changes until we are happy with the categorisations. Then
> > we can default the logging level to INFO.
> >
>
> I created a issue for this a while ago:
> https://issues.apache.org/jira/browse/CLOUDSTACK-8645
>
> Short: Yes. Just change this where needed.
>
> Wido
>
> >
> >
> > Thoughts everyone?
> >
> >
> >
> >
> > Kind regards,
> >
> > Paul Angus
> >
> >
> > Regards,
> >
> > Paul Angus
> >
> > paul.an...@shapeblue.com
> > www.shapeblue.com
> > 53 Chandos Place, Covent Garden, London  WC2N 4HSUK @shapeblue
>

[GitHub] cloudstack pull request: CLOUDSTACK-8562: Dynamic Role-Based API C...

[pdube]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59730081
  
--- Diff: api/src/org/apache/cloudstack/acl/RoleType.java ---
@@ -16,18 +16,90 @@
 // under the License.
 package org.apache.cloudstack.acl;
 
+import com.cloud.user.Account;
+import com.google.common.base.Enums;
+import com.google.common.base.Strings;
+
 // Enum for default roles in CloudStack
 public enum RoleType {
-Admin(1), ResourceAdmin(2), DomainAdmin(4), User(8), Unknown(0);
+Admin(1L, Account.ACCOUNT_TYPE_ADMIN, 1),
+ResourceAdmin(2L, Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN, 2),
+DomainAdmin(3L, Account.ACCOUNT_TYPE_DOMAIN_ADMIN, 4),
+User(4L, Account.ACCOUNT_TYPE_NORMAL, 8),
+Unknown(-1L, (short) -1, 0);
 
+private long id;
+private short accountType;
 private int mask;
 
-private RoleType(int mask) {
+RoleType(final long id, final short accountType, final int mask) {
+this.id = id;
+this.accountType = accountType;
 this.mask = mask;
 }
 
-public int getValue() {
+public long getId() {
+return id;
+}
+
+public short getAccountType() {
+return accountType;
+}
+
+public int getMask() {
 return mask;
 }
-}
 
+public static RoleType fromString(final String name) {
+if (!Strings.isNullOrEmpty(name)
+&& Enums.getIfPresent(RoleType.class, name).isPresent()) {
+return RoleType.valueOf(name);
+}
+return null;
+}
+
+public static RoleType fromMask(int mask) {
+for (RoleType roleType : RoleType.values()) {
+if (roleType.getMask() == mask) {
+return roleType;
+}
+}
+return Unknown;
+}
+
+public static RoleType getByAccountType(final short accountType) {
+RoleType roleType = RoleType.Unknown;
+switch (accountType) {
+case Account.ACCOUNT_TYPE_ADMIN:
+roleType = RoleType.Admin;
+break;
+case Account.ACCOUNT_TYPE_DOMAIN_ADMIN:
+roleType = RoleType.DomainAdmin;
+break;
+case Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN:
+roleType = RoleType.ResourceAdmin;
--- End diff --

What is this role type? Is there any documentation on it?


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

[GitHub] cloudstack pull request: [CLOUDSTACK-9003] Resource Naming Policie...

[ProjectMoon]

Github user ProjectMoon commented on the pull request:

https://github.com/apache/cloudstack/pull/1492#issuecomment-209963503
  
@DaanHoogland Do you have any kind of integration tests in mind?


---
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-8562: Dynamic Role-Based API C...

[pdube]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59730927
  
--- Diff: api/src/org/apache/cloudstack/acl/RoleService.java ---
@@ -0,0 +1,43 @@
+// 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.acl;
+
+import org.apache.cloudstack.framework.config.ConfigKey;
+
+import java.util.List;
+
+public interface RoleService {
+
+ConfigKey EnableDynamicApiChecker = new ConfigKey<>("Hidden", 
Boolean.class, "dynamic.apichecker.enabled", "false",
+"If set to true, this enables the dynamic role-based api 
access checker and disables the default static role-based api access checker.",
+true);
+
+boolean isEnabled();
+Role findRole(final Long id);
+Role createRole(final String name, final RoleType roleType, final 
String description);
+boolean updateRole(final Role role, final String name, final RoleType 
roleType, final String description);
+boolean deleteRole(final Role role);
+
+RolePermission findRolePermission(final Long id);
+RolePermission createRolePermission(final Role role, final Rule rule, 
final RolePermission.Permission permission, final String description);
+boolean updateRolePermission(final RolePermission rolePermission, 
final Rule rule, final RolePermission.Permission permission, final String 
description);
+boolean deleteRolePermission(final RolePermission rolePermission);
+
+List findAllRolesBy(final Long id, final String name, final 
RoleType roleType);
--- End diff --

I would have to agree with @DaanHoogland. Splitting into separate methods 
would be clearer and probably simplify the business logic per method as well 
(so it would be simpler to unit test).


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

Re: [discuss] CloudStack logging

[Simon Weller]

I'm all for this as well.

Training our systems folks on how to work through the logs is a pain. Focusing 
in on the most important items is more useful and also makes it easier to 
automate log fetching and categorization. 

- Si


From: Paul Angus 
Sent: Thursday, April 14, 2016 9:06 AM
To: Wido den Hollander; dev@cloudstack.apache.org
Subject: RE: [discuss] CloudStack logging

Grabbing a couple of examples - these should be debug - I can't 'do' anything 
with this information.

INFO  [o.a.c.f.j.i.AsyncJobManagerImpl] (AsyncJobMgr-Heartbeat-1:ctx-4119d5bc) 
Begin cleanup expired async-jobs
INFO  [o.a.c.f.j.i.AsyncJobManagerImpl] (AsyncJobMgr-Heartbeat-1:ctx-4119d5bc) 
End cleanup expired async-jobs
INFO  [c.c.h.v.r.VmwareResource] (DirectAgentCronJob-2:ctx-3209f65c) Scan hung 
worker VM to recycle
INFO  [c.c.h.v.r.VmwareResource] (DirectAgentCronJob-113:ctx-56d9f9f2) Scan 
hung worker VM to recycle

Whereas this should be WARN. I wouldn't want to lose this by switching off DEBUG

DEBUG [c.c.c.ClusterManagerImpl] (Cluster-Heartbeat-1:ctx-91226be7) Detected 
management node left, id:2, nodeIP:10.2.0.6
DEBUG [c.c.c.ClusterManagerImpl] (Cluster-Heartbeat-1:ctx-0bccd078) Detected 
management node left, id:2, nodeIP:10.2.0.6



Kind regards,

Paul Angus

Regards,

Paul Angus

paul.an...@shapeblue.com
www.shapeblue.com
53 Chandos Place, Covent Garden, London  WC2N 4HSUK
@shapeblue

-Original Message-
From: Wido den Hollander [mailto:w...@widodh.nl]
Sent: 14 April 2016 15:04
To: Paul Angus ; dev@cloudstack.apache.org
Subject: Re: [discuss] CloudStack logging


> Op 14 april 2016 om 14:49 schreef Paul Angus :
>
>
> Hi All
>
> I think that we can all agree that CloudStack logs are very difficult
> to read, especially for operational staff.
>
> I believe that the primary reason for this is that a large amount of
> what should be INFO level events are categorised as DEBUG.  The
> logging therefore must be set at DEBUG for the logs to capture these
> events.  By defaulting the logging to DEBUG we include a lot of detail
> which is counter-productive when using the logs to diagnose operational 
> issues.
>

Yes!

> I think the situation can be greatly improved by reviewing the
> categorisation of logged events and where necessary re-categorise then
> such that INFO, WARN and ERROR logging would be sufficient for an 
> 'operational' admin.
>
> I think a phased approach where the default logging level remains at
> DEBUG while re-categorisation occurs would mean that nothing
> materially changes until we are happy with the categorisations. Then
> we can default the logging level to INFO.
>

I created a issue for this a while ago:
https://issues.apache.org/jira/browse/CLOUDSTACK-8645

Short: Yes. Just change this where needed.

Wido

>
>
> Thoughts everyone?
>
>
>
>
> Kind regards,
>
> Paul Angus
>
>
> Regards,
>
> Paul Angus
>
> paul.an...@shapeblue.com
> www.shapeblue.com
> 53 Chandos Place, Covent Garden, London  WC2N 4HSUK @shapeblue

[GitHub] cloudstack pull request: CLOUDSTACK-8562: Dynamic Role-Based API C...

[pdube]

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

https://github.com/apache/cloudstack/pull/1489#discussion_r59730334
  
--- Diff: api/src/org/apache/cloudstack/acl/RoleType.java ---
@@ -16,18 +16,90 @@
 // under the License.
 package org.apache.cloudstack.acl;
 
+import com.cloud.user.Account;
+import com.google.common.base.Enums;
+import com.google.common.base.Strings;
+
 // Enum for default roles in CloudStack
 public enum RoleType {
-Admin(1), ResourceAdmin(2), DomainAdmin(4), User(8), Unknown(0);
+Admin(1L, Account.ACCOUNT_TYPE_ADMIN, 1),
+ResourceAdmin(2L, Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN, 2),
+DomainAdmin(3L, Account.ACCOUNT_TYPE_DOMAIN_ADMIN, 4),
+User(4L, Account.ACCOUNT_TYPE_NORMAL, 8),
+Unknown(-1L, (short) -1, 0);
 
+private long id;
+private short accountType;
 private int mask;
 
-private RoleType(int mask) {
+RoleType(final long id, final short accountType, final int mask) {
+this.id = id;
+this.accountType = accountType;
 this.mask = mask;
 }
 
-public int getValue() {
+public long getId() {
+return id;
+}
+
+public short getAccountType() {
+return accountType;
+}
+
+public int getMask() {
 return mask;
 }
-}
 
+public static RoleType fromString(final String name) {
+if (!Strings.isNullOrEmpty(name)
+&& Enums.getIfPresent(RoleType.class, name).isPresent()) {
+return RoleType.valueOf(name);
+}
+return null;
+}
+
+public static RoleType fromMask(int mask) {
+for (RoleType roleType : RoleType.values()) {
+if (roleType.getMask() == mask) {
+return roleType;
+}
+}
+return Unknown;
+}
+
+public static RoleType getByAccountType(final short accountType) {
+RoleType roleType = RoleType.Unknown;
+switch (accountType) {
+case Account.ACCOUNT_TYPE_ADMIN:
+roleType = RoleType.Admin;
+break;
+case Account.ACCOUNT_TYPE_DOMAIN_ADMIN:
+roleType = RoleType.DomainAdmin;
+break;
+case Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN:
+roleType = RoleType.ResourceAdmin;
+break;
+case Account.ACCOUNT_TYPE_NORMAL:
+roleType = RoleType.User;
+break;
+}
+return roleType;
+}
+
+public static Long getRoleByAccountType(final Long roleId, final Short 
accountType) {
--- End diff --

Why are you passing in a roleId, if you are getting a role id by account 
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.
---

Re: [discuss] CloudStack logging

[Koushik Das]

Based on the namespace log level can be set. It needs to be configured in the 
log4j.
Check client/tomcatconf/log4j-cloud.xml.in in the source code and you will get 
an idea.

-Koushik


From: williamstev...@gmail.com  on behalf of Will 
Stevens 
Sent: Thursday, April 14, 2016 7:29 PM
To: dev@cloudstack.apache.org
Subject: Re: [discuss] CloudStack logging

Do you have to recompile in order to turn off the logging for a specific
package or class?  If yes, that is a show stopper for almost everyone...
If it only requires a management server restart, that is more realistic.

*Will STEVENS*
Lead Developer

*CloudOps* *| *Cloud Solutions Experts
420 rue Guy *|* Montreal *|* Quebec *|* H3J 1S6
w cloudops.com *|* tw @CloudOps_

On Thu, Apr 14, 2016 at 9:48 AM, Daan Hoogland 
wrote:

> On Thu, Apr 14, 2016 at 3:41 PM, Will Stevens 
> wrote:
>
> > Is there an easy way to do that Daan, or is it a tedious task you just
> have
> > to power through?
> >
> ​It is hard work, mostly tedious, sometimes hilarious and most definitely ​
>
> ​devops!​ It certainly wouldn't classify as development.
>
>
>
>
> --
> Daan
>


DISCLAIMER
==
This e-mail may contain privileged and confidential information which is the 
property of Accelerite, a Persistent Systems business. It is intended only for 
the use of the individual or entity to which it is addressed. If you are not 
the intended recipient, you are not authorized to read, retain, copy, print, 
distribute or use this message. If you have received this communication in 
error, please notify the sender and delete all copies of this message. 
Accelerite, a Persistent Systems business does not accept any liability for 
virus infected mails.

RE: [discuss] CloudStack logging

[Chaz PC]

I am confused about the configuration file there two files one is named 
Log4j-cloud.xml The other is Log4j-cloud.xml.in
What is the difference between these two files ? I hope someone can help me 
clear this confusion

 Original message 
From: Koushik Das  
Date: 14/04/2016  6:49 PM  (GMT+04:00) To: 
dev@cloudstack.apache.org Subject: Re: [discuss] CloudStack logging 


Based on the namespace log level can be set. It needs to be configured in the 
log4j.
Check client/tomcatconf/log4j-cloud.xml.in in the source code and you will get 
an idea.

-Koushik


From: williamstev...@gmail.com  on behalf of Will 
Stevens 
Sent: Thursday, April 14, 2016 7:29 PM
To: dev@cloudstack.apache.org
Subject: Re: [discuss] CloudStack logging

Do you have to recompile in order to turn off the logging for a specific
package or class?  If yes, that is a show stopper for almost everyone...
If it only requires a management server restart, that is more realistic.

*Will STEVENS*
Lead Developer

*CloudOps* *| *Cloud Solutions Experts
420 rue Guy *|* Montreal *|* Quebec *|* H3J 1S6
w cloudops.com *|* tw @CloudOps_

On Thu, Apr 14, 2016 at 9:48 AM, Daan Hoogland 
wrote:

> On Thu, Apr 14, 2016 at 3:41 PM, Will Stevens 
> wrote:
>
> > Is there an easy way to do that Daan, or is it a tedious task you just
> have
> > to power through?
> >
> ​It is hard work, mostly tedious, sometimes hilarious and most definitely ​
>
> ​devops!​ It certainly wouldn't classify as development.
>
>
>
>
> --
> Daan
>


DISCLAIMER
==
This e-mail may contain privileged and confidential information which is the 
property of Accelerite, a Persistent Systems business. It is intended only for 
the use of the individual or entity to which it is addressed. If you are not 
the intended recipient, you are not authorized to read, retain, copy, print, 
distribute or use this message. If you have received this communication in 
error, please notify the sender and delete all copies of this message. 
Accelerite, a Persistent Systems business does not accept any liability for 
virus infected mails.

[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...

[swill]

Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1403#issuecomment-210009483
  
This does not compile:
```
[ERROR] COMPILATION ERROR : 
[INFO] -
[ERROR] 
/data/git/cs1/cloudstack/dist/rpmbuild/BUILD/cloudstack-4.9.0-SNAPSHOT/plugins/api/solidfire-intg-test/src/org/apache/cloudstack/solidfire/ApiSolidFireServiceImpl.java:[93,68]
 error: cannot find symbol
[INFO] 1 error
```

It fails here:
```
[INFO] Apache CloudStack Plugin - Storage Volume default provider  SUCCESS 
[2.494s]
[INFO] Apache CloudStack Plugin - Storage Volume SolidFire Provider  
SUCCESS [6.477s]
[INFO] Apache CloudStack Plugin - API SolidFire .. FAILURE [2.601s]
[INFO] Apache CloudStack Plugin - API Discovery .. SKIPPED
[INFO] Apache CloudStack Plugin - ACL Static Role Based .. SKIPPED
```


---
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: Taking fast and efficient volume snapshot...

[swill]

Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1403#issuecomment-210012980
  
Looking closer at this code.  It look like `primaryStoreDriver` could 
easily be `null` in this case, so that return statement could potentially cause 
a null pointer 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: Add perl-modules as install dependency fo...

[eriweb]

Github user eriweb commented on the pull request:

https://github.com/apache/cloudstack/pull/1495#issuecomment-210016535
  
Quite horribly that we need those modules for a simple perl script of 58 
lines (including license header and empty lines). Not related to your PR 
though, but a good candidate to be rewritten in bash or python


---
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: Taking fast and efficient volume snapshot...

[swill]

Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1403#issuecomment-210085510
  
i will blow away my git repo and start from scratch and we will go from 
there.  :)  thx...


---
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: Feature proposal: Resource naming policies

[Daan Hoogland]

sounds usefull for companies that for instance want to enforce uuid in name
or include some user string in it, same would be true for networks. look
forward to your design.

On Thu, Apr 14, 2016 at 1:40 PM, Jeff Hair  wrote:

> Yesterday, we submitted this pull request:
> https://github.com/apache/cloudstack/pull/1492
>
> This originally grew out of making the VirtualMachineName class non-static
> (original PR is mentioned in the above link). We're presenting this as a
> refactoring of the existing code to enable more extensibility and
> flexibility, make unit testing easier, and unify the way CloudStack
> generates resource names.
>
> There is an associated JIRA ticket at CLOUDSTACK-9003. I will be writing up
> a design document for the CS wiki in the next few days.
>
> jburwell wanted me to open a discussion on the developer list about the PR
> and how it is implemented:
>
> There is now a ResourceNamingPolicyManager and a bunch of
> ResourceNamingPolicies. The manager exposes a method to get a policy for a
> type of resource, and the naming policies generate UUIDs + resource names.
>
> The default naming policies generate names exactly the same way as they are
> created now. This is in a new module called default-naming-policies. By
> excluding this module and loading our own naming policies, we gain the
> ability to change how names are generated.
>



-- 
Daan

Re: [discuss] CloudStack logging

[Will Stevens]

Is there an easy way to do that Daan, or is it a tedious task you just have
to power through?

I agree this initiative would be helpful.
On Apr 14, 2016 9:04 AM, "Daan Hoogland"  wrote:

> I largely agree with just one sneer to the operators amongst us: the level
> can be set on a per package and even class basis. So you can work through
> disabling anoying logging step by step. Makes sense to keep looking at the
> source in the meanwhile to make sure sensible diagnostics remains
> available.
>
> On Thu, Apr 14, 2016 at 2:49 PM, Paul Angus 
> wrote:
>
> > Hi All
> >
> > I think that we can all agree that CloudStack logs are very difficult to
> > read, especially for operational staff.
> >
> > I believe that the primary reason for this is that a large amount of what
> > should be INFO level events are categorised as DEBUG.  The logging
> > therefore must be set at DEBUG for the logs to capture these events.  By
> > defaulting the logging to DEBUG we include a lot of detail which is
> > counter-productive when using the logs to diagnose operational issues.
> >
> > I think the situation can be greatly improved by reviewing the
> > categorisation of logged events and where necessary re-categorise then
> such
> > that INFO, WARN and ERROR logging would be sufficient for an
> 'operational'
> > admin.
> >
> > I think a phased approach where the default logging level remains at
> DEBUG
> > while re-categorisation occurs would mean that nothing materially changes
> > until we are happy with the categorisations. Then we can default the
> > logging level to INFO.
> >
> >
> >
> > Thoughts everyone?
> >
> >
> >
> >
> > Kind regards,
> >
> > Paul Angus
> >
> >
> > Regards,
> >
> > Paul Angus
> >
> > paul.an...@shapeblue.com
> > www.shapeblue.com
> > 53 Chandos Place, Covent Garden, London  WC2N 4HSUK
> > @shapeblue
> >
>
>
>
> --
> Daan
>

[GitHub] cloudstack pull request: CLOUDSTACK-9180: Optimize concurrent VM d...

[bvbharatk]

Github user bvbharatk commented on the pull request:

https://github.com/apache/cloudstack/pull/1251#issuecomment-209922105
  
### ACS CI BVT Run
 **Sumarry:**
 Build Number 175
 Hypervisor xenserver
 NetworkType Advanced
 Passed=34
 Failed=21
 Skipped=2

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


**Failed tests:**
* test_affinity_groups_projects.py

 * ContextSuite context=TestDeployVmWithAffinityGroup>:teardown Failed

* test_vpc_vpn.py

 * ContextSuite context=TestVpcRemoteAccessVpn>:setup Failing since 5 runs

 * ContextSuite context=TestVpcSite2SiteVpn>:setup Failing since 3 runs

* test_scale_vm.py

 * ContextSuite context=TestScaleVm>:teardown Failed

* test_routers_iptables_default_policy.py

 * test_01_single_VPC_iptables_policies Failed

 * ContextSuite context=TestVPCIpTablesPolicies>:teardown Failed

* test_quota.py

 * test_01_quota Failing since 3 runs

 * test_02_quota Failing since 3 runs

 * test_03_quota Failing since 3 runs

 * test_04_quota Failing since 3 runs

 * test_05_quota Failing since 3 runs

 * test_06_quota Failing since 3 runs

 * test_07_quota Failing since 3 runs

* test_loadbalance.py

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

* test_network.py

 * test_delete_account Failing since 2 runs

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

 * ContextSuite context=TestPublicIP>:setup Failed

 * test_reboot_router Failing since 2 runs

 * test_releaseIP Failing since 2 runs

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

* test_password_server.py

 * test_isolate_network_password_server Failing since 2 runs


**Skipped tests:**
test_vm_nic_adapter_vmxnet3
test_deploy_vgpu_enabled_vm

**Passed test suits:**
test_deploy_vm_with_userdata.py
test_over_provisioning.py
test_guest_vlan_range.py
test_service_offerings.py
test_reset_vm_on_reboot.py
test_deploy_vms_with_varied_deploymentplanners.py
test_deploy_vm_iso.py
test_public_ip_range.py
test_multipleips_per_nic.py
test_regions.py
test_resource_detail.py
test_secondary_storage.py
test_vm_life_cycle.py


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

[GitHub] cloudstack pull request: Fixes regarding VOLUME_DELETE events resu...

[nnesic]

Github user nnesic commented on the pull request:

https://github.com/apache/cloudstack/pull/1491#issuecomment-209922376
  
@koushik-das  We are mostly working with 4.7 version, where other volume 
usage events get emitted correctly. We haven't investigated the behavior on 
master yet.


---
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-9003] Resource Naming Policie...

[DaanHoogland]

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

https://github.com/apache/cloudstack/pull/1492#discussion_r59717381
  
--- Diff: engine/schema/src/com/cloud/naming/ConsoleProxyNamingPolicy.java 
---
@@ -0,0 +1,9 @@
+package com.cloud.naming;
--- End diff --

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

[nnesic]

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

https://github.com/apache/cloudstack/pull/1491#discussion_r59707938
  
--- Diff: server/src/com/cloud/user/AccountManagerImpl.java ---
@@ -761,6 +774,17 @@ protected boolean cleanupAccount(AccountVO account, 
long callerUserId, Account c
 s_logger.error("Unable to expunge vm: " + vm.getId());
 accountCleanupNeeded = true;
 }
+else if 
(!vm.getState().equals(VirtualMachine.State.Destroyed)) {
--- End diff --

The check on line 777 makes sure that the volume wasn't already destroyed 
before expunging. In that case, the delete event has already been emitted 
somewhere else. 

The expunge thread does emit other usage events correctly (we tested 
deleting an account having active VMs, datadisks, allocated IP's, templates, 
and snapshots, and upon deleting the account, "delete" usage events were 
emitted for each of them). The emission of the ROOT volume's "delete" event is 
handled on a more case-by-case basis (In UserVmManager for regular VM destroys, 
in VolumeOrchestrator for failed vm creation, etc), and the case where the root 
volume gets deleted as part of account deletion was not being handled.


---
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: [discuss] CloudStack logging

[Daan Hoogland]

I largely agree with just one sneer to the operators amongst us: the level
can be set on a per package and even class basis. So you can work through
disabling anoying logging step by step. Makes sense to keep looking at the
source in the meanwhile to make sure sensible diagnostics remains available.

On Thu, Apr 14, 2016 at 2:49 PM, Paul Angus 
wrote:

> Hi All
>
> I think that we can all agree that CloudStack logs are very difficult to
> read, especially for operational staff.
>
> I believe that the primary reason for this is that a large amount of what
> should be INFO level events are categorised as DEBUG.  The logging
> therefore must be set at DEBUG for the logs to capture these events.  By
> defaulting the logging to DEBUG we include a lot of detail which is
> counter-productive when using the logs to diagnose operational issues.
>
> I think the situation can be greatly improved by reviewing the
> categorisation of logged events and where necessary re-categorise then such
> that INFO, WARN and ERROR logging would be sufficient for an 'operational'
> admin.
>
> I think a phased approach where the default logging level remains at DEBUG
> while re-categorisation occurs would mean that nothing materially changes
> until we are happy with the categorisations. Then we can default the
> logging level to INFO.
>
>
>
> Thoughts everyone?
>
>
>
>
> Kind regards,
>
> Paul Angus
>
>
> Regards,
>
> Paul Angus
>
> paul.an...@shapeblue.com
> www.shapeblue.com
> 53 Chandos Place, Covent Garden, London  WC2N 4HSUK
> @shapeblue
>



-- 
Daan

Re: [discuss] CloudStack logging

[Daan Hoogland]

On Thu, Apr 14, 2016 at 3:41 PM, Will Stevens 
wrote:

> Is there an easy way to do that Daan, or is it a tedious task you just have
> to power through?
>
​It is hard work, mostly tedious, sometimes hilarious and most definitely ​

​devops!​ It certainly wouldn't classify as development.




-- 
Daan

Re: [discuss] CloudStack logging

[Wido den Hollander]

> Op 14 april 2016 om 14:49 schreef Paul Angus :
> 
> 
> Hi All
> 
> I think that we can all agree that CloudStack logs are very difficult to read,
> especially for operational staff.
> 
> I believe that the primary reason for this is that a large amount of what
> should be INFO level events are categorised as DEBUG.  The logging therefore
> must be set at DEBUG for the logs to capture these events.  By defaulting the
> logging to DEBUG we include a lot of detail which is counter-productive when
> using the logs to diagnose operational issues.
> 

Yes!

> I think the situation can be greatly improved by reviewing the categorisation
> of logged events and where necessary re-categorise then such that INFO, WARN
> and ERROR logging would be sufficient for an 'operational' admin.
> 
> I think a phased approach where the default logging level remains at DEBUG
> while re-categorisation occurs would mean that nothing materially changes
> until we are happy with the categorisations. Then we can default the logging
> level to INFO.
> 

I created a issue for this a while ago:
https://issues.apache.org/jira/browse/CLOUDSTACK-8645

Short: Yes. Just change this where needed.

Wido

> 
> 
> Thoughts everyone?
> 
> 
> 
> 
> Kind regards,
> 
> Paul Angus
> 
> 
> Regards,
> 
> Paul Angus
> 
> paul.an...@shapeblue.com 
> www.shapeblue.com
> 53 Chandos Place, Covent Garden, London  WC2N 4HSUK
> @shapeblue

[discuss] CloudStack logging

[Paul Angus]

Hi All

I think that we can all agree that CloudStack logs are very difficult to read, 
especially for operational staff.

I believe that the primary reason for this is that a large amount of what 
should be INFO level events are categorised as DEBUG.  The logging therefore 
must be set at DEBUG for the logs to capture these events.  By defaulting the 
logging to DEBUG we include a lot of detail which is counter-productive when 
using the logs to diagnose operational issues.

I think the situation can be greatly improved by reviewing the categorisation 
of logged events and where necessary re-categorise then such that INFO, WARN 
and ERROR logging would be sufficient for an 'operational' admin.

I think a phased approach where the default logging level remains at DEBUG 
while re-categorisation occurs would mean that nothing materially changes until 
we are happy with the categorisations. Then we can default the logging level to 
INFO.



Thoughts everyone?




Kind regards,

Paul Angus


Regards,

Paul Angus

paul.an...@shapeblue.com 
www.shapeblue.com
53 Chandos Place, Covent Garden, London  WC2N 4HSUK
@shapeblue

[GitHub] cloudstack pull request: [CLOUDSTACK-9003] Resource Naming Policie...

[DaanHoogland]

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

https://github.com/apache/cloudstack/pull/1492#discussion_r59717124
  
--- Diff: 
plugins/naming-policies/src/com/cloud/naming/DefaultConsoleProxyNamingPolicy.java
 ---
@@ -0,0 +1,58 @@
+package com.cloud.naming;
--- End diff --

license 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-9003] Resource Naming Policie...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1492#issuecomment-209945535
  
@ProjectMoon I will run through the code after I read the detailed design. 
one remark so far. This is a nice feature to have. please add integration tests 
for it.


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

Re: [discuss] CloudStack logging

[Will Stevens]

Do you have to recompile in order to turn off the logging for a specific
package or class?  If yes, that is a show stopper for almost everyone...
If it only requires a management server restart, that is more realistic.

*Will STEVENS*
Lead Developer

*CloudOps* *| *Cloud Solutions Experts
420 rue Guy *|* Montreal *|* Quebec *|* H3J 1S6
w cloudops.com *|* tw @CloudOps_

On Thu, Apr 14, 2016 at 9:48 AM, Daan Hoogland 
wrote:

> On Thu, Apr 14, 2016 at 3:41 PM, Will Stevens 
> wrote:
>
> > Is there an easy way to do that Daan, or is it a tedious task you just
> have
> > to power through?
> >
> ​It is hard work, mostly tedious, sometimes hilarious and most definitely ​
>
> ​devops!​ It certainly wouldn't classify as development.
>
>
>
>
> --
> Daan
>

Re: Feature proposal: Resource naming policies

[Koushik Das]

Are these the names that are sent to HV? For e.g. VM name in the format 
i-xx-yy-VM. Currently there are functionalities that rely on the internal 
naming convention. All such functionalities needs to be handled appropriately.

-Koushik


From: Jeff Hair 
Sent: Thursday, April 14, 2016 5:10 PM
To: dev@cloudstack.apache.org
Subject: Feature proposal: Resource naming policies

Yesterday, we submitted this pull request:
https://github.com/apache/cloudstack/pull/1492

This originally grew out of making the VirtualMachineName class non-static
(original PR is mentioned in the above link). We're presenting this as a
refactoring of the existing code to enable more extensibility and
flexibility, make unit testing easier, and unify the way CloudStack
generates resource names.

There is an associated JIRA ticket at CLOUDSTACK-9003. I will be writing up
a design document for the CS wiki in the next few days.

jburwell wanted me to open a discussion on the developer list about the PR
and how it is implemented:

There is now a ResourceNamingPolicyManager and a bunch of
ResourceNamingPolicies. The manager exposes a method to get a policy for a
type of resource, and the naming policies generate UUIDs + resource names.

The default naming policies generate names exactly the same way as they are
created now. This is in a new module called default-naming-policies. By
excluding this module and loading our own naming policies, we gain the
ability to change how names are generated.


DISCLAIMER
==
This e-mail may contain privileged and confidential information which is the 
property of Accelerite, a Persistent Systems business. It is intended only for 
the use of the individual or entity to which it is addressed. If you are not 
the intended recipient, you are not authorized to read, retain, copy, print, 
distribute or use this message. If you have received this communication in 
error, please notify the sender and delete all copies of this message. 
Accelerite, a Persistent Systems business does not accept any liability for 
virus infected mails.

Re: Feature proposal: Resource naming policies

[Jeff Hair]

With this feature, it is possible to change the names that get sent to the
hypervisor, yes. In 4.2 we actually had to fix an issue with the security
groups because they weren't parsing properly. That isn't in the commit yet.
We will put it in.

It would be useful to have a list of all the places that rely on the
internal naming convention. security_groups.py and friends is the only one
i know of off the top of my head.

On Thu, Apr 14, 2016 at 4:01 PM, Koushik Das 
wrote:

> Are these the names that are sent to HV? For e.g. VM name in the format
> i-xx-yy-VM. Currently there are functionalities that rely on the internal
> naming convention. All such functionalities needs to be handled
> appropriately.
>
> -Koushik
>
> 
> From: Jeff Hair 
> Sent: Thursday, April 14, 2016 5:10 PM
> To: dev@cloudstack.apache.org
> Subject: Feature proposal: Resource naming policies
>
> Yesterday, we submitted this pull request:
> https://github.com/apache/cloudstack/pull/1492
>
> This originally grew out of making the VirtualMachineName class non-static
> (original PR is mentioned in the above link). We're presenting this as a
> refactoring of the existing code to enable more extensibility and
> flexibility, make unit testing easier, and unify the way CloudStack
> generates resource names.
>
> There is an associated JIRA ticket at CLOUDSTACK-9003. I will be writing up
> a design document for the CS wiki in the next few days.
>
> jburwell wanted me to open a discussion on the developer list about the PR
> and how it is implemented:
>
> There is now a ResourceNamingPolicyManager and a bunch of
> ResourceNamingPolicies. The manager exposes a method to get a policy for a
> type of resource, and the naming policies generate UUIDs + resource names.
>
> The default naming policies generate names exactly the same way as they are
> created now. This is in a new module called default-naming-policies. By
> excluding this module and loading our own naming policies, we gain the
> ability to change how names are generated.
>
>
> DISCLAIMER
> ==
> This e-mail may contain privileged and confidential information which is
> the property of Accelerite, a Persistent Systems business. It is intended
> only for the use of the individual or entity to which it is addressed. If
> you are not the intended recipient, you are not authorized to read, retain,
> copy, print, distribute or use this message. If you have received this
> communication in error, please notify the sender and delete all copies of
> this message. Accelerite, a Persistent Systems business does not accept any
> liability for virus infected mails.
>

Re: [GitHub] cloudstack pull request: [CLOUDSTACK-9003] Resource Naming Policie...

[Daan Hoogland]

On Thu, Apr 14, 2016 at 4:14 PM, ProjectMoon  wrote:

> Github user ProjectMoon commented on the pull request:
>
> https://github.com/apache/cloudstack/pull/1492#issuecomment-209963503
>
> @DaanHoogland Do you have any kind of integration tests in mind?
>
​I could imagine a policy that keeps a static counter and name a resource
bla- and then checks if the next created resource has the
right name for each newly created resource; bla-?​


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



-- 
Daan

[GitHub] cloudstack pull request: Fix Sync of template.properties in Swift

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1331#issuecomment-210018309
  
@syed, I totally understand that you do not have much experience with Java 
development and that you were following the practices already put in place. The 
problem is that we have so many cases of what not to do. I find it very good 
that you are willing to learn and help us improve the ACS code base.

So let’s go to the work.

Sure I changed the variable “logger” to protected, I needed to access 
it on a test case, so I changed it to an access delimiter that I could use; you 
should always try to use the most restrictive one, but at the end in Java those 
access delimiters can always be bypassed using Reflections. The protected 
delimiter states that children’s of that class and classes at the same 
package will have access to that variable/method.

I also removed the “final” because I needed to change that variable to 
the test with a mocked one. The final delimiter would not allow that; 
therefore, I had to remove it. That is why I would normally avoid the use of 
final everywhere, it can complicate things. I would only use final where I 
really want to be strict and not allow a variable change on the fly.

I removed the static to transform that variable into an “instance” 
attribute; those objects are spring beans, and they are working as singletons, 
there will be no more than one instance of them. That is why I see no reason to 
use that attribute as a class field.
Now, you could go over all of the classes that extend 
“NfsSecondaryStorageResource”, they would have their own “logger” 
variable, then you could remove the “s_logger” they are using and use the 
one inherited. Additionally, you would be using a more common name for that 
variable that is “logger”. 

For your second batch of questions, I prefer to use the most restrictive 
delimiter access possible. I always start with private and then I go opening 
them on a need basis. I have no idea why someone would do different, such as 
using “public static final” when it is not needed. ACS has a lot of code 
that should not be taken as reference. In doubt, you can always call for help; 
there are folks here that are great devs.






---
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: Add perl-modules as install dependency fo...

[jburwell]

Github user jburwell commented on the pull request:

https://github.com/apache/cloudstack/pull/1495#issuecomment-210018290
  
@sverrirab how much effort would it be to port to Python?  That seems far 
preferable for two reasons.  First, as @eriweb rightly points out, it is a 
large dependency to take on for such a small script.  Second, Python and bash 
are our scripting languages of choice.  It does not seem sustainable to add 
runtimes/libraries for every scripting language people might want to use.  Such 
sprawl will not only lead to bloat, but a system that fewer and fewer people 
can maintain.


---
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: [discuss] CloudStack logging

[Paul Angus]

The file in a running installation you want is:
/etc/cloudstack/management/ log4j-cloud.xml

It is generally dynamic - you don't have to restart mgmt. server, it just takes 
a few seconds to kick in.

If you're looking at the source code, various components use logging - 
SystemVMs, KVM agents, CloudStack mgmt. servers.
So there'll be multiple occurrences
As Koushik says, I think ./client/tomcatconf/log4j-cloud.xml.in is the one for 
the mgmt. server.

Kind regards,

Paul Angus

Regards,

Paul Angus

paul.an...@shapeblue.com 
www.shapeblue.com
53 Chandos Place, Covent Garden, London  WC2N 4HSUK
@shapeblue

-Original Message-
From: Chaz PC [mailto:dreeems4e...@hotmail.com] 
Sent: 14 April 2016 16:01
To: dev@cloudstack.apache.org
Subject: RE: [discuss] CloudStack logging

I am confused about the configuration file there two files one is named 
Log4j-cloud.xml The other is Log4j-cloud.xml.in What is the difference between 
these two files ? I hope someone can help me clear this confusion 

 Original message 
From: Koushik Das  
Date: 14/04/2016  6:49 PM  (GMT+04:00) To: 
dev@cloudstack.apache.org Subject: Re: [discuss] CloudStack logging 
  Based on the namespace log level can be set. It needs to be 
configured in the log4j.
Check client/tomcatconf/log4j-cloud.xml.in in the source code and you will get 
an idea.

-Koushik


From: williamstev...@gmail.com  on behalf of Will 
Stevens 
Sent: Thursday, April 14, 2016 7:29 PM
To: dev@cloudstack.apache.org
Subject: Re: [discuss] CloudStack logging

Do you have to recompile in order to turn off the logging for a specific 
package or class?  If yes, that is a show stopper for almost everyone...
If it only requires a management server restart, that is more realistic.

*Will STEVENS*
Lead Developer

*CloudOps* *| *Cloud Solutions Experts
420 rue Guy *|* Montreal *|* Quebec *|* H3J 1S6 w cloudops.com *|* tw @CloudOps_

On Thu, Apr 14, 2016 at 9:48 AM, Daan Hoogland 
wrote:

> On Thu, Apr 14, 2016 at 3:41 PM, Will Stevens 
> 
> wrote:
>
> > Is there an easy way to do that Daan, or is it a tedious task you 
> > just
> have
> > to power through?
> >
> ​It is hard work, mostly tedious, sometimes hilarious and most 
> definitely ​
>
> ​devops!​ It certainly wouldn't classify as development.
>
>
>
>
> --
> Daan
>


DISCLAIMER
==
This e-mail may contain privileged and confidential information which is the 
property of Accelerite, a Persistent Systems business. It is intended only for 
the use of the individual or entity to which it is addressed. If you are not 
the intended recipient, you are not authorized to read, retain, copy, print, 
distribute or use this message. If you have received this communication in 
error, please notify the sender and delete all copies of this message. 
Accelerite, a Persistent Systems business does not accept any liability for 
virus infected mails.

[GitHub] cloudstack pull request: [CLOUDSTACK-9003] Resource Naming Policie...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1492#issuecomment-210056555
  
@jburwell we are talking about integration test code here! not about a 
production policy.


---
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: Fix Sync of template.properties in Swift

[jburwell]

Github user jburwell commented on the pull request:

https://github.com/apache/cloudstack/pull/1331#issuecomment-210066343
  
@rafaelweingartner all statics should ``final`` unless there is explicit 
handling for their value changing due to thread safety issues. For loggers, 
they should always be ``private`` and ``static``.  Log4j provides the means to 
retrieve the same logger across threads and contexts by name.  Therefore, if 
you want 5 classes to share the same logger, retrieve the logger for that class 
by name in each class to avoid the potential pitfalls of static shadowing (i.e. 
instead of declaring ``private static final Logger LOGGER = 
Logger.getLogger(MyClass.class)``, declare it as ``private static final Logger 
LOGGER = Logger.getLogger(CONSTANT_WITH_THE_NAME_OF_ COMMON_LOGGER)``).

As I mentioned in a previous comment, these type of compiler gymnastics are 
not necessary to test log output.  Log4j provides the means to attach a custom 
appender to the logger so that you can monitor and test output.


---
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-9003] Resource Naming Policie...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1492#issuecomment-210052430
  
sorry for not answerring on github

I could imagine a policy that keeps a static counter and name a resource
bla- and then checks if the next created resource has the
right name for each newly created resource; bla-


---
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-9003] Resource Naming Policie...

[jburwell]

Github user jburwell commented on the pull request:

https://github.com/apache/cloudstack/pull/1492#issuecomment-210055791
  
@DaanHoogland -1 to counters -- the require coordination across all nodes 
in the cluster.  In multi-region configurations, they would require 
coordination across multi-regions.  (Database sequences are incredibly 
expensive in clustered configurations)  We have enough technical debt with our 
sequence based counters in the database.  


---
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: Taking fast and efficient volume snapshot...

[mike-tutkowski]

Github user mike-tutkowski commented on the pull request:

https://github.com/apache/cloudstack/pull/1403#issuecomment-210066399
  
@swill Weird, I've compiled ee53705 with the following command on two 
separate machines and I ran regression tests against that SHA for a few hours 
yesterday:

mvn -P developer,systemvm clean install -D noredist

Let me look into this - thanks.


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

[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...

[mike-tutkowski]

Github user mike-tutkowski commented on the pull request:

https://github.com/apache/cloudstack/pull/1403#issuecomment-210072035
  
@swill This is pretty strange. The code that is failing to compile for you 
is actually not part of the codebase for ee53705:

https://github.com/mike-tutkowski/cloudstack/tree/xs-snapshots/plugins/api

All API plug-ins are in the location above and the one failing for you is 
not in there.

What do you think, Will?


---
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: Taking fast and efficient volume snapshot...

[mike-tutkowski]

Github user mike-tutkowski commented on the pull request:

https://github.com/apache/cloudstack/pull/1403#issuecomment-210072697
  
This might be a better link (as it includes the SHA):


https://github.com/apache/cloudstack/tree/ee5370536ac3f87457d5f74adbbb8c6af88da449/plugins/api


---
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: Fix Sync of template.properties in Swift

[rafaelweingartner]

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

https://github.com/apache/cloudstack/pull/1331#discussion_r59762357
  
--- Diff: 
services/secondary-storage/server/test/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResourceTest.java
 ---
@@ -88,6 +93,19 @@ public void testMount() throws Exception {
 }
 }
 
+@Test
+public void testCleanupNfsStaging(){
+TemplateObjectTO templateMock = 
Mockito.mock(TemplateObjectTO.class);
+Exception exception = new Exception();
+resource.logger = Mockito.mock(Logger.class);
+
+Mockito.doNothing().when(resource.logger).debug("Failed to clean 
up staging area:", exception);
+
Mockito.when(resource.execute(any(DeleteCommand.class))).thenThrow(exception);
--- End diff --

I didn't use that other approach because I think it adds more 
complications. I find it easier to use the Mockito and then just checking if 
the method was used. It feels more natural.


---
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: Fix Sync of template.properties in Swift

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1331#issuecomment-210078579
  
@jburwell I agree with you that all static can be final. I disagree that 
loggers should be static, but that is a matter of taste. I understand the cache 
methods of Log4J, but still I do not like much of using static variables. It 
feels more natural to use the instance attribute when logging.

I find it much more complications to add an appender and then use it to 
test if some method has been used or some message logged.



---
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-9003] Resource Naming Policie...

[jburwell]

Github user jburwell commented on the pull request:

https://github.com/apache/cloudstack/pull/1492#issuecomment-210078861
  
@DaanHoogland I apologize -- I missed the context.  Yes, for an integration 
test, a counter would be perfectly acceptable, and likely, the best choice to 
ensure determinism.


---
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: Add perl-modules as install dependency fo...

[sverrirab]

GitHub user sverrirab opened a pull request:

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

Add perl-modules as install dependency for cloudstack-agent

Required to run perl scripts that configure networking for VMs.

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

$ git pull https://github.com/greenqloud/cloudstack 
pr-install-perl-modules-on-agent

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

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


commit 6d6bd510c2f5dc3fe076183379388521a55f95fe
Author: Sverrir Berg 
Date:   2016-04-13T16:48:31Z

Add perl-modules as install dependency for cloudstack-agent

Required to run perl scripts that configure networking for VMs.  That 
script fails silently if this is not installed.




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