HomeMesosphereNo notifications. 4 unresolved issues.
  • Search
Differential D932

Always set the RootGroup version to the current timestamp whenever it or its elements are updated.
ClosedAll Users
Actions

Authored by ichernetsky on Jul 19 2017, 12:09 AM.

Details

Reviewers
timcharper
kensipe
zen-dog
meichstedt
jenkins
Commits
rMARATHON762a11fbda36: Always set the RootGroup version to the current timestamp whenever it or its…
JIRA Issues
JIRA MARATHON-7401 Apps get removed during Marathon upgrade and leader re-election
JIRA MARATHON-7469 Marathon killed tasks involved in Deployment after leader election
Test Plan

sbt 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.
ichernetsky created this revision.Jul 19 2017, 12:09 AM
jenkins requested changes to this revision.Jul 19 2017, 12:12 AM
This revision now requires changes to proceed.Jul 19 2017, 12:12 AM
jenkins added a comment.Jul 19 2017, 12:29 AM
✗ Build of 3815 failed jenkins-public-marathon-phabricator-536.

Error message:

Stage Compile and Test failed.

(๑′°︿°๑)

jenkins added a comment.Jul 19 2017, 12:58 AM
✗ Build of 3815 failed jenkins-public-marathon-phabricator-537.

Error message:

Stage Compile and Test failed.

(๑′°︿°๑)

jenkins added a comment.Jul 19 2017, 4:28 AM
✗ Build of 3815 failed jenkins-public-marathon-phabricator-538.

Error message:

Stage Compile and Test failed.

(๑′°︿°๑)

jenkins added a comment.Jul 19 2017, 5:36 AM
✗ Build of 3815 failed jenkins-public-marathon-phabricator-539.

Error message:

Stage Compile and Test failed.

(๑′°︿°๑)

jenkins accepted this revision.Jul 19 2017, 6:09 AM
This revision is now accepted and ready to land.Jul 19 2017, 6:09 AM
jenkins added a comment.Jul 19 2017, 6:09 AM
✔ Build of 3815 completed jenkins-public-marathon-phabricator-540.

You can create a DC/OS with your patched Marathon by creating a new pull
request with the following changes in buildinfo.json:

"url": "https://downloads.mesosphere.io/marathon/snapshots/marathon-1.5.0-SNAPSHOT-660-g128bc41.tgz",
"sha1": "43d84d10cfba6c6d040103ca1ce0e852ec624325"

＼\ ٩( ᐛ )و /／

jenkins requested changes to this revision.Jul 19 2017, 6:22 AM
This revision now requires changes to proceed.Jul 19 2017, 6:22 AM
jenkins added a comment.Jul 19 2017, 6:33 AM
✗ Build of 3815 failed jenkins-public-marathon-phabricator-542.

Error message:

Stage Compile and Test failed.

(๑′°︿°๑)

jenkins accepted this revision.Jul 19 2017, 6:53 AM
This revision is now accepted and ready to land.Jul 19 2017, 6:53 AM
jenkins added a comment.Jul 19 2017, 6:53 AM
✔ Build of 3815 completed jenkins-public-marathon-phabricator-543.

You can create a DC/OS with your patched Marathon by creating a new pull
request with the following changes in buildinfo.json:

"url": "https://downloads.mesosphere.io/marathon/snapshots/marathon-1.5.0-SNAPSHOT-660-g128bc41.tgz",
"sha1": "3e13ad168ee82f5bfea81f5dff369708a8d95895"

＼\ ٩( ᐛ )و /／

zen-dog added a comment.Jul 20 2017, 11:48 AM

Could you please add a diff description? I'm not sure for example why GroupManager defaults version to Group.defaultVersion (instead of Timestamp.now()).
Also - why does the RootGroup has a default at all - shouldn't we rather leave it empty and force the caller to pass the version?

ichernetsky added a comment.Jul 20 2017, 6:54 PM
In D932#36691, @zen-dog wrote:

Could you please add a diff description? I'm not sure for example why GroupManager defaults version to Group.defaultVersion (instead of Timestamp.now()).
Also - why does the RootGroup has a default at all - shouldn't we rather leave it empty and force the caller to pass the version?

Group.defaultVersion is Timestamp.now(). As far as I can tell, initially the intention was to use it everywhere, implying that unless one has a good reason to pass a specific version, it is better to use Group.defaultVersion by default which in turn is Timestamp.now().

ichernetsky added a comment.Jul 20 2017, 6:56 PM

As to why "GroupManager defaults version to Group.defaultVersion", it is because GroupManager deals with group versions.

zen-dog accepted this revision.Jul 25 2017, 3:25 PM

Accept assuming a proper diff description ;) Thx!

kensipe accepted this revision.Jul 25 2017, 3:46 PM
ichernetsky added 1 JIRA issue(s): MARATHON-7401.Jul 25 2017, 6:49 PM
Closed by commit rMARATHON762a11fbda36: Always set the RootGroup version to the current timestamp whenever it or its… (authored by ichernetsky). · Explain WhyJul 25 2017, 6:50 PM
This revision was automatically updated to reflect the committed changes.
jdef added a subscriber: jdef.Jul 25 2017, 7:21 PM

Sad to see this land w/o a proper description in the review Summary. Please don't let this happen again

zen-dog added a comment.Jul 25 2017, 10:46 PM

Seriously! My accept was conditional. Next guy to come across this code will wonder why you did what you did. The bug wasn't subtle and it deserves explanation and a link to JIRA ticket.

ichernetsky added a comment.Jul 26 2017, 11:35 AM

Guys, would you like me to elaborate on why there is no reason to be sad or serious or both about it?

Revision Contents

Diff 3847

View Options

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

Show First 20 Lines • Show All 230 Lines • ▼ Show 20 Line(s)
231231
232232 @DELETE
233233 @Path("""{id:.+}""")
234234 def delete(
235235 @DefaultValue("true")@QueryParam("force") force: Boolean,
236236 @PathParam("id") id: String,
237237 @Context req: HttpServletRequest): Response = authenticated(req) { implicit identity =>
238238 val appId = id.toRootPath
239 val version = Timestamp.now()
239240
240241 def deleteApp(rootGroup: RootGroup) = {
241242 checkAuthorization(DeleteRunSpec, rootGroup.app(appId), AppNotFoundException(appId))
242 rootGroup.removeApp(appId)
243 rootGroup.removeApp(appId, version)
243244 }
244245
245 deploymentResult(result(groupManager.updateRoot(appId.parent, deleteApp, force = force)))
246 deploymentResult(result(groupManager.updateRoot(appId.parent, deleteApp, version = version, force = force)))
246247 }
247248
248249 @Path("{appId:.+}/tasks")
249250 def appTasksResource(): AppTasksResource = appTasksRes
250251
251252 @Path("{appId:.+}/versions")
252253 def appVersionsResource(): AppVersionsResource = new AppVersionsResource(service, groupManager, authenticator,
253254 authorizer, config)
Show All 35 Lines
289290 */
290291 private[this] def update(id: String, body: Array[Byte], force: Boolean, partialUpdate: Boolean,
291292 req: HttpServletRequest, allowCreation: Boolean)(implicit identity: Identity): Response = {
292293 val appId = id.toRootPath
293294
294295 assumeValid {
295296 val appUpdate = canonicalAppUpdateFromJson(appId, body, partialUpdate)
296297 val version = clock.now()
297 val plan = result(groupManager.updateApp(appId, updateOrCreate(appId, _, appUpdate, partialUpdate, allowCreation), version, force))
298 val plan = result(groupManager.updateApp(
299 appId, updateOrCreate(appId, _, appUpdate, partialUpdate, allowCreation),
300 version, force))
298301 val response = plan.original.app(appId)
299302 .map(_ => Response.ok())
300303 .getOrElse(Response.created(new URI(appId.toString)))
301304 plan.target.app(appId).foreach { appDef =>
302305 maybePostEvent(req, appDef)
303306 }
304307 deploymentResult(plan, response)
305308 }
Show All 13 Lines
319322 body: Array[Byte], allowCreation: Boolean)(implicit identity: Identity): Response = {
320323
321324 assumeValid {
322325 val version = clock.now()
323326 val updates = canonicalAppUpdatesFromJson(body, partialUpdate)
324327
325328 def updateGroup(rootGroup: RootGroup): RootGroup = updates.foldLeft(rootGroup) { (group, update) =>
326329 update.id.map(PathId(_)) match {
327 case Some(id) => group.updateApp(id, updateOrCreate(id, _, update, partialUpdate, allowCreation = allowCreation), version)
330 case Some(id) =>
331 group.updateApp(
332 id, updateOrCreate(id, _, update, partialUpdate, allowCreation = allowCreation), version)
328333 case None => group
329334 }
330335 }
331336
332337 deploymentResult(result(groupManager.updateRoot(PathId.empty, updateGroup, version, force)))
333338 }
334339 }
335340
▲ Show 20 Lines • Show All 107 Lines • Show Last 20 Lines
View Options

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

Show First 20 Lines • Show All 236 Lines • ▼ Show 20 Line(s)
237237
238238 val groupValidator = Group.validNestedGroupUpdateWithBase(effectivePath)
239239 val groupUpdate = validateOrThrow(
240240 normalizeApps(
241241 effectivePath,
242242 raw
243243 ))(groupValidator)
244244
245 val newVersion = Timestamp.now()
246
247245 if (dryRun) {
248
246 val newVersion = Timestamp.now()
249247 val originalGroup = groupManager.rootGroup()
250248 val updatedGroup = applyGroupUpdate(originalGroup, effectivePath, groupUpdate, newVersion)
251249
252250 ok(
253251 Json.obj(
254252 "steps". ->(DeploymentPlan(originalGroup, updatedGroup).steps)
255253 ).toString()
256254 )
▲ Show 20 Lines • Show All 88 Lines • ▼ Show 20 Line(s)
345343
346344 private def updateOrCreate(
347345 id: PathId,
348346 update: raml.GroupUpdate,
349347 force: Boolean)(implicit identity: Identity): (DeploymentPlan, PathId) = {
350348 val version = Timestamp.now()
351349
352350 val effectivePath = update.id.map(PathId(_).canonicalPath(id)).getOrElse(id)
353 val deployment = result(groupManager.updateRoot(id.parent, applyGroupUpdate(_, effectivePath, update, version), version, force))
351 val deployment = result(groupManager.updateRoot(
352 id.parent, applyGroupUpdate(_, effectivePath, update, version), version, force))
354353 (deployment, effectivePath)
355354 }
356355
357356 def authorizationSelectors(implicit identity: Identity): GroupInfoService.Selectors = {
358357 GroupInfoService.Selectors(
359358 AppsResource.authzSelector,
360359 PodsResource.authzSelector,
361360 authzSelector)
Show All 9 Lines
View Options

src/main/scala/mesosphere/marathon/core/deployment/impl/DeploymentPlanReverter.scala

Show All 16 Lines
1717 *
1818 * If they were no concurrent deployments and the passed group is equal to
1919 * the target group, this method should result in the original group
2020 * except for version/timestamp changes.
2121 *
2222 * If there were concurrent changes, this method tries to revert the changes
2323 * of this deployment only without affecting the other deployments.
2424 */
25 def revert(original: RootGroup, target: RootGroup, newVersion: Timestamp = Timestamp.now()): RootGroup => RootGroup = {
25 def revert(original: RootGroup, target: RootGroup, newVersion: Timestamp = Group.defaultVersion): RootGroup => RootGroup = {
2626
2727 def changesOnIds[T](originalById: Map[PathId, T], targetById: Map[PathId, T]): Seq[(Option[T], Option[T])] = {
2828 val ids = originalById.keys ++ targetById.keys
2929 ids.map { id => originalById.get(id) -> targetById.get(id) }(collection.breakOut)
3030 }
3131
3232 /* a sequence of tuples with the old and the new group definition (also for unchanged groups) */
3333 val groupChanges: Seq[(Option[Group], Option[Group])] =
Show All 12 Lines
4646
4747 /**
4848 * Reverts any group additions, changes, removals.
4949 *
5050 * It is more difficult than reverting any app definition changes
5151 * because groups are not locked by deployments and concurrent changes are allowed.
5252 */
5353 private[this] def revertGroupChanges(
54 version: Timestamp, groupChanges: Seq[(Option[Group], Option[Group])])(
55 rootGroup: RootGroup): RootGroup = {
54 version: Timestamp = Group.defaultVersion,
55 groupChanges: Seq[(Option[Group], Option[Group])])(rootGroup: RootGroup): RootGroup = {
5656
5757 def revertGroupRemoval(oldGroup: Group)(dependencies: Set[PathId]): Set[PathId] = {
5858 log.debug("re-adding group {} with dependencies {}", Seq(oldGroup.id, oldGroup.dependencies): _*)
5959 if ((oldGroup.dependencies -- dependencies).nonEmpty) dependencies ++ oldGroup.dependencies else dependencies
6060 }
6161
6262 def revertDependencyChanges(oldGroup: Group, newGroup: Group)(dependencies: Set[PathId]): Set[PathId] = {
6363 val removedDependencies = oldGroup.dependencies -- newGroup.dependencies
▲ Show 20 Lines • Show All 111 Lines • Show Last 20 Lines
View Options

src/main/scala/mesosphere/marathon/core/group/GroupManager.scala

Show First 20 Lines • Show All 102 Lines • ▼ Show 20 Line(s)
103103 * @param version the new version of the group, after the change has applied.
104104 * @param force only one update can be applied to applications at a time. with this flag
105105 * one can control, to stop a current deployment and start a new one.
106106 * @return the deployment plan which will be executed.
107107 */
108108 def updateRoot(
109109 id: PathId,
110110 fn: RootGroup => RootGroup,
111 version: Timestamp = Timestamp.now(),
111 version: Timestamp = Group.defaultVersion,
112112 force: Boolean = false,
113113 toKill: Map[PathId, Seq[Instance]] = Map.empty): Future[DeploymentPlan]
114114
115115 /**
116116 * Update application with given identifier and update function.
117117 * The change could take time to get deployed.
118118 * For this reason, we return the DeploymentPlan as result, which can be queried in the marathon scheduler.
119119 *
120120 * @param appId the identifier of the application
121121 * @param fn the application change function
122122 * @param version the version of the change
123123 * @param force if the change has to be forced.
124124 * @return the deployment plan which will be executed.
125125 */
126126 def updateApp(
127127 appId: PathId,
128128 fn: Option[AppDefinition] => AppDefinition,
129 version: Timestamp = Timestamp.now(),
129 version: Timestamp = Group.defaultVersion,
130130 force: Boolean = false,
131131 toKill: Seq[Instance] = Seq.empty): Future[DeploymentPlan] =
132132 updateRoot(appId.parent, _.updateApp(appId, fn, version), version, force, Map(appId -> toKill))
133133
134134 /**
135135 * Update pod with given identifier and update function.
136136 * The change could take time to get deployed.
137137 * For this reason, we return the DeploymentPlan as result, which can be queried in the marathon scheduler.
138138 *
139139 * @param podId the identifier of the pod
140140 * @param fn the pod change function
141141 * @param version the version of the change
142142 * @param force if the change has to be forced.
143143 * @return the deployment plan which will be executed.
144144 */
145145 def updatePod(
146146 podId: PathId,
147147 fn: Option[PodDefinition] => PodDefinition,
148 version: Timestamp = Timestamp.now(),
148 version: Timestamp = Group.defaultVersion,
149149 force: Boolean = false,
150150 toKill: Seq[Instance] = Seq.empty
151 ): Future[DeploymentPlan] = updateRoot(podId.parent, _.updatePod(podId, fn, version), version, force, Map(podId -> toKill))
151 ): Future[DeploymentPlan] = updateRoot(
152 podId.parent, _.updatePod(podId, fn, version), version, force, Map(podId -> toKill))
152153
153154 /**
154155 * Refresh the internal root group cache. When calling this function, the internal hold cached root group will be dropped
155156 * and loaded when accessing the next time.
156157 *
157158 * @return Done if refresh was successful
158159 */
159160 def invalidateGroupCache(): Future[Done]
160161}
View Options

src/main/scala/mesosphere/marathon/core/pod/impl/PodManagerImpl.scala

Show All 27 Lines
2828 }
2929
3030 def find(id: PathId): Option[PodDefinition] = groupManager.pod(id)
3131
3232 def update(p: PodDefinition, force: Boolean): Future[DeploymentPlan] =
3333 groupManager.updatePod(p.id, _ => p, p.version, force)
3434
3535 def delete(id: PathId, force: Boolean): Future[DeploymentPlan] = {
36 groupManager.updateRoot(id.parent, _.removePod(id), force = force)
36 val version = Timestamp.now()
37 groupManager.updateRoot(id.parent, _.removePod(id, version), version = version, force = force)
3738 }
3839
3940 override def versions(id: PathId): Source[Timestamp, NotUsed] = groupManager.podVersions(id).map(Timestamp(_))
4041
4142 override def version(id: PathId, version: Timestamp): Future[Option[PodDefinition]] =
4243 groupManager.podVersion(id, version.toOffsetDateTime)
4344}
View Options

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

Show First 20 Lines • Show All 93 Lines • ▼ Show 20 Line(s)
9494 * This is similar to the behavior of `mkdir -p`.
9595 *
9696 * Every transitive group gets the new version.
9797 *
9898 * @param newGroup the new group to be added
9999 * @param version the new version of the root group
100100 * @return the new root group with `newGroup` added.
101101 */
102 def putGroup(newGroup: Group, version: Timestamp): RootGroup = {
102 def putGroup(newGroup: Group, version: Timestamp = Group.defaultVersion): RootGroup = {
103103 val oldGroup = group(newGroup.id).getOrElse(Group.empty(newGroup.id))
104104 @tailrec def rebuildTree(allParents: List[PathId], result: Group): Group = {
105105 allParents match {
106106 case Nil => result
107107 case head :: tail =>
108108 val oldParent = group(head).getOrElse(Group.empty(head))
109109 val newParent = Group(
110110 id = oldParent.id,
Show All 33 Lines
144144 *
145145 * Every transitive group gets the new version.
146146 *
147147 * @param groupId the root of the group subtree to update
148148 * @param app the update function, which is applied to every transitive app under `groupId`.
149149 * @param version the new version of the root group
150150 * @return the new root group with group with id `groupId`.
151151 */
152 def updateTransitiveApps(groupId: PathId, app: AppDefinition => AppDefinition, version: Timestamp): RootGroup = {
152 def updateTransitiveApps(
153 groupId: PathId, app: AppDefinition => AppDefinition, version: Timestamp = Group.defaultVersion): RootGroup = {
153154 def updateApps(group: Group): Group = {
154155 Group(
155156 id = group.id,
156157 apps = group.apps.map { case (appId, appDef) => appId -> app(appDef) },
157158 pods = group.pods,
158159 groupsById = group.groupsById.map { case (subGroupId, subGroup) => subGroupId -> updateApps(subGroup) },
159160 dependencies = group.dependencies,
160161 version = version,
Show All 16 Lines
177178 * @param groupId the id of the group to be updated
178179 * @param apps the update function, which is applied to the specified group's apps.
179180 * @param version the new version of the root group
180181 * @return the new root group with the specified group updated.
181182 */
182183 def updateApps(
183184 groupId: PathId,
184185 apps: Map[AppDefinition.AppKey, AppDefinition] => Map[AppDefinition.AppKey, AppDefinition],
185 version: Timestamp): RootGroup = {
186 version: Timestamp = Group.defaultVersion): RootGroup = {
186187 val oldGroup = group(groupId).getOrElse(Group.empty(groupId))
187188 val oldApps = oldGroup.apps
188189 val newApps = apps(oldApps)
189190 val newGroup = Group(
190191 id = oldGroup.id,
191192 apps = newApps,
192193 pods = oldGroup.pods,
193194 groupsById = oldGroup.groupsById,
Show All 12 Lines
206207 *
207208 * Every transitive group gets the new version.
208209 *
209210 * @param groupId the id of the group to be updated
210211 * @param dependencies the update function, which is applied to the specified group's dependencies.
211212 * @param version the new version of the root group
212213 * @return the new root group with the specified group updated.
213214 */
214 def updateDependencies(groupId: PathId, dependencies: Set[PathId] => Set[PathId], version: Timestamp): RootGroup = {
215 def updateDependencies(
216 groupId: PathId, dependencies: Set[PathId] => Set[PathId], version: Timestamp = Group.defaultVersion): RootGroup = {
215217 val oldGroup = group(groupId).getOrElse(Group.empty(groupId))
216218 val oldDependencies = oldGroup.dependencies
217219 val newDependencies = dependencies(oldDependencies)
218220 val newGroup = Group(
219221 id = oldGroup.id,
220222 apps = oldGroup.apps,
221223 pods = oldGroup.pods,
222224 groupsById = oldGroup.groupsById,
Show All 15 Lines
238240 *
239241 * Every transitive group gets the new version.
240242 *
241243 * @param appId the id of the app to be updated
242244 * @param fn the update function, which is applied to the specified app
243245 * @param version the new version of the root group
244246 * @return the new root group with the specified app updated.
245247 */
246 def updateApp(appId: PathId, fn: Option[AppDefinition] => AppDefinition, version: Timestamp): RootGroup = {
248 def updateApp(
249 appId: PathId, fn: Option[AppDefinition] => AppDefinition, version: Timestamp = Group.defaultVersion): RootGroup = {
247250 val oldGroup = group(appId.parent).getOrElse(Group.empty(appId.parent))
248251 val newApp = fn(app(appId))
249252 require(newApp.id == appId, "app id must not be changed by `fn`.")
250253 val newGroup = Group(
251254 id = oldGroup.id,
252255 // replace potentially existing app definition
253256 apps = oldGroup.apps + (newApp.id -> newApp),
254257 pods = oldGroup.pods,
Show All 19 Lines
274277 *
275278 * Every transitive group gets the new version.
276279 *
277280 * @param podId the id of the pod to be updated
278281 * @param fn the update function, which is applied to the specified pod
279282 * @param version the new version of the root group
280283 * @return the new root group with the specified pod updated.
281284 */
282 def updatePod(podId: PathId, fn: Option[PodDefinition] => PodDefinition, version: Timestamp): RootGroup = {
285 def updatePod(
286 podId: PathId, fn: Option[PodDefinition] => PodDefinition, version: Timestamp = Group.defaultVersion): RootGroup = {
283287 val oldGroup = group(podId.parent).getOrElse(Group.empty(podId.parent))
284288 val newPod = fn(pod(podId))
285289 require(newPod.id == podId, "pod id must not be changed by `fn`.")
286290 val newGroup = Group(
287291 id = oldGroup.id,
288292 apps = oldGroup.apps,
289293 // replace potentially existing pod definition
290294 pods = oldGroup.pods + (newPod.id -> newPod),
Show All 21 Lines
312316 transitivePodsById = group.transitivePodsById)
313317 }
314318
315319 /**
316320 * Returns a new `RootGroup` where all transitive groups have their `version` set to the specified timestamp.
317321 *
318322 * @param version the new version of the root group.
319323 */
320 def updateVersion(version: Timestamp): RootGroup = RootGroup.fromGroup(updateVersion(this, version))
324 def updateVersion(version: Timestamp = Group.defaultVersion): RootGroup = {
325 RootGroup.fromGroup(updateVersion(this, version))
326 }
321327
322328 /**
323329 * Remove the group with the specified group id.
324330 *
325331 * Every transitive group gets the new version.
326332 *
327333 * @param groupId the id of the group to be removed
328334 * @param version the new version of the root group
329335 * @return the new root group with the specified group removed.
330336 */
331 def removeGroup(groupId: PathId, version: Timestamp = Timestamp.now()): RootGroup = {
337 def removeGroup(groupId: PathId, version: Timestamp = Group.defaultVersion): RootGroup = {
332338 require(!groupId.isRoot, "The root group cannot be removed.")
333339 group(groupId).fold(updateVersion(version)) { oldGroup =>
334340 val oldParent = transitiveGroupsById(oldGroup.id.parent)
335341 putGroup(
336342 Group(
337343 id = oldParent.id,
338344 apps = oldParent.apps,
339345 pods = oldParent.pods,
Show All 9 Lines
349355 * Remove the app with the specified app id.
350356 *
351357 * Every transitive group gets the new version.
352358 *
353359 * @param appId the id of the app to be removed
354360 * @param version the new version of the root group
355361 * @return the new root group with the specified app removed.
356362 */
357 def removeApp(appId: PathId, version: Timestamp = Timestamp.now()): RootGroup = {
363 def removeApp(appId: PathId, version: Timestamp = Group.defaultVersion): RootGroup = {
358364 app(appId).fold(updateVersion(version)) { oldApp =>
359365 val oldGroup = transitiveGroupsById(oldApp.id.parent)
360366 putGroup(Group(
361367 id = oldGroup.id,
362368 apps = oldGroup.apps - oldApp.id,
363369 pods = oldGroup.pods,
364370 groupsById = oldGroup.groupsById,
365371 dependencies = oldGroup.dependencies,
366372 version = version,
367373 transitiveAppsById = oldGroup.transitiveAppsById - oldApp.id,
368374 transitivePodsById = oldGroup.transitivePodsById), version)
369375 }
370376 }
371377
372378 /**
373379 * Remove the pod with the specified pod id.
374380 *
375381 * Every transitive group gets the new version.
376382 *
377383 * @param podId the id of the pod to be removed
378384 * @param version the new version of the root group
379385 * @return the new root group with the specified pod removed.
380386 */
381 def removePod(podId: PathId, version: Timestamp = Timestamp.now()): RootGroup = {
387 def removePod(podId: PathId, version: Timestamp = Group.defaultVersion): RootGroup = {
382388 pod(podId).fold(updateVersion(version)) { oldPod =>
383389 val oldGroup = transitiveGroupsById(oldPod.id.parent)
384390 putGroup(Group(
385391 id = oldGroup.id,
386392 apps = oldGroup.apps,
387393 pods = oldGroup.pods - oldPod.id,
388394 groupsById = oldGroup.groupsById,
389395 dependencies = oldGroup.dependencies,
▲ Show 20 Lines • Show All 50 Lines • Show Last 20 Lines
View Options

src/main/scala/mesosphere/marathon/upgrade/GroupVersioningUtil.scala

11package mesosphere.marathon
22package upgrade
33
44import mesosphere.marathon.state.{ AppDefinition, RootGroup, Timestamp, VersionInfo }
55import org.slf4j.LoggerFactory
66
77/**
88 * Tools related to app/group versioning.
99 */
1010object GroupVersioningUtil {
1111 private[this] val log = LoggerFactory.getLogger(getClass)
1212
1313 /**
1414 * Calculate a new group from the given `to` parameter that sets the version of all changed apps
1515 * to the given `version`.
1616 *
17 * The `RootGroup` object returned from this function, has its version set to the given `version`.
18 * Therefore even if there are no changed apps, the version of `RootGroup` object returned is always
19 * set to `version`.
20 *
1721 * @param version the version of all changed apps
1822 * @param from the original group
1923 * @param to the updated group
2024 * @return the updated group with updated app versions
2125 */
2226 def updateVersionInfoForChangedApps(version: Timestamp, from: RootGroup, to: RootGroup): RootGroup = {
2327
2428 def updateAppVersionInfo(maybeOldApp: Option[AppDefinition], newApp: AppDefinition): AppDefinition = {
Show All 19 Lines
4448 newApp.copy(versionInfo = newVersionInfo)
4549 }
4650
4751 val originalApps = from.transitiveAppsById
4852 val updatedTargetApps = to.transitiveApps.flatMap { newApp =>
4953 val updated = updateAppVersionInfo(originalApps.get(newApp.id), newApp)
5054 if (updated.versionInfo != newApp.versionInfo) Some(updated) else None
5155 }
52 updatedTargetApps.foldLeft(to) { (resultGroup, updatedApp) =>
56 val updatedTo = to.updateVersion(version)
57 updatedTargetApps.foldLeft(updatedTo) { (resultGroup, updatedApp) =>
5358 resultGroup.updateApp(updatedApp.id, _ => updatedApp, version)
5459 }
5560 }
5661
5762}
View Options

src/test/scala/mesosphere/marathon/upgrade/GroupVersioningUtilTest.scala

Show First 20 Lines • Show All 43 Lines • ▼ Show 20 Line(s)
4444 apps = Map(updatedApp.id -> updatedApp),
4545 version = Timestamp(2)
4646 )
4747 ),
4848 version = Timestamp(2)
4949 )
5050
5151 "GroupVersioningUtil" should {
52 "No changes for empty group" in {
52 "No changes for empty group should result in the resulting root group version set to the given one" in {
5353 When("Calculating version infos for an empty group")
5454 val updated = GroupVersioningUtil.updateVersionInfoForChangedApps(Timestamp(10), emptyGroup, emptyGroup)
55 Then("nothing is changed")
56 updated should be(emptyGroup)
55 Then("the version is updated anyway")
56 updated should be(emptyGroup.updateVersion(Timestamp(10)))
5757 }
5858
59 "No changes for nested app" in {
59 "No changes for nested app should result in the resulting root group version set to the given one" in {
6060 When("Calculating version infos with no changes")
6161 val updated = GroupVersioningUtil.updateVersionInfoForChangedApps(Timestamp(10), nestedApp, nestedApp)
62 Then("nothing is changed")
63 updated should be(nestedApp)
62 Then("the version is updated anyway")
63 updated should be(nestedApp.updateVersion(Timestamp(10)))
6464 }
6565
6666 "A new app should get proper versionInfo" in {
6767 When("Calculating version infos with an added app")
6868 val updated = GroupVersioningUtil.updateVersionInfoForChangedApps(Timestamp(10), emptyGroup, nestedApp)
6969 Then("The timestamp of the app and groups are updated appropriately")
7070 def update(maybeApp: Option[AppDefinition]): AppDefinition =
7171 maybeApp.map(_.copy(versionInfo = VersionInfo.forNewConfig(Timestamp(10)))).get
Show All 34 Lines