This is an automated email from the ASF dual-hosted git repository.
liuxun 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 e0fdfb21 SUBMARINE-1304. Fix bug with agent listening to resource
names (#988)
e0fdfb21 is described below
commit e0fdfb21bd125c4e4b18fb5dd71ac904c26d04ff
Author: Thinking Chen <[email protected]>
AuthorDate: Tue Sep 13 18:52:23 2022 +0800
SUBMARINE-1304. Fix bug with agent listening to resource names (#988)
### What is this PR for?
Currently, there is a bug in the name parameter passed to the agent pod of
the notebook. We need to fix this problem.
### What type of PR is it?
Bug Fix
### Todos
* [x] - Rename the name of the agent for fixing error and Reduce the length
of the name (move resource name to labels).
* [x] - Format the log to ensure the consistency of log output (the
original log is shown in the screenshot) .
### What is the Jira issue?
https://issues.apache.org/jira/projects/SUBMARINE/issues/SUBMARINE-1304
### How should this be tested?
Have fixed test case.
### 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
---
.../src/main/resources/log4j.properties | 17 ++++++++++++
.../server/submitter/k8s/K8sSubmitter.java | 30 ++++++++++------------
.../server/submitter/k8s/model/AgentPod.java | 13 +++++++---
.../submitter/k8s/SubmitterNotebookApiTest.java | 7 +++--
.../k8s/mljob/SubmitterPyTorchApiTest.java | 3 +--
.../k8s/mljob/SubmitterTensorflowApiTest.java | 3 +--
.../k8s/mljob/SubmitterXGBoostApiTest.java | 3 +--
7 files changed, 48 insertions(+), 28 deletions(-)
diff --git
a/submarine-server/server-submitter/submarine-k8s-agent/src/main/resources/log4j.properties
b/submarine-server/server-submitter/submarine-k8s-agent/src/main/resources/log4j.properties
new file mode 100644
index 00000000..55e02b6d
--- /dev/null
+++
b/submarine-server/server-submitter/submarine-k8s-agent/src/main/resources/log4j.properties
@@ -0,0 +1,17 @@
+# Licensed 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. See accompanying LICENSE file.
+log4j.rootLogger = info, stdout
+
+log4j.appender.stdout = org.apache.log4j.ConsoleAppender
+log4j.appender.stdout.Target = System.out
+log4j.appender.stdout.layout = org.apache.log4j.PatternLayout
+log4j.appender.stdout.layout.ConversionPattern = [%-5p] %d{yyyy-MM-dd
HH:mm:ss,SSS} method:%l%n%m%n
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 755298c7..9be76a9c 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
@@ -345,8 +345,7 @@ public class K8sSubmitter implements Submitter {
NotebookUtils.DEFAULT_OVERWRITE_FILE_NAME, OVERWRITE_JSON);
}
// index-4: agent
- AgentPod agentPod = new AgentPod(namespace, spec.getMeta().getName(),
- CustomResourceType.Notebook, notebookId);
+ AgentPod agentPod = new AgentPod(namespace, name,
CustomResourceType.Notebook, notebookId);
// index-5: notebook VirtualService custom resource
IstioVirtualService istioVirtualService = new
IstioVirtualService(createMeta(namespace, name));
@@ -368,39 +367,38 @@ public class K8sSubmitter implements Submitter {
@Override
public Notebook deleteNotebook(NotebookSpec spec, String notebookId) throws
SubmarineRuntimeException {
+ // delete notebook
NotebookCR notebookCR = new NotebookCR(spec, notebookId,
getServerNamespace());
final String name = notebookCR.getMetadata().getName();
final String namespace = notebookCR.getMetadata().getNamespace();
- // delete crd
- Notebook notebook = notebookCR.delete(k8sClient);
+ // dependent resources
+ List<K8sResource> dependents = new ArrayList<K8sResource>();
// delete VirtualService
- new IstioVirtualService(createMeta(namespace, name)).delete(k8sClient);
+ dependents.add(new IstioVirtualService(createMeta(namespace, name)));
// delete pvc
// workspace pvc
- new PersistentVolumeClaim(namespace, String.format("%s-%s",
NotebookUtils.PVC_PREFIX, name),
- NotebookUtils.STORAGE).delete(k8sClient);
+ dependents.add(new PersistentVolumeClaim(namespace,
+ String.format("%s-%s", NotebookUtils.PVC_PREFIX, name),
NotebookUtils.STORAGE));
// user set pvc
- new PersistentVolumeClaim(namespace, String.format("%s-user-%s",
NotebookUtils.PVC_PREFIX, name),
- NotebookUtils.DEFAULT_USER_STORAGE).delete(k8sClient);
+ dependents.add(new PersistentVolumeClaim(namespace,
+ String.format("%s-user-%s", NotebookUtils.PVC_PREFIX, name),
NotebookUtils.DEFAULT_USER_STORAGE));
// configmap
if (StringUtils.isNoneBlank(OVERWRITE_JSON)) {
- new Configmap(namespace, String.format("%s-%s",
NotebookUtils.OVERWRITE_PREFIX, name))
- .delete(k8sClient);
+ dependents.add(new Configmap(namespace, String.format("%s-%s",
NotebookUtils.OVERWRITE_PREFIX, name)));
}
// delete agent
- AgentPod agentPod = new AgentPod(namespace, spec.getMeta().getName(),
- CustomResourceType.Notebook, notebookId);
+ AgentPod agentPod = new AgentPod(namespace, name,
CustomResourceType.Notebook, notebookId);
LOG.info(String.format("Notebook:%s had been deleted, start to delete
agent pod:%s",
spec.getMeta().getName(), agentPod.getMetadata().getName()));
- new AgentPod(namespace, spec.getMeta().getName(),
CustomResourceType.Notebook, notebookId)
- .delete(k8sClient);
+ dependents.add(agentPod);
- return notebook;
+ // delete resources
+ return deleteResourcesTransaction(notebookCR,
dependents.toArray(dependents.toArray(new K8sResource[0])));
}
@Override
diff --git
a/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/model/AgentPod.java
b/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/model/AgentPod.java
index f220d35f..d1f7c8d9 100644
---
a/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/model/AgentPod.java
+++
b/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/model/AgentPod.java
@@ -51,9 +51,10 @@ public class AgentPod extends V1Pod implements
K8sResource<AgentPod> {
super();
V1ObjectMetaBuilder metaBuilder = new V1ObjectMetaBuilder();
- metaBuilder.withName(getNormalizePodName(type, name, resourceId))
+ metaBuilder.withName(getNormalizePodName(type, resourceId))
.withNamespace(namespace)
.addToLabels("app", type.toString().toLowerCase())
+ .addToLabels("resource-name", name)
// There is no need to add istio sidecar. Otherwise, the pod may not
end normally
//
https://istio.io/latest/docs/setup/additional-setup/sidecar-injection/
// Controlling the injection policy Section
@@ -103,12 +104,16 @@ public class AgentPod extends V1Pod implements
K8sResource<AgentPod> {
containers.add(agentContainer);
spec.setRestartPolicy("OnFailure");
+ // Add service account, we temporarily use the service account of
submarine-server
+ // TODO(cdmikechen): We need to add a service account and role/rolebinding
in the operator
+ // to manage the resource permission that the agent can list/get,
subsequently.
+ spec.serviceAccount("submarine-server");
this.setSpec(spec);
}
- public static String getNormalizePodName(CustomResourceType type, String
name, String resourceId) {
- return String.format("%s-%s-%s-%s", resourceId.toLowerCase().replace('_',
'-'),
- type.toString().toLowerCase(), name, CONTAINER_NAME);
+ public static String getNormalizePodName(CustomResourceType type, String
resourceId) {
+ return String.format("%s-%s-%s", resourceId.toLowerCase().replace('_',
'-'),
+ type.toString().toLowerCase(), CONTAINER_NAME);
}
@Override
diff --git
a/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/SubmitterNotebookApiTest.java
b/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/SubmitterNotebookApiTest.java
index 3f304290..5a08b534 100644
---
a/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/SubmitterNotebookApiTest.java
+++
b/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/SubmitterNotebookApiTest.java
@@ -23,6 +23,7 @@ import com.github.tomakehurst.wiremock.client.MappingBuilder;
import com.github.tomakehurst.wiremock.junit.WireMockRule;
import com.github.tomakehurst.wiremock.matching.EqualToPattern;
import org.apache.submarine.commons.utils.SubmarineConfiguration;
+import org.apache.submarine.server.api.common.CustomResourceType;
import org.apache.submarine.server.api.notebook.Notebook;
import org.apache.submarine.server.api.spec.EnvironmentSpec;
import org.apache.submarine.server.api.spec.NotebookMeta;
@@ -31,6 +32,7 @@ import org.apache.submarine.server.api.spec.NotebookSpec;
import org.apache.submarine.server.submitter.k8s.client.K8sClient;
import org.apache.submarine.server.submitter.k8s.client.K8sMockClient;
import org.apache.submarine.server.submitter.k8s.client.MockClientUtil;
+import org.apache.submarine.server.submitter.k8s.model.AgentPod;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@@ -61,7 +63,6 @@ public class SubmitterNotebookApiTest {
private static final String pvcName =
"notebook-pvc-notebook-1642402491519-0003-test-notebook";
private static final String userPvcName =
"notebook-pvc-user-notebook-1642402491519-0003-test-notebook";
private static final String configmapName =
"overwrite-configmap-notebook-1642402491519-0003-test-notebook";
- private static final String agentName =
"notebook-1642402491519-0003-notebook-test-notebook-agent";
@Rule
public WireMockRule wireMockRule = K8sMockClient.getWireMockRule();
@@ -100,12 +101,13 @@ public class SubmitterNotebookApiTest {
.withStatus(200)
.withBody(getResourceFileContent("client/notebook/notebook-read-api.json")));
// save pod agent url
+ String agentName =
AgentPod.getNormalizePodName(CustomResourceType.Notebook, notebookId);
MappingBuilder agentPost =
post(urlPathEqualTo("/api/v1/namespaces/default/pods"))
.withHeader("Content-Type", new EqualToPattern("application/json;
charset=UTF-8"))
.willReturn(
aResponse()
.withStatus(200)
-
.withBody("{\"metadata\":{\"name\":\"test-notebook-agent\",\"namespace\":\"default\"}}"));
+ .withBody("{\"metadata\":{\"name\":\"" + agentName +
"\",\"namespace\":\"default\"}}"));
// save istio url
MappingBuilder istioPost = post(urlPathEqualTo(
"/apis/networking.istio.io/v1beta1/namespaces/default/virtualservices"))
@@ -223,5 +225,6 @@ public class SubmitterNotebookApiTest {
Notebook notebook = submitter.deleteNotebook(spec, notebookId);
// check return value
assertNotNull(notebook);
+ assertEquals(notebook.getName(), "test-notebook");
}
}
diff --git
a/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/mljob/SubmitterPyTorchApiTest.java
b/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/mljob/SubmitterPyTorchApiTest.java
index 11b37307..d7883390 100644
---
a/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/mljob/SubmitterPyTorchApiTest.java
+++
b/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/mljob/SubmitterPyTorchApiTest.java
@@ -75,8 +75,7 @@ public class SubmitterPyTorchApiTest {
.withStatus(200)
.withBody(getResourceFileContent("client/experiment/pytorch-read-api.json")));
// save pod agent url
- String agentName = AgentPod.getNormalizePodName(
- CustomResourceType.PyTorchJob, "pytorch-dist-mnist", experimentId);
+ String agentName =
AgentPod.getNormalizePodName(CustomResourceType.PyTorchJob, experimentId);
MappingBuilder agentPost =
post(urlPathEqualTo("/api/v1/namespaces/default/pods"))
.withHeader("Content-Type", new EqualToPattern("application/json;
charset=UTF-8"))
.willReturn(
diff --git
a/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/mljob/SubmitterTensorflowApiTest.java
b/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/mljob/SubmitterTensorflowApiTest.java
index 7959ab22..6a5682c9 100644
---
a/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/mljob/SubmitterTensorflowApiTest.java
+++
b/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/mljob/SubmitterTensorflowApiTest.java
@@ -75,8 +75,7 @@ public class SubmitterTensorflowApiTest {
.withStatus(200)
.withBody(getResourceFileContent("client/experiment/tf-read-api.json")));
// save pod agent url
- String agentName = AgentPod.getNormalizePodName(
- CustomResourceType.TFJob, "tensorflow-dist-mnist", experimentId);
+ String agentName = AgentPod.getNormalizePodName(CustomResourceType.TFJob,
experimentId);
MappingBuilder agentPost =
post(urlPathEqualTo("/api/v1/namespaces/default/pods"))
.withHeader("Content-Type", new EqualToPattern("application/json;
charset=UTF-8"))
.willReturn(
diff --git
a/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/mljob/SubmitterXGBoostApiTest.java
b/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/mljob/SubmitterXGBoostApiTest.java
index b8f9317e..d5bf9137 100644
---
a/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/mljob/SubmitterXGBoostApiTest.java
+++
b/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/mljob/SubmitterXGBoostApiTest.java
@@ -75,8 +75,7 @@ public class SubmitterXGBoostApiTest {
.withStatus(200)
.withBody(getResourceFileContent("client/experiment/xgboost-read-api.json")));
// save pod agent url
- String agentName = AgentPod.getNormalizePodName(
- CustomResourceType.XGBoost, "xgboost-dist-mnist", experimentId);
+ String agentName =
AgentPod.getNormalizePodName(CustomResourceType.XGBoost, experimentId);
MappingBuilder agentPost =
post(urlPathEqualTo("/api/v1/namespaces/default/pods"))
.withHeader("Content-Type", new EqualToPattern("application/json;
charset=UTF-8"))
.willReturn(
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
