Skip to content
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

Enable game server ip address allocation for macvlan driver. #49

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Freddo3000
Copy link

This is pterodactyl/wings#203 cherrypicked onto current main branch.

Issue:
For macvlan driver default docker config assign first available ip. Game servers get ip assigned in order of container creation.

Solution proposal:
Wings support only one docker network for each game server. To assign correct IP default allocation could be used. Allocations > doesn't limit IPs at all, admin could set desired game server container ip and port by default allocation.

Tagging @madpeteguy for notice.

Issue:
For macvlan driver default docker config assign first available ip.
Game servers get ip assigned in order of container creation.

Solution proposal:
Wings support only one docker network for each game server.
To assign correct IP default allocation could be used.
Allocations doesn't limit IPs at all, admin could set desired game server container ip and port by default allocation.
@QuintenQVD0
Copy link
Contributor

This seems fine to me. I don't think it would work yet as our config loading order still first loads the default struct

@madpeteguy
Copy link

I can test if it will work with pelican.
This would be a good reason for me to switch from pterodactyl.
For my setup this feature is a must, and I'm tired of building own images. Using official ones is more convenient for my homelab cd.

@Freddo3000
Copy link
Author

I can test if it will work with pelican. This would be a good reason for me to switch from pterodactyl. For my setup this feature is a must, and I'm tired of building own images. Using official ones is more convenient for my homelab cd.

I've been running it for more than a month now without any noteworthy issue.

@QuintenQVD0
Copy link
Contributor

I can test if it will work with pelican. This would be a good reason for me to switch from pterodactyl. For my setup this feature is a must, and I'm tired of building own images. Using official ones is more convenient for my homelab cd.

I've been running it for more than a month now without any noteworthy issue.

The question is does it still work for people that does not use macvlan.

@madpeteguy
Copy link

madpeteguy commented Dec 4, 2024

I can test if it will work with pelican. This would be a good reason for me to switch from pterodactyl. For my setup this feature is a must, and I'm tired of building own images. Using official ones is more convenient for my homelab cd.

I've been running it for more than a month now without any noteworthy issue.

The question is does it still work for people that does not use macvlan.

In short: the proposed change applies only to wings that set docker.network.driver == “macvlan” in the configuration, for standard bridge and host allocations work as before.

The current implementation does not work for an interface of type macvlan, because server containers that connect to this type of network receive the first free IP address from the network pool, so there is no way to determine on which IP the game server will be available (unless we have only one server on this network).

My proposed fix makes the containers assigned to macvlan receive an IP address from the Default Allocation in the server configuration.
If the other allocations are assigned other IP addresses then those IP addresses will be ignored.

It is worth remembering that the use of macvlan has its security downsides, I can describe the ones I know of if necessary.
In addition, port allocations in the case of macvlan are only for informational purposes in the panel, in the network configuration they have no meaning.

Edit:
I am not a GO programmer, but i think this line may not be null safe: var serverNetConfig = config.Get().Docker.Network.
It's worth checking whether there will be a problem in the absence of a docker network configuration.

@QuintenQVD0
Copy link
Contributor

I can test if it will work with pelican. This would be a good reason for me to switch from pterodactyl. For my setup this feature is a must, and I'm tired of building own images. Using official ones is more convenient for my homelab cd.

I've been running it for more than a month now without any noteworthy issue.

The question is does it still work for people that does not use macvlan.

In short: the proposed change applies only to wings that set docker.network.driver == “macvlan” in the configuration, for standard bridge and host allocations work as before.

The current implementation does not work for an interface of type macvlan, because server containers that connect to this type of network receive the first free IP address from the network pool, so there is no way to determine on which IP the game server will be available (unless we have only one server on this network).

My proposed fix makes the containers assigned to macvlan receive an IP address from the Default Allocation in the server configuration. If the other allocations are assigned other IP addresses then those IP addresses will be ignored.

It is worth remembering that the use of macvlan has its security downsides, I can describe the ones I know of if necessary. In addition, port allocations in the case of macvlan are only for informational purposes in the panel, in the network configuration they have no meaning.

Edit: I am not a GO programmer, but i think this line may not be null safe: var serverNetConfig = config.Get().Docker.Network. It's worth checking whether there will be a problem in the absence of a docker network configuration.

The config logic should fil in all fields that are empty and can not be empty

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants