Skip to content

Commit

Permalink
[BACKPORT 2.20][PLAT-13438]: Add support for preview gflags
Browse files Browse the repository at this point in the history
Summary:
Added support for preview gFlags in YBA.
As part of this change,
 -  We have added validation in BE to ensure that the user should set the preview flag name in the allowed_preview_flags_csv flag.
 -  Added changes in the validate gflags API used by UI to catch this issue earlier in UI, even before the actual upgrade trigger.
 -  While setting the gflags in memory, we will first set the allowed_preview_flags_csv if it exists and then put the remaining gflags so that DB servers can accept them.

Original diff/commit: D36687/4ee3511be6

Test Plan:
Tested manually by setting a preview flag successfully when the flag is passed in allowed_preview_flags_csv and verified that the request is rejected if we do not put it in allowed_preview_flags_csv.
Also, verified that allowed_preview_flags_csv is set first when exists during a non-restart upgrade through logs and Java debugger.

Reviewers: yshchetinin, sanketh

Reviewed By: yshchetinin

Subscribers: yugaware

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D36784
  • Loading branch information
vipul-yb committed Jul 24, 2024
1 parent ecc2d23 commit bbe20d7
Show file tree
Hide file tree
Showing 12 changed files with 404 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.yugabyte.yw.commissioner.BaseTaskDependencies;
import com.yugabyte.yw.commissioner.tasks.UniverseTaskBase.ServerType;
import com.yugabyte.yw.commissioner.tasks.params.ServerSubTaskParams;
import com.yugabyte.yw.common.gflags.GFlagsUtil;
import java.util.Map;
import java.util.Map.Entry;
import javax.inject.Inject;
Expand Down Expand Up @@ -65,23 +66,31 @@ public void run() {
String flagToSet = isTserverTask ? TSERVER_MASTER_ADDR_FLAG : MASTER_MASTER_ADDR_FLAG;
gflags = ImmutableMap.of(flagToSet, masterAddresses);
}
if (gflags == null) {
throw new IllegalArgumentException("Gflags cannot be null during a setFlag operation.");
}

YBClient client = getClient();
try {
if (gflags == null) {
throw new IllegalArgumentException("Gflags cannot be null during a setFlag operation.");
// allowed_preview_flags_csv should be set first in order to set the preview flags.
if (gflags.containsKey(GFlagsUtil.ALLOWED_PREVIEW_FLAGS_CSV)) {
log.info(
"Setting Allowed Preview Flags for {} on node {}",
taskParams().serverType,
taskParams().nodeName);
setFlag(
client,
GFlagsUtil.ALLOWED_PREVIEW_FLAGS_CSV,
gflags.get(GFlagsUtil.ALLOWED_PREVIEW_FLAGS_CSV),
hp);
gflags.remove(GFlagsUtil.ALLOWED_PREVIEW_FLAGS_CSV);
log.info(
"Setting remaining flags for {} on node {}",
taskParams().serverType,
taskParams().nodeName);
}
for (Entry<String, String> gflag : gflags.entrySet()) {
boolean setSuccess =
client.setFlag(hp, gflag.getKey(), gflag.getValue(), taskParams().force);
if (!setSuccess) {
throw new RuntimeException(
"Could not set gflag "
+ gflag
+ " for "
+ taskParams().serverType
+ " on node "
+ taskParams().nodeName);
}
setFlag(client, gflag.getKey(), gflag.getValue(), hp);
}
} catch (Exception e) {
log.error("{} hit error : {}", getName(), e.getMessage());
Expand All @@ -90,4 +99,18 @@ public void run() {
closeClient(client);
}
}

private void setFlag(YBClient client, String gflag, String value, HostAndPort hp)
throws Exception {
boolean setSuccess = client.setFlag(hp, gflag, value, taskParams().force);
if (!setSuccess) {
throw new RuntimeException(
"Could not set gflag "
+ gflag
+ " for "
+ taskParams().serverType
+ " on node "
+ taskParams().nodeName);
}
}
}
4 changes: 2 additions & 2 deletions managed/src/main/java/com/yugabyte/yw/common/NodeManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -1412,9 +1412,9 @@ private static SkipCertValidationType getSkipCertValidationType(
skipHostValidation =
skipHostValidation
|| GFlagsUtil.shouldSkipServerEndpointVerification(
userIntent.specificGFlags.getGFlags(null, ServerType.MASTER))
userIntent.specificGFlags.getGFlagsForNode(null, ServerType.MASTER))
|| GFlagsUtil.shouldSkipServerEndpointVerification(
userIntent.specificGFlags.getGFlags(null, ServerType.TSERVER));
userIntent.specificGFlags.getGFlagsForNode(null, ServerType.TSERVER));
}
}
return skipHostValidation ? SkipCertValidationType.HOSTNAME : SkipCertValidationType.NONE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import com.typesafe.config.Config;
import com.yugabyte.yw.commissioner.Common;
import com.yugabyte.yw.commissioner.tasks.UniverseTaskBase;
import com.yugabyte.yw.commissioner.tasks.UniverseTaskBase.ServerType;
import com.yugabyte.yw.commissioner.tasks.XClusterConfigTaskBase;
import com.yugabyte.yw.commissioner.tasks.subtasks.AnsibleConfigureServers;
import com.yugabyte.yw.common.CallHomeManager;
Expand Down Expand Up @@ -46,6 +47,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.TreeMap;
import java.util.UUID;
Expand All @@ -61,6 +63,7 @@
import org.apache.commons.csv.CSVParser;
import org.apache.commons.csv.CSVPrinter;
import org.apache.commons.csv.CSVRecord;
import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -162,6 +165,7 @@ public class GFlagsUtil {
public static final String NOTIFY_PEER_OF_REMOVAL_FROM_CLUSTER =
"notify_peer_of_removal_from_cluster";
public static final String MASTER_JOIN_EXISTING_UNIVERSE = "master_join_existing_universe";
public static final String ALLOWED_PREVIEW_FLAGS_CSV = "allowed_preview_flags_csv";

private static final Set<String> GFLAGS_FORBIDDEN_TO_OVERRIDE =
ImmutableSet.<String>builder()
Expand Down Expand Up @@ -869,7 +873,7 @@ public static Map<String, String> getGFlagsForNode(
}
return getGFlagsForNode(node, serverType, primary, allClusters);
}
return userIntent.specificGFlags.getGFlags(node, serverType);
return userIntent.specificGFlags.getGFlagsForNode(node, serverType);
} else {
if (cluster.clusterType == UniverseDefinitionTaskParams.ClusterType.ASYNC) {
return getGFlagsForNode(node, serverType, primary, allClusters);
Expand Down Expand Up @@ -1194,6 +1198,99 @@ public static String checkForbiddenToOverride(
return null;
}

/**
* Checks the preview flags in the user-provided gflags map against the allowed preview flags.
*
* @param userGFlags The map of user-provided gflags.
* @param ybSoftwareVersion The version of the YB software.
* @param serverType The type of server.
* @param gFlagsValidation The gflags validation object.
* @return A string indicating any issues with the preview flags, or null if there are no issues.
* @throws IOException If an I/O error occurs.
*/
public static String checkPreviewGFlags(
Map<String, String> userGFlags,
String ybSoftwareVersion,
ServerType serverType,
GFlagsValidation gFlagsValidation)
throws IOException {
if (MapUtils.isEmpty(userGFlags)) {
return null;
}
List<String> previewGFlags = new ArrayList<>();
for (String key : userGFlags.keySet()) {
Optional<GFlagDetails> gflagDetails =
gFlagsValidation.getGFlagDetails(ybSoftwareVersion, serverType.name(), key);
if (gflagDetails.isPresent()
&& gflagDetails.get().tags != null
&& gflagDetails.get().tags.contains("preview")) {
previewGFlags.add(key);
}
}
if (previewGFlags.size() == 0) {
return null;
}
String allowedPreviewFlags = userGFlags.get(ALLOWED_PREVIEW_FLAGS_CSV);
if (StringUtils.isEmpty(allowedPreviewFlags)) {
return String.format(
"Universe is equipped with preview flags but %s is missing. ", ALLOWED_PREVIEW_FLAGS_CSV);
}
List<String> allowedPreviewFlagsList =
(Arrays.asList(allowedPreviewFlags.split(",")))
.stream().map(String::trim).collect(Collectors.toList());
for (String previewFlag : previewGFlags) {
if (!allowedPreviewFlagsList.contains(previewFlag)) {
return String.format(
"Universe is equipped with preview flags %s but it is not set in %s : %s ",
previewFlag, ALLOWED_PREVIEW_FLAGS_CSV, allowedPreviewFlags);
}
}
return null;
}

/**
* Checks the preview GFlags on specific GFlags.
*
* @param specificGFlags The specific GFlags to check.
* @param gFlagsValidation The GFlags validation object.
* @param ybSoftwareVersion The YB software version.
* @return An error message if any of the preview GFlags are invalid, otherwise null.
* @throws IOException If an I/O error occurs.
*/
public static String checkPreviewGFlagsOnSpecificGFlags(
SpecificGFlags specificGFlags, GFlagsValidation gFlagsValidation, String ybSoftwareVersion)
throws IOException {
if (specificGFlags == null) {
return null;
}
if (specificGFlags.hasPerAZOverrides()) {
for (UUID azUUID : specificGFlags.getPerAZ().keySet()) {
for (ServerType serverType : Arrays.asList(ServerType.MASTER, ServerType.TSERVER)) {
Map<String, String> perAZGFlags = specificGFlags.getGFlags(azUUID, serverType);
String errMsg =
checkPreviewGFlags(perAZGFlags, ybSoftwareVersion, serverType, gFlagsValidation);
if (errMsg != null) {
return errMsg;
}
}
}
} else {
if (specificGFlags.getPerProcessFlags() != null
&& specificGFlags.getPerProcessFlags().value != null) {
for (ServerType serverType : Arrays.asList(ServerType.MASTER, ServerType.TSERVER)) {
Map<String, String> perProcessGFlags =
specificGFlags.getPerProcessFlags().value.get(serverType);
String errMsg =
checkPreviewGFlags(perProcessGFlags, ybSoftwareVersion, serverType, gFlagsValidation);
if (errMsg != null) {
return errMsg;
}
}
}
}
return null;
}

public static void removeGFlag(
UniverseDefinitionTaskParams.UserIntent userIntent,
String gflagKey,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,12 @@ public List<GFlagDetails> extractGFlags(String version, String serverType, boole
}
}

public Optional<GFlagDetails> getGFlagDetails(String version, String serverType, String gflagName)
throws IOException {
List<GFlagDetails> gflagsList = extractGFlags(version, serverType, false);
return gflagsList.stream().filter(flag -> flag.name.equals(gflagName)).findFirst();
}

public synchronized void fetchGFlagFilesFromTarGZipInputStream(
InputStream inputStream,
String dbVersion,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import javax.annotation.Nullable;
import lombok.Data;
import lombok.EqualsAndHashCode;
import org.apache.commons.collections4.MapUtils;

@Data
@ApiModel(description = "GFlags for current cluster")
Expand All @@ -45,14 +46,13 @@ public String toString() {
private Map<UUID, PerProcessFlags> perAZ = new HashMap<>();

@JsonIgnore
public Map<String, String> getGFlags(
public Map<String, String> getGFlagsForNode(
NodeDetails nodeDetails, UniverseTaskBase.ServerType process) {
UUID azUUID = nodeDetails == null ? null : nodeDetails.azUuid;
return getGFlags(azUUID, process);
}

private Map<String, String> getGFlags(
@Nullable UUID azUuid, UniverseTaskBase.ServerType process) {
public Map<String, String> getGFlags(@Nullable UUID azUuid, UniverseTaskBase.ServerType process) {
Map<String, String> result = new HashMap<>();
if (perProcessFlags != null) {
result.putAll(perProcessFlags.value.getOrDefault(process, new HashMap<>()));
Expand Down Expand Up @@ -132,6 +132,18 @@ public static boolean isEmpty(SpecificGFlags specificGFlags) {
return true;
}

public boolean hasPerAZOverrides() {
if (MapUtils.isEmpty(perAZ)) {
return false;
}
for (PerProcessFlags value : perAZ.values()) {
if (value != null && !MapUtils.isEmpty(value.value)) {
return true;
}
}
return false;
}

public static SpecificGFlags construct(
Map<String, String> masterGFlags, Map<String, String> tserverGFlags) {
SpecificGFlags result = new SpecificGFlags();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,18 @@
import com.yugabyte.yw.commissioner.tasks.UniverseTaskBase.ServerType;
import com.yugabyte.yw.common.PlatformServiceException;
import com.yugabyte.yw.common.gflags.GFlagDetails;
import com.yugabyte.yw.common.gflags.GFlagsUtil;
import com.yugabyte.yw.common.gflags.GFlagsValidation;
import com.yugabyte.yw.forms.GFlagsValidationFormData;
import com.yugabyte.yw.forms.GFlagsValidationFormData.GFlagValidationDetails;
import com.yugabyte.yw.forms.GFlagsValidationFormData.GFlagsValidationRequest;
import com.yugabyte.yw.forms.GFlagsValidationFormData.GFlagsValidationResponse;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
import java.util.regex.Matcher;
Expand Down Expand Up @@ -99,12 +102,39 @@ public List<GFlagsValidationResponse> validateGFlags(
gflagsValidation.extractGFlags(version, ServerType.TSERVER.toString(), false).stream()
.collect(Collectors.toMap(gflag -> gflag.name, Function.identity()));

List<String> allowedPreviewMasterFlags = new ArrayList<>();
List<String> allowedPreviewTServerFlags = new ArrayList<>();
if (gflags.gflagsList.size() > 0) {
Optional<GFlagsValidationRequest> allowedPreviewFlagValidationRequest =
gflags.gflagsList.stream()
.filter(flag -> flag.name.equals(GFlagsUtil.ALLOWED_PREVIEW_FLAGS_CSV))
.findFirst();
if (allowedPreviewFlagValidationRequest.isPresent()) {
if (allowedPreviewFlagValidationRequest.get().masterValue != null) {
allowedPreviewMasterFlags =
ImmutableList.copyOf(
Arrays.stream(allowedPreviewFlagValidationRequest.get().masterValue.split(","))
.map(String::trim)
.collect(Collectors.toList()));
}
if (allowedPreviewFlagValidationRequest.get().tserverValue != null) {
allowedPreviewTServerFlags =
ImmutableList.copyOf(
Arrays.stream(allowedPreviewFlagValidationRequest.get().tserverValue.split(","))
.map(String::trim)
.collect(Collectors.toList()));
}
}
}

List<GFlagsValidationResponse> validationResponseArrayList = new ArrayList<>();
for (GFlagsValidationRequest gflag : gflags.gflagsList) {
GFlagsValidationResponse validationResponse = new GFlagsValidationResponse();
validationResponse.name = gflag.name;
validationResponse.masterResponse = checkGflags(gflag, ServerType.MASTER, masterGflagsMap);
validationResponse.tserverResponse = checkGflags(gflag, ServerType.TSERVER, tserverGflagsMap);
validationResponse.masterResponse =
checkGflags(gflag, ServerType.MASTER, masterGflagsMap, allowedPreviewMasterFlags);
validationResponse.tserverResponse =
checkGflags(gflag, ServerType.TSERVER, tserverGflagsMap, allowedPreviewTServerFlags);
validationResponseArrayList.add(validationResponse);
}
return validationResponseArrayList;
Expand All @@ -124,7 +154,10 @@ public GFlagDetails getGFlagsMetadata(String version, String serverType, String
}

private GFlagValidationDetails checkGflags(
GFlagsValidationRequest gflag, ServerType serverType, Map<String, GFlagDetails> gflags) {
GFlagsValidationRequest gflag,
ServerType serverType,
Map<String, GFlagDetails> gflags,
List<String> allowedPreviewFlags) {
GFlagValidationDetails validationDetails = new GFlagValidationDetails();
GFlagDetails gflagDetails = gflags.get(gflag.name);
if (gflagDetails != null) {
Expand All @@ -140,6 +173,14 @@ private GFlagValidationDetails checkGflags(
validationDetails.error = "Given value is not valid";
}
}
if (gflagDetails.tags != null && gflagDetails.tags.contains("preview")) {
if (!allowedPreviewFlags.contains(gflag.name)) {
validationDetails.error =
String.format(
"Given flag '%s' is not allowed to be set unless it is added in '%s' flag",
gflag.name, GFlagsUtil.ALLOWED_PREVIEW_FLAGS_CSV);
}
}
}
return validationDetails;
}
Expand Down
Loading

0 comments on commit bbe20d7

Please sign in to comment.