You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by nitin-maharana <gi...@git.apache.org> on 2016/11/23 11:16:55 UTC

[GitHub] cloudstack pull request #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN rang...

GitHub user nitin-maharana opened a pull request:

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

    CLOUDSTACK-9611: Dedicating a Guest VLAN range to Project does not work.

    Description:
    =========
    
    Trying to dedicate a guest VLAN range to an account fails. Either API documentation is wrong or code path needs to be corrected. If we pass both account and projectId parameters to the dedicateGuestVlanRange (which are not mentioned as mutually exclusive in API description) the API layer throws error saying both are mutually exclusive.
    
    Steps to Reproduce:
    ===============
    Create an account. Create a project in that account.
    
    Go to Admin account and change the view to the above project.
    
    Navigate to Infrastructure -> Zone -> Physical Network -> Guest -> Dedicate Guest VLAN range.
    
    Try to dedicate the guest VLAN range from the project view for the account associated with the project.
    
    It fails with error saying "accountName and projectId are mutually exclusive".
    
    ![image](https://cloud.githubusercontent.com/assets/12583725/20559763/15fe55d8-b19c-11e6-9996-62602d269f10.png)
    
    ![image](https://cloud.githubusercontent.com/assets/12583725/20559765/1957af36-b19c-11e6-938f-e9c7c9eee987.png)
    
    Expected:
    =======
    The VLAN range should get dedicated to the project account.
    
    ![image](https://cloud.githubusercontent.com/assets/12583725/20559787/33ef67b2-b19c-11e6-90fd-41c9c583efd9.png)
    
    ![image](https://cloud.githubusercontent.com/assets/12583725/20559789/376cae22-b19c-11e6-9755-ff1e7237fb21.png)
    
    Resolution:
    ========
    Changed the account field of API to optional.
    Even if the account name is passed while adding from a particular project view, then it is ignored in API.
    
    Notes:
    =====
    If we do the dedication from default view then it works fine as no projectId is associated over there.

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

    $ git pull https://github.com/nitin-maharana/CloudStack-Nitin nitin3

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

    https://github.com/apache/cloudstack/pull/1771.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 #1771
    
----
commit ef85dfd041858aeaee9aecfdc6bddabe1e21739d
Author: Nitin Kumar Maharana <ni...@accelerite.com>
Date:   2016-11-23T11:07:20Z

    CLOUDSTACK-9611: Dedicating a Guest VLAN range to Project does not work
    
    Changed the account field of API to optional.
    Even if the account name is passed while adding from a particular project view, then it is ignored in 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    @nitin-maharana cannot we dedicate guest vlan range to projects in other domains ?


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    Gone through the patch. LGTM.


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

[GitHub] cloudstack issue #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    @ustcweizhou : Please verify, I have changed according to your suggestion.


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    Packaging result: \u2716centos6 \u2714centos7 \u2714debian. JID-375


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    @ustcweizhou : You are correct. We have to remove the domain scope. If we support the domain scope, we need more changes on the logic. In that case we can create a new ticket for 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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN rang...

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

    https://github.com/apache/cloudstack/pull/1771#discussion_r101486770
  
    --- Diff: server/src/com/cloud/network/NetworkServiceImpl.java ---
    @@ -3085,9 +3085,10 @@ public GuestVlan dedicateGuestVlanRange(DedicateGuestVlanRangeCmd cmd) {
             // Verify account is valid
             Account vlanOwner = null;
             if (projectId != null) {
    -            if (accountName != null) {
    -                throw new InvalidParameterValueException("accountName and projectId are mutually exclusive");
    -            }
    +            //accountName and projectId are mutually exclusive
    --- End diff --
    
    @nitin-maharana So as I understand in order to avoid making a breaking API change you are simply ignoring account and giving priority to project if both are specified.


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    @blueorangutan package


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    @rhtyd : Changed PR's base branch to 4.9


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN rang...

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

    https://github.com/apache/cloudstack/pull/1771#discussion_r104902049
  
    --- Diff: ui/scripts/system.js ---
    @@ -2090,27 +2090,57 @@
                                                     fields: {
                                                         vlanrange: {
                                                             label: 'label.vlan.vni.range',
    -                                                        /*  select: function(args) {
    -                                                        var items = [];
    -                                                        if(args.context.physicalNetworks[0].vlan != null && args.context.physicalNetworks[0].vlan.length > 0) {
    -                                                        var vlanranges = args.context.physicalNetworks[0].vlan.split(";");
    -                                                        for(var i = 0; i < vlanranges.length ; i++) {
    -                                                        items.push({id: vlanranges[i], description: vlanranges[i]});
    -                                                        }
    -                                                        }
    -                                                        args.response.success({data: items});
    -                                                        },*/
                                                             validation: {
                                                                 required: true
                                                             }
                                                         },
    -                                                    account: {
    -                                                        label: 'label.account',
    -                                                        validation: {
    -                                                            required: true
    +                                                    scope: {
    +                                                        label: 'label.scope',
    +                                                        docID: 'helpGuestNetworkZoneScope',
    +                                                        select: function(args) {
    +                                                            var array1 = [];
    +
    +                                                            array1.push({
    +                                                                id: 'account-specific',
    +                                                                description: 'label.account'
    +                                                            });
    +                                                            array1.push({
    +                                                                id: 'project-specific',
    +                                                                description: 'label.project'
    +                                                            });
    +
    +                                                            args.response.success({
    +                                                                data: array1
    +                                                            });
    +
    +                                                            args.$select.change(function() {
    +                                                                var $form = $(this).closest('form');
    +
    +                                                                if ($(this).val() == "account-specific") {
    +                                                                    $form.find('.form-item[rel=domainId]').css('display', 'inline-block');
    +                                                                    $form.find('.form-item[rel=account]').css('display', 'inline-block');
    +                                                                    $form.find('.form-item[rel=projectId]').hide();
    +                                                                } else if ($(this).val() == "project-specific") {
    +                                                                    $form.find('.form-item[rel=domainId]').css('display', 'inline-block');
    +                                                                    $form.find('.form-item[rel=account]').hide();
    +                                                                    $form.find('.form-item[rel=projectId]').css('display', 'inline-block');
    +                                                                }
    +
    +                                                                if (args.context.projects != null && args.context.projects.length > 0) {
    +                                                                    $form.find('.form-item[rel=domainId]').hide();
    +                                                                    $form.find('.form-item[rel=account]').hide();
    +                                                                    $form.find('.form-item[rel=projectId]').hide();
    +                                                                }
    --- End diff --
    
    @ustcweizhou : The above code handles my previous explanation.


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    @ustcweizhou : Passing project with domain won't create any issue but not passing any account details when we call the API from default view creates an issue. Because the internal logic expects account owner of VLAN when dedicated from the default view. In the case of project view, it gets the account detail from selected project.


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    @nitin-maharana I think it is better to list projects by domain.
    Just an example, if there are 3 projects with same name created in 3 domains, it will be difficult to choose.


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    @nitin-maharana should projectId dependsOn domainId ? If the domain changes, the project list should change according to the new domainid.


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    @nitin-maharana yes, you are right. I forgot it. 
    LGTM


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

[GitHub] cloudstack issue #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    ### ACS CI BVT Run
     **Sumarry:**
     Build Number 468
     Hypervisor xenserver
     NetworkType Advanced
     Passed=105
     Failed=0
     Skipped=7
    
    _Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
    
    
    **Failed tests:**
    
    **Skipped tests:**
    test_01_test_vm_volume_snapshot
    test_vm_nic_adapter_vmxnet3
    test_static_role_account_acls
    test_11_ss_nfs_version_on_ssvm
    test_nested_virtualization_vmware
    test_3d_gpu_support
    test_deploy_vgpu_enabled_vm
    
    **Passed test suits:**
    test_deploy_vm_with_userdata.py
    test_affinity_groups_projects.py
    test_portable_publicip.py
    test_over_provisioning.py
    test_global_settings.py
    test_scale_vm.py
    test_service_offerings.py
    test_routers_iptables_default_policy.py
    test_loadbalance.py
    test_routers.py
    test_reset_vm_on_reboot.py
    test_deploy_vms_with_varied_deploymentplanners.py
    test_network.py
    test_router_dns.py
    test_non_contigiousvlan.py
    test_login.py
    test_deploy_vm_iso.py
    test_list_ids_parameter.py
    test_public_ip_range.py
    test_multipleips_per_nic.py
    test_regions.py
    test_affinity_groups.py
    test_network_acl.py
    test_pvlan.py
    test_volumes.py
    test_nic.py
    test_deploy_vm_root_resize.py
    test_resource_detail.py
    test_secondary_storage.py
    test_vm_life_cycle.py
    test_routers_network_ops.py
    test_disk_offerings.py


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

[GitHub] cloudstack issue #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    @ustcweizhou : The createProject API allows to created a project under a domain but no idea if there is any support from UI. Whenever I create a project, it always goes to ROOT domain.


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN rang...

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

    https://github.com/apache/cloudstack/pull/1771#discussion_r101491925
  
    --- Diff: server/src/com/cloud/network/NetworkServiceImpl.java ---
    @@ -3085,9 +3085,10 @@ public GuestVlan dedicateGuestVlanRange(DedicateGuestVlanRangeCmd cmd) {
             // Verify account is valid
             Account vlanOwner = null;
             if (projectId != null) {
    -            if (accountName != null) {
    -                throw new InvalidParameterValueException("accountName and projectId are mutually exclusive");
    -            }
    +            //accountName and projectId are mutually exclusive
    --- End diff --
    
    @koushik-das : Yes, We give priority to the project, if not specified then checking the account. 


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN rang...

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

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


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    ping @rhtyd @karuturi 


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    ping @karuturi @sateesh-chodapuneedi 


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    @ustcweizhou @koushik-das : I implemented the way @ustcweizhou's suggested. It was purely an UI change. But I found an issue while testing.
    
    On API side, the entire logic uses VLAN owner account details, they find it from the account given by the user. So, they made account field as mandatory. But that contradicts when it is accessed from a project view(This issue). 
    
    The current logic works in a default view because we don't add projectid details while accessing the API. But when it is accessed from a particular project view, by default the projectid parameter is added with the API call, So this issue arises. 
    
    Now, if we go with the change suggested by @ustcweizhou, once we select the scope as domain, we don't pass any account details, so the logic can't able to find the owner hence gives an error.
    
    According to my suggestion, On API side, we should make the account field as optional and on UI front, we should make it mandatory and the account field will appear only in case of default view, which will fix the issue. Please give your input on this. Accordingly, I will proceed.
    
    Please see the current change(I pushed it here)


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

[GitHub] cloudstack issue #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    @nitin-maharana 
    if this is performed in project view, should projectid be passed ?
    projectId: args.context.projects[0].id


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN rang...

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

    https://github.com/apache/cloudstack/pull/1771#discussion_r101451791
  
    --- Diff: server/src/com/cloud/network/NetworkServiceImpl.java ---
    @@ -3085,9 +3085,10 @@ public GuestVlan dedicateGuestVlanRange(DedicateGuestVlanRangeCmd cmd) {
             // Verify account is valid
             Account vlanOwner = null;
             if (projectId != null) {
    -            if (accountName != null) {
    -                throw new InvalidParameterValueException("accountName and projectId are mutually exclusive");
    -            }
    +            //accountName and projectId are mutually exclusive
    --- End diff --
    
    @nitin-maharana If account and project are mutually exclusive why have you removed the validation?


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN rang...

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

    https://github.com/apache/cloudstack/pull/1771#discussion_r102173991
  
    --- Diff: server/src/com/cloud/network/NetworkServiceImpl.java ---
    @@ -3085,9 +3085,10 @@ public GuestVlan dedicateGuestVlanRange(DedicateGuestVlanRangeCmd cmd) {
             // Verify account is valid
             Account vlanOwner = null;
             if (projectId != null) {
    -            if (accountName != null) {
    -                throw new InvalidParameterValueException("accountName and projectId are mutually exclusive");
    -            }
    +            //accountName and projectId are mutually exclusive
    --- End diff --
    
    @koushik-das @rajesh-battala : Do you agree with @ustcweizhou suggestion?


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    @ustcweizhou : Yes, if domain changes, the project list should change. 
    As per my understanding, a project is similar to an account. When we create a project, by default it is always created in ROOT domain. But an account can be created in a specific domain. I am still not clear what is the need of project.
    
    As all the projects are under ROOT domain, if we select any domain other than ROOT, the project list would be empty. So I think we needn't use the domain list in project scope and instead we should list all projects.


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    @ustcweizhou : In case of project view, the projectId is always passed. Also we consider it as project scope but we won't have any option to choose projects from a list because we are already under a project.


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    tag:mergeready


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    @nitin-maharana I thought dedicating guest vlan range is already implemented in CLOUDSTACK-8958.
    Actually they are two stories.
    In this case, we have to remove the Domain scope from UI as it is not supported, unless you make more changes to support it (similar to 37301ed4540450c29be4f975d58b38dbeec6c296 for CLOUDSTACK-8958).


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    @nitin-maharana you may add "ignoreProject: true" in createURL('dedicateGuestVlanRange') if scope is set to Domain


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN range to Pr...

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

    https://github.com/apache/cloudstack/pull/1771
  
    @ustcweizhou : I have modified the code, can you review it one more time. Please see the below snapshot.
    
    <img width="522" alt="screen shot 2017-03-01 at 3 48 10 pm" src="https://cloud.githubusercontent.com/assets/12583725/23455743/b4d7be00-fe96-11e6-8a41-398c41f6d019.png">


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN rang...

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

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

    CLOUDSTACK-9611: Dedicating a Guest VLAN range to Project does not work.

    Description:
    =========
    
    Trying to dedicate a guest VLAN range to an account fails. Either API documentation is wrong or code path needs to be corrected. If we pass both account and projectId parameters to the dedicateGuestVlanRange (which are not mentioned as mutually exclusive in API description) the API layer throws error saying both are mutually exclusive.
    
    Steps to Reproduce:
    ===============
    Create an account. Create a project in that account.
    
    Go to Admin account and change the view to the above project.
    
    Navigate to Infrastructure -> Zone -> Physical Network -> Guest -> Dedicate Guest VLAN range.
    
    Try to dedicate the guest VLAN range from the project view for the account associated with the project.
    
    It fails with error saying "accountName and projectId are mutually exclusive".
    
    ![image](https://cloud.githubusercontent.com/assets/12583725/20559763/15fe55d8-b19c-11e6-9996-62602d269f10.png)
    
    ![image](https://cloud.githubusercontent.com/assets/12583725/20559765/1957af36-b19c-11e6-938f-e9c7c9eee987.png)
    
    Expected:
    =======
    The VLAN range should get dedicated to the project account.
    
    ![image](https://cloud.githubusercontent.com/assets/12583725/20559787/33ef67b2-b19c-11e6-90fd-41c9c583efd9.png)
    
    ![image](https://cloud.githubusercontent.com/assets/12583725/20559789/376cae22-b19c-11e6-9755-ff1e7237fb21.png)
    
    Resolution:
    ========
    Changed the account field of API to optional.
    Even if the account name is passed while adding from a particular project view, then it is ignored in API.
    
    Notes:
    =====
    If we do the dedication from default view then it works fine as no projectId is associated over there.

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

    $ git pull https://github.com/nitin-maharana/CloudStack-Nitin nitin3

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

    https://github.com/apache/cloudstack/pull/1771.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 #1771
    
----
commit f56961392662f4b45e3e4cb989203f918ec19916
Author: Nitin Kumar Maharana <ni...@accelerite.com>
Date:   2017-02-28T10:57:38Z

    CLOUDSTACK-9611: Dedicating a Guest VLAN range to Project does not work.

----


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

[GitHub] cloudstack pull request #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN rang...

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

    https://github.com/apache/cloudstack/pull/1771#discussion_r102445777
  
    --- Diff: server/src/com/cloud/network/NetworkServiceImpl.java ---
    @@ -3085,9 +3085,10 @@ public GuestVlan dedicateGuestVlanRange(DedicateGuestVlanRangeCmd cmd) {
             // Verify account is valid
             Account vlanOwner = null;
             if (projectId != null) {
    -            if (accountName != null) {
    -                throw new InvalidParameterValueException("accountName and projectId are mutually exclusive");
    -            }
    +            //accountName and projectId are mutually exclusive
    --- End diff --
    
    @nitin-maharana @ustcweizhou The scope field in UI should be fine. But since there is a breaking change (accountName is made optional from required), there may be scenarios which will break. As long as it is documented should be ok.


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN rang...

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

    https://github.com/apache/cloudstack/pull/1771#discussion_r102453548
  
    --- Diff: server/src/com/cloud/network/NetworkServiceImpl.java ---
    @@ -3085,9 +3085,10 @@ public GuestVlan dedicateGuestVlanRange(DedicateGuestVlanRangeCmd cmd) {
             // Verify account is valid
             Account vlanOwner = null;
             if (projectId != null) {
    -            if (accountName != null) {
    -                throw new InvalidParameterValueException("accountName and projectId are mutually exclusive");
    -            }
    +            //accountName and projectId are mutually exclusive
    --- End diff --
    
    @koushik-das : If we go with the scope field, we have to revert that change and have to make required depending on the scope we select. Like shown in the below snapshot.
    
    <img width="420" alt="screen shot 2017-02-22 at 6 14 19 pm" src="https://cloud.githubusercontent.com/assets/12583725/23212092/1ba1cd22-f92b-11e6-83ba-72dab04dc497.png">



---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN rang...

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

    https://github.com/apache/cloudstack/pull/1771#discussion_r101470562
  
    --- Diff: server/src/com/cloud/network/NetworkServiceImpl.java ---
    @@ -3085,9 +3085,10 @@ public GuestVlan dedicateGuestVlanRange(DedicateGuestVlanRangeCmd cmd) {
             // Verify account is valid
             Account vlanOwner = null;
             if (projectId != null) {
    -            if (accountName != null) {
    -                throw new InvalidParameterValueException("accountName and projectId are mutually exclusive");
    -            }
    +            //accountName and projectId are mutually exclusive
    --- End diff --
    
    @koushik-das : As I mentioned in resolution part, Even if the account name is passed while adding from a particular project view, then it is ignored(Removal part) in API. If we don't remove that validation check, then the same issue will happen again. 


---
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 #1771: CLOUDSTACK-9611: Dedicating a Guest VLAN rang...

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

    https://github.com/apache/cloudstack/pull/1771#discussion_r101618476
  
    --- Diff: server/src/com/cloud/network/NetworkServiceImpl.java ---
    @@ -3085,9 +3085,10 @@ public GuestVlan dedicateGuestVlanRange(DedicateGuestVlanRangeCmd cmd) {
             // Verify account is valid
             Account vlanOwner = null;
             if (projectId != null) {
    -            if (accountName != null) {
    -                throw new InvalidParameterValueException("accountName and projectId are mutually exclusive");
    -            }
    +            //accountName and projectId are mutually exclusive
    --- End diff --
    
    @nitin-maharana I disagree this change. We'd better keep like it.
    I suggest to make ui change to add a new field 'scope' which can be set to 'domain', 'account', 'project'.
    It is similar to the scope field in the dialog to create a shared network in advanced zone.
    
    scope='Domain', domain field is shown.
    scope='Account, domain and account are shown
    scope='Project', domain and project are shown.
    



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