You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by karuturi <gi...@git.apache.org> on 2016/12/06 10:57:21 UTC

[GitHub] cloudstack pull request #1818: CLOUDSTACK-9655 The template which is registe...

GitHub user karuturi opened a pull request:

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

    CLOUDSTACK-9655 The template which is registered in all zones will be deleted by deleting 1 template on any zone

    added extra warning message if it's a cross-zone template ("This is a
    cross zone template and will be deleted from all the zones. Are you sure
    you want to proceed?").

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

    $ git pull https://github.com/Accelerite/cloudstack CLOUDSTACK-9655

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

    https://github.com/apache/cloudstack/pull/1818.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 #1818
    
----
commit d6a430990c899f95b5181ae1fd53df9ea7d57b07
Author: Rajani Karuturi <ra...@accelerite.com>
Date:   2016-12-06T10:55:01Z

    CLOUDSTACK-9655 The template which is registered in all zones will be
    deleted by deleting 1 template on any zone
    
    added extra warning message if it's a cross-zone template ("This is a
    cross zone template and will be deleted from all the zones. Are you sure
    you want to proceed?").

----


---
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 #1818: CLOUDSTACK-9655 The template which is registered in ...

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

    https://github.com/apache/cloudstack/pull/1818
  
    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 #1818: CLOUDSTACK-9655 The template which is registered in ...

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

    https://github.com/apache/cloudstack/pull/1818
  
    ### ACS CI BVT Run
     **Sumarry:**
     Build Number 161
     Hypervisor xenserver
     NetworkType Advanced
     Passed=96
     Failed=4
     Skipped=7
    
    _Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
    
    
    **Failed tests:**
    * test_vm_snapshots.py
    
     * ContextSuite context=TestVmSnapshot>:setup Failed
    
    * test_snapshots.py
    
     * test_01_snapshot_root_disk Failing since 7 runs
    
    * test_deploy_vm_iso.py
    
     * test_deploy_vm_from_iso Failing since 31 runs
    
    * test_vm_life_cycle.py
    
     * test_10_attachAndDetach_iso Failing since 32 runs
    
    
    **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_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_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 #1818: CLOUDSTACK-9655 The template which is registered in ...

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

    https://github.com/apache/cloudstack/pull/1818
  
    ### ACS CI BVT Run
     **Sumarry:**
     Build Number 160
     Hypervisor xenserver
     NetworkType Advanced
     Passed=92
     Failed=8
     Skipped=7
    
    _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 18 runs
    
    * test_snapshots.py
    
     * test_01_snapshot_root_disk Failing since 6 runs
    
    * test_non_contigiousvlan.py
    
     * test_extendPhysicalNetworkVlan Failed
    
    * test_deploy_vm_iso.py
    
     * test_deploy_vm_from_iso Failing since 30 runs
    
    * test_volumes.py
    
     * ContextSuite context=TestCreateVolume>:setup Failing since 3 runs
    
    * test_nic.py
    
     * test_01_nic Failing since 2 runs
    
    * test_vm_life_cycle.py
    
     * test_08_migrate_vm Failed
    
     * test_10_attachAndDetach_iso Failing since 31 runs
    
    
    **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_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_login.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_deploy_vm_root_resize.py
    test_resource_detail.py
    test_secondary_storage.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 #1818: CLOUDSTACK-9655 The template which is registered in ...

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

    https://github.com/apache/cloudstack/pull/1818
  
    @ustcweizhou agreed. I am merging this PR. I added your comment to the description of the bug(so that the bug is not closed and can be taken up later)
    I am merging this for now. 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 issue #1818: CLOUDSTACK-9655 The template which is registered in ...

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

    https://github.com/apache/cloudstack/pull/1818
  
    @ustcweizhou in the above snippet, its adding the zoneid if its not cross-zone. if no zoneid is provided, it will default to -1 which means cross zone. So, this check is required. 
    This fix is just enhancing the message if its a cross-zone template. Its not trying to do the delete only in that zone. That is not possible.


---
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 #1818: CLOUDSTACK-9655 The template which is registe...

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

    https://github.com/apache/cloudstack/pull/1818#discussion_r100771599
  
    --- Diff: ui/scripts/templates.js ---
    @@ -1447,7 +1447,11 @@
                                                      label: 'label.action.delete.template',
                                                      messages: {
                                                          confirm: function(args) {
    -                                                         return 'message.action.delete.template';
    +                                                         if(args.context.templates[0].crossZones == true) {
    +                                                             return 'This is a cross zone template and will be deleted from all the zones. Are you sure you want to proceed?';
    --- End diff --
    
    used an existing message from properties


---
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 #1818: CLOUDSTACK-9655 The template which is registered in ...

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

    https://github.com/apache/cloudstack/pull/1818
  
    ### ACS CI BVT Run
     **Sumarry:**
     Build Number 159
     Hypervisor xenserver
     NetworkType Advanced
     Passed=101
     Failed=4
     Skipped=7
    
    _Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
    
    
    **Failed tests:**
    * test_snapshots.py
    
     * test_01_snapshot_root_disk Failing since 5 runs
    
    * test_deploy_vm_iso.py
    
     * test_deploy_vm_from_iso Failing since 29 runs
    
    * test_nic.py
    
     * test_01_nic Failed
    
    * test_vm_life_cycle.py
    
     * test_10_attachAndDetach_iso Failing since 30 runs
    
    
    **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_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_deploy_vm_root_resize.py
    test_resource_detail.py
    test_secondary_storage.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 #1818: CLOUDSTACK-9655 The template which is registered in ...

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

    https://github.com/apache/cloudstack/pull/1818
  
    @karuturi in my understanding, the zoneid is not necessary if template is not cross-zone.
    If template is cross-zone, the zoneid should be passed if template is deleted from the 'Zones' tab (it means template will be deleted from the zone), Otherwise, the template will be deleted from all zones (this is why I suggest to add the new button in template details page, then the zoneid will not be set)
    Please correct me if I am wrong.



---
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 #1818: CLOUDSTACK-9655 The template which is registered in ...

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

    https://github.com/apache/cloudstack/pull/1818
  
    related to this PR, I doubt the action in deleteTemplate on UI (from a zone).
    ```
                                                         if (!args.context.templates[0].crossZones){
                                                            queryParams += "&zoneid=" + args.context.zones[0].zoneid;
                                                         }
    ```
    should "!" is removed in the first line ?
    
    I would suggest to add the new button in the template details page, and use this warning message ("message.action.delete.template.for.all.zones").
    



---
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 #1818: CLOUDSTACK-9655 The template which is registered in ...

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

    https://github.com/apache/cloudstack/pull/1818
  
    @karuturi I think it should work like 
    (1) list template status in all zones (now only one is shown)
    (2) delete template from a zone will pass zoneid and remove template from the zone (now zoneid is not passed and template will be removed from all zones).
    (3) allow user to delete template from all zones somewhere (we have some customers asked how to delete template at the beginning of upgrade from 4.2.1 to 4.7.1, as they are used to delete template from template details page, the deletetemplate button in the zones tab is difficult to be found).
    
    API changes is needed for (1). UI changes is needed for (2) and (3).
    
    For now, as the template will be deleted from all zones when we select a zone in zones tab, it is confused for users, so I agree we should change the description as a workaround.
    
    LGTM +1.


---
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 #1818: CLOUDSTACK-9655 The template which is registered in ...

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

    https://github.com/apache/cloudstack/pull/1818
  
    @ustcweizhou mm.. I am confused now :) a new button definitely makes sense but, a better error message doesn't worsen the current situation. So, until we have new button, we can have this improved error message. Do you agree? 


---
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 #1818: CLOUDSTACK-9655 The template which is registe...

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

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


---
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 #1818: CLOUDSTACK-9655 The template which is registe...

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

    https://github.com/apache/cloudstack/pull/1818#discussion_r91448021
  
    --- Diff: ui/scripts/templates.js ---
    @@ -1447,7 +1447,11 @@
                                                      label: 'label.action.delete.template',
                                                      messages: {
                                                          confirm: function(args) {
    -                                                         return 'message.action.delete.template';
    +                                                         if(args.context.templates[0].crossZones == true) {
    +                                                             return 'This is a cross zone template and will be deleted from all the zones. Are you sure you want to proceed?';
    --- End diff --
    
    @karuturi can you add this to a new dictionary, in messages.properties etc so this can be translated in other languages?


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