[submarine] branch master updated: SUBMARINE-1204. Notebook names cannot start with "submarine"

[ch...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

chishengliu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/submarine.git


The following commit(s) were added to refs/heads/master by this push:
     new 09dd7e94 SUBMARINE-1204. Notebook names cannot start with "submarine"
09dd7e94 is described below

commit 09dd7e946394322f1f91801e9df7863ceec7b969
Author: Richie FY <b0...@ntu.edu.tw>
AuthorDate: Mon May 16 16:17:45 2022 +0800

    SUBMARINE-1204. Notebook names cannot start with "submarine"
    
    ### What is this PR for?
    The name of the notebook cannot start with "submarine". Otherwise, the service name will conflict with the other services that the operator created.
    For example, if the name of the notebook is "submarine-server", then it will conflict with the service "submarine-server" that the operator created.
    This PR aims to avoid this problem by adding the notebookId in front of notebookName at k8sSubmitter, but the notebookId will not display on frontend.
    
    ### What type of PR is it?
    [Bug Fix]
    
    ### Todos
    
    ### What is the Jira issue?
    https://issues.apache.org/jira/projects/SUBMARINE/issues/SUBMARINE-1204?filter=allopenissues
    ### How should this be tested?
    Run the submarine service, and check `kubectl get notebook -n submarine-user-test`, if the notebook name in k8s is an <id>-<name> format, and the frontend only displays notebook name, then it is correct.
    ### Screenshots (if appropriate)
    
    ### Questions:
    * Do the license files need updating? No
    * Are there breaking changes for older versions? No
    * Does this need new documentation? No
    
    Author: Richie FY <b0...@ntu.edu.tw>
    
    Signed-off-by: Chi-Sheng Liu <ch...@apache.org>
    
    Closes #922 from FYRichie/SUBMARINE-1204 and squashes the following commits:
    
    94c8cf0a [Richie FY] remove comment
    83887546 [Richie FY] add id to notebook api test
    7151b769 [Richie FY] findNotebook add notebook id
    f92a2112 [Richie FY] findNotebook list all notebook for match
    d95c9827 [Richie FY] findNotebook add notebook id
    3d9f9ad2 [Richie FY] SUBMARINE-1204. Notebook names cannot start with submarine
---
 .../java/org/apache/submarine/server/api/Submitter.java     |  3 ++-
 .../apache/submarine/server/manager/NotebookManager.java    |  2 +-
 .../apache/submarine/server/submitter/k8s/K8sSubmitter.java | 13 ++++++++++---
 .../java/org/apache/submarine/rest/NotebookRestApiTest.java |  9 +++++++--
 4 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/Submitter.java b/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/Submitter.java
index 7f3a480b..917fe90b 100644
--- a/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/Submitter.java
+++ b/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/Submitter.java
@@ -103,10 +103,11 @@ public interface Submitter {
   /**
    * Find a notebook with spec
    * @param spec spec
+   * @param notebookId notebookId
    * @return object
    * @throws SubmarineRuntimeException running error
    */
-  Notebook findNotebook(NotebookSpec spec) throws SubmarineRuntimeException;
+  Notebook findNotebook(NotebookSpec spec, String notebookId) throws SubmarineRuntimeException;
 
   /**
    * Delete a notebook with spec
diff --git a/submarine-server/server-core/src/main/java/org/apache/submarine/server/manager/NotebookManager.java b/submarine-server/server-core/src/main/java/org/apache/submarine/server/manager/NotebookManager.java
index add2eb26..c6918095 100644
--- a/submarine-server/server-core/src/main/java/org/apache/submarine/server/manager/NotebookManager.java
+++ b/submarine-server/server-core/src/main/java/org/apache/submarine/server/manager/NotebookManager.java
@@ -155,7 +155,7 @@ public class NotebookManager {
       throw new SubmarineRuntimeException(Response.Status.NOT_FOUND.getStatusCode(),
           "Notebook not found.");
     }
-    Notebook foundNotebook = submitter.findNotebook(notebook.getSpec());
+    Notebook foundNotebook = submitter.findNotebook(notebook.getSpec(), id);
     foundNotebook.rebuild(notebook);
     foundNotebook.setNotebookId(NotebookId.fromString(id));
 
diff --git a/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sSubmitter.java b/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sSubmitter.java
index afa9296f..2be4c00d 100644
--- a/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sSubmitter.java
+++ b/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sSubmitter.java
@@ -466,7 +466,7 @@ public class K8sSubmitter implements Submitter {
   @Override
   public Notebook createNotebook(NotebookSpec spec, String notebookId) throws SubmarineRuntimeException {
     Notebook notebook;
-    final String name = spec.getMeta().getName();
+    final String name = String.format("%s-%s", notebookId.replace("_", "-"), spec.getMeta().getName());
     final String scName = NotebookUtils.SC_NAME;
     final String host = NotebookUtils.HOST_PATH;
     final String workspacePvc = String.format("%s-%s", NotebookUtils.PVC_PREFIX, name);
@@ -478,6 +478,7 @@ public class K8sSubmitter implements Submitter {
     NotebookCR notebookCR;
     try {
       notebookCR = NotebookSpecParser.parseNotebook(spec, namespace);
+      notebookCR.getMetadata().setName(name);
       notebookCR.getMetadata().setNamespace(namespace);
       notebookCR.getMetadata().setOwnerReferences(OwnerReferenceUtils.getOwnerReference());
     } catch (JsonSyntaxException e) {
@@ -562,16 +563,19 @@ public class K8sSubmitter implements Submitter {
               "object failed by " + e.getMessage());
     }
 
+    notebook.setName(spec.getMeta().getName());
     return notebook;
   }
 
   @Override
-  public Notebook findNotebook(NotebookSpec spec) throws SubmarineRuntimeException {
+  public Notebook findNotebook(NotebookSpec spec, String notebookId) throws SubmarineRuntimeException {
     Notebook notebook = null;
+    final String name = String.format("%s-%s", notebookId.replace("_", "-"), spec.getMeta().getName());
     String namespace = getServerNamespace();
 
     try {
       NotebookCR notebookCR = NotebookSpecParser.parseNotebook(spec, null);
+      notebookCR.getMetadata().setName(name);
       Object object = notebookCRClient.get(namespace, notebookCR.getMetadata().getName())
               .throwsApiException().getObject();
       notebook = NotebookUtils.parseObject(object, NotebookUtils.ParseOpt.PARSE_OPT_GET);
@@ -589,13 +593,15 @@ public class K8sSubmitter implements Submitter {
       notebook.setReason(e.getMessage());
       notebook.setStatus(Notebook.Status.STATUS_NOT_FOUND.getValue());
     }
+    
+    notebook.setName(spec.getMeta().getName());
     return notebook;
   }
 
   @Override
   public Notebook deleteNotebook(NotebookSpec spec, String notebookId) throws SubmarineRuntimeException {
     Notebook notebook = null;
-    final String name = spec.getMeta().getName();
+    final String name = String.format("%s-%s", notebookId.replace("_", "-"), spec.getMeta().getName());
     String namespace = getServerNamespace();
     NotebookCR notebookCR = NotebookSpecParser.parseNotebook(spec, null);
     AgentPod agentPod = new AgentPod(namespace, spec.getMeta().getName(),
@@ -638,6 +644,7 @@ public class K8sSubmitter implements Submitter {
             spec.getMeta().getName(), agentPod.getMetadata().getName()));
     podClient.delete(agentPod.getMetadata().getNamespace(), agentPod.getMetadata().getName());
 
+    notebook.setName(spec.getMeta().getName());
     return notebook;
   }
 
diff --git a/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/NotebookRestApiTest.java b/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/NotebookRestApiTest.java
index ccfb8859..9f2fae5c 100644
--- a/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/NotebookRestApiTest.java
+++ b/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/NotebookRestApiTest.java
@@ -287,8 +287,11 @@ public class NotebookRestApiTest extends AbstractSubmarineServerTest {
   }
 
   private void assertGetK8sResult(Notebook notebook) throws Exception {
+    final String k8sName = String.format("%s-%s", notebook.getNotebookId().toString().replace('_', '-'),
+        notebook.getName());
+        
     JsonObject rootObject = getNotebookByK8sApi(GROUP, VERSION, notebook.getSpec().getMeta().getNamespace(),
-        PLURAL, notebook.getName());
+        PLURAL, k8sName);
 
     JsonObject metadataObject = rootObject.getAsJsonObject("metadata");
     String uid = metadataObject.getAsJsonPrimitive("uid").getAsString();
@@ -312,9 +315,11 @@ public class NotebookRestApiTest extends AbstractSubmarineServerTest {
 
   private void assertDeleteK8sResult(Notebook notebook) {
     JsonObject rootObject = null;
+    final String k8sName = String.format("%s-%s", notebook.getNotebookId().toString().replace('_', '-'),
+        notebook.getName());
     try {
       rootObject = getNotebookByK8sApi(GROUP, VERSION, notebook.getSpec().getMeta().getNamespace(),
-          PLURAL, notebook.getName());
+          PLURAL, k8sName);
     } catch (ApiException e) {
       Assert.assertEquals(Response.Status.NOT_FOUND.getStatusCode(), e.getCode());
     } finally {


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@submarine.apache.org
For additional commands, e-mail: dev-help@submarine.apache.org