-
Notifications
You must be signed in to change notification settings - Fork 68
accept offers for default role * in addition to the specified mesosRole #124
accept offers for default role * in addition to the specified mesosRole #124
Conversation
…role but other role specified
Thanks for submitting @flosell, feedback to follow shortly. |
@@ -172,6 +180,83 @@ public static CassandraNodeTask getTaskForNode(@NotNull final CassandraNode cass | |||
return builder; | |||
} | |||
|
|||
@NotNull | |||
public static Function<Resource, TreeSet<Long>> resourceToPortSet() { | |||
return new Function<Resource, TreeSet<Long>>() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make a private static final inner class of this function with a singleton instance returned by this method.
See https://github.com/mesosphere/cassandra-mesos/blob/master/cassandra-mesos-model/src/main/java/io/mesosphere/mesos/util/CassandraFrameworkProtosUtils.java#L176 for an example of how this had been done previously.
I was able to come up with a scenario that fails when ran against mesos 0.23.0. Details below. Start a slave with only a subset of the ports dedicated to the configured role ( It looks like the port is being claimed for the incorrect role Relevant Scheduler Logs
Mesos Master
Mesos Slave
|
Thanks for the quick feedback! |
Sounds good, I'll review when you're ready. |
… role, the rest to the * role
The last commit should fix the issue you mentioned. It was previously just trying to find a role that provides all ports and falls back to * in the other case. Can you have a second look? |
Thanks for the updates @flosell and adding another unit tests for the scenario. |
accept offers for default role * in addition to the specified mesosRole
Currently, when specifying a role using
CASSANDRA_FRAMEWORK_MESOS_ROLE
offers for the default*
role are ignored.Marathon and Chronos (since mesos/chronos#462) accept
*
and the specified mesosRole in this case.This makes it possible to share common resources but keep some resources exclusively. E.g. to run Marathon and cassandra-mesos, slaves could offer mem, cpu and disk with role
*
but the cassandra ports only for rolecassandra
.This pull request adds this feature. What do you think?