AIO-interface: add Unhealthy container state

[docjyJ]

• Add Unhealty state
• Replace class by backed enum

[st3iny]

Hey Jean-Yves, Simon asked me to have a look at your changes. I left some feedback regarding the code itself.

I really like you refactoring the container state logic from an interface to an enum. However, I think that both concerns could have been split into separate PRs. One PR to refactor the interface and another one to implement the new healthy states. But the code is already there so why not.

Looks good otherwise but did not test.

[docjyJ]

Hi,

Thanks for the answer, as soon as I have time I'll look into it.

I will try to separate and make several PR.

Take care,

[docjyJ]

I keep this PR open for the unhealthy state.
See #5372 for enum

[szaimen]

Conflicts :/

[docjyJ]

Don't worry, i'll handle it.

[docjyJ]

Solved and up to date and ready (to be tested anyway...)

[docjyJ]

I have podman on my machine and I can't launch the container... I can't test it and I don't have time to debug...

[szaimen]

I have podman on my machine and I can't launch the container...

Why may I ask? Do you run into an issue here?

[docjyJ]

Why may I ask? Do you run into an issue here?

podman run \
--rm \
--name nextcloud-aio-mastercontainer \
--volume nextcloud_aio_mastercontainer:/mnt/docker-aio-config:z \
--volume /var/run/user/1000/podman/podman.sock:/var/run/docker.sock:z \
--env WATCHTOWER_DOCKER_SOCKET_PATH=/var/run/user/1000/podman/podman.sock \
nextcloud/all-in-one:latest

Trying to fix docker.sock permissions internally...
Adding internal www-data to group root
Cannot connect to the docker socket. Cannot proceed.
Did you maybe remove group read permissions for the docker socket? AIO needs them in order to access the docker socket.
If SELinux is enabled on your host, see https://github.com/nextcloud/all-in-one#are-there-known-problems-when-selinux-is-enabled
If you are on TrueNas SCALE, see https://github.com/nextcloud/all-in-one#can-i-run-aio-on-truenas-scale

I don't know what happen

[szaimen]

Why may I ask? Do you run into an issue here?

podman run \
--rm \
--name nextcloud-aio-mastercontainer \
--volume nextcloud_aio_mastercontainer:/mnt/docker-aio-config:z \
--volume /var/run/user/1000/podman/podman.sock:/var/run/docker.sock:z \
--env WATCHTOWER_DOCKER_SOCKET_PATH=/var/run/user/1000/podman/podman.sock \
nextcloud/all-in-one:latest

Trying to fix docker.sock permissions internally...
Adding internal www-data to group root
Cannot connect to the docker socket. Cannot proceed.
Did you maybe remove group read permissions for the docker socket? AIO needs them in order to access the docker socket.
If SELinux is enabled on your host, see https://github.com/nextcloud/all-in-one#are-there-known-problems-when-selinux-is-enabled
If you are on TrueNas SCALE, see https://github.com/nextcloud/all-in-one#can-i-run-aio-on-truenas-scale

I don't know what happen

Maybe this helps? https://github.com/nextcloud/all-in-one?tab=readme-ov-file#are-there-known-problems-when-selinux-is-enabled

[docjyJ]

🙄 🙄 🙄 🙄

thank 😅

[docjyJ]

It doesn't work, I give up.
Can you test this?
One way to test would be to go into the Nextcloud container and add exit 1 in the heathcheck.sh script.

According to the docker documentation: https://docs.docker.com/reference/api/engine/version/v1.45/#tag/Container/operation/ContainerInspect

State - Health - Status

Status is one of none, starting, healthy or unhealthy

"none" Indicates there is no healthcheck
"starting" Starting indicates that the container is not yet ready
"healthy" Healthy indicates that the container is running correctly
"unhealthy" Unhealthy indicates that the container has a problem

Adding this information would be good for detecting potential problems and for debugging.

[docjyJ]

Another question Why not use starting state and restarting state from docker ?

Enum: "created" "running" "paused" "restarting" "removing" "exited" "dead"

String representation of the container state. Can be one of "created", "running", "paused", "restarting", "removing", "exited", or "dead".

Could we use this indicator?
It would be easier to maintain.
If you agree, I can try to rewrite the functions GetContainerRestartingState, GetContainerRunningState, GetContainerRestartingState.

[docjyJ]

Docker doc for meaning of state https://docs.docker.com/reference/cli/docker/container/ls/#status

[docjyJ]

A proposal in commit b903cc3.

(I renamed the enums to match the getters name, it's more consistent but I can rollback)

[docjyJ]

A proposal in commit b903cc3.

What do you think @szaimen ?

[szaimen]

Another question Why not use starting state and restarting state from docker ?

Enum: "created" "running" "paused" "restarting" "removing" "exited" "dead"
String representation of the container state. Can be one of "created", "running", "paused", "restarting", "removing", "exited", or "dead".

Could we use this indicator? It would be easier to maintain. If you agree, I can try to rewrite the functions GetContainerRestartingState, GetContainerRunningState, GetContainerRestartingState.

I overread this. Yes sounds good imo :)