New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added Strict mode support #580
Conversation
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.
Mostly looks good, just some small questions.
|
||
{{#service.secret_name}} | ||
"DCOS_SERVICE_ACCOUNT_CREDENTIAL": { "secret": "serviceCredential" }, | ||
"MESOS_MODULES": "{\"libraries\": [{\"file\": \"libdcos_security.so\", \"modules\": [{\"name\": \"com_mesosphere_dcos_ClassicRPCAuthenticatee\"}]}]}", |
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.
This should be unnecessary with the bundle that provides modules.json
out of the box, right? Or is there something preventing us from using that bundle here?
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.
As per an offline conversation with @karya0, this is something that we need to include now.
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.
Okay. @karya0 , maybe this is a good place to ask: why is this necessary? I thought modules were linked on agent launch, so how is this value being used? And will schedulers running in strict mode always have to care about what seems like a Mesos implementation detail?
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.
We should address this ASAP. @mohitsoni: Can you ping @karya0 out of band please?
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.
The MESOS_MODULES
env requires an absolute path of the form file:///
and for those frameworks that untar the bundle in their $MESOS_SANDBOX
directory, we can't predict the absolute path.
For framework that untar the bundle at a known location, i.e., /opt/libmesos-bundle
, we can include a mesos_authenticatee_module.json
file in the bundle and let the app point to it:
"MESOS_MODULES": "file:///opt/libmesos-bundle/etc/mesos_authenticatee_module.json"
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.
@gabrielhartmann: It's because Mesos doesn't support it. We can try to get a patch into Mesos. Currently, Mesos just hands over the string to the 'open' call. What we need is to change this behavior to resolve the env vars before passing it to open. I'll lookup open semantics around env var resolution and write back with an update on the possibilities.
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.
Out of curiosity, what does passing file:///dev/urandom
do?
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.
@nickbp: The way Mesos stout processes the filepaths, it will first try to read the entire file contents into a buffer (8KB at a time) and then will parse it to JSON parser and will follow it by a JSON->Proto conversion. As you can guess, the reading will never finish up and at some point, you'll run out of memory+swap and the process will get killed.
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.
I assume 'the process' in this case is the agent? When it restarts will it just try to do the same thing again?
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.
It would be whatever process was trying to load the module manifest. It could be the master, agent, or the scheduler.
principal: {{FRAMEWORK_PRINCIPLE}} | ||
zookeeper: {{MESOS_ZOOKEEPER_URI}} | ||
api-port: {{PORT_API}} | ||
principal: {{FRAMEWORK_PRINCIPAL}} |
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.
Does this one need a user too?
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.
Good catch. Let me update the PR.
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.
Done!
Juts curious, what sets elastic apart w/r/t strict mode? Is this one more way that it's a special snowflake? Also, what about the cassandra FW? |
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.
I would go ahead and add ES and Cassandra strict mode changes here.
@loren Elastic needs a 1.9 cluster, and at the time of sending this PR there was a bug in 1.9-dev strict mode. And, because of that I was unable to verify my changes, and hence decided to not include elastic in this PR (until the bug gets fixed). As far as the strict-mode support changes are concerned, elastic is not a snowflake. Cassandra is on it's way, when I started working on this PR, Cassandra wasn't a thing on master. |
@gabrielhartmann @loren Added strict mode support for Cassandra. |
@loren @gabrielhartmann Added strict mode support for elastic. |
retest this please |
Summary of changes:
MESOS_API_VERSION
variable.