...
Details
Diff Detail
- Repository
- rMARATHON marathon
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Thanks for your work. This fixes #4810, doesn't it?
Is the update not specified in the RAML? Is it just the App?
Fun fact is that changing the UnreachableStrategy in the current implementation is neither an upgrade nor a scale change – we don't want any instance to be restart, nor to launch an additional one. But @aquamatthias is right, it must bei either of those, otherwise it's not persisted.
I think the probability of this change regressing in the future is low. I don't feel like a test is required. If I am outvoted then I will write one.
As @aquamatthias mentioned, the unreachableStrategy must be checked for changes in AppDefinition.isUpgrade – otherwise an AppUpdate that changes only the unreachableStrategy will not be considered a change.
It's probably best to start with an additional test case in AppsResourceTest (and later also in PodsResource) that verifies that a runSpec's unreachableStrategy can be updated.
Refer to DeploymentPlan.dependencyOrderedSteps – since changing the unreachableStrategy is neither a scaleChange nor needsRestart, there will be no deploymentStep and the change will not be persisted. To not restart all instances when changing the strategy, you should either treat it as a Some(ScaleApplication(...)) (not so nice), or dig into the code and see how you can persist the runSpec w/o creating a deployment.
Build is green https://jenkins.mesosphere.com/service/jenkins/job/public-test-marathon-phabricator/625/ for more details.
src/main/proto/marathon.proto | ||
---|---|---|
117 | interesting that I don't see killSelection in here. @jeschkies ? | |
src/main/scala/mesosphere/marathon/state/AppDefinition.scala | ||
149 | also don't see killSelection here @jeschkies |
Build has FAILED
Link to build: https://jenkins.mesosphere.com/service/jenkins/job/public-test-marathon-phabricator/639/
See console output for more information: https://jenkins.mesosphere.com/service/jenkins/job/public-test-marathon-phabricator/639/console
Build has FAILED
Link to build: https://jenkins.mesosphere.com/service/jenkins/job/public-test-marathon-phabricator/641/
See console output for more information: https://jenkins.mesosphere.com/service/jenkins/job/public-test-marathon-phabricator/641/console
Build has FAILED
Link to build: https://jenkins.mesosphere.com/service/jenkins/job/public-test-marathon-phabricator/642/
See console output for more information: https://jenkins.mesosphere.com/service/jenkins/job/public-test-marathon-phabricator/642/console
interesting that I don't see killSelection in here. @jeschkies ?