From 2a40433377d7c82e3da62ac2ba350199d5e2bbbd Mon Sep 17 00:00:00 2001 From: Kumar Vivek Date: Fri, 13 Sep 2024 05:53:55 +0000 Subject: [PATCH] [PLAT-15262]Add more checks for non-namespace scope supported universes 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 --- .../tasks/EditKubernetesUniverse.java | 16 +++--- .../yugabyte/yw/common/KubernetesUtil.java | 54 ++++++++++++++----- .../handlers/UniverseCRUDHandler.java | 12 ++--- .../tasks/EditKubernetesUniverseTest.java | 8 +-- .../UniverseCreateControllerTestBase.java | 1 + 5 files changed, 61 insertions(+), 30 deletions(-) diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/EditKubernetesUniverse.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/EditKubernetesUniverse.java index 8aa41089337b..f55762bc044e 100644 --- a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/EditKubernetesUniverse.java +++ b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/EditKubernetesUniverse.java @@ -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. @@ -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 azOverrides = primaryCluster.userIntent.azOverrides; if (azOverrides == null) { diff --git a/managed/src/main/java/com/yugabyte/yw/common/KubernetesUtil.java b/managed/src/main/java/com/yugabyte/yw/common/KubernetesUtil.java index eef8d11ad099..ea7f17821f4d 100644 --- a/managed/src/main/java/com/yugabyte/yw/common/KubernetesUtil.java +++ b/managed/src/main/java/com/yugabyte/yw/common/KubernetesUtil.java @@ -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; @@ -759,6 +760,9 @@ private static Map> getFinalOverrides( universeOverrides = mapper.readValue(universeOverridesStr, Map.class); } + if (CollectionUtils.isEmpty(placementInfo.cloudList)) { + return result; + } Map cloudConfig = CloudInfoInterface.fetchEnvVars(provider); for (PlacementRegion pr : placementInfo.cloudList.get(0).regionList) { Region region = Region.getOrBadRequest(pr.uuid); @@ -804,7 +808,7 @@ private static Set getNamespacesInCluster( if (cluster.userIntent.providerType != CloudType.kubernetes) { continue; } - if (clusterType != cluster.clusterType) { + if ((clusterType != null) && (clusterType != cluster.clusterType)) { continue; } PlacementInfo pi = cluster.placementInfo; @@ -855,7 +859,7 @@ public static Map>> generateNamespaceAZOve if (cluster.userIntent.providerType != CloudType.kubernetes) { continue; } - if (clusterType != cluster.clusterType) { + if ((clusterType != null) && (clusterType != cluster.clusterType)) { continue; } Map> finalOverrides = @@ -920,8 +924,10 @@ private static Map> 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 ):
+ * 1. Returns null for default scope "AZ"
+ * 2. Returns empty map for default scope "Namespaced" * * @param universeParams * @return Generated namespaced NS scope services in the form <Namespace, <AZ_uuid, @@ -934,6 +940,9 @@ private static Map> getServicesFromOverrides( throws IOException { Map>> namespaceAZOverrides = generateNamespaceAZOverridesMap(universeParams, clusterType); + if (namespaceAZOverrides == null) { + return null; + } Map>>> nsNamespacedServices = new HashMap<>(); String defaultScopeUserIntent = universeParams.getPrimaryCluster().userIntent.defaultServiceScopeAZ ? "AZ" : "Namespaced"; @@ -945,8 +954,13 @@ private static Map> getServicesFromOverrides( azOverridesEntry -> { Map override = azOverridesEntry.getValue(); Map> 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>()); } else { Map> namespacedServices = services.entrySet().stream() @@ -967,11 +981,16 @@ private static Map> 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; } @@ -1224,9 +1243,20 @@ private static void validateConflictingService(UniverseDefinitionTaskParams univ public static void validateServiceEndpoints( UniverseDefinitionTaskParams universeParams, Map universeConfig) throws IOException { - // Return if should not configure if (!shouldConfigureNamespacedService(universeParams, universeConfig)) { - log.debug("Universe configuration does not support Namespace scoped services, skipping"); + Map>>> 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. @@ -1250,11 +1280,6 @@ public static void validateUpgradeServiceEndpoints( UniverseDefinitionTaskParams universeParams, Map 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; @@ -1262,6 +1287,11 @@ public static void validateUpgradeServiceEndpoints( // 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"; diff --git a/managed/src/main/java/com/yugabyte/yw/controllers/handlers/UniverseCRUDHandler.java b/managed/src/main/java/com/yugabyte/yw/controllers/handlers/UniverseCRUDHandler.java index fa970e421141..9a2aa3793c25 100644 --- a/managed/src/main/java/com/yugabyte/yw/controllers/handlers/UniverseCRUDHandler.java +++ b/managed/src/main/java/com/yugabyte/yw/controllers/handlers/UniverseCRUDHandler.java @@ -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) { diff --git a/managed/src/test/java/com/yugabyte/yw/commissioner/tasks/EditKubernetesUniverseTest.java b/managed/src/test/java/com/yugabyte/yw/commissioner/tasks/EditKubernetesUniverseTest.java index 7760948cfc3d..0a7d5cff8ba3 100644 --- a/managed/src/test/java/com/yugabyte/yw/commissioner/tasks/EditKubernetesUniverseTest.java +++ b/managed/src/test/java/com/yugabyte/yw/commissioner/tasks/EditKubernetesUniverseTest.java @@ -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, @@ -228,7 +228,6 @@ private void setupUniverseMultiAZ(boolean setMasters, int numTservers) { private List getExpectedAddPodTaskResults() { return ImmutableList.of( - Json.toJson(ImmutableMap.of()), Json.toJson(ImmutableMap.of()), Json.toJson(ImmutableMap.of()), Json.toJson(ImmutableMap.of("commandType", HELM_UPGRADE.name())), @@ -236,6 +235,7 @@ private List getExpectedAddPodTaskResults() { 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()), @@ -247,9 +247,9 @@ private List getExpectedAddPodTaskResults() { ImmutableList.of( TaskType.CheckLeaderlessTablets, TaskType.FreezeUniverse, - TaskType.HandleKubernetesNamespacedServices, TaskType.UpdatePlacementInfo, TaskType.WaitForDataMove, + TaskType.HandleKubernetesNamespacedServices, TaskType.CheckNodeSafeToDelete, TaskType.KubernetesCommandExecutor, TaskType.KubernetesCheckNumPod, @@ -284,8 +284,8 @@ private List getExpectedRemovePodTaskResults() { ImmutableList.of( TaskType.CheckLeaderlessTablets, TaskType.FreezeUniverse, - TaskType.HandleKubernetesNamespacedServices, TaskType.UpdatePlacementInfo, + TaskType.HandleKubernetesNamespacedServices, TaskType.CheckUnderReplicatedTablets, TaskType.CheckNodesAreSafeToTakeDown, TaskType.KubernetesCommandExecutor, diff --git a/managed/src/test/java/com/yugabyte/yw/controllers/UniverseCreateControllerTestBase.java b/managed/src/test/java/com/yugabyte/yw/controllers/UniverseCreateControllerTestBase.java index 77ba88112073..331b8ddb8e1a 100644 --- a/managed/src/test/java/com/yugabyte/yw/controllers/UniverseCreateControllerTestBase.java +++ b/managed/src/test/java/com/yugabyte/yw/controllers/UniverseCreateControllerTestBase.java @@ -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);