-
Notifications
You must be signed in to change notification settings - Fork 77
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
update versions/BUF_VERSION #2703
Conversation
Signed-off-by: vankichi <[email protected]>
📝 WalkthroughWalkthroughThe pull request includes modifications to two Dockerfiles, specifically Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
[CHATOPS:HELP] ChatOps commands.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
dockers/ci/base/Dockerfile (1)
Line range hint
1-124
: Consider optimizing Dockerfile for better caching and smaller image sizeWhile the current Dockerfile structure is good, consider the following optimizations:
- Group related
RUN
commands: Combine related installation commands into a singleRUN
instruction to reduce the number of layers.- Use
--no-install-recommends
consistently: Ensure this flag is used for allapt-get install
commands to minimize unnecessary package installations.- Clean up package manager cache: Add
rm -rf /var/lib/apt/lists/*
at the end of apt-get operations to reduce image size.- Use specific versions: For tools installed via
make
, consider specifying versions to ensure reproducibility.Here's an example of how you could optimize the
RUN
instruction:RUN --mount=type=bind,target=.,rw \ --mount=type=tmpfs,target=/tmp \ --mount=type=cache,target=/var/lib/apt,sharing=locked,id=${APP_NAME} \ --mount=type=cache,target=/var/cache/apt,sharing=locked,id=${APP_NAME} \ --mount=type=cache,target="${GOPATH}/pkg",id="go-build-${TARGETARCH}" \ --mount=type=cache,target="${HOME}/.cache/go-build",id="go-build-${TARGETARCH}" \ --mount=type=tmpfs,target="${GOPATH}/src" \ set -ex \ && echo 'Binary::apt::APT::Keep-Downloaded-Packages "true";' > /etc/apt/apt.conf.d/keep-cache \ && echo 'APT::Install-Recommends "false";' > /etc/apt/apt.conf.d/no-install-recommends \ && apt-get update -y \ && apt-get upgrade -y \ && apt-get install -y --no-install-recommends --fix-missing \ build-essential ca-certificates curl tzdata locales git npm cmake g++ gcc \ libssl-dev unzip liblapack-dev libomp-dev libopenblas-dev gfortran pkg-config \ file gawk gnupg2 graphviz jq libaec-dev libhdf5-dev sed zip \ && ldconfig \ && echo "${LANG} UTF-8" > /etc/locale.gen \ && ln -fs /usr/share/zoneinfo/${TZ} /etc/localtime \ && locale-gen ${LANGUAGE} \ && update-locale LANG=${LANGUAGE} \ && dpkg-reconfigure -f noninteractive tzdata \ && apt-get clean \ && apt-get autoclean -y \ && apt-get autoremove -y \ && rm -rf /var/lib/apt/lists/* \ && make GOPATH="${GOPATH}" GOROOT="${GOROOT}" GO_VERSION="${GO_VERSION}" go/install \ && make GOPATH="${GOPATH}" GOROOT="${GOROOT}" GO_VERSION="${GO_VERSION}" go/download \ && make RUST_VERSION="${RUST_VERSION}" rust/install \ && make GOARCH=${TARGETARCH} GOOS=${TARGETOS} deps GO_CLEAN_DEPS=false \ && make GOARCH=${TARGETARCH} GOOS=${TARGETOS} golangci-lint/install \ && make GOARCH=${TARGETARCH} GOOS=${TARGETOS} gotestfmt/install \ && make cmake/install buf/install hdf5/install helm-docs/install helm/install \ k3d/install k9s/install kind/install kubectl/install kubelinter/install \ minikube/install reviewdog/install stern/install telepresence/install \ tparse/install yq/install ngt/install faiss/install usearch/install \ && rm -rf ${GOPATH}/src/github.com/${ORG}/${REPO}/*This optimization reduces the number of layers and ensures consistent cleanup across all package installations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- dockers/agent/core/agent/Dockerfile (1 hunks)
- dockers/ci/base/Dockerfile (1 hunks)
- versions/BUF_VERSION (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- versions/BUF_VERSION
🧰 Additional context used
🔇 Additional comments (2)
dockers/agent/core/agent/Dockerfile (1)
43-43
: LGTM: Addition of RUSTUP_HOME environment variableThe addition of the
RUSTUP_HOME
environment variable is a good practice when working with Rust in Docker environments. It explicitly sets the location for Rust toolchain files, which can improve consistency and ease of management. This change aligns well with the existingCARGO_HOME
variable and follows Rust's recommended setup.dockers/ci/base/Dockerfile (1)
48-48
: LGTM! Consider verifying Rust tool installations.The addition of the
RUSTUP_HOME
environment variable is a good practice for Rust development environments. It properly separates Rustup-specific files from other Rust-related files and is consistent with the existingCARGO_HOME
variable.To ensure this change doesn't affect existing Rust tool installations, consider running the following verification script:
Signed-off-by: vankichi <[email protected]>
Signed-off-by: vankichi <[email protected]> Co-authored-by: Kiichiro YUKAWA <[email protected]>
Signed-off-by: vankichi <[email protected]>
Signed-off-by: vankichi <[email protected]>
Description
Due to the current buf version, we cannot execute the
make license format
completely, so I have updated the buf version.( buf@v2 can be used over 1.32.0 )
Note:
After updating the buf version, I executed
make license format
and observed that 2 docker files had changed theENV
order.Related Issue
Versions
Checklist
Special notes for your reviewer
Summary by CodeRabbit
New Features
RUSTUP_HOME
to enhance Rust tool configurations in Docker containers.Version Update
v1.28.1
tov1.45.0
.