Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLOUDSTACK-9694: Unable to limit the Public IPs in VPC #1850

Merged
merged 1 commit into from Jun 6, 2017

Conversation

sudhansu7
Copy link

@sudhansu7 sudhansu7 commented Dec 21, 2016

Unable to limit the Public IPs in VPC.
In VPC network, while acquiring the IP addresses, in the resource_count table, count for the domain is getting increased. However, when the resource count is updated at Domain level, resource count is getting reverted to only non-vpc ip count.

Steps to Reproduce:

  1. Create a VPC
  2. Create a VPC tier.
  3. Check resource_count table and note the ip address count. (say 1)
  4. Keep acquiring the IP addresses, (say 4 IP addresses). Now new ip address count resource_count table is 5.
  5. update the resource count at domain level.
  6. the resource_count is updated back 1

Root Cause: Update resource count command recalculates the resource count. While computing public IP we are not considering the ips allocated to VPC.

ResourceLimitManagerImpl.java -> calculatePublicIpForAccount() -> IPAddressDaoImpl.countAllocatedIPsForAccount()

Currently we have below query builder. Which does not consider vpc_id column.

        AllocatedIpCountForAccount = createSearchBuilder(Long.class);
        AllocatedIpCountForAccount.select(null, Func.COUNT, AllocatedIpCountForAccount.entity().getAddress());
        AllocatedIpCountForAccount.and("account", AllocatedIpCountForAccount.entity().getAllocatedToAccountId(), Op.EQ);
        AllocatedIpCountForAccount.and("allocated", AllocatedIpCountForAccount.entity().getAllocatedTime(), Op.NNULL);
        AllocatedIpCountForAccount.and("network", AllocatedIpCountForAccount.entity().getAssociatedWithNetworkId(), Op.NNULL);
        AllocatedIpCountForAccount.done();

it generates below sql query

SELECT COUNT(user_ip_address.public_ip_address) FROM user_ip_address WHERE user_ip_address.account_id = 6  AND user_ip_address.allocated IS NOT NULL  AND user_ip_address.network_id IS NOT NULL  AND user_ip_address.removed IS NULL

Fix:
Add vpc_id check in query.

        AllocatedIpCountForAccount = createSearchBuilder(Long.class);
        AllocatedIpCountForAccount.select(null, Func.COUNT, AllocatedIpCountForAccount.entity().getAddress());
        AllocatedIpCountForAccount.and("account", AllocatedIpCountForAccount.entity().getAllocatedToAccountId(), Op.EQ);
        AllocatedIpCountForAccount.and("allocated", AllocatedIpCountForAccount.entity().getAllocatedTime(), Op.NNULL);
        AllocatedIpCountForAccount.and().op("network", AllocatedIpCountForAccount.entity().getAssociatedWithNetworkId(), Op.NNULL);
        AllocatedIpCountForAccount.or("vpc", AllocatedIpCountForAccount.entity().getVpcId(), Op.NNULL);
        AllocatedIpCountForAccount.cp();
        AllocatedIpCountForAccount.done();

SQL:

SELECT COUNT(user_ip_address.public_ip_address) FROM user_ip_address WHERE user_ip_address.account_id = 6  AND user_ip_address.allocated IS NOT NULL  AND ( user_ip_address.network_id IS NOT NULL or user_ip_address.vpc_id IS NOT NULL) AND user_ip_address.removed IS NULL

Test Result:
Initial resource count

mysql> select * from resource_count where type in ('vpc','public_ip') and domain_id = 3;
+----+------------+-----------+-----------+-------+
| id | account_id | domain_id | type      | count |
+----+------------+-----------+-----------+-------+
| 66 |       NULL |         3 | public_ip |     2 |
| 72 |       NULL |         3 | vpc       |     1 |
+----+------------+-----------+-----------+-------+
2 rows in set (0.00 sec)

Aquire 2 more public ip in VPC network.

mysql> select * from resource_count where type in ('vpc','public_ip') and domain_id = 3;
+----+------------+-----------+-----------+-------+
| id | account_id | domain_id | type      | count |
+----+------------+-----------+-----------+-------+
| 66 |       NULL |         3 | public_ip |     4 |
| 72 |       NULL |         3 | vpc       |     1 |
+----+------------+-----------+-----------+-------+
2 rows in set (0.00 sec)

After updating resource count at for doamin 3.

mysql> select * from resource_count where type in ('vpc','public_ip') and domain_id = 3;
+----+------------+-----------+-----------+-------+
| id | account_id | domain_id | type      | count |
+----+------------+-----------+-----------+-------+
| 66 |       NULL |         3 | public_ip |     1 |
| 72 |       NULL |         3 | vpc       |     1 |
+----+------------+-----------+-----------+-------+
2 rows in set (0.00 sec)

Output of INCORRECT query used for calculating public ip addresses.

mysql> SELECT COUNT(user_ip_address.public_ip_address) FROM user_ip_address WHERE user_ip_address.account_id = 4  AND user_ip_address.allocated IS NOT NULL  AND user_ip_address.network_id IS NOT NULL  AND user_ip_address.removed IS NULL;
+------------------------------------------+
| COUNT(user_ip_address.public_ip_address) |
+------------------------------------------+
|                                        1 |
+------------------------------------------+
1 row in set (0.00 sec)

Output of CORRECT query used for calculating public ip addresses.

mysql> SELECT COUNT(user_ip_address.public_ip_address) FROM user_ip_address WHERE user_ip_address.account_id = 4  AND user_ip_address.allocated IS NOT NULL  AND ( user_ip_address.network_id IS NOT NULL or user_ip_address.vpc_id IS NOT NULL) AND user_ip_address.removed IS NULL;
+------------------------------------------+
| COUNT(user_ip_address.public_ip_address) |
+------------------------------------------+
|                                        4 |
+------------------------------------------+
1 row in set (0.00 sec)

After FIX:

mysql> select * from resource_count where type in ('vpc','public_ip') and domain_id = 3;
+----+------------+-----------+-----------+-------+
| id | account_id | domain_id | type      | count |
+----+------------+-----------+-----------+-------+
| 66 |       NULL |         3 | public_ip |     4 |
| 72 |       NULL |         3 | vpc       |     1 |
+----+------------+-----------+-----------+-------+
2 rows in set (0.00 sec)

@koushik-das
Copy link
Contributor

Code changes LGTM. Can you add some unit/marvin tests for the scenario that you have mentioned?

@sudhansu7 sudhansu7 force-pushed the CLOUDSTACK-9694 branch 2 times, most recently from b210722 to 2b056ff Compare March 8, 2017 09:37
@sudhansu7
Copy link
Author

@koushik-das
Added marvin test. Please review.

@niteshsarda
Copy link
Contributor

niteshsarda commented Mar 22, 2017

I have tested this and LGTM for test.

@sudhansu7
Copy link
Author

tag:mergeready

@sudhansu7 sudhansu7 closed this Apr 27, 2017
@sudhansu7 sudhansu7 reopened this Apr 27, 2017
Added missing clause to check for vpc_id
@cloudmonger
Copy link

ACS CI BVT Run

Sumarry:
Build Number 725
Hypervisor xenserver
NetworkType Advanced
Passed=112
Failed=0
Skipped=12

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

Failed tests:

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_vm_snapshots.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_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_routers_network_ops.py
test_disk_offerings.py

@karuturi karuturi merged commit cf4cde6 into apache:master Jun 6, 2017
GaOrtiga pushed a commit to scclouds/cloudstack that referenced this pull request Apr 25, 2024
Refatoração da classe KubernetesClusterResourceModifierActionWorker

Closes apache#1850

See merge request scclouds/scclouds!752
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants