feat!: pass args and command as string (#64)

Author: oleksii-donets

build.gradle

@@ -85,6 +85,7 @@ dependencies {
     implementation 'commons-io:commons-io:2.18.0'
     implementation 'org.apache.commons:commons-lang3:3.18.0'
     implementation 'org.apache.commons:commons-collections4:4.5.0-M3'
+    implementation 'org.apache.commons:commons-exec:1.4.0'
     implementation 'org.springframework.retry:spring-retry'
 
     implementation 'jakarta.validation:jakarta.validation-api:3.0.2'

src/main/java/com/epam/aidial/deployment/manager/service/manifest/InferenceManifestGenerator.java

@@ -7,8 +7,10 @@
 import com.epam.aidial.deployment.manager.model.SimpleEnvVar;
 import com.epam.aidial.deployment.manager.utils.K8sNamingUtils;
 import com.epam.aidial.deployment.manager.utils.mapping.InferenceMappers;
+import com.epam.aidial.deployment.manager.utils.mapping.MappingChain;
 import io.fabric8.kubernetes.api.model.IntOrString;
 import io.kserve.serving.v1beta1.InferenceService;
+import io.kserve.serving.v1beta1.inferenceservicespec.predictor.Model;
 import io.kserve.serving.v1beta1.inferenceservicespec.predictor.model.Env;
 import io.kserve.serving.v1beta1.inferenceservicespec.predictor.model.Ports;
 import io.kserve.serving.v1beta1.inferenceservicespec.predictor.model.env.ValueFrom;
@@ -77,7 +79,7 @@ public InferenceService serviceConfig(
         // Explicitly set model name to ensure the model uses the intended name.
         // If omitted, the inference service will default to the Kubernetes service name,
         // which may differ from the actual model name due to naming transformations.
-        modelChain.get(InferenceMappers.MODEL_ARGS_FIELD).data().addAll(List.of(MODEL_NAME_ARGUMENT_NAME, name));
+        setModelNameIfNotSet(name, modelChain);
 
         var envListMapper = modelChain
                 .getList(InferenceMappers.MODEL_ENV_FIELD, InferenceMappers.ENV_VAR_NAME);
@@ -101,6 +103,26 @@ public InferenceService serviceConfig(
         return config.data();
     }
 
+    private void setModelNameIfNotSet(String modelName, MappingChain<Model> modelChain) {
+        var command = modelChain.getNullable(InferenceMappers.MODEL_COMMAND_FIELD).data();
+        var args = modelChain.getNullable(InferenceMappers.MODEL_ARGS_FIELD).data();
+
+        if (isArgPresent(MODEL_NAME_ARGUMENT_NAME, command) || isArgPresent(MODEL_NAME_ARGUMENT_NAME, args)) {
+            log.info("Argument {} is already set for model '{}', skipping.", MODEL_NAME_ARGUMENT_NAME, modelName);
+            return;
+        }
+        modelChain.get(InferenceMappers.MODEL_ARGS_FIELD).data().addAll(List.of(MODEL_NAME_ARGUMENT_NAME, modelName));
+    }
+
+    private boolean isArgPresent(String targetArg, List<String> existingArgs) {
+        if (existingArgs == null) {
+            return false;
+        }
+        var targetArgWithEquals = targetArg + "=";
+        return existingArgs.stream()
+                .anyMatch(arg -> arg.equals(targetArg) || arg.startsWith(targetArgWithEquals));
+    }
+
     private ValueFrom buildInferenceServiceSecretRef(SensitiveEnvVar env) {
         var secretKeyRef = new SecretKeyRef();
         secretKeyRef.setKey(env.getK8sSecretKey());

src/main/java/com/epam/aidial/deployment/manager/utils/mapping/MappingChain.java

@@ -7,6 +7,10 @@ public <Y> MappingChain<Y> get(FieldMapper<T, Y> fieldMapper) {
         return new MappingChain<>(fieldMapper.getOrSet(data));
     }
 
+    public <Y> MappingChain<Y> getNullable(FieldMapper<T, Y> fieldMapper) {
+        return new MappingChain<>(fieldMapper.getter().apply(data));
+    }
+
     public <Y> ListMapper<Y> getList(FieldMapper<T, List<Y>> fieldMapper, NamedItemMapper<Y> itemMapper) {
         return new ListMapper<>(fieldMapper.getOrSet(data), itemMapper);
     }

src/main/java/com/epam/aidial/deployment/manager/web/dto/deployment/CreateInferenceDeploymentRequestDto.java

@@ -9,8 +9,6 @@
 import lombok.ToString;
 import org.jetbrains.annotations.Nullable;
 
-import java.util.List;
-
 @Data
 @NoArgsConstructor
 @AllArgsConstructor
@@ -23,8 +21,8 @@ public class CreateInferenceDeploymentRequestDto extends CreateDeploymentRequest
     @NotNull @Valid
     private InferenceDeploymentSourceDto source;
     @Nullable
-    private List<String> command;
+    private String command;
     @Nullable
-    private List<String> args;
+    private String args;
 
 }

src/main/java/com/epam/aidial/deployment/manager/web/dto/deployment/InferenceDeploymentDto.java

@@ -9,8 +9,6 @@
 import lombok.ToString;
 import org.jetbrains.annotations.Nullable;
 
-import java.util.List;
-
 @Data
 @NoArgsConstructor
 @AllArgsConstructor
@@ -22,7 +20,7 @@ public class InferenceDeploymentDto extends DeploymentDto {
     @NotNull @Valid
     private InferenceDeploymentSourceDto source;
     @Nullable
-    private List<String> command;
+    private String command;
     @Nullable
-    private List<String> args;
+    private String args;
 }

src/main/java/com/epam/aidial/deployment/manager/web/mapper/DeploymentDtoMapper.java

@@ -23,7 +23,6 @@
 import com.epam.aidial.deployment.manager.model.deployment.NimDeployment;
 import com.epam.aidial.deployment.manager.model.deployment.NimDeploymentNgcRegistrySource;
 import com.epam.aidial.deployment.manager.model.deployment.NimDeploymentSource;
-import com.epam.aidial.deployment.manager.service.ImageDefinitionService;
 import com.epam.aidial.deployment.manager.service.McpEndpointPathResolver;
 import com.epam.aidial.deployment.manager.web.dto.DeploymentInfoDto;
 import com.epam.aidial.deployment.manager.web.dto.DeploymentMetadataDto;
@@ -57,8 +56,12 @@
 import com.epam.aidial.deployment.manager.web.dto.internal.McpDeploymentInternalDto;
 import com.epam.aidial.deployment.manager.web.dto.internal.NimDeploymentInternalDto;
 import com.epam.aidial.deployment.manager.web.dto.value.EnvVarValueDto;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.collections4.CollectionUtils;
 import org.apache.commons.collections4.ListUtils;
 import org.apache.commons.collections4.MapUtils;
+import org.apache.commons.exec.CommandLine;
+import org.apache.commons.lang3.StringUtils;
 import org.mapstruct.AfterMapping;
 import org.mapstruct.Mapper;
 import org.mapstruct.Mapping;
@@ -69,10 +72,12 @@
 import org.springframework.beans.factory.annotation.Autowired;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
 
+@Slf4j
 @Mapper(
         componentModel = "spring",
         uses = {EnvVarValueDtoMapper.class},
@@ -83,8 +88,6 @@ public abstract class DeploymentDtoMapper {
     @Autowired
     private EnvVarValueDtoMapper envVarValueDtoMapper;
     @Autowired
-    private ImageDefinitionService imageDefinitionService;
-    @Autowired
     private McpEndpointPathResolver mcpEndpointPathResolver;
 
     @Mapping(target = "id", source = "name")
@@ -243,4 +246,46 @@ protected McpTransportDto toMcpTransportDtoWithDefault(McpTransport transport) {
         };
     }
 
+    protected List<String> stringToList(String str) {
+        if (StringUtils.isBlank(str)) {
+            return null;
+        }
+
+        try {
+            // Parse the string; this automatically handles quoted tokens (e.g., "foo bar")
+            CommandLine commandLine = CommandLine.parse(str);
+
+            List<String> result = new ArrayList<>();
+
+            // 1. Add the executable (the first token)
+            result.add(commandLine.getExecutable());
+            // 2. Add the arguments (remaining tokens)
+            Collections.addAll(result, commandLine.getArguments());
+
+            return result;
+        } catch (IllegalArgumentException e) {
+            var errorMessage = "Cannot parse command/arguments: '%s'".formatted(str);
+            log.warn(errorMessage, e);
+            throw new IllegalArgumentException(errorMessage, e);
+        }
+    }
+
+    protected String listToString(List<String> list) {
+        if (CollectionUtils.isEmpty(list)) {
+            return null;
+        }
+
+        // 1. Create CommandLine using the first element as the executable
+        CommandLine commandLine = new CommandLine(list.get(0));
+
+        // 2. Add the rest of the list as arguments
+        if (list.size() > 1) {
+            String[] args = list.subList(1, list.size()).toArray(new String[0]);
+            commandLine.addArguments(args);
+        }
+
+        // 3. toString() automatically quotes the executable and arguments as needed
+        return commandLine.toString();
+    }
+
 }

src/test/java/com/epam/aidial/deployment/manager/service/manifest/InferenceManifestGeneratorTest.java

@@ -23,6 +23,7 @@
 import java.util.List;
 import java.util.Map;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.when;
 
 @ExtendWith(MockitoExtension.class)
@@ -155,6 +156,65 @@ void testServiceConfig_withArgs() throws JsonProcessingException, JSONException
         JSONAssert.assertEquals(expected, jsonOutput, true);
     }
 
+    @Test
+    void testServiceConfig_doesNotOverrideExplicitModelNameArg_valueForm() {
+        // Given
+        var deploymentName = "explicit-model-name-app";
+        var storageUri = "s3://my-bucket/model";
+        var args = List.of("--arg1", "value1", "--model_name", "my-explicit-name", "--arg2", "value2");
+
+        // When
+        var generatedService = manifestGenerator.serviceConfig(
+                deploymentName, MODEL_FORMAT, storageUri, Collections.emptyList(), Collections.emptyList(), new Resources(),
+                null, null, null, args, null
+        );
+
+        // Then
+        var actualArgs = generatedService.getSpec().getPredictor().getModel().getArgs();
+        assertThat(actualArgs).containsSubsequence("--model_name", "my-explicit-name");
+        assertThat(actualArgs).doesNotContain("--model_name=" + deploymentName);
+        assertThat(actualArgs).doesNotContainSubsequence("--model_name", deploymentName);
+    }
+
+    @Test
+    void testServiceConfig_doesNotOverrideExplicitModelNameArg_equalsForm() {
+        // Given
+        var deploymentName = "explicit-equals-model-name-app";
+        var storageUri = "s3://my-bucket/model";
+        var args = List.of("--arg1", "value1", "--model_name=my-explicit-name", "--arg2", "value2");
+
+        // When
+        var generatedService = manifestGenerator.serviceConfig(
+                deploymentName, MODEL_FORMAT, storageUri, Collections.emptyList(), Collections.emptyList(), new Resources(),
+                null, null, null, args, null
+        );
+
+        // Then
+        var actualArgs = generatedService.getSpec().getPredictor().getModel().getArgs();
+        assertThat(actualArgs).contains("--model_name=my-explicit-name");
+        assertThat(actualArgs).doesNotContain("--model_name=" + deploymentName);
+        assertThat(actualArgs).doesNotContainSubsequence("--model_name", deploymentName);
+    }
+
+    @Test
+    void testServiceConfig_doesNotAddModelNameWhenPresentInCommand() {
+        // Given
+        var deploymentName = "command-model-name-app";
+        var storageUri = "s3://my-bucket/model";
+        var command = List.of("--model_name=" + deploymentName, "--serve");
+
+        // When
+        var generatedService = manifestGenerator.serviceConfig(
+                deploymentName, MODEL_FORMAT, storageUri, Collections.emptyList(), Collections.emptyList(), new Resources(),
+                null, null, command, Collections.emptyList(), null
+        );
+
+        // Then
+        var model = generatedService.getSpec().getPredictor().getModel();
+        assertThat(model.getCommand()).containsExactlyElementsOf(command);
+        assertThat(model.getArgs()).isEmpty();
+    }
+
     private String serialize(Object obj) throws JsonProcessingException {
         return objectMapper.writeValueAsString(obj);
     }