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/01/13 10:51:54 UTC

[GitHub] cloudstack pull request: CLOUDSTACK-9228: Network update with mist...

GitHub user nitin-maharana opened a pull request:

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

    CLOUDSTACK-9228: Network update with mistmatch in services require forced option

    Steps to reproduce:
    ===============
    1.Bring up CloudStack in advanced zone 
    2.Create isolated network with sourcenat, pf, lb, firewall services
    3.Deploy a VM in the above network
    4.Create another network offering with sourcenat, pf, firewall services
    5.Try to update the network with offering created in step4
    
    Result:
    ======
    The new offering:DefaultIsolatedNetworkOfferingForVpcNetworksNoLB will remove the following services [Lb]along with all the related configuration currently in use. will not proceed with the network update.set forced parameter to true for forcing an update."
    
    Workaround:
    ===========
    Use api with forced=true
    
    Fix:
    ===
    Added a confirmation dialog box to check whether to make force update or not.
    The dialog appears only for the Admin. Only admin can make force update.
    The new dialog appears after the first CIDR unchanged confirmation dialog.

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

    $ git pull https://github.com/nitin-maharana/CloudStack CloudStack-Nitin16_4.7

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

    https://github.com/apache/cloudstack/pull/1333.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 #1333
    
----
commit 4b7fef6a68ee2684e3b8c8eb0035f7e36cb1be01
Author: Nitin Kumar Maharana <ni...@gmail.com>
Date:   2016-01-01T15:38:20Z

    CLOUDSTACK-9228: Network update with mistmatch in services require forced option
    
    Added a confirmation dialog box to check whether to make force update or not.
    The dialog appears only for the Admin. Only admin can make force update.
    The new dialog appears after the first CIDR unchanged confirmation dialog.

----


---
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-9228: Network update with mist...

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

    https://github.com/apache/cloudstack/pull/1333#issuecomment-174411698
  
    Hi @rafaelweingartner , I have updated the required change. Please have a look. Thank you.


---
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-9228: Network update with mist...

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

    https://github.com/apache/cloudstack/pull/1333#issuecomment-214306315
  
    @koushik-das : I will check it. Thanks.


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

[GitHub] cloudstack pull request: CLOUDSTACK-9228: Network update with mist...

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

    https://github.com/apache/cloudstack/pull/1333#issuecomment-175690879
  
    LGTM, has anyone tested this other than the author?


---
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-9228: Network update with mist...

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

    https://github.com/apache/cloudstack/pull/1333#issuecomment-174578490
  
    @rafaelweingartner : Yes, it will be forward merged to 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 #1333: CLOUDSTACK-9228: Network update with mistmatc...

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

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


---
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-9228: Network update with mist...

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

    https://github.com/apache/cloudstack/pull/1333#issuecomment-174176375
  
    Hi @nitin-maharana , 
    The lines from 970-1040 and lines from 1048-1118 are the same code. 
    Would you mind extracting them to a function? Then you could just call this function at lines 970 and 1048.
    
    You have to avoid these massively duplicated blocks.


---
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-9228: Network update with mist...

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

    https://github.com/apache/cloudstack/pull/1333#issuecomment-216220849
  
    tag:easypr


---
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-9228: Network update with mist...

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

    https://github.com/apache/cloudstack/pull/1333#issuecomment-201016129
  
    _Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
    
    ### ACS CI BVT Run
     **Sumarry:**
     Build Number 124
     Hypervisor xenserver
     NetworkType Advanced
     Passed=103
     Failed=3
     Skipped=4
    
    
    **Failed tests:**
    * integration.smoke.test_loadbalance.TestLoadBalance
    
     * test_01_create_lb_rule_src_nat Failed
    
     * test_02_create_lb_rule_non_nat Failed
    
     * test_assign_and_removal_lb Failed
    
    
    **Skipped tests:**
    test_vm_nic_adapter_vmxnet3
    test_deploy_vgpu_enabled_vm
    test_06_copy_template
    test_06_copy_iso
    
    **Passed test suits:**
    integration.smoke.test_deploy_vm_with_userdata.TestDeployVmWithUserData
    integration.smoke.test_affinity_groups_projects.TestDeployVmWithAffinityGroup
    integration.smoke.test_portable_publicip.TestPortablePublicIPAcquire
    integration.smoke.test_over_provisioning.TestUpdateOverProvision
    integration.smoke.test_global_settings.TestUpdateConfigWithScope
    integration.smoke.test_guest_vlan_range.TestDedicateGuestVlanRange
    integration.smoke.test_scale_vm.TestScaleVm
    integration.smoke.test_service_offerings.TestCreateServiceOffering
    integration.smoke.test_routers.TestRouterServices
    integration.smoke.test_reset_vm_on_reboot.TestResetVmOnReboot
    integration.smoke.test_snapshots.TestSnapshotRootDisk
    integration.smoke.test_deploy_vms_with_varied_deploymentplanners.TestDeployVmWithVariedPlanners
    integration.smoke.test_network.TestDeleteAccount
    integration.smoke.test_non_contigiousvlan.TestUpdatePhysicalNetwork
    integration.smoke.test_deploy_vm_iso.TestDeployVMFromISO
    integration.smoke.test_public_ip_range.TestDedicatePublicIPRange
    integration.smoke.test_multipleips_per_nic.TestDeployVM
    integration.smoke.test_regions.TestRegions
    integration.smoke.test_affinity_groups.TestDeployVmWithAffinityGroup
    integration.smoke.test_network_acl.TestNetworkACL
    integration.smoke.test_pvlan.TestPVLAN
    integration.smoke.test_volumes.TestCreateVolume
    integration.smoke.test_ssvm.TestSSVMs
    integration.smoke.test_nic.TestNic
    integration.smoke.test_deploy_vm_root_resize.TestDeployVM
    integration.smoke.test_resource_detail.TestResourceDetail
    integration.smoke.test_secondary_storage.TestSecStorageServices
    integration.smoke.test_vm_life_cycle.TestDeployVM
    integration.smoke.test_disk_offerings.TestCreateDiskOffering


---
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-9228: Network update with mist...

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

    https://github.com/apache/cloudstack/pull/1333#issuecomment-174442821
  
    @nitin-maharana, it is much better now (did you see the difference?).
    I would just recommend you declaring a function as “var functionName = function (parameterIfAny){};”
    Is this going to be merged forward to master?
    
    The code LGTM now.



---
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-9228: Network update with mist...

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

    https://github.com/apache/cloudstack/pull/1333#issuecomment-222045623
  
    ### ACS CI BVT Run
     **Sumarry:**
     Build Number 61
     Hypervisor xenserver
     NetworkType Advanced
     Passed=66
     Failed=2
     Skipped=3
    
    _Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
    
    
    **Failed tests:**
    * test_service_offerings.py
    
     * ContextSuite context=TestServiceOfferings>:setup Failing since 3 runs
    
    * test_deploy_vm_iso.py
    
     * test_deploy_vm_from_iso Failing since 4 runs
    
    
    **Skipped tests:**
    test_vm_nic_adapter_vmxnet3
    test_static_role_account_acls
    test_deploy_vgpu_enabled_vm
    
    **Passed test suits:**
    test_deploy_vm_with_userdata.py
    test_affinity_groups_projects.py
    test_portable_publicip.py
    test_vpc_vpn.py
    test_over_provisioning.py
    test_global_settings.py
    test_scale_vm.py
    test_routers_iptables_default_policy.py
    test_routers.py
    test_reset_vm_on_reboot.py
    test_snapshots.py
    test_deploy_vms_with_varied_deploymentplanners.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_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 pull request: CLOUDSTACK-9228: Network update with mist...

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

    https://github.com/apache/cloudstack/pull/1333#issuecomment-218959269
  
    LGTM @nitin-maharana can you comment on Koushik's remark and also share screenshot


---
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-9228: Network update with mist...

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

    https://github.com/apache/cloudstack/pull/1333#issuecomment-214204041
  
    @nitin-maharana The second dialog is not going away after clicking "yes" even though the API call is issued with forced=true. I don't think this is expected. It only goes away on clicking "no" but results in re-issue of same API call. Tried it in both FF and Safari.


---
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-9228: Network update with mist...

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

    https://github.com/apache/cloudstack/pull/1333#issuecomment-211903333
  
    @nitin-maharana Looks like there is some problem. In confirmation dialog when I click "yes" nothing happens and on clicking "no" the offering is changed. 


---
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 #1333: CLOUDSTACK-9228: Network update with mistmatch in se...

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

    https://github.com/apache/cloudstack/pull/1333
  
    Updated 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: CLOUDSTACK-9228: Network update with mist...

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

    https://github.com/apache/cloudstack/pull/1333#issuecomment-212799750
  
    @koushik-das: Actually this dialog appears after the first CIDR unchanged confirmation dialog. If its an admin account, Even if we select yes/no, it will be redirected to second dialog asking for forced update. According to the input, it will execute the updateNetwork API with/without forced=true option.
    
    For user account, It won't be redirected to the second dialog asking for forced update, it will directly execute updateNetwork API.
    
    Can you please share some snapshots of what error you are getting.


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