[PLAT-499][Platform] K8s universe names need to be sanitized for auto…

…-generated K8s namespaces (#8822) Summary: This change allows uppercase universe names. The code for getting namespace is deeply nested and changing its param may lead to much bigger change. I think this change is minimal to take care of the issue. Change description: 1. The existing regex was allowing universe names like --ABC. It has been changed to not allow this by restricting the start and ending chars to alphanumeric chars only. For non-boundary chars, it remains the same as before. 2. Old namespaces which have already been created in k8s have already passed the k8s restriction (i.e no caps and length). For them, there is no additional padding done for backward compatibility. 3. Only the names not passing the k8s restrcition are modified to add 9 chars suffix derived from the hash of the name. 4. Similar changes done for helm release name as it was also complaining. Test Plan: 1. Added unit tests 2. Manually created and edited a universe in minikube locally. 3. The change should affect only the names which do not conform to the name k8s validation. ``` This is from platform log for nsing-Test-Universe-2 helm' 'upgrade' 'yb-admin-nsingh-test-universe2-b294f052' '/Users/nkhogen/experiment/k8s/helm/yugabyte-2.13.1.tgz' '-f' '/var/folders/wh/r0qbd7454bx9q5ks7zgkwp4h0000gp/T/6712ab10-d727-4c77-b52a-83da17256ac31578400980055061488.yml' '--namespace' 'yb-admin-nsingh-test-universe2-b294f052' '--timeout' '900s' '--wait' - logging stdout=/var/folders/wh/r0qbd7454bx9q5ks7zgkwp4h0000gp/T/shell_process_out8794086235554227257tmp, stderr=/var/folders/wh/r0qbd7454bx9q5ks7zgkwp4h0000gp/T/shell_process_err7666501670236561566tmp Naorems-MacBook-Pro-2:k8s nkhogen$ k get ns NAME STATUS AGE default Active 9h kube-node-lease Active 9h kube-public Active 9h kube-system Active 9h yb-admin-nsingh-test-universe2-b294f052 Active 5m42s yb-nsingh Naorems-MacBook-Pro-2:k8s nkhogen$ k -n yb-admin-nsingh-test-universe2-b294f052 get pods NAME READY STATUS RESTARTS AGE yb-master-0 2/2 Running 0 2m43s yb-tserver-0 0/2 Pending 0 116s ``` Reviewers: sdu, sanketh Reviewed By: sanketh Subscribers: jenkins-bot, yugaware Differential Revision: https://phabricator.dev.yugabyte.com/D16404
yugabyte · Apr 11, 2022 · 4f5e5c1 · 4f5e5c1
1 parent e0c37b8
commit 4f5e5c1
Show file tree

Hide file tree

Showing 13 changed files with 157 additions and 58 deletions.
diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/KubernetesTaskBase.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/KubernetesTaskBase.java
@@ -11,7 +11,6 @@
 import com.yugabyte.yw.commissioner.tasks.subtasks.KubernetesCommandExecutor.CommandType;
 import com.yugabyte.yw.commissioner.tasks.subtasks.KubernetesWaitForPod;
 import com.yugabyte.yw.common.PlacementInfoUtil;
-import com.yugabyte.yw.common.Util;
 import com.yugabyte.yw.forms.UniverseDefinitionTaskParams;
 import com.yugabyte.yw.models.AvailabilityZone;
 import com.yugabyte.yw.models.Universe;

diff --git a/managed/src/main/java/com/yugabyte/yw/common/KubernetesManager.java b/managed/src/main/java/com/yugabyte/yw/common/KubernetesManager.java
@@ -1,27 +1,25 @@
+// Copyright (c) YugaByte, Inc.
+
 package com.yugabyte.yw.common;
 
+import com.google.common.collect.ImmutableList;
+import io.fabric8.kubernetes.api.model.LoadBalancerIngress;
+import io.fabric8.kubernetes.api.model.Node;
+import io.fabric8.kubernetes.api.model.Pod;
+import io.fabric8.kubernetes.api.model.PodCondition;
+import io.fabric8.kubernetes.api.model.PodStatus;
+import io.fabric8.kubernetes.api.model.Secret;
+import io.fabric8.kubernetes.api.model.Service;
 import java.io.File;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
-
 import javax.annotation.Nullable;
 import javax.inject.Inject;
-
-import com.google.common.collect.ImmutableList;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import io.fabric8.kubernetes.api.model.LoadBalancerIngress;
-import io.fabric8.kubernetes.api.model.Node;
-import io.fabric8.kubernetes.api.model.Pod;
-import io.fabric8.kubernetes.api.model.PodCondition;
-import io.fabric8.kubernetes.api.model.PodStatus;
-import io.fabric8.kubernetes.api.model.Secret;
-import io.fabric8.kubernetes.api.model.Service;
-
 public abstract class KubernetesManager {
 
   @Inject ReleaseManager releaseManager;
@@ -47,12 +45,13 @@ public void helmInstall(
       String overridesFile) {
 
     String helmPackagePath = this.getHelmPackagePath(ybSoftwareVersion);
+    String helmReleaseName = Util.sanitizeHelmReleaseName(universePrefix);
 
     List<String> commandList =
         ImmutableList.of(
             "helm",
             "install",
-            universePrefix,
+            helmReleaseName,
             helmPackagePath,
             "--namespace",
             namespace,
@@ -74,12 +73,13 @@ public void helmUpgrade(
       String overridesFile) {
 
     String helmPackagePath = this.getHelmPackagePath(ybSoftwareVersion);
+    String helmReleaseName = Util.sanitizeHelmReleaseName(universePrefix);
 
     List<String> commandList =
         ImmutableList.of(
             "helm",
             "upgrade",
-            universePrefix,
+            helmReleaseName,
             helmPackagePath,
             "-f",
             overridesFile,
@@ -94,7 +94,8 @@ public void helmUpgrade(
   }
 
   public void helmDelete(Map<String, String> config, String universePrefix, String namespace) {
-    List<String> commandList = ImmutableList.of("helm", "delete", universePrefix, "-n", namespace);
+    String helmReleaseName = Util.sanitizeHelmReleaseName(universePrefix);
+    List<String> commandList = ImmutableList.of("helm", "delete", helmReleaseName, "-n", namespace);
     execCommand(config, commandList);
   }
 

diff --git a/managed/src/main/java/com/yugabyte/yw/common/KubernetesManagerFactory.java b/managed/src/main/java/com/yugabyte/yw/common/KubernetesManagerFactory.java
@@ -1,3 +1,5 @@
+// Copyright (c) YugaByte, Inc.
+
 package com.yugabyte.yw.common;
 
 import com.google.inject.Inject;

diff --git a/managed/src/main/java/com/yugabyte/yw/common/NativeKubernetesManager.java b/managed/src/main/java/com/yugabyte/yw/common/NativeKubernetesManager.java
@@ -1,13 +1,6 @@
-package com.yugabyte.yw.common;
+// Copyright (c) YugaByte, Inc.
 
-import java.io.IOException;
-import java.nio.file.Files;
-import java.nio.file.Paths;
-import java.util.List;
-import java.util.Map;
-
-import javax.annotation.Nullable;
-import javax.inject.Singleton;
+package com.yugabyte.yw.common;
 
 import io.fabric8.kubernetes.api.model.NamespaceBuilder;
 import io.fabric8.kubernetes.api.model.Node;
@@ -18,6 +11,13 @@
 import io.fabric8.kubernetes.client.Config;
 import io.fabric8.kubernetes.client.DefaultKubernetesClient;
 import io.fabric8.kubernetes.client.KubernetesClient;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Map;
+import javax.annotation.Nullable;
+import javax.inject.Singleton;
 
 @Singleton
 public class NativeKubernetesManager extends KubernetesManager {
@@ -36,16 +36,12 @@ private KubernetesClient getClient(Map<String, String> config) {
   }
 
   @Override
-  public void createNamespace(Map<String, String> config, String universePrefix) {
+  public void createNamespace(Map<String, String> config, String namespace) {
     try (KubernetesClient client = getClient(config)) {
       client
           .namespaces()
           .create(
-              new NamespaceBuilder()
-                  .withNewMetadata()
-                  .withName(universePrefix)
-                  .endMetadata()
-                  .build());
+              new NamespaceBuilder().withNewMetadata().withName(namespace).endMetadata().build());
     }
   }
 
@@ -62,11 +58,13 @@ public void applySecret(Map<String, String> config, String namespace, String pul
   @Override
   public List<Pod> getPodInfos(
       Map<String, String> config, String universePrefix, String namespace) {
+    // Implementation specific helm release name.
+    String helmReleaseName = Util.sanitizeHelmReleaseName(universePrefix);
     try (KubernetesClient client = getClient(config)) {
       return client
           .pods()
           .inNamespace(namespace)
-          .withLabel("release", universePrefix)
+          .withLabel("release", helmReleaseName)
           .list()
           .getItems();
     }
@@ -75,11 +73,13 @@ public List<Pod> getPodInfos(
   @Override
   public List<Service> getServices(
       Map<String, String> config, String universePrefix, String namespace) {
+    // Implementation specific helm release name.
+    String helmReleaseName = Util.sanitizeHelmReleaseName(universePrefix);
     try (KubernetesClient client = getClient(config)) {
       return client
           .services()
           .inNamespace(namespace)
-          .withLabel("release", universePrefix)
+          .withLabel("release", helmReleaseName)
           .list()
           .getItems();
     }
@@ -129,18 +129,20 @@ public void updateNumNodes(Map<String, String> config, String namespace, int num
 
   @Override
   public void deleteStorage(Map<String, String> config, String universePrefix, String namespace) {
+    // Implementation specific helm release name.
+    String helmReleaseName = Util.sanitizeHelmReleaseName(universePrefix);
     try (KubernetesClient client = getClient(config)) {
       client
           .persistentVolumeClaims()
           .inNamespace(namespace)
           .withLabel("app", "yb-master")
-          .withLabel("release", universePrefix)
+          .withLabel("release", helmReleaseName)
           .delete();
       client
           .persistentVolumeClaims()
           .inNamespace(namespace)
           .withLabel("app", "yb-tserver")
-          .withLabel("release", universePrefix)
+          .withLabel("release", helmReleaseName)
           .delete();
     }
   }

diff --git a/managed/src/main/java/com/yugabyte/yw/common/PlacementInfoUtil.java b/managed/src/main/java/com/yugabyte/yw/common/PlacementInfoUtil.java
@@ -60,6 +60,7 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import javax.annotation.Nullable;
+import org.apache.commons.lang3.StringUtils;
 import org.joda.time.DateTime;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -2332,8 +2333,15 @@ public static String getKubernetesNamespace(
    */
   public static String getKubernetesNamespace(
       boolean isMultiAZ, String nodePrefix, String azName, Map<String, String> azConfig) {
-    String defaultNamespace = isMultiAZ ? String.format("%s-%s", nodePrefix, azName) : nodePrefix;
-    return azConfig.getOrDefault("KUBENAMESPACE", defaultNamespace);
+    String namespace = azConfig.get("KUBENAMESPACE");
+    if (StringUtils.isBlank(namespace)) {
+      int suffixLen = isMultiAZ ? azName.length() + 1 : 0;
+      namespace = Util.sanitizeKubernetesNamespace(nodePrefix, suffixLen);
+      if (isMultiAZ) {
+        namespace = String.format("%s-%s", namespace, azName);
+      }
+    }
+    return namespace;
   }
 
   // TODO(bhavin192): what if the same namespace is being used for

diff --git a/managed/src/main/java/com/yugabyte/yw/common/ShellKubernetesManager.java b/managed/src/main/java/com/yugabyte/yw/common/ShellKubernetesManager.java
@@ -5,15 +5,6 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.collect.ImmutableList;
 import com.google.inject.Inject;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Map;
-import java.util.concurrent.CancellationException;
-
-import javax.inject.Singleton;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 import io.fabric8.kubernetes.api.model.Node;
 import io.fabric8.kubernetes.api.model.NodeList;
 import io.fabric8.kubernetes.api.model.Pod;
@@ -22,6 +13,13 @@
 import io.fabric8.kubernetes.api.model.Secret;
 import io.fabric8.kubernetes.api.model.Service;
 import io.fabric8.kubernetes.api.model.ServiceList;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.CancellationException;
+import javax.inject.Singleton;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 @Singleton
 public class ShellKubernetesManager extends KubernetesManager {
@@ -57,8 +55,8 @@ private <T> T deserialize(String json, Class<T> type) {
   }
 
   @Override
-  public void createNamespace(Map<String, String> config, String universePrefix) {
-    List<String> commandList = ImmutableList.of("kubectl", "create", "namespace", universePrefix);
+  public void createNamespace(Map<String, String> config, String namespace) {
+    List<String> commandList = ImmutableList.of("kubectl", "create", "namespace", namespace);
     processShellResponse(execCommand(config, commandList));
   }
 
@@ -78,6 +76,8 @@ public void applySecret(Map<String, String> config, String namespace, String pul
   @Override
   public List<Pod> getPodInfos(
       Map<String, String> config, String universePrefix, String namespace) {
+    // Implementation specific helm release name.
+    String helmReleaseName = Util.sanitizeHelmReleaseName(universePrefix);
     List<String> commandList =
         ImmutableList.of(
             "kubectl",
@@ -88,14 +88,16 @@ public List<Pod> getPodInfos(
             "-o",
             "json",
             "-l",
-            "release=" + universePrefix);
+            "release=" + helmReleaseName);
     String response = execCommand(config, commandList).message;
     return deserialize(response, PodList.class).getItems();
   }
 
   @Override
   public List<Service> getServices(
       Map<String, String> config, String universePrefix, String namespace) {
+    // Implementation specific helm release name.
+    String helmReleaseName = Util.sanitizeHelmReleaseName(universePrefix);
     List<String> commandList =
         ImmutableList.of(
             "kubectl",
@@ -106,7 +108,7 @@ public List<Service> getServices(
             "-o",
             "json",
             "-l",
-            "release=" + universePrefix);
+            "release=" + helmReleaseName);
     String response = execCommand(config, commandList).message;
     return deserialize(response, ServiceList.class).getItems();
   }
@@ -182,6 +184,8 @@ public void updateNumNodes(Map<String, String> config, String namespace, int num
 
   @Override
   public void deleteStorage(Map<String, String> config, String universePrefix, String namespace) {
+    // Implementation specific helm release name.
+    String helmReleaseName = Util.sanitizeHelmReleaseName(universePrefix);
     // Delete Master Volumes
     List<String> masterCommandList =
         ImmutableList.of(
@@ -191,7 +195,7 @@ public void deleteStorage(Map<String, String> config, String universePrefix, Str
             "--namespace",
             namespace,
             "-l",
-            "app=yb-master,release=" + universePrefix);
+            "app=yb-master,release=" + helmReleaseName);
     execCommand(config, masterCommandList);
     // Delete TServer Volumes
     List<String> tserverCommandList =
@@ -202,7 +206,7 @@ public void deleteStorage(Map<String, String> config, String universePrefix, Str
             "--namespace",
             namespace,
             "-l",
-            "app=yb-tserver,release=" + universePrefix);
+            "app=yb-tserver,release=" + helmReleaseName);
     execCommand(config, tserverCommandList);
     // TODO: check the execCommand outputs.
   }