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(dockerfiles/bases): shrink base image size #431

Merged
merged 1 commit into from
Sep 23, 2024

Conversation

wuhuizuo
Copy link
Contributor

add clean step to rmove /var/cache/yum.

Signed-off-by: wuhuizuo wuhuizuo@126.com

add clean step to rmove `/var/cache/yum`.

Signed-off-by: wuhuizuo <wuhuizuo@126.com>
@ti-chi-bot ti-chi-bot bot requested a review from purelind September 20, 2024 02:56
Copy link

ti-chi-bot bot commented Sep 20, 2024

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

Based on the pull request's title and description, it looks like the main change is to shrink the base image size by adding a clean step to remove /var/cache/yum in several Dockerfiles. The changes have been made to multiple files in the dockerfiles/bases directory.

Overall, the changes look good and should help reduce the size of the Docker images. However, there are a few potential problems that should be addressed:

  • The rm -rf /var/cache/yum command may not work as expected if the directory does not exist or if the user running the command does not have sufficient permissions. A safer approach would be to use yum clean all to clear the cache.
  • It's unclear why some of the Dockerfiles are pinned to specific dates (_date=20240919). This may cause issues in the future if the packages are updated and the base image is rebuilt.
  • The changes to the skaffold.yaml file seem unrelated to the main change in this pull request. It may be better to create a separate pull request for those changes.

To address these potential problems, I suggest the following changes:

  • Replace rm -rf /var/cache/yum with yum clean all in all the affected Dockerfiles.
  • Remove the _date variable and let dnf upgrade use the latest packages.
  • Create a separate pull request for the changes to skaffold.yaml.

Once these changes have been made, the pull request should be ready to merge.

@ti-chi-bot ti-chi-bot bot added the size/M label Sep 20, 2024
@wuhuizuo
Copy link
Contributor Author

/approve

Copy link

ti-chi-bot bot commented Sep 23, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wuhuizuo

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

The pull request process is described 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 ti-chi-bot bot added the approved label Sep 23, 2024
@ti-chi-bot ti-chi-bot bot merged commit 3c23267 into main Sep 23, 2024
8 checks passed
@ti-chi-bot ti-chi-bot bot deleted the fix/shrink-base-image-size branch September 23, 2024 08:21
@wuhuizuo
Copy link
Contributor Author

It seems not reduce the image total size. I will revert it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant