Skip to content
This repository has been archived by the owner on Oct 23, 2024. It is now read-only.

Validation for app creation & update should not differ #1446

Closed
MikeMichel opened this issue Apr 30, 2015 · 3 comments
Closed

Validation for app creation & update should not differ #1446

MikeMichel opened this issue Apr 30, 2015 · 3 comments
Assignees
Milestone

Comments

@MikeMichel
Copy link
Contributor

starting an app with

{
  "container": {
    "type": "DOCKER",
    "docker": {
      "image": "sloppy/apache-php",
      "network": "BRIDGE",
      "portMappings": [
                {
                    "containerPort": 80,
                    "hostPort": 0,
                    "protocol": "tcp"
                }
            ]
    }
  },
  "id": "apache",
  "instances": 1,
  "cpus": 0.1,
  "mem": 128,
  "cmd": ""
}

then changing memory with PUT

{
  "container": {
    "type": "DOCKER",
    "docker": {
      "image": "sloppy/apache-php",
      "network": "BRIDGE",
      "portMappings": [
                {
                    "containerPort": 80,
                    "hostPort": 0,
                    "protocol": "tcp"
                }
            ]
    }
  },
  "id": "apache",
  "instances": 1,
  "cpus": 0.1,
  "mem": 512,
  "cmd": ""
}

fails with

"error": "AppDefinition must either contain one of 'cmd' or 'args', and/or a 'container'."
@kolloch
Copy link
Contributor

kolloch commented May 4, 2015

That's a bug but there is a work around. You can omit the cmd: "" in the second request and actually also in the first.

{
  "container": {
    "type": "DOCKER",
    "docker": {
      "image": "sloppy/apache-php",
      "network": "BRIDGE",
      "portMappings": [
                {
                    "containerPort": 80,
                    "hostPort": 0,
                    "protocol": "tcp"
                }
            ]
    }
  },
  "id": "apache",
  "instances": 1,
  "cpus": 0.1,
  "mem": 512
}

Also notice that PUT in Marathon only updates the fields that you specify, leaving everything else unchanged. Therefore, you can also just specify the changed mem-field in the second request.

(See #1418 if you think that should be changed.)

@kolloch
Copy link
Contributor

kolloch commented May 18, 2015

Note: Creating an app should have the same validation rule as updating an app.

@kolloch kolloch changed the title PUT fails in 0.8.1 Validation for app creation & update should not differ May 18, 2015
@kolloch kolloch added this to the 0.9.0 milestone May 18, 2015
@kolloch
Copy link
Contributor

kolloch commented May 21, 2015

The problem was actually not the validation itself but the serialization of cmd: "".

An app without args and cmd: "" is automatically translated into an app with cmd: null and args: []. When you try to add a cmd to that, you see the validation error message above.

I will solve it in two ways:

  • Make empty cmds invalid. You should not specify them.
  • If the args array is empty, translate that into no args at all. (We cannot distinguish between empty and not specified in our serialization.)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants