HomeMesosphereNo notifications. 4 unresolved issues.
  • Search
Differential D872

putGroup with nested groups now correctly handles transitive apps for v1.4
ClosedAll Users
Actions

Authored by kensipe on Jun 20 2017, 9:40 AM.

Details

Reviewers
zen-dog
meichstedt
jeschkies
timcharper
ichernetsky
Commits
rMARATHON7ed91a961374: putGroup with nested groups now correctly handles transitive apps for v1.4
JIRA Issues
JIRA MARATHON-7433 Deploying group is broken in Version 1.4.3
Summary

Same change as on master for similar named diff. The difference is testing infra completely changed between 1.4 and 1.5 and cherry picking wasn't an option.

Test Plan

run test

Diff Detail

Repository
rMARATHON marathon
Branch
ken/group-fix-1.4
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2941
Build 5588: Marathon (revised)Jenkins
Build 5587: arc lint + arc unit
Changes from before your most recent comment are hidden. Show Older Changes
jeschkies accepted this revision.Jun 20 2017, 12:02 PM

Thanks. I'd have the same comments as for D869, though.

zen-dog accepted this revision.Jun 20 2017, 4:00 PM

Be careful not to land it on the master ;)

kensipe closed this revision.Jun 20 2017, 8:33 PM

landed on release branch and didn't auto close

Revision Contents

Diff 3559

View Options

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

Show First 20 Lines • Show All 76 Lines • ▼ Show 20 Line(s)
7777 groupsById == that.groupsById &&
7878 dependencies == that.dependencies &&
7979 version == that.version
8080 case _ => false
8181 }
8282
8383 override def hashCode(): Int = Objects.hash(id, apps, pods, groupsById, dependencies, version)
8484
85 override def toString = s"Group($id, ${apps.values}, ${pods.values}, ${groupsById.values}, $dependencies, $version)"
85 override def toString = s"Group($id, ${apps.values}, ${pods.values}, ${groupsById.values}, $dependencies, $version, ${transitiveAppsById}, ${transitivePodsById})"
8686}
8787
8888object Group {
8989 type GroupKey = PathId
9090
9191 def apply(
9292 id: PathId,
9393 apps: Map[AppDefinition.AppKey, AppDefinition] = Group.defaultApps,
▲ Show 20 Lines • Show All 70 Lines • Show Last 20 Lines
View Options

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

Show First 20 Lines • Show All 109 Lines • ▼ Show 20 Line(s)
110110 val oldParent = group(head).getOrElse(Group.empty(head))
111111 val newParent = Group(
112112 id = oldParent.id,
113113 apps = oldParent.apps,
114114 pods = oldParent.pods,
115115 groupsById = oldParent.groupsById + (result.id -> result),
116116 dependencies = oldParent.dependencies,
117117 version = version,
118 transitiveAppsById = oldParent.transitiveAppsById -- oldGroup.apps.keys ++ newGroup.apps,
119 transitivePodsById = oldParent.transitivePodsById -- oldGroup.pods.keys ++ newGroup.pods)
118 transitiveAppsById = oldParent.transitiveAppsById -- oldGroup.transitiveAppsById.keys ++ newGroup.transitiveAppsById,
119 transitivePodsById = oldParent.transitivePodsById -- oldGroup.transitivePodsById.keys ++ newGroup.transitivePodsById)
120120 rebuildTree(tail, newParent)
121121 }
122122 }
123123 RootGroup.fromGroup(updateVersion(rebuildTree(newGroup.id.allParents, newGroup), version))
124124 }
125125
126126 /**
127127 * Make a group with the specified id exist in the root group.
▲ Show 20 Lines • Show All 316 Lines • Show Last 20 Lines
View Options

src/test/scala/mesosphere/marathon/state/RootGroupTest.scala

Show First 20 Lines • Show All 520 Lines • ▼ Show 20 Line(s)
521521 When("Round trip toProto -> fromProto")
522522 val protoGroup = Group.fromProto(group.toProto)
523523 val protoRootGroup = RootGroup.fromProto(rootGroup.toProto)
524524
525525 Then("The round trip is successful")
526526 protoGroup should be(group)
527527 protoRootGroup should be(rootGroup)
528528 }
529
530 it("properly propagate transitiveAppsById") {
531 Given("A non-root group with nested groups")
532 val appPath = "/domain/developers/gitlab/git".toPath
533 val app = AppDefinition(appPath)
534
535 val groupUpdate = createGroup(
536 PathId("/domain/developers"),
537 groups = Set(
538 createGroup(
539 PathId("/domain/developers/gitlab"),
540 apps = Map(appPath -> app))))
541
542 val newVersion = Timestamp.now()
543
544 When("updating")
545 val updated = RootGroup.empty.putGroup(groupUpdate, newVersion)
546
547 Then("The app path is in the transitiveApps key set ")
548 updated.transitiveAppsById.keySet should contain(appPath)
549 }
550
551 it("properly propagate transitiveAppsById2") {
552 Given("A non-root group with nested groups")
553 val appPath = "/domain/developers/gitlab/git".toPath
554 val app = AppDefinition(appPath)
555
556 val groupUpdate = createGroup(
557 PathId("/domain"),
558 groups = Set(createGroup(
559 PathId("/domain/developers"),
560 groups = Set(
561 createGroup(
562 PathId("/domain/developers/gitlab"),
563 apps = Map(appPath -> app))))))
564
565 val newVersion = Timestamp.now()
566 When("updating")
567 val updated = RootGroup.empty.putGroup(groupUpdate, newVersion)
568
569 Then("The app path is in the transitiveApps key set ")
570 updated.transitiveAppsById.keySet should contain(appPath)
571 }
572
573 it("properly propagate transitiveAppsById3") {
574 Given("A non-root group without nested groups")
575 val appPath = "/domain/developers/gitlab/git".toPath
576 val app = AppDefinition(appPath)
577
578 val groupUpdate = createGroup(
579 PathId("/domain/developers/gitlab"),
580 apps = Map(appPath -> app))
581
582 val newVersion = Timestamp.now()
583
584 When("updating")
585 val updated = RootGroup.empty.putGroup(groupUpdate, newVersion)
586
587 Then("The app path is in the transitiveApps key set ")
588 updated.transitiveAppsById.keySet should contain(appPath)
589 }
529590 }
530591}
These changes have not finished building yet and may have build failures.