Skip to content

Commit 40e804f

Browse files
committed
Netris: address review comments on static NAT, L2 and offering form
- Match the legacy static NAT rule only when it was created for the requested public IP, on both create and delete - Roll back the dedicated L2 VPC if vNet creation fails - Hide the L2 guest type for Netris in the add offering form - Add VPN to the external provider service map for Netris only - Fix the L2 offering services javadoc
1 parent f8669aa commit 40e804f

3 files changed

Lines changed: 38 additions & 14 deletions

File tree

‎engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3182,10 +3182,9 @@ private boolean hasGuestBypassVlanOverlapCheck(final boolean bypassVlanOverlapCh
31823182
}
31833183

31843184
/**
3185-
* Checks for L2 network offering services. Only 3 cases allowed:
3185+
* Checks for L2 network offering services. Only 2 cases allowed:
31863186
* - No services
31873187
* - UserData service only, provided by ConfigDrive
3188-
* - Connectivity service only, provided by Netris
31893188
*
31903189
* @param ntwkOff network offering
31913190
*/

‎plugins/network-elements/netris/src/main/java/org/apache/cloudstack/service/NetrisApiClientImpl.java‎

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -396,13 +396,9 @@ public boolean deleteNatRule(DeleteNetrisNatRuleCommand cmd) {
396396
NatGetBody existingNatRule = netrisNatRuleExists(natRuleName);
397397
// Backward compatibility: rules created before the public-IP suffix was added use the legacy name
398398
if (existingNatRule == null && "STATICNAT".equals(cmd.getNatRuleType())) {
399-
String legacyName = getLegacyStaticNatRuleName(natRuleName);
400-
if (legacyName != null) {
401-
logger.debug("Static NAT rule not found with name '{}', falling back to legacy name '{}'", natRuleName, legacyName);
402-
existingNatRule = netrisNatRuleExists(legacyName);
403-
if (existingNatRule != null) {
404-
natRuleName = legacyName;
405-
}
399+
existingNatRule = findLegacyStaticNatRule(natRuleName, cmd.getNatIp());
400+
if (existingNatRule != null) {
401+
natRuleName = existingNatRule.getName();
406402
}
407403
}
408404
boolean ruleExists = Objects.nonNull(existingNatRule);
@@ -1170,6 +1166,14 @@ private VPCListing getOrCreateL2Vpc(NetrisCommand cmd) {
11701166
return getVpcByNameAndTenant(l2VpcName);
11711167
}
11721168

1169+
private void rollbackL2Vpc(VPCListing l2Vpc) {
1170+
try {
1171+
deleteVpcInternal(l2Vpc);
1172+
} catch (Exception e) {
1173+
logger.error("Failed to rollback L2 VPC {} after vNet creation failure - manual cleanup may be required: {}", l2Vpc.getName(), e.getMessage());
1174+
}
1175+
}
1176+
11731177
private VPCListing getVpcByNameAndTenant(String vpcName) {
11741178
try {
11751179
List<VPCListing> vpcListings = listVPCs();
@@ -1256,9 +1260,11 @@ public boolean createVnet(CreateNetrisVnetCommand cmd) {
12561260
String netrisV6IpamAllocationName = null;
12571261
String netrisV6SubnetName = null;
12581262
boolean createdIpv6Allocation = false;
1263+
boolean createdL2Vpc = false;
12591264

12601265
try {
12611266
if (isL2) {
1267+
createdL2Vpc = getVpcByNameAndTenant(getL2VpcName(cmd, cmd.getName())) == null;
12621268
associatedVpc = getOrCreateL2Vpc(cmd);
12631269
if (associatedVpc == null) {
12641270
logger.error("Failed to get or create dedicated L2 VPC to create the corresponding vNet for L2 network {}", networkName);
@@ -1309,13 +1315,17 @@ public boolean createVnet(CreateNetrisVnetCommand cmd) {
13091315
if (!isL2) {
13101316
rollbackVnetResources(associatedVpc, netrisSubnetName, netrisV6SubnetName,
13111317
createdIpv6Allocation ? netrisV6IpamAllocationName : null, networkName);
1318+
} else if (createdL2Vpc) {
1319+
rollbackL2Vpc(associatedVpc);
13121320
}
13131321
return false;
13141322
}
13151323
} catch (Exception e) {
13161324
if (!isL2 && associatedVpc != null) {
13171325
rollbackVnetResources(associatedVpc, netrisSubnetName, netrisV6SubnetName,
13181326
createdIpv6Allocation ? netrisV6IpamAllocationName : null, networkName);
1327+
} else if (isL2 && createdL2Vpc && associatedVpc != null) {
1328+
rollbackL2Vpc(associatedVpc);
13191329
}
13201330
throw new CloudRuntimeException(String.format("Failed to create Netris vNet %s", networkName), e);
13211331
}
@@ -1710,9 +1720,9 @@ public boolean createStaticNatRule(CreateOrUpdateNetrisNatCommand cmd) {
17101720
return true;
17111721
}
17121722
// Backward compatibility: rule with legacy naming convention (no public IP post-fixed) exists - don't create a duplicate
1713-
String legacyName = getLegacyStaticNatRuleName(staticNatRuleName);
1714-
if (legacyName != null && netrisNatRuleExists(legacyName) != null) {
1715-
logger.debug("Legacy static NAT rule '{}' already exists on Netris, skipping creation of '{}'", legacyName, staticNatRuleName);
1723+
NatGetBody legacyRule = findLegacyStaticNatRule(staticNatRuleName, cmd.getNatIp());
1724+
if (legacyRule != null) {
1725+
logger.debug("Legacy static NAT rule '{}' already exists on Netris, skipping creation of '{}'", legacyRule.getName(), staticNatRuleName);
17161726
return true;
17171727
}
17181728
// Create a /32 subnet for the DNAT IP
@@ -2194,6 +2204,21 @@ private String getLegacyStaticNatRuleName(String newName) {
21942204
return newName.substring(0, idx + "-STATICNAT".length());
21952205
}
21962206

2207+
/**
2208+
* Returns the legacy-named static NAT rule only if it was created for the given public IP.
2209+
*/
2210+
private NatGetBody findLegacyStaticNatRule(String newName, String natIp) {
2211+
String legacyName = getLegacyStaticNatRuleName(newName);
2212+
if (legacyName == null) {
2213+
return null;
2214+
}
2215+
NatGetBody legacyRule = netrisNatRuleExists(legacyName);
2216+
if (legacyRule == null || !(natIp + "/32").equals(legacyRule.getDestinationAddress())) {
2217+
return null;
2218+
}
2219+
return legacyRule;
2220+
}
2221+
21972222
private VPCListing getNetrisVpcResource(String netrisVpcName) {
21982223
VPCListing vpcResource = getVpcByNameAndTenant(netrisVpcName);
21992224
if (vpcResource == null) {

‎ui/src/views/offering/AddNetworkOffering.vue‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@
6060
<a-radio-button value="isolated">
6161
{{ $t('label.isolated') }}
6262
</a-radio-button>
63-
<a-radio-button value="l2" v-if="form.provider !== 'NSX'">
63+
<a-radio-button value="l2" v-if="form.provider !== 'NSX' && form.provider !== 'Netris'">
6464
{{ $t('label.l2') }}
6565
</a-radio-button>
6666
<a-radio-button value="shared" v-if="form.provider !== 'NSX' && form.provider !== 'Netris'">
@@ -1021,7 +1021,7 @@ export default {
10211021
SourceNat: externalProvider,
10221022
StaticNat: externalProvider,
10231023
PortForwarding: externalProvider,
1024-
Vpn: this.forVpc ? this.VPCVR : this.VR,
1024+
...(!isNsxProvider && { Vpn: this.forVpc ? this.VPCVR : this.VR }),
10251025
...((!isNsxProvider || this.form.nsxsupportlb) && { Lb: externalProvider })
10261026
}
10271027

0 commit comments

Comments
 (0)