mesosphere / marathon

Deploy and manage containers (including Docker) on top of Apache Mesos at scale.
https://mesosphere.github.io/marathon/
Apache License 2.0
4.07k stars 845 forks source link

Apps can't be scaled from the UI #198

Closed burke closed 10 years ago

burke commented 10 years ago

If I use the suspend or scale buttons in the UI, the changes don't stick. This is the request payload:

screen shot 2014-03-26 at 2 14 52 pm

Of note:

1) It's sending all the attributes, when it really should just be sending instances.

2) It's attempting to clobber container with null, even though we have container set.

3) It's sending version, which is defined to make marathon ignore all the other params, including instances.

When I interact with the API directly, it does stick:

curl -XPUT \
-H "Content-Type: application/json" \
-H "Accept: application/json" \
-d '{"instances": "3"}' \
$marathon/v2/apps/shopify

This started happening at the same time as #194, I think. It's still broken on master.

ssorallen commented 10 years ago

Thanks for the report.

1) Marathon currently supports only PUT requests, which must accept the full representation of the resource (see PATCH vs. PUT). It would be useful to support PATCH as well though.

2) That's a problem I haven't seen yet. I will try to reproduce it.

3) I submitted pull request #202 to fix this. The App model will only submit a whitelist of attributes when saving, which excludes the immutable "tasksRateLimit", "tasksRunning", etc. and the "version".

ssorallen commented 10 years ago

@burke I can't reproduce the container value being clobbered via the UI. Can you provide an example to reproduce it?

burke commented 10 years ago

The container value is only clobbered if I treat PUT as a PATCH and omit it from the input. Don't worry too much about that; it's just that it mostly works as a PATCH :)

When I said "even though we have container set" above, I actually meant we have it set in marathon, not in the query.

ssorallen commented 10 years ago

Aha, thanks for the clarification. PATCH would be a nice addition it sounds like, but for right now you'll have to request the state of the app before sending a PUT if you want to change any attributes.

If you verify the pull request fixes this issue for you, I'll close this and open an improvement issue for adding PATCH support.

burke commented 10 years ago

Much better, thanks!