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

Add container to AIOs community containers to make configuration for them easier #1

Closed
szaimen opened this issue Oct 13, 2023 · 9 comments
Assignees

Comments

@szaimen
Copy link
Contributor

szaimen commented Oct 13, 2023

Hi, I just wanted to mention that AiO has this now: https://github.com/nextcloud/all-in-one/tree/main/community-containers#how-to-add-containers. So the community could potentially add the container
as additional container there. Feel free to ping me if you should need help on this!


Regarding the json, I just leave this here as an example:

{
    "aio_services_v1": [
        {
            "container_name": "nextcloud-aio-dlna",
            "display_name": "DLNA",
            "documentation": "https://github.com/nextcloud/all-in-one/tree/main/community-containers/dlna",
            "image": "thanek/nextcloud-dlna",
            "image_tag": "latest",
            "internal_port": "8200",
            "restart": "unless-stopped",
            "ports": [
                {
                  "ip_binding": "",
                  "port_number": "8200",
                  "protocol": "tcp"
                }
            ],
            "environment": [
                "TZ=%TIMEZONE%",
                "NEXTCLOUD_DLNA_SERVER_PORT=8200",
                "NEXTCLOUD_DLNA_FRIENDLY_NAME=nextcloud-aio",
                "NEXTCLOUD_DATA_DIR=/nextcloud",
                "NEXTCLOUD_DB_HOST=nextcloud-aio-database",
                "NEXTCLOUD_DB_PORT=5432",
                "NEXTCLOUD_DB_NAME=nextcloud_database",
                "NEXTCLOUD_DB_USER=oc_nextcloud",
                "NEXTCLOUD_DB_PASS=%DATABASE_PASSWORD%"
            ],
            "secrets": [
                  "DATABASE_PASSWORD",
            ],
            "volumes": [
                {
                    "source": "%NEXTCLOUD_DATADIR%",
                    "destination": "/nextcloud",
                    "writeable": false
                }
            ]
        }
    ]
}
@thanek
Copy link
Owner

thanek commented Oct 14, 2023

Thanks, I'll take a look at it!

@thanek
Copy link
Owner

thanek commented Oct 23, 2023

I've created a file very similar to your example and it worked on my mac book, but there is a known issue with the "host" networking type on docker, so I cannot access the host's main network interface and test if the whole UPnP communication works well. I'll try to create a Linux machine in my local network and test it :)

@thanek
Copy link
Owner

thanek commented Oct 23, 2023

I made some experiments and finally managed nextcloud-dlna to work inside the nextcloud-aio and finally I was able to get content of the admin nextcloud user. However, I've encountered some issues, because nextcloud-dlna requires the network=host option and, even if it's supported by the aio ecosystem, it causes my container to loose the access to the database container (since the "nextcloud-aio-database" address is unavailable in this network mode).

I've handled it by modifying the database container by adding the port mappings:

+      "ports": [
+        {
+          "ip_binding": "5432",
+          "port_number": "5432",
+          "protocol": "tcp"
+        }
+      ],

in the database container section. This change makes postgres available by the host and by my container under localhost.

However, this change is quite significant and I don't know if you'll be okay with it. Or maybe you have any suggestion how to avoid this problem? From what I know, the UPnP protocol requires the access to "main" network interface since it is initialized by multicast udp requests which are received by other machines in the local network, so I guess, there is no way to avoid the network=host option (from what I've seen, the Plex container also uses this option).

Btw, I need to make some minor fixes in my code, because I found some bugs when experimenting with aio ;)

P.S. The final version of dlna.json would then look like this:

{
    "aio_services_v1": [
        {
            "container_name": "nextcloud-aio-dlna",
            "display_name": "DLNA",
            "documentation": "https://github.com/nextcloud/all-in-one/tree/main/community-containers/dlna",
            "image": "thanek/nextcloud-dlna",
            "image_tag": "latest",
            "internal_port": "host",
            "restart": "unless-stopped",
            "ports": [
                {
                    "ip_binding": "",
                    "port_number": "9999",
                    "protocol": "tcp"
                }
            ],
            "environment": [
                "NEXTCLOUD_DLNA_SERVER_PORT=9999",
                "NEXTCLOUD_DLNA_FRIENDLY_NAME=Nextcloud-AIO",
                "NEXTCLOUD_DATA_DIR=/data",
                "NEXTCLOUD_DB_TYPE=postgres",
                "NEXTCLOUD_DB_HOST=localhost",
                "NEXTCLOUD_DB_PORT=5432",
                "NEXTCLOUD_DB_NAME=nextcloud_database",
                "NEXTCLOUD_DB_USER=nextcloud",
                "NEXTCLOUD_DB_PASS=%DATABASE_PASSWORD%"
            ],
            "secrets": [
                "DATABASE_PASSWORD"
            ],
            "volumes": [
                {
                    "source": "%NEXTCLOUD_DATADIR%",
                    "destination": "/data",
                    "writeable": false
                }
            ],
            "backup_volumes": [
                "nextcloud_aio_dlna"
            ]
        }
    ]
}

@thanek
Copy link
Owner

thanek commented Oct 25, 2023

@szaimen I think, I've found a better solution to the problem. I've introduced new env variable placeholder %AIO_DATABASE_HOST% which is translated in the DockerActionManager.php as gethostbyname('nextcloud-aio-database'). Thanks to this, I was able to use just another env variable inside the dlna.json, which now can look like this:

{
    "aio_services_v1": [
        {
            "container_name": "nextcloud-aio-dlna",
            "display_name": "DLNA",
            "documentation": "https://github.com/nextcloud/all-in-one/tree/main/community-containers/dlna",
            "image": "thanek/nextcloud-dlna",
            "image_tag": "latest",
            "internal_port": "host",
            "restart": "unless-stopped",
            "depends_on": [
                "nextcloud-aio-database",
                "nextcloud-aio-nextcloud"
            ],
            "ports": [
                {
                    "ip_binding": "",
                    "port_number": "9999",
                    "protocol": "tcp"
                }
            ],
            "environment": [
                "NEXTCLOUD_DLNA_SERVER_PORT=9999",
                "NEXTCLOUD_DLNA_FRIENDLY_NAME=Nextcloud-AIO",
                "NEXTCLOUD_DATA_DIR=/data",
                "NEXTCLOUD_DB_TYPE=postgres",
                "NEXTCLOUD_DB_HOST=%AIO_DATABASE_HOST%",
                "NEXTCLOUD_DB_PORT=5432",
                "NEXTCLOUD_DB_NAME=nextcloud_database",
                "NEXTCLOUD_DB_USER=nextcloud",
                "NEXTCLOUD_DB_PASS=%DATABASE_PASSWORD%"
            ],
            "secrets": [
                "DATABASE_PASSWORD"
            ],
            "volumes": [
                {
                    "source": "%NEXTCLOUD_DATADIR%",
                    "destination": "/data",
                    "writeable": false
                }
            ],
            "backup_volumes": [
                "nextcloud_aio_dlna"
            ]
        }
    ]
}

With this approach we avoid exposing the postgresql port on the host network and the database is still accessible with use of the DB-container's IP address. I've also added the dependsOn section to the config since it's necessary for the DB-container to be alive when the mastercontainer translates the AIO_DB_HOST placeholder to it's IP number. But this dependency seems to be quite natural.

What do you think about it?

@szaimen
Copy link
Contributor Author

szaimen commented Oct 26, 2023

What do you think about it?

That looks like a good idea! Just created a PR to add this: nextcloud/all-in-one#3611.

@szaimen
Copy link
Contributor Author

szaimen commented Oct 26, 2023

If the container runs in host mode, you don't need to specify the port it uses btw :)

So the following should work:

{
    "aio_services_v1": [
        {
            "container_name": "nextcloud-aio-dlna",
            "display_name": "DLNA",
            "documentation": "https://github.com/nextcloud/all-in-one/tree/main/community-containers/dlna",
            "image": "thanek/nextcloud-dlna",
            "image_tag": "latest",
            "internal_port": "host",
            "restart": "unless-stopped",
            "depends_on": [
                "nextcloud-aio-database"
            ],
            "environment": [
                "NEXTCLOUD_DLNA_SERVER_PORT=9999",
                "NEXTCLOUD_DLNA_FRIENDLY_NAME=Nextcloud-AIO",
                "NEXTCLOUD_DATA_DIR=/data",
                "NEXTCLOUD_DB_TYPE=postgres",
                "NEXTCLOUD_DB_HOST=%AIO_DATABASE_HOST%",
                "NEXTCLOUD_DB_PORT=5432",
                "NEXTCLOUD_DB_NAME=nextcloud_database",
                "NEXTCLOUD_DB_USER=oc_nextcloud",
                "NEXTCLOUD_DB_PASS=%DATABASE_PASSWORD%"
            ],
            "secrets": [
                "DATABASE_PASSWORD"
            ],
            "volumes": [
                {
                    "source": "%NEXTCLOUD_DATADIR%",
                    "destination": "/data",
                    "writeable": false
                }
            ]
        }
    ]
}

@thanek
Copy link
Owner

thanek commented Oct 26, 2023

Yeah, you're right about the ports, thanks! :)
I'm glad you like the idea, already seen your PR in AIO repo (added a comment).
Don't you think that nextcloud-aio-nextcloud should also be in the depends_on section since nextcloud-dlna relies on the NEXTCLOUD_DATADIR?

@thanek thanek self-assigned this Oct 26, 2023
@thanek
Copy link
Owner

thanek commented Oct 26, 2023

OK, right after you add the AIO_DATABASE_HOST variable support, my PR with the nextcloud-dlna as a community-container in AIO will be ready for a review: nextcloud/all-in-one#3614 :)

@thanek
Copy link
Owner

thanek commented Oct 31, 2023

OK, so the nextcloud-dlna is now available from the Nextcloud All in One. Thank you very much for your help and cooperation! :)

@thanek thanek closed this as completed Oct 31, 2023
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

No branches or pull requests

2 participants