Minor docker related changes

[Gornoka]

Rcon change:

• Disable Rcon by default
• mostly so that new users don't accidentally have open rcons with default passwords

Updates in compose in readme:

• simplify Syntax for port forwarding
• change restart behaviour to "unless-stopped" -> this way your server does not restart after a system reboot if you previously stopped it
• explicitly disable rcon start

[Plarpoon]

I personally do not agree that RCON should be disabled by default as this repo offers ready to instantly import and use docker-compose files and if you follow the examples a password will always be defined, so I see why you would like to change that but I think it's unnecessary.

On the other hand I actually fully agree on changing the default restart policy to "unless-stopped", but I already had a conversation with @jammsen about it and said he was not interested in changing it. If he does change his mind on this tho I will fully support this change.

[Gornoka]

I personally do not agree that RCON should be disabled by default as this repo offers ready to instantly import and use docker-compose files and if you follow the examples a password will always be defined, so I see why you would like to change that but I think it's unnecessary.

This instant nature is why I would not enable it by default, some people just want to play and will not check the 50 lines of settings.

Maybe it would be better to add the recon container to the non recon compose file, so that you can access it from the same host using that, without exposing it to the internet.

[jammsen]

Hello @Gornoka - Thanks for your contribution and your time!

Few questions though:

Rcon change:

• Disable Rcon by default
• mostly so that new users don't accidentally have open rcons with default passwords

I dont think i understand what you mean by that. Could you please explain in more detail with an example?

Updates in compose in readme:

• simplify Syntax for port forwarding

This was requested by users to be more "explanatory" what does what do and the short syntax sometimes just doesnt help novices. I agree with that and thats why i changed it for the benefit of all in the community. But you are free to change it on your end. Or do you have a solid reason for that beeing only shortend?

• change restart behaviour to "unless-stopped" -> this way your server does not restart after a system reboot if you previously stopped it

@Plarpoon Wanted this already too, here is what i said: "In a server environment where everything is head-less i want to have the most and higehst uptime possible." In a way people build restart crons around this container to deal with the memory leaks, so i think always is fitting, because its just the docker-way to have a high uptime no matter what. Why would you want to change this? Please more detail.

• explicitly disable rcon start

Backup function was like requested over 25 times on various channels, so i implemented a solution. Why would like to have the standard, in the examples, to be off instead of on?

AND to remove network_mode: bridge is already planed and executed, its just not pushed yet. Its the default set, yeah i know, i just thought it would be more descriptive in a readable way.

[Gornoka]

Hello @Gornoka - Thanks for your contribution and your time!

Few questions though:

Rcon change:

• Disable Rcon by default
• mostly so that new users don't accidentally have open rcons with default passwords

I dont think i understand what you mean by that. Could you please explain in more detail with an example?

The current container default is that Rcon is enabled with a default Password ( the env var is set to that). Since RCON is not needed for any base feature I am ware of I would turn it off instead so that noone can mess with it.

Updates in compose in readme:

• simplify Syntax for port forwarding

This was requested by users to be more "explanatory" what does what do and the short syntax sometimes just doesnt help novices. I agree with that and thats why i changed it for the benefit of all in the community. But you are free to change it on your end. Or do you have a solid reason for that beeing only shortend?

To me going with the extended syntax implies that you need something very specific.

This application does not need anything special, just the most basic port forwarding docker has to offer, so I think the simple syntax would be more appropriate here.

• change restart behaviour to "unless-stopped" -> this way your server does not restart after a system reboot if you previously stopped it

@Plarpoon Wanted this already too, here is what i said: "In a server environment where everything is head-less i want to have the most and higehst uptime possible." In a way people build restart crons around this container to deal with the memory leaks, so i think always is fitting, because its just the docker-way to have a high uptime no matter what. Why would you want to change this? Please more detail.

In my Opinion the always restart behaviour is overly aggresive, If you intentionally stop a service or compose stack I would assume that you want to have it stopped until you start it again, and not until the next time the docker deamon restarts. Unless stopped behaves exactly like that. When your service was running before the system restarted or crashed it will be started again, if it was stopped it will stay stopped until you restart it again.

• explicitly disable rcon start

Backup function was like requested over 25 times on various channels, so i implemented a solution. Why would like to have the standard, in the examples, to be off instead of on?

If the backup feature requires RCON then it makes sense to enable RCON by default, I would still disable the port forwarding for it in the example docker-compose to prevent external access unless it is explicitly requested.

[jammsen]

Hello @Gornoka - Thanks for your contribution and your time! Few questions though:

Rcon change:

• Disable Rcon by default
• mostly so that new users don't accidentally have open rcons with default passwords

I dont think i understand what you mean by that. Could you please explain in more detail with an example?

The current container default is that Rcon is enabled with a default Password ( the env var is set to that). Since RCON is not needed for any base feature I am ware of I would turn it off instead so that noone can mess with it.

Ahhhh i see, you mean you would set it by deafult to false if i understand you correctly. It doesnt have to be on, if you simply dont use it, but you have the option to turn it on. That we acutally could do in the Dockerfile and in the Docker-Compose Non-RCON version, This could make sense yes.

Updates in compose in readme:

• simplify Syntax for port forwarding

This was requested by users to be more "explanatory" what does what do and the short syntax sometimes just doesnt help novices. I agree with that and thats why i changed it for the benefit of all in the community. But you are free to change it on your end. Or do you have a solid reason for that beeing only shortend?

To me going with the extended syntax implies that you need something very specific.

Yes, indeed. Thats me beeing nice and cater to new-people to Docker, because most of them simply dont know, when you only do "1234:1234" that this only defaults to tcp and thats by default. Yes, tts for sure a longer version, BUT its to undertand what is papening and to be more readable (See Solid Principles). If its 3 rows for each more for ports and people beeing happy that they understand whats actually happening, thats the way im going every time.

This application does not need anything special, just the most basic port forwarding docker has to offer, so I think the simple syntax would be more appropriate here.

• change restart behaviour to "unless-stopped" -> this way your server does not restart after a system reboot if you previously stopped it

@Plarpoon Wanted this already too, here is what i said: "In a server environment where everything is head-less i want to have the most and higehst uptime possible." In a way people build restart crons around this container to deal with the memory leaks, so i think always is fitting, because its just the docker-way to have a high uptime no matter what. Why would you want to change this? Please more detail.

In my Opinion the always restart behaviour is overly aggresive, If you intentionally stop a service or compose stack I would assume that you want to have it stopped until you start it again, and not until the next time the docker deamon restarts. Unless stopped behaves exactly like that. When your service was running before the system restarted or crashed it will be started again, if it was stopped it will stay stopped until you restart it again.

Thanks for sharing you point of view, i respect that, just liked i did with @Plarpoon regarding this. I know the differences between both very well, but maybe i was to hung up on that .... have to sleep a night above that, pretty sure if i dont change my opinion over night, I guess would could change it to the other value, dont want to be a harda** about something like this.

• explicitly disable rcon start

Backup function was like requested over 25 times on various channels, so i implemented a solution. Why would like to have the standard, in the examples, to be off instead of on?

If the backup feature requires RCON then it makes sense to enable RCON by default, I would still disable the port forwarding for it in the example docker-compose to prevent external access unless it is explicitly requested.

OMG im so sorry, i totally missread that, sorry, thats on me forget what i wrote please.

But why would you in this case add restart no to a container that only start to run a command and exists right away? AFAIK this container doesnt "host" anything, its just a pipeline tooling to run something and exit right away again. The profiles definition already accounts for the purpose that it just starts with the server, but you have to run it manually, just like planned.

[Gornoka]

on another Note, since now any readme change creates issues with the docker files it may make sense to split these out into separate files instead of having them as part of the readme.

( Not scope of this pr tho)

[Gornoka]

Updates in compose in readme:

• simplify Syntax for port forwarding

This was requested by users to be more "explanatory" what does what do and the short syntax sometimes just doesnt help novices. I agree with that and thats why i changed it for the benefit of all in the community. But you are free to change it on your end. Or do you have a solid reason for that beeing only shortend?

To me going with the extended syntax implies that you need something very specific.

Yes, indeed. Thats me beeing nice and cater to new-people to Docker, because most of them simply dont know, when you only do "1234:1234" that this only defaults to tcp and thats by default. Yes, tts for sure a longer version, BUT its to undertand what is papening and to be more readable (See Solid Principles). If its 3 rows for each more for ports and people beeing happy that they understand whats actually happening, thats the way im going every time.

As I See it this is the only open point in the PR ( unless-stopped appears to have already been merged to master)

My interpretation of SOLID is, that we should rely on the abstraction provided by docker networking and not explicitly specify more of the behavior than is strictly necessary.

[jammsen]

on another Note, since now any readme change creates issues with the docker files it may make sense to split these out into separate files instead of having them as part of the readme.

( Not scope of this pr tho)

I wanted to have only 1 file to manage, so i got rid of the docker-compose.yml and i didnt wanted to add 3 more, i like fat readmes 😎

Rcon change:

• Disable Rcon by default

i did that, on the Dockerfile and the Standalone examples, the others will stay that way ✅

• mostly so that new users don't accidentally have open rcons with default passwords Updates in compose in readme:

• simplify Syntax for port forwarding

The syntax is target to new-users, its simpler this way ✅

• change restart behaviour to "unless-stopped" -> this way your server does not restart after a system reboot if you previously stopped it

I did change this, i agree with you and also it was 2 vs 1 😝 ✅

• explicitly disable rcon start

Same as the answer above.

So basically the PR is done, but sorry that you didnt receive the credit for it 😢

[jammsen]

on another Note, since now any readme change creates issues with the docker files it may make sense to split these out into separate files instead of having them as part of the readme.

( Not scope of this pr tho)

Should be less now, we reduces from 3 examples to 1