erlware / relx

Sane, simple release creation for Erlang
http://erlware.github.io/relx
Apache License 2.0
697 stars 232 forks source link

don't require a cookie in the start script #708

Closed tsloughter closed 5 years ago

tsloughter commented 5 years ago

@tolbrino @uwiger

I think I finally understand what the root issue was. Is it true that you were seeing the node refuse to start because no cookie was set but that this error was from the start script itself?

This change makes it so the start script doesn't enforce the existence of a cookie and allows it to be created by Erlang on startup.

Though, I don't believe @tolbrino 's latest changes would have worked for you since the creation ran after that check in the script, so if that was working then I may be going the wrong way with this.

tolbrino commented 5 years ago

Excuse my incomplete comprehension of the extended script -.- I think your approach is good. However, from a UX perspective this leaves out the case where you don't want to ship a cookie with the node package and would want the end-user to be able to use the started node properly anyway (with all relx commands supported). Thus I'd suggest to run the ensure_dot_erlang_cookie procedure in that case.

This case usually doesn't occur in managed hosting environment where you have full control via provision tools such as ansible and others, but when you'd want an enduser to run the node with the least amount of addition setup.

tsloughter commented 5 years ago

There should be no reason you can't use all the functionality.

The warning should only show up if no node has been started, the node starting will create the .erlang.cookie file and so subsequent runs with calls to remote_console and what not will not have a warning and work fine.

The only reason it wouldn't would be if you have a vmargs that forces epmd not to start and no name and so get no cookie -- and then set the cookie manually and start epmd/dist erl manually too in the running node's code. In which case even with the pre-created .erlang.cookie it wouldn't work since the cookie would be different.

tolbrino commented 5 years ago

@tsloughter You are correct. My test setup was incorrectly configured.

Could you also change the win32 variant accordingly?

tsloughter commented 5 years ago

@tolbrino maybe. I assume it should be easy enough as it is just removing code, but I won't be able to test it.

If I add a commit to do it in the windows script can you test it?

tolbrino commented 5 years ago

@tsloughter Sure, I'll test it and will propose additional changes if needed.

tsloughter commented 5 years ago

So as far as I can tell the windows script does not do the check for a cookie and fail if it doesn't exist. Is that not the case?

I removed the ensure dot erlang cookie function but didn't see anything else to change except to add the feature to set the cookie variable based on .erlang.cookie if it exists and -setcookie doesn't.

So nodetool and such won't work on Windows if cookie isn't set in vm.args, and wouldn't have even with your ensure_dot_erlang_cookie being called as far as I can tell (since they use an explicit -setcookie and the variable would be empty.

No idea how this:

:: Extract cookie from vm.args
@for /f "usebackq tokens=1-2" %%I in ('findstr /b \-setcookie "%vm_args%"') do @(
  set cookie=%%J
)

works :). Could you add functionality to have it look for the cookie file and use that if it exists?

tolbrino commented 5 years ago

@tsloughter I will make a PR to your working branch tomorrow. Then you can use it in this PR here. It's already late on this side of the globe :-)

tolbrino commented 5 years ago

Alternatively you can merge this PR and I create a new one.

tsloughter commented 5 years ago

Great, thanks :).

I'll be waiting for your patch to make a new relx release, so in no hurry to merge this PR.