From 4916e8e60e52b82a639c0c8d0d783fde98bf4682 Mon Sep 17 00:00:00 2001 From: Akash Dwivedi Date: Wed, 24 Jun 2026 12:44:10 -0700 Subject: [PATCH 1/3] Add non_retryable output data signal to fatally fail the step --- .../maestro/engine/dto/OutputData.java | 15 ++++++- .../engine/params/OutputDataManager.java | 16 ++++++++ .../maestro/engine/tasks/MaestroTask.java | 18 +++++++-- .../engine/dao/MaestroOutputDataDaoTest.java | 9 +++-- .../maestro/engine/dto/OutputDataTest.java | 17 ++++++++ .../engine/params/OutputDataManagerTest.java | 33 +++++++++++++-- .../engine/stepruntime/HttpStepRuntime.java | 3 +- .../sample-kubernetes-nonretryable-wf.json | 40 +++++++++++++++++++ 8 files changed, 139 insertions(+), 12 deletions(-) create mode 100644 maestro-server/src/test/resources/samples/sample-kubernetes-nonretryable-wf.json diff --git a/maestro-engine/src/main/java/com/netflix/maestro/engine/dto/OutputData.java b/maestro-engine/src/main/java/com/netflix/maestro/engine/dto/OutputData.java index 0d900240..b36a2144 100644 --- a/maestro-engine/src/main/java/com/netflix/maestro/engine/dto/OutputData.java +++ b/maestro-engine/src/main/java/com/netflix/maestro/engine/dto/OutputData.java @@ -42,7 +42,8 @@ "params", "artifacts", "create_time", - "modify_time" + "modify_time", + "non_retryable" }) @AllArgsConstructor @ToString @@ -56,10 +57,18 @@ public class OutputData { private final Map params; private final Map artifacts; + /** + * Optional flag from the step's output indicating that a failed step must not be retried by the + * system. A null value (the default for records that never set it, including all records written + * before this field existed) means no opinion; callers must null-probe before reading. + */ + private final Boolean nonRetryable; + /** Constructor. */ public OutputData(Map params, Map artifacts) { this.params = params; this.artifacts = artifacts; + this.nonRetryable = null; } /** builder class for lombok and jackson. */ @@ -69,6 +78,8 @@ public static final class OutputDataBuilder {} @JsonIgnore public boolean isNotEmpty() { - return (params != null && !params.isEmpty()) || (artifacts != null && !artifacts.isEmpty()); + return (params != null && !params.isEmpty()) + || (artifacts != null && !artifacts.isEmpty()) + || nonRetryable != null; } } diff --git a/maestro-engine/src/main/java/com/netflix/maestro/engine/params/OutputDataManager.java b/maestro-engine/src/main/java/com/netflix/maestro/engine/params/OutputDataManager.java index e7c81932..5a16d123 100644 --- a/maestro-engine/src/main/java/com/netflix/maestro/engine/params/OutputDataManager.java +++ b/maestro-engine/src/main/java/com/netflix/maestro/engine/params/OutputDataManager.java @@ -91,6 +91,22 @@ public void validateAndMergeOutputParamsAndArtifacts(StepRuntimeSummary runtimeS } } + /** + * Checks whether the step's output data marks a failed step as non-retryable by the system. + * + * @param runtimeSummary step runtime summary used to locate the output data + * @return true if the output data marks the step non-retryable, false otherwise + */ + public boolean isStepNonRetryable(StepRuntimeSummary runtimeSummary) { + Optional externalJobId = extractExternalJobId(runtimeSummary); + if (externalJobId.isEmpty()) { + return false; + } + Optional outputDataOpt = + outputDataDao.getOutputDataForExternalJob(externalJobId.get(), runtimeSummary.getType()); + return outputDataOpt.map(OutputData::getNonRetryable).map(Boolean.TRUE::equals).orElse(false); + } + private Optional extractExternalJobId(StepRuntimeSummary runtimeSummary) { Map artifacts = runtimeSummary.getArtifacts(); String jobId = null; diff --git a/maestro-engine/src/main/java/com/netflix/maestro/engine/tasks/MaestroTask.java b/maestro-engine/src/main/java/com/netflix/maestro/engine/tasks/MaestroTask.java index 29148c2b..663cfd3c 100644 --- a/maestro-engine/src/main/java/com/netflix/maestro/engine/tasks/MaestroTask.java +++ b/maestro-engine/src/main/java/com/netflix/maestro/engine/tasks/MaestroTask.java @@ -750,7 +750,7 @@ private void handleTimeoutError( } /** Executes the step instance. It returns true, if the task is in dummy run mode. */ - @SuppressWarnings("PMD.ExhaustiveSwitchHasDefault") + @SuppressWarnings({"PMD.ExhaustiveSwitchHasDefault", "checkstyle:MethodLength"}) private boolean doExecute( Flow flow, Task task, @@ -847,6 +847,20 @@ private boolean doExecute( evaluateNextConditionParams(flow, stepDefinition, runtimeSummary); doneWithExecute = true; break; + case USER_FAILED: + case PLATFORM_FAILED: + // A retryable failure is escalated to FATALLY_FAILED when the step's output data marks + // it as non-retryable, then falls through to the FATALLY_FAILED handling. + if (!outputDataManager.isStepNonRetryable(runtimeSummary)) { + doneWithExecute = true; + break; + } + runtimeSummary.markTerminated(StepInstance.Status.FATALLY_FAILED, tracingManager); + runtimeSummary.addTimeline( + TimelineLogEvent.info( + "Step failed with [%s] and its output data classified it as non-retryable.", + runtimeSummary.getRuntimeState().getStatus())); + // fall through, to apply failure mode handling case FATALLY_FAILED: // Failure mode only applies to FATALLY_FAILED if (!runtimeSummary.isIgnoreFailureMode()) { if (FailureMode.IGNORE_FAILURE == stepDefinition.getFailureMode()) { @@ -864,8 +878,6 @@ private boolean doExecute( } // fall through, otherwise case INTERNALLY_FAILED: // Ignoring failure model as the error happens within Maestro - case USER_FAILED: - case PLATFORM_FAILED: case TIMEOUT_FAILED: case STOPPED: case TIMED_OUT: diff --git a/maestro-engine/src/test/java/com/netflix/maestro/engine/dao/MaestroOutputDataDaoTest.java b/maestro-engine/src/test/java/com/netflix/maestro/engine/dao/MaestroOutputDataDaoTest.java index 5a9e3bc5..4bd030b3 100644 --- a/maestro-engine/src/test/java/com/netflix/maestro/engine/dao/MaestroOutputDataDaoTest.java +++ b/maestro-engine/src/test/java/com/netflix/maestro/engine/dao/MaestroOutputDataDaoTest.java @@ -78,7 +78,8 @@ public void testParamsSizeOverLimit() throws Exception { System.currentTimeMillis(), System.currentTimeMillis(), params, - new HashMap<>()))); + new HashMap<>(), + null))); } @Test(expected = NullPointerException.class) @@ -97,7 +98,8 @@ public void testValidateParamsAndArtifactsEmpty() { System.currentTimeMillis(), System.currentTimeMillis(), new HashMap<>(), - new HashMap<>()); + new HashMap<>(), + null); dao.insertOrUpdateOutputData(data); } @@ -177,7 +179,8 @@ private void addOutputData(Map params, Map System.currentTimeMillis(), System.currentTimeMillis(), params, - artifacts); + artifacts, + null); dao.insertOrUpdateOutputData(data); } diff --git a/maestro-engine/src/test/java/com/netflix/maestro/engine/dto/OutputDataTest.java b/maestro-engine/src/test/java/com/netflix/maestro/engine/dto/OutputDataTest.java index 4c6cdeb1..3edbb220 100644 --- a/maestro-engine/src/test/java/com/netflix/maestro/engine/dto/OutputDataTest.java +++ b/maestro-engine/src/test/java/com/netflix/maestro/engine/dto/OutputDataTest.java @@ -13,6 +13,8 @@ package com.netflix.maestro.engine.dto; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import com.netflix.maestro.MaestroBaseTest; import org.junit.BeforeClass; @@ -33,4 +35,19 @@ public void testRoundTripSerdeCron() throws Exception { String ser2 = MAPPER.writeValueAsString(actual); assertEquals(ser1, ser2); } + + @Test + public void testDeserializeNonRetryable() throws Exception { + OutputData actual = + MAPPER.readValue(""" + {"non_retryable": true} + """, OutputData.class); + assertTrue(actual.getNonRetryable()); + } + + @Test + public void testDeserializeWithoutNonRetryable() throws Exception { + OutputData actual = MAPPER.readValue("{}", OutputData.class); + assertNull(actual.getNonRetryable()); + } } diff --git a/maestro-engine/src/test/java/com/netflix/maestro/engine/params/OutputDataManagerTest.java b/maestro-engine/src/test/java/com/netflix/maestro/engine/params/OutputDataManagerTest.java index 70fdf379..e164a32d 100644 --- a/maestro-engine/src/test/java/com/netflix/maestro/engine/params/OutputDataManagerTest.java +++ b/maestro-engine/src/test/java/com/netflix/maestro/engine/params/OutputDataManagerTest.java @@ -83,7 +83,8 @@ public void before() throws JsonProcessingException { System.currentTimeMillis(), System.currentTimeMillis(), params, - new HashMap<>()); + new HashMap<>(), + null); outputData.setExternalJobId(TASK_ID); outputData.setExternalJobType(StepType.TITUS); } @@ -110,6 +111,29 @@ public void testSaveOutputData() { Mockito.verify(outputDataDao, times(1)).insertOrUpdateOutputData(outputData); } + @Test + public void testIsStepNonRetryableWhenSignaled() { + OutputData nonRetryable = + new OutputData(StepType.TITUS, TASK_ID, "wfid", null, null, null, null, true); + when(outputDataDao.getOutputDataForExternalJob(TASK_ID, StepType.TITUS)) + .thenReturn(Optional.of(nonRetryable)); + runtimeSummary = runtimeSummaryBuilder().type(StepType.TITUS).artifacts(artifacts).build(); + assertTrue(outputDataManager.isStepNonRetryable(runtimeSummary)); + } + + @Test + public void testIsStepNonRetryableWhenNotSignaled() { + setupOutputDataDao(); + runtimeSummary = runtimeSummaryBuilder().type(StepType.TITUS).artifacts(artifacts).build(); + assertFalse(outputDataManager.isStepNonRetryable(runtimeSummary)); + } + + @Test + public void testIsStepNonRetryableWhenNoOutputData() { + runtimeSummary = runtimeSummaryBuilder().type(StepType.TITUS).artifacts(artifacts).build(); + assertFalse(outputDataManager.isStepNonRetryable(runtimeSummary)); + } + @Test public void testMissingJobIdArtifact() { outputDataManager.validateAndMergeOutputParamsAndArtifacts(runtimeSummary); @@ -251,7 +275,8 @@ public void testValidOutputParamTypes() throws IOException { System.currentTimeMillis(), System.currentTimeMillis(), outputParams, - new HashMap<>()); + new HashMap<>(), + null); setupOutputDataDao(); runtimeSummary = runtimeSummaryBuilder() @@ -294,7 +319,8 @@ public void testOutputArtifacts() { System.currentTimeMillis(), System.currentTimeMillis(), Collections.emptyMap(), - Map.of(Artifact.Type.DYNAMIC_OUTPUT.key(), signalsArtifact)); + Map.of(Artifact.Type.DYNAMIC_OUTPUT.key(), signalsArtifact), + null); when(outputDataDao.getOutputDataForExternalJob(TASK_ID, StepType.TITUS)) .thenReturn(Optional.of(outputData)); @@ -322,6 +348,7 @@ public void testOutputNullableParamsAndArtifacts() { System.currentTimeMillis(), System.currentTimeMillis(), null, + null, null); when(outputDataDao.getOutputDataForExternalJob(TASK_ID, StepType.TITUS)) .thenReturn(Optional.of(outputData)); diff --git a/maestro-http/src/main/java/com/netflix/maestro/engine/stepruntime/HttpStepRuntime.java b/maestro-http/src/main/java/com/netflix/maestro/engine/stepruntime/HttpStepRuntime.java index 84f81133..2edc12ba 100644 --- a/maestro-http/src/main/java/com/netflix/maestro/engine/stepruntime/HttpStepRuntime.java +++ b/maestro-http/src/main/java/com/netflix/maestro/engine/stepruntime/HttpStepRuntime.java @@ -224,7 +224,8 @@ private Map outputParams( System.currentTimeMillis(), System.currentTimeMillis(), outputParams, - Collections.emptyMap())); + Collections.emptyMap(), + null)); return outputParams; } diff --git a/maestro-server/src/test/resources/samples/sample-kubernetes-nonretryable-wf.json b/maestro-server/src/test/resources/samples/sample-kubernetes-nonretryable-wf.json new file mode 100644 index 00000000..6f39aefa --- /dev/null +++ b/maestro-server/src/test/resources/samples/sample-kubernetes-nonretryable-wf.json @@ -0,0 +1,40 @@ +{ + "properties": { + "owner": "tester", + "run_strategy": "sequential" + }, + "workflow": { + "id": "sample-kubernetes-nonretryable-wf", + "name": "Test kubernetes workflow that fails and is classified non-retryable via output data", + "steps": [ + { + "step": { + "id": "job1", + "type": "kubernetes", + "retry_policy": { + "error_retry_limit": 3 + }, + "params": { + "kubernetes": { + "value": { + "image": { + "value": "busybox", + "type": "STRING" + }, + "command": { + "value": ["/bin/sh"], + "type": "STRING_ARRAY" + }, + "args": { + "value": ["-c", "sleep 5 && echo $$MAESTRO_OUTPUT_START$1$$MAESTRO_OUTPUT_END && exit 1", "sh", "{\"non_retryable\":true}"], + "type": "STRING_ARRAY" + } + }, + "type": "MAP" + } + } + } + } + ] + } +} From 3f035832d7e6c62267f90e6077eb727bdfdace47 Mon Sep 17 00:00:00 2001 From: Akash Dwivedi Date: Fri, 26 Jun 2026 14:02:03 -0700 Subject: [PATCH 2/3] Use RetryArtifact retryable flag to fatally fail the step --- .../maestro/models/artifact/Artifact.java | 14 ++++++- .../models/artifact/RetryArtifact.java | 42 +++++++++++++++++++ .../models/artifact/RetryArtifactTest.java | 42 +++++++++++++++++++ .../maestro/engine/dto/OutputData.java | 15 +------ .../engine/params/OutputDataManager.java | 6 ++- .../maestro/engine/tasks/MaestroTask.java | 3 +- .../engine/dao/MaestroOutputDataDaoTest.java | 9 ++-- .../maestro/engine/dto/OutputDataTest.java | 17 -------- .../engine/params/OutputDataManagerTest.java | 32 ++++++++------ .../engine/stepruntime/HttpStepRuntime.java | 3 +- .../sample-kubernetes-nonretryable-wf.json | 2 +- 11 files changed, 131 insertions(+), 54 deletions(-) create mode 100644 maestro-common/src/main/java/com/netflix/maestro/models/artifact/RetryArtifact.java create mode 100644 maestro-common/src/test/java/com/netflix/maestro/models/artifact/RetryArtifactTest.java diff --git a/maestro-common/src/main/java/com/netflix/maestro/models/artifact/Artifact.java b/maestro-common/src/main/java/com/netflix/maestro/models/artifact/Artifact.java index e5660883..cfc0b5f6 100644 --- a/maestro-common/src/main/java/com/netflix/maestro/models/artifact/Artifact.java +++ b/maestro-common/src/main/java/com/netflix/maestro/models/artifact/Artifact.java @@ -33,6 +33,7 @@ @JsonSubTypes.Type(name = "DYNAMIC_OUTPUT", value = DynamicOutputArtifact.class), @JsonSubTypes.Type(name = "KUBERNETES", value = KubernetesArtifact.class), @JsonSubTypes.Type(name = "HTTP", value = HttpArtifact.class), + @JsonSubTypes.Type(name = "RETRY", value = RetryArtifact.class), }) @SuppressWarnings("PMD.ImplicitFunctionalInterface") public interface Artifact { @@ -58,7 +59,9 @@ enum Type { /** kubernetes artifact. */ KUBERNETES(Constants.MAESTRO_PREFIX + "kubernetes"), /** http artifact. */ - HTTP(Constants.MAESTRO_PREFIX + "http"); + HTTP(Constants.MAESTRO_PREFIX + "http"), + /** retry artifact. */ + RETRY(Constants.MAESTRO_PREFIX + "retry"); private final String key; @@ -152,4 +155,13 @@ default KubernetesArtifact asKubernetes() { default HttpArtifact asHttp() { throw new MaestroInternalError("Artifact type [%s] cannot be used as HTTP", getType()); } + + /** + * Get Retry type artifact. + * + * @return concrete artifact object. + */ + default RetryArtifact asRetry() { + throw new MaestroInternalError("Artifact type [%s] cannot be used as RETRY", getType()); + } } diff --git a/maestro-common/src/main/java/com/netflix/maestro/models/artifact/RetryArtifact.java b/maestro-common/src/main/java/com/netflix/maestro/models/artifact/RetryArtifact.java new file mode 100644 index 00000000..d7847ff7 --- /dev/null +++ b/maestro-common/src/main/java/com/netflix/maestro/models/artifact/RetryArtifact.java @@ -0,0 +1,42 @@ +/* + * Copyright 2024 Netflix, Inc. + * + * 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. + */ +package com.netflix.maestro.models.artifact; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonPropertyOrder; +import com.fasterxml.jackson.databind.PropertyNamingStrategies; +import com.fasterxml.jackson.databind.annotation.JsonNaming; +import lombok.Data; + +/** Retry artifact for a step to influence whether the system retries it on failure. */ +@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class) +@JsonInclude(JsonInclude.Include.NON_NULL) +@JsonPropertyOrder( + value = {"retryable"}, + alphabetic = true) +@Data +public class RetryArtifact implements Artifact { + private boolean retryable = true; // whether the system should retry the step on failure + + @JsonIgnore + @Override + public RetryArtifact asRetry() { + return this; + } + + @Override + public Type getType() { + return Type.RETRY; + } +} diff --git a/maestro-common/src/test/java/com/netflix/maestro/models/artifact/RetryArtifactTest.java b/maestro-common/src/test/java/com/netflix/maestro/models/artifact/RetryArtifactTest.java new file mode 100644 index 00000000..dfa7de39 --- /dev/null +++ b/maestro-common/src/test/java/com/netflix/maestro/models/artifact/RetryArtifactTest.java @@ -0,0 +1,42 @@ +/* + * Copyright 2024 Netflix, Inc. + * + * 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. + */ +package com.netflix.maestro.models.artifact; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import com.netflix.maestro.MaestroBaseTest; +import org.junit.Test; + +public class RetryArtifactTest extends MaestroBaseTest { + @Test + public void testDeserializeRetryable() throws Exception { + Artifact artifact = + MAPPER.readValue( + """ + {"type": "RETRY", "retryable": false} + """, Artifact.class); + assertEquals(Artifact.Type.RETRY, artifact.getType()); + assertFalse(artifact.asRetry().isRetryable()); + } + + @Test + public void testDeserializeDefaultsToRetryable() throws Exception { + Artifact artifact = + MAPPER.readValue(""" + {"type": "RETRY"} + """, Artifact.class); + assertTrue(artifact.asRetry().isRetryable()); + } +} diff --git a/maestro-engine/src/main/java/com/netflix/maestro/engine/dto/OutputData.java b/maestro-engine/src/main/java/com/netflix/maestro/engine/dto/OutputData.java index b36a2144..0d900240 100644 --- a/maestro-engine/src/main/java/com/netflix/maestro/engine/dto/OutputData.java +++ b/maestro-engine/src/main/java/com/netflix/maestro/engine/dto/OutputData.java @@ -42,8 +42,7 @@ "params", "artifacts", "create_time", - "modify_time", - "non_retryable" + "modify_time" }) @AllArgsConstructor @ToString @@ -57,18 +56,10 @@ public class OutputData { private final Map params; private final Map artifacts; - /** - * Optional flag from the step's output indicating that a failed step must not be retried by the - * system. A null value (the default for records that never set it, including all records written - * before this field existed) means no opinion; callers must null-probe before reading. - */ - private final Boolean nonRetryable; - /** Constructor. */ public OutputData(Map params, Map artifacts) { this.params = params; this.artifacts = artifacts; - this.nonRetryable = null; } /** builder class for lombok and jackson. */ @@ -78,8 +69,6 @@ public static final class OutputDataBuilder {} @JsonIgnore public boolean isNotEmpty() { - return (params != null && !params.isEmpty()) - || (artifacts != null && !artifacts.isEmpty()) - || nonRetryable != null; + return (params != null && !params.isEmpty()) || (artifacts != null && !artifacts.isEmpty()); } } diff --git a/maestro-engine/src/main/java/com/netflix/maestro/engine/params/OutputDataManager.java b/maestro-engine/src/main/java/com/netflix/maestro/engine/params/OutputDataManager.java index 5a16d123..f514b918 100644 --- a/maestro-engine/src/main/java/com/netflix/maestro/engine/params/OutputDataManager.java +++ b/maestro-engine/src/main/java/com/netflix/maestro/engine/params/OutputDataManager.java @@ -104,7 +104,11 @@ public boolean isStepNonRetryable(StepRuntimeSummary runtimeSummary) { } Optional outputDataOpt = outputDataDao.getOutputDataForExternalJob(externalJobId.get(), runtimeSummary.getType()); - return outputDataOpt.map(OutputData::getNonRetryable).map(Boolean.TRUE::equals).orElse(false); + return outputDataOpt + .map(OutputData::getArtifacts) + .map(artifacts -> artifacts.get(Artifact.Type.RETRY.key())) + .map(artifact -> !artifact.asRetry().isRetryable()) + .orElse(false); } private Optional extractExternalJobId(StepRuntimeSummary runtimeSummary) { diff --git a/maestro-engine/src/main/java/com/netflix/maestro/engine/tasks/MaestroTask.java b/maestro-engine/src/main/java/com/netflix/maestro/engine/tasks/MaestroTask.java index 663cfd3c..845c1eb6 100644 --- a/maestro-engine/src/main/java/com/netflix/maestro/engine/tasks/MaestroTask.java +++ b/maestro-engine/src/main/java/com/netflix/maestro/engine/tasks/MaestroTask.java @@ -855,11 +855,12 @@ private boolean doExecute( doneWithExecute = true; break; } + StepInstance.Status failedStatus = runtimeSummary.getRuntimeState().getStatus(); runtimeSummary.markTerminated(StepInstance.Status.FATALLY_FAILED, tracingManager); runtimeSummary.addTimeline( TimelineLogEvent.info( "Step failed with [%s] and its output data classified it as non-retryable.", - runtimeSummary.getRuntimeState().getStatus())); + failedStatus)); // fall through, to apply failure mode handling case FATALLY_FAILED: // Failure mode only applies to FATALLY_FAILED if (!runtimeSummary.isIgnoreFailureMode()) { diff --git a/maestro-engine/src/test/java/com/netflix/maestro/engine/dao/MaestroOutputDataDaoTest.java b/maestro-engine/src/test/java/com/netflix/maestro/engine/dao/MaestroOutputDataDaoTest.java index 4bd030b3..5a9e3bc5 100644 --- a/maestro-engine/src/test/java/com/netflix/maestro/engine/dao/MaestroOutputDataDaoTest.java +++ b/maestro-engine/src/test/java/com/netflix/maestro/engine/dao/MaestroOutputDataDaoTest.java @@ -78,8 +78,7 @@ public void testParamsSizeOverLimit() throws Exception { System.currentTimeMillis(), System.currentTimeMillis(), params, - new HashMap<>(), - null))); + new HashMap<>()))); } @Test(expected = NullPointerException.class) @@ -98,8 +97,7 @@ public void testValidateParamsAndArtifactsEmpty() { System.currentTimeMillis(), System.currentTimeMillis(), new HashMap<>(), - new HashMap<>(), - null); + new HashMap<>()); dao.insertOrUpdateOutputData(data); } @@ -179,8 +177,7 @@ private void addOutputData(Map params, Map System.currentTimeMillis(), System.currentTimeMillis(), params, - artifacts, - null); + artifacts); dao.insertOrUpdateOutputData(data); } diff --git a/maestro-engine/src/test/java/com/netflix/maestro/engine/dto/OutputDataTest.java b/maestro-engine/src/test/java/com/netflix/maestro/engine/dto/OutputDataTest.java index 3edbb220..4c6cdeb1 100644 --- a/maestro-engine/src/test/java/com/netflix/maestro/engine/dto/OutputDataTest.java +++ b/maestro-engine/src/test/java/com/netflix/maestro/engine/dto/OutputDataTest.java @@ -13,8 +13,6 @@ package com.netflix.maestro.engine.dto; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; import com.netflix.maestro.MaestroBaseTest; import org.junit.BeforeClass; @@ -35,19 +33,4 @@ public void testRoundTripSerdeCron() throws Exception { String ser2 = MAPPER.writeValueAsString(actual); assertEquals(ser1, ser2); } - - @Test - public void testDeserializeNonRetryable() throws Exception { - OutputData actual = - MAPPER.readValue(""" - {"non_retryable": true} - """, OutputData.class); - assertTrue(actual.getNonRetryable()); - } - - @Test - public void testDeserializeWithoutNonRetryable() throws Exception { - OutputData actual = MAPPER.readValue("{}", OutputData.class); - assertNull(actual.getNonRetryable()); - } } diff --git a/maestro-engine/src/test/java/com/netflix/maestro/engine/params/OutputDataManagerTest.java b/maestro-engine/src/test/java/com/netflix/maestro/engine/params/OutputDataManagerTest.java index e164a32d..23b2bd84 100644 --- a/maestro-engine/src/test/java/com/netflix/maestro/engine/params/OutputDataManagerTest.java +++ b/maestro-engine/src/test/java/com/netflix/maestro/engine/params/OutputDataManagerTest.java @@ -31,6 +31,7 @@ import com.netflix.maestro.models.artifact.Artifact; import com.netflix.maestro.models.artifact.DynamicOutputArtifact; import com.netflix.maestro.models.artifact.KubernetesArtifact; +import com.netflix.maestro.models.artifact.RetryArtifact; import com.netflix.maestro.models.artifact.TitusArtifact; import com.netflix.maestro.models.definition.StepType; import com.netflix.maestro.models.parameter.InternalParamMode; @@ -83,8 +84,7 @@ public void before() throws JsonProcessingException { System.currentTimeMillis(), System.currentTimeMillis(), params, - new HashMap<>(), - null); + new HashMap<>()); outputData.setExternalJobId(TASK_ID); outputData.setExternalJobType(StepType.TITUS); } @@ -112,17 +112,28 @@ public void testSaveOutputData() { } @Test - public void testIsStepNonRetryableWhenSignaled() { - OutputData nonRetryable = - new OutputData(StepType.TITUS, TASK_ID, "wfid", null, null, null, null, true); + public void testIsStepNonRetryableWhenMarkedNonRetryable() { + RetryArtifact retryArtifact = new RetryArtifact(); + retryArtifact.setRetryable(false); + OutputData output = new OutputData(null, Map.of(Artifact.Type.RETRY.key(), retryArtifact)); when(outputDataDao.getOutputDataForExternalJob(TASK_ID, StepType.TITUS)) - .thenReturn(Optional.of(nonRetryable)); + .thenReturn(Optional.of(output)); runtimeSummary = runtimeSummaryBuilder().type(StepType.TITUS).artifacts(artifacts).build(); assertTrue(outputDataManager.isStepNonRetryable(runtimeSummary)); } @Test - public void testIsStepNonRetryableWhenNotSignaled() { + public void testIsStepNonRetryableWhenMarkedRetryable() { + OutputData output = + new OutputData(null, Map.of(Artifact.Type.RETRY.key(), new RetryArtifact())); + when(outputDataDao.getOutputDataForExternalJob(TASK_ID, StepType.TITUS)) + .thenReturn(Optional.of(output)); + runtimeSummary = runtimeSummaryBuilder().type(StepType.TITUS).artifacts(artifacts).build(); + assertFalse(outputDataManager.isStepNonRetryable(runtimeSummary)); + } + + @Test + public void testIsStepNonRetryableWhenNoRetryArtifact() { setupOutputDataDao(); runtimeSummary = runtimeSummaryBuilder().type(StepType.TITUS).artifacts(artifacts).build(); assertFalse(outputDataManager.isStepNonRetryable(runtimeSummary)); @@ -275,8 +286,7 @@ public void testValidOutputParamTypes() throws IOException { System.currentTimeMillis(), System.currentTimeMillis(), outputParams, - new HashMap<>(), - null); + new HashMap<>()); setupOutputDataDao(); runtimeSummary = runtimeSummaryBuilder() @@ -319,8 +329,7 @@ public void testOutputArtifacts() { System.currentTimeMillis(), System.currentTimeMillis(), Collections.emptyMap(), - Map.of(Artifact.Type.DYNAMIC_OUTPUT.key(), signalsArtifact), - null); + Map.of(Artifact.Type.DYNAMIC_OUTPUT.key(), signalsArtifact)); when(outputDataDao.getOutputDataForExternalJob(TASK_ID, StepType.TITUS)) .thenReturn(Optional.of(outputData)); @@ -348,7 +357,6 @@ public void testOutputNullableParamsAndArtifacts() { System.currentTimeMillis(), System.currentTimeMillis(), null, - null, null); when(outputDataDao.getOutputDataForExternalJob(TASK_ID, StepType.TITUS)) .thenReturn(Optional.of(outputData)); diff --git a/maestro-http/src/main/java/com/netflix/maestro/engine/stepruntime/HttpStepRuntime.java b/maestro-http/src/main/java/com/netflix/maestro/engine/stepruntime/HttpStepRuntime.java index 2edc12ba..84f81133 100644 --- a/maestro-http/src/main/java/com/netflix/maestro/engine/stepruntime/HttpStepRuntime.java +++ b/maestro-http/src/main/java/com/netflix/maestro/engine/stepruntime/HttpStepRuntime.java @@ -224,8 +224,7 @@ private Map outputParams( System.currentTimeMillis(), System.currentTimeMillis(), outputParams, - Collections.emptyMap(), - null)); + Collections.emptyMap())); return outputParams; } diff --git a/maestro-server/src/test/resources/samples/sample-kubernetes-nonretryable-wf.json b/maestro-server/src/test/resources/samples/sample-kubernetes-nonretryable-wf.json index 6f39aefa..ab226a9e 100644 --- a/maestro-server/src/test/resources/samples/sample-kubernetes-nonretryable-wf.json +++ b/maestro-server/src/test/resources/samples/sample-kubernetes-nonretryable-wf.json @@ -26,7 +26,7 @@ "type": "STRING_ARRAY" }, "args": { - "value": ["-c", "sleep 5 && echo $$MAESTRO_OUTPUT_START$1$$MAESTRO_OUTPUT_END && exit 1", "sh", "{\"non_retryable\":true}"], + "value": ["-c", "sleep 5 && echo $$MAESTRO_OUTPUT_START$1$$MAESTRO_OUTPUT_END && exit 1", "sh", "{\"artifacts\":{\"maestro_retry\":{\"type\":\"RETRY\",\"retryable\":false}}}"], "type": "STRING_ARRAY" } }, From 0318fe9f02560dcc4e0270f8c75f24fb79c35d16 Mon Sep 17 00:00:00 2001 From: Akash Dwivedi Date: Mon, 29 Jun 2026 23:56:34 -0700 Subject: [PATCH 3/3] Add round-trip serde test for RetryArtifact --- .../maestro/models/artifact/RetryArtifactTest.java | 8 ++++++++ .../fixtures/artifact/sample-retry-artifact.json | 4 ++++ 2 files changed, 12 insertions(+) create mode 100644 maestro-common/src/testFixtures/resources/fixtures/artifact/sample-retry-artifact.json diff --git a/maestro-common/src/test/java/com/netflix/maestro/models/artifact/RetryArtifactTest.java b/maestro-common/src/test/java/com/netflix/maestro/models/artifact/RetryArtifactTest.java index dfa7de39..ba49a22b 100644 --- a/maestro-common/src/test/java/com/netflix/maestro/models/artifact/RetryArtifactTest.java +++ b/maestro-common/src/test/java/com/netflix/maestro/models/artifact/RetryArtifactTest.java @@ -20,6 +20,14 @@ import org.junit.Test; public class RetryArtifactTest extends MaestroBaseTest { + @Test + public void testRoundTripSerde() throws Exception { + RetryArtifact artifact = + loadObject("fixtures/artifact/sample-retry-artifact.json", RetryArtifact.class); + assertEquals( + artifact, MAPPER.readValue(MAPPER.writeValueAsString(artifact), RetryArtifact.class)); + } + @Test public void testDeserializeRetryable() throws Exception { Artifact artifact = diff --git a/maestro-common/src/testFixtures/resources/fixtures/artifact/sample-retry-artifact.json b/maestro-common/src/testFixtures/resources/fixtures/artifact/sample-retry-artifact.json new file mode 100644 index 00000000..cc160a89 --- /dev/null +++ b/maestro-common/src/testFixtures/resources/fixtures/artifact/sample-retry-artifact.json @@ -0,0 +1,4 @@ +{ + "retryable": false, + "type": "RETRY" +}