Open BrickXu opened 8 years ago
LGTM, really great catch! I know @gabrielhartmann tested this before, but I'm not sure if he explicitly tested the ACL's. If no objections from @gabrielhartmann or @kensipe, I will merge.
@elingg Someone needs to test on DCOS.
Sounds good. Let's test before merging. Do you have some documentation of your steps to test framework auth on DCOS, @gabrielhartmann?
@elingg: I don't have documentation for testing on DCOS. Mesos has to be deployed with --credentials or its equivalent. The Wiki has some info on deploying a custom cluster, or maybe just restarting master with the right flags would be easier.
@BrickXu: I'm actually not sure this is a totally harmless change. We may not want to be setting the principal always. We may want to only set it if it exists.
@gabrielhartmann principal would be a "" string if it does not exist in mesos-site.xml, so this PR does not change the default behavior. IMO, the principal should be the same as credential's principal always.
Thanks @gabrielhartmann. One of us can try this out, and it would be great to create some docs for how to do framework auth (cc @mgummelt who will be testing framework auth for spark)
@elingg we tested spark on mesos with auth, the "auth" function work OK. But unfourtunatly, 1.5.0 and 1.5.1 could not support run task with a specify role, but work well with role "*".
spark.mesos.principal=spark
spark.mesos.secret=XXXXXXXXXXXX
spark.mesos.role=spark
Here is the JIRA issue https://issues.apache.org/jira/browse/SPARK-10749, PR https://github.com/apache/spark/pull/8872
Maybe you need patch to mesosphere's distribution if you really want to support multiple tenant in the spark.
I'm not quite sure that use roles to control resource usage limit for each node in the cluster is in your road map.
@BrickXu: There is a difference between setting something to the empty string and not setting it at all. Regardless of whether the internal implementation of protobuf generation differentiates between the two, we should indicate our intent clearly. When we do not have a principal we should not set it. When we do have a principal we should set it to a non-empty string. There, actually isn't a case in which the default "" (empty string), should be used.
FYI, resource reservation (and therefore the use of roles) is on our roadmap.
@gabrielhartmann :+1: I will rebase the commit later.
@BrickXu: Sorry, I don't understand your comment. Are you going to make a change so the principal is only set when it is specified?
@gabrielhartmann yes.
Great. Thanks. On Thu, Nov 12, 2015 at 7:06 PM BrickXu notifications@github.com wrote:
@gabrielhartmann https://github.com/gabrielhartmann yes.
— Reply to this email directly or view it on GitHub https://github.com/mesosphere/hdfs/pull/229#issuecomment-156309989.
@gabrielhartmann sorry to rebase this commit so late.
Please note that this repository is now deprecated. See this issue for a more detailed explanation.
Hi guys,
This PR depends on #175, to finish some works when mesos cluster enable ACL rules.
Our mesos cluster set ACL to control the framework register and run tasks, the rules always depends on roles and principal.
If we enable framework authentication, hdfs scheduler only set credential for the FrameworkInfo, but not set principal value, it does not work with ACL like this:
because principal always set to "" by default.