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 ddf02aa SUBMARINE-954. Experiments and notebooks are still alive
after submarine is deleted
ddf02aa is described below
commit ddf02aab80df755ff87986813b2a332f399f9f6b
Author: Kenchu123 <k889...@gmail.com>
AuthorDate: Tue Aug 10 19:20:49 2021 +0800
SUBMARINE-954. Experiments and notebooks are still alive after submarine is
deleted
### What is this PR for?
<!-- A few sentences describing the overall goals of the pull request's
commits.
First time? Check out the contributing guide -
https://submarine.apache.org/contribution/contributions.html
-->
After the submarine is down, we can find that the experiment and notebook
pods are still alive in the cluster.
Reproduce steps:
1. create a submarine
2. access to the workbench by port forwarding
3. add notebook or experiment from the workbench
4. delete submarine
5. we can find out that notebooks, tfjobs, pytorchjobs and relating
components are still alive.
To fix the bug, we can maybe add onwerReferences to the created resources.
Reference:
ownerReference:
https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/
ps. This only works on submarine created by submarine-operator. If you use
helm to install submarine, you should clean up by yourself.
### What type of PR is it?
[Bug Fix]
### Todos
* [x] - add submarine custom resource apiVersion, kind, name and uid to
submarine-server's environment on creating deployment by submarine-operator
* [x] - set ownerReference on creating notebooks (including
persistentVolumeClaim, ingressroute) and MLJobs
### What is the Jira issue?
<!-- * Open an issue on Jira
https://issues.apache.org/jira/browse/SUBMARINE/
* Put link here, and add [SUBMARINE-*Jira number*] in PR title, eg.
`SUBMARINE-23. PR title`
-->
https://issues.apache.org/jira/browse/SUBMARINE-954
### How should this be tested?
<!--
* First time? Setup Travis CI as described on
https://submarine.apache.org/contribution/contributions.html#continuous-integration
* Strongly recommended: add automated unit tests for any new or changed
behavior
* Outline any manual steps to test the PR here.
-->
1. Follow submarine-operator
[README.md](https://github.com/apache/submarine/tree/master/submarine-cloud-v2)
to setup submarine
2. Create notebooks and experiments
3. Delete submarine custom resource
And you can find out that all the stuff was deleted.
### Screenshots (if appropriate)
https://user-images.githubusercontent.com/17617373/128871152-854a6813-3a4a-445c-ae26-e98aae91876f.mov
### Questions:
* Do the license files need updating? No
* Are there breaking changes for older versions? No
* Does this need new documentation? No
Author: Kenchu123 <k889...@gmail.com>
Signed-off-by: Kevin <pings...@apache.org>
Closes #705 from Kenchu123/SUBMARINE-954 and squashes the following commits:
85a5a032 [Kenchu123] SUBMARINE-954. Set Notebook and MLJob's ownerReference
to submarineCR.
---
.../pkg/controller/submarine_server.go | 20 ++++++-
.../server/submitter/k8s/K8sSubmitter.java | 68 ++++++++++------------
.../submitter/k8s/util/OwnerReferenceUtils.java | 52 +++++++++++++++++
3 files changed, 103 insertions(+), 37 deletions(-)
diff --git a/submarine-cloud-v2/pkg/controller/submarine_server.go
b/submarine-cloud-v2/pkg/controller/submarine_server.go
index a1c7a0b..10ac5e3 100644
--- a/submarine-cloud-v2/pkg/controller/submarine_server.go
+++ b/submarine-cloud-v2/pkg/controller/submarine_server.go
@@ -75,11 +75,13 @@ func newSubmarineServerDeployment(submarine
*v1alpha1.Submarine) *appsv1.Deploym
serverImage = "apache/submarine:server-" +
submarine.Spec.Version
}
+ ownerReference := *metav1.NewControllerRef(submarine,
v1alpha1.SchemeGroupVersion.WithKind("Submarine"))
+
return &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: serverName,
OwnerReferences: []metav1.OwnerReference{
- *metav1.NewControllerRef(submarine,
v1alpha1.SchemeGroupVersion.WithKind("Submarine")),
+ ownerReference,
},
},
Spec: appsv1.DeploymentSpec{
@@ -122,6 +124,22 @@ func newSubmarineServerDeployment(submarine
*v1alpha1.Submarine) *appsv1.Deploym
Name:
"ENV_NAMESPACE",
Value:
submarine.Namespace,
},
+ {
+ Name:
"SUBMARINE_APIVERSION",
+ Value:
ownerReference.APIVersion,
+ },
+ {
+ Name:
"SUBMARINE_KIND",
+ Value:
ownerReference.Kind,
+ },
+ {
+ Name:
"SUBMARINE_NAME",
+ Value:
ownerReference.Name,
+ },
+ {
+ Name:
"SUBMARINE_UID",
+ Value:
string(ownerReference.UID),
+ },
},
Ports:
[]corev1.ContainerPort{
{
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 063b978..bf1d645 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
@@ -72,6 +72,8 @@ import
org.apache.submarine.server.submitter.k8s.parser.ServeSpecParser;
import org.apache.submarine.server.submitter.k8s.parser.VolumeSpecParser;
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.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -137,6 +139,8 @@ public class K8sSubmitter implements Submitter {
Experiment experiment;
try {
MLJob mlJob = ExperimentSpecParser.parseJob(spec);
+ mlJob.getMetadata().setNamespace(getServerNamespace());
+
mlJob.getMetadata().setOwnerReferences(OwnerReferenceUtils.getOwnerReference());
Object object = api.createNamespacedCustomObject(mlJob.getGroup(),
mlJob.getVersion(),
mlJob.getMetadata().getNamespace(), mlJob.getPlural(), mlJob,
"true");
@@ -157,6 +161,8 @@ public class K8sSubmitter implements Submitter {
Experiment experiment;
try {
MLJob mlJob = ExperimentSpecParser.parseJob(spec);
+ mlJob.getMetadata().setNamespace(getServerNamespace());
+
Object object = api.getNamespacedCustomObject(mlJob.getGroup(),
mlJob.getVersion(),
mlJob.getMetadata().getNamespace(), mlJob.getPlural(),
mlJob.getMetadata().getName());
experiment = parseExperimentResponseObject(object,
ParseOp.PARSE_OP_RESULT);
@@ -175,6 +181,8 @@ public class K8sSubmitter implements Submitter {
Experiment experiment;
try {
MLJob mlJob = ExperimentSpecParser.parseJob(spec);
+ mlJob.getMetadata().setNamespace(getServerNamespace());
+
Object object = api.patchNamespacedCustomObject(mlJob.getGroup(),
mlJob.getVersion(),
mlJob.getMetadata().getNamespace(), mlJob.getPlural(),
mlJob.getMetadata().getName(),
mlJob);
@@ -192,6 +200,8 @@ public class K8sSubmitter implements Submitter {
Experiment experiment;
try {
MLJob mlJob = ExperimentSpecParser.parseJob(spec);
+ mlJob.getMetadata().setNamespace(getServerNamespace());
+
Object object = api.deleteNamespacedCustomObject(mlJob.getGroup(),
mlJob.getVersion(),
mlJob.getMetadata().getNamespace(), mlJob.getPlural(),
mlJob.getMetadata().getName(),
MLJobConverter.toDeleteOptionsFromMLJob(mlJob), null, null, null);
@@ -229,7 +239,7 @@ public class K8sSubmitter implements Submitter {
experimentLog.setExperimentId(id);
try {
final V1PodList podList = coreApi.listNamespacedPod(
- spec.getMeta().getNamespace(),
+ getServerNamespace(),
"false", null, null,
getJobLabelSelector(spec), null, null,
null, null);
@@ -249,16 +259,15 @@ public class K8sSubmitter implements Submitter {
experimentLog.setExperimentId(id);
try {
final V1PodList podList = coreApi.listNamespacedPod(
- spec.getMeta().getNamespace(),
+ getServerNamespace(),
"false", null, null,
getJobLabelSelector(spec), null, null,
null, null);
for (V1Pod pod : podList.getItems()) {
String podName = pod.getMetadata().getName();
- String namespace = pod.getMetadata().getNamespace();
String podLog = coreApi.readNamespacedPodLog(
- podName, namespace, null, Boolean.FALSE,
+ podName, getServerNamespace(), null, Boolean.FALSE,
Integer.MAX_VALUE, null, Boolean.FALSE,
Integer.MAX_VALUE, null, Boolean.FALSE);
@@ -274,10 +283,7 @@ public class K8sSubmitter implements Submitter {
public TensorboardInfo getTensorboardInfo() throws SubmarineRuntimeException
{
final String name = "submarine-tensorboard";
final String ingressRouteName = "submarine-tensorboard-ingressroute";
- String namespace = "default";
- if (System.getenv(ENV_NAMESPACE) != null) {
- namespace = System.getenv(ENV_NAMESPACE);
- }
+ String namespace = getServerNamespace();
try {
V1Deployment deploy = appsV1Api.readNamespacedDeploymentStatus(name,
namespace, "true");
@@ -316,10 +322,7 @@ public class K8sSubmitter implements Submitter {
public MlflowInfo getMlflowInfo() throws SubmarineRuntimeException {
final String name = "submarine-mlflow";
final String ingressRouteName = "submarine-mlflow-ingressroute";
- String namespace = "default";
- if (System.getenv(ENV_NAMESPACE) != null) {
- namespace = System.getenv(ENV_NAMESPACE);
- }
+ String namespace = getServerNamespace();
try {
V1Deployment deploy = appsV1Api.readNamespacedDeploymentStatus(name,
namespace, "true");
@@ -362,12 +365,8 @@ public class K8sSubmitter implements Submitter {
final String host = NotebookUtils.HOST_PATH;
final String storage = NotebookUtils.STORAGE;
final String pvcName = NotebookUtils.PVC_PREFIX + name;
- String namespace = "default";
-
- if (System.getenv(ENV_NAMESPACE) != null) {
- namespace = System.getenv(ENV_NAMESPACE);
- }
-
+ String namespace = getServerNamespace();
+
// parse notebook custom resource
NotebookCR notebookCR;
try {
@@ -376,6 +375,7 @@ public class K8sSubmitter implements Submitter {
labels.put(NotebookCR.NOTEBOOK_OWNER_SELECTOR_KET,
spec.getMeta().getOwnerId());
notebookCR.getMetadata().setLabels(labels);
notebookCR.getMetadata().setNamespace(namespace);
+
notebookCR.getMetadata().setOwnerReferences(OwnerReferenceUtils.getOwnerReference());
} catch (JsonSyntaxException e) {
LOG.error("K8s submitter: parse response object failed by " +
e.getMessage(), e);
throw new SubmarineRuntimeException(500, "K8s Submitter parse upstream
response failed.");
@@ -425,11 +425,7 @@ public class K8sSubmitter implements Submitter {
@Override
public Notebook findNotebook(NotebookSpec spec) throws
SubmarineRuntimeException {
Notebook notebook;
- String namespace = "default";
-
- if (System.getenv(ENV_NAMESPACE) != null) {
- namespace = System.getenv(ENV_NAMESPACE);
- }
+ String namespace = getServerNamespace();
try {
NotebookCR notebookCR = NotebookSpecParser.parseNotebook(spec);
@@ -448,11 +444,7 @@ public class K8sSubmitter implements Submitter {
Notebook notebook;
final String name = spec.getMeta().getName();
final String pvcName = NotebookUtils.PVC_PREFIX + name;
- String namespace = "default";
-
- if (System.getenv(ENV_NAMESPACE) != null) {
- namespace = System.getenv(ENV_NAMESPACE);
- }
+ String namespace = getServerNamespace();
try {
NotebookCR notebookCR = NotebookSpecParser.parseNotebook(spec);
@@ -473,13 +465,8 @@ public class K8sSubmitter implements Submitter {
@Override
public List<Notebook> listNotebook(String id) throws
SubmarineRuntimeException {
List<Notebook> notebookList;
-
- String namespace = "default";
-
- if (System.getenv(ENV_NAMESPACE) != null) {
- namespace = System.getenv(ENV_NAMESPACE);
- }
-
+ String namespace = getServerNamespace();
+
try {
Object object =
api.listNamespacedCustomObject(NotebookCR.CRD_NOTEBOOK_GROUP_V1,
NotebookCR.CRD_NOTEBOOK_VERSION_V1, namespace,
NotebookCR.CRD_NOTEBOOK_PLURAL_V1,
@@ -561,7 +548,7 @@ public class K8sSubmitter implements Submitter {
public void createPersistentVolumeClaim(String pvcName, String namespace,
String scName, String storage)
throws ApiException {
V1PersistentVolumeClaim pvc =
VolumeSpecParser.parsePersistentVolumeClaim(pvcName, scName, storage);
-
+
pvc.getMetadata().setOwnerReferences(OwnerReferenceUtils.getOwnerReference());
try {
V1PersistentVolumeClaim result =
coreApi.createNamespacedPersistentVolumeClaim(
namespace, pvc, "true", null, null
@@ -618,6 +605,7 @@ public class K8sSubmitter implements Submitter {
V1ObjectMeta meta = new V1ObjectMeta();
meta.setName(name);
meta.setNamespace(namespace);
+ meta.setOwnerReferences(OwnerReferenceUtils.getOwnerReference());
ingressRoute.setMetadata(meta);
ingressRoute.setSpec(parseIngressRouteSpec(meta.getNamespace(),
meta.getName()));
api.createNamespacedCustomObject(
@@ -689,6 +677,14 @@ public class K8sSubmitter implements Submitter {
}
}
+ private String getServerNamespace() {
+ String namespace = "default";
+ if (System.getenv(ENV_NAMESPACE) != null) {
+ namespace = System.getenv(ENV_NAMESPACE);
+ }
+ return namespace;
+ }
+
private enum ParseOp {
PARSE_OP_RESULT,
PARSE_OP_DELETE
diff --git
a/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/util/OwnerReferenceUtils.java
b/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/util/OwnerReferenceUtils.java
new file mode 100644
index 0000000..4a319d1
--- /dev/null
+++
b/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/util/OwnerReferenceUtils.java
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.submarine.server.submitter.k8s.util;
+
+import java.util.ArrayList;
+
+import io.kubernetes.client.models.V1OwnerReference;
+
+public class OwnerReferenceUtils {
+ private static final String SUBMARINE_APIVERSION = "SUBMARINE_APIVERSION";
+ private static final String SUBMARINE_KIND = "SUBMARINE_KIND";
+ private static final String SUBMARINE_NAME = "SUBMARINE_NAME";
+ private static final String SUBMARINE_UID = "SUBMARINE_UID";
+
+ public static ArrayList<V1OwnerReference> getOwnerReference() {
+ ArrayList<V1OwnerReference> ownerReferences = new ArrayList<>();
+ V1OwnerReference owner = new V1OwnerReference();
+ if (System.getenv(SUBMARINE_UID) != null) {
+ String apiVersion = System.getenv(SUBMARINE_APIVERSION);
+ String kind = System.getenv(SUBMARINE_KIND);
+ String name = System.getenv(SUBMARINE_NAME);
+ String uid = System.getenv(SUBMARINE_UID);
+ Boolean blockOwnerDeletion = true;
+ Boolean controller = true;
+ owner.setApiVersion(apiVersion);
+ owner.setKind(kind);
+ owner.setName(name);
+ owner.setUid(uid);
+ owner.setBlockOwnerDeletion(blockOwnerDeletion);
+ owner.setController(controller);
+ ownerReferences.add(owner);
+ }
+ return ownerReferences;
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@submarine.apache.org
For additional commands, e-mail: dev-h...@submarine.apache.org
