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 6fe72e2 SUBMARINE-951. Revert SUBMARINE-942. Make experiment ID
consistent with TFJob and PyTorch Job
6fe72e2 is described below
commit 6fe72e294afed2711556837c38457d26bad8f95d
Author: Kevin Su <[email protected]>
AuthorDate: Sun Jul 25 23:47:31 2021 +0800
SUBMARINE-951. Revert SUBMARINE-942. Make experiment ID consistent with
TFJob and PyTorch Job
### 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
-->
The tests in GitHub action are failed. I'm going to revert this commit.
### What type of PR is it?
[Hot Fix]
### Todos
No
### 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`
-->
### 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.
-->
https://issues.apache.org/jira/browse/SUBMARINE-951
### 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: Kevin Su <[email protected]>
Signed-off-by: Kevin <[email protected]>
Closes #690 from pingsutw/SUBMARINE-951 and squashes the following commits:
4206256e [Kevin Su] Revert "SUBMARINE-942. Make experiment ID consistent
with TFJob and PyTorch Job"
---
.../server/api/experiment/Experiment.java | 21 +++++++++++++++++++--
.../server/api/experiment/ExperimentId.java | 12 ++++++------
.../server/experiment/ExperimentManager.java | 3 ++-
.../server/gson/ExperimentIdDeserializer.java | 1 +
.../server/experiment/ExperimentManagerTest.java | 1 +
.../server/rest/ExperimentRestApiTest.java | 2 ++
.../server/submitter/k8s/util/MLJobConverter.java | 2 ++
.../server/submitter/k8s/K8SJobSubmitterTest.java | 2 ++
.../apache/submarine/rest/ExperimentRestApiIT.java | 15 +++++++++------
.../rest/ExperimentTemplateManagerRestApiIT.java | 3 ++-
website/docs/devDocs/IntegrationTestK8s.md | 22 ++++++++++------------
11 files changed, 56 insertions(+), 28 deletions(-)
diff --git
a/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/experiment/Experiment.java
b/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/experiment/Experiment.java
index ff080b7..2f14460 100644
---
a/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/experiment/Experiment.java
+++
b/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/experiment/Experiment.java
@@ -26,6 +26,7 @@ import org.apache.submarine.server.api.spec.ExperimentSpec;
*/
public class Experiment {
private ExperimentId experimentId;
+ private String name;
private String uid;
private String status;
private String acceptedTime;
@@ -51,6 +52,22 @@ public class Experiment {
}
/**
+ * Get the job name which specified by user through the JobSpec
+ * @return the job name
+ */
+ public String getName() {
+ return name;
+ }
+
+ /**
+ * Set the job name which specified by user
+ * @param name job name
+ */
+ public void setName(String name) {
+ this.name = name;
+ }
+
+ /**
* The unique identifier for the job, used to retire the job info from the
cluster management
* system.
*
@@ -142,8 +159,8 @@ public class Experiment {
public void rebuild(Experiment experiment) {
if (experiment != null) {
- if (experiment.getExperimentId() != null) {
- this.setExperimentId(experiment.getExperimentId());
+ if (experiment.getName() != null) {
+ this.setName(experiment.getName());
}
if (experiment.getUid() != null) {
this.setUid(experiment.getUid());
diff --git
a/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/experiment/ExperimentId.java
b/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/experiment/ExperimentId.java
index cc5fa5e..42be1b1 100644
---
a/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/experiment/ExperimentId.java
+++
b/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/experiment/ExperimentId.java
@@ -22,12 +22,12 @@ package org.apache.submarine.server.api.experiment;
import org.apache.submarine.commons.utils.AbstractUniqueIdGenerator;
/**
- * The unique id for experiment. Formatter:
experiment-${server_timestamp}-${counter}
- * Such as: experiment-1577627710-0001
+ * The unique id for experiment. Formatter:
experiment_${server_timestamp}_${counter}
+ * Such as: experiment_1577627710_0001
*/
public class ExperimentId extends AbstractUniqueIdGenerator<ExperimentId> {
- private static final String EXPERIMENT_ID_PREFIX = "experiment-";
-
+ private static final String EXPERIMENT_ID_PREFIX = "experiment_";
+
/**
* Get the object of JobId.
* @param jobId job id string
@@ -37,7 +37,7 @@ public class ExperimentId extends
AbstractUniqueIdGenerator<ExperimentId> {
if (jobId == null) {
return null;
}
- String[] components = jobId.split("\\-");
+ String[] components = jobId.split("\\_");
if (components.length != 3) {
return null;
}
@@ -60,7 +60,7 @@ public class ExperimentId extends
AbstractUniqueIdGenerator<ExperimentId> {
@Override
public String toString() {
StringBuilder sb = new StringBuilder(64);
- sb.append(EXPERIMENT_ID_PREFIX).append(getServerTimestamp()).append("-");
+ sb.append(EXPERIMENT_ID_PREFIX).append(getServerTimestamp()).append("_");
format(sb, getId());
return sb.toString();
}
diff --git
a/submarine-server/server-core/src/main/java/org/apache/submarine/server/experiment/ExperimentManager.java
b/submarine-server/server-core/src/main/java/org/apache/submarine/server/experiment/ExperimentManager.java
index 7ccf35b..cc57dca 100644
---
a/submarine-server/server-core/src/main/java/org/apache/submarine/server/experiment/ExperimentManager.java
+++
b/submarine-server/server-core/src/main/java/org/apache/submarine/server/experiment/ExperimentManager.java
@@ -116,7 +116,8 @@ public class ExperimentManager {
String lowerName = spec.getMeta().getName().toLowerCase();
spec.getMeta().setName(lowerName);
- spec.getMeta().setExperimentId(id.toString());
+ spec.getMeta().setExperimentId(id.toString().replaceAll("_", "-"));
+ LOG.info(spec.getMeta().getExperimentId());
Experiment experiment = submitter.createExperiment(spec);
experiment.setExperimentId(id);
diff --git
a/submarine-server/server-core/src/main/java/org/apache/submarine/server/gson/ExperimentIdDeserializer.java
b/submarine-server/server-core/src/main/java/org/apache/submarine/server/gson/ExperimentIdDeserializer.java
index 1b7718e..74c047e 100644
---
a/submarine-server/server-core/src/main/java/org/apache/submarine/server/gson/ExperimentIdDeserializer.java
+++
b/submarine-server/server-core/src/main/java/org/apache/submarine/server/gson/ExperimentIdDeserializer.java
@@ -28,6 +28,7 @@ import
org.apache.submarine.server.api.experiment.ExperimentId;
import java.lang.reflect.Type;
public class ExperimentIdDeserializer implements
JsonDeserializer<ExperimentId> {
+
@Override
public ExperimentId deserialize(JsonElement json, Type typeOfT,
JsonDeserializationContext context)
throws JsonParseException {
diff --git
a/submarine-server/server-core/src/test/java/org/apache/submarine/server/experiment/ExperimentManagerTest.java
b/submarine-server/server-core/src/test/java/org/apache/submarine/server/experiment/ExperimentManagerTest.java
index d70639b..578f86d 100644
---
a/submarine-server/server-core/src/test/java/org/apache/submarine/server/experiment/ExperimentManagerTest.java
+++
b/submarine-server/server-core/src/test/java/org/apache/submarine/server/experiment/ExperimentManagerTest.java
@@ -224,6 +224,7 @@ public class ExperimentManagerTest {
assertEquals(expected.getCreatedTime(), actual.getCreatedTime());
assertEquals(expected.getRunningTime(), actual.getRunningTime());
assertEquals(expected.getAcceptedTime(), actual.getAcceptedTime());
+ assertEquals(expected.getName(), actual.getName());
assertEquals(expected.getStatus(), actual.getStatus());
assertEquals(expected.getExperimentId(), actual.getExperimentId());
assertEquals(expected.getFinishedTime(), actual.getFinishedTime());
diff --git
a/submarine-server/server-core/src/test/java/org/apache/submarine/server/rest/ExperimentRestApiTest.java
b/submarine-server/server-core/src/test/java/org/apache/submarine/server/rest/ExperimentRestApiTest.java
index 38be2cc..5ffff37 100644
---
a/submarine-server/server-core/src/test/java/org/apache/submarine/server/rest/ExperimentRestApiTest.java
+++
b/submarine-server/server-core/src/test/java/org/apache/submarine/server/rest/ExperimentRestApiTest.java
@@ -109,6 +109,7 @@ public class ExperimentRestApiTest {
actualExperiment.setRunningTime(experimentRunningTime);
actualExperiment.setFinishedTime(experimentFinishedTime);
actualExperiment.setUid(experimentUid);
+ actualExperiment.setName(experimentName);
actualExperiment.setStatus(experimentStatus);
actualExperiment.setExperimentId(experimentId);
kernelSpec.setName(kernelSpecName);
@@ -219,6 +220,7 @@ public class ExperimentRestApiTest {
assertEquals(experimentCreatedTime, experiment.getCreatedTime());
assertEquals(experimentRunningTime, experiment.getRunningTime());
assertEquals(experimentAcceptedTime, experiment.getAcceptedTime());
+ assertEquals(experimentName, experiment.getName());
assertEquals(experimentStatus, experiment.getStatus());
assertEquals(experimentId, experiment.getExperimentId());
assertEquals(experimentFinishedTime, experiment.getFinishedTime());
diff --git
a/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/util/MLJobConverter.java
b/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/util/MLJobConverter.java
index 725491c..99decf1 100644
---
a/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/util/MLJobConverter.java
+++
b/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/util/MLJobConverter.java
@@ -39,6 +39,7 @@ public class MLJobConverter {
public static Experiment toJobFromMLJob(MLJob mlJob) {
Experiment experiment = new Experiment();
experiment.setUid(mlJob.getMetadata().getUid());
+ experiment.setName(mlJob.getMetadata().getName());
DateTime dateTime = mlJob.getMetadata().getCreationTimestamp();
if (dateTime != null) {
experiment.setAcceptedTime(dateTime.toString());
@@ -79,6 +80,7 @@ public class MLJobConverter {
V1StatusDetails details = status.getDetails();
if (details != null) {
experiment.setUid(details.getUid());
+ experiment.setName(details.getName());
}
if (status.getStatus().toLowerCase().equals("success")) {
experiment.setStatus(Experiment.Status.STATUS_DELETED.getValue());
diff --git
a/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/K8SJobSubmitterTest.java
b/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/K8SJobSubmitterTest.java
index 5be013c..ca8d0d8 100644
---
a/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/K8SJobSubmitterTest.java
+++
b/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/K8SJobSubmitterTest.java
@@ -113,6 +113,7 @@ public class K8SJobSubmitterTest extends SpecBuilder {
Experiment experimentFound = submitter.findExperiment(spec);
Assert.assertNotNull(experimentFound);
Assert.assertEquals(experimentCreated.getUid(), experimentFound.getUid());
+ Assert.assertEquals(experimentCreated.getName(),
experimentFound.getName());
Assert.assertEquals(experimentCreated.getAcceptedTime(),
experimentFound.getAcceptedTime());
// delete
@@ -120,5 +121,6 @@ public class K8SJobSubmitterTest extends SpecBuilder {
Assert.assertNotNull(experimentDeleted);
Assert.assertEquals(Experiment.Status.STATUS_DELETED.getValue(),
experimentDeleted.getStatus());
Assert.assertEquals(experimentFound.getUid(), experimentDeleted.getUid());
+ Assert.assertEquals(experimentFound.getName(),
experimentDeleted.getName());
}
}
diff --git
a/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/ExperimentRestApiIT.java
b/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/ExperimentRestApiIT.java
index c9dc564..4f247d3 100644
---
a/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/ExperimentRestApiIT.java
+++
b/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/ExperimentRestApiIT.java
@@ -202,8 +202,10 @@ public class ExperimentRestApiIT extends
AbstractSubmarineServerTest {
String json = postMethod.getResponseBodyAsString();
JsonResponse jsonResponse = gson.fromJson(json, JsonResponse.class);
Assert.assertEquals(Response.Status.OK.getStatusCode(),
jsonResponse.getCode());
+
Experiment createdExperiment =
gson.fromJson(gson.toJson(jsonResponse.getResult()), Experiment.class);
verifyCreateJobApiResult(createdExperiment);
+
// find
GetMethod getMethod = httpGet(BASE_API_PATH + "/" +
createdExperiment.getExperimentId().toString());
Assert.assertEquals(Response.Status.OK.getStatusCode(),
getMethod.getStatusCode());
@@ -223,7 +225,8 @@ public class ExperimentRestApiIT extends
AbstractSubmarineServerTest {
// https://tools.ietf.org/html/rfc5789
// delete
- DeleteMethod deleteMethod = httpDelete(BASE_API_PATH + "/" +
createdExperiment.getExperimentId().toString());
+ DeleteMethod deleteMethod = httpDelete(
+ BASE_API_PATH + "/" + createdExperiment.getExperimentId().toString());
Assert.assertEquals(Response.Status.OK.getStatusCode(),
deleteMethod.getStatusCode());
json = deleteMethod.getResponseBodyAsString();
@@ -304,7 +307,7 @@ public class ExperimentRestApiIT extends
AbstractSubmarineServerTest {
Experiment createdExperiment, Experiment foundExperiment) throws
Exception {
Assert.assertEquals(createdExperiment.getExperimentId(),
foundExperiment.getExperimentId());
Assert.assertEquals(createdExperiment.getUid(), foundExperiment.getUid());
- Assert.assertEquals(createdExperiment.getExperimentId().toString(),
foundExperiment.getExperimentId().toString());
+ Assert.assertEquals(createdExperiment.getName(),
foundExperiment.getName());
Assert.assertEquals(createdExperiment.getAcceptedTime(),
foundExperiment.getAcceptedTime());
assertK8sResultEquals(foundExperiment);
@@ -314,7 +317,7 @@ public class ExperimentRestApiIT extends
AbstractSubmarineServerTest {
KfOperator operator =
kfOperatorMap.get(experiment.getSpec().getMeta().getFramework().toLowerCase());
JsonObject rootObject =
getJobByK8sApi(operator.getGroup(), operator.getVersion(),
- operator.getNamespace(), operator.getPlural(),
experiment.getExperimentId().toString());
+ operator.getNamespace(), operator.getPlural(),
experiment.getName());
JsonArray actualCommand = (JsonArray) rootObject.getAsJsonObject("spec")
.getAsJsonObject("tfReplicaSpecs").getAsJsonObject("Worker")
.getAsJsonObject("template").getAsJsonObject("spec")
@@ -386,7 +389,7 @@ public class ExperimentRestApiIT extends
AbstractSubmarineServerTest {
private void assertK8sResultEquals(Experiment experiment) throws Exception {
KfOperator operator =
kfOperatorMap.get(experiment.getSpec().getMeta().getFramework().toLowerCase());
JsonObject rootObject = getJobByK8sApi(operator.getGroup(),
operator.getVersion(),
- operator.getNamespace(), operator.getPlural(),
experiment.getExperimentId().toString());
+ operator.getNamespace(), operator.getPlural(), experiment.getName());
JsonObject metadataObject = rootObject.getAsJsonObject("metadata");
String uid = metadataObject.getAsJsonPrimitive("uid").getAsString();
@@ -404,7 +407,7 @@ public class ExperimentRestApiIT extends
AbstractSubmarineServerTest {
}
private void verifyDeleteJobApiResult(Experiment createdExperiment,
Experiment deletedExperiment) {
- Assert.assertEquals(createdExperiment.getExperimentId().toString(),
deletedExperiment.getExperimentId().toString());
+ Assert.assertEquals(createdExperiment.getName(),
deletedExperiment.getName());
Assert.assertEquals(Experiment.Status.STATUS_DELETED.getValue(),
deletedExperiment.getStatus());
// verify the result by K8s api
@@ -413,7 +416,7 @@ public class ExperimentRestApiIT extends
AbstractSubmarineServerTest {
JsonObject rootObject = null;
try {
rootObject = getJobByK8sApi(operator.getGroup(), operator.getVersion(),
- operator.getNamespace(), operator.getPlural(),
createdExperiment.getExperimentId().toString());
+ operator.getNamespace(), operator.getPlural(),
createdExperiment.getName());
} catch (ApiException e) {
Assert.assertEquals(Response.Status.NOT_FOUND.getStatusCode(),
e.getCode());
} finally {
diff --git
a/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/ExperimentTemplateManagerRestApiIT.java
b/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/ExperimentTemplateManagerRestApiIT.java
index f902ac7..b0d7b75 100644
---
a/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/ExperimentTemplateManagerRestApiIT.java
+++
b/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/ExperimentTemplateManagerRestApiIT.java
@@ -213,8 +213,9 @@ public class ExperimentTemplateManagerRestApiIT extends
AbstractSubmarineServerT
LOG.info(gson.toJson(jsonResponse.getResult()));
Experiment experiment =
gson.fromJson(gson.toJson(jsonResponse.getResult()), Experiment.class);
+
DeleteMethod deleteMethod = httpDelete("/api/" + RestConstants.V1 + "/" +
RestConstants.EXPERIMENT + "/"
- + experiment.getExperimentId().toString());
+ + experiment.getExperimentId().toString());
Assert.assertEquals(Response.Status.OK.getStatusCode(),
deleteMethod.getStatusCode());
json = deleteMethod.getResponseBodyAsString();
diff --git a/website/docs/devDocs/IntegrationTestK8s.md
b/website/docs/devDocs/IntegrationTestK8s.md
index 2754318..d486a74 100644
--- a/website/docs/devDocs/IntegrationTestK8s.md
+++ b/website/docs/devDocs/IntegrationTestK8s.md
@@ -32,18 +32,16 @@ title: How to Run Integration K8s Test
2. Build the submarine from source and upgrade the server pod through this
[`guide`](./Development/#build-from-source)
3. Forward port
- ```bash
- kubectl port-forward --address 0.0.0.0 service/submarine-traefik 8080:80
- ```
-4. Install the latest package "submarine-server-core" into the local
repository, for use as a dependency in the module `test-k8s`
- ```bash
- mvn install -DskipTests
- ```
-
-5. Execute the test command
- ```bash
- mvn verify -DskipRat -pl :submarine-test-k8s -Phadoop-2.9 -B
- ```
+
+ ```bash
+ kubectl port-forward --address 0.0.0.0 service/submarine-traefik 8080:80
+ ```
+
+4. Execute the test command
+
+ ```bash
+ mvn verify -DskipRat -pl :submarine-test-k8s -Phadoop-2.9 -B
+ ```
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
