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 <pingsutw@apache.org>
Signed-off-by: Kevin <pingsutw@apache.org>
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"
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 @@
*/
public class Experiment {
private ExperimentId experimentId;
+ private String name;
private String uid;
private String status;
private String acceptedTime;
@@ -51,6 +52,22 @@
}
/**
+ * 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 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 @@
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 @@
if (jobId == null) {
return null;
}
- String[] components = jobId.split("\\-");
+ String[] components = jobId.split("\\_");
if (components.length != 3) {
return null;
}
@@ -60,7 +60,7 @@
@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 @@
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 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 @@
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 @@
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 @@
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 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 @@
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 @@
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 @@
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 @@
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 @@
// 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 @@
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 @@
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 @@
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 @@
}
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 @@
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 @@
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 @@
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
+ ```
