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 9292943 SUBMARINE-942. Make experiment ID consistent with TFJob and PyTorch Job 9292943 is described below commit 929294341ef5c9d9a423fad6f75da3406086efec Author: Kai-Hsun Chen <b03901...@ntu.edu.tw> AuthorDate: Thu Jul 22 11:25:26 2021 +0800 SUBMARINE-942. Make experiment ID consistent with TFJob and PyTorch Job ### What is this PR for? 1. Please refer to the following two JIRA issues. * https://issues.apache.org/jira/projects/SUBMARINE/issues/SUBMARINE-886?filter=reportedbyme * https://issues.apache.org/jira/projects/SUBMARINE/issues/SUBMARINE-880?filter=reportedbyme In this JIRA issue, we need to make the experiment ID consistent with the name of TFJob and PyTorch Job. The difference is caused by this [link](https://github.com/apache/submarine/commit/8da9f478de9323dd098d06bbd52afdda3a27ce07#diff-c1cad0951ad3663eb075d964171d74ae94a35bec2b79f8c9894030c0446a6b99R119). Update ExperimentId.java. 2. Update [IntegrationTestK8s.md](http://submarine.apache.org/docs/devDocs/IntegrationTestK8s) * "submarine-server-core" is a dependency package specified in pom.xml of test-k8s. * In the document [BuildFromCode.md](http://submarine.apache.org/docs/devDocs/BuildFromCode), the command to build Submarine is `mvn clean package -DskipTests`. However, the package will be installed into the local repository at **install phase**, a later phase than both **package and verify phases**. * Hence, we need to execute `mvn install -DskipTests` to ensure that the **test-k8s** module uses the latest "submarine-server-core" module. 3. Remove the field `name` in Experiment.java because the value of `name` is the same as `experimentId`. ### What type of PR is it? [Improvement] ### Todos ### What is the Jira issue? https://issues.apache.org/jira/browse/SUBMARINE-942 ### How should this be tested? ### 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: Kai-Hsun Chen <b03901...@ntu.edu.tw> Signed-off-by: Kevin <pings...@apache.org> Closes #683 from kevin85421/SUBMARINE-942 and squashes the following commits: 594e46ff [Kai-Hsun Chen] Remove experiment name f051ff87 [Kai-Hsun Chen] Update IntegrationTestK8s.md c88a384a [Kai-Hsun Chen] Refactor b81c2daa [Kai-Hsun Chen] Refactor 495ceb32 [Kai-Hsun Chen] SUBMARINE-942. Make experiment ID consistent with TFJob and PyTorch Job ffd2c168 [Kai-Hsun Chen] SUBMARINE-942. Make experiment ID consistent with TFJob and PyTorch Job (cherry picked from commit 610535806c6208e53dbe0c2889cc944cf32835c5) Signed-off-by: Kevin <pings...@apache.org> --- .../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, 28 insertions(+), 56 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 2f14460..ff080b7 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,7 +26,6 @@ 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; @@ -52,22 +51,6 @@ 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. * @@ -159,8 +142,8 @@ public class Experiment { public void rebuild(Experiment experiment) { if (experiment != null) { - if (experiment.getName() != null) { - this.setName(experiment.getName()); + if (experiment.getExperimentId() != null) { + this.setExperimentId(experiment.getExperimentId()); } 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 42be1b1..cc5fa5e 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 cc57dca..7ccf35b 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,8 +116,7 @@ public class ExperimentManager { String lowerName = spec.getMeta().getName().toLowerCase(); spec.getMeta().setName(lowerName); - spec.getMeta().setExperimentId(id.toString().replaceAll("_", "-")); - LOG.info(spec.getMeta().getExperimentId()); + spec.getMeta().setExperimentId(id.toString()); 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 74c047e..1b7718e 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,7 +28,6 @@ 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 578f86d..d70639b 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,7 +224,6 @@ 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 5ffff37..38be2cc 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,7 +109,6 @@ public class ExperimentRestApiTest { actualExperiment.setRunningTime(experimentRunningTime); actualExperiment.setFinishedTime(experimentFinishedTime); actualExperiment.setUid(experimentUid); - actualExperiment.setName(experimentName); actualExperiment.setStatus(experimentStatus); actualExperiment.setExperimentId(experimentId); kernelSpec.setName(kernelSpecName); @@ -220,7 +219,6 @@ 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 99decf1..725491c 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,7 +39,6 @@ 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()); @@ -80,7 +79,6 @@ 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 ca8d0d8..5be013c 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,7 +113,6 @@ 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 @@ -121,6 +120,5 @@ 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 4f247d3..c9dc564 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,10 +202,8 @@ 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()); @@ -225,8 +223,7 @@ 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(); @@ -307,7 +304,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.getName(), foundExperiment.getName()); + Assert.assertEquals(createdExperiment.getExperimentId().toString(), foundExperiment.getExperimentId().toString()); Assert.assertEquals(createdExperiment.getAcceptedTime(), foundExperiment.getAcceptedTime()); assertK8sResultEquals(foundExperiment); @@ -317,7 +314,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.getName()); + operator.getNamespace(), operator.getPlural(), experiment.getExperimentId().toString()); JsonArray actualCommand = (JsonArray) rootObject.getAsJsonObject("spec") .getAsJsonObject("tfReplicaSpecs").getAsJsonObject("Worker") .getAsJsonObject("template").getAsJsonObject("spec") @@ -389,7 +386,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.getName()); + operator.getNamespace(), operator.getPlural(), experiment.getExperimentId().toString()); JsonObject metadataObject = rootObject.getAsJsonObject("metadata"); String uid = metadataObject.getAsJsonPrimitive("uid").getAsString(); @@ -407,7 +404,7 @@ public class ExperimentRestApiIT extends AbstractSubmarineServerTest { } private void verifyDeleteJobApiResult(Experiment createdExperiment, Experiment deletedExperiment) { - Assert.assertEquals(createdExperiment.getName(), deletedExperiment.getName()); + Assert.assertEquals(createdExperiment.getExperimentId().toString(), deletedExperiment.getExperimentId().toString()); Assert.assertEquals(Experiment.Status.STATUS_DELETED.getValue(), deletedExperiment.getStatus()); // verify the result by K8s api @@ -416,7 +413,7 @@ public class ExperimentRestApiIT extends AbstractSubmarineServerTest { JsonObject rootObject = null; try { rootObject = getJobByK8sApi(operator.getGroup(), operator.getVersion(), - operator.getNamespace(), operator.getPlural(), createdExperiment.getName()); + operator.getNamespace(), operator.getPlural(), createdExperiment.getExperimentId().toString()); } 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 b0d7b75..f902ac7 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,9 +213,8 @@ 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 d486a74..2754318 100644 --- a/website/docs/devDocs/IntegrationTestK8s.md +++ b/website/docs/devDocs/IntegrationTestK8s.md @@ -32,16 +32,18 @@ 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. 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. 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 + ``` ![](../assets/test-k8s-result.png) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@submarine.apache.org For additional commands, e-mail: dev-h...@submarine.apache.org