[PLAT-14595] Ability to change communication ports via edit universe

Summary: Fixed edit flow with communication ports: 1) Removed hack with json converter- setting communication ports during edit operations instead 2) Allow to change communication ports during edit (for VMs) Test Plan: local provider test Reviewers: cwang, nsingh, sanketh Reviewed By: cwang Subscribers: sneelakantan, yugaware Differential Revision: https://phorge.dev.yugabyte.com/D36659
yugabyte · Aug 19, 2024 · b983d56 · b983d56
1 parent 7e1f72c
commit b983d56
Show file tree

Hide file tree

Showing 11 changed files with 96 additions and 10 deletions.
diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/CreateKubernetesUniverse.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/CreateKubernetesUniverse.java
@@ -107,7 +107,8 @@ public void run() {
 
       Universe universe =
           lockAndFreezeUniverseForUpdate(
-              taskParams().expectedUniverseVersion, null /* Txn callback */);
+              taskParams().expectedUniverseVersion,
+              u -> setCommunicationPortsForNodes(true) /* Txn callback */);
       kubernetesStatus.startYBUniverseEventStatus(
           universe,
           taskParams().getKubernetesResourceDetails(),

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/CreateUniverse.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/CreateUniverse.java
@@ -71,6 +71,7 @@ private void freezeUniverseInTxn(Universe universe) {
     setCloudNodeUuids(universe);
     // Update on-prem node UUIDs.
     updateOnPremNodeUuidsOnTaskParams(true /* commit changes */);
+    setCommunicationPortsForNodes(true);
     // Set the prepared data to universe in-memory.
     updateUniverseNodesAndSettings(universe, taskParams(), false);
     for (Cluster cluster : taskParams().clusters) {

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
@@ -107,7 +107,8 @@ public void run() {
       // some precheck operations to verify kubeconfig, svcaccount, connectivity to universe here ?
       Universe universe =
           lockAndFreezeUniverseForUpdate(
-              taskParams().expectedUniverseVersion, null /* Txn callback */);
+              taskParams().expectedUniverseVersion,
+              u -> setCommunicationPortsForNodes(false) /* Txn callback */);
 
       kubernetesStatus.startYBUniverseEventStatus(
           universe,

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/EditUniverse.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/EditUniverse.java
@@ -23,6 +23,7 @@
 import com.yugabyte.yw.models.helpers.NodeDetails;
 import java.util.Comparator;
 import java.util.List;
+import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.stream.Collectors;
@@ -113,6 +114,11 @@ public void run() {
         // Updating placement info and userIntent in DB
         createUpdateUniverseIntentTask(cluster);
       }
+      if (taskParams().communicationPorts != null
+          && !Objects.equals(
+              universe.getUniverseDetails().communicationPorts, taskParams().communicationPorts)) {
+        createUpdateUniverseCommunicationPortsTask(taskParams().communicationPorts);
+      }
 
       // Wait for the master leader to hear from all tservers.
       // NOTE: Universe expansion will fail in the master leader failover scenario - if a node

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/EditUniverseTaskBase.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/EditUniverseTaskBase.java
@@ -86,8 +86,11 @@ protected void freezeUniverseInTxn(Universe universe) {
     preTaskActions(universe);
     // Confirm the nodes on hold.
     commitReservedNodes();
+    setCommunicationPortsForNodes(false);
+
     // Set the prepared data to universe in-memory.
     updateUniverseNodesAndSettings(universe, taskParams(), false);
+
     // Task params contain the exact blueprint of what is desired.
     // There is a rare possibility that this succeeds and
     // saving the Universe fails. It is ok because the retry

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/ReadOnlyClusterCreate.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/ReadOnlyClusterCreate.java
@@ -62,6 +62,7 @@ public void run() {
                 setCloudNodeUuids(u);
                 // Update on-prem node UUIDs.
                 updateOnPremNodeUuidsOnTaskParams(true);
+                setCommunicationPortsForNodes(false);
                 // Set the prepared data to universe in-memory.
                 updateUniverseNodesAndSettings(u, taskParams(), true);
                 u.getUniverseDetails()

diff --git a/...ged/src/main/java/com/yugabyte/yw/commissioner/tasks/ReadOnlyKubernetesClusterCreate.java b/...ged/src/main/java/com/yugabyte/yw/commissioner/tasks/ReadOnlyKubernetesClusterCreate.java
@@ -50,7 +50,7 @@ public void run() {
       verifyParams(UniverseOpType.CREATE);
       Universe universe =
           lockAndFreezeUniverseForUpdate(
-              taskParams().expectedUniverseVersion, null /* Txn callback */);
+              taskParams().expectedUniverseVersion, u -> setCommunicationPortsForNodes(false));
       preTaskActions(universe);
       addBasicPrecheckTasks();
 

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/UniverseDefinitionTaskBase.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/UniverseDefinitionTaskBase.java
@@ -455,6 +455,19 @@ public void setNodeNames(Universe universe) {
     PlacementInfoUtil.ensureUniqueNodeNames(taskParams().nodeDetailsSet);
   }
 
+  protected void setCommunicationPortsForNodes(boolean isCreate) {
+    UniverseTaskParams.CommunicationPorts communicationPorts = taskParams().communicationPorts;
+    if (communicationPorts == null) {
+      communicationPorts = getUniverse().getUniverseDetails().communicationPorts;
+    }
+    for (NodeDetails nodeDetails : taskParams().nodeDetailsSet) {
+      if (isCreate || nodeDetails.state == NodeState.ToBeAdded) {
+        UniverseTaskParams.CommunicationPorts.setCommunicationPorts(
+            communicationPorts, nodeDetails);
+      }
+    }
+  }
+
   /**
    * Pick nodes from node-instance table, set the instance UUIDs to the nodes in task params and
    * reserve in memory or persist the changes to the table.

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
@@ -68,6 +68,7 @@
 import com.yugabyte.yw.forms.UniverseDefinitionTaskParams.ClusterType;
 import com.yugabyte.yw.forms.UniverseDefinitionTaskParams.UserIntent;
 import com.yugabyte.yw.forms.UniverseResp;
+import com.yugabyte.yw.forms.UniverseTaskParams;
 import com.yugabyte.yw.forms.UpgradeParams;
 import com.yugabyte.yw.models.AvailabilityZone;
 import com.yugabyte.yw.models.CertificateInfo;
@@ -2265,6 +2266,16 @@ private void checkTaskParamsForUpdate(
       throw new PlatformServiceException(
           BAD_REQUEST, "No changes that could be applied by EditUniverse");
     }
+
+    UniverseTaskParams.CommunicationPorts communicationPorts = taskParams.communicationPorts;
+    if (communicationPorts != null
+        && !Objects.equals(communicationPorts, universe.getUniverseDetails().communicationPorts)
+        && universe.getUniverseDetails().getPrimaryCluster().userIntent.providerType
+            == Common.CloudType.kubernetes) {
+      throw new PlatformServiceException(
+          BAD_REQUEST, "Cannot change communication ports for k8s universe");
+    }
+
     for (Cluster newCluster : taskParams.clusters) {
       Cluster curCluster = universe.getCluster(newCluster.uuid);
       UserIntent newIntent = newCluster.userIntent;

diff --git a/managed/src/main/java/com/yugabyte/yw/forms/UniverseDefinitionTaskParams.java b/managed/src/main/java/com/yugabyte/yw/forms/UniverseDefinitionTaskParams.java
@@ -1532,13 +1532,6 @@ public static class BaseConverter<T extends UniverseDefinitionTaskParams>
 
     @Override
     public T convert(T taskParams) {
-      // If there is universe level communication port set then push it down to node level
-      if (taskParams.communicationPorts != null && taskParams.nodeDetailsSet != null) {
-        taskParams.nodeDetailsSet.forEach(
-            nodeDetails ->
-                CommunicationPorts.setCommunicationPorts(
-                    taskParams.communicationPorts, nodeDetails));
-      }
       if (taskParams.expectedUniverseVersion == null) {
         taskParams.expectedUniverseVersion = -1;
       }

diff --git a/managed/src/test/java/com/yugabyte/yw/commissioner/tasks/local/EditUniverseLocalTest.java b/managed/src/test/java/com/yugabyte/yw/commissioner/tasks/local/EditUniverseLocalTest.java
@@ -15,11 +15,15 @@
 import com.yugabyte.yw.common.utils.Pair;
 import com.yugabyte.yw.forms.UniverseConfigureTaskParams;
 import com.yugabyte.yw.forms.UniverseDefinitionTaskParams;
+import com.yugabyte.yw.forms.UniverseTaskParams;
 import com.yugabyte.yw.models.RuntimeConfigEntry;
 import com.yugabyte.yw.models.TaskInfo;
 import com.yugabyte.yw.models.Universe;
 import com.yugabyte.yw.models.helpers.NodeDetails;
 import com.yugabyte.yw.models.helpers.TaskType;
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.ServerSocket;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -383,6 +387,58 @@ public void testDecreaseRFPrimary() throws InterruptedException {
     assertEquals(1, universe.getMasters().size());
   }
 
+  @Test
+  public void testUpdateCommPorts() throws InterruptedException {
+    UniverseDefinitionTaskParams.UserIntent userIntent = getDefaultUserIntent();
+    userIntent.specificGFlags = SpecificGFlags.construct(GFLAGS, GFLAGS);
+    Universe universe = createUniverse(userIntent);
+    initYSQL(universe);
+    UniverseDefinitionTaskParams taskParams = universe.getUniverseDetails();
+    UniverseTaskParams.CommunicationPorts newPorts = new UniverseTaskParams.CommunicationPorts();
+    newPorts.masterHttpPort = 11010;
+    newPorts.masterRpcPort = 11011;
+    newPorts.tserverHttpPort = 11050;
+    newPorts.tserverRpcPort = 11051;
+    taskParams.communicationPorts = newPorts;
+
+    PlacementInfoUtil.updateUniverseDefinition(
+        taskParams,
+        customer.getId(),
+        taskParams.getPrimaryCluster().uuid,
+        UniverseConfigureTaskParams.ClusterOperationType.EDIT);
+    verifyNodeModifications(universe, 3, 3);
+
+    UUID taskID =
+        universeCRUDHandler.update(
+            customer, Universe.getOrBadRequest(universe.getUniverseUUID()), taskParams);
+    TaskInfo taskInfo = waitForTask(taskID, universe);
+    verifyUniverseTaskSuccess(taskInfo);
+    universe = Universe.getOrBadRequest(universe.getUniverseUUID());
+    verifyUniverseState(universe);
+    assertEquals(newPorts, universe.getUniverseDetails().communicationPorts);
+    for (NodeDetails nodeDetails : universe.getNodes()) {
+      if (nodeDetails.isMaster) {
+        verifyListeningPort(nodeDetails, newPorts.masterHttpPort);
+        verifyListeningPort(nodeDetails, newPorts.masterRpcPort);
+      }
+      if (nodeDetails.isTserver) {
+        verifyListeningPort(nodeDetails, newPorts.tserverHttpPort);
+        verifyListeningPort(nodeDetails, newPorts.tserverRpcPort);
+      }
+    }
+  }
+
+  private void verifyListeningPort(NodeDetails nodeDetails, int port) {
+    InetAddress inetAddress = null;
+    try {
+      inetAddress = InetAddress.getByName(nodeDetails.cloudInfo.private_ip);
+      ServerSocket ignored = new ServerSocket(port, 50, inetAddress);
+      throw new IllegalStateException(
+          String.format("Expected %s to listen %s port", nodeDetails.cloudInfo.private_ip, port));
+    } catch (IOException ign) {
+    }
+  }
+
   // FAILURE TESTS
 
   @Test