webgui icon indicating copy to clipboard operation
webgui copied to clipboard

Network info display improvements

Open mtongnz opened this issue 1 year ago • 24 comments

This PR aims to improve how networks are displayed for docker containers.

  • fix where network ID instead of a name is displayed (generally for docker compose created containers).
  • display multiple networks and the associated IP for each

mtongnz avatar Feb 10 '24 08:02 mtongnz

Addresses issue #1569

mtongnz avatar Feb 11 '24 00:02 mtongnz

Do you have a screenshot when multiple networks are displayed? (wondering how that looks like)

bergware avatar Feb 14 '24 21:02 bergware

Capture

mtongnz avatar Feb 16 '24 19:02 mtongnz

thanks, looks good.

bergware avatar Feb 21 '24 08:02 bergware

is it possible to test the fix before the release? I'm waiting for this very long... If I just have to swap a file, that's great. Thx

kangaroo72 avatar Mar 03 '24 13:03 kangaroo72

is it possible to test the fix before the release? I'm waiting for this very long... If I just have to swap a file, that's great. Thx

Yes. Edit the webui files on your server as per the commits above.

Easiest way to do this is to use vscode with the sftp plugin as per the unraid GUI Git recommendations. Or you can SSH/terminal into your unraid and use nano - but this is more prone to typos.

I'm away from my PC this week so can't give much more detail. Let me know how you get on.

mtongnz avatar Mar 04 '24 22:03 mtongnz

I fixed the problem by swapping the DockerClient.php. Thanks a lot for this. Is it also mandatory to swap the DockerContainers.php? Thx

kangaroo72 avatar Mar 05 '24 08:03 kangaroo72

I fixed the problem by swapping the DockerClient.php. Thanks a lot for this. Is it also mandatory to swap the DockerContainers.php? Thx

Yes. You need to swap both

mtongnz avatar Mar 05 '24 22:03 mtongnz

It looks like docker ports that aren't mapped are showing that they are: image e.g. these all use 7001 internally but don't map the port

FunkeCoder23 avatar Mar 11 '24 02:03 FunkeCoder23

This also removes the ability to edit existing dockers. This seems like a big issue

FunkeCoder23 avatar Mar 11 '24 03:03 FunkeCoder23

Implemented and tested changes. Cheers

mtongnz avatar Apr 06 '24 00:04 mtongnz

I've been using with my tweaks for a while now, working great

FunkeCoder23 avatar Apr 06 '24 00:04 FunkeCoder23

Just noticed that non-bridge networks didn't show external ports. Fixed in latest commit

mtongnz avatar Apr 06 '24 03:04 mtongnz

@FunkeCoder23 when you get a chance, could you review the last commit I made to this. I found the NAT detection logic was flawed (based on the old code) and so it wasn't showing external port mappings in many cases. Cheers

mtongnz avatar Apr 20 '24 00:04 mtongnz

@FunkeCoder23 when you get a chance, could you review the last commit I made to this. I found the NAT detection logic was flawed (based on the old code) and so it wasn't showing external port mappings in many cases. Cheers

lgtm, I'd just fix up the formatting differences

FunkeCoder23 avatar May 22 '24 01:05 FunkeCoder23

Capture

Are each of these deployed with docker-compose? ~Looking at the change, this PR picks up the network list only for networks in the HostConfig but not for containers attached to a network with Post args.~ Actually, after looking again, I'm not sure why it's not showing both networks... I can see my containers list their "primary" network (configured in the template) but the NetworkSettings shows the additional (in this case, traefik) network:

{
    "Id": "1234...",
    ...
    "HostConfig": { "NetworkMode": "lsio_network" },
    "NetworkSettings": {
        "Networks": {
            "lsio_network": {...},
            "traefik": {...},
        }
    },
    ...
}

image

phillipjf avatar Jun 06 '24 19:06 phillipjf

@FunkeCoder23 when you get a chance, could you review the last commit I made to this. I found the NAT detection logic was flawed (based on the old code) and so it wasn't showing external port mappings in many cases. Cheers

lgtm, I'd just fix up the formatting differences

Sorry, stepped away from this and missed your comment.

What formatting differences are you meaning? All I can see is DockerClient.php ln 51-56 where spaces changed to tabs to match the rest of the file. The rest appears correctly formatted.

mtongnz avatar Jun 11 '24 04:06 mtongnz

@FunkeCoder23 when you get a chance, could you review the last commit I made to this. I found the NAT detection logic was flawed (based on the old code) and so it wasn't showing external port mappings in many cases. Cheers

lgtm, I'd just fix up the formatting differences

Sorry, stepped away from this and missed your comment.

What formatting differences are you meaning? All I can see is DockerClient.php ln 51-56 where spaces changed to tabs to match the rest of the file. The rest appears correctly formatted.

Ah, I thought the whitespace was spaces for the original file. If it's tabs, then the formatting is good.

FunkeCoder23 avatar Jun 11 '24 10:06 FunkeCoder23

@ljm42 & @mtongnz I saw that the header row is unchanged: grafik

What about: grafik

ich777 avatar Jul 16 '24 18:07 ich777

@mtongnz after thinking about that PR a bit, this will most likely confuse new/inexperienced users and they would possible think that they have to connect to IP address 172.18.0.14 instead of 10.0.0.1 (HostIP) in my case because the Host IP is now missing when the container is on a bridge or custom bridge network.

grafik

Where it was obvious before: grafik

Could you maybe display the Host IP too so that both are displayed for a bridge or custom bridge network? What are your thoughts on that?

ich777 avatar Jul 17 '24 17:07 ich777

@ich777 thanks heaps for the suggestions. I've taken them and made a few tweaks in my latest commit:

  • Separated network & IP into separate columns
  • Removed the "app to host" from Port Mappings heading (it's a little misleading now)
  • Added "advanced" class to non-exposed ports so they're hidden in basic view.
  • Added "(internal)" text to non-exposed ports when they're shown in advanced view. This one I'm not too sure about as they're already hidden in basic mode.

I'm interested to hear your thoughts.

Basic view: image

Advanced view: image

mtongnz avatar Jul 20 '24 00:07 mtongnz

@mtongnz phil1c from the unraid forums here. I implemented the changes to DockerContainers.page, DockerContainers.php, and DockerClient.php manually and have a few issues and a couple suggestions. I'll start with issues:

  1. My dockers on a macvlan subinterface (bond0.43) correctly show the assigned external IP address and port but the ports have the "(internal)" flag. image
  2. For the container referenced in Issue 1 above (on macvlan network), the WebUI link that is generated is an incorrect combination of the unraids primary IP and the external container port. ex: my server IP is 10.10.0.20, a container on bond0.43 has the external IP of 10.43.64.32 and is accessed at port 8080. The webUI link should be "10.43.64.32:8080" but instead it's "10.10.0.20:8080".
  3. I spun up the same stack I have been discussing in the unraid forums (single container, compose-created bridge network NOT bond0.43 as per last discussions) and I see the network and IPs as you've detailed above. However, the webUI link is taking the container port instead of the host-assigned port for the url. ex: In the screenshot, the container port for web access is 6080 which I've mapped to 6090 on the host, so the webUI should be "10.10.0.20:6090" but the webUI link is creating "10.10.0.20:6080". image NOTE: This also happens to any docker containers I've created using the unraid GUI where I select the network "bridge". If I select "host" then there isn't a separate internal and host IP and the webUI URL created takes me to the proper url. image
  4. If I modify that stack from Issue 3 to use my bond0.43 network, the "Port Mappings" column still shows the theoretical container and host ports as though it was a bridge network AND the webUI url that is created is still the "10.10.0.20:6080" url; now, that is almost correct as the port should be 6080 but the URL should be the new bond0.43 IP of 10.43.64.33. To cover my own butt, yes: I destroyed the docker.json file as described in my post on the unraid forums to ensure a new URL is generated and I confirmed as such by making the webui be "google.com" to ensure the changes were taking place. image

Additionally, I would like to make a couple suggestions, I think echoing similar to what @ich777 was suggesting earlier:

  1. IMO, it would be easiest to understand the network layout - especially for new users - if you could display the network name, the container IP, the container port, the external or host IP, and the external or host port. I haven't the faintest idea where to begin change the unraid GUI code to show this, so hopefully the following table created in word will help explain what I mean. In this example, "10.10.0.20" is the unraid host IP address. image

  2. To expand on Suggestion 1: I'm explicitly repeating IPs with port assignments so that in instances where a container is on multiple networks you can easily identify which external IP & port corresponds to which internal IP & port. In your screenshots above, it's not clear to me for adminer which container IP "8080:TCP (internal)" corresponds to (one or the other or both?).

Let me know what other troubleshooting is needed and if you want more explanation on anything.

mrsdoubtfire613 avatar Jul 21 '24 05:07 mrsdoubtfire613

Added "(internal)" text to non-exposed ports when they're shown in advanced view. This one I'm not too sure about as they're already hidden in basic mode.

I agree with @mrsdoubtfire613 on that, it would be best to lay it out like in his table view since it is way easier for new users to understand how the container network is functioning.

What about renaming the columns from Internal IP & Port to Docker IP & Port and External IP & Port to Host IP & Port with that it would be more obvious but I'm also perfectly fine like it is in the example from @mrsdoubtfire613

The main concern that I have is that the Host IP is nowhere to be found currently and that is really confusing (except of course your internal network is the 172.19.0.x which would be very confusing to me).

ich777 avatar Jul 21 '24 11:07 ich777

What about renaming the columns from Internal IP & Port to Docker IP & Port and External IP & Port to Host IP & Port with that it would be more obvious but I'm also perfectly fine like it is in the example from @mrsdoubtfire613

I'm only hesitant labeling the "external" IP column as "host" because host is a network type and implies the server IP and may cause confusion when a container can be accessed from an IP that isn't the host IP (ie: maclvan or ipvlan). Container IP & Port and External IP & Port could be good as well.

In these cases, I'd even suggest a change to my table where instead of the LubeLogger container having "N/A" in the "internal" column, it would have the "external" IP repeated because that's both the internal container IP and the externally-accessible IP. For networks that don't have external access like my Adminer backend network example, it would still have "N/A" in the external column cell.

mrsdoubtfire613 avatar Jul 22 '24 02:07 mrsdoubtfire613

@mtongnz what do you think about that and the table from @mrsdoubtfire613 above?

I would really like to have the IP from the host (if host, bridge or a custom bridge network is set) or the custom IP (if the bridge itself is set) visible on the "external" side so that it is clear for users which IP they can connect to. I also really like the idea from @mrsdoubtfire613 that when a IP/Port mapping is empty that N/A is displayed <- this is a long standing visual issue in the WebUI for some users when they use a custom IP on the bridge itself and the maintainer has no exposed ports in the Dockerfile.

ich777 avatar Jul 25 '24 08:07 ich777

Try this... image

mtongnz avatar Jul 27 '24 23:07 mtongnz

I recreated the same 3 test containers of nginx that you have to hopefully simplify troubleshooting - except my "nginx_test" is attached to the macvlan bond0.43 as I don't have a br0 or bond0 to use. I also added in a docker compose stack attached to two networks to see how that is handled. image

  1. The external ports on my bond0.43 network show nothing (completely blank) and the WebUI URL still uses the servers IP, not the assigned IP.
  2. The docker compose stack (pluginMakemkvTest) shows both networks but doesn't show any external IP:Ports.
  3. The docker compose stack (pluginMakemkvTest) webui URL is still taking the server IP, not the bond0.43 IP.

Given that I can't see how the UI will look with a container on multiple networks with external IP:PORT, I'll wait to comment on the need to repeat the network and container port to match each external until I can see how it looks

Not container specific issue: I'm having a strange issue where the "basic view/advanced view" toggle isn't setting properly or fully(?). When I open the docker tab of the UI, the toggle shows "basic view" but the actual UI is advanced view (I rearranged some containers to shrink the screenshots a bit): image If I then toggle it to "advanced view" then it switches to basic view but the "force update" link appears on the FolderView folders: image

General questions about testing these changes: In order to implement the changes to the UI that you've made here, I've been manually copying the complete raw file code of DockerContainers.page, /include/DockerClient.php, and /include/DockerCotaniners.php from this PR into the respective files via the terminal. Every other webUI related file is whatever is included in unraid 6.12.11.

  1. Are there other files I should be updating?
  2. Is there a better method to implement and test these changes?
  3. My simplistic understanding of how unraid works is that the files I'm manually editing to test the changes are not stored on the boot drive, only in memory, so these changes would not persist across a reboot of the server. Is that correct?

mrsdoubtfire613 avatar Jul 28 '24 16:07 mrsdoubtfire613

All the containers I've tried seem to work. The 3 nginx examples are using dockerman but I've also got a number which are created using compose.

@mrsdoubtfire613 do you have a compose file with a few of your containers so I can try to replicate? All my containers on custom networks ATM to display correctly (I think they're ipvlan though). Please ensure there's no sensitive info in it. Also details of any external networks there containers need.

In regards to updating the files, I use VSCode Server running in a docker work the SFTP plugin. Setup of SFTP is a bit confusing but once you figure out our, it's awesome. It will automatically sync files when you save to a remote system (unRAID in this case). I then use git to manage versioning. This let's me switch between different branches in VSCode easily and then I sync it across work SFTP. Makes it very quick to A/B any changes. Simply copying files works however. They get cleared on reset as you suspect.

mtongnz avatar Jul 28 '24 20:07 mtongnz

@mtongnz Below is the complete contents of the compose file of the pluginMakemkvTest container from my screenshot. The 3 nginx containers were created using the unraid UI so nothing special about them. I just pulled from Community Apps and only changed the network mode and ports where appropriate.

services:
  makemkv:
   container_name: pluginMakemkvTest
    networks:
      - bond043
      - default
    ports:
      - 5910:5900
      - 6090:6080
    privileged: true
    environment:
      - TZ=America/New_York
      - WEBPAGE_TITLE=MakeMKVplugin
      - VNC_PASSWORD=
      - ENABLE_STARTUP_SCRIPTS=yes
      - UMASK=000
      - PUID=99
      - PGID=100
    volumes:
      - /mnt/user:/media:rw
      - /mnt/cache-unprotected/Downloads/:/downloads:rw
      - /mnt/cache-protected-refresh/appdata/dockgeMakemkvTest:/config:rw
    image: binhex/arch-makemkv
networks:
  bond043:
    name: bond0.43
    external: true

Below is the output of 'docker network inspect bond0.43'. I removed references to my other containers just to shorten the output a bit.

[
    {
        "Name": "bond0.43",
        "Id": "48d92dd914b3319138296b5016da749e795767b808632b2a87c37eb851444d9c",
        "Created": "2024-07-17T08:00:11.865073653-04:00",
        "Scope": "local",
        "Driver": "macvlan",
        "EnableIPv6": false,
        "IPAM": {
            "Driver": "default",
            "Options": {},
            "Config": [
                {
                    "Subnet": "10.43.0.0/16",
                    "IPRange": "10.43.64.0/18",
                    "Gateway": "10.43.0.1",
                    "AuxiliaryAddresses": {
                        "server": "10.43.0.18"
                    }
                }
            ]
        },
        "Internal": false,
        "Attachable": false,
        "Ingress": false,
        "ConfigFrom": {
            "Network": ""
        },
        "ConfigOnly": false,
        "Containers": {
            ...
            "7ea11bb0e9045ba7a4bc3a10895e418694f6aa2272dbd5fb9dd5d0d9c3503d88": {
                "Name": "nginx-MACVLAN",
                "EndpointID": "293c8b744f8e42ba189ca0f1cb2729fc3e27e234889dc9645a32e24b29f8e24c",
                "MacAddress": "02:42:0a:2b:40:21",
                "IPv4Address": "10.43.64.33/16",
                "IPv6Address": ""
            },
            ...
        },
        "Options": {
            "parent": "vhost0.43"
        },
        "Labels": {}
    }
]

Thanks for explaining your workflow. I figured out how to clone the webui master repo locally and pull this PR and I have Code Server running as well so things should be a bit simpler for me now.

I'll try and run in safe mode tomorrow to disable plugins and see if that changes anything. If there is anything else you want me to try, just let me know.

mrsdoubtfire613 avatar Jul 29 '24 03:07 mrsdoubtfire613

@mtongnz I've made a PR on your repo, please merge if you like it, everything should now work: grafik

@mrsdoubtfire613 you can see my PR here.

ich777 avatar Jul 29 '24 13:07 ich777