ci icon indicating copy to clipboard operation
ci copied to clipboard

chore(tiflash): update dind image version

Open purelind opened this issue 10 months ago • 4 comments

Update dind image version. Avoid encountering this issue in the future https://github.com/docker-library/golang/issues/467

purelind avatar Apr 22 '24 07:04 purelind

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please ask for approval from purelind, ensuring that each of them provides their approval before proceeding. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot[bot] avatar Apr 22 '24 07:04 ti-chi-bot[bot]

I have already done a preliminary review for you, and I hope to help you do a better job.

Summary:

This is a simple PR that updates the Docker-in-Docker (dind) image version in several YAML files. The new image version is "hub.pingcap.net/jenkins/docker:20.10.14-dind". Additionally, two environment variables have been added to the container definition, and a readiness probe has been defined to check the Docker daemon status.

Potential problems:

There are no major problems with this PR. However, there are some suggestions:

  1. The PR description could be more descriptive and informative. It would be helpful to explain why the image version is being updated and what benefits it brings.
  2. It is not clear why the two environment variables are necessary. A comment explaining their purpose would be helpful.
  3. The readiness probe checks the Docker daemon status, but it does not check if the Docker daemon is fully functional. A check that tests a basic Docker command such as "docker ps" would be more comprehensive.

Fixing suggestions:

  1. The PR description could be improved by stating the reason for the update, such as "Updating dind image version to improve security and performance".
  2. A comment explaining the purpose of the environment variables should be added. For example, "The DOCKER_TLS_CERTDIR variable disables TLS certificate verification, which is necessary for some private registries. The DOCKER_HOST variable specifies the Docker daemon address."
  3. A more comprehensive readiness probe could be added. For example, "readinessProbe: exec: command: - sh - -c - 'docker ps > /dev/null 2>&1 && docker version > /dev/null 2>&1'"

ti-chi-bot[bot] avatar Apr 22 '24 07:04 ti-chi-bot[bot]

/hold

hub.pingcap.net/tiflash/tiflash-ci-base always restarting with those error

/usr/local/bin/../include/c++/v1/vector:1571: _LIBCPP_ASSERT '__n < size()' failed. vector[] index out of bounds
/usr/local/bin/../include/c++/v1/vector:1571: _LIBCPP_ASSERT '__n < size()' failed. vector[] index out of bounds
/usr/local/bin/../include/c++/v1/vector:1571: _LIBCPP_ASSERT '__n < size()' failed. vector[] index out of bounds
libc++abi: terminate_handler unexpectedly threw an exception
libc++abi: terminate_handler unexpectedly threw an exception
libc++abi: terminate_handler unexpectedly threw an exception
libc++abi: terminate_handler unexpectedly threw an exception
libc++abi: terminate_handler unexpectedly threw an exception

purelind avatar Apr 22 '24 09:04 purelind

ref: #2820

wuhuizuo avatar Apr 23 '24 01:04 wuhuizuo