Skip to content

Commit

Permalink
[yugabyte#7942] Actually remove Universe.toJson and fix DBJson usage
Browse files Browse the repository at this point in the history
Summary:
This diff also fixes unnecessary json marshalling/unmarshalling code.
Just fixed couple such cases to serve as an example so we can do a larger fix across codebase.

Test Plan:
Unittests.
Also ran the server and checked that Universe.config gets deserialized for existing universe and newly created universe.

Reviewers: wesley, jvigil

Reviewed By: jvigil

Subscribers: jenkins-bot, yugaware

Differential Revision: https://phabricator.dev.yugabyte.com/D11369
  • Loading branch information
sb-yb committed Apr 27, 2021
1 parent faea9cb commit c0b9786
Show file tree
Hide file tree
Showing 24 changed files with 231 additions and 409 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -184,15 +184,19 @@ public static UniverseResourceDetails create(Collection<NodeDetails> nodes,
if (node.placementUuid != null) {
userIntent = params.getClusterByUuid(node.placementUuid).userIntent;
}
if (userIntent.deviceInfo != null) {
if (userIntent.deviceInfo != null &&
userIntent.deviceInfo.volumeSize != null &&
userIntent.deviceInfo.numVolumes != null) {
details.addVolumeCount(userIntent.deviceInfo.numVolumes);
details.addVolumeSizeGB(
userIntent.deviceInfo.volumeSize * userIntent.deviceInfo.numVolumes);
}
if (node.cloudInfo != null) {
if (node.cloudInfo != null &&
node.cloudInfo.az != null &&
node.cloudInfo.instance_type != null) {
details.addAz(node.cloudInfo.az);
InstanceType instanceType = InstanceType.get(UUID.fromString(userIntent.provider),
node.cloudInfo.instance_type);
InstanceType instanceType = InstanceType.get(UUID.fromString(userIntent.provider),
node.cloudInfo.instance_type);
if (instanceType == null) {
LOG.error("Couldn't find instance type " + node.cloudInfo.instance_type +
" for provider " + userIntent.providerType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public void run() {
return;
}
try {
BackupTableParams backupParams = Json.fromJson(backup.backupInfo, BackupTableParams.class);
BackupTableParams backupParams = backup.getBackupInfo();
List<BackupTableParams> backupList =
backupParams.backupList == null ? ImmutableList.of(backupParams) : backupParams.backupList;
if (deleteAllBackups(backupList)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,59 +2,39 @@
package com.yugabyte.yw.controllers;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.google.inject.Inject;
import com.typesafe.config.Config;
import com.yugabyte.yw.cloud.AWSInitializer;
import com.yugabyte.yw.cloud.AZUInitializer;
import com.yugabyte.yw.cloud.GCPInitializer;
import com.yugabyte.yw.commissioner.Commissioner;
import com.yugabyte.yw.commissioner.Common;
import com.yugabyte.yw.commissioner.tasks.CloudBootstrap;
import com.yugabyte.yw.commissioner.tasks.CloudCleanup;
import com.yugabyte.yw.commissioner.tasks.params.CloudTaskParams;
import com.yugabyte.yw.commissioner.tasks.params.KubernetesClusterInitParams;
import com.yugabyte.yw.common.ApiResponse;
import com.yugabyte.yw.common.CloudQueryHelper;
import com.yugabyte.yw.common.ConfigHelper;
import com.yugabyte.yw.common.AccessManager;
import com.yugabyte.yw.common.DnsManager;
import com.yugabyte.yw.common.ShellProcessHandler;
import com.yugabyte.yw.common.ShellResponse;
import com.yugabyte.yw.forms.CloudProviderFormData;
import com.yugabyte.yw.common.*;
import com.yugabyte.yw.forms.CloudBootstrapFormData;
import com.yugabyte.yw.forms.CloudProviderFormData;
import com.yugabyte.yw.forms.KubernetesProviderFormData;
import com.yugabyte.yw.forms.KubernetesProviderFormData.RegionData;
import com.yugabyte.yw.forms.KubernetesProviderFormData.RegionData.ZoneData;
import com.yugabyte.yw.models.*;
import com.yugabyte.yw.models.helpers.TaskType;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.fasterxml.jackson.databind.node.ObjectNode;
import com.fasterxml.jackson.databind.ObjectMapper;

import com.google.inject.Inject;

import play.api.Play;
import play.data.Form;
import play.data.FormFactory;
import play.Environment;
import play.libs.Json;
import play.mvc.Result;

import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.UUID;
import javax.persistence.PersistenceException;
import java.util.*;

import static com.yugabyte.yw.common.ConfigHelper.ConfigType.DockerInstanceTypeMetadata;
import static com.yugabyte.yw.common.ConfigHelper.ConfigType.DockerRegionMetadata;
import static com.yugabyte.yw.models.helpers.CommonUtils.DEFAULT_YB_HOME_DIR;

public class CloudProviderController extends AuthenticatedController {
private final Config config;
Expand Down Expand Up @@ -372,7 +352,7 @@ private boolean updateKubeConfig(Provider provider, Region region, AvailabilityZ
} else if (zone == null) {
region.setConfig(config);
} else {
zone.setConfig(config);
zone.updateConfig(config);
}
return hasKubeConfig;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableMap;
import com.google.inject.Inject;
import com.typesafe.config.Config;
import com.yugabyte.yw.cloud.PublicCloudConstants;
import com.yugabyte.yw.cloud.UniverseResourceDetails;
import com.yugabyte.yw.commissioner.Commissioner;
Expand Down Expand Up @@ -38,18 +37,21 @@
import play.data.Form;
import play.data.FormFactory;
import play.libs.Json;
import play.libs.concurrent.HttpExecutionContext;
import play.mvc.Http.HeaderNames;
import play.mvc.Result;
import play.mvc.Results;
import play.libs.concurrent.HttpExecutionContext;

import java.io.*;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.InputStream;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.*;
import java.util.concurrent.CompletableFuture;
import java.util.stream.Collectors;
import java.util.concurrent.CompletionStage;
import java.util.stream.Collectors;

import static com.yugabyte.yw.common.PlacementInfoUtil.checkIfNodeParamsValid;
import static com.yugabyte.yw.common.PlacementInfoUtil.updatePlacementInfo;
Expand Down Expand Up @@ -512,7 +514,7 @@ public Result create(UUID customerUUID) {
primaryIntent.tserverGFlags = trimFlags(primaryIntent.tserverGFlags);
if (primaryCluster.userIntent.providerType.equals(CloudType.kubernetes)) {
taskType = TaskType.CreateKubernetesUniverse;
universe.setConfig(ImmutableMap.of(Universe.HELM2_LEGACY,
universe.updateConfig(ImmutableMap.of(Universe.HELM2_LEGACY,
Universe.HelmLegacy.V3.toString()));
} else {
if (primaryCluster.userIntent.enableIPV6) {
Expand Down Expand Up @@ -593,7 +595,7 @@ public Result create(UUID customerUUID) {
}
}

universe.setConfig(ImmutableMap.of(Universe.TAKE_BACKUPS, "true"));
universe.updateConfig(ImmutableMap.of(Universe.TAKE_BACKUPS, "true"));

// Submit the task to create the universe.
UUID taskUUID = commissioner.submit(taskType, taskParams);
Expand Down Expand Up @@ -897,7 +899,7 @@ public Result setBackupFlag(UUID customerUUID, UUID universeUUID) {
} else {
return ApiResponse.error(BAD_REQUEST, "Invalid Query: Need to specify markActive value");
}
universe.setConfig(config);
universe.updateConfig(config);
Audit.createAuditEntry(ctx(), request());
return ApiResponse.success();
} catch (Exception e) {
Expand Down Expand Up @@ -928,7 +930,7 @@ public Result setHelm3Compatible(UUID customerUUID, UUID universeUUID) {
try {
Map<String, String> config = new HashMap<>();
config.put(Universe.HELM2_LEGACY, Universe.HelmLegacy.V2TO3.toString());
universe.setConfig(config);
universe.updateConfig(config);
Audit.createAuditEntry(ctx(), request());
return ApiResponse.success();
} catch (Exception e) {
Expand Down Expand Up @@ -970,7 +972,7 @@ public Result configureAlerts(UUID customerUUID, UUID universeUUID) {
));
}
config.put(Universe.DISABLE_ALERTS_UNTIL, Long.toString(disabledUntilSecs));
universe.setConfig(config);
universe.updateConfig(config);

return ApiResponse.success();
} catch (Exception e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package com.yugabyte.yw.forms;

import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.JsonNode;
Expand All @@ -17,11 +18,9 @@
import com.yugabyte.yw.models.helpers.DeviceInfo;
import com.yugabyte.yw.models.helpers.NodeDetails;
import com.yugabyte.yw.models.helpers.PlacementInfo;
import io.ebean.annotation.DbJson;
import play.data.validation.Constraints;
import play.libs.Json;

import javax.persistence.Column;
import java.util.*;
import java.util.stream.Collectors;

Expand All @@ -43,17 +42,14 @@
* <p>
* NOTE #1: The regions can potentially be present in different clouds.
*/

@JsonIgnoreProperties(ignoreUnknown = true)
public class UniverseDefinitionTaskParams extends UniverseTaskParams {

@Constraints.Required()
@Constraints.MinLength(1)
public List<Cluster> clusters = new LinkedList<>();

@Constraints.Required()
@DbJson
@Column(nullable = false, columnDefinition = "TEXT")
public JsonNode preflight_checks;

@JsonIgnore
// This is set during configure to figure out which cluster type is intended to be modified.
public ClusterType currentClusterType = ClusterType.PRIMARY;
Expand Down
6 changes: 3 additions & 3 deletions managed/src/main/java/com/yugabyte/yw/forms/UniverseResp.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@
package com.yugabyte.yw.forms;

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.databind.JsonNode;
import com.yugabyte.yw.cloud.UniverseResourceDetails;
import com.yugabyte.yw.models.Universe;
import com.yugabyte.yw.models.helpers.NodeDetails;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.Collection;
import java.util.Map;
import java.util.UUID;

@JsonInclude(JsonInclude.Include.NON_NULL)
Expand All @@ -35,7 +35,7 @@ public class UniverseResp {
public final UniverseResourceDetails resources;

public final UniverseDefinitionTaskParamsResp universeDetails;
public final JsonNode universeConfig;
public final Map<String, String> universeConfig;
public final String taskUUID;

public UniverseResp(Universe entity) {
Expand All @@ -57,7 +57,7 @@ public UniverseResp(Universe entity, UUID taskUUID, UniverseResourceDetails reso
Collection<NodeDetails> nodes = entity.getUniverseDetails().nodeDetailsSet;

this.resources = resources;
universeConfig = entity.config;
universeConfig = entity.getConfig();
}

// TODO(UI folks): Remove this. This is redundant as it is already available in resources
Expand Down
13 changes: 6 additions & 7 deletions managed/src/main/java/com/yugabyte/yw/models/AccessKey.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@

package com.yugabyte.yw.models;

import io.ebean.*;
import io.ebean.annotation.DbJson;
import com.fasterxml.jackson.annotation.JsonBackReference;
import com.fasterxml.jackson.databind.JsonNode;
import io.ebean.Finder;
import io.ebean.Model;
import io.ebean.annotation.DbJson;
import play.data.validation.Constraints;
import play.libs.Json;

import javax.persistence.Column;
import javax.persistence.EmbeddedId;
Expand Down Expand Up @@ -45,10 +44,10 @@ public static class KeyInfo {
@Constraints.Required
@Column(nullable = false, columnDefinition = "TEXT")
@DbJson
public JsonNode keyInfo;
private KeyInfo keyInfo;

public void setKeyInfo(KeyInfo info) { this.keyInfo = Json.toJson(info); }
public KeyInfo getKeyInfo() { return Json.fromJson(this.keyInfo, KeyInfo.class); }
public void setKeyInfo(KeyInfo info) { this.keyInfo = info; }
public KeyInfo getKeyInfo() { return this.keyInfo; }

public static AccessKey create(UUID providerUUID, String keyCode, KeyInfo keyInfo) {
AccessKey accessKey = new AccessKey();
Expand Down
28 changes: 7 additions & 21 deletions managed/src/main/java/com/yugabyte/yw/models/AvailabilityZone.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,17 @@

import com.fasterxml.jackson.annotation.JsonBackReference;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.databind.JsonNode;
import com.yugabyte.yw.common.YWServiceException;
import io.ebean.*;
import io.ebean.annotation.DbJson;
import play.data.validation.Constraints;
import play.libs.Json;

import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.Id;
import javax.persistence.ManyToOne;
import java.util.*;

import static com.yugabyte.yw.models.helpers.CommonUtils.maskConfig;
import static play.mvc.Http.Status.BAD_REQUEST;

@Entity
Expand Down Expand Up @@ -54,38 +51,27 @@ public void setActiveFlag(Boolean active) {

@DbJson
@Column(columnDefinition = "TEXT")
public JsonNode config;
public Map<String, String> config;

public String getKubeconfigPath() {
Map<String, String> configMap = this.getConfig();
return configMap.getOrDefault("KUBECONFIG", null);
}

public void setConfig(Map<String, String> configMap) {
Map<String, String> currConfig = this.getConfig();
for (String key : configMap.keySet()) {
currConfig.put(key, configMap.get(key));
}
this.config = Json.toJson(currConfig);
this.config = configMap;
this.save();
}

@JsonIgnore
public JsonNode getMaskedConfig() {
if (this.config == null) {
return Json.newObject();
} else {
return maskConfig(this.config);
}
public void updateConfig(Map<String, String> configMap) {
Map<String, String> config = getConfig();
config.putAll(configMap);
setConfig(config);
}

@JsonIgnore
public Map<String, String> getConfig() {
if (this.config == null) {
return new HashMap();
} else {
return Json.fromJson(this.config, Map.class);
}
return this.config == null ? new HashMap<>() : this.config;
}

/**
Expand Down
7 changes: 3 additions & 4 deletions managed/src/main/java/com/yugabyte/yw/models/Backup.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import io.ebean.annotation.UpdatedTimestamp;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import play.libs.Json;

import javax.persistence.Column;
import javax.persistence.Entity;
Expand Down Expand Up @@ -60,7 +59,7 @@ public enum BackupState {

@Column(columnDefinition = "TEXT", nullable = false)
@DbJson
public JsonNode backupInfo;
private BackupTableParams backupInfo;

@Column(unique = true)
public UUID taskUUID;
Expand All @@ -75,11 +74,11 @@ public enum BackupState {
public Date getExpiry() { return expiry; }

public void setBackupInfo(BackupTableParams params) {
this.backupInfo = Json.toJson(params);
this.backupInfo = params;
}

public BackupTableParams getBackupInfo() {
return Json.fromJson(this.backupInfo, BackupTableParams.class);
return this.backupInfo;
}

@CreatedTimestamp
Expand Down
Loading

0 comments on commit c0b9786

Please sign in to comment.