unraid / webgui

Unraid Web UI
159 stars 70 forks source link

Docker Compose - Network info on GUI displays incorrectly #1569

Closed mtongnz closed 3 months ago

mtongnz commented 10 months ago

The unRaid GUI has changed how it handles displaying network info for docker containers. This means that containers created outside of the GUI (like when using the Docker Compose Manager plugin) display a network ID instead of the actual network name. See this post: https://forums.unraid.net/topic/114415-plugin-docker-compose-manager/?do=findComment&comment=1340206

I have started work on a solution: https://github.com/mtongnz/unRaid-webgui/tree/network-display-improvements This will display all network names and the associated IP for each.

I'd like thoughts and suggestions before creating a PR.

dcflachs commented 10 months ago

I am not sure i like the idea of using the net.unraid.docker.managed label to special case the display of networks in dockerman. It would be preferable to have a more general solution to fix displaying network names. Especially since it may encourage people to use net.unraid.docker.managed=composeman for any container that has multiple networks, whether or not it was created by the compose.manager plugin. Any container can be connected to multiple networks, and those launched by other means, portainer, pterodactyl, command line, etc. should not use the net.unraid.docker.managed=composeman.

mtongnz commented 10 months ago

That makes a lot of sense and I agree. I hadn't thought about that and was looking for the easiest solution to my issue. Do you have a recommended solution? Perhaps a label called net.unraid.docker.netdisplay. It could then have multiple options available such as default, multiple, none...

dcflachs commented 10 months ago

My suggestion would be no labels. I think the existing Network and Port Mappings display code in dockerman needs to be re-written to handle more complex network setups. I only had time to skim the existing code and your changes but it looks like you are on the right track. It seems that "NetworkSettings" is a better place to pull the network names from than what is currently used. It also looks like the "Ports" listing under "NetworkSettings" might be the first choice for pulling the port/IP pairs. Ports and IP pairs though is where things get complicated and likely requires some careful thought.

mtongnz commented 10 months ago

I've pushed a new commit which addresses those issues. It also removes the update links that don't work with non docker man containers. I do feel that there may be some edge cases that the ports are not shown correctly but I don't know enough about docker networking to test it. For example, does docker let you expose a port on a specific IP and not on others? If it does, my UI tweaks won't indicate this.

mtongnz commented 10 months ago

I've gone through and tidied commits into a new branch (updated above) as it was getting messy and I kinda started from scratch.

Changes in DockerClient.php:

Changes in DockerContainers.php:

Any other suggestions or thoughts before I submit a PR?

mtongnz commented 3 months ago

This PR was merged so issue is fixed.