Skip to content

Commit 20306d6

Browse files
authored
Allow creating atmost 1 physical network with null tag (#6781)
1 parent c2b75f4 commit 20306d6

File tree

6 files changed

+509
-61
lines changed

6 files changed

+509
-61
lines changed

engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ protected void init() {
129129
AllFieldsSearch.and("account", AllFieldsSearch.entity().getAccountId(), Op.EQ);
130130
AllFieldsSearch.and("related", AllFieldsSearch.entity().getRelated(), Op.EQ);
131131
AllFieldsSearch.and("guestType", AllFieldsSearch.entity().getGuestType(), Op.EQ);
132-
AllFieldsSearch.and("physicalNetwork", AllFieldsSearch.entity().getPhysicalNetworkId(), Op.EQ);
132+
AllFieldsSearch.and("physicalNetworkId", AllFieldsSearch.entity().getPhysicalNetworkId(), Op.EQ);
133133
AllFieldsSearch.and("broadcastUri", AllFieldsSearch.entity().getBroadcastUri(), Op.EQ);
134134
AllFieldsSearch.and("vpcId", AllFieldsSearch.entity().getVpcId(), Op.EQ);
135135
AllFieldsSearch.and("aclId", AllFieldsSearch.entity().getNetworkACLId(), Op.EQ);

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6720,19 +6720,10 @@ public Pair<List<? extends NetworkOffering>, Integer> searchForNetworkOfferings(
67206720
final Boolean sourceNatSupported = cmd.getSourceNatSupported();
67216721
final List<String> pNtwkTags = new ArrayList<String>();
67226722
boolean checkForTags = false;
6723+
boolean allowNullTag = false;
67236724
if (zone != null) {
6724-
final List<PhysicalNetworkVO> pNtwks = _physicalNetworkDao.listByZoneAndTrafficType(zoneId, TrafficType.Guest);
6725-
if (pNtwks.size() > 1) {
6726-
checkForTags = true;
6727-
// go through tags
6728-
for (final PhysicalNetworkVO pNtwk : pNtwks) {
6729-
final List<String> pNtwkTag = pNtwk.getTags();
6730-
if (pNtwkTag == null || pNtwkTag.isEmpty()) {
6731-
throw new CloudRuntimeException("Tags are not defined for physical network in the zone id=" + zoneId);
6732-
}
6733-
pNtwkTags.addAll(pNtwkTag);
6734-
}
6735-
}
6725+
allowNullTag = allowNetworkOfferingWithNullTag(zoneId, pNtwkTags);
6726+
checkForTags = !pNtwkTags.isEmpty() || allowNullTag;
67366727
}
67376728

67386729
// filter by supported services
@@ -6762,10 +6753,8 @@ public Pair<List<? extends NetworkOffering>, Integer> searchForNetworkOfferings(
67626753
boolean addOffering = true;
67636754
List<Service> checkForProviders = new ArrayList<Service>();
67646755

6765-
if (checkForTags) {
6766-
if (!pNtwkTags.contains(offering.getTags())) {
6767-
continue;
6768-
}
6756+
if (checkForTags && ! checkNetworkOfferingTags(pNtwkTags, allowNullTag, offering.getTags())) {
6757+
continue;
67696758
}
67706759

67716760
if (listBySupportedServices) {
@@ -6815,6 +6804,28 @@ public Pair<List<? extends NetworkOffering>, Integer> searchForNetworkOfferings(
68156804
}
68166805
}
68176806

6807+
private boolean allowNetworkOfferingWithNullTag(Long zoneId, List<String> allPhysicalNetworkTags) {
6808+
boolean allowNullTag = false;
6809+
final List<PhysicalNetworkVO> physicalNetworks = _physicalNetworkDao.listByZoneAndTrafficType(zoneId, TrafficType.Guest);
6810+
for (final PhysicalNetworkVO physicalNetwork : physicalNetworks) {
6811+
final List<String> physicalNetworkTags = physicalNetwork.getTags();
6812+
if (CollectionUtils.isEmpty(physicalNetworkTags)) {
6813+
if (!allowNullTag) {
6814+
allowNullTag = true;
6815+
} else {
6816+
throw new CloudRuntimeException("There are more than 1 physical network with empty tag in the zone id=" + zoneId);
6817+
}
6818+
} else {
6819+
allPhysicalNetworkTags.addAll(physicalNetworkTags);
6820+
}
6821+
}
6822+
return allowNullTag;
6823+
}
6824+
6825+
private boolean checkNetworkOfferingTags(List<String> physicalNetworkTags, boolean allowNullTag, String offeringTags) {
6826+
return (offeringTags != null || allowNullTag) && (offeringTags == null || physicalNetworkTags.contains(offeringTags));
6827+
}
6828+
68186829
@Override
68196830
public boolean isOfferingForVpc(final NetworkOffering offering) {
68206831
return offering.isForVpc();

server/src/main/java/com/cloud/network/NetworkModelImpl.java

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,26 +1189,31 @@ public long findPhysicalNetworkId(long zoneId, String tag, TrafficType trafficTy
11891189
}
11901190

11911191
if (pNtwks.size() > 1) {
1192-
if (tag == null) {
1193-
throw new InvalidParameterValueException("More than one physical networks exist in zone id=" + zoneId +
1194-
" and no tags are specified in order to make a choice");
1195-
}
1192+
return getPhysicalNetworkId(zoneId, pNtwks, tag);
1193+
} else {
1194+
return pNtwks.get(0).getId();
1195+
}
1196+
}
11961197

1197-
Long pNtwkId = null;
1198-
for (PhysicalNetwork pNtwk : pNtwks) {
1199-
if (pNtwk.getTags().contains(tag)) {
1200-
s_logger.debug("Found physical network id=" + pNtwk.getId() + " based on requested tags " + tag);
1201-
pNtwkId = pNtwk.getId();
1202-
break;
1198+
private Long getPhysicalNetworkId(long zoneId, List<PhysicalNetworkVO> pNtwks, String tag) {
1199+
Long pNtwkId = null;
1200+
for (PhysicalNetwork pNtwk : pNtwks) {
1201+
if (tag == null && pNtwk.getTags().isEmpty()) {
1202+
s_logger.debug("Found physical network id=" + pNtwk.getId() + " with null tag");
1203+
if (pNtwkId != null) {
1204+
throw new CloudRuntimeException("There is more than 1 physical network with empty tag in the zone id=" + zoneId);
12031205
}
1206+
pNtwkId = pNtwk.getId();
1207+
} else if (tag != null && pNtwk.getTags().contains(tag)) {
1208+
s_logger.debug("Found physical network id=" + pNtwk.getId() + " based on requested tags " + tag);
1209+
pNtwkId = pNtwk.getId();
1210+
break;
12041211
}
1205-
if (pNtwkId == null) {
1206-
throw new InvalidParameterValueException("Unable to find physical network which match the tags " + tag);
1207-
}
1208-
return pNtwkId;
1209-
} else {
1210-
return pNtwks.get(0).getId();
12111212
}
1213+
if (pNtwkId == null) {
1214+
throw new InvalidParameterValueException("Unable to find physical network which match the tags " + tag);
1215+
}
1216+
return pNtwkId;
12121217
}
12131218

12141219
@Override

server/src/main/java/com/cloud/network/NetworkServiceImpl.java

Lines changed: 46 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3950,6 +3950,12 @@ public PhysicalNetwork updatePhysicalNetwork(Long id, String networkSpeed, List<
39503950
throw new InvalidParameterException("Unable to support more than one tag on network yet");
39513951
}
39523952

3953+
// If tags are null, then check if there are any other networks with null tags
3954+
// of the same traffic type. If so then dont update the tags
3955+
if (tags != null && tags.size() == 0) {
3956+
checkForPhysicalNetworksWithoutTag(network);
3957+
}
3958+
39533959
PhysicalNetwork.State networkState = null;
39543960
if (state != null && !state.isEmpty()) {
39553961
try {
@@ -3980,6 +3986,18 @@ public PhysicalNetwork updatePhysicalNetwork(Long id, String networkSpeed, List<
39803986

39813987
}
39823988

3989+
private void checkForPhysicalNetworksWithoutTag(PhysicalNetworkVO network) {
3990+
// Get all physical networks according to traffic type
3991+
Pair<List<PhysicalNetworkTrafficTypeVO>, Integer> result = _pNTrafficTypeDao
3992+
.listAndCountBy(network.getId());
3993+
if (result.second() > 0) {
3994+
for (PhysicalNetworkTrafficTypeVO physicalNetworkTrafficTypeVO : result.first()) {
3995+
TrafficType trafficType = physicalNetworkTrafficTypeVO.getTrafficType();
3996+
checkForPhysicalNetworksWithoutTag(network, trafficType);
3997+
}
3998+
}
3999+
}
4000+
39834001
@DB
39844002
public void addOrRemoveVnets(String[] listOfRanges, final PhysicalNetworkVO network) {
39854003
List<String> addVnets = null;
@@ -4849,36 +4867,30 @@ public PhysicalNetworkServiceProvider getCreatedPhysicalNetworkServiceProvider(L
48494867

48504868
@Override
48514869
public long findPhysicalNetworkId(long zoneId, String tag, TrafficType trafficType) {
4852-
List<PhysicalNetworkVO> pNtwks = new ArrayList<PhysicalNetworkVO>();
4853-
if (trafficType != null) {
4854-
pNtwks = _physicalNetworkDao.listByZoneAndTrafficType(zoneId, trafficType);
4855-
} else {
4856-
pNtwks = _physicalNetworkDao.listByZone(zoneId);
4857-
}
4858-
4859-
if (pNtwks.isEmpty()) {
4860-
throw new InvalidParameterValueException("Unable to find physical network in zone id=" + zoneId);
4861-
}
4870+
return _networkModel.findPhysicalNetworkId(zoneId, tag, trafficType);
4871+
}
48624872

4863-
if (pNtwks.size() > 1) {
4864-
if (tag == null) {
4865-
throw new InvalidParameterValueException("More than one physical networks exist in zone id=" + zoneId + " and no tags are specified in order to make a choice");
4866-
}
4873+
/**
4874+
* Function to check if there are any physical networks with traffic type of "trafficType"
4875+
* and check their tags. If there is more than one network with null tags then throw exception
4876+
* @param physicalNetwork
4877+
* @param trafficType
4878+
*/
4879+
private void checkForPhysicalNetworksWithoutTag(PhysicalNetworkVO physicalNetwork, TrafficType trafficType) {
4880+
int networkWithoutTagCount = 0;
4881+
List<PhysicalNetworkVO> physicalNetworkVOList = _physicalNetworkDao
4882+
.listByZoneAndTrafficType(physicalNetwork.getDataCenterId(), trafficType);
48674883

4868-
Long pNtwkId = null;
4869-
for (PhysicalNetwork pNtwk : pNtwks) {
4870-
if (pNtwk.getTags().contains(tag)) {
4871-
s_logger.debug("Found physical network id=" + pNtwk.getId() + " based on requested tags " + tag);
4872-
pNtwkId = pNtwk.getId();
4873-
break;
4874-
}
4884+
for (PhysicalNetworkVO physicalNetworkVO : physicalNetworkVOList) {
4885+
List<String> tags = physicalNetworkVO.getTags();
4886+
if (CollectionUtils.isEmpty(tags)) {
4887+
networkWithoutTagCount++;
48754888
}
4876-
if (pNtwkId == null) {
4877-
throw new InvalidParameterValueException("Unable to find physical network which match the tags " + tag);
4878-
}
4879-
return pNtwkId;
4880-
} else {
4881-
return pNtwks.get(0).getId();
4889+
}
4890+
if (networkWithoutTagCount > 0) {
4891+
s_logger.error("Number of physical networks without tags are " + networkWithoutTagCount);
4892+
throw new CloudRuntimeException("There are more than 1 physical network without tags in the zone= " +
4893+
physicalNetwork.getDataCenterId());
48824894
}
48834895
}
48844896

@@ -4929,6 +4941,13 @@ public PhysicalNetworkTrafficType addTrafficTypeToPhysicalNetwork(Long physicalN
49294941
}
49304942
}
49314943

4944+
// Check if there are more than 1 physical network with null tags in same traffic type.
4945+
// If so then dont allow to add traffic type.
4946+
List<String> tags = network.getTags();
4947+
if (CollectionUtils.isEmpty(tags)) {
4948+
checkForPhysicalNetworksWithoutTag(network, trafficType);
4949+
}
4950+
49324951
try {
49334952
// Create the new traffic type in the database
49344953
if (xenLabel == null) {

0 commit comments

Comments
 (0)