-
Notifications
You must be signed in to change notification settings - Fork 1.4k
CLOUDSTACK-10199: Support requesting a specific IPv4 address #2595
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -231,27 +231,27 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra | |
| @Inject | ||
| EntityManager _entityMgr; | ||
| @Inject | ||
| DataCenterDao _dcDao = null; | ||
| DataCenterDao _dcDao; | ||
| @Inject | ||
| VlanDao _vlanDao = null; | ||
| VlanDao _vlanDao; | ||
| @Inject | ||
| IPAddressDao _ipAddressDao = null; | ||
| IPAddressDao _ipAddressDao; | ||
| @Inject | ||
| AccountDao _accountDao = null; | ||
| AccountDao _accountDao; | ||
| @Inject | ||
| ConfigurationDao _configDao; | ||
| @Inject | ||
| UserVmDao _userVmDao = null; | ||
| UserVmDao _userVmDao; | ||
| @Inject | ||
| AlertManager _alertMgr; | ||
| @Inject | ||
| ConfigurationManager _configMgr; | ||
| @Inject | ||
| NetworkOfferingDao _networkOfferingDao = null; | ||
| NetworkOfferingDao _networkOfferingDao; | ||
| @Inject | ||
| NetworkDao _networksDao = null; | ||
| NetworkDao _networksDao; | ||
| @Inject | ||
| NicDao _nicDao = null; | ||
| NicDao _nicDao; | ||
| @Inject | ||
| RulesManager _rulesMgr; | ||
| @Inject | ||
|
|
@@ -860,6 +860,11 @@ public Pair<NicProfile, Integer> allocateNic(final NicProfile requested, final N | |
|
|
||
| NicVO vo = new NicVO(guru.getName(), vm.getId(), network.getId(), vm.getType()); | ||
|
|
||
| DataCenterVO dcVo = _dcDao.findById(network.getDataCenterId()); | ||
| if (dcVo.getNetworkType() == NetworkType.Basic) { | ||
| configureNicProfileBasedOnRequestedIp(requested, profile, network); | ||
| } | ||
|
|
||
| deviceId = applyProfileToNic(vo, profile, deviceId); | ||
|
|
||
| vo = _nicDao.persist(vo); | ||
|
|
@@ -871,6 +876,85 @@ public Pair<NicProfile, Integer> allocateNic(final NicProfile requested, final N | |
| return new Pair<NicProfile, Integer>(vmNic, Integer.valueOf(deviceId)); | ||
| } | ||
|
|
||
| /** | ||
| * If the requested IPv4 address from the NicProfile was configured then it configures the IPv4 address, Netmask and Gateway to deploy the VM with the requested IP. | ||
| */ | ||
| protected void configureNicProfileBasedOnRequestedIp(NicProfile requestedNicProfile, NicProfile nicProfile, Network network) { | ||
| if (requestedNicProfile == null) { | ||
| return; | ||
| } | ||
| String requestedIpv4Address = requestedNicProfile.getRequestedIPv4(); | ||
| if (requestedIpv4Address == null) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can extract lines 893-902 to a specific method that validates
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @GabrielBrascher, @rafaelweingartner, I think the last if condition covers both null and blank check. (isValidIp4 method)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, you are right @nitin-maharana; however, the null check should exist to return without an exception and consider that the user did not request a specific IPv4. |
||
| return; | ||
| } | ||
| if (!NetUtils.isValidIp4(requestedIpv4Address)) { | ||
| throw new InvalidParameterValueException(String.format("The requested [IPv4 address='%s'] is not a valid IP address", requestedIpv4Address)); | ||
| } | ||
|
|
||
| VlanVO vlanVo = _vlanDao.findByNetworkIdAndIpv4(network.getId(), requestedIpv4Address); | ||
| if (vlanVo == null) { | ||
| throw new InvalidParameterValueException(String.format("Trying to configure a Nic with the requested [IPv4='%s'] but cannot find a Vlan for the [network id='%s']", | ||
| requestedIpv4Address, network.getId())); | ||
| } | ||
|
|
||
| String ipv4Gateway = vlanVo.getVlanGateway(); | ||
| String ipv4Netmask = vlanVo.getVlanNetmask(); | ||
|
|
||
| if (!NetUtils.isValidIp4(ipv4Gateway)) { | ||
| throw new InvalidParameterValueException(String.format("The [IPv4Gateway='%s'] from [VlanId='%s'] is not valid", ipv4Gateway, vlanVo.getId())); | ||
| } | ||
| if (!NetUtils.isValidIp4Netmask(ipv4Netmask)) { | ||
| throw new InvalidParameterValueException(String.format("The [IPv4Netmask='%s'] from [VlanId='%s'] is not valid", ipv4Netmask, vlanVo.getId())); | ||
| } | ||
|
|
||
| acquireLockAndCheckIfIpv4IsFree(network, requestedIpv4Address); | ||
|
|
||
| nicProfile.setIPv4Address(requestedIpv4Address); | ||
| nicProfile.setIPv4Gateway(ipv4Gateway); | ||
| nicProfile.setIPv4Netmask(ipv4Netmask); | ||
|
|
||
| if (nicProfile.getMacAddress() == null) { | ||
| try { | ||
| String macAddress = _networkModel.getNextAvailableMacAddressInNetwork(network.getId()); | ||
| nicProfile.setMacAddress(macAddress); | ||
| } catch (InsufficientAddressCapacityException e) { | ||
| throw new CloudRuntimeException(String.format("Cannot get next available mac address in [network id='%s']", network.getId()), e); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Acquires lock in "user_ip_address" and checks if the requested IPv4 address is Free. | ||
| */ | ||
| protected void acquireLockAndCheckIfIpv4IsFree(Network network, String requestedIpv4Address) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These exceptions that you throw here, I think it might be better to throw an
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that makes sense. I will update it.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will this check work for requested IP addresses in case of Advanced networking?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, @fmaximus. I will check on that.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @fmaximus cannot users define a guest IP? I mean, as long as the guest IP defined by the user is within the bounds of the guest network, it should be possible, right?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rafaelweingartner The issue is that when a user selects a guest IP, it won't be found by _ipAddressDao, because IPAddressVO is for public IP's, not for guest IP's.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @fmaximus thanks for pointing it out and sorry for the delay. I did some testing in advanced network and confirmed that a few modifications in this PR will be necessary. When I finish them I will ping you. |
||
| IPAddressVO ipVO = _ipAddressDao.findByIpAndSourceNetworkId(network.getId(), requestedIpv4Address); | ||
| if (ipVO == null) { | ||
| throw new InvalidParameterValueException( | ||
| String.format("Cannot find IPAddressVO for guest [IPv4 address='%s'] and [network id='%s']", requestedIpv4Address, network.getId())); | ||
| } | ||
| try { | ||
| IPAddressVO lockedIpVO = _ipAddressDao.acquireInLockTable(ipVO.getId()); | ||
| validateLockedRequestedIp(ipVO, lockedIpVO); | ||
| lockedIpVO.setState(IPAddressVO.State.Allocated); | ||
| _ipAddressDao.update(lockedIpVO.getId(), lockedIpVO); | ||
| } finally { | ||
| _ipAddressDao.releaseFromLockTable(ipVO.getId()); | ||
|
GabrielBrascher marked this conversation as resolved.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The finally block will release the lock in case of success or an exception. |
||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Validates the locked IP, throwing an exeption if the locked IP is null or the locked IP is not in 'Free' state. | ||
| */ | ||
| protected void validateLockedRequestedIp(IPAddressVO ipVO, IPAddressVO lockedIpVO) { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Created validateLockedRequestedIp to extract some verifications, documenting it and creating test cases. The logic stills the same. as the caller (method acquireLockAndCheckIfIpv4IsFree) does have a |
||
| if (lockedIpVO == null) { | ||
| throw new InvalidParameterValueException(String.format("Cannot acquire guest [IPv4 address='%s'] as it was removed while acquiring lock", ipVO.getAddress())); | ||
| } | ||
| if (lockedIpVO.getState() != IPAddressVO.State.Free) { | ||
| throw new InvalidParameterValueException( | ||
| String.format("Cannot acquire guest [IPv4 address='%s']; The Ip address is in [state='%s']", ipVO.getAddress(), lockedIpVO.getState().toString())); | ||
| } | ||
| } | ||
|
|
||
| protected Integer applyProfileToNic(final NicVO vo, final NicProfile profile, Integer deviceId) { | ||
| if (profile.getDeviceId() != null) { | ||
| vo.setDeviceId(profile.getDeviceId()); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fmaximus I changed the execution flow; now this workflow will be executed just in case of a basic network.
After this change I could add IPv4 either in an advanced network, with a 'guest IP', and in a basic network with a 'public IP' (from the IPAddressVO in this case). Thanks again for pointing that issue.