DaanHoogland commented on code in PR #8906:
URL: https://github.com/apache/cloudstack/pull/8906#discussion_r1730973435
##########
engine/components-api/src/main/java/com/cloud/network/vpc/VpcManager.java:
##########
@@ -122,7 +122,7 @@ public interface VpcManager {
* @throws InsufficientAddressCapacityException
* @throws ConcurrentOperationException
*/
- PublicIp assignSourceNatIpAddressToVpc(Account owner, Vpc vpc) throws
InsufficientAddressCapacityException, ConcurrentOperationException;
+ PublicIp assignSourceNatIpAddressToVpc(Account owner, Vpc vpc, Long podId,
boolean forNsx) throws InsufficientAddressCapacityException,
ConcurrentOperationException;
Review Comment:
how about creating a default parameter and calling an overloaded method.
In addition to that `forNsx` seems very implementation specific; can it be
retrieved somehow from configuration and hidden from the service interface?
##########
plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java:
##########
@@ -526,24 +557,37 @@ public void createStaticNatRule(String vpcName, String
tier1GatewayName,
}
}
+ protected void deletePortForwardingNatRuleService(String ruleName, String
privatePort, String protocol) {
+ String svcName = getServiceName(ruleName, privatePort, protocol, null,
null);
+ try {
+ Services services = (Services) nsxService.apply(Services.class);
+ com.vmware.nsx_policy.model.Service servicePFRule =
services.get(svcName);
+ if (servicePFRule != null && !servicePFRule.getMarkedForDelete()
&& !BooleanUtils.toBoolean(servicePFRule.getIsDefault())) {
+ services.delete(svcName);
+ }
+ } catch (Error error) {
+ String msg = String.format("Cannot find service %s associated to
rule %s, skipping its deletion: %s",
+ svcName, ruleName, error.getMessage());
+ logger.debug(msg);
+ }
+ }
+
public void deleteNatRule(Network.Service service, String privatePort,
String protocol, String networkName, String tier1GatewayName, String ruleName) {
try {
NatRules natService = (NatRules) nsxService.apply(NatRules.class);
- logger.debug(String.format("Deleting NSX static NAT rule %s for
tier-1 gateway %s (network: %s)", ruleName, tier1GatewayName, networkName));
- // delete NAT rule
- natService.delete(tier1GatewayName, NatId.USER.name(), ruleName);
- if (service == Network.Service.PortForwarding) {
- String svcName = getServiceName(ruleName, privatePort,
protocol, null, null);
- // Delete service
- Services services = (Services)
nsxService.apply(Services.class);
- services.delete(svcName);
+ logger.debug(String.format("Deleting NSX NAT rule %s for tier-1
gateway %s (network: %s)", ruleName, tier1GatewayName, networkName));
Review Comment:
```suggestion
logger.debug("Deleting NSX NAT rule {} for tier-1 gateway {}
(network: {})", ruleName, tier1GatewayName, networkName);
```
##########
plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java:
##########
@@ -485,13 +513,16 @@ private PolicyGroupMembersListResult
getSegmentPortList(SegmentPorts segmentPort
false, null, 50L, false, null);
}
- private Long retrySegmentDeletion(SegmentPorts segmentPortsService, Long
portCount, String segmentName, String enforcementPointPath) {
- int retries = 20;
+ private Long retrySegmentDeletion(SegmentPorts segmentPortsService, String
segmentName, String enforcementPointPath, long zoneId) {
+ int retries = NsxService.NSX_API_FAILURE_RETRIES.valueIn(zoneId);
+ int waitingSecs = NsxService.NSX_API_FAILURE_INTERVAL.valueIn(zoneId);
int count = 1;
+ Long portCount;
do {
try {
- logger.info("Waiting for all port groups to be unlinked from
the segment - Attempt: " + count++ + " Waiting for 5 secs");
- Thread.sleep(5000);
+ logger.info(String.format("Waiting for all port groups to be
unlinked from the segment %s - " +
+ "Attempt: %s. Waiting for %s secs", segmentName,
count++, waitingSecs));
Review Comment:
```suggestion
logger.info("Waiting for all port groups to be unlinked from
the segment {} - " +
"Attempt: {}. Waiting for {} secs", segmentName,
count++, waitingSecs);
```
##########
plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/utils/NsxControllerUtils.java:
##########
@@ -124,6 +124,9 @@ public static String getServerPoolName(String
tier1GatewayName, long lbId) {
}
public static String getActiveMonitorProfileName(String lbServerPoolName,
String port, String protocol) {
+ if (protocol.equalsIgnoreCase("udp")) {
+ protocol = "ICMP";
+ }
Review Comment:
why is udp forbidden and why is it replaced by icmp? (next icmp + port is
returned which is not realy a thing)
##########
server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java:
##########
@@ -3161,17 +3170,21 @@ protected List<IPAddressVO>
listPublicIpsAssignedToVpc(final long accountId, fin
}
@Override
- public PublicIp assignSourceNatIpAddressToVpc(final Account owner, final
Vpc vpc) throws InsufficientAddressCapacityException,
ConcurrentOperationException {
+ public PublicIp assignSourceNatIpAddressToVpc(final Account owner, final
Vpc vpc, final Long podId, final boolean forNsx) throws
InsufficientAddressCapacityException, ConcurrentOperationException {
final long dcId = vpc.getZoneId();
- final IPAddressVO sourceNatIp =
getExistingSourceNatInVpc(owner.getId(), vpc.getId());
+ final IPAddressVO sourceNatIp =
getExistingSourceNatInVpc(owner.getId(), vpc.getId(), forNsx);
PublicIp ipToReturn = null;
if (sourceNatIp != null) {
ipToReturn = PublicIp.createFromAddrAndVlan(sourceNatIp,
_vlanDao.findById(sourceNatIp.getVlanId()));
} else {
- ipToReturn = _ipAddrMgr.assignDedicateIpAddress(owner, null,
vpc.getId(), dcId, true);
+ if (!forNsx) {
+ ipToReturn = _ipAddrMgr.assignDedicateIpAddress(owner, null,
vpc.getId(), dcId, true);
+ } else {
+ ipToReturn = _ipAddrMgr.assignPublicIpAddress(dcId, podId,
owner, Vlan.VlanType.VirtualNetwork, null, null, false, true);
+ }
Review Comment:
negative condition first is not optimal
```suggestion
if (forNsx) {
ipToReturn = _ipAddrMgr.assignPublicIpAddress(dcId, podId,
owner, Vlan.VlanType.VirtualNetwork, null, null, false, true);
} else {
ipToReturn = _ipAddrMgr.assignDedicateIpAddress(owner, null,
vpc.getId(), dcId, true);
}
```
##########
plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java:
##########
@@ -527,45 +544,77 @@ public boolean applyStaticNats(Network config, List<?
extends StaticNat> rules)
return false;
}
+ protected synchronized boolean applyPFRulesInternal(Network network,
List<PortForwardingRule> rules) {
+ return Transaction.execute((TransactionCallback<Boolean>) status -> {
+ boolean result = true;
+ for (PortForwardingRule rule : rules) {
+ IPAddressVO publicIp =
ApiDBUtils.findIpAddressById(rule.getSourceIpAddressId());
+ UserVm vm =
ApiDBUtils.findUserVmById(rule.getVirtualMachineId());
+ if (vm == null && rule.getState() !=
FirewallRule.State.Revoke) {
+ continue;
+ }
+ NsxOpObject nsxObject = getNsxOpObject(network);
+ String publicPort = getPublicPortRange(rule);
+
+ String privatePort = getPrivatePFPortRange(rule);
+
+ NsxNetworkRule networkRule = new NsxNetworkRule.Builder()
+ .setDomainId(nsxObject.getDomainId())
+ .setAccountId(nsxObject.getAccountId())
+ .setZoneId(nsxObject.getZoneId())
+ .setNetworkResourceId(nsxObject.getNetworkResourceId())
+
.setNetworkResourceName(nsxObject.getNetworkResourceName())
+ .setVpcResource(nsxObject.isVpcResource())
+ .setVmId(Objects.nonNull(vm) ? vm.getId() : 0)
+ .setVmIp(Objects.nonNull(vm) ?
vm.getPrivateIpAddress() : null)
+ .setPublicIp(publicIp.getAddress().addr())
+ .setPrivatePort(privatePort)
+ .setPublicPort(publicPort)
+ .setRuleId(rule.getId())
+
.setProtocol(rule.getProtocol().toUpperCase(Locale.ROOT))
+ .build();
+ FirewallRuleDetailVO ruleDetail =
firewallRuleDetailsDao.findDetail(rule.getId(), ApiConstants.FOR_NSX);
+ if (Arrays.asList(FirewallRule.State.Add,
FirewallRule.State.Active).contains(rule.getState())) {
+ if ((ruleDetail == null && FirewallRule.State.Add ==
rule.getState()) || (ruleDetail != null &&
!ruleDetail.getValue().equalsIgnoreCase("true"))) {
+ logger.debug(String.format("Creating port forwarding
rule on NSX for VM %s to ports %s - %s",
+ vm.getUuid(), rule.getDestinationPortStart(),
rule.getDestinationPortEnd()));
Review Comment:
```suggestion
logger.debug("Creating port forwarding rule on NSX
for VM {} to ports {} - {}",
vm.getUuid(),
rule.getDestinationPortStart(), rule.getDestinationPortEnd());
```
##########
plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java:
##########
@@ -577,9 +621,14 @@ public boolean doesPfRuleExist(String ruleName, String
tier1GatewayName) {
try {
NatRules natService = (NatRules) nsxService.apply(NatRules.class);
PolicyNatRule rule = natService.get(tier1GatewayName, NAT_ID,
ruleName);
+ logger.debug(String.format("Rule %s from Tier 1 GW %s: %s",
ruleName, tier1GatewayName,
+ rule == null ? "null" : rule.getId() + " " +
rule.getPath()));
Review Comment:
```suggestion
logger.debug("Rule {} from Tier 1 GW {}: {}", ruleName,
tier1GatewayName,
rule == null ? "null" : rule.getId() + " " +
rule.getPath());
```
##########
plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxResource.java:
##########
@@ -385,16 +393,21 @@ private NsxAnswer
executeRequest(CreateNsxPortForwardRuleCommand cmd) {
cmd.getNetworkResourceId(), cmd.isResourceVpc());
try {
String privatePort = cmd.getPrivatePort();
- String service = privatePort.contains("-") ?
nsxApiClient.getServicePath(ruleName, privatePort, cmd.getProtocol(), null,
null) :
- nsxApiClient.getNsxInfraServices(ruleName, privatePort,
cmd.getProtocol(), null, null);
+ logger.debug(String.format("Checking if rule %s exists on Tier 1
Gateway: %s", ruleName, tier1GatewayName));
Review Comment:
```suggestion
logger.debug("Checking if rule {} exists on Tier 1 Gateway: {}",
ruleName, tier1GatewayName);
```
##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java:
##########
@@ -542,10 +552,11 @@ protected void removePortForwardingRules(final IpAddress
publicIp, final Network
for (PortForwardingRuleVO pfRule : pfRules) {
if (startPort <= pfRule.getSourcePortStart() &&
pfRule.getSourcePortStart() <= endPort) {
portForwardingRulesDao.remove(pfRule.getId());
- logger.debug("The Port forwarding rule [{}] with the id [{}]
was removed.", pfRule.getName(), pfRule.getId());
+ logger.trace("Marking PF rule " + pfRule + " with Revoke
state");
Review Comment:
this log message is a bit of a regression, does it really need to change?
##########
server/src/main/java/com/cloud/network/NetworkServiceImpl.java:
##########
@@ -1752,7 +1752,7 @@ public Network createGuestNetwork(CreateNetworkCmd cmd)
throws InsufficientCapac
private void validateNetworkCreationSupported(long zoneId, String
zoneName, GuestType guestType) {
NsxProviderVO nsxProviderVO = nsxProviderDao.findByZoneId(zoneId);
- if (Objects.nonNull(nsxProviderVO) && List.of(GuestType.L2,
GuestType.Shared).contains(guestType)) {
+ if (Objects.nonNull(nsxProviderVO) &&
List.of(GuestType.L2).contains(guestType)) {
Review Comment:
```suggestion
if (Objects.nonNull(nsxProviderVO) &&
GuestType.L2.equals(guestType)) {
```
##########
plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java:
##########
@@ -199,6 +205,26 @@ public NsxApiClient(String hostname, String port, String
username, char[] passwo
nsxService = apiClient::createStub;
}
+ public boolean isNsxControllerActive() {
+ try {
+ Status statusService = (Status) nsxService.apply(Status.class);
+ ClusterStatus clusterStatus = statusService.get();
+ if (clusterStatus == null) {
+ logger.error("Cannot get NSX Cluster Status");
+ return false;
+ }
+ ControllerClusterStatus status =
clusterStatus.getControlClusterStatus();
+ if (status == null) {
+ logger.error("Cannot get NSX Controller Cluster Status");
+ return false;
+ }
+ return CLUSTER_STATUS_STABLE.equalsIgnoreCase(status.getStatus());
+ } catch (Error error) {
+ logger.error(String.format("Error checking NSX Controller Health:
%s", error.getMessage()));
Review Comment:
```suggestion
logger.error("Error checking NSX Controller Health: {}",
error.getMessage());
```
##########
plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java:
##########
@@ -526,24 +557,37 @@ public void createStaticNatRule(String vpcName, String
tier1GatewayName,
}
}
+ protected void deletePortForwardingNatRuleService(String ruleName, String
privatePort, String protocol) {
+ String svcName = getServiceName(ruleName, privatePort, protocol, null,
null);
+ try {
+ Services services = (Services) nsxService.apply(Services.class);
+ com.vmware.nsx_policy.model.Service servicePFRule =
services.get(svcName);
+ if (servicePFRule != null && !servicePFRule.getMarkedForDelete()
&& !BooleanUtils.toBoolean(servicePFRule.getIsDefault())) {
+ services.delete(svcName);
+ }
+ } catch (Error error) {
+ String msg = String.format("Cannot find service %s associated to
rule %s, skipping its deletion: %s",
+ svcName, ruleName, error.getMessage());
+ logger.debug(msg);
+ }
+ }
+
public void deleteNatRule(Network.Service service, String privatePort,
String protocol, String networkName, String tier1GatewayName, String ruleName) {
try {
NatRules natService = (NatRules) nsxService.apply(NatRules.class);
- logger.debug(String.format("Deleting NSX static NAT rule %s for
tier-1 gateway %s (network: %s)", ruleName, tier1GatewayName, networkName));
- // delete NAT rule
- natService.delete(tier1GatewayName, NatId.USER.name(), ruleName);
- if (service == Network.Service.PortForwarding) {
- String svcName = getServiceName(ruleName, privatePort,
protocol, null, null);
- // Delete service
- Services services = (Services)
nsxService.apply(Services.class);
- services.delete(svcName);
+ logger.debug(String.format("Deleting NSX NAT rule %s for tier-1
gateway %s (network: %s)", ruleName, tier1GatewayName, networkName));
+ PolicyNatRule natRule = natService.get(tier1GatewayName,
NatId.USER.name(), ruleName);
+ if (natRule != null && !natRule.getMarkedForDelete()) {
+ logger.debug(String.format("Deleting rule %s from Tier 1
Gateway %s", ruleName, tier1GatewayName));
Review Comment:
```suggestion
logger.debug("Deleting rule {} from Tier 1 Gateway {}",
ruleName, tier1GatewayName);
```
##########
plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java:
##########
@@ -527,45 +544,77 @@ public boolean applyStaticNats(Network config, List<?
extends StaticNat> rules)
return false;
}
+ protected synchronized boolean applyPFRulesInternal(Network network,
List<PortForwardingRule> rules) {
+ return Transaction.execute((TransactionCallback<Boolean>) status -> {
+ boolean result = true;
+ for (PortForwardingRule rule : rules) {
+ IPAddressVO publicIp =
ApiDBUtils.findIpAddressById(rule.getSourceIpAddressId());
+ UserVm vm =
ApiDBUtils.findUserVmById(rule.getVirtualMachineId());
+ if (vm == null && rule.getState() !=
FirewallRule.State.Revoke) {
+ continue;
+ }
+ NsxOpObject nsxObject = getNsxOpObject(network);
+ String publicPort = getPublicPortRange(rule);
+
+ String privatePort = getPrivatePFPortRange(rule);
+
+ NsxNetworkRule networkRule = new NsxNetworkRule.Builder()
+ .setDomainId(nsxObject.getDomainId())
+ .setAccountId(nsxObject.getAccountId())
+ .setZoneId(nsxObject.getZoneId())
+ .setNetworkResourceId(nsxObject.getNetworkResourceId())
+
.setNetworkResourceName(nsxObject.getNetworkResourceName())
+ .setVpcResource(nsxObject.isVpcResource())
+ .setVmId(Objects.nonNull(vm) ? vm.getId() : 0)
+ .setVmIp(Objects.nonNull(vm) ?
vm.getPrivateIpAddress() : null)
+ .setPublicIp(publicIp.getAddress().addr())
+ .setPrivatePort(privatePort)
+ .setPublicPort(publicPort)
+ .setRuleId(rule.getId())
+
.setProtocol(rule.getProtocol().toUpperCase(Locale.ROOT))
+ .build();
+ FirewallRuleDetailVO ruleDetail =
firewallRuleDetailsDao.findDetail(rule.getId(), ApiConstants.FOR_NSX);
+ if (Arrays.asList(FirewallRule.State.Add,
FirewallRule.State.Active).contains(rule.getState())) {
+ if ((ruleDetail == null && FirewallRule.State.Add ==
rule.getState()) || (ruleDetail != null &&
!ruleDetail.getValue().equalsIgnoreCase("true"))) {
+ logger.debug(String.format("Creating port forwarding
rule on NSX for VM %s to ports %s - %s",
+ vm.getUuid(), rule.getDestinationPortStart(),
rule.getDestinationPortEnd()));
+ NsxAnswer answer =
nsxService.createPortForwardRule(networkRule);
+ boolean pfRuleResult = answer.getResult();
+ if (pfRuleResult && !answer.isObjectExistent()) {
+ logger.debug(String.format("Port forwarding rule
%s created on NSX, adding detail on firewall rules details", rule.getId()));
Review Comment:
```suggestion
logger.debug("Port forwarding rule {} created on
NSX, adding detail on firewall rules details", rule.getId());
```
##########
plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java:
##########
@@ -527,45 +544,77 @@ public boolean applyStaticNats(Network config, List<?
extends StaticNat> rules)
return false;
}
+ protected synchronized boolean applyPFRulesInternal(Network network,
List<PortForwardingRule> rules) {
+ return Transaction.execute((TransactionCallback<Boolean>) status -> {
+ boolean result = true;
+ for (PortForwardingRule rule : rules) {
+ IPAddressVO publicIp =
ApiDBUtils.findIpAddressById(rule.getSourceIpAddressId());
+ UserVm vm =
ApiDBUtils.findUserVmById(rule.getVirtualMachineId());
+ if (vm == null && rule.getState() !=
FirewallRule.State.Revoke) {
+ continue;
+ }
+ NsxOpObject nsxObject = getNsxOpObject(network);
+ String publicPort = getPublicPortRange(rule);
+
+ String privatePort = getPrivatePFPortRange(rule);
+
+ NsxNetworkRule networkRule = new NsxNetworkRule.Builder()
+ .setDomainId(nsxObject.getDomainId())
+ .setAccountId(nsxObject.getAccountId())
+ .setZoneId(nsxObject.getZoneId())
+ .setNetworkResourceId(nsxObject.getNetworkResourceId())
+
.setNetworkResourceName(nsxObject.getNetworkResourceName())
+ .setVpcResource(nsxObject.isVpcResource())
+ .setVmId(Objects.nonNull(vm) ? vm.getId() : 0)
+ .setVmIp(Objects.nonNull(vm) ?
vm.getPrivateIpAddress() : null)
+ .setPublicIp(publicIp.getAddress().addr())
+ .setPrivatePort(privatePort)
+ .setPublicPort(publicPort)
+ .setRuleId(rule.getId())
+
.setProtocol(rule.getProtocol().toUpperCase(Locale.ROOT))
+ .build();
+ FirewallRuleDetailVO ruleDetail =
firewallRuleDetailsDao.findDetail(rule.getId(), ApiConstants.FOR_NSX);
+ if (Arrays.asList(FirewallRule.State.Add,
FirewallRule.State.Active).contains(rule.getState())) {
+ if ((ruleDetail == null && FirewallRule.State.Add ==
rule.getState()) || (ruleDetail != null &&
!ruleDetail.getValue().equalsIgnoreCase("true"))) {
+ logger.debug(String.format("Creating port forwarding
rule on NSX for VM %s to ports %s - %s",
+ vm.getUuid(), rule.getDestinationPortStart(),
rule.getDestinationPortEnd()));
+ NsxAnswer answer =
nsxService.createPortForwardRule(networkRule);
+ boolean pfRuleResult = answer.getResult();
+ if (pfRuleResult && !answer.isObjectExistent()) {
+ logger.debug(String.format("Port forwarding rule
%s created on NSX, adding detail on firewall rules details", rule.getId()));
+ if (ruleDetail == null && FirewallRule.State.Add
== rule.getState()) {
+ logger.debug(String.format("Adding new
firewall detail for rule %s", rule.getId()));
+ firewallRuleDetailsDao.addDetail(rule.getId(),
ApiConstants.FOR_NSX, "true", false);
+ } else {
+ logger.debug(String.format("Updating firewall
detail for rule %s", rule.getId()));
+ ruleDetail.setValue("true");
+
firewallRuleDetailsDao.update(ruleDetail.getId(), ruleDetail);
+ }
+ }
+ result &= pfRuleResult;
+ }
+ } else if (rule.getState() == FirewallRule.State.Revoke) {
+ if (ruleDetail == null || (ruleDetail != null &&
ruleDetail.getValue().equalsIgnoreCase("true"))) {
+ boolean pfRuleResult =
nsxService.deletePortForwardRule(networkRule);
+ if (pfRuleResult && ruleDetail != null) {
+ logger.debug(String.format("Updating firewall rule
detail %s for rule %s, set to false", ruleDetail.getId(), rule.getId()));
Review Comment:
```suggestion
logger.debug("Updating firewall rule detail {}
for rule {}, set to false", ruleDetail.getId(), rule.getId());
```
##########
plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java:
##########
@@ -527,45 +544,77 @@ public boolean applyStaticNats(Network config, List<?
extends StaticNat> rules)
return false;
}
+ protected synchronized boolean applyPFRulesInternal(Network network,
List<PortForwardingRule> rules) {
+ return Transaction.execute((TransactionCallback<Boolean>) status -> {
+ boolean result = true;
+ for (PortForwardingRule rule : rules) {
+ IPAddressVO publicIp =
ApiDBUtils.findIpAddressById(rule.getSourceIpAddressId());
+ UserVm vm =
ApiDBUtils.findUserVmById(rule.getVirtualMachineId());
+ if (vm == null && rule.getState() !=
FirewallRule.State.Revoke) {
+ continue;
+ }
+ NsxOpObject nsxObject = getNsxOpObject(network);
+ String publicPort = getPublicPortRange(rule);
+
+ String privatePort = getPrivatePFPortRange(rule);
+
+ NsxNetworkRule networkRule = new NsxNetworkRule.Builder()
+ .setDomainId(nsxObject.getDomainId())
+ .setAccountId(nsxObject.getAccountId())
+ .setZoneId(nsxObject.getZoneId())
+ .setNetworkResourceId(nsxObject.getNetworkResourceId())
+
.setNetworkResourceName(nsxObject.getNetworkResourceName())
+ .setVpcResource(nsxObject.isVpcResource())
+ .setVmId(Objects.nonNull(vm) ? vm.getId() : 0)
+ .setVmIp(Objects.nonNull(vm) ?
vm.getPrivateIpAddress() : null)
+ .setPublicIp(publicIp.getAddress().addr())
+ .setPrivatePort(privatePort)
+ .setPublicPort(publicPort)
+ .setRuleId(rule.getId())
+
.setProtocol(rule.getProtocol().toUpperCase(Locale.ROOT))
+ .build();
+ FirewallRuleDetailVO ruleDetail =
firewallRuleDetailsDao.findDetail(rule.getId(), ApiConstants.FOR_NSX);
+ if (Arrays.asList(FirewallRule.State.Add,
FirewallRule.State.Active).contains(rule.getState())) {
+ if ((ruleDetail == null && FirewallRule.State.Add ==
rule.getState()) || (ruleDetail != null &&
!ruleDetail.getValue().equalsIgnoreCase("true"))) {
+ logger.debug(String.format("Creating port forwarding
rule on NSX for VM %s to ports %s - %s",
+ vm.getUuid(), rule.getDestinationPortStart(),
rule.getDestinationPortEnd()));
+ NsxAnswer answer =
nsxService.createPortForwardRule(networkRule);
+ boolean pfRuleResult = answer.getResult();
+ if (pfRuleResult && !answer.isObjectExistent()) {
+ logger.debug(String.format("Port forwarding rule
%s created on NSX, adding detail on firewall rules details", rule.getId()));
+ if (ruleDetail == null && FirewallRule.State.Add
== rule.getState()) {
+ logger.debug(String.format("Adding new
firewall detail for rule %s", rule.getId()));
Review Comment:
```suggestion
logger.debug("Adding new firewall detail for
rule {}", rule.getId());
```
##########
plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java:
##########
@@ -527,45 +544,77 @@ public boolean applyStaticNats(Network config, List<?
extends StaticNat> rules)
return false;
}
+ protected synchronized boolean applyPFRulesInternal(Network network,
List<PortForwardingRule> rules) {
+ return Transaction.execute((TransactionCallback<Boolean>) status -> {
+ boolean result = true;
+ for (PortForwardingRule rule : rules) {
+ IPAddressVO publicIp =
ApiDBUtils.findIpAddressById(rule.getSourceIpAddressId());
+ UserVm vm =
ApiDBUtils.findUserVmById(rule.getVirtualMachineId());
+ if (vm == null && rule.getState() !=
FirewallRule.State.Revoke) {
+ continue;
+ }
+ NsxOpObject nsxObject = getNsxOpObject(network);
+ String publicPort = getPublicPortRange(rule);
+
+ String privatePort = getPrivatePFPortRange(rule);
+
+ NsxNetworkRule networkRule = new NsxNetworkRule.Builder()
+ .setDomainId(nsxObject.getDomainId())
+ .setAccountId(nsxObject.getAccountId())
+ .setZoneId(nsxObject.getZoneId())
+ .setNetworkResourceId(nsxObject.getNetworkResourceId())
+
.setNetworkResourceName(nsxObject.getNetworkResourceName())
+ .setVpcResource(nsxObject.isVpcResource())
+ .setVmId(Objects.nonNull(vm) ? vm.getId() : 0)
+ .setVmIp(Objects.nonNull(vm) ?
vm.getPrivateIpAddress() : null)
+ .setPublicIp(publicIp.getAddress().addr())
+ .setPrivatePort(privatePort)
+ .setPublicPort(publicPort)
+ .setRuleId(rule.getId())
+
.setProtocol(rule.getProtocol().toUpperCase(Locale.ROOT))
+ .build();
+ FirewallRuleDetailVO ruleDetail =
firewallRuleDetailsDao.findDetail(rule.getId(), ApiConstants.FOR_NSX);
+ if (Arrays.asList(FirewallRule.State.Add,
FirewallRule.State.Active).contains(rule.getState())) {
+ if ((ruleDetail == null && FirewallRule.State.Add ==
rule.getState()) || (ruleDetail != null &&
!ruleDetail.getValue().equalsIgnoreCase("true"))) {
+ logger.debug(String.format("Creating port forwarding
rule on NSX for VM %s to ports %s - %s",
+ vm.getUuid(), rule.getDestinationPortStart(),
rule.getDestinationPortEnd()));
+ NsxAnswer answer =
nsxService.createPortForwardRule(networkRule);
+ boolean pfRuleResult = answer.getResult();
+ if (pfRuleResult && !answer.isObjectExistent()) {
+ logger.debug(String.format("Port forwarding rule
%s created on NSX, adding detail on firewall rules details", rule.getId()));
+ if (ruleDetail == null && FirewallRule.State.Add
== rule.getState()) {
+ logger.debug(String.format("Adding new
firewall detail for rule %s", rule.getId()));
+ firewallRuleDetailsDao.addDetail(rule.getId(),
ApiConstants.FOR_NSX, "true", false);
+ } else {
+ logger.debug(String.format("Updating firewall
detail for rule %s", rule.getId()));
Review Comment:
```suggestion
logger.debug("Updating firewall detail for
rule ", rule.getId());
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
