search

hassio-addons / addon-unifi

UniFi Network Application - Home Assistant Community Add-ons
https://addons.community
MIT License
276 stars 137 forks source link

Dockerfile improvements #481

Closed cfergeau closed 8 months ago

cfergeau commented 9 months ago

Proposed Changes

This PR does various improvements to the Dockerfile:

Related Issues

(Github link to related issues or pull requests)

frenck commented 9 months ago

run apt-get upgrade to get the latest security updates

I'm not aware there is one? If so, please let me know what is going on, and let's fix it in the base image.

Upgrading packages on this level is also pretty inefficient.

removes unneeded package, which reduces the image size by about 10%

You've removed jsvc, which is assumingly used internally by UniFi, see also their installer scripts by the community linked to from UniFi: https://get.glennr.nl/unifi/install/unifi-8.0.26.sh

I don't think that is correct? Can you provide the reasoning for this removal and where I can information on this otherwise?

improve the way package version are specified in apt-get install so that this line does not have to be updated every time one of these packages is updated

I was not aware this was a problem in this repo? Could you clarify where and why this is an issue?

cfergeau commented 9 months ago

I'm not aware there is one? If so, please let me know what is going on, and let's fix it in the base image.

Running apt-get upgrade in ghcr.io/hassio-addons/ubuntu-base:8.2.0 gives:

apt base-files bsdutils ca-certificates curl fdisk gcc-10-base libapt-pkg6.0 libasn1-8-heimdal libblkid1 libc-bin libc6 libcurl4 libfdisk1 libgcc-s1 libgnutls30 libgssapi-krb5-2 libgssapi3-heimdal libhcrypto4-heimdal libheimbase1-heimdal libheimntlm0-heimdal libhx509-5-heimdal libk5crypto3 libkrb5-26-heimdal libkrb5-3 libkrb5support0 libmount1 libncurses6 libncursesw6 libnghttp2-14 libpam-modules libpam-modules-bin libpam-runtime libpam0g libprocps8 libroken18-heimdal libsmartcols1 libsqlite3-0 libssh-4 libssl1.1 libstdc++6 libsystemd0 libtinfo6 libudev1 libuuid1 libwind0-heimdal mount ncurses-base ncurses-bin openssl perl-base procps tar tzdata util-linux

which is a fairly large set of packages. https://github.com/hassio-addons/addon-unifi/pull/481#discussion_r1453679837 has more details why I could not find a better base image to use instead. Or maybe official addon builds are using automatically built base image or something like this, and not ghcr.io/hassio-addons/ubuntu-base:8.2.0 as specified in the Dockerfile?

cfergeau commented 9 months ago

You've removed jsvc, which is assumingly used internally by UniFi, see also their installer scripts by the community linked to from UniFi: https://get.glennr.nl/unifi/install/unifi-8.0.26.sh

I don't think that is correct? Can you provide the reasoning for this removal and where I can information on this otherwise?

I've added some reasoning in the commit log:

The unifi package does not depend on it, `git grep jsvc` does not return a match, this is the only binary the jsvc package provides, and jsvc pulls openjdk-jre-11 while the rest of the code switched to 17

"does not depend on it" in the sense of "the unifi package does not have a dpkg/apt dependency for it" I've also checked I can start the image without it (I did not try to go further than the first unifi screen on port 8443)

cfergeau commented 9 months ago

I was not aware this was a problem in this repo? Could you clarify where and why this is an issue?

Not a huge issue if there's a bot automatically updating the repo, but for example this PR is not buildable because main is missing

-        binutils=2.34-6ubuntu1.7 \
+        binutils=2.34-6ubuntu1.8 \
cfergeau commented 9 months ago

You've removed jsvc, which is assumingly used internally by UniFi, see also their installer scripts by the community linked to from UniFi: https://get.glennr.nl/unifi/install/unifi-8.0.26.sh

I've looked closer at this script, for unifi 8.x it has:

if [[ "${first_digit_unifi}" -gt '8' ]] || [[ "${first_digit_unifi}" == '8' && "${second_digit_unifi}" -ge "1" ]]; then
  ...
  required_java_version="openjdk-17"
  required_java_version_short="17"
fi

and jsvc installation is only done for openjdk-8, not for newer openjdk versions:

  if [[ "${required_java_version}" == "openjdk-8" ]]; then
    echo -e "${WHITE_R}#${RESET} Installing jsvc and libcommons-daemon-java..."
    if DEBIAN_FRONTEND='noninteractive' apt-get -y "${apt_options[@]}" -o Dpkg::Options::='--force-confdef' -o Dpkg::Options::='--force-confold' install jsvc libcommons-daemon-java &>> "${eus_dir}/logs/apt.log"; then echo -e "${GREEN}#${RESET} Successfully installed jsvc and libcommons-daemon-java! \\n"; else echo -e "${RED}#${RESET} Failed to install jsvc and libcommons-daemon-java in the first run...\\n"; unifi_dependencies=fail; fi
  fi

I also found https://community.ui.com/questions/New-UniFi-Controller-Installation-Issues-Ubuntu-18-04/a4a3b308-0ed4-4a4a-b54d-e0b59f9e4661 which indicates that at some point in the past, the unifi debian package had a requirement the jsvc package: The following packages have unmet dependencies: unifi : Depends: jsvc (>= 1.0.8) but it is not installable. This dependency is not present in the latest version of the package.