Skip to content

Commit

Permalink
[BACKPORT 2.21.1.3269][PLAT-13463] Added nodes have wrong master addr…
Browse files Browse the repository at this point in the history
…ess assignments

Summary:
Original diff - https://phorge.dev.yugabyte.com/D33954 (a5f09eb)

Use secondary IP for dual NIC for YBM.

Test Plan: Manually create a Universe.

Reviewers: yshchetinin, cwang, sanketh, daniel

Reviewed By: daniel

Subscribers: yugaware

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D33999
  • Loading branch information
nkhogen committed Apr 10, 2024
1 parent 058b7b9 commit af2fdc8
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,8 @@ public Supplier<String> getMasterAddrsSupplier() {
return universe.getHostPortsString(
universe.getNodes().stream().filter(n -> nodes.contains(n)).collect(Collectors.toSet()),
ServerType.MASTER,
PortType.RPC);
PortType.RPC,
config.getBoolean("yb.cloud.enabled"));
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.yugabyte.yw.commissioner.tasks.params.NodeTaskParams;
import com.yugabyte.yw.common.Util;
import com.yugabyte.yw.common.config.UniverseConfKeys;
import com.yugabyte.yw.common.gflags.GFlagsUtil;
import com.yugabyte.yw.common.services.config.YbClientConfig;
import com.yugabyte.yw.common.services.config.YbClientConfigFactory;
import com.yugabyte.yw.models.Universe;
Expand Down Expand Up @@ -115,15 +116,10 @@ public void run() {
}
// If the cluster has a secondary IP, we want to ensure that we use the correct addresses.
// The ipToUse is the address that we need to add to the config.
boolean hasSecondaryIp =
node.cloudInfo.secondary_private_ip != null
&& !node.cloudInfo.secondary_private_ip.equals("null");
boolean shouldUseSecondary =
universe.getConfig().getOrDefault(Universe.DUAL_NET_LEGACY, "true").equals("false");
GFlagsUtil.isUseSecondaryIP(universe, node, config.getBoolean("yb.cloud.enabled"));
String ipToUse =
hasSecondaryIp && shouldUseSecondary
? node.cloudInfo.secondary_private_ip
: node.cloudInfo.private_ip;
shouldUseSecondary ? node.cloudInfo.secondary_private_ip : node.cloudInfo.private_ip;
String certificate = universe.getCertificateNodetoNode();
YbClientConfig config = ybClientConfigFactory.create(masterAddresses, certificate);
// The call changeMasterConfig is not idempotent. The client library internally keeps retrying
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,10 @@ private List<String> getConfigureSubCommand(AnsibleConfigureServers.Params taskP
List<String> subcommand = new ArrayList<>();
String masterAddresses = taskParam.getMasterAddrsOverride();
if (StringUtils.isBlank(masterAddresses)) {
masterAddresses = universe.getMasterAddresses();
// TODO This seems unused but keep the master addresses same as those ones set via GFlags.
masterAddresses =
universe.getMasterAddresses(
false /*mastersQueryable*/, config.getBoolean("yb.cloud.enabled"));
}
if (StringUtils.isBlank(masterAddresses)) {
log.warn("No valid masters found during configure for {}.", taskParam.getUniverseUUID());
Expand Down
17 changes: 6 additions & 11 deletions managed/src/main/java/com/yugabyte/yw/models/Universe.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.yugabyte.yw.common.RedactingService.RedactionTarget;
import com.yugabyte.yw.common.Util;
import com.yugabyte.yw.common.concurrent.KeyLock;
import com.yugabyte.yw.common.gflags.GFlagsUtil;
import com.yugabyte.yw.common.inject.StaticInjectorHolder;
import com.yugabyte.yw.common.rbac.PermissionInfo.ResourceType;
import com.yugabyte.yw.common.rbac.RoleBindingUtil;
Expand Down Expand Up @@ -781,12 +782,12 @@ public String getMasterAddresses(boolean mastersQueryable) {
* @return a comma separated string of master 'host:port' or, if masters are not queryable, an
* empty string.
*/
public String getMasterAddresses(boolean mastersQueryable, boolean getSecondary) {
public String getMasterAddresses(boolean mastersQueryable, boolean useSecondaryIfEnabled) {
List<NodeDetails> masters = getMasters();
if (mastersQueryable && !verifyMastersAreQueryable(masters)) {
return "";
}
return getHostPortsString(masters, ServerType.MASTER, PortType.RPC, getSecondary);
return getHostPortsString(masters, ServerType.MASTER, PortType.RPC, useSecondaryIfEnabled);
}

/**
Expand Down Expand Up @@ -893,22 +894,16 @@ public String getHostPortsString(
}

// Helper API to create the based on the server type.
private String getHostPortsString(
public String getHostPortsString(
Collection<NodeDetails> serverNodes,
ServerType type,
PortType portType,
boolean getSecondary) {
boolean useSecondaryIfEnabled) {
StringBuilder servers = new StringBuilder();
for (NodeDetails node : serverNodes) {
// Only get secondary if dual net legacy is false.
boolean shouldGetSecondary =
this.getConfig().getOrDefault(DUAL_NET_LEGACY, "true").equals("false") && getSecondary;
boolean shouldGetSecondary = GFlagsUtil.isUseSecondaryIP(this, node, useSecondaryIfEnabled);
String nodeIp =
shouldGetSecondary ? node.cloudInfo.secondary_private_ip : node.cloudInfo.private_ip;
// In case the secondary IP is null, just re-assign to primary.
if (nodeIp == null || nodeIp.equals("null")) {
nodeIp = node.cloudInfo.private_ip;
}
if (nodeIp != null) {
int port = 0;
switch (type) {
Expand Down

0 comments on commit af2fdc8

Please sign in to comment.