vm.args check for name/sname parameter

[juhlig]

The current version of extended_bin checks if there is a name or sname parameter in vm.args and refuses to start if there is none. However, it is allowed that the vm.args file (more abstract, any -args_file that is given to erl/erlexec etc) itself may contain -args_file parameters (see http://erlang.org/doc/man/erl.html), which may contain the name/sname parameters.

This change will recursively scan the files mentioned in -args_file parameters in vm.args as well as -args_file parameters in the mentioned files etcetc, and return the first occurence of a name/sname parameter.

Two points are worth mentioning, though:

• The name/sname check works only with absolute paths in the args_file parameters. Relative paths are probably a bad idea there, anyway, since it would make any setup rather fragile.
• There is no check for circular dependencies. There was none before, and this change does not add any.

[juhlig]

Note: the check for name/sname is actually incomplete (and this PR will not change this) in that it is possible to have multiple and even mixed name/sname parameters, even in the same args_file. erl/erlexec will not complain in this case, but:

1. if exactly 1 sname and name parameter are given, sname takes precedence over name
2. if multiple sname parameters are given, the node will assume the name "nonode@nohost"
3. if no sname but multiple name parameters are given, the node also assumes the name "nonode@nohost"

This was tested on Linux, I don't know if other OSs exhibit different behaviors, but I guess not. Maybe the name/sname check could be refined to check for multiple/mixed name/sname parameters, for completeness' sake.

[lrascao]

thanks! are you able to add a test for the args_file case to prevent future regressions?

[juhlig]

what kind of test do you mean exactly? a test for circular dependencies?

[lrascao]

nah, one that takes name/sname from args_file instead of vm.args (if i understood correctly)

[juhlig]

hm, so you mean a test for the case outlined in my "Note" comment? multiple sname/name/mixed parameters?

i think it is not feasible/possible to solve this case automatically in the sense of "if its in vm.args and another args_file, take the one from the args file and forget the one from vm.args". after all, vm.args is an args_file, the "base" args_file so to speak, which is passed to erlexec, and which may or may not have args_file parameters in it (somewhat include-ish), and those may again have even more of those etcetc, and any of those could have name/sname parameters in it. while it would be possible to modify the vm.args file (like the replace os vars featue does), I believe we should not try to modify the referenced args_files in any way. they eventually reside in a directory where the user executing the extended_bin has only read access, so the only way possible would be to copy the file to somewhere else (/tmp maybe), but this would mean we would also need to modify the args_file parameters in vm.args to point to the copied file.

what would be possible is to implement a check if there are any multiple sname/name/mixed parameters, and refuse to start if that is the case. along the way, i could probably also include a check for circular dependencies.

[juhlig]

Anyway. I will be on vacation next week, so I will get back to this when I return :)

[lrascao]

cool, enjoy your vacations!

[juhlig]

@lrascao
I'm back from vacation :) As promised, I extended the args_file checks a lot, and also added some tests. Please review my commits so far.
The Travis CI checks are failing for OTP 18.3 for some reason which seems totally unrelated to any of my changes, so please look into it and re-run this check.

[lrascao]

awesome, thanks!

[juhlig]

you're welcome :)

[lrascao]

I've just come across something which the changes in PR are preventing people (namely me) from doing, which is including other .args file from one supplied vm.args through the -args_file parameter with relative paths. A relative path for -args_file is supported and it works, you just need to consider the root of the release dir for it to work.

[lrascao]

@juhlig i know it's been a long time, but i'd like your opinion on this, should we relax the checks or keep them as optional (enabled by default)
cc @tsloughter @ferd

[ferd]

In such a case I guess you could expand the path by checking "$(pwd)/$relative" if we detect a relative path and see how that works. That might not be safe for all invocations, and arguably rather than being the cwd it should be the root dir or something?

[lrascao]

vm.args inclusion has the same behaviour as sys.config, so imo we shouldn't be blocking it (eg.):

[
    %% SASL config

  "releases/0.1.0/config/generated/user_defined.config"
].

i'd expect (and it works like that) vm.args to behave similarly:

-name a_server

-setcookie a_server_cookie

-args_file releases/0.1.0/vm.generated.args

[lrascao]

note the generated names in there, the use case here is using cuttlefish to extend the current config/vm.args parameters

[ferd]

Yeah that's fair. As long as it works we could expand things to allow it.

[juhlig]

@juhlig i know it's been a long time

2017, yeah, so understatement ;)

but i'd like your opinion on this, should we relax the checks or keep them as optional (enabled by default)

As long as it works, it should be ok. It may be a bit difficult to follow the relative paths around, and it may be a bit more fragile when it comes to deployment, that's all.

[lrascao]

I've got a PR in the works where i'm going with removing these checks, -args_file of relative paths is fully supported by the VM so there's no reason relx should be blocking it