Improve is docker env checks

[candrews]

Proposed change

Improve is docker env checks

Check for the existence of the /run/.containerenv file. This file is created by podman (https://github.com/containers/podman/blob/v5.3.1/docs/source/markdown/podman-run.1.md.in#L31), cri-o (cri-o/cri-o#5461), and other runtimes.

Check for the "container" environment variable having a value. This environment variable is always set by podman (containers/podman#3586 (comment)) and other runtimes. Users can easily set this environment variable to ensure detection as a container.

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New integration (thank you!)
• New feature (which adds functionality to an existing integration)
• Deprecation (breaking change to happen in the future)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes Integrations that require a Python module installation fail to add on 2024.10.1 when installed through HACS #127966

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• I have followed the perfect PR recommendations
• The code has been formatted using Ruff (ruff format homeassistant tests)
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

• I have reviewed two other open pull requests in this repository.

[home-assistant]

Hi @candrews

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

[home-assistant]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

Stale

[frenck]

This is a Docker env check, not Podman.

Additionally, Podman cannot be used to run Home Assistant in a supported way as far as I am aware.

../Frenck

[javydekoning]

This wouldn't fix detection in containerd or potentially other container runtimes. Might as well add a check if env KUBERNETES_SERVICE_HOST exists. But why do we need this check anyway?

[malaakso]

This is a Docker env check, not Podman.

Additionally, Podman cannot be used to run Home Assistant in a supported way as far as I am aware.

../Frenck

Home Assistant works inside Podman just fine. It is more or less a drop-in replacement for Docker. However, according to the issue mentioned in the PR, custom integrations are incorrectly installed since other container environments apart from Docker are not detected properly.

[candrews]

This wouldn't fix detection in containerd or potentially other container runtimes. Might as well add a check if env KUBERNETES_SERVICE_HOST exists. But why do we need this check anyway?

containerd uses cri-o per https://github.com/containerd/containerd?tab=readme-ov-file#cri-status and cri-o provides /run/.containerenv per cri-o/cri-o#5461 Therefore, I think containerd is already covered.

This proposal also checks the container environment variable which is easy to set if there are any cases where the detection doesn't work correctly.

[muhlba91]

This is a Docker env check, not Podman.
Additionally, Podman cannot be used to run Home Assistant in a supported way as far as I am aware.
../Frenck

Home Assistant works inside Podman just fine. It is more or less a drop-in replacement for Docker. However, according to the issue mentioned in the PR, custom integrations are incorrectly installed since other container environments apart from Docker are not detected properly.

nowadays, i think the naming is_docker_env is misleading if you consider the various container runtimes and orchestrators.

i am running HA on Kubernetes using containerd and am testing my integration locally using Podman (macOS). it all works perfectly fine, which is the main point of having a container image. i am pretty sure any runtime/orchestrator is able to run HA nowadays.
however, in both ways, i am facing the referenced issue with custom integration not being able to correctly install.

looking at the container tech landscape and compatibilities between them, i do think it's a reasonable request to include other runtimes in this check and not limit it to one runtime/orchestrator only, especially considering the few lines required to do so, and that there's no requirement of any orchestrator in HA's codebase (e.g. specific runtime function being required or triggered).

it could potentially make sense to rename the check to a broader name like is_container_env to circumvent specific naming issues.

if the lack of automated testing for other runtimes/orchestrators causes hesitation regarding this pull request, maybe being more specific about it in the docs can be a solution. probably something along the lines that container runtimes are generally supported, but only Docker is automatically tested for each release.

[javydekoning]

While this change would somewhat work, it's a workaround, not a solution. There is no definitive way to assess whether or not one is running in a container and therefore relying on it is risky. For example, the changes wouldn't work on k3s/containerd

I've noticed this which could help as well? Again a workaround, but perhaps a more reliable one.

def is_official_image() -> bool:
    """Return True if Home Assistant is running in an official container."""
    return os.path.isfile("/OFFICIAL_IMAGE")

[candrews]

While this change would somewhat work, it's a workaround, not a solution. There is no definitive way to assess whether or not one is running in a container and therefore relying on it is risky.

Let's not let the desire for perfection block progress. This is a solution that automatically addresses many cases, providing a marked improvement for users, while also providing an "escape hatch" in the form of the "container" environment variable for when the auto-detection doesn't work.

For example, the changes wouldn't work on k3s/containerd

I believe this approach would work on containerd: #132404 (comment)

Again a workaround, but perhaps a more reliable one.

Instead of a file, how about using this PR and setting the container environment variable? That's a common approach used by and supported by many other tools (ex, systemd https://github.com/systemd/systemd/blob/fdd25311706bd32580ec4d43211cdf4665d2f9de/src/shared/virt.c#L245).

[javydekoning]

While this change would somewhat work, it's a workaround, not a solution. There is no definitive way to assess whether or not one is running in a container and therefore relying on it is risky.

Let's not let the desire for perfection block progress. This is a solution that automatically addresses many cases, providing a marked improvement for users, while also providing an "escape hatch" in the form of the "container" environment variable for when the auto-detection doesn't work.

Agreed!

For example, the changes wouldn't work on k3s/containerd

I believe this approach would work on containerd: #132404 (comment)

Sorry it doesn't cri-o != containerd

root@k3s [04:45:57 PM] [~]
-> # crictl inspect 68677a7693893 | jq -r '.status.image.image'
ghcr.io/home-assistant/home-assistant:2024.12.0
root@k3s [04:46:11 PM] [~]
-> # k exec -it home-assistant-0 -- /bin/sh
/config # ls -l /run/.containerenv
ls: /run/.containerenv: No such file or directory
/config #

Again a workaround, but perhaps a more reliable one.

Instead of a file, how about using this PR and setting the container environment variable? That's a common approach used by and supported by many other tools (ex, systemd https://github.com/systemd/systemd/blob/fdd25311706bd32580ec4d43211cdf4665d2f9de/src/shared/virt.c#L245).

As this file is present in the official container image, perhaps worth adding as alternative check to this patch?

[candrews]

How about adding ENV container=oci to the Dockerfile at

core/Dockerfile

Line 8 in 46e5136

ENV \

?

That approach is preferrable over checking for a file as HA may move that file later inadvertently breaking this check. The environment variable is more or less standard and common (red hat sets it on all of its images, ex https://hub.docker.com/layers/redhat/ubi8/latest/images/sha256-4dd46257b3a8d40b0f4e25c39e3c79696077090e97a71ace4e2c7080ba37fb99?context=explore) so it should be more expected.

[javydekoning]

That approach is preferrable over checking for a file as HA may move that file later inadvertently breaking this check.

To remove the file, every reference to this function also needs to be cleaned:

core/homeassistant/helpers/system_info.py

Line 27 in 46e5136

def is_official_image() -> bool:

And therefore this use-case will be found as well.

Both solutions won't work for non-official images like https://github.com/linuxserver/docker-homeassistant/blob/main/Dockerfile

[frenck]

The CI is failing, can you please make sure pre-commit passes before marking the PR as ready?

Thanks! 👍

../Frenck

[eohlde]

[> How about adding ENV container=oci to the Dockerfile at

core/Dockerfile

Line 8 in 46e5136

ENV \

?

That approach is preferrable over checking for a file as HA may move that file later inadvertently breaking this check. The environment variable is more or less standard and common (red hat sets it on all of its images, ex https://hub.docker.com/layers/redhat/ubi8/latest/images/sha256-4dd46257b3a8d40b0f4e25c39e3c79696077090e97a71ace4e2c7080ba37fb99?context=explore) so it should be more expected.](#132404 (comment))

Isn't this the best approach?
Set an ENV in the dockerfile IM_A_CONTAINER=true (or an appropriately named key) That way method simply checks for that environment variable KVP?
-> Users could still override this, but they would then intentionally be breaking their application.

Unofficial images could grab that change also, or use the official image as a base?

[malaakso]

[> How about adding ENV container=oci to the Dockerfile at

core/Dockerfile

Line 8 in 46e5136

ENV \

?
That approach is preferrable over checking for a file as HA may move that file later inadvertently breaking this check. The environment variable is more or less standard and common (red hat sets it on all of its images, ex https://hub.docker.com/layers/redhat/ubi8/latest/images/sha256-4dd46257b3a8d40b0f4e25c39e3c79696077090e97a71ace4e2c7080ba37fb99?context=explore) so it should be more expected.](#132404 (comment))

Isn't this the best approach? Set an ENV in the dockerfile IM_A_CONTAINER=true (or an appropriately named key) That way method simply checks for that environment variable KVP? -> Users could still override this, but they would then intentionally be breaking their application.

Unofficial images could grab that change also, or use the official image as a base?

As discussed here, including environment variables in the image is a bad practice.

[frenck]

Just a heads up regarding #127966 Please see my write-up at #127966 (comment)

Looks like this just masks the actual bug because when HASS detects a docker environment, it installs packages into the system python libs instead of /config/deps. This is not preferable since that isn't persistent across pod restarts.

This is not related to this issue. We actually do not want to persist it, that is by design.

../Frenck

[frenck]

@candrews I've seen you made a change, but the PR is still in draft?

Are you still working on things or did you forget to mark it ready for review?

Additionally, it would be nice to add some tests, as this hit the core of Home Assistant.

../Frenck

[edenhaus]

I will pick it up, so we can include it in 2025.1

[frenck]

Thanks, @candrews & @edenhaus!

Happy New Year 🥂

../Frenck