Pearl1594 commented on code in PR #7479:
URL: https://github.com/apache/cloudstack/pull/7479#discussion_r1187676054
##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java:
##########
@@ -352,56 +352,57 @@ public VMTemplateVO
getKubernetesServiceTemplate(DataCenter dataCenter, Hypervis
return template;
}
- private boolean validateIsolatedNetwork(Network network, int
clusterTotalNodeCount) {
- if (Network.GuestType.Isolated.equals(network.getGuestType())) {
- if (Network.State.Allocated.equals(network.getState())) { //
Allocated networks won't have IP and rules
- return true;
+ private void validateIsolatedNetwork(Network network, int
clusterTotalNodeCount) {
+ if (!Network.GuestType.Isolated.equals(network.getGuestType())) {
+ return;
+ }
+ if (Network.State.Allocated.equals(network.getState())) { // Allocated
networks won't have IP and rules
+ return;
+ }
+ IpAddress sourceNatIp = getSourceNatIp(network);
+ if (sourceNatIp == null) {
+ throw new InvalidParameterValueException(String.format("Network
ID: %s does not have a source NAT IP associated with it. To provision a
Kubernetes Cluster, source NAT IP is required", network.getUuid()));
+ }
+ List<FirewallRuleVO> rules =
firewallRulesDao.listByIpAndPurposeAndNotRevoked(sourceNatIp.getId(),
FirewallRule.Purpose.Firewall);
+ for (FirewallRuleVO rule : rules) {
+ Integer startPort = rule.getSourcePortStart();
+ Integer endPort = rule.getSourcePortEnd();
+ if (LOGGER.isDebugEnabled()) {
+ LOGGER.debug("Network rule : " + startPort + " " + endPort);
}
- IpAddress sourceNatIp = getSourceNatIp(network);
- if (sourceNatIp == null) {
- throw new
InvalidParameterValueException(String.format("Network ID: %s does not have a
source NAT IP associated with it. To provision a Kubernetes Cluster, source NAT
IP is required", network.getUuid()));
+ if (startPort <= KubernetesClusterActionWorker.CLUSTER_API_PORT &&
KubernetesClusterActionWorker.CLUSTER_API_PORT <= endPort) {
+ throw new
InvalidParameterValueException(String.format("Network ID: %s has conflicting
firewall rules to provision Kubernetes cluster for API access",
network.getUuid()));
}
- List<FirewallRuleVO> rules =
firewallRulesDao.listByIpAndPurposeAndNotRevoked(sourceNatIp.getId(),
FirewallRule.Purpose.Firewall);
- for (FirewallRuleVO rule : rules) {
- Integer startPort = rule.getSourcePortStart();
- Integer endPort = rule.getSourcePortEnd();
- if (LOGGER.isDebugEnabled()) {
- LOGGER.debug("Network rule : " + startPort + " " +
endPort);
- }
- if (startPort <=
KubernetesClusterActionWorker.CLUSTER_API_PORT &&
KubernetesClusterActionWorker.CLUSTER_API_PORT <= endPort) {
- throw new
InvalidParameterValueException(String.format("Network ID: %s has conflicting
firewall rules to provision Kubernetes cluster for API access",
network.getUuid()));
- }
- if (startPort <=
KubernetesClusterActionWorker.CLUSTER_NODES_DEFAULT_START_SSH_PORT &&
KubernetesClusterActionWorker.CLUSTER_NODES_DEFAULT_START_SSH_PORT +
clusterTotalNodeCount <= endPort) {
- throw new
InvalidParameterValueException(String.format("Network ID: %s has conflicting
firewall rules to provision Kubernetes cluster for node VM SSH access",
network.getUuid()));
- }
+ if (startPort <=
KubernetesClusterActionWorker.CLUSTER_NODES_DEFAULT_START_SSH_PORT &&
KubernetesClusterActionWorker.CLUSTER_NODES_DEFAULT_START_SSH_PORT +
clusterTotalNodeCount <= endPort) {
+ throw new
InvalidParameterValueException(String.format("Network ID: %s has conflicting
firewall rules to provision Kubernetes cluster for node VM SSH access",
network.getUuid()));
}
- rules =
firewallRulesDao.listByIpAndPurposeAndNotRevoked(sourceNatIp.getId(),
FirewallRule.Purpose.PortForwarding);
- for (FirewallRuleVO rule : rules) {
- Integer startPort = rule.getSourcePortStart();
- Integer endPort = rule.getSourcePortEnd();
- if (LOGGER.isDebugEnabled()) {
- LOGGER.debug("Network rule : " + startPort + " " +
endPort);
- }
- if (startPort <=
KubernetesClusterActionWorker.CLUSTER_API_PORT &&
KubernetesClusterActionWorker.CLUSTER_API_PORT <= endPort) {
- throw new
InvalidParameterValueException(String.format("Network ID: %s has conflicting
port forwarding rules to provision Kubernetes cluster for API access",
network.getUuid()));
- }
- if (startPort <=
KubernetesClusterActionWorker.CLUSTER_NODES_DEFAULT_START_SSH_PORT &&
KubernetesClusterActionWorker.CLUSTER_NODES_DEFAULT_START_SSH_PORT +
clusterTotalNodeCount <= endPort) {
- throw new
InvalidParameterValueException(String.format("Network ID: %s has conflicting
port forwarding rules to provision Kubernetes cluster for node VM SSH access",
network.getUuid()));
- }
+ }
+ rules =
firewallRulesDao.listByIpAndPurposeAndNotRevoked(sourceNatIp.getId(),
FirewallRule.Purpose.PortForwarding);
Review Comment:
Can we extract L380 - L 391 , as it seems similar to L366 - L377?
##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterDestroyWorker.java:
##########
@@ -258,6 +296,14 @@ public boolean destroy() throws CloudRuntimeException {
updateKubernetesClusterEntryForGC();
throw new CloudRuntimeException(msg, e);
}
+ try {
+ releaseVpcTierPublicIpIfNeeded();
+ } catch (InsufficientAddressCapacityException e) {
Review Comment:
Not sure if Insufficient address capacity would be the most appropriate
exception type?
##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java:
##########
@@ -447,52 +447,51 @@ protected void provisionFirewallRules(final IpAddress
publicIp, final Account ac
firewallService.applyIngressFwRules(publicIp.getId(), account);
}
+ protected void provisionPublicIpPortForwardingRule(IpAddress publicIp,
Network network, Account account,
+ final long vmId, final
int sourcePort) throws NetworkRuleConflictException,
ResourceUnavailableException {
+ final long publicIpId = publicIp.getId();
+ final long networkId = network.getId();
+ final long accountId = account.getId();
+ final long domainId = account.getDomainId();
+ Nic vmNic = networkModel.getNicInNetwork(vmId, networkId);
+ final Ip vmIp = new Ip(vmNic.getIPv4Address());
+ PortForwardingRuleVO pfRule =
Transaction.execute((TransactionCallbackWithException<PortForwardingRuleVO,
NetworkRuleConflictException>) status -> {
+ PortForwardingRuleVO newRule =
+ new PortForwardingRuleVO(null, publicIpId,
+ sourcePort, sourcePort,
+ vmIp,
+ DEFAULT_SSH_PORT, DEFAULT_SSH_PORT,
+ "tcp", networkId, accountId, domainId, vmId);
+ newRule.setDisplay(true);
+ newRule.setState(FirewallRule.State.Add);
+ newRule = portForwardingRulesDao.persist(newRule);
+ return newRule;
+ });
+ rulesService.applyPortForwardingRules(publicIp.getId(), account);
+ if (LOGGER.isInfoEnabled()) {
+ LOGGER.info(String.format("Provisioned SSH port forwarding rule:
%s from port %d to 22 on %s to the VM IP : %s in Kubernetes cluster : %s",
pfRule.getUuid(), sourcePort, publicIp.getAddress().addr(), vmIp.toString(),
kubernetesCluster.getName()));
Review Comment:
nit: we could probably replace the hard coded end port (22) to the constant
DEFAULT_SSH_PORT to be safer..
--
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]
