Skip to content

Conversation

@afbjorklund
Copy link
Collaborator

@afbjorklund afbjorklund commented Dec 17, 2025

And apt install cmake to avoid host-cmake


Building cmake is the second longest build time (after the kernel)!

It even takes longer time than the compiler (gcc) and glibc packages.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: afbjorklund
Once this PR has been reviewed and has the lgtm label, please assign medyagh for approval. For more information see the Code Review Process.

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

Details 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

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 17, 2025
@k8s-ci-robot k8s-ci-robot requested a review from medyagh December 17, 2025 19:25
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 17, 2025
@medyagh medyagh requested a review from Copilot December 17, 2025 20:01
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to improve build performance for the minikube ISO by avoiding the lengthy process of building cmake from source and enabling persistent ccache storage via a Docker volume. The changes install cmake via apt (which is significantly faster than building from source) and configure a ccache directory that can be persisted across builds.

Key changes:

  • Add cmake to the list of apt packages to avoid building it from source during the buildroot process
  • Configure a custom ccache directory at /var/cache/buildroot/ccache and declare it as a Docker volume for potential persistence across builds

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

# the default ccache dir is "$HOME/.buildroot-ccache"
ENV BR_CACHE_DIR=/var/cache/buildroot/ccache
RUN mkdir -p /var/cache/buildroot/ccache && chmod 1777 /var/cache/buildroot/ccache
VOLUME ["/var/cache/buildroot"]
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The VOLUME declaration will create an anonymous volume that won't persist between builds by default. To make the ccache persist and provide the performance benefits intended by this PR, the Makefile's docker run commands (lines 338 and 344) should mount a named volume or host directory to /var/cache/buildroot. Without this, each build will start with an empty ccache, negating the performance benefit. Consider either: 1) Adding an explicit volume mount in the Makefile's docker run commands, or 2) Documenting that users should set ISO_DOCKER_EXTRA_ARGS="-v minikube-buildroot-cache:/var/cache/buildroot" to benefit from ccache persistence.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While true, this can come in a later PR

@afbjorklund afbjorklund marked this pull request as draft December 18, 2025 17:39
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 18, 2025
@afbjorklund
Copy link
Collaborator Author

afbjorklund commented Dec 18, 2025

I double-checked, and both variables will have the same end result. Not sure why it was failing before?

The problem was that it was prepending $HOME to the directory, so /tmp/var/cache/buildroot/ccache

Seems to be working now:

I have no name!@ff4e04d005f8:/mnt$ out/buildroot/output-x86_64/host/bin/ccache -sv
Cache directory:    /var/cache/buildroot/ccache
Config file:        /var/cache/buildroot/ccache/ccache.conf
System config file: /mnt/out/buildroot/output-x86_64/host/etc/ccache.conf

And apt install cmake to avoid host-cmake
@afbjorklund afbjorklund marked this pull request as ready for review December 18, 2025 17:44
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 18, 2025
@k8s-ci-robot k8s-ci-robot requested a review from nirs December 18, 2025 17:44
@k8s-ci-robot
Copy link
Contributor

@afbjorklund: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
integration-vfkit-docker-macos-arm64 c704dc1 link false /test integration-vfkit-docker-macos-arm64
integration-docker-crio-linux-x86-64 c704dc1 link true /test integration-docker-crio-linux-x86-64
integration-kvm-docker-linux-x86-64 c704dc1 link true /test integration-kvm-docker-linux-x86-64
integration-kvm-crio-linux-x86-64 c704dc1 link true /test integration-kvm-crio-linux-x86-64
integration-kvm-docker-linux-x86 c704dc1 link true /test integration-kvm-docker-linux-x86
integration-docker-docker-linux-arm c704dc1 link true /test integration-docker-docker-linux-arm
integration-kvm-crio-linux-x86 c704dc1 link true /test integration-kvm-crio-linux-x86
integration-docker-docker-linux-x86 c704dc1 link true /test integration-docker-docker-linux-x86
integration-docker-containerd-linux-x86 c704dc1 link true /test integration-docker-containerd-linux-x86
integration-docker-crio-linux-x86 c704dc1 link true /test integration-docker-crio-linux-x86
integration-kvm-containerd-linux-x86 c704dc1 link true /test integration-kvm-containerd-linux-x86
integration-none-docker-linux-x86 c704dc1 link true /test integration-none-docker-linux-x86

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

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

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants