You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Soheil Eizadi <se...@infoblox.com> on 2013/07/24 00:03:12 UTC
Review Request 12883: DNS value in Zone is not getting set in the Network
created for the Zone
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12883/
-----------------------------------------------------------
Review request for cloudstack.
Bugs: https://issues.apache.org/jira/browse/CLOUDSTACK-3754
Repository: cloudstack-git
Description
-------
DNS value in Zone is not getting set in the Network created for the Zone. Due to this the prepare and release calls to the Network Plugins are missing this information for the VMs.
see Jira bug https://issues.apache.org/jira/browse/CLOUDSTACK-3754 for additional details.
Administrators-MacBook-Pro-7:cloudstack seizadi$ git diff
diff --git a/ui/scripts/zoneWizard.js b/ui/scripts/zoneWizard.js
index cb92c4e..9885b1d 100755
--- a/ui/scripts/zoneWizard.js
+++ b/ui/scripts/zoneWizard.js
@@ -3084,7 +3084,9 @@
zoneid: args.data.returnedZone.id,
name: 'defaultGuestNetwork',
displaytext: 'defaultGuestNetwork',
- networkofferingid: args.data.zone.networkOfferingId
+ networkofferingid: args.data.zone.networkOfferingId,
+ networkDomain: args.data.returnedZone.domain
};
//Advanced zone with SG
Diffs
-----
Diff: https://reviews.apache.org/r/12883/diff/
Testing
-------
Created Basic Network with Wizard and validated with FireFox debugger that value was properly set and validated with CloudMonkey that the defaultGuestNetwork created had the DNS Domain value set.
I noted that there are other Ajax calls in UI that createNetwork() and don't set this field, also there is a use case where the zone value can be modified and this value would need to be updated in the attached Networks. So I would not close the Jira bug until those additional cases are fixed in the UI.
Thanks,
Soheil Eizadi
Re: Review Request 12883: DNS value in Zone is not getting set in the
Network created for the Zone
Posted by Soheil Eizadi <se...@infoblox.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12883/#review23785
-----------------------------------------------------------
After thinking about this some more I think a better way to fix this problem is to have the UI leave this NULL and have the Network Manager fill in the value by looking up the value in the Network, if it is NULL then lookup the value in the Zone. This behavior will avoid having additional code to handle the case where the Zone DNS field is updated.
I will look at this some more and submitted another patch for this problem.
- Soheil Eizadi
On July 23, 2013, 10:03 p.m., Soheil Eizadi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12883/
> -----------------------------------------------------------
>
> (Updated July 23, 2013, 10:03 p.m.)
>
>
> Review request for cloudstack.
>
>
> Bugs: https://issues.apache.org/jira/browse/CLOUDSTACK-3754
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> DNS value in Zone is not getting set in the Network created for the Zone. Due to this the prepare and release calls to the Network Plugins are missing this information for the VMs.
>
> see Jira bug https://issues.apache.org/jira/browse/CLOUDSTACK-3754 for additional details.
>
> Administrators-MacBook-Pro-7:cloudstack seizadi$ git diff
> diff --git a/ui/scripts/zoneWizard.js b/ui/scripts/zoneWizard.js
> index cb92c4e..9885b1d 100755
> --- a/ui/scripts/zoneWizard.js
> +++ b/ui/scripts/zoneWizard.js
> @@ -3084,7 +3084,9 @@
> zoneid: args.data.returnedZone.id,
> name: 'defaultGuestNetwork',
> displaytext: 'defaultGuestNetwork',
> - networkofferingid: args.data.zone.networkOfferingId
> + networkofferingid: args.data.zone.networkOfferingId,
> + networkDomain: args.data.returnedZone.domain
> };
>
> //Advanced zone with SG
>
>
> Diffs
> -----
>
>
> Diff: https://reviews.apache.org/r/12883/diff/
>
>
> Testing
> -------
>
> Created Basic Network with Wizard and validated with FireFox debugger that value was properly set and validated with CloudMonkey that the defaultGuestNetwork created had the DNS Domain value set.
>
> I noted that there are other Ajax calls in UI that createNetwork() and don't set this field, also there is a use case where the zone value can be modified and this value would need to be updated in the attached Networks. So I would not close the Jira bug until those additional cases are fixed in the UI.
>
>
> Thanks,
>
> Soheil Eizadi
>
>
Re: Review Request 12883: DNS value in Zone is not getting set in the
Network created for the Zone
Posted by Soheil Eizadi <se...@infoblox.com>.
> On July 24, 2013, 9:22 p.m., Soheil Eizadi wrote:
> > After thinking about this some more I think a better way to fix this problem is to have the UI leave this NULL and have the Network Manager fill in the value by looking up the value in the Network, if it is NULL then lookup the value in the Zone. This behavior will avoid having additional code to handle the case where the Zone DNS field is updated.
> >
> > I will look at this some more and submitted another patch for this problem.
Fixed the problem by changing the Server side and implementing a change in the Network Manager, so that the individual plugins are not burdened with this logic.
- Soheil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12883/#review23786
-----------------------------------------------------------
On July 23, 2013, 10:03 p.m., Soheil Eizadi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12883/
> -----------------------------------------------------------
>
> (Updated July 23, 2013, 10:03 p.m.)
>
>
> Review request for cloudstack.
>
>
> Bugs: https://issues.apache.org/jira/browse/CLOUDSTACK-3754
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> DNS value in Zone is not getting set in the Network created for the Zone. Due to this the prepare and release calls to the Network Plugins are missing this information for the VMs.
>
> see Jira bug https://issues.apache.org/jira/browse/CLOUDSTACK-3754 for additional details.
>
> Administrators-MacBook-Pro-7:cloudstack seizadi$ git diff
> diff --git a/ui/scripts/zoneWizard.js b/ui/scripts/zoneWizard.js
> index cb92c4e..9885b1d 100755
> --- a/ui/scripts/zoneWizard.js
> +++ b/ui/scripts/zoneWizard.js
> @@ -3084,7 +3084,9 @@
> zoneid: args.data.returnedZone.id,
> name: 'defaultGuestNetwork',
> displaytext: 'defaultGuestNetwork',
> - networkofferingid: args.data.zone.networkOfferingId
> + networkofferingid: args.data.zone.networkOfferingId,
> + networkDomain: args.data.returnedZone.domain
> };
>
> //Advanced zone with SG
>
>
> Diffs
> -----
>
>
> Diff: https://reviews.apache.org/r/12883/diff/
>
>
> Testing
> -------
>
> Created Basic Network with Wizard and validated with FireFox debugger that value was properly set and validated with CloudMonkey that the defaultGuestNetwork created had the DNS Domain value set.
>
> I noted that there are other Ajax calls in UI that createNetwork() and don't set this field, also there is a use case where the zone value can be modified and this value would need to be updated in the attached Networks. So I would not close the Jira bug until those additional cases are fixed in the UI.
>
>
> Thanks,
>
> Soheil Eizadi
>
>
Re: Review Request 12883: DNS value in Zone is not getting set in the
Network created for the Zone
Posted by Soheil Eizadi <se...@infoblox.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12883/#review23786
-----------------------------------------------------------
After thinking about this some more I think a better way to fix this problem is to have the UI leave this NULL and have the Network Manager fill in the value by looking up the value in the Network, if it is NULL then lookup the value in the Zone. This behavior will avoid having additional code to handle the case where the Zone DNS field is updated.
I will look at this some more and submitted another patch for this problem.
- Soheil Eizadi
On July 23, 2013, 10:03 p.m., Soheil Eizadi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12883/
> -----------------------------------------------------------
>
> (Updated July 23, 2013, 10:03 p.m.)
>
>
> Review request for cloudstack.
>
>
> Bugs: https://issues.apache.org/jira/browse/CLOUDSTACK-3754
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> DNS value in Zone is not getting set in the Network created for the Zone. Due to this the prepare and release calls to the Network Plugins are missing this information for the VMs.
>
> see Jira bug https://issues.apache.org/jira/browse/CLOUDSTACK-3754 for additional details.
>
> Administrators-MacBook-Pro-7:cloudstack seizadi$ git diff
> diff --git a/ui/scripts/zoneWizard.js b/ui/scripts/zoneWizard.js
> index cb92c4e..9885b1d 100755
> --- a/ui/scripts/zoneWizard.js
> +++ b/ui/scripts/zoneWizard.js
> @@ -3084,7 +3084,9 @@
> zoneid: args.data.returnedZone.id,
> name: 'defaultGuestNetwork',
> displaytext: 'defaultGuestNetwork',
> - networkofferingid: args.data.zone.networkOfferingId
> + networkofferingid: args.data.zone.networkOfferingId,
> + networkDomain: args.data.returnedZone.domain
> };
>
> //Advanced zone with SG
>
>
> Diffs
> -----
>
>
> Diff: https://reviews.apache.org/r/12883/diff/
>
>
> Testing
> -------
>
> Created Basic Network with Wizard and validated with FireFox debugger that value was properly set and validated with CloudMonkey that the defaultGuestNetwork created had the DNS Domain value set.
>
> I noted that there are other Ajax calls in UI that createNetwork() and don't set this field, also there is a use case where the zone value can be modified and this value would need to be updated in the attached Networks. So I would not close the Jira bug until those additional cases are fixed in the UI.
>
>
> Thanks,
>
> Soheil Eizadi
>
>
Re: Review Request 12883: DNS value in Zone is not getting set in the
Network created for the Zone
Posted by Amogh Vasekar <am...@citrix.com>.
> On July 30, 2013, 3:57 a.m., Soheil Eizadi wrote:
> > In researching this more look like there is a code in NetworkManager that will set the Network DNS Domain, given proper DNS Capabilities (Capability.AllowDnsSuffixModification) is set in the DNS Offering. I need to investigate what happens when the DNS value for Zone is modified, so perhaps the code in the prepare/release is not needed.
> >
> > // If networkDomain is not specified, take it from the global configuration
> > if (_networkModel.areServicesSupportedByNetworkOffering(networkOfferingId, Service.Dns)) {
> > Map<Network.Capability, String> dnsCapabilities = _networkModel.getNetworkOfferingServiceCapabilities
> > (_configMgr.getNetworkOffering(networkOfferingId), Service.Dns);
> > String isUpdateDnsSupported = dnsCapabilities.get(Capability.AllowDnsSuffixModification);
> > if (isUpdateDnsSupported == null || !Boolean.valueOf(isUpdateDnsSupported)) {
> > if (networkDomain != null) {
> > // TBD: NetworkOfferingId and zoneId. Send uuids instead.
> > throw new InvalidParameterValueException("Domain name change is not supported by network offering id="
> > + networkOfferingId + " in zone id=" + zoneId);
> > }
Reminder -
Hi,
This review has been pending for long. Please update to "submitted" if this has already been committed.
Thanks
- Amogh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12883/#review24205
-----------------------------------------------------------
On July 26, 2013, 10:22 p.m., Soheil Eizadi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12883/
> -----------------------------------------------------------
>
> (Updated July 26, 2013, 10:22 p.m.)
>
>
> Review request for cloudstack.
>
>
> Bugs: https://issues.apache.org/jira/browse/CLOUDSTACK-3754
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/CLOUDSTACK-3754
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> DNS value in Zone is not getting set in the Network created for the Zone. Due to this the prepare and release calls to the Network Plugins are missing this information for the VMs.
>
> see Jira bug https://issues.apache.org/jira/browse/CLOUDSTACK-3754 for additional details.
>
> diff --git a/server/src/com/cloud/network/NetworkManagerImpl.java b/server/src/com/cloud/network/NetworkManagerImpl.java
> index 8e90cb7..53715c2 100755
> --- a/server/src/com/cloud/network/NetworkManagerImpl.java
> +++ b/server/src/com/cloud/network/NetworkManagerImpl.java
> @@ -2041,6 +2041,12 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L
> dest, ReservationContext context, long nicId, NetworkVO network)
> throws InsufficientVirtualNetworkCapcityException, InsufficientAddressCapacityException,
> ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
> +
> + if (dest.getDataCenter().getNetworkType() == NetworkType.Basic) {
> + if ((network.getNetworkDomain() == null) && (dest.getDataCenter().getDomain() != null)) {
> + network.setNetworkDomain(dest.getDataCenter().getDomain());
> + }
> + }
>
> Integer networkRate = _networkModel.getNetworkRate(network.getId(), vmProfile.getId());
> NetworkGuru guru = AdapterBase.getAdapterByName(_networkGurus, network.getGuruName());
> @@ -2116,6 +2122,13 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L
> s_logger.error("NetworkGuru "+guru+" prepareForMigration failed."); // XXX: Transaction error
> }
> }
> +
> + if (dest.getDataCenter().getNetworkType() == NetworkType.Basic) {
> + if ((network.getNetworkDomain() == null) && (dest.getDataCenter().getDomain() != null)) {
> + network.setNetworkDomain(dest.getDataCenter().getDomain());
> + }
> + }
> +
> for (NetworkElement element : _networkElements) {
> if(element instanceof NetworkMigrationResponder){
> if(!((NetworkMigrationResponder) element).prepareMigration(profile, network, vm, dest, context)){
> @@ -2238,7 +2251,14 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L
> //commit the transaction before proceeding releasing nic profile on the network elements
> txn.commit();
>
> - // Perform release on network elements
> + DataCenterVO zone = _dcDao.findById(network.getDataCenterId());
> + if ( zone.getNetworkType() == NetworkType.Basic) {
> + if ((network.getNetworkDomain() == null) && (zone.getDomain() != null)) {
> + network.setNetworkDomain(zone.getDomain());
> + }
> + }
> +
> + // Perform release on network elements
> for (NetworkElement element : _networkElements) {
> if (s_logger.isDebugEnabled()) {
> s_logger.debug("Asking " + element.getName() + " to release " + nic);
>
>
> Diffs
> -----
>
>
> Diff: https://reviews.apache.org/r/12883/diff/
>
>
> Testing
> -------
>
> Validated that the defaultGuestNetwork created had the DNS Domain value set to NULL.
>
> I mades sure that calls to prepare/release to Network Element Plugins had the DNS Domain set to the Zone value.
>
>
> Thanks,
>
> Soheil Eizadi
>
>
Re: Review Request 12883: DNS value in Zone is not getting set in the
Network created for the Zone
Posted by Soheil Eizadi <se...@infoblox.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12883/#review24205
-----------------------------------------------------------
In researching this more look like there is a code in NetworkManager that will set the Network DNS Domain, given proper DNS Capabilities (Capability.AllowDnsSuffixModification) is set in the DNS Offering. I need to investigate what happens when the DNS value for Zone is modified, so perhaps the code in the prepare/release is not needed.
// If networkDomain is not specified, take it from the global configuration
if (_networkModel.areServicesSupportedByNetworkOffering(networkOfferingId, Service.Dns)) {
Map<Network.Capability, String> dnsCapabilities = _networkModel.getNetworkOfferingServiceCapabilities
(_configMgr.getNetworkOffering(networkOfferingId), Service.Dns);
String isUpdateDnsSupported = dnsCapabilities.get(Capability.AllowDnsSuffixModification);
if (isUpdateDnsSupported == null || !Boolean.valueOf(isUpdateDnsSupported)) {
if (networkDomain != null) {
// TBD: NetworkOfferingId and zoneId. Send uuids instead.
throw new InvalidParameterValueException("Domain name change is not supported by network offering id="
+ networkOfferingId + " in zone id=" + zoneId);
}
- Soheil Eizadi
On July 26, 2013, 10:22 p.m., Soheil Eizadi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12883/
> -----------------------------------------------------------
>
> (Updated July 26, 2013, 10:22 p.m.)
>
>
> Review request for cloudstack.
>
>
> Bugs: https://issues.apache.org/jira/browse/CLOUDSTACK-3754
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> DNS value in Zone is not getting set in the Network created for the Zone. Due to this the prepare and release calls to the Network Plugins are missing this information for the VMs.
>
> see Jira bug https://issues.apache.org/jira/browse/CLOUDSTACK-3754 for additional details.
>
> diff --git a/server/src/com/cloud/network/NetworkManagerImpl.java b/server/src/com/cloud/network/NetworkManagerImpl.java
> index 8e90cb7..53715c2 100755
> --- a/server/src/com/cloud/network/NetworkManagerImpl.java
> +++ b/server/src/com/cloud/network/NetworkManagerImpl.java
> @@ -2041,6 +2041,12 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L
> dest, ReservationContext context, long nicId, NetworkVO network)
> throws InsufficientVirtualNetworkCapcityException, InsufficientAddressCapacityException,
> ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
> +
> + if (dest.getDataCenter().getNetworkType() == NetworkType.Basic) {
> + if ((network.getNetworkDomain() == null) && (dest.getDataCenter().getDomain() != null)) {
> + network.setNetworkDomain(dest.getDataCenter().getDomain());
> + }
> + }
>
> Integer networkRate = _networkModel.getNetworkRate(network.getId(), vmProfile.getId());
> NetworkGuru guru = AdapterBase.getAdapterByName(_networkGurus, network.getGuruName());
> @@ -2116,6 +2122,13 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L
> s_logger.error("NetworkGuru "+guru+" prepareForMigration failed."); // XXX: Transaction error
> }
> }
> +
> + if (dest.getDataCenter().getNetworkType() == NetworkType.Basic) {
> + if ((network.getNetworkDomain() == null) && (dest.getDataCenter().getDomain() != null)) {
> + network.setNetworkDomain(dest.getDataCenter().getDomain());
> + }
> + }
> +
> for (NetworkElement element : _networkElements) {
> if(element instanceof NetworkMigrationResponder){
> if(!((NetworkMigrationResponder) element).prepareMigration(profile, network, vm, dest, context)){
> @@ -2238,7 +2251,14 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L
> //commit the transaction before proceeding releasing nic profile on the network elements
> txn.commit();
>
> - // Perform release on network elements
> + DataCenterVO zone = _dcDao.findById(network.getDataCenterId());
> + if ( zone.getNetworkType() == NetworkType.Basic) {
> + if ((network.getNetworkDomain() == null) && (zone.getDomain() != null)) {
> + network.setNetworkDomain(zone.getDomain());
> + }
> + }
> +
> + // Perform release on network elements
> for (NetworkElement element : _networkElements) {
> if (s_logger.isDebugEnabled()) {
> s_logger.debug("Asking " + element.getName() + " to release " + nic);
>
>
> Diffs
> -----
>
>
> Diff: https://reviews.apache.org/r/12883/diff/
>
>
> Testing
> -------
>
> Validated that the defaultGuestNetwork created had the DNS Domain value set to NULL.
>
> I mades sure that calls to prepare/release to Network Element Plugins had the DNS Domain set to the Zone value.
>
>
> Thanks,
>
> Soheil Eizadi
>
>
Re: Review Request 12883: DNS value in Zone is not getting set in the
Network created for the Zone
Posted by Soheil Eizadi <se...@infoblox.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12883/
-----------------------------------------------------------
(Updated July 26, 2013, 10:22 p.m.)
Review request for cloudstack.
Changes
-------
Changed Network Manager for this fix (server/src/com/cloud/network/NetworkManagerImpl.java).
Bugs: https://issues.apache.org/jira/browse/CLOUDSTACK-3754
Repository: cloudstack-git
Description (updated)
-------
DNS value in Zone is not getting set in the Network created for the Zone. Due to this the prepare and release calls to the Network Plugins are missing this information for the VMs.
see Jira bug https://issues.apache.org/jira/browse/CLOUDSTACK-3754 for additional details.
diff --git a/server/src/com/cloud/network/NetworkManagerImpl.java b/server/src/com/cloud/network/NetworkManagerImpl.java
index 8e90cb7..53715c2 100755
--- a/server/src/com/cloud/network/NetworkManagerImpl.java
+++ b/server/src/com/cloud/network/NetworkManagerImpl.java
@@ -2041,6 +2041,12 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L
dest, ReservationContext context, long nicId, NetworkVO network)
throws InsufficientVirtualNetworkCapcityException, InsufficientAddressCapacityException,
ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
+
+ if (dest.getDataCenter().getNetworkType() == NetworkType.Basic) {
+ if ((network.getNetworkDomain() == null) && (dest.getDataCenter().getDomain() != null)) {
+ network.setNetworkDomain(dest.getDataCenter().getDomain());
+ }
+ }
Integer networkRate = _networkModel.getNetworkRate(network.getId(), vmProfile.getId());
NetworkGuru guru = AdapterBase.getAdapterByName(_networkGurus, network.getGuruName());
@@ -2116,6 +2122,13 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L
s_logger.error("NetworkGuru "+guru+" prepareForMigration failed."); // XXX: Transaction error
}
}
+
+ if (dest.getDataCenter().getNetworkType() == NetworkType.Basic) {
+ if ((network.getNetworkDomain() == null) && (dest.getDataCenter().getDomain() != null)) {
+ network.setNetworkDomain(dest.getDataCenter().getDomain());
+ }
+ }
+
for (NetworkElement element : _networkElements) {
if(element instanceof NetworkMigrationResponder){
if(!((NetworkMigrationResponder) element).prepareMigration(profile, network, vm, dest, context)){
@@ -2238,7 +2251,14 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L
//commit the transaction before proceeding releasing nic profile on the network elements
txn.commit();
- // Perform release on network elements
+ DataCenterVO zone = _dcDao.findById(network.getDataCenterId());
+ if ( zone.getNetworkType() == NetworkType.Basic) {
+ if ((network.getNetworkDomain() == null) && (zone.getDomain() != null)) {
+ network.setNetworkDomain(zone.getDomain());
+ }
+ }
+
+ // Perform release on network elements
for (NetworkElement element : _networkElements) {
if (s_logger.isDebugEnabled()) {
s_logger.debug("Asking " + element.getName() + " to release " + nic);
Diffs
-----
Diff: https://reviews.apache.org/r/12883/diff/
Testing (updated)
-------
Validated that the defaultGuestNetwork created had the DNS Domain value set to NULL.
I mades sure that calls to prepare/release to Network Element Plugins had the DNS Domain set to the Zone value.
Thanks,
Soheil Eizadi