[GitHub] cloudstack pull request: Check the existence of 'forceencap' param...
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/1402 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Check the existence of 'forceencap' param...
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1402#issuecomment-211916588 Thanks. Looks good 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Check the existence of 'forceencap' param...
Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/1402#issuecomment-211818185 @alexandrelimassantana @swill Thanks guys, fixed 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Check the existence of 'forceencap' param...
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1402#issuecomment-211562114 @remibergsma can you review the functionality you expect. I think the logic is wrong, and if it is not, then it is VERY misleading. The code in question that is being executed is this: `CsHelper.bool_to_yn(obj['encap'])` For reference, this is the code being executed: ``` def bool_to_yn(val): if val: return "yes" return "no" ``` If I just read your code, I would assume you would want the final result to be `forceencaps=no`, but this code as written will result in `forceencaps=yes`. Here is a simple test to prove the case...  I think @alexandrelimassantana is right. In order to get the expected behavior, you need to use the boolean `False`. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Check the existence of 'forceencap' param...
Github user alexandrelimassantana commented on the pull request: https://github.com/apache/cloudstack/pull/1402#issuecomment-201681267 @remibergsma can't you asking false instead of "false" ? If the function accepts boolean values I think that it would be more efficient. Besides that, the addition is quite straightforward and I would be happy to hand out my 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Check the existence of 'forceencap' param...
Github user bvbharatk commented on the pull request: https://github.com/apache/cloudstack/pull/1402#issuecomment-196143942 ### ACS CI BVT Run **Sumarry:** Build Number 92 Hypervisor xenserver NetworkType Advanced Passed=103 Failed=15 Skipped=4 **The follwing tests have known issues** integration.smoke.test_iso.TestISO.test_07_list_default_iso integration.smoke.test_privategw_acl.py integration.smoke.test_vm_snapshots.TestSnapshots.test_01_test_vm_volume_snapshot integration.smoke.test_vpc_vpn.TestVpcSite2SiteVpn.test_vpc_site2site_vpn integration.smoke.test_iso.TestISO.test_04_extract_Iso _Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0 **Failed tests:** * integration.smoke.test_vpc_vpn.TestVpcRemoteAccessVpn * test_vpc_remote_access_vpn * test_vpc_site2site_vpn * integration.smoke.test_vm_snapshots.TestSnapshots * test_01_test_vm_volume_snapshot * integration.smoke.test_privategw_acl.TestPrivateGwACL * test_01_vpc_privategw_acl * test_02_vpc_privategw_static_routes * test_03_rvpc_privategw_static_routes * integration.smoke.test_guest_vlan_range.TestDedicateGuestVlanRange * test_dedicateGuestVlanRange * :setup * :setup * integration.smoke.test_primary_storage.TestPrimaryStorageServices * test_01_primary_storage_iscsi * integration.smoke.test_internal_lb.TestInternalLb * test02_internallb_haproxy_stats_on_all_interfaces * test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 * integration.smoke.test_templates.TestCreateTemplate * test_04_extract_template * integration.smoke.test_iso.TestCreateIso * test_04_extract_Iso * test_07_list_default_iso **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_service_offerings.TestCreateServiceOffering integration.smoke.test_loadbalance.TestLoadBalance 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Check the existence of 'forceencap' param...
Github user kiwiflyer commented on the pull request: https://github.com/apache/cloudstack/pull/1402#issuecomment-193872249 Since forceencaps is no by default, I think this is a fair change. 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Check the existence of 'forceencap' param...
Github user bhaisaab commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1402#discussion_r52102132
--- Diff: systemvm/patches/debian/config/opt/cloud/bin/configure.py ---
@@ -531,6 +531,8 @@ def configure_ipsec(self, obj):
file.addeq(" pfs=%s" % CsHelper.bool_to_yn(obj['dpd']))
file.addeq(" keyingtries=2")
file.addeq(" auto=start")
+if not obj.has_key('encap'):
--- End diff --
Consider using: if 'encap' not in obj, instead of has_key(). More pythonic
that way ;)
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Check the existence of 'forceencap' param...
GitHub user remibergsma opened a pull request:
https://github.com/apache/cloudstack/pull/1402
Check the existence of 'forceencap' parameter before use
Check the existence of 'forceencap' parameter before use.
Error seen:
```
Traceback (most recent call last):
File "/opt/cloud/bin/update_config.py", line 140, in
process_file()
File "/opt/cloud/bin/update_config.py", line 54, in process_file
finish_config()
File "/opt/cloud/bin/update_config.py", line 44, in finish_config
returncode = configure.main(sys.argv)
File "/opt/cloud/bin/configure.py", line 1003, in main
vpns.process()
File "/opt/cloud/bin/configure.py", line 488, in process
self.configure_ipsec(self.dbag[vpn])
File "/opt/cloud/bin/configure.py", line 544, in configure_ipsec
file.addeq(" forceencaps=%s" % CsHelper.bool_to_yn(obj['encap']))
KeyError: 'encap'
```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/remibergsma/cloudstack forceencap_check
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1402.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 #1402
commit 4ad05d1ade9d98e3a1bd268bcc1d7a0f0767d26d
Author: Remi Bergsma
Date: 2016-02-05T15:58:01Z
Check the existence of 'forceencap' parameter before use
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
