You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by ProjectMoon <gi...@git.apache.org> on 2016/04/13 19:05:52 UTC

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

GitHub user ProjectMoon opened a pull request:

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

    [CLOUDSTACK-9003] Resource Naming Policies

    Pull request #988, completed and reinvented.
    
    This is an initial submission, a request for commentary.
    
    This started with attempting to make the VirtualMachineName class non-static, and turned into a complete overhaul of how resources in CloudStack are named. Each supported resource has a naming policy that can be overridden to change the way UUIDs and names are generated for them. The primary use case for this is so CloudStack can emulate the naming schemes of other clouds. The default naming policies are in cloud-plugin-naming-policies, cloud-server has the server-naming module which contains the registry lifecycle, and the registered policies are injected into the core managers context.
    
    The supported resources are:
    
    * User virtual machines
    * All kinds of system VMs
    * Security groups
    * Volumes
    * Snapshots
    
    There is one issue we have before we consider this ready for submission. The server-naming module has an ordering dependency. It must be loaded first in cloud-server in order for everything to work. Currently attached to this PR is a second commit which adds a comparator that forces the server-naming module to the top of the list when loading modules. We obviously want to get rid of this and are looking for suggestions on how to do this.

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

    $ git pull https://github.com/greenqloud/cloudstack pr-naming-policies

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

    https://github.com/apache/cloudstack/pull/1492.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 #1492
    
----
commit df8fab0c79e16c2afbd89274d4df23cf139106f8
Author: nnesic <ne...@greenqloud.com>
Date:   2015-12-28T10:52:54Z

    Implement naming policy feature.
    
    The naming policies feature is a large refactoring of how CloudStack
    generates names and UUIDs for the following resources: virtual
    machines (user instances and system VMs), security groups, templates,
    volumes, and snapshots. Previously, most of these resources were
    generated in a non-uniform way, making use of static classes or simple
    UUID generation.
    
    With the naming policies feature, each type of supported resource has
    a specific naming policy to go along with it that generates UUIDs and
    resource names. This allows CloudStack to emulate different kinds of
    naming schemes (such as those found in other clouds).

commit 006ce088f531eda6b792dfa73cbb77fdb34c38ca
Author: nnesic <ne...@greenqloud.com>
Date:   2016-02-18T11:00:23Z

    Added a comparator that loads the server-naming module first.

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by DaanHoogland <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1492#discussion_r59860632
  
    --- Diff: api/src/com/cloud/naming/ResourceNamingPolicyManager.java ---
    @@ -0,0 +1,32 @@
    +// 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 com.cloud.naming;
    +
    +import com.cloud.utils.component.Manager;
    +
    +public interface ResourceNamingPolicyManager extends Manager {
    +
    +
    +    /**
    +     * Finds a resource naming policy among the registered ones by its class
    +     * @param policyClass
    +     * @return
    +     */
    +    @SuppressWarnings("rawtypes")
    +    public <T extends ResourceNamingPolicy> T getPolicy(Class<T> policyClass);
    --- End diff --
    
    I'm all for simplifying the code as much as possible. CloudStack has reinvented a lot of wheels since its creation (the entire DAO being the biggest offender in my opinion).
    
    If we can change these to Spring singletons and inject them across all relevant modules and maintain the ability to change them, then I am fine with the idea. Cross-module injection is accomplished by using the extension registries and referencing their "getRegistered" property in Spring expression language in the XML files.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1492: [CLOUDSTACK-9003] Resource Naming Policies

Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the issue:

    https://github.com/apache/cloudstack/pull/1492
  
    Rebased to latest master.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the pull request:

    https://github.com/apache/cloudstack/pull/1492#issuecomment-209634604
  
    @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?
    
    Also, could you please add an [FS](https://cwiki.apache.org/confluence/display/CLOUDSTACK/Design+Document+Template) to the [wiki](https://cwiki.apache.org/confluence/display/CLOUDSTACK/Design) 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.
    
    /cc @bhaisaab @PaulAngus


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the pull request:

    https://github.com/apache/cloudstack/pull/1492#issuecomment-209550386
  
    Jenkins build is currently failing on this due to needing to "resolve index first." Seems like an issue with the build server.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1492#discussion_r59863002
  
    --- Diff: api/src/com/cloud/naming/ResourceNamingPolicyManager.java ---
    @@ -0,0 +1,32 @@
    +// 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 com.cloud.naming;
    +
    +import com.cloud.utils.component.Manager;
    +
    +public interface ResourceNamingPolicyManager extends Manager {
    +
    +
    +    /**
    +     * Finds a resource naming policy among the registered ones by its class
    +     * @param policyClass
    +     * @return
    +     */
    +    @SuppressWarnings("rawtypes")
    +    public <T extends ResourceNamingPolicy> T getPolicy(Class<T> policyClass);
    --- End diff --
    
    I was doing going over your PR and noticed that you have already created those policies as a Spring singletons. It would be a matter of design a way to enable the overwrite through XML configuration. 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1492#discussion_r59859628
  
    --- Diff: api/src/com/cloud/naming/ResourceNamingPolicyManager.java ---
    @@ -0,0 +1,32 @@
    +// 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 com.cloud.naming;
    +
    +import com.cloud.utils.component.Manager;
    +
    +public interface ResourceNamingPolicyManager extends Manager {
    +
    +
    +    /**
    +     * Finds a resource naming policy among the registered ones by its class
    +     * @param policyClass
    +     * @return
    +     */
    +    @SuppressWarnings("rawtypes")
    +    public <T extends ResourceNamingPolicy> T getPolicy(Class<T> policyClass);
    --- End diff --
    
    Let’s check if I understood this method.
    It will look up for a naming policy that is registered given a naming policy class, right?
    
    The registered policy is an instance of that class that is sent as a parameter, right? If I already know the class I need, why look for an instance of it?
    
    What about creating singletons those classes using spring? And then, just injecting them where needed. Sorry if overlooked something, but have you tried to design some solution that uses IoC instead of creating our own?



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by DaanHoogland <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by DaanHoogland <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1492#discussion_r59860389
  
    --- Diff: api/src/com/cloud/naming/ResourceNamingPolicyManager.java ---
    @@ -0,0 +1,32 @@
    +// 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 com.cloud.naming;
    +
    +import com.cloud.utils.component.Manager;
    +
    +public interface ResourceNamingPolicyManager extends Manager {
    +
    +
    +    /**
    +     * Finds a resource naming policy among the registered ones by its class
    +     * @param policyClass
    +     * @return
    +     */
    +    @SuppressWarnings("rawtypes")
    +    public <T extends ResourceNamingPolicy> T getPolicy(Class<T> policyClass);
    --- End diff --
    
    At the end, to me it seems that what you create here is a way to create and manage our own singletons with our own solution, right?
    
    Please do not take me in a bad way, but I really believe we should avoid this kind of solution if can use a simpler solution that is the base of the framework we already use. To me, this looks similar to what has been done to the “JPA” and “Webservice/Rest API” of ACS.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by ProjectMoon <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1492: [CLOUDSTACK-9003] Resource Naming Policies

Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the issue:

    https://github.com/apache/cloudstack/pull/1492
  
    Rebased to latest master.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1492#discussion_r59861537
  
    --- Diff: api/src/com/cloud/naming/ResourceNamingPolicyManager.java ---
    @@ -0,0 +1,32 @@
    +// 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 com.cloud.naming;
    +
    +import com.cloud.utils.component.Manager;
    +
    +public interface ResourceNamingPolicyManager extends Manager {
    +
    +
    +    /**
    +     * Finds a resource naming policy among the registered ones by its class
    +     * @param policyClass
    +     * @return
    +     */
    +    @SuppressWarnings("rawtypes")
    +    public <T extends ResourceNamingPolicy> T getPolicy(Class<T> policyClass);
    --- End diff --
    
    That is great we seem to be on the same page ;)
    Sorry for just going through this PR now that you have already done a lot of work.
    
    I think instead of using the “getRegistered” to retrieve a bean across modules, we could take advantage of the application context hierarchic that CloudStack already has.
    
    To use it as Spring singletons and to plan the changes, we would need to know in which modules those objects will be needed. CloudStack already has a module hierarchic, so it would be a matter mapping the modules that will need those objects and them finding a nice spot to put those objects. For that we might even create another module (not sure yet). First things first, do you have a mapping of the modules/objects that need those policies? Something visual would be great :)


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

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

Posted by DaanHoogland <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by DaanHoogland <gi...@git.apache.org>.
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-<counter-value> and then checks if the next created resource has the
    right name for each newly created resource; bla-<counter value minus one>


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by Daan Hoogland <da...@gmail.com>.
On Thu, Apr 14, 2016 at 4:14 PM, ProjectMoon <gi...@git.apache.org> 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-<counter-value> and then checks if the next created resource has the
right name for each newly created resource; bla-<present-counter minus
one>?​


>
>
> ---
> 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 infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
>



-- 
Daan

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

Posted by ProjectMoon <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1492#discussion_r59859824
  
    --- Diff: api/src/com/cloud/naming/ResourceNamingPolicyManager.java ---
    @@ -0,0 +1,32 @@
    +// 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 com.cloud.naming;
    +
    +import com.cloud.utils.component.Manager;
    +
    +public interface ResourceNamingPolicyManager extends Manager {
    +
    +
    +    /**
    +     * Finds a resource naming policy among the registered ones by its class
    +     * @param policyClass
    +     * @return
    +     */
    +    @SuppressWarnings("rawtypes")
    +    public <T extends ResourceNamingPolicy> T getPolicy(Class<T> policyClass);
    --- End diff --
    
    Using singletons is one way, but the modules code is tied quite tightly to the whole CloudStack registry thing as far as I know. The lifecycle registry is why we have this class.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1492: [CLOUDSTACK-9003] Resource Naming Policies

Posted by ProjectMoon <gi...@git.apache.org>.
GitHub user ProjectMoon reopened a pull request:

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

    [CLOUDSTACK-9003] Resource Naming Policies

    Pull request #988, completed and reinvented.
    
    This is an initial submission, a request for commentary.
    
    This started with attempting to make the VirtualMachineName class non-static, and turned into a complete overhaul of how resources in CloudStack are named. Each supported resource has a naming policy that can be overridden to change the way UUIDs and names are generated for them. The primary use case for this is so CloudStack can emulate the naming schemes of other clouds. The default naming policies are in cloud-plugin-naming-policies, cloud-server has the server-naming module which contains the registry lifecycle, and the registered policies are injected into the core managers context.
    
    The supported resources are:
    
    * User virtual machines
    * All kinds of system VMs
    * Security groups
    * Volumes
    * Snapshots
    
    There is one issue we have before we consider this ready for submission. The server-naming module has an ordering dependency. It must be loaded first in cloud-server in order for everything to work. Currently in this PR is a class called `ModuleLoadingComparator` which adds a comparator that forces the server-naming module to the top of the list when loading modules. We obviously want to get rid of this and are looking for suggestions on how to do this.

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

    $ git pull https://github.com/greenqloud/cloudstack pr-naming-policies

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

    https://github.com/apache/cloudstack/pull/1492.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 #1492
    
----
commit 6be87da0d98400ecb7254c200cf9fe7821181e4c
Author: nnesic <ne...@greenqloud.com>
Date:   2015-12-28T10:52:54Z

    Implement the naming policy feature.
    
    The naming policies feature is a large refactoring of how CloudStack
    generates names and UUIDs for the following resources: virtual
    machines (user instances and system VMs), security groups, templates,
    volumes, and snapshots. Previously, most of these resources were
    generated in a non-uniform way, making use of static classes or simple
    UUID generation.
    
    With the naming policies feature, each type of supported resource has
    a specific naming policy to go along with it that generates UUIDs and
    resource names. This allows CloudStack to emulate different kinds of
    naming schemes (such as those found in other clouds).

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1492: [CLOUDSTACK-9003] Resource Naming Policies

Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon closed the pull request at:

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


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1492#discussion_r59859123
  
    --- Diff: api/src/com/cloud/naming/ResourceNamingPolicy.java ---
    @@ -0,0 +1,36 @@
    +// 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 com.cloud.naming;
    +
    +import org.apache.cloudstack.api.Identity;
    +
    +public interface ResourceNamingPolicy <V extends Identity, VO extends V> {
    --- End diff --
    
    Why are you using V extends Identity, VO extends V?
    Wouldn’t V extends Identity sufficient? I am only seeing you using the "VO" card and not the "V"


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by jburwell <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1492: [CLOUDSTACK-9003] Resource Naming Policies

Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the issue:

    https://github.com/apache/cloudstack/pull/1492
  
    Travis is failing due to some connection error, it seems. Any ideas?
    
    ```
    [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/DependencyResolutionException
    nc: connect to localhost port 8096 (tcp) failed: Connection refused
    nc: connect to localhost port 8096 (tcp) failed: Connection refused
    nc: connect to localhost port 8096 (tcp) failed: Network is unreachable
    ````


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by bhaisaab <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by jburwell <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by DaanHoogland <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

Posted by nnesic <gi...@git.apache.org>.
Github user nnesic commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1492#discussion_r61122746
  
    --- Diff: api/src/com/cloud/naming/ResourceNamingPolicyManager.java ---
    @@ -0,0 +1,32 @@
    +// 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 com.cloud.naming;
    +
    +import com.cloud.utils.component.Manager;
    +
    +public interface ResourceNamingPolicyManager extends Manager {
    +
    +
    +    /**
    +     * Finds a resource naming policy among the registered ones by its class
    +     * @param policyClass
    +     * @return
    +     */
    +    @SuppressWarnings("rawtypes")
    +    public <T extends ResourceNamingPolicy> T getPolicy(Class<T> policyClass);
    --- End diff --
    
    Hi, here's the mapping of modules using naming policies.
    
    ```
    Modules dependent on ResourceNamingPolicyManager:
    
    bootstrap
        system
            core
                server-naming
                compute
                    network
                        contrail
                        elb
                    storage
                        baremetal-storage
                        server-template-adapter
                discoverer
                    baremetal-discoverer
                    hyperv-discoverer
                    ovm-discoverer
                    ovm3-discoverer
                    secondary-storage-discoverer
                    server-discoverer
                    vmware-discoverer
                    xenserver-discoverer
    
    Classes relying on ResourceNamingPolicyManager:
    core:
        InternalLoadBalancerVMManagerImpl
        VolumeOrchestrator
        VirtualMachineManagerImpl
        ClusteredVirtualMachineManagerImpl
        SecondaryStorageManagerImpl
        PremiumSecondaryStorageManagerImpl
        ConsoleProxyManagerImpl
        VolumeApiServiceImpl
        SnapshotManagerImpl
        TemplateManagerImpl
        NetworkHelperImpl
        VpcNetworkHelperImpl
        SecurityGroupManagerImpl
        SecurityGroupManagerImpl2
        UserVmManagerImpl
    
    contrail:
        ServiceManagerImpl
    
    elb:
        ElasticLoadBalancerManagerImpl
        LoadBalanceRuleHandler
    
    baremetal-storage:
        BareMetalTemplateAdapter
    
    server-template-adapter:
        HypervisorTemplateAdapter
    
    baremetal-discoverer:
        BaremetalDiscoverer
    
    hyperv-discoverer:
        HypervServerDiscoverer
    
    server-discoverer:
        KvmServerDiscoverer
        LxcServerDiscoverer
    
    ovm3-discoverer:
        Ovm3Discoverer
    
    ovm-discoverer:
        OvmDiscoverer
    
    simulator-discoverer:
        SimulatorDiscoverer
    
    xenserver-discoverer:
        XcpServerDiscoverer
    
    vmware-discoverer:
        VmwareServerDiscoverer
    ```


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1492: [CLOUDSTACK-9003] Resource Naming Policies

Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:

    https://github.com/apache/cloudstack/pull/1492
  
    @ProjectMoon can you close/open the PR to re-kick Travis


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---