Skip to content
This repository has been archived by the owner on Oct 23, 2024. It is now read-only.

Commit

Permalink
Fixes #4842 by also writing deprecated pods to the proto.
Browse files Browse the repository at this point in the history
Summary: Added a test case which shows the round trip works.

Test Plan:
#1 sbt test
#2 start marathon with --internal_store_backend legacy_zk, create pods, restart marathon and get pods and the root group.

Reviewers: unterstein, jasongilanfarr

Reviewed By: unterstein, jasongilanfarr

Subscribers: jenkins, marathon-team

Differential Revision: https://phabricator.mesosphere.com/D342
  • Loading branch information
aquamatthias authored and unterstein committed Dec 15, 2016
1 parent 3c80485 commit 6578976
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 18 deletions.
1 change: 1 addition & 0 deletions src/main/scala/mesosphere/marathon/state/Group.scala
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class Group(
.setId(id.toString)
.setVersion(version.toString)
.addAllDeprecatedApps(apps.map { case (_, app) => app.toProto })
.addAllDeprecatedPods(pods.map { case (_, pod) => pod.toProto })
.addAllGroups(groupsById.map { case (_, group) => group.toProto })
.addAllDependencies(dependencies.map(_.toString))
.build()
Expand Down
19 changes: 1 addition & 18 deletions src/main/scala/mesosphere/marathon/state/RootGroup.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ import com.wix.accord.dsl._
import mesosphere.marathon.Protos.GroupDefinition
import mesosphere.marathon.api.v2.Validation._
import mesosphere.marathon.core.externalvolume.ExternalVolumes
import mesosphere.marathon.state.PathId._
import mesosphere.marathon.core.pod.PodDefinition
import mesosphere.marathon.stream._

import org.jgrapht.DirectedGraph
import org.jgrapht.alg.CycleDetector
Expand Down Expand Up @@ -426,22 +424,7 @@ object RootGroup {

def empty: RootGroup = RootGroup(version = Timestamp(0))

def fromProto(msg: GroupDefinition): RootGroup = {
require(msg.getId.toPath.isRoot, "`RootGroup.fromProto` requires that the `id` be root.")
RootGroup(
apps = msg.getDeprecatedAppsList.map { proto =>
val app = AppDefinition.fromProto(proto)
app.id -> app
}(collection.breakOut),
pods = msg.getDeprecatedPodsList.map { proto =>
val pod = PodDefinition.fromProto(proto)
pod.id -> pod
}(collection.breakOut),
groupsById = msg.getGroupsList.map(fromProto).map(group => group.id -> group)(collection.breakOut),
dependencies = msg.getDependenciesList.map(PathId.apply)(collection.breakOut),
version = Timestamp(msg.getVersion)
)
}
def fromProto(msg: GroupDefinition): RootGroup = RootGroup.fromGroup(Group.fromProto(msg))

def fromGroup(group: Group): RootGroup = {
require(group.id.isRoot)
Expand Down
28 changes: 28 additions & 0 deletions src/test/scala/mesosphere/marathon/state/RootGroupTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -486,5 +486,33 @@ class RootGroupTest extends FunSpec with GivenWhenThen with Matchers with GroupC
Then("validation is successful")
validResult.isSuccess should be(true)
}

it("can be written to proto and read back again") {
Given("A group with subgroups, apps and pods")
val app = AppDefinition(PathId("/app"))
val pod = PodDefinition(PathId("/pod"))
val subGroup = Group(PathId("/group"), transitiveAppsById = Map.empty, transitivePodsById = Map.empty)
val group = Group(
id = PathId.empty,
apps = Map(app.id -> app),
pods = Map(pod.id -> pod),
groupsById = Map(subGroup.id -> subGroup),
transitiveAppsById = Map(app.id -> app),
transitivePodsById = Map(pod.id -> pod)
)
val rootGroup = RootGroup(
apps = Map(app.id -> app),
pods = Map(pod.id -> pod),
groupsById = Map(subGroup.id -> subGroup)
)

When("Round trip toProto -> fromProto")
val protoGroup = Group.fromProto(group.toProto)
val protoRootGroup = RootGroup.fromProto(rootGroup.toProto)

Then("The round trip is successful")
protoGroup should be(group)
protoRootGroup should be(rootGroup)
}
}
}

0 comments on commit 6578976

Please sign in to comment.