-
Notifications
You must be signed in to change notification settings - Fork 1.4k
NSX Integration fixes #8906
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
NSX Integration fixes #8906
Changes from 22 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
523620f
Prevent addition of duplicate PF rules on scale up and no rules left …
Pearl1594 3579806
fix missing dependency injection
Pearl1594 f228c7a
NSX: Fix concurrency issues on port forwarding rules deletion (#37)
nvazquez a899648
CKS: Externalize control and worker node setup wait time and installa…
Pearl1594 968235a
NSX: Add shared network support (#41)
Pearl1594 bcc8ff2
NSX: Fix number of physical networks for Guest traffic checks and lef…
nvazquez f44b8aa
fix logger
Pearl1594 e79d771
NSX: Handle CheckHealthCommand to avoid host disconnection and errors…
nvazquez 4e57cc9
Merge branch 'main' of https://github.com/apache/cloudstack into nsx-…
Pearl1594 0163da4
fix logger
Pearl1594 0903ba6
Merge branch 'main' of https://github.com/apache/cloudstack into nsx-…
Pearl1594 2726987
Update UDP active monitor to ICMP
Pearl1594 c393973
Fix NPE on restarting VPC with additional public IPs
Pearl1594 587f6fe
NSX / VPC: Reuse Source NAT IP from systemVM range on restarts
Pearl1594 459c23b
CKS: Public IP not found for VPC networks
Pearl1594 89aa0a5
Externalize retries and inverval for NSX segment deletion (#67)
nvazquez 9733984
remove unused import
Pearl1594 9c1df1d
remove duplicate imports
Pearl1594 7e0007b
Merge branch 'main' of https://github.com/apache/cloudstack into nsx-…
Pearl1594 d2c386c
remove unused import
Pearl1594 0a31b13
revert externalizing cks settings
Pearl1594 70f70b2
fix test
Pearl1594 665b698
Refactor log messages
nvazquez 738f934
Address comments
nvazquez 66ee5da
Merge branch 'main' into nsx-integration-fixes
nvazquez 09159af
Fix issue caused due to forward merge: 90fe1d
nvazquez d1a6038
Merge branch 'main' into nsx-integration-fixes
yadvr File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,31 @@ | |
|
|
||
| package com.cloud.kubernetes.cluster.actionworkers; | ||
|
|
||
| import static com.cloud.utils.NumbersUtil.toHumanReadableSize; | ||
|
|
||
| import java.io.File; | ||
| import java.io.IOException; | ||
| import java.util.ArrayList; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.concurrent.ConcurrentHashMap; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| import javax.inject.Inject; | ||
|
|
||
| import com.cloud.network.rules.FirewallManager; | ||
| import com.cloud.offering.NetworkOffering; | ||
| import com.cloud.offerings.dao.NetworkOfferingDao; | ||
| import org.apache.cloudstack.api.ApiConstants; | ||
| import org.apache.cloudstack.api.BaseCmd; | ||
| import org.apache.cloudstack.api.command.user.firewall.CreateFirewallRuleCmd; | ||
| import org.apache.cloudstack.api.command.user.network.CreateNetworkACLCmd; | ||
| import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd; | ||
| import org.apache.commons.codec.binary.Base64; | ||
| import org.apache.commons.collections.CollectionUtils; | ||
| import org.apache.commons.lang3.StringUtils; | ||
|
|
||
| import com.cloud.capacity.CapacityManager; | ||
| import com.cloud.dc.ClusterDetailsDao; | ||
| import com.cloud.dc.ClusterDetailsVO; | ||
|
|
@@ -61,9 +86,7 @@ | |
| import com.cloud.network.vpc.NetworkACLItemDao; | ||
| import com.cloud.network.vpc.NetworkACLItemVO; | ||
| import com.cloud.network.vpc.NetworkACLService; | ||
| import com.cloud.offering.NetworkOffering; | ||
| import com.cloud.offering.ServiceOffering; | ||
| import com.cloud.offerings.dao.NetworkOfferingDao; | ||
| import com.cloud.resource.ResourceManager; | ||
| import com.cloud.storage.Volume; | ||
| import com.cloud.storage.VolumeApiService; | ||
|
|
@@ -88,29 +111,9 @@ | |
| import com.cloud.vm.VmDetailConstants; | ||
| import com.cloud.vm.dao.VMInstanceDao; | ||
| import org.apache.cloudstack.api.ApiCommandResourceType; | ||
| import org.apache.cloudstack.api.ApiConstants; | ||
| import org.apache.cloudstack.api.BaseCmd; | ||
| import org.apache.cloudstack.api.command.user.firewall.CreateFirewallRuleCmd; | ||
| import org.apache.cloudstack.api.command.user.network.CreateNetworkACLCmd; | ||
| import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd; | ||
| import org.apache.cloudstack.context.CallContext; | ||
| import org.apache.commons.codec.binary.Base64; | ||
| import org.apache.commons.collections.CollectionUtils; | ||
| import org.apache.commons.lang3.StringUtils; | ||
| import org.apache.logging.log4j.Level; | ||
|
|
||
| import javax.inject.Inject; | ||
| import java.io.File; | ||
| import java.io.IOException; | ||
| import java.util.ArrayList; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.concurrent.ConcurrentHashMap; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| import static com.cloud.utils.NumbersUtil.toHumanReadableSize; | ||
|
|
||
| public class KubernetesClusterResourceModifierActionWorker extends KubernetesClusterActionWorker { | ||
|
|
||
| @Inject | ||
|
|
@@ -134,6 +137,8 @@ public class KubernetesClusterResourceModifierActionWorker extends KubernetesClu | |
| @Inject | ||
| protected RulesService rulesService; | ||
| @Inject | ||
| protected FirewallManager firewallManager; | ||
| @Inject | ||
| protected PortForwardingRulesDao portForwardingRulesDao; | ||
| @Inject | ||
| protected ResourceManager resourceManager; | ||
|
|
@@ -169,6 +174,7 @@ private String getKubernetesNodeConfig(final String joinIp, final boolean ejectI | |
| final String joinIpKey = "{{ k8s_control_node.join_ip }}"; | ||
| final String clusterTokenKey = "{{ k8s_control_node.cluster.token }}"; | ||
| final String ejectIsoKey = "{{ k8s.eject.iso }}"; | ||
|
|
||
| String pubKey = "- \"" + configurationDao.getValue("ssh.publickey") + "\""; | ||
| String sshKeyPair = kubernetesCluster.getKeyPair(); | ||
| if (StringUtils.isNotEmpty(sshKeyPair)) { | ||
|
|
@@ -181,7 +187,6 @@ private String getKubernetesNodeConfig(final String joinIp, final boolean ejectI | |
| k8sNodeConfig = k8sNodeConfig.replace(joinIpKey, joinIp); | ||
| k8sNodeConfig = k8sNodeConfig.replace(clusterTokenKey, KubernetesClusterUtil.generateClusterToken(kubernetesCluster)); | ||
| k8sNodeConfig = k8sNodeConfig.replace(ejectIsoKey, String.valueOf(ejectIso)); | ||
|
|
||
| k8sNodeConfig = updateKubeConfigWithRegistryDetails(k8sNodeConfig); | ||
|
|
||
| return k8sNodeConfig; | ||
|
|
@@ -522,17 +527,22 @@ protected FirewallRule removeSshFirewallRule(final IpAddress publicIp) { | |
|
|
||
| protected void removePortForwardingRules(final IpAddress publicIp, final Network network, final Account account, final List<Long> removedVMIds) throws ResourceUnavailableException { | ||
| if (!CollectionUtils.isEmpty(removedVMIds)) { | ||
| List<PortForwardingRuleVO> pfRules = new ArrayList<>(); | ||
| List<PortForwardingRuleVO> revokedRules = new ArrayList<>(); | ||
| for (Long vmId : removedVMIds) { | ||
| List<PortForwardingRuleVO> pfRules = portForwardingRulesDao.listByNetwork(network.getId()); | ||
| pfRules.addAll(portForwardingRulesDao.listByNetwork(network.getId())); | ||
| for (PortForwardingRuleVO pfRule : pfRules) { | ||
| if (pfRule.getVirtualMachineId() == vmId) { | ||
| portForwardingRulesDao.remove(pfRule.getId()); | ||
| logger.trace("Marking PF rule {} with Revoke state", pfRule); | ||
| pfRule.setState(FirewallRule.State.Revoke); | ||
| revokedRules.add(pfRule); | ||
| logger.debug("The Port forwarding rule [%s] with the id [%s] was removed.", pfRule.getName(), pfRule.getId()); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| rulesService.applyPortForwardingRules(publicIp.getId(), account); | ||
| firewallManager.applyRules(revokedRules, false, true); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -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"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this log message is a bit of a regression, does it really need to change?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, refactored |
||
| pfRule.setState(FirewallRule.State.Revoke); | ||
| } | ||
| } | ||
| rulesService.applyPortForwardingRules(publicIp.getId(), account); | ||
| firewallManager.applyRules(pfRules, false, true); | ||
| } | ||
|
|
||
| protected void removeLoadBalancingRule(final IpAddress publicIp, final Network network, | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about creating a default parameter and calling an overloaded method.
In addition to that
forNsxseems very implementation specific; can it be retrieved somehow from configuration and hidden from the service interface?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DaanHoogland thanks I've refactored it to remove the boolean from the service interface