HomeMesosphereNo notifications. 4 unresolved issues.
  • Search
Differential D552

PUT on /v2/apps has a PATCH semantic. (#5157)
ClosedAll Users
Actions

Authored by unterstein on Feb 24 2017, 3:17 PM.

Details

Reviewers
aquamatthias
jdef
jenkins
• jasongilanfarr
meichstedt
Commits
rMARATHON717989a1eebf: PUT on /v2/apps has a PATCH semantic. (#5157)
Summary

1.) PUT on /v2/apps has a PATCH semantic. (#5157)
See 8df2a5d8b43e8a1c8609f88bc056245bbeef7523

  • document that, by default, PUT on /v2/apps has a PATCH semantic: only the fields specified in the app update are applied to the existing app definition.
  • add a query parameter, partialUpdate, that allows for proper PUT semantics for an app. false means "completely replace the existing app with the new one that I'm fully specifying here"

2.) Add support for PATCH updates to apps in 1.4 (#5183)
See 1ba8ea7352045119b9d4803010f03b88ee79e861

  • added support to PATCH apps

also-by: unterstein

3.) Fixes #5211 by defining the jersey annotated methods explicitly. (#5217)
See e1b79523741bf1c266b953eba1bcf8783f819c3a

  • With the newly introduced PATCH semantic, we introduced a misleading method declaration which is not resolvable for jersey. Therefore this logic was restructured and methods are defined explicitly.
Test Plan

sbt test
sbt integration:test

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.
unterstein created this revision.Feb 24 2017, 3:17 PM
jenkins requested changes to this revision.Feb 24 2017, 3:18 PM
This revision now requires changes to proceed.Feb 24 2017, 3:18 PM
unterstein added a comment.EditedFeb 24 2017, 3:31 PM

Only cherry-picks of:
#5217
#5183
#5157

Had some minor merge conflicts therefore create this revision.

Currently doing manual tests on this revision, like we did for 1.4.1.

jenkins accepted this revision.Feb 24 2017, 3:36 PM

Ship it!

This revision is now accepted and ready to land.Feb 24 2017, 3:36 PM
jdef accepted this revision.Feb 26 2017, 12:02 AM
jdef added a subscriber: jdef.
jdef added inline comments.
src/test/scala/mesosphere/marathon/api/v2/AppsResourceTest.scala
183

there's limits to what this is actually testing. because the groupManager is fake for the default fixture (AFAIR) there's a fair amount of (callback) update logic that's never actually invoked here. the other new test case (added below this one) is a bit more narrowly focused - the downside being that it's not invoking the REST endpoint directly. the callback-oriented design of groupManager makes testing this a bit more complicated that it should be

unterstein added a comment.Feb 27 2017, 11:01 AM

manual retest delivers same results as expected

meichstedt requested changes to this revision.Feb 27 2017, 12:15 PM

Minor comments

docs/docs/rest-api/public/api/v2/apps.raml
146

I figure this is actually not really correct. If we apply a patch {instances: 2} to a seq of apps that currently have {instances: 1}, Marathon will not replace any instances but only start additional ones. In general, the needsRestart logic will be applied. This is probably not well documented anywhere and has know edges like when instances are restarted although only the upgradeStrategy was changed.

Maybe change this to something that doesn't lie like
_Instances will be started or restarted with according to the usual logic._?

373

Again, in case needsRestart evaluates to false, no instances will be restarted.

src/main/scala/mesosphere/marathon/state/AppDefinition.scala
371

I hadn't seen this in the original changeset. Wrong place imo: AppUpdate already has a dependency on AppDefinition (e.g. def apply(app AppDefinition): AppDefinition)). To not have both classes depend on each other, I'd suggest to move this to an AppUpdate.fromApp(...)

This revision now requires changes to proceed.Feb 27 2017, 12:15 PM
jdef added a comment.Feb 27 2017, 1:55 PM

AppUpdate is a misguided half-breed of a model and API class. It's
completely eliminated in D201. Please don't move anything else there

meichstedt added a comment.Feb 27 2017, 2:10 PM
In D552#18035, @jdef wrote:

AppUpdate is a misguided half-breed of a model and API class. It's
completely eliminated in D201. Please don't move anything else there

Ok, fine with that.

unterstein added a comment.Mar 2 2017, 4:17 PM

Hi @jdef @meichstedt,

i filed https://github.com/mesosphere/marathon/issues/5291 to cover your documentation feedback. I would love to only forward port the changes and cover mentioned feedback in #5291. Are you ok with this?

meichstedt accepted this revision.Mar 2 2017, 4:53 PM

Totally – thanks!

This revision is now accepted and ready to land.Mar 2 2017, 4:53 PM
unterstein edited the summary of this revision. (Show Details)Mar 15 2017, 1:51 PM
unterstein edited the summary of this revision. (Show Details)Mar 15 2017, 1:55 PM
Closed by commit rMARATHON717989a1eebf: PUT on /v2/apps has a PATCH semantic. (#5157) (authored by unterstein). · Explain WhyMar 15 2017, 1:57 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

Diff 2279

View Options

docs/docs/rest-api/public/api/v2/apps.raml

Show First 20 Lines • Show All 84 Lines • ▼ Show 20 Line(s)
8585 The order of dependencies will be applied correctly.
8686 It is possible to mix upgrades and installs.
8787
8888 If you have more complex scenarios with upgrades, use the groups endpoint.
8989
9090 Note: This operation will create a deployment. The operation finishes, if the deployment succeeds.
9191 You can query the deployments endoint to see the status of the deployment.
9292 is: [ secured, deployable ]
93 queryParameters:
94 partialUpdate:
95 required: false
96 default: true
97 description: |
98 Without specifying this parameter, this method has a patch like semantic:
99 All values that are not defined in the json, will not change existing values.
100 This was the default behaviour in previous Marathon versions.
101 For backward compatibility, we will not change this behaviour, but let users opt in for a proper PUT.
102 Note: We will change the default behaviour in the next Marathon version to support PATCH and PUT as HTTP methods.
93103 body:
94104 application/json:
95105 example: !include examples/apps_create.json
96106 type: app.App[]
97107 responses:
98108 409:
99109 description: There is an already deployed application with this name
100110 body:
Show All 21 Lines
122132 "path": "/upgradeStrategy/minimumHealthCapacity",
123133 "errors": [
124134 "is greater than 1"
125135 ]
126136 }
127137 ]
128138 }
129139
140 ###
141 # apply a patch update to a list of applications
142 #
143 patch:
144 description:
145 Change multiple existing applications by applying a patch.
146 All instances of these applications get replaced by the new version.
meichstedtUnsubmitted
Not Done

I figure this is actually not really correct. If we apply a patch {instances: 2} to a seq of apps that currently have {instances: 1}, Marathon will not replace any instances but only start additional ones. In general, the needsRestart logic will be applied. This is probably not well documented anywhere and has know edges like when instances are restarted although only the upgradeStrategy was changed.

Maybe change this to something that doesn't lie like
_Instances will be started or restarted with according to the usual logic._?

147 The order of dependencies will be applied correctly.
148 Each upgradeStrategy defines the behaviour of the upgrade for the related app.
130149
150 The whole operation fails if the IDs of one or more applications are unknown.
151 The order of dependencies will be applied correctly.
131152
153 If you have more complex scenarios with upgrades, use the groups endpoint.
132154
155 Note: This operation will create a deployment. The operation finishes, if the deployment succeeds.
156 You can query the deployments endoint to see the status of the deployment.
157 is: [ secured, deployable ]
158 queryParameters:
159 body:
160 application/json:
161 example: !include examples/apps_patch.json
162 type: app.App[]
163 responses:
164 409:
165 description: One or more specified applications is currently locked by a deployment
166 body:
167 application/json:
168 type: error.Error
169 example: |
170 {"message":"App is locked by one or more deployments."}
171 400:
172 description: The application definition provided in the body is not valid.
173 body:
174 application/json:
175 type: error.Error
176 example: |
177 {"message":"Invalid JSON","details":[{"path":"/id","errors":["error.expected.jsstring"]}]}
178 422:
179 description: The entity send can not be processed, since there are validation errors
180 body:
181 application/json:
182 type: error.Error
183 example: |
184 {
185 "message": "Object is not valid",
186 "details": [
187 {
188 "path": "/upgradeStrategy/minimumHealthCapacity",
189 "errors": [
190 "is greater than 1"
191 ]
192 }
193 ]
194 }
195
133196 ###
134197 # Create application
135198 #
136199 post:
137200 description:
138201 Create and start a new application.
139202
140203 Note: This operation will create a deployment. The operation finishes, if the deployment succeeds.
Show All 36 Lines
177240 "path": "/upgradeStrategy/minimumHealthCapacity",
178241 "errors": [
179242 "is greater than 1"
180243 ]
181244 }
182245 ]
183246 }
184247
185
186248 /{app_id}:
187249
188250 ###
189251 # Get a specific app
190252 #
191253 get:
192254 description:
193255 Get the application with id `app_id`.
▲ Show 20 Lines • Show All 52 Lines • ▼ Show 20 Line(s)
246308 Replaces parameters of a running application. If no application with the given id
247309 exists, it will be created. If there is an application with this id, all running instances get
248310 upgraded to the new definition.
249311
250312
251313 Note: This operation will create a deployment. The operation finishes, if the deployment succeeds.
252314 You can query the deployments endoint to see the status of the deployment.
253315 is: [ secured, deployable ]
316 queryParameters:
317 partialUpdate:
318 required: false
319 default: true
320 description: |
321 Without specifying this parameter, this method has a patch like semantic:
322 All values that are not defined in the json, will not change existing values.
323 This was the default behaviour in previous Marathon versions.
324 For backward compatibility, we will not change this behaviour, but let users opt in for a proper PUT.
325 Note: We will change the default behaviour in the next Marathon version to support PATCH and PUT as HTTP methods.
254326 body:
255327 application/json:
256328 example: !include examples/app.json
257329 type: app.App
258330 responses:
259331 201:
260332 description: The application has been created and a deployment is started.
261333 body:
Show All 26 Lines
288360 "path": "/upgradeStrategy/minimumHealthCapacity",
289361 "errors": [
290362 "is greater than 1"
291363 ]
292364 }
293365 ]
294366 }
295367
368 ###
369 # Apply a patch update to a specific app.
370 #
371 patch:
372 description:
373 Replaces parameters of a running application. All running instances get upgraded to the new definition.
meichstedtUnsubmitted
Not Done

Again, in case needsRestart evaluates to false, no instances will be restarted.

374 Any given application ID will be ignored.
296375
376 Note: This operation will create a deployment. The operation finishes, if the deployment succeeds.
377 You can query the deployments endoint to see the status of the deployment.
378 is: [ secured, deployable ]
379 queryParameters:
380 body:
381 application/json:
382 example: !include examples/app_patch.json
383 type: app.App
384 responses:
385 200:
386 description: The application has been updated and a deployment is started.
387 body:
388 application/json:
389 example: !include examples/deployments_result.json
390 404:
391 description: No task found with this `app_id`.
392 body:
393 application/json:
394 type: error.Error
395 example: |
396 { "message": "App '/not_existent' does not exist" }
397 400:
398 description: The application definition provided in the body is not valid.
399 body:
400 application/json:
401 type: error.Error
402 example: |
403 {"message":"Invalid JSON","details":[{"path":"/id","errors":["error.expected.jsstring"]}]}
404 422:
405 description: The entity sent can not be preocessed, since there are validation errors
406 body:
407 application/json:
408 type: error.Error
409 example: |
410 {
411 "message": "Object is not valid",
412 "details": [
413 {
414 "path": "/upgradeStrategy/minimumHealthCapacity",
415 "errors": [
416 "is greater than 1"
417 ]
418 }
419 ]
420 }
297421
298422 ###
299423 # Delete a specific app
300424 #
301425 delete:
302426 description: Destroy an application. All data about that application will be deleted.
303427
304428 Note: This operation will create a deployment. The operation finishes, if the deployment succeeds.
▲ Show 20 Lines • Show All 152 Lines • Show Last 20 Lines
View Options

docs/docs/rest-api/public/api/v2/examples/app_patch.json

  • This file was added.
1{
2 "id": "/foo",
3 "instances": 2
4}
View Options

docs/docs/rest-api/public/api/v2/examples/apps_patch.json

  • This file was added.
1[{
2 "id": "/test/sleep60",
3 "cpus": 0.3
4},{
5 "id": "/test/sleep120",
6 "instances": 2
7}]
View Options

src/main/scala/mesosphere/marathon/api/RestResource.scala

11package mesosphere.marathon
22package api
33
44import java.net.URI
55import javax.ws.rs.core.Response
66import javax.ws.rs.core.Response.{ ResponseBuilder, Status }
7
8import akka.http.scaladsl.model.StatusCodes
79import com.wix.accord._
8import mesosphere.marathon.MarathonConf
10import mesosphere.marathon.{ MarathonConf, ValidationFailedException }
911import mesosphere.marathon.api.v2.Validation._
1012import mesosphere.marathon.api.v2.json.Formats._
1113import mesosphere.marathon.state.{ PathId, Timestamp }
1214import mesosphere.marathon.core.deployment.DeploymentPlan
13
1415import play.api.libs.json.Json.JsValueWrapper
1516import play.api.libs.json.{ Json, Writes }
1617
1718import scala.concurrent.{ Await, Awaitable }
1819
1920trait RestResource {
2021
2122 protected val config: MarathonConf
▲ Show 20 Lines • Show All 47 Lines • ▼ Show 20 Line(s)
6970 */
7071 protected def withValid[T](t: T, description: Option[String] = None)(fn: T => Response)(implicit validator: Validator[T]): Response = {
7172 validator(t) match {
7273 case f: Failure =>
7374 val entity = Json.toJson(description.map(f.withDescription).getOrElse(f)).toString
7475 Response.status(422).entity(entity).build()
7576 case Success => fn(t)
7677 }
78 }
79
80 /**
81 * Execute the given function and if any validation errors crop up, generate an UnprocessableEntity
82 * HTTP status code and send the validation error as the response body (in JSON form).
83 */
84 protected def assumeValid(f: => Response): Response =
85 try {
86 f
87 } catch {
88 case vfe: ValidationFailedException =>
89 val entity = Json.toJson(vfe.failure).toString
90 Response.status(StatusCodes.UnprocessableEntity.intValue).entity(entity).build()
7791 }
7892}
7993
8094object RestResource {
8195 val DeploymentHeader = "Marathon-Deployment-Id"
8296}
View Options

src/main/scala/mesosphere/marathon/api/v2/AppsResource.scala

11package mesosphere.marathon
22package api.v2
33
44import java.net.URI
55import javax.inject.Inject
66import javax.servlet.http.HttpServletRequest
77import javax.ws.rs._
88import javax.ws.rs.core.{ Context, MediaType, Response }
99
1010import akka.event.EventStream
1111import mesosphere.marathon.api.v2.Validation._
1212import mesosphere.marathon.api.v2.json.AppUpdate
1313import mesosphere.marathon.api.v2.json.Formats._
14import mesosphere.marathon.api.{ AuthResource, MarathonMediaType, RestResource }
14import mesosphere.marathon.api.{ AuthResource, MarathonMediaType, PATCH, RestResource }
1515import mesosphere.marathon.core.appinfo.{ AppInfo, AppInfoService, AppSelector, Selector, TaskCounts }
1616import mesosphere.marathon.core.base.Clock
1717import mesosphere.marathon.core.event.ApiPostEvent
1818import mesosphere.marathon.core.group.GroupManager
1919import mesosphere.marathon.core.plugin.PluginManager
2020import mesosphere.marathon.plugin.auth._
2121import mesosphere.marathon.state.PathId._
2222import mesosphere.marathon.state._
2323import mesosphere.marathon.stream.Implicits._
24import play.api.libs.json.Json
24import play.api.libs.json.{ JsObject, Json }
2525
2626@Path("v2/apps")
2727@Consumes(Array(MediaType.APPLICATION_JSON))
2828@Produces(Array(MarathonMediaType.PREFERRED_APPLICATION_JSON))
2929class AppsResource @Inject() (
3030 clock: Clock,
3131 eventBus: EventStream,
3232 appTasksRes: AppTasksResource,
Show All 21 Lines
5454 val selector = selectAuthorized(search(Option(cmd), Option(id), Option(label)))
5555 // additional embeds are deprecated!
5656 val resolvedEmbed = InfoEmbedResolver.resolveApp(embed) +
5757 AppInfo.Embed.Counts + AppInfo.Embed.Deployments
5858 val mapped = result(appInfoService.selectAppsBy(selector, resolvedEmbed))
5959 Response.ok(jsonObjString("apps" -> mapped)).build()
6060 }
6161
62 @POST
63 def create(
64 body: Array[Byte],
65 @DefaultValue("false")@QueryParam("force") force: Boolean,
66 @Context req: HttpServletRequest): Response = authenticated(req) { implicit identity =>
67 withValid(Json.parse(body).as[AppDefinition].withCanonizedIds()) { appDef =>
68 val now = clock.now()
69 val app = appDef.copy(
62 def normalizedApp(appDef: AppDefinition, now: Timestamp): AppDefinition = {
63 appDef.copy(
7064 ipAddress = appDef.ipAddress.map { ipAddress =>
7165 config.defaultNetworkName.get.collect {
7266 case (defaultName: String) if defaultName.nonEmpty && ipAddress.networkName.isEmpty =>
7367 ipAddress.copy(networkName = Some(defaultName))
7468 }.getOrElse(ipAddress)
7569 },
7670 versionInfo = VersionInfo.OnlyVersion(now)
7771 )
72 }
7873
74 @POST
75 def create(
76 body: Array[Byte],
77 @DefaultValue("false")@QueryParam("force") force: Boolean,
78 @Context req: HttpServletRequest): Response = authenticated(req) { implicit identity =>
79 withValid(Json.parse(body).as[AppDefinition].withCanonizedIds()) { appDef =>
80 val app = normalizedApp(appDef, clock.now())
7981 checkAuthorization(CreateRunSpec, app)
8082
8183 def createOrThrow(opt: Option[AppDefinition]) = opt
8284 .map(_ => throw ConflictingChangeException(s"An app with id [${app.id}] already exists."))
8385 .getOrElse(app)
8486
8587 val plan = result(groupManager.updateApp(app.id, createOrThrow, app.version, force))
8688
▲ Show 20 Lines • Show All 45 Lines • ▼ Show 20 Line(s)
132134 }
133135
134136 id match {
135137 case ListApps(gid) => transitiveApps(gid.toRootPath)
136138 case _ => app(id.toRootPath)
137139 }
138140 }
139141
142 /**
143 * Validate and normalize a single application update submitted via the REST API. Validation exceptions are not
144 * handled here, that's left as an exercise for the caller.
145 *
146 * @param appId used as the id of the generated app update (vs. whatever might be in the JSON body)
147 * @param body is the raw, unparsed JSON
148 * @param partialUpdate true if the JSON should be parsed as a partial application update (all fields optional)
149 * or as a wholesale replacement (parsed like an app definition would be)
150 */
151 def canonicalAppUpdateFromJson(appId: PathId, body: Array[Byte], partialUpdate: Boolean): AppUpdate = {
152 if (partialUpdate) {
153 validateOrThrow(Json.parse(body).as[AppUpdate].copy(id = Some(appId)).withCanonizedIds())
154 } else {
155 // this is a complete replacement of the app as we know it, so parse and normalize as if we're dealing
156 // with a brand new app because the rules are different (for example, many fields are non-optional with brand-new apps).
157 // however since this is an update, the user isn't required to specify an ID as part of the definition so we do
158 // some hackery here to pass initial JSON parsing.
159 val jsObj = Json.parse(body).as[JsObject] + ("id" -> Json.toJson(appId.toString))
160 // the version is thrown away in toUpdate so just pass `zero` for now
161 normalizedApp(validateOrThrow(jsObj.as[AppDefinition].withCanonizedIds()), Timestamp.zero).toUpdate
162 }
163 }
164
165 /**
166 * Validate and normalize an array of application updates submitted via the REST API. Validation exceptions are not
167 * handled here, that's left as an exercise for the caller.
168 *
169 * @param body is the raw, unparsed JSON
170 * @param partialUpdate true if the JSON should be parsed as a partial application update (all fields optional)
171 * or as a wholesale replacement (parsed like an app definition would be)
172 */
173 def canonicalAppUpdatesFromJson(body: Array[Byte], partialUpdate: Boolean): Seq[AppUpdate] = {
174 if (partialUpdate) {
175 validateOrThrow(Json.parse(body).as[Seq[AppUpdate]].map(_.withCanonizedIds()))
176 } else {
177 // this is a complete replacement of the app as we know it, so parse and normalize as if we're dealing
178 // with a brand new app because the rules are different (for example, many fields are non-optional with brand-new apps).
179 // the version is thrown away in toUpdate so just pass `zero` for now.
180 validateOrThrow(Json.parse(body).as[Seq[AppDefinition]].map(_.withCanonizedIds())).map { app =>
181 normalizedApp(app, Timestamp.zero).toUpdate
182 }
183 }
184 }
185
140186 @PUT
141187 @Path("""{id:.+}""")
142188 def replace(
143189 @PathParam("id") id: String,
144190 body: Array[Byte],
145191 @DefaultValue("false")@QueryParam("force") force: Boolean,
192 @DefaultValue("true")@QueryParam("partialUpdate") partialUpdate: Boolean,
146193 @Context req: HttpServletRequest): Response = authenticated(req) { implicit identity =>
147 val appId = id.toRootPath
148 val now = clock.now()
149194
150 withValid(Json.parse(body).as[AppUpdate].copy(id = Some(appId))) { appUpdate =>
151 val plan = result(groupManager.updateApp(appId, updateOrCreate(appId, _, appUpdate), now, force))
195 update(id, body, force, partialUpdate, req, allowCreation = true)
196 }
152197
153 val response = plan.original.app(appId)
154 .map(_ => Response.ok())
155 .getOrElse(Response.created(new URI(appId.toString)))
156 plan.target.app(appId).foreach { appDef =>
157 maybePostEvent(req, appDef)
198 @PATCH
199 @Path("""{id:.+}""")
200 @Timed
201 def patch(
202 @PathParam("id") id: String,
203 body: Array[Byte],
204 @DefaultValue("false")@QueryParam("force") force: Boolean,
205 @Context req: HttpServletRequest): Response = authenticated(req) { implicit identity =>
206
207 update(id, body, force, partialUpdate = true, req, allowCreation = false)
158208 }
159 deploymentResult(plan, response)
160 }
161 }
162209
163210 @PUT
164211 def replaceMultiple(
165212 @DefaultValue("false")@QueryParam("force") force: Boolean,
213 @DefaultValue("true")@QueryParam("partialUpdate") partialUpdate: Boolean,
166214 body: Array[Byte],
167215 @Context req: HttpServletRequest): Response = authenticated(req) { implicit identity =>
168 withValid(Json.parse(body).as[Seq[AppUpdate]].map(_.withCanonizedIds())) { updates =>
169 val version = clock.now()
170216
171 def updateGroup(rootGroup: RootGroup): RootGroup = updates.foldLeft(rootGroup) { (group, update) =>
172 update.id match {
173 case Some(id) => group.updateApp(id, updateOrCreate(id, _, update), version)
174 case None => group
217 updateMultiple(force, partialUpdate, body, allowCreation = true)
175218 }
176 }
177219
178 deploymentResult(result(groupManager.updateRoot(PathId.empty, updateGroup, version, force)))
220 @PATCH
221 @Timed
222 def patchMultiple(
223 @DefaultValue("false")@QueryParam("force") force: Boolean,
224 body: Array[Byte],
225 @Context req: HttpServletRequest): Response = authenticated(req) { implicit identity =>
226
227 updateMultiple(force, partialUpdate = true, body, allowCreation = false)
179228 }
180 }
181229
182230 @DELETE
183231 @Path("""{id:.+}""")
184232 def delete(
185233 @DefaultValue("true")@QueryParam("force") force: Boolean,
186234 @PathParam("id") id: String,
187235 @Context req: HttpServletRequest): Response = authenticated(req) { implicit identity =>
188236 val appId = id.toRootPath
Show All 31 Lines
220268 val newVersion = clock.now()
221269 val restartDeployment = result(
222270 groupManager.updateApp(id.toRootPath, markForRestartingOrThrow, newVersion, force)
223271 )
224272
225273 deploymentResult(restartDeployment)
226274 }
227275
228 private def updateOrCreate(
276 /**
277 * Internal representation of `replace or update` logic.
278 *
279 * @param id appId
280 * @param body request body
281 * @param force force update?
282 * @param partialUpdate partial update?
283 * @param req http servlet request
284 * @param allowCreation is creation allowed?
285 * @param identity implicit identity
286 * @return http servlet response
287 */
288 private[this] def update(id: String, body: Array[Byte], force: Boolean, partialUpdate: Boolean,
289 req: HttpServletRequest, allowCreation: Boolean)(implicit identity: Identity): Response = {
290 val appId = id.toRootPath
291
292 assumeValid {
293 val appUpdate = canonicalAppUpdateFromJson(appId, body, partialUpdate)
294 val version = clock.now()
295 val plan = result(groupManager.updateApp(appId, updateOrCreate(appId, _, appUpdate, partialUpdate, allowCreation), version, force))
296 val response = plan.original.app(appId)
297 .map(_ => Response.ok())
298 .getOrElse(Response.created(new URI(appId.toString)))
299 plan.target.app(appId).foreach { appDef =>
300 maybePostEvent(req, appDef)
301 }
302 deploymentResult(plan, response)
303 }
304 }
305
306 /**
307 * Internal representation of `replace or update` logic for multiple apps.
308 *
309 * @param force force update?
310 * @param partialUpdate partial update?
311 * @param body request body
312 * @param allowCreation is creation allowed?
313 * @param identity implicit identity
314 * @return http servlet response
315 */
316 private[this] def updateMultiple(force: Boolean, partialUpdate: Boolean,
317 body: Array[Byte], allowCreation: Boolean)(implicit identity: Identity): Response = {
318
319 assumeValid {
320 val version = clock.now()
321 val updates = canonicalAppUpdatesFromJson(body, partialUpdate)
322
323 def updateGroup(rootGroup: RootGroup): RootGroup = updates.foldLeft(rootGroup) { (group, update) =>
324 update.id match {
325 case Some(id) => group.updateApp(id, updateOrCreate(id, _, update, partialUpdate, allowCreation = allowCreation), version)
326 case None => group
327 }
328 }
329
330 deploymentResult(result(groupManager.updateRoot(PathId.empty, updateGroup, version, force)))
331 }
332 }
333
334 private[v2] def updateOrCreate(
229335 appId: PathId,
230336 existing: Option[AppDefinition],
231 appUpdate: AppUpdate)(implicit identity: Identity): AppDefinition = {
337 appUpdate: AppUpdate,
338 partialUpdate: Boolean,
339 allowCreation: Boolean)(implicit identity: Identity): AppDefinition = {
232340 def createApp(): AppDefinition = {
233341 val app = validateOrThrow(appUpdate.empty(appId))
234342 checkAuthorization(CreateRunSpec, app)
235343 }
236344
237345 def updateApp(current: AppDefinition): AppDefinition = {
238 val app = validateOrThrow(appUpdate(current))
239 checkAuthorization(UpdateRunSpec, app)
346 val updatedApp = if (partialUpdate) appUpdate(current) else appUpdate.empty(appId)
347 val validatedApp = validateOrThrow(updatedApp)
348 checkAuthorization(UpdateRunSpec, validatedApp)
240349 }
241350
242351 def rollback(current: AppDefinition, version: Timestamp): AppDefinition = {
243352 val app = service.getApp(appId, version).getOrElse(throw AppNotFoundException(appId))
244353 checkAuthorization(ViewRunSpec, app)
245354 checkAuthorization(UpdateRunSpec, current)
246355 app
247356 }
248357
249358 def updateOrRollback(current: AppDefinition): AppDefinition = appUpdate.version
250359 .map(rollback(current, _))
251360 .getOrElse(updateApp(current))
252361
253362 existing match {
254363 case Some(app) =>
255364 // we can only rollback existing apps because we deleted all old versions when dropping an app
256365 updateOrRollback(app)
257 case None =>
366 case None if allowCreation =>
258367 createApp()
368 case None =>
369 throw AppNotFoundException(appId)
259370 }
260371 }
261372
262373 private def maybePostEvent(req: HttpServletRequest, app: AppDefinition) =
263374 eventBus.publish(ApiPostEvent(req.getRemoteAddr, req.getRequestURI, app))
264375
265376 private[v2] def search(cmd: Option[String], id: Option[String], label: Option[String]): AppSelector = {
266377 def containCaseInsensitive(a: String, b: String): Boolean = b.toLowerCase contains a.toLowerCase
Show All 19 Lines
View Options

src/main/scala/mesosphere/marathon/state/AppDefinition.scala

11package mesosphere.marathon
22package state
33
44import java.util.regex.Pattern
55
66import com.wix.accord._
77import com.wix.accord.combinators.GeneralPurposeCombinators
88import com.wix.accord.dsl._
99import mesosphere.marathon.Protos.Constraint
1010import mesosphere.marathon.api.serialization._
1111import mesosphere.marathon.api.v2.Validation._
12import mesosphere.marathon.api.v2.json.AppUpdate
1213import mesosphere.marathon.core.externalvolume.ExternalVolumes
1314import mesosphere.marathon.core.health._
1415import mesosphere.marathon.core.plugin.PluginManager
1516import mesosphere.marathon.core.readiness.ReadinessCheck
1617import mesosphere.marathon.plugin.validation.RunSpecValidator
1718import mesosphere.marathon.raml.Resources
1819import mesosphere.marathon.state.AppDefinition.Labels
1920import mesosphere.marathon.state.Container.{ Docker, MesosAppC, MesosDocker }
▲ Show 20 Lines • Show All 337 Lines • ▼ Show 20 Line(s)
357358 def fromDiscoveryInfo = ipAddress.map(_.discoveryInfo.ports.map(_.name).toList).getOrElse(Seq.empty)
358359 def fromPortMappings = container.map(_.portMappings.flatMap(_.name)).getOrElse(Seq.empty)
359360 def fromPortDefinitions = portDefinitions.flatMap(_.name)
360361
361362 if (networkModeBridge || networkModeUser) fromPortMappings
362363 else if (ipAddress.isDefined) fromDiscoveryInfo
363364 else fromPortDefinitions
364365 }
366
367 /**
368 * @return an app update generated from this app definition, but without the `version` field (since that can only
369 * be combined with `id` for app updates)
370 */
371 def toUpdate: AppUpdate =
meichstedtUnsubmitted
Not Done

I hadn't seen this in the original changeset. Wrong place imo: AppUpdate already has a dependency on AppDefinition (e.g. def apply(app AppDefinition): AppDefinition)). To not have both classes depend on each other, I'd suggest to move this to an AppUpdate.fromApp(...)

372 AppUpdate(
373 id = Some(id),
374 cmd = cmd,
375 args = Some(args),
376 user = user,
377 env = Some(env),
378 instances = Some(instances),
379 cpus = Some(resources.cpus),
380 mem = Some(resources.mem),
381 disk = Some(resources.disk),
382 gpus = Some(resources.gpus),
383 executor = Some(executor),
384 constraints = Some(constraints),
385 fetch = Some(fetch),
386 storeUrls = Some(storeUrls),
387 portDefinitions = Some(portDefinitions),
388 requirePorts = Some(requirePorts),
389 backoff = Some(backoffStrategy.backoff),
390 backoffFactor = Some(backoffStrategy.factor),
391 maxLaunchDelay = Some(backoffStrategy.maxLaunchDelay),
392 container = container,
393 healthChecks = Some(healthChecks),
394 readinessChecks = Some(readinessChecks),
395 taskKillGracePeriod = taskKillGracePeriod,
396 dependencies = Some(dependencies),
397 upgradeStrategy = Some(upgradeStrategy),
398 labels = Some(labels),
399 acceptedResourceRoles = Some(acceptedResourceRoles),
400 ipAddress = ipAddress,
401 residency = residency,
402 secrets = Some(secrets),
403 unreachableStrategy = Some(unreachableStrategy),
404 killSelection = Some(killSelection)
405 // purposefully ignore version since it can only be combined with the `id` field
406 )
365407}
366408
367409@SuppressWarnings(Array("IsInstanceOf")) // doesn't work well in the validation macros?!
368410object AppDefinition extends GeneralPurposeCombinators {
369411
370412 type AppKey = PathId
371413
372414 val RandomPortValue: Int = 0
▲ Show 20 Lines • Show All 343 Lines • Show Last 20 Lines
View Options

src/test/scala/mesosphere/marathon/api/v2/AppsResourceTest.scala

Show First 20 Lines • Show All 153 Lines • ▼ Show 20 Line(s)
154154 ipAddress = Some(IpAddress()),
155155 portDefinitions = Seq.empty[PortDefinition])
156156 prepareApp(app, groupManager)
157157
158158 When("The application is updated")
159159 val updatedApp = app.copy(ipAddress = Some(IpAddress(networkName = Some("bar"))))
160160 val updatedJson = Json.toJson(updatedApp).as[JsObject] - "uris" - "version"
161161 val updatedBody = Json.stringify(updatedJson).getBytes("UTF-8")
162 val response = appsResource.replace(updatedApp.id.toString, updatedBody, false, auth.request)
162 val response = appsResource.replace(updatedApp.id.toString, updatedBody, force = false, partialUpdate = true, auth.request)
163163
164164 Then("It is successful")
165165 response.getStatus should be(200)
166166 response.getMetadata.containsKey(RestResource.DeploymentHeader) should be(true)
167167 }
168168
169 "Do partial update with patch methods" in new Fixture {
170 Given("An app")
171 val id = PathId("/app")
172 val app = AppDefinition(
173 id = id,
174 cmd = Some("cmd"),
175 instances = 1
176 )
177 prepareApp(app, groupManager) // app is stored
178
179 When("The application is updated")
180 val updateRequest = AppDefinition(id = id, instances = 2)
181 val updatedJson = Json.toJson(updateRequest).as[JsObject] - "uris" - "version"
182 val updatedBody = Json.stringify(updatedJson).getBytes("UTF-8")
183 val response = appsResource.patch(app.id.toString, updatedBody, force = false, auth.request)
jdefUnsubmitted
Not Done

there's limits to what this is actually testing. because the groupManager is fake for the default fixture (AFAIR) there's a fair amount of (callback) update logic that's never actually invoked here. the other new test case (added below this one) is a bit more narrowly focused - the downside being that it's not invoking the REST endpoint directly. the callback-oriented design of groupManager makes testing this a bit more complicated that it should be

184
185 Then("It is successful")
186 response.getStatus should be(200)
187 response.getMetadata.containsKey(RestResource.DeploymentHeader) should be(true)
188 }
189
169190 "Create a new app with IP/CT on virtual network foo" in new Fixture {
170191 Given("An app and group")
171192 val app = AppDefinition(
172193 id = PathId("/app"),
173194 cmd = Some("cmd"),
174195 ipAddress = Some(IpAddress(networkName = Some("foo"))),
175196 portDefinitions = Seq.empty[PortDefinition])
176197 val (body, plan) = prepareApp(app, groupManager)
▲ Show 20 Lines • Show All 59 Lines • ▼ Show 20 Line(s)
236257 ipAddress = Some(IpAddress(networkName = Some("foo"))),
237258 portDefinitions = Seq.empty[PortDefinition])
238259 prepareApp(app, groupManager)
239260
240261 When("The application is updated")
241262 val updatedApp = app.copy(ipAddress = Some(IpAddress(networkName = Some("bar"))))
242263 val updatedJson = Json.toJson(updatedApp).as[JsObject] - "uris" - "version"
243264 val updatedBody = Json.stringify(updatedJson).getBytes("UTF-8")
244 val response = appsResource.replace(updatedApp.id.toString, updatedBody, false, auth.request)
265 val response = appsResource.replace(updatedApp.id.toString, updatedBody, force = false, partialUpdate = true, auth.request)
245266
246267 Then("It is successful")
247268 response.getStatus should be(200)
248269 response.getMetadata.containsKey(RestResource.DeploymentHeader) should be(true)
249270 }
250271
251272 "Create a new app with IP/CT on virtual network foo, then update it to nothing" in new Fixture {
252273 Given("An app and group")
253274 val app = AppDefinition(
254275 id = PathId("/app"),
255276 cmd = Some("cmd"),
256277 ipAddress = Some(IpAddress(networkName = Some("foo"))),
257278 portDefinitions = Seq.empty[PortDefinition])
258279 prepareApp(app, groupManager)
259280
260281 When("The application is updated")
261282 val updatedApp = app.copy(ipAddress = Some(IpAddress()))
262283 val updatedJson = Json.toJson(updatedApp).as[JsObject] - "uris" - "version"
263284 val updatedBody = Json.stringify(updatedJson).getBytes("UTF-8")
264 val response = appsResource.replace(updatedApp.id.toString, updatedBody, false, auth.request)
285 val response = appsResource.replace(updatedApp.id.toString, updatedBody, force = false, partialUpdate = true, auth.request)
265286
266287 Then("It is successful")
267288 response.getStatus should be(200)
268289 response.getMetadata.containsKey(RestResource.DeploymentHeader) should be(true)
269290 }
270291
271292 "Create a new app without IP/CT when default virtual network is bar" in new Fixture(configArgs = Seq("--default_network_name", "bar")) {
272293 Given("An app and group")
▲ Show 20 Lines • Show All 417 Lines • ▼ Show 20 Line(s)
690711 val app = AppDefinition(id = PathId("/app"), cmd = Some("foo"))
691712 val rootGroup = createRootGroup(Map(app.id -> app))
692713 val plan = DeploymentPlan(rootGroup, rootGroup)
693714 val body = """{ "cmd": "bla" }""".getBytes("UTF-8")
694715 groupManager.updateApp(any, any, any, any, any) returns Future.successful(plan)
695716 groupManager.app(PathId("/app")) returns Some(app)
696717
697718 When("The application is updated")
698 val response = appsResource.replace(app.id.toString, body, false, auth.request)
719 val response = appsResource.replace(app.id.toString, body, force = false, partialUpdate = true, auth.request)
699720
700721 Then("The application is updated")
701722 response.getStatus should be(200)
702723 response.getMetadata.containsKey(RestResource.DeploymentHeader) should be(true)
703724 }
704725
705726 "Replace an existing application using ports instead of portDefinitions" in new Fixture {
706727 Given("An app and group")
707728 val app = AppDefinition(id = PathId("/app"), cmd = Some("foo"))
708729 prepareApp(app, groupManager)
709730
710731 val appJson = Json.toJson(app).as[JsObject]
711732 val appJsonWithOnlyPorts = appJson - "uris" - "portDefinitions" - "version" +
712733 ("ports" -> Json.parse("""[1000, 1001]"""))
713734 val body = Json.stringify(appJsonWithOnlyPorts).getBytes("UTF-8")
714735
715736 When("The application is updated")
716 val response = appsResource.replace(app.id.toString, body, false, auth.request)
737 val response = appsResource.replace(app.id.toString, body, force = false, partialUpdate = true, auth.request)
717738
718739 Then("The application is updated")
719740 response.getStatus should be(200)
720741 response.getMetadata.containsKey(RestResource.DeploymentHeader) should be(true)
721742 }
722743
723744 "Replace an existing application fails due to docker container validation" in new Fixture {
724745 Given("An app update with an invalid container (missing docker field)")
725746 val app = AppDefinition(id = PathId("/app"), cmd = Some("foo"))
726747 prepareApp(app, groupManager)
727748
728749 val body =
729750 """{
730751 | "cmd": "sleep 1",
731752 | "container": {
732753 | "type": "DOCKER"
733754 | }
734755 |}""".
735756 stripMargin.getBytes("UTF-8")
736757
737758 Then("A serialization exception is thrown")
738759 intercept[JsResultException] {
739 appsResource.replace(app.id.toString, body, force = false, auth.
760 appsResource.replace(app.id.toString, body, force = false, partialUpdate = true, auth.
740761 request)
741762 }
742763 }
743764
744765 def createAppWithVolumes(`type`: String, volumes: String, groupManager: GroupManager, appsResource: AppsResource, auth: TestAuthFixture): Response = {
745766 val app = AppDefinition(id = PathId("/app"), cmd = Some(
746767 "foo"))
747768
▲ Show 20 Lines • Show All 342 Lines • ▼ Show 20 Line(s)
10901111 | "type": "MESOS",
10911112 | "docker": {
10921113 | "image": "/test:latest"
10931114 | }
10941115 | }
10951116 |}""".stripMargin.getBytes("UTF-8")
10961117
10971118 When("The application is updated")
1098 val response = appsResource.replace(app.id.toString, body, force = false, auth.request)
1119 val response = appsResource.replace(app.id.toString, body, force = false, partialUpdate = true, auth.request)
10991120
11001121 Then("The return code indicates success")
11011122 response.getStatus should be(200)
11021123 response.getMetadata.containsKey(RestResource.DeploymentHeader) should be(true)
11031124 }
11041125
1126 "Replacing an existing docker application, upgrading from host to user networking" in new Fixture {
1127 Given("a docker app using host networking and non-empty port definitions")
1128 val app = AppDefinition(
1129 id = PathId("/app"), container = Some(Container.Docker(image = "foo")), portDefinitions = PortDefinitions(0))
1130
1131 When("upgraded to user networking using full-replacement semantics (no port definitions)")
1132 val body =
1133 """{
1134 | "cmd": "sleep 1",
1135 | "container": {
1136 | "type": "DOCKER",
1137 | "docker": {
1138 | "image": "/test:latest",
1139 | "network": "USER"
1140 | }
1141 | },
1142 | "ipAddress": { "name": "dcos" }
1143 |}""".stripMargin.getBytes("UTF-8")
1144 val appUpdate = appsResource.canonicalAppUpdateFromJson(app.id, body, partialUpdate = false)
1145
1146 Then("the application is updated")
1147 appsResource.updateOrCreate(
1148 app.id, Some(app), appUpdate, partialUpdate = false, allowCreation = true)(auth.identity)
1149
1150 And("fails when the update operation uses partial-update semantics")
1151 val caught = intercept[IllegalArgumentException] {
1152 val partUpdate = appsResource.canonicalAppUpdateFromJson(app.id, body, partialUpdate = true)
1153 appsResource.updateOrCreate(
1154 app.id, Some(app), partUpdate, partialUpdate = true, allowCreation = false)(auth.identity)
1155 }
1156 assert(caught.getMessage.indexOf(
1157 s"IP address (${Option(IpAddress())}) and ports (${PortDefinitions(0)}) are not allowed at the same time"
1158 ) > -1)
1159 }
1160
11051161 "Restart an existing app" in new Fixture {
11061162 val app = AppDefinition(id = PathId("/app"))
11071163 val rootGroup = createRootGroup(Map(app.id -> app))
11081164 val plan = DeploymentPlan(rootGroup, rootGroup)
11091165 service.deploy(any, any) returns Future.successful(Done)
11101166 groupManager.app(PathId("/app")) returns Some(app)
11111167
11121168 groupManager.updateApp(any, any, any, any, any) returns Future.successful(plan)
▲ Show 20 Lines • Show All 98 Lines • ▼ Show 20 Line(s)
12111267 create.getStatus should be(auth.NotAuthenticatedStatus)
12121268
12131269 When("we try to fetch an app")
12141270 val show = appsResource.show("", embed, req)
12151271 Then("we receive a NotAuthenticated response")
12161272 show.getStatus should be(auth.NotAuthenticatedStatus)
12171273
12181274 When("we try to update an app")
1219 val replace = appsResource.replace("", app.getBytes("UTF-8"), false, req)
1275 val replace = appsResource.replace("", app.getBytes("UTF-8"), force = false, partialUpdate = true, req)
12201276 Then("we receive a NotAuthenticated response")
12211277 replace.getStatus should be(auth.NotAuthenticatedStatus)
12221278
12231279 When("we try to update multiple apps")
1224 val replaceMultiple = appsResource.replaceMultiple(false, s"[$app]".getBytes("UTF-8"), req)
1280 val replaceMultiple = appsResource.replaceMultiple(force = false, partialUpdate = true, s"[$app]".getBytes("UTF-8"), req)
12251281 Then("we receive a NotAuthenticated response")
12261282 replaceMultiple.getStatus should be(auth.NotAuthenticatedStatus)
12271283
12281284 When("we try to delete an app")
12291285 val delete = appsResource.delete(false, "", req)
12301286 Then("we receive a NotAuthenticated response")
12311287 delete.getStatus should be(auth.NotAuthenticatedStatus)
12321288
Show All 21 Lines
12541310 create.getStatus should be(auth.UnauthorizedStatus)
12551311
12561312 When("we try to fetch an app")
12571313 val show = appsResource.show("*", embed, req)
12581314 Then("we receive a NotAuthorized response")
12591315 show.getStatus should be(auth.UnauthorizedStatus)
12601316
12611317 When("we try to update an app")
1262 val replace = appsResource.replace("/a", app.getBytes("UTF-8"), false, req)
1318 val replace = appsResource.replace("/a", app.getBytes("UTF-8"), force = false, partialUpdate = true, req)
12631319 Then("we receive a NotAuthorized response")
12641320 replace.getStatus should be(auth.UnauthorizedStatus)
12651321
12661322 When("we try to update multiple apps")
1267 val replaceMultiple = appsResource.replaceMultiple(false, s"[$app]".getBytes("UTF-8"), req)
1323 val replaceMultiple = appsResource.replaceMultiple(force = false, partialUpdate = true, s"[$app]".getBytes("UTF-8"), req)
12681324 Then("we receive a NotAuthorized response")
12691325 replaceMultiple.getStatus should be(auth.UnauthorizedStatus)
12701326
12711327 When("we try to remove an app")
12721328 val delete = appsResource.delete(false, "/a", req)
12731329 Then("we receive a NotAuthorized response")
12741330 delete.getStatus should be(auth.UnauthorizedStatus)
12751331
▲ Show 20 Lines • Show All 51 Lines • Show Last 20 Lines
View Options

src/test/scala/mesosphere/marathon/integration/AppDeployIntegrationTest.scala

Show First 20 Lines • Show All 330 Lines • ▼ Show 20 Line(s)
331331
332332 Then("The app gets updated")
333333 update.code should be(200)
334334 waitForDeployment(update)
335335 waitForTasks(id, before.value.size)
336336 check.pingSince(5.seconds) should be(true) //make sure, the new version is alive
337337 }
338338
339 "update an app through patch request" in {
340 Given("a new app")
341 val appId = testBasePath / "app"
342 val v1 = appProxy(appId, "v1", instances = 1, healthCheck = Some(appProxyHealthCheck()))
343 val create = marathon.createAppV2(v1)
344 create.code should be (201)
345 waitForDeployment(create)
346 val before = marathon.tasks(appId)
347
348 When("The app is updated")
349 val check = appProxyCheck(appId, "v2", state = true)
350 val update = marathon.patchApp(v1.id, AppUpdate(cmd = appProxy(appId, "v2", 1).cmd))
351
352 Then("The app gets updated")
353 update.code should be (200)
354 waitForDeployment(update)
355 waitForTasks(appId, before.value.size)
356 check.pingSince(5.seconds) should be (true) //make sure, the new version is alive
357
358 Then("Check if healthcheck is not updated")
359 val appResult = marathon.app(appId)
360 appResult.code should be (200)
361 appResult.value.app.healthChecks
362 }
363
339364 "scale an app up and down" in {
340365 Given("a new app")
341366 val app = appProxy(appId(), "v1", instances = 1, healthCheck = None)
342367 val create = marathon.createAppV2(app)
343368 create.code should be(201)
344369 waitForDeployment(create)
345370
346371 When("The app gets an update to be scaled up")
▲ Show 20 Lines • Show All 390 Lines • Show Last 20 Lines
View Options

src/test/scala/mesosphere/marathon/integration/facades/MarathonFacade.scala

Show First 20 Lines • Show All 152 Lines • ▼ Show 20 Line(s)
153153 requireInBaseGroup(id)
154154 val pipeline = marathonSendReceive ~> read[ITDeploymentResult]
155155 val putUrl: String = s"$url/v2/apps$id?force=$force"
156156 LoggerFactory.getLogger(getClass).info(s"put url = $putUrl")
157157
158158 result(pipeline(Put(putUrl, app)), waitTime)
159159 }
160160
161 def patchApp(id: PathId, app: AppUpdate, force: Boolean = false): RestResult[ITDeploymentResult] = {
162 requireInBaseGroup(id)
163 val pipeline = marathonSendReceive ~> read[ITDeploymentResult]
164 val putUrl: String = s"$url/v2/apps$id?force=$force"
165 LoggerFactory.getLogger(getClass).info(s"put url = $putUrl")
166
167 result(pipeline(Patch(putUrl, app)), waitTime)
168 }
169
161170 def restartApp(id: PathId, force: Boolean = false): RestResult[ITDeploymentResult] = {
162171 requireInBaseGroup(id)
163172 val pipeline = marathonSendReceive ~> read[ITDeploymentResult]
164173 result(pipeline(Post(s"$url/v2/apps$id/restart?force=$force")), waitTime)
165174 }
166175
167176 def listAppVersions(id: PathId): RestResult[ITAppVersions] = {
168177 requireInBaseGroup(id)
▲ Show 20 Lines • Show All 252 Lines • Show Last 20 Lines