CLOUDSTACK-9812:Update "updatePortForwardingRule" api to include additional parameter to update the end port in case of port range by mrunalinikankariya · Pull Request #1985 · apache/cloudstack

mrunalinikankariya · 2017-03-03T11:00:31Z

Configure a PF rule Private port : Start port ; 20 ENd POrt 25 || Public Port : Start port 20 ; ENd Port : 25.
Trigger UpdatePortForwardingRule api
ApI fails with following error : " Unable to update the private port of port forwarding rule as the rule has port range "

Solution-
Port range gets modified

yvsubhash · 2017-05-10T08:19:09Z

+        }
+        if (privateEndPort != null) {
+            rule.setDestinationPortStart((privatePort == null) ? privateEndPort.intValue() : privatePort.intValue());
+            rule.setDestinationPortEnd(privateEndPort);


This line is not needed. It would have got set already

This line is for the case when privatePort is null and privateEndPort is not null. Rather else part in conditional if(privateEndport == null) can be removed

However, above two can be combined into one "if else" with nested conditions to avoid any duplicate statements

yvsubhash · 2017-05-10T08:20:14Z

+            rule.setDestinationPortEnd((privateEndPort == null) ? privatePort.intValue() : privateEndPort.intValue());
+        }
+        if (privateEndPort != null) {
+            rule.setDestinationPortStart((privatePort == null) ? privateEndPort.intValue() : privatePort.intValue());


else of part of conditional is redundant. already taken care in previous if

Agreed, will remove it

yvsubhash

two successive ifs can be written like this
if (privatePort != null) { rule.setDestinationPortStart(privatePort.intValue());
rule.setDestinationPortEnd((privateEndPort == null) ? privatePort.intValue() : privateEndPort.intValue());
}
else if (privateEndPort != null) {
rule.setDestinationPortStart( privateEndPort.intValue());
rule.setDestinationPortEnd(privateEndPort);
}

mrunalinikankariya · 2017-05-19T13:38:18Z

Done, please check

jayapalu · 2017-05-29T06:45:27Z

Code LGTM

mrunalinikankariya · 2017-06-09T04:11:03Z

tag:This is Ready to Merge

yadvr · 2017-08-23T15:46:21Z

+from marvin.codes import PASS
+from nose.plugins.attrib import attr
+
+class TestPortForwardingRules(cloudstackTestCase):


If this test is rather quick, move it to smoke please.

…tional parameter end port

cloudmonger · 2017-08-28T13:55:24Z

ACS CI BVT Run

Sumarry:
Build Number 1157
Hypervisor xenserver
NetworkType Advanced
Passed=110
Failed=6
Skipped=12

Link to logs Folder (search by build_no): https://www.dropbox.com/sh/r2si930m8xxzavs/AAAzNrnoF1fC3auFrvsKo_8-a?dl=0

Failed tests:

test_vm_snapshots.py
test_change_service_offering_for_vm_with_snapshots Failed
test_routers_network_ops.py
test_01_isolate_network_FW_PF_default_routes_egress_true Failing since 22 runs
test_02_isolate_network_FW_PF_default_routes_egress_false Failing since 149 runs
test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true Failing since 144 runs
test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false Failing since 144 runs
ContextSuite context=TestRedundantIsolateNetworks>:teardown Failing since 13 runs

Skipped tests:
test_vm_nic_adapter_vmxnet3
test_01_verify_libvirt
test_02_verify_libvirt_after_restart
test_03_verify_libvirt_attach_disk
test_04_verify_guest_lspci
test_05_change_vm_ostype_restart
test_06_verify_guest_lspci_again
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_router_dnsservice.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_portforwardingrules.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_metrics_api.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

SowjanyaPatha · 2017-08-30T06:28:01Z

LGTM for Testing
Able to update Port Forwarding Rules.
First Created PortForwarding rule with private port range (22-22) and public port range (3000-3000) Assigned this PF rule to a VM.
executed updatePortForwrdingRule api
http://localhost:8080/client/api?command=updatePortForwardingRule&id=a75f2c9d-484b-41b0-bf85-59f4a532c9db&privateport=80&privateendport=80
private ports got updated.

yadvr · 2017-08-30T08:56:17Z

@blueorangutan package

blueorangutan · 2017-08-30T08:57:07Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2017-08-30T09:25:36Z

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1045

yadvr · 2017-08-30T09:37:12Z

@blueorangutan test

blueorangutan · 2017-08-30T09:38:07Z

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2017-08-30T18:37:49Z

Trillian test result (tid-1457)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 32379 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr1985-t1457-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_iso.py
Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Test completed. 56 look OK, 3 have error(s)

Test	Result	Time (s)	Test File
test_01_vpc_remote_access_vpn	`Failure`	60.69	test_vpc_vpn.py
test_04_rvpc_privategw_static_routes	`Failure`	364.80	test_privategw_acl.py
test_05_iso_permissions	`Failure`	0.04	test_iso.py
test_02_edit_iso	`Failure`	0.04	test_iso.py
test_change_service_offering_for_vm_with_snapshots	Skipped	0.00	test_vm_snapshots.py
test_09_copy_delete_template	Skipped	0.01	test_templates.py
test_06_copy_template	Skipped	0.00	test_templates.py
test_static_role_account_acls	Skipped	0.01	test_staticroles.py
test_11_ss_nfs_version_on_ssvm	Skipped	0.02	test_ssvm.py
test_01_scale_vm	Skipped	0.00	test_scale_vm.py
test_01_primary_storage_iscsi	Skipped	0.03	test_primary_storage.py
test_vm_nic_adapter_vmxnet3	Skipped	0.00	test_nic_adapter_type.py
test_nested_virtualization_vmware	Skipped	0.00	test_nested_virtualization.py
test_06_copy_iso	Skipped	0.00	test_iso.py
test_deploy_vgpu_enabled_vm	Skipped	0.02	test_deploy_vgpu_enabled_vm.py
test_3d_gpu_support	Skipped	0.03	test_deploy_vgpu_enabled_vm.py

yadvr · 2017-08-31T07:42:27Z

LGTM, no new regressions found. The errors are known intermittent failures, being handled separately.
Checked, new smoke test passed in Trillian:

@summary: Test to list, create and delete Port Forwarding for ... === TestName: test_01_create_delete_portforwarding_fornonvpc | Status : SUCCESS ===
ok

Ran 1 test in 253.043s

OK

kiwiflyer added the type:enhancement label May 3, 2017

yvsubhash reviewed May 10, 2017

View reviewed changes

yvsubhash approved these changes May 19, 2017

View reviewed changes

yadvr reviewed Aug 23, 2017

View reviewed changes

CLOUDSTACK-9812:Update 'updatePortForwardingRule' api to include addi…

aadc7e9

…tional parameter end port

yadvr merged commit d1e61eb into apache:master Aug 31, 2017

Uh oh!

Conversation

mrunalinikankariya commented Mar 3, 2017

Uh oh!

yvsubhash May 10, 2017

Choose a reason for hiding this comment

Uh oh!

mrunalinikankariya May 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yvsubhash May 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yvsubhash May 10, 2017

Choose a reason for hiding this comment

Uh oh!

mrunalinikankariya May 11, 2017

Choose a reason for hiding this comment

Uh oh!

yvsubhash left a comment

Choose a reason for hiding this comment

Uh oh!

mrunalinikankariya commented May 19, 2017

Uh oh!

jayapalu commented May 29, 2017

Uh oh!

mrunalinikankariya commented Jun 9, 2017

Uh oh!

yadvr Aug 23, 2017

Choose a reason for hiding this comment

Uh oh!

mrunalinikankariya Aug 30, 2017

Choose a reason for hiding this comment

Uh oh!

cloudmonger commented Aug 28, 2017

ACS CI BVT Run

Uh oh!

SowjanyaPatha commented Aug 30, 2017

Uh oh!

yadvr commented Aug 30, 2017

Uh oh!

blueorangutan commented Aug 30, 2017

Uh oh!

blueorangutan commented Aug 30, 2017

Uh oh!

yadvr commented Aug 30, 2017

Uh oh!

blueorangutan commented Aug 30, 2017

Uh oh!

blueorangutan commented Aug 30, 2017

Uh oh!

yadvr commented Aug 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

mrunalinikankariya May 11, 2017 •

edited

Loading

yvsubhash May 11, 2017 •

edited

Loading

yadvr commented Aug 31, 2017 •

edited

Loading