You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by sudhansu7 <gi...@git.apache.org> on 2016/11/29 09:10:48 UTC

[GitHub] cloudstack pull request #1797: CLOUDSTACK-9630: Cannot use listNics API as a...

GitHub user sudhansu7 opened a pull request:

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

    CLOUDSTACK-9630: Cannot use listNics API as advertised

    
    
    listNics API for a VM, "type" was not returned within API response.
    
    EXPECTED BEHAVIOR
    ==================
    The listNics API response return type of NIC (type), as specified in https://cloudstack.apache.org/api/apidocs-4.8/user/listNics.html
    
    ACTUAL BEHAVIOR
    ==================
    The listNics API response does not return type of NIC.
    
    (local) \U0001f435 > list nics virtualmachineid=a69edaf5-8f21-41ff-8c05-263dc4bd5354
    count = 1
    nic:
    id = 211e0d46-6b94-4425-99f7-e8e9efea2472
    deviceid = 0
    gateway = 10.1.1.1
    ipaddress = 10.1.1.45
    isdefault = True
    macaddress = 02:00:06:f6:00:01
    netmask = 255.255.255.0
    networkid = c08fddf1-fd77-4810-a062-ea9d03c5c7e6
    virtualmachineid = a69edaf5-8f21-41ff-8c05-263dc4bd5354
    
    Solution:
    added "type" details for listNics API response.
    
    After Fix:
    
    (local) \U0001f435 > list nics virtualmachineid=a69edaf5-8f21-41ff-8c05-263dc4bd5354 
    count = 1
    nic:
    id = 211e0d46-6b94-4425-99f7-e8e9efea2472
    broadcasturi = vlan://743
    deviceid = 0
    gateway = 10.1.1.1
    ipaddress = 10.1.1.45
    isdefault = True
    isolationuri = vlan://743
    macaddress = 02:00:06:f6:00:01
    netmask = 255.255.255.0
    networkid = c08fddf1-fd77-4810-a062-ea9d03c5c7e6
    traffictype = Guest
    **type = Isolated**
    virtualmachineid = a69edaf5-8f21-41ff-8c05-263dc4bd5354

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

    $ git pull https://github.com/sudhansu7/cloudstack CLOUDSTACK-9630

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

    https://github.com/apache/cloudstack/pull/1797.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 #1797
    
----
commit f5e6136265580af069879bcb5b9cb86646f79ea9
Author: Sudhansu <su...@accelerite.com>
Date:   2016-11-29T08:44:24Z

    CLOUDSTACK-9630: Cannot use listNics API as advertised
    
    added "type" details for listNics API response.

----


---
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 #1797: CLOUDSTACK-9630: Cannot use listNics API as advertis...

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

    https://github.com/apache/cloudstack/pull/1797
  
    @pdion891 removed broadcasturi from api response. Also added marvin 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1797: CLOUDSTACK-9630: Cannot use listNics API as a...

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

    https://github.com/apache/cloudstack/pull/1797#discussion_r90009164
  
    --- Diff: server/src/com/cloud/api/ApiResponseHelper.java ---
    @@ -3461,10 +3470,21 @@ public NicResponse createNicResponse(Nic result) {
             response.setNetmask(result.getIPv4Netmask());
             response.setMacAddress(result.getMacAddress());
     
    +        if (result.getBroadcastUri() != null) {
    +            response.setBroadcastUri(result.getBroadcastUri().toString());
    --- End diff --
    
    @ustcweizhou As per api doc these info should be part of response. Even listVirtualMachines with details=nics 
    `command=listVirtualMachines&details=nics&id=6eea5ceb-e943-4ef3-9f40-4671364d03ab&response=json` provides these details.



---
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 #1797: CLOUDSTACK-9630: Cannot use listNics API as advertis...

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

    https://github.com/apache/cloudstack/pull/1797
  
    @sudhansu7 could you please either add or update an existing a Marvin test case to verify this change?
    
    Also, this change seems like it would be useful for LTS users.  Could you please change the 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 #1797: CLOUDSTACK-9630: Cannot use listNics API as a...

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

    https://github.com/apache/cloudstack/pull/1797#discussion_r89973281
  
    --- Diff: server/src/com/cloud/api/ApiResponseHelper.java ---
    @@ -3461,10 +3470,21 @@ public NicResponse createNicResponse(Nic result) {
             response.setNetmask(result.getIPv4Netmask());
             response.setMacAddress(result.getMacAddress());
     
    +        if (result.getBroadcastUri() != null) {
    +            response.setBroadcastUri(result.getBroadcastUri().toString());
    --- End diff --
    
    should these two only be visible for root admin?


---
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 #1797: CLOUDSTACK-9630: Cannot use listNics API as advertis...

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

    https://github.com/apache/cloudstack/pull/1797
  
    ### ACS CI BVT Run
     **Sumarry:**
     Build Number 413
     Hypervisor xenserver
     NetworkType Advanced
     Passed=104
     Failed=1
     Skipped=7
    
    _Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
    
    
    **Failed tests:**
    * test_routers_network_ops.py
    
     * test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true Failed
    
    
    **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_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 #1797: CLOUDSTACK-9630: Cannot use listNics API as advertis...

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

    https://github.com/apache/cloudstack/pull/1797
  
    Have you tested as @jburwell comments, `broadcasturi` cannot be part of the response for user, only for root-admin. Does this PR make `broadcasturi` visible to users?


---
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 #1797: CLOUDSTACK-9630: Cannot use listNics API as advertis...

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

    https://github.com/apache/cloudstack/pull/1797
  
    as it been confirmed the the attribute `broadcasturi = vlan://743` is only visible as admin and not as user? because the example have 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 #1797: CLOUDSTACK-9630: Cannot use listNics API as advertis...

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

    https://github.com/apache/cloudstack/pull/1797
  
    LGTM on the code changes. Make sense to keep listVirtualMachines and listNics in sync for nic response. 


---
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 #1797: CLOUDSTACK-9630: Cannot use listNics API as advertis...

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

    https://github.com/apache/cloudstack/pull/1797
  
    @jburwell  I have modified the base branch to 4.9 . Also added a marvin test to check nic details. 


---
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 #1797: CLOUDSTACK-9630: Cannot use listNics API as advertis...

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

    https://github.com/apache/cloudstack/pull/1797
  
    Test LGTM based on manual testing of the fix:
    
    **Before Fix:**
    <nic>
    <id>d82b2278-ca19-46da-b532-0a044e778bb8</id>
    <networkid>71424164-015c-4163-9dde-74019fb22ce2</networkid>
    <netmask>255.255.255.0</netmask>
    <gateway>10.1.1.1</gateway>
    <ipaddress>10.1.1.119</ipaddress>
    <traffictype>Guest</traffictype>
    <isdefault>true</isdefault>
    <macaddress>02:00:1c:fe:00:01</macaddress>
    <deviceid>0</deviceid>
    <virtualmachineid>71a0be44-69e4-4821-b8d9-e579ed04d52a</virtualmachineid>
    </nic>
    
    **After Fix:**
    <nic>
    <id>d82b2278-ca19-46da-b532-0a044e778bb8</id>
    <networkid>71424164-015c-4163-9dde-74019fb22ce2</networkid>
    <netmask>255.255.255.0</netmask>
    <gateway>10.1.1.1</gateway>
    <ipaddress>10.1.1.119</ipaddress>
    <traffictype>Guest</traffictype>
    **<type>Isolated</type>**
    <isdefault>true</isdefault>
    <macaddress>02:00:1c:fe:00:01</macaddress>
    <deviceid>0</deviceid>
    <virtualmachineid>71a0be44-69e4-4821-b8d9-e579ed04d52a</virtualmachineid>
    </nic>


---
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 #1797: CLOUDSTACK-9630: Cannot use listNics API as advertis...

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

    https://github.com/apache/cloudstack/pull/1797
  
    @pdion891 
    The first listNic example is incorrect code. Which has been fixed in this PR.  You can use listVirtualMachine command to see the nic response. 
    
    As per code there is no restriction on broadcasturi. If broadcasturi is not null then it will part of vm response. 
    So the ListNic should also behave the same way.
    
    '''
    if (details.contains(VMDetails.all) || details.contains(VMDetails.nics)) {
                long nic_id = userVm.getNicId();
                if (nic_id > 0) {
                    NicResponse nicResponse = new NicResponse();
                    nicResponse.setId(userVm.getNicUuid());
                    nicResponse.setIpaddress(userVm.getIpAddress());
                    nicResponse.setGateway(userVm.getGateway());
                    nicResponse.setNetmask(userVm.getNetmask());
                    nicResponse.setNetworkid(userVm.getNetworkUuid());
                    nicResponse.setNetworkName(userVm.getNetworkName());
                    nicResponse.setMacAddress(userVm.getMacAddress());
                    nicResponse.setIp6Address(userVm.getIp6Address());
                    nicResponse.setIp6Gateway(userVm.getIp6Gateway());
                    nicResponse.setIp6Cidr(userVm.getIp6Cidr());
                    if (userVm.getBroadcastUri() != null) {
                        nicResponse.setBroadcastUri(userVm.getBroadcastUri().toString());
                    }
                    if (userVm.getIsolationUri() != null) {
                        nicResponse.setIsolationUri(userVm.getIsolationUri().toString());
                    }
                    if (userVm.getTrafficType() != null) {
                        nicResponse.setTrafficType(userVm.getTrafficType().toString());
                    }
                    if (userVm.getGuestType() != null) {
                        nicResponse.setType(userVm.getGuestType().toString());
                    }
                    nicResponse.setIsDefault(userVm.isDefaultNic());
                    List<NicSecondaryIpVO> secondaryIps = ApiDBUtils.findNicSecondaryIps(userVm.getNicId());
                    if (secondaryIps != null) {
                        List<NicSecondaryIpResponse> ipList = new ArrayList<NicSecondaryIpResponse>();
                        for (NicSecondaryIpVO ip : secondaryIps) {
                            NicSecondaryIpResponse ipRes = new NicSecondaryIpResponse();
                            ipRes.setId(ip.getUuid());
                            ipRes.setIpAddr(ip.getIp4Address());
                            ipList.add(ipRes);
                        }
                        nicResponse.setSecondaryIps(ipList);
                    }
                    nicResponse.setObjectName("nic");
                    userVmResponse.addNic(nicResponse);
                }
            }
    '''
    Is there any 


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