This is an automated email from the ASF dual-hosted git repository.
pingsutw 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 a9b97bc SUBMARINE-1124 Add the status of NOT_FOUND to replace the
delete operation
a9b97bc is described below
commit a9b97bc040cabc40807637834eab2106502271e6
Author: Thinking <[email protected]>
AuthorDate: Mon Jan 10 21:44:55 2022 +0800
SUBMARINE-1124 Add the status of NOT_FOUND to replace the delete operation
### What is this PR for?
Submarine delete notebook row from mysql database when k8s client can not
find this related NoteBook CRD. But sometimes maybe due to timeout or API error
caused by network and other reasons, it also report an error. At this time,
submarine will think that the resource has been lost and forcibly delete it.
This is not a perfect logic. I think the better logic is to prompt the
foreground that this resource has been lost and allow the user to delete it by
himself.
At the same time, the logic of deletion should also be readjusted. If the
resource does not exist, the program can continue to execute without throwing
an exception. Otherwise, an endless loop will appear, and users cannot add or
delete resources with the same name.
### What type of PR is it?
Bug Fix
### Todos
* [x] - Add a new status NOT_FOUND
* [x] - Modify delete logic, if resources do not exist, skip it.
### What is the Jira issue?
https://issues.apache.org/jira/projects/SUBMARINE/issues/SUBMARINE-1124
### How should this be tested?
Right now there no new test cases.
### 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: Thinking <[email protected]>
Author: Thinking Chen <[email protected]>
Signed-off-by: Kevin <[email protected]>
Closes #855 from cdmikechen/SUBMARINE-1124 and squashes the following
commits:
7fa892d8 [Thinking] remove exception
c356aa6b [Thinking Chen] Merge branch 'master' into SUBMARINE-1124
562f2f23 [Thinking] Remove error data time format with timezone
1b60ba2b [Thinking] SUBMARINE-1124 Add the status of NOT_FOUND to replace
the delete operation
---
.../submarine/commons/utils/SubmarineConfVars.java | 4 +-
.../submarine/server/api/notebook/Notebook.java | 1 +
.../submarine/server/notebook/NotebookManager.java | 6 +-
.../server/submitter/k8s/K8sSubmitter.java | 100 +++++++++++++--------
.../server/submitter/k8s/util/NotebookUtils.java | 7 +-
5 files changed, 77 insertions(+), 41 deletions(-)
diff --git
a/submarine-commons/commons-utils/src/main/java/org/apache/submarine/commons/utils/SubmarineConfVars.java
b/submarine-commons/commons-utils/src/main/java/org/apache/submarine/commons/utils/SubmarineConfVars.java
index 42622e0..a96a885 100644
---
a/submarine-commons/commons-utils/src/main/java/org/apache/submarine/commons/utils/SubmarineConfVars.java
+++
b/submarine-commons/commons-utils/src/main/java/org/apache/submarine/commons/utils/SubmarineConfVars.java
@@ -54,7 +54,9 @@ public class SubmarineConfVars {
JDBC_DRIVERCLASSNAME("jdbc.driverClassName", "com.mysql.jdbc.Driver"),
JDBC_URL("jdbc.url", "jdbc:mysql://127.0.0.1:3306/submarine" +
"?useUnicode=true&characterEncoding=UTF-8&autoReconnect=true&allowMultiQueries=true&"
+
-
"failOverReadOnly=false&zeroDateTimeBehavior=convertToNull&useSSL=false"),
+
"failOverReadOnly=false&zeroDateTimeBehavior=convertToNull&useSSL=false&" +
+ // use timezone for dateformat, current default database timezone is
utc
+ "serverTimezone=UTC&useTimezone=true&useLegacyDatetimeCode=true"),
JDBC_USERNAME("jdbc.username", "submarine"),
JDBC_PASSWORD("jdbc.password", "password"),
METASTORE_JDBC_URL("metastore.jdbc.url",
"jdbc:mysql://127.0.0.1:3306/metastore" +
diff --git
a/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/notebook/Notebook.java
b/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/notebook/Notebook.java
index db8d055..6b048d2 100644
---
a/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/notebook/Notebook.java
+++
b/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/notebook/Notebook.java
@@ -112,6 +112,7 @@ public class Notebook {
STATUS_RUNNING("running"),
STATUS_WAITING("waiting"),
STATUS_TERMINATING("terminating"),
+ STATUS_NOT_FOUND("not_found"),
STATUS_PULLING("pulling");
private String value;
diff --git
a/submarine-server/server-core/src/main/java/org/apache/submarine/server/notebook/NotebookManager.java
b/submarine-server/server-core/src/main/java/org/apache/submarine/server/notebook/NotebookManager.java
index cb373cd..17abae3 100644
---
a/submarine-server/server-core/src/main/java/org/apache/submarine/server/notebook/NotebookManager.java
+++
b/submarine-server/server-core/src/main/java/org/apache/submarine/server/notebook/NotebookManager.java
@@ -142,10 +142,12 @@ public class NotebookManager {
Notebook notebook = submitter.findNotebook(nb.getSpec());
notebook.setNotebookId(nb.getNotebookId());
notebook.setSpec(nb.getSpec());
+ if (notebook.getCreatedTime() == null) {
+ notebook.setCreatedTime(nb.getCreatedTime());
+ }
notebookList.add(notebook);
} catch (SubmarineRuntimeException e) {
- LOG.warn("Submitter can not find notebook: {}, will delete it",
nb.getNotebookId());
- notebookService.delete(nb.getNotebookId().toString());
+ LOG.error("Error when get notebook resource, skip this row!", e);
}
}
return notebookList;
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 b5e44c8..dc1c34f 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
@@ -28,7 +28,7 @@ import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
-
+import java.util.function.Function;
import com.google.common.reflect.TypeToken;
import com.google.gson.Gson;
@@ -89,6 +89,7 @@ import
org.apache.submarine.server.submitter.k8s.util.MLJobConverter;
import org.apache.submarine.server.submitter.k8s.util.NotebookUtils;
import org.apache.submarine.server.submitter.k8s.util.OwnerReferenceUtils;
+import org.joda.time.DateTime;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -96,6 +97,7 @@ import org.slf4j.LoggerFactory;
* JobSubmitter for Kubernetes Cluster.
*/
public class K8sSubmitter implements Submitter {
+
private static final Logger LOG =
LoggerFactory.getLogger(K8sSubmitter.class);
private static final String KUBECONFIG_ENV = "KUBECONFIG";
@@ -105,6 +107,17 @@ public class K8sSubmitter implements Submitter {
private static final String ENV_NAMESPACE = "ENV_NAMESPACE";
+
+ // Add an exception Consumer, handle the problem that delete operation does
not have the resource
+ public static final Function<ApiException, Object>
API_EXCEPTION_404_CONSUMER = e -> {
+ if (e.getCode() != 404) {
+ LOG.error("When submit resource to k8s get ApiException with code " +
e.getCode(), e);
+ throw new SubmarineRuntimeException(e.getCode(), e.getMessage());
+ } else {
+ return null;
+ }
+ };
+
private static final String OVERWRITE_JSON;
static {
@@ -113,6 +126,7 @@ public class K8sSubmitter implements Submitter {
SubmarineConfVars.ConfVars.SUBMARINE_NOTEBOOK_DEFAULT_OVERWRITE_JSON);
}
+
// K8s API client for CRD
private CustomObjectsApi api;
@@ -478,7 +492,7 @@ public class K8sSubmitter implements Submitter {
@Override
public Notebook findNotebook(NotebookSpec spec) throws
SubmarineRuntimeException {
- Notebook notebook;
+ Notebook notebook = null;
String namespace = getServerNamespace();
try {
@@ -508,39 +522,62 @@ public class K8sSubmitter implements Submitter {
}
}
} catch (ApiException e) {
- throw new SubmarineRuntimeException(e.getCode(), e.getMessage());
+ // SUBMARINE-1124
+ // The exception that obtaining CRD resources is not necessarily because
the CRD is deleted,
+ // but maybe due to timeout or API error caused by network and other
reasons.
+ // Therefore, the status of the notebook should be set to a new enum
NOTFOUND.
+ LOG.warn("Get error when submitter is finding notebook: {}",
spec.getMeta().getName());
+ if (notebook == null) {
+ notebook = new Notebook();
+ }
+ notebook.setName(spec.getMeta().getName());
+ notebook.setSpec(spec);
+ notebook.setReason(e.getMessage());
+ notebook.setStatus(Notebook.Status.STATUS_NOT_FOUND.getValue());
}
return notebook;
}
@Override
public Notebook deleteNotebook(NotebookSpec spec) throws
SubmarineRuntimeException {
- Notebook notebook;
+ Notebook notebook = null;
final String name = spec.getMeta().getName();
String namespace = getServerNamespace();
+ NotebookCR notebookCR = NotebookSpecParser.parseNotebook(spec);
try {
- NotebookCR notebookCR = NotebookSpecParser.parseNotebook(spec);
Object object = api.deleteNamespacedCustomObject(notebookCR.getGroup(),
notebookCR.getVersion(),
namespace, notebookCR.getPlural(),
notebookCR.getMetadata().getName(), null, null, null,
- null, new
V1DeleteOptionsBuilder().withApiVersion(notebookCR.getApiVersion()).build());
+ null, new
V1DeleteOptionsBuilder().withApiVersion(notebookCR.getApiVersion()).build());
notebook = NotebookUtils.parseObject(object,
NotebookUtils.ParseOpt.PARSE_OPT_DELETE);
- deleteIngressRoute(namespace, notebookCR.getMetadata().getName());
+ } catch (ApiException e) {
+ API_EXCEPTION_404_CONSUMER.apply(e);
+ } finally {
+ if (notebook == null) {
+ // add metadata time info
+ notebookCR.getMetadata().setDeletionTimestamp(new DateTime());
+ // build notebook response
+ notebook = NotebookUtils.buildNotebookResponse(notebookCR);
+ notebook.setStatus(Notebook.Status.STATUS_NOT_FOUND.getValue());
+ notebook.setReason("The notebook instance is not found");
+ }
+ }
- // delete pvc
- // workspace pvc
- deletePersistentVolumeClaim(String.format("%s-%s",
NotebookUtils.PVC_PREFIX, name), namespace);
- // user set pvc
- deletePersistentVolumeClaim(String.format("%s-user-%s",
NotebookUtils.PVC_PREFIX, name), namespace);
+ // delete ingress route
+ deleteIngressRoute(namespace, name);
- // configmap
- if (StringUtils.isNoneBlank(OVERWRITE_JSON)) {
- deleteConfigMap(namespace, String.format("%s-%s",
NotebookUtils.OVERWRITE_PREFIX, name));
- }
- } catch (ApiException e) {
- throw new SubmarineRuntimeException(e.getCode(), e.getMessage());
+ // delete pvc
+ // workspace pvc
+ deletePersistentVolumeClaim(String.format("%s-%s",
NotebookUtils.PVC_PREFIX, name), namespace);
+ // user set pvc
+ deletePersistentVolumeClaim(String.format("%s-user-%s",
NotebookUtils.PVC_PREFIX, name), namespace);
+
+ // configmap
+ if (StringUtils.isNoneBlank(OVERWRITE_JSON)) {
+ deleteConfigMap(namespace, String.format("%s-%s",
NotebookUtils.OVERWRITE_PREFIX, name));
}
+
return notebook;
}
@@ -734,7 +771,7 @@ public class K8sSubmitter implements Submitter {
}
}
- public void deletePersistentVolumeClaim(String pvcName, String namespace)
throws ApiException {
+ public void deletePersistentVolumeClaim(String pvcName, String namespace) {
/*
This version of Kubernetes-client/java has bug here.
It will trigger exception as in
https://github.com/kubernetes-client/java/issues/86
@@ -748,7 +785,7 @@ public class K8sSubmitter implements Submitter {
);
} catch (ApiException e) {
LOG.error("Exception when deleting persistent volume claim " +
e.getMessage(), e);
- throw e;
+ API_EXCEPTION_404_CONSUMER.apply(e);
} catch (JsonSyntaxException e) {
if (e.getCause() instanceof IllegalStateException) {
IllegalStateException ise = (IllegalStateException) e.getCause();
@@ -803,7 +840,7 @@ public class K8sSubmitter implements Submitter {
null, new
V1DeleteOptionsBuilder().withApiVersion(IngressRoute.CRD_APIVERSION_V1).build());
} catch (ApiException e) {
LOG.error("K8s submitter: Delete Traefik custom resource object failed
by " + e.getMessage(), e);
- throw new SubmarineRuntimeException(e.getCode(), e.getMessage());
+ API_EXCEPTION_404_CONSUMER.apply(e);
}
}
@@ -862,14 +899,14 @@ public class K8sSubmitter implements Submitter {
/**
* Delete ConfigMap
*/
- public void deleteConfigMap(String namespace, String name) throws
ApiException {
+ public void deleteConfigMap(String namespace, String name) {
try {
coreApi.deleteNamespacedConfigMap(name, namespace,
"true", null, null, null,
null, null);
} catch (ApiException e) {
LOG.error("Exception when deleting config map " + e.getMessage(), e);
- throw e;
+ API_EXCEPTION_404_CONSUMER.apply(e);
}
}
@@ -878,23 +915,14 @@ public class K8sSubmitter implements Submitter {
*/
private void rollbackCreationConfigMap(String namespace, String ... names)
throws SubmarineRuntimeException {
- try {
- for (String name : names) {
- deleteConfigMap(namespace, name);
- }
- } catch (ApiException e) {
- throw new SubmarineRuntimeException(e.getCode(), e.getMessage());
+ for (String name : names) {
+ deleteConfigMap(namespace, name);
}
}
private void rollbackCreationPVC(String namespace, String ... pvcNames) {
- try {
- for (String pvcName : pvcNames) {
- deletePersistentVolumeClaim(pvcName, namespace);
- }
- } catch (ApiException e) {
- LOG.error("K8s submitter: delete persistent volume claim failed by {},
may cause some dirty data",
- e.getMessage());
+ for (String pvcName : pvcNames) {
+ deletePersistentVolumeClaim(pvcName, namespace);
}
}
diff --git
a/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/util/NotebookUtils.java
b/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/util/NotebookUtils.java
index 504c21d..126b663 100644
---
a/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/util/NotebookUtils.java
+++
b/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/util/NotebookUtils.java
@@ -96,11 +96,14 @@ public class NotebookUtils {
throw new SubmarineRuntimeException(500, "K8s Submitter parse upstream
response failed.");
}
- private static Notebook buildNotebookResponse(NotebookCR notebookCR) {
+ public static Notebook buildNotebookResponse(NotebookCR notebookCR) {
Notebook notebook = new Notebook();
notebook.setUid(notebookCR.getMetadata().getUid());
notebook.setName(notebookCR.getMetadata().getName());
-
notebook.setCreatedTime(notebookCR.getMetadata().getCreationTimestamp().toString());
+ if (notebookCR.getMetadata().getCreationTimestamp() != null) {
+
notebook.setCreatedTime(notebookCR.getMetadata().getCreationTimestamp().toString());
+ }
+
// notebook url
notebook.setUrl("/notebook/" + notebookCR.getMetadata().getNamespace() +
"/" +
notebookCR.getMetadata().getName() + "/lab");
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
