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

fix: docker repository initialization race condition #9992

Merged
merged 1 commit into from
Jun 27, 2023

Conversation

dennis-tra
Copy link
Collaborator

@dennis-tra dennis-tra commented Jun 26, 2023

Fixes: #8725

When running the health check command without passing the --api command line flag and if the Kubo daemon is not active, executing ipfs dag stat will initialize the repository. It is common for the health check command to be run with root privileges. As a result, the repository will be owned by the root user. Then, if the Kubo daemon process attempts to access the repository later on, it will encounter a permission denied error because it runs as a non-privileged user by default.

Hence, this PR simply provides the --api flag to the ipfs dag stat command. Given that we are operating on a docker container, we can make some assumptions. I can't come up with a scenario where one would desire to assign a different port to the internal API rather than using the default 5001. Therefore, I have hard-coded the value accordingly.

Before I settled on this opinion, I had this script:

#!/bin/sh

set -e

# strip any potential trailing slashes and append the filename
API_PATH="${1%/}/api"

# If the file does not exist, we know the daemon
# is not running and the healthcheck failed
if [ ! -f "$API_PATH" ]; then
  exit 1
fi

# The CID of the empty directory
CID=QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn

# Do the actual health check
ipfs dag --api=$(cat "$API_PATH") stat /ipfs/$CID 2>&1 > /dev/null || exit 1

with

HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \
  CMD /usr/local/bin/healthcheck "$IPFS_PATH"

@dennis-tra dennis-tra requested a review from a team as a code owner June 26, 2023 17:59
When running the health check command without passing the `--api` command line flag and if the Kubo daemon is not active, executing `ipfs dag stat` will initialize the repository. It is common for the health check command to be run with root privileges. As a result, the repository will be owned by the root user. Then, if the Kubo daemon process attempts to access the repository later on, it will encounter a permission denied error because it runs as a non-privileged user by default.

Hence, this modification simply provides the `--api` flag to the `ipfs dag stat` command. Given that we are operating within the limited confines of a docker container, we can make a few assumptions. I can't come up with a scenario where one would desire to assign a different port to the internal API rather than using the default 5001. Therefore, I have hard-coded the value accordingly.
@dennis-tra
Copy link
Collaborator Author

@Jorropo what's the deal with the failing check? It seems to time out

@hacdias
Copy link
Member

hacdias commented Jun 27, 2023

@dennis-tra it was an issue in go-ipfs-http-api. Fixed now. Will merge.

@hacdias hacdias merged commit 1972a49 into ipfs:master Jun 27, 2023
15 checks passed
@dennis-tra dennis-tra deleted the docker-healthcheck branch June 27, 2023 11:25
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

Successfully merging this pull request may close these issues.

Ipfs in container creates files with root as owner
3 participants