Skip to content

Commit

Permalink
Changes and refactoring according to review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
igoro1975 committed Dec 25, 2016
1 parent 5333a77 commit 03a7fd7
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 18 deletions.
42 changes: 29 additions & 13 deletions package/cloudshell/cp/azure/domain/services/ip_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,41 @@ def __init__(self):
self._cached_available_private_ips = {}
self._available_private_ips_lock = Lock()

def get_available_private_ip(self, network_client, management_group_name, virtual_network_name, ip_address, logger):
cached_key = (management_group_name, virtual_network_name, ip_address)

with self._available_private_ips_lock:
available_private_ips = self._cached_available_private_ips.get(cached_key)
if not available_private_ips:
def get_available_private_ip(self, network_client, group_name, virtual_network_name, ip_address, logger):
"""
The method returns the checked ip_address if this available or the next available ip
:param network_client:
:param group_name:
:param virtual_network_name: network in the group name
:param ip_address: ip address checked for availability
:param logger:
:return:
"""

cached_key = (group_name, virtual_network_name, ip_address)

available_private_ips = self._cached_available_private_ips.get(cached_key)
if not available_private_ips:
with self._available_private_ips_lock:
availability = network_client.virtual_networks.check_ip_address_availability(
management_group_name,
group_name,
virtual_network_name,
ip_address
)

available_private_ips = availability.available_ip_addresses

self._cached_available_private_ips[cached_key] = available_private_ips

logger.info(
"Retrieving available IPs for {} in {}".format(virtual_network_name, management_group_name))

"Retrieving available IP for {} in {}, checking {}".format(virtual_network_name,
group_name, ip_address))

if availability.available:
return ip_address
else:
self._cached_available_private_ips[cached_key] = availability.available_ip_addresses

if ip_address in self._cached_available_private_ips[cached_key]:
self._cached_available_private_ips[cached_key].remove(ip_address)
available_ip = ip_address
else:
available_ip = self._cached_available_private_ips[cached_key].pop(0)

return available_ip
15 changes: 10 additions & 5 deletions package/cloudshell/cp/azure/domain/services/network_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ def create_nic(self, interface_name, group_name, management_group_name, network_
private_ip_allocation_method, tags, virtual_network_name,
logger):
"""
The method creates or updates network interface.
Parameter
:param logger:
:param virtual_network_name:
:param group_name:
Expand All @@ -91,10 +92,14 @@ def create_nic(self, interface_name, group_name, management_group_name, network_
:return:
"""

private_ip_address = self.ip_service.get_available_private_ip(network_client, management_group_name,
virtual_network_name,
subnet.address_prefix[:-3],
logger)
# private_ip_address in required only in the case of static allocation method
# in the case of dynamic allocation method is ignored
private_ip_address = ""
if private_ip_allocation_method.static:
private_ip_address = self.ip_service.get_available_private_ip(network_client, management_group_name,
virtual_network_name,
subnet.address_prefix[:-3],
logger)

operation_poller = network_client.network_interfaces.create_or_update(
group_name,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
from unittest import TestCase

from mock import MagicMock

from cloudshell.cp.azure.domain.services.ip_service import IpService


class TestIpService(TestCase):
def setUp(self):
self.key_pair_service = IpService()
self.group_name = "tested_group"
self.virtual_network_name = "tested_network"
self.tested_ip_address = "10.0.1.1"
self.cached_key = (self.group_name, self.virtual_network_name, self.tested_ip_address)
self.logger = MagicMock()
self.network_client = MagicMock()

def test_no_calls_if_cache_exists(self):
# Arrange
self.key_pair_service._cached_available_private_ips[self.cached_key] = [self.tested_ip_address]

# Act
self.key_pair_service.get_available_private_ip(self.network_client, self.group_name, self.virtual_network_name,
self.tested_ip_address, self.logger)

# Verify
self.network_client.virtual_networks.check_ip_address_availability.assert_not_called()

def test_check_ip_address_availability_called(self):
# Arrange
self.key_pair_service._cached_available_private_ips = {}

# Act
self.key_pair_service.get_available_private_ip(self.network_client, self.group_name, self.virtual_network_name,
self.tested_ip_address, self.logger)

# Verify
self.network_client.virtual_networks.check_ip_address_availability.assert_called_once_with(
self.group_name, self.virtual_network_name, self.tested_ip_address
)

0 comments on commit 03a7fd7

Please sign in to comment.