Skip to content

Commit

Permalink
[PLAT-15262]Add more checks for non-namespace scope supported universes
Browse files Browse the repository at this point in the history 
Summary:
1. Added check to ensure non-namespace scope supported universes are not allowed to pass
Namespaced services in overrides.
2. Moved the subtask location for Namespaced service handling. Earlier we were re-owning
and deleting namespaced services before creating new ones. Now moved it to after new services
are created and just before the current pods are deleted.

Test Plan:
Verified manually using following scenarios:
# If universe uses old naming style, verified Namespaced scope services are not allowed in overrides list.
# Verified corner case scenario with empty serviceEndpoints array works correctly: `serviceEndpoints: []`
# Verified corner case scenario with serviceEndpoints override not applied as overrides works correctly.
# Verified the subtask position move from before new Pods creation to after new Pods creation works correctly.

Reviewers: anijhawan, #yba-api-review

Reviewed By: anijhawan, #yba-api-review

Subscribers: yugaware

Differential Revision: https://phorge.dev.yugabyte.com/D38031
  • Loading branch information
@kv83821-yb
kv83821-yb committed Sep 18, 2024
1 parent 240e8f0 commit 2a40433
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 30 deletions.
16 changes: 8 additions & 8 deletions managed/src/main/java/com/yugabyte/yw/commissioner/tasks/EditKubernetesUniverse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,14 +380,6 @@ private boolean editCluster(
PlacementInfoUtil.addPlacementZone(currAZs, activeZones);
}

// Handle Namespaced services ownership change/delete
addHandleKubernetesNamespacedServices(
false /* readReplicaDelete */,
taskParams(),
taskParams().getUniverseUUID(),
true /* handleOwnershipChanges */)
.setSubTaskGroupType(SubTaskGroupType.KubernetesHandleNamespacedService);

if (!mastersToAdd.isEmpty()) {
// Bring up new masters and update the configs.
// No need to check mastersToRemove as total number of masters is invariant.
Expand Down Expand Up @@ -484,6 +476,14 @@ private boolean editCluster(
createWaitForLoadBalanceTask().setSubTaskGroupType(SubTaskGroupType.WaitForDataMigration);
}

// Handle Namespaced services ownership change/delete
addHandleKubernetesNamespacedServices(
false /* readReplicaDelete */,
taskParams(),
taskParams().getUniverseUUID(),
true /* handleOwnershipChanges */)
.setSubTaskGroupType(SubTaskGroupType.KubernetesHandleNamespacedService);

String universeOverrides = primaryCluster.userIntent.universeOverrides;
Map<String, String> azOverrides = primaryCluster.userIntent.azOverrides;
if (azOverrides == null) {
Expand Down
54 changes: 42 additions & 12 deletions managed/src/main/java/com/yugabyte/yw/common/KubernetesUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Set;
import java.util.UUID;
import java.util.function.Function;
Expand Down Expand Up @@ -759,6 +760,9 @@ private static Map<UUID, Map<String, Object>> getFinalOverrides(
universeOverrides = mapper.readValue(universeOverridesStr, Map.class);
}

if (CollectionUtils.isEmpty(placementInfo.cloudList)) {
return result;
}
Map<String, String> cloudConfig = CloudInfoInterface.fetchEnvVars(provider);
for (PlacementRegion pr : placementInfo.cloudList.get(0).regionList) {
Region region = Region.getOrBadRequest(pr.uuid);
Expand Down Expand Up @@ -804,7 +808,7 @@ private static Set<String> getNamespacesInCluster(
if (cluster.userIntent.providerType != CloudType.kubernetes) {
continue;
}
if (clusterType != cluster.clusterType) {
if ((clusterType != null) && (clusterType != cluster.clusterType)) {
continue;
}
PlacementInfo pi = cluster.placementInfo;
Expand Down Expand Up @@ -855,7 +859,7 @@ public static Map<String, Map<UUID, Map<String, Object>>> generateNamespaceAZOve
if (cluster.userIntent.providerType != CloudType.kubernetes) {
continue;
}
if (clusterType != cluster.clusterType) {
if ((clusterType != null) && (clusterType != cluster.clusterType)) {
continue;
}
Map<UUID, Map<String, Object>> finalOverrides =
Expand Down Expand Up @@ -920,8 +924,10 @@ private static Map<String, Map<String, Object>> getServicesFromOverrides(

/**
* Generate Map of Namespace and Per-AZ Namespace scoped services. Returns empty map for AZs where
* serviceEndpoints is not defined. Returns null for AZs where serviceEndpoints is defined but
* empty.
* serviceEndpoints is not defined. Returns null for AZs where serviceEndpoints is defined as
* empty array. For helm default overrides( i.e. no overrides specified ): <br>
* 1. Returns null for default scope "AZ" <br>
* 2. Returns empty map for default scope "Namespaced"
*
* @param universeParams
* @return Generated namespaced NS scope services in the form &lt;Namespace, &lt;AZ_uuid,
Expand All @@ -934,6 +940,9 @@ private static Map<String, Map<String, Object>> getServicesFromOverrides(
throws IOException {
Map<String, Map<UUID, Map<String, Object>>> namespaceAZOverrides =
generateNamespaceAZOverridesMap(universeParams, clusterType);
if (namespaceAZOverrides == null) {
return null;
}
Map<String, Map<UUID, Map<String, Map<String, Object>>>> nsNamespacedServices = new HashMap<>();
String defaultScopeUserIntent =
universeParams.getPrimaryCluster().userIntent.defaultServiceScopeAZ ? "AZ" : "Namespaced";
Expand All @@ -945,8 +954,13 @@ private static Map<String, Map<String, Object>> getServicesFromOverrides(
azOverridesEntry -> {
Map<String, Object> override = azOverridesEntry.getValue();
Map<String, Map<String, Object>> services = getServicesFromOverrides(override);
if (services == null) {
if (services == null
|| (MapUtils.isEmpty(services) && defaultScopeUserIntent.equals("AZ"))) {
azNamespacedServicesMap.put(azOverridesEntry.getKey(), null /* empty Services */);
} else if (MapUtils.isEmpty(services)
&& defaultScopeUserIntent.equals("Namespaced")) {
azNamespacedServicesMap.put(
azOverridesEntry.getKey(), new HashMap<String, Map<String, Object>>());
} else {
Map<String, Map<String, Object>> namespacedServices =
services.entrySet().stream()
Expand All @@ -967,11 +981,16 @@ private static Map<String, Map<String, Object>> getServicesFromOverrides(
return scope.equals("Namespaced");
})
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
// Set null if using default helm overrides and userIntent service scope is "AZ"
if (MapUtils.isEmpty(namespacedServices)) {
namespacedServices = null;
}
azNamespacedServicesMap.put(azOverridesEntry.getKey(), namespacedServices);
}
});
nsNamespacedServices.put(namespace, azNamespacedServicesMap);
}
log.debug("Namespaced services: {}", nsNamespacedServices);
return nsNamespacedServices;
}

Expand Down Expand Up @@ -1224,9 +1243,20 @@ private static void validateConflictingService(UniverseDefinitionTaskParams univ
public static void validateServiceEndpoints(
UniverseDefinitionTaskParams universeParams, Map<String, String> universeConfig)
throws IOException {
// Return if should not configure
if (!shouldConfigureNamespacedService(universeParams, universeConfig)) {
log.debug("Universe configuration does not support Namespace scoped services, skipping");
Map<String, Map<UUID, Map<String, Map<String, Object>>>> nsScopedServices =
getNamespaceNSScopedServices(universeParams, null /* clusterType */);
if (nsScopedServices == null) {
return;
}
boolean nsServicePresent =
nsScopedServices.values().stream()
.flatMap(azsMap -> azsMap.values().stream())
.anyMatch(Objects::nonNull);
if (nsServicePresent) {
throw new RuntimeException(
"Universe configuration does not support Namespace scoped services");
}
return;
}
// Validate service name does not appear twice in final overrides per AZ.
Expand All @@ -1250,18 +1280,18 @@ public static void validateUpgradeServiceEndpoints(
UniverseDefinitionTaskParams universeParams,
Map<String, String> universeConfig)
throws IOException {
// Return if should not configure
if (!shouldConfigureNamespacedService(universeParams, universeConfig)) {
log.debug("Universe configuration does not support Namespace scoped services, skipping");
return;
}
UniverseDefinitionTaskParams taskParams =
Json.fromJson(Json.toJson(universeParams), UniverseDefinitionTaskParams.class);
taskParams.getPrimaryCluster().userIntent.universeOverrides = newUniverseOverrides;
taskParams.getPrimaryCluster().userIntent.azOverrides = newAZOverrides;
// Validate new overrides for service endpoint independently
validateServiceEndpoints(taskParams, universeConfig);

// Return if not supported
if (!shouldConfigureNamespacedService(taskParams, universeConfig)) {
return;
}

String defaultScope =
universeParams.getPrimaryCluster().userIntent.defaultServiceScopeAZ ? "AZ" : "Namespaced";

Expand Down
12 changes: 6 additions & 6 deletions managed/src/main/java/com/yugabyte/yw/controllers/handlers/UniverseCRUDHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -941,12 +941,12 @@ public UniverseResp createUniverse(Customer customer, UniverseDefinitionTaskPara
// Default service scope should be 'Namespaced'
primaryIntent.defaultServiceScopeAZ = false;
}
// Validate service endpoints
try {
KubernetesUtil.validateServiceEndpoints(taskParams, universe.getConfig());
} catch (IOException e) {
throw new RuntimeException("Failed to parse Kubernetes overrides!", e.getCause());
}
}
// Validate service endpoints
try {
KubernetesUtil.validateServiceEndpoints(taskParams, universe.getConfig());
} catch (IOException e) {
throw new RuntimeException("Failed to parse Kubernetes overrides!", e.getCause());
}
} else {
if (primaryCluster.userIntent.enableIPV6) {
Expand Down
8 changes: 4 additions & 4 deletions managed/src/test/java/com/yugabyte/yw/commissioner/tasks/EditKubernetesUniverseTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,12 +214,12 @@ private void setupUniverseMultiAZ(boolean setMasters, int numTservers) {
ImmutableList.of(
TaskType.CheckLeaderlessTablets,
TaskType.FreezeUniverse,
TaskType.HandleKubernetesNamespacedServices,
TaskType.KubernetesCommandExecutor,
TaskType.KubernetesCheckNumPod,
TaskType.KubernetesCommandExecutor,
TaskType.WaitForServer,
TaskType.UpdatePlacementInfo,
TaskType.HandleKubernetesNamespacedServices,
TaskType.KubernetesCommandExecutor,
TaskType.InstallingThirdPartySoftware,
TaskType.UpdateUniverseIntent,
Expand All @@ -228,14 +228,14 @@ private void setupUniverseMultiAZ(boolean setMasters, int numTservers) {

private List<JsonNode> getExpectedAddPodTaskResults() {
return ImmutableList.of(
Json.toJson(ImmutableMap.of()),
Json.toJson(ImmutableMap.of()),
Json.toJson(ImmutableMap.of()),
Json.toJson(ImmutableMap.of("commandType", HELM_UPGRADE.name())),
Json.toJson(ImmutableMap.of("commandType", WAIT_FOR_PODS.name())),
Json.toJson(ImmutableMap.of("commandType", POD_INFO.name())),
Json.toJson(ImmutableMap.of()),
Json.toJson(ImmutableMap.of()),
Json.toJson(ImmutableMap.of()),
Json.toJson(ImmutableMap.of("commandType", POD_INFO.name())),
Json.toJson(ImmutableMap.of()),
Json.toJson(ImmutableMap.of()),
Expand All @@ -247,9 +247,9 @@ private List<JsonNode> getExpectedAddPodTaskResults() {
ImmutableList.of(
TaskType.CheckLeaderlessTablets,
TaskType.FreezeUniverse,
TaskType.HandleKubernetesNamespacedServices,
TaskType.UpdatePlacementInfo,
TaskType.WaitForDataMove,
TaskType.HandleKubernetesNamespacedServices,
TaskType.CheckNodeSafeToDelete,
TaskType.KubernetesCommandExecutor,
TaskType.KubernetesCheckNumPod,
Expand Down Expand Up @@ -284,8 +284,8 @@ private List<JsonNode> getExpectedRemovePodTaskResults() {
ImmutableList.of(
TaskType.CheckLeaderlessTablets,
TaskType.FreezeUniverse,
TaskType.HandleKubernetesNamespacedServices,
TaskType.UpdatePlacementInfo,
TaskType.HandleKubernetesNamespacedServices,
TaskType.CheckUnderReplicatedTablets,
TaskType.CheckNodesAreSafeToTakeDown,
TaskType.KubernetesCommandExecutor,
Expand Down
1 change: 1 addition & 0 deletions managed/src/test/java/com/yugabyte/yw/controllers/UniverseCreateControllerTestBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,7 @@ public void testUniverseCreateDeviceInfoValidation(
node.placementUuid = cluster.uuid;
bodyJson.set("clusters", Json.newArray().add(Json.toJson(cluster)));
bodyJson.set("nodeDetailsSet", Json.newArray().add(Json.toJson(node)));
bodyJson.put("nodePrefix", "demo-node");

if (errorMessage == null) {
Result result = sendCreateRequest(bodyJson);
Expand Down

0 comments on commit 2a40433

Please sign in to comment.