Update BuildKit configurations to enhance compatibility #5937

XinShuYang · 2024-01-29T03:27:05Z

Add BuildKit cache cleanup in CI
Declare DOCKER_BUILDKIT when building image for docker compatibility.

fixes #5941

XinShuYang · 2024-01-30T02:42:41Z

/test-all

XinShuYang · 2024-01-30T04:51:05Z

/test-all

luolanzone · 2024-01-30T06:05:11Z

ci/test-conformance-eks.sh

+    free_space=$(df -h -B 1G / | awk 'NR==2 {print $4}')
+    free_space_threshold=40
+    if [[ $free_space -lt $free_space_threshold ]]; then
+        # If cleaning up dangling images unused in the last hour doesn't free up sufficient disk space,
+        # we will have to clean up all buildkit cache to release enough disk space.
+        docker buildx prune -af > /dev/null
+    fi
+    docker buildx du
    set -e


There are code duplication, I feel it's better to move them into a shell file, maybe docker-utils.sh?

Added it to utils.sh.

luolanzone

one question about the cache cleanup.

luolanzone · 2024-01-30T07:47:58Z

ci/jenkins/utils.sh

+        # If cleaning up dangling images unused in the last hour doesn't free up sufficient disk space,
+        # we will have to clean up all buildkit cache to release enough disk space.


Do we really need to cleanup all cache? I think cache can still help to speed up the image building. I saw there is an option --keep-storage, could you check if it's useful in this case? Thanks.

@luolanzone We can use this parameter to reduce the cache to 10GB during the first round cleanup. If the storage space is still insufficient, we will have to clean up all build cache.

tnqn

Not sure if this is the right way to fix it. @antoninbas may be more familiar with this.

antoninbas · 2024-01-30T20:53:51Z

ci/jenkins/test-mc.sh

@@ -296,7 +297,7 @@ function deliver_antrea_multicluster {
    chmod -R g-w build/images/ovs
    chmod -R g-w build/images/base

-    DOCKER_REGISTRY="${DOCKER_REGISTRY}" ./hack/build-antrea-linux-all.sh --pull
+    DOCKER_REGISTRY="${DOCKER_REGISTRY}" DOCKER_BUILDKIT=1 ./hack/build-antrea-linux-all.sh --pull


Given that ./hack/build-antrea-linux-all.sh assumes that BuildKit is enabled, we could just set DOCKER_BUILDKIT=1 in the script directly, to avoid all these individual changes?

BTW, I have been thinking of using docker buildx build directly as I believe it offers more command-line options (and better caching support), so it would make sense to update Docker everywhere.

BTW, I have been thinking of using docker buildx build directly as I believe it offers more command-line options (and better caching support), so it would make sense to update Docker everywhere.

Can I add buildx support in a seperate PR once I have upgraded docker on all testbeds? I can include this target in the issue of #5937 (comment).

antoninbas · 2024-01-30T20:55:11Z

ci/jenkins/utils.sh

+function check_and_cleanup_buildkit_cache() {
+    free_space=$(df -h -B 1G / | awk 'NR==2 {print $4}')
+    free_space_threshold=40
+    if [[ $free_space -lt $free_space_threshold ]]; then
+        # If cleaning up unused dangling images doesn't free up sufficient disk space,
+        # we will have to reduce the buildkit cache to 10GB to release enough disk space.
+        docker buildx prune -af --keep-storage=10gb > /dev/null
+        free_space=$(df -h -B 1G / | awk 'NR==2 {print $4}')
+        if [[ $free_space -lt $free_space_threshold ]]; then
+            # If the first round cleanup doesn't free up sufficient disk space,
+            # we will have to clean up all buildkit cache to release enough disk space.
+            docker buildx prune -af > /dev/null
+        fi
+    fi
+    docker buildx du
+}


I thought we considered some logic like this previously but decided not to use it? Am I misremembering?

For the previous code changes, we don't want to remove all images because doing so may delete necessary images for other concurrent jobs on the same testbed. However, for the BuildKit cache, it can be safely deleted without encountering any build image errors. The only difference is the building speed.

So this is because there is no equivalent command to docker buildx prune in legacy docker? Is docker builder prune different from this one?

I wonder if we should just unconditionally run docker buildx prune -af --keep-storage=10gb instead of checking for free space first?

So this is because there is no equivalent command to docker buildx prune in legacy docker? Is docker builder prune different from this one?
I tried both commands and docker builder prune can achieve the same result of docker buildx prune.

I wonder if we should just unconditionally run docker buildx prune -af --keep-storage=10gb instead of checking for free space first?

Updated, I am okay with both methods.

I meant only using docker builder prune -af --keep-storage=10gb, and removing the rest of the logic. However, I am fine with the current approach. Let's see if it eliminates our space issues.

antoninbas · 2024-01-30T20:55:46Z

ci/jenkins/test-mc.sh

    docker images | grep -E 'mc-controller|antrea-ubuntu' | awk '{print $3}' | xargs -r docker rmi -f || true
    # Clean up dangling images generated in previous builds.
    docker image prune -f --filter "until=24h" || true > /dev/null
+    check_and_cleanup_buildkit_cache


should we upgrade Docker on all build machines, and avoid the 2 cases (legacy docker / buildkit)?

Hi @antoninbas, since we are working on migrating cloud testbeds(some of which can only be accessed by jenkins now), I tend to recover jenkins jobs first. I will create a separate PR to remove all legacy commands once I have upgraded docker on all testbeds. If you agree I can create an issue to track it.

Sounds good

XinShuYang · 2024-01-31T10:36:46Z

/test-all
/test-windows-all

antoninbas · 2024-01-31T19:02:09Z

ci/jenkins/utils.sh

+    if [[ $free_space -lt $free_space_threshold ]]; then
+        # If cleaning up unused dangling images doesn't free up sufficient disk space,
+        # we will have to reduce the buildkit cache to 10GB to release enough disk space.
+        docker buildx prune -af --keep-storage=10gb > /dev/null


so all Jenkins workers can already run docker buildx?

No it doesn't work with some legacy docker versions, the command failure won't block the CI pipeline. However, given that docker builder prune can achieve the same function and can be supported on all docker releases, I have updated it for better compatibility.

antoninbas · 2024-02-01T00:31:53Z

ci/jenkins/utils.sh

@@ -14,6 +14,19 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.

+function check_and_cleanup_buildkit_cache() {


should we replace buildkit_cache with docker_build_cache in the function name?

Yes, updated.

antoninbas · 2024-02-01T00:32:26Z

ci/jenkins/utils.sh

+function check_and_cleanup_buildkit_cache() {
+    free_space_threshold=40
+
+    docker builder prune -af --keep-storage=10gb > /dev/null


I hope that in the general case this is enough and we can keep some cache around to speed up future builds

I feel reducing cache space doesn't guarantee the effectiveness of the latest used cache. I tend to continuing to use LRU method based on the timer filter. What do you think?

maybe we should start with the size one actually. I imagine that the implementation is "smart" enough to delete old objects first. Otherwise, if we don't trigger a job for 1h, we lose all the cache and it becomes useless.
if you prefer, you can go back to your first version:

if free_space < threshold docker builder prune -af --keep-storage=10gb > /dev/null if free_space < threshold docker builder prune -af > /dev/null

I think the size filter is a better fit for us (for the build cache)

Sure, I am okay with it.

antoninbas · 2024-02-01T00:39:27Z

docs/minikube.md

@@ -26,7 +26,7 @@ minikube start --cni=antrea.yml --network-plugin=cni

 These instructions assume that you have built the Antrea Docker image locally
 (e.g. by running `make` from the root of the repository, or in case of arm64 architecture by running
-`DOCKER_BUILDKIT=1 ./hack/build-antrea-ubuntu-all.sh --platform linux/arm64`).
+`./hack/build-antrea-ubuntu-all.sh --platform linux/arm64`).


since you are modifying this line, the current name of the script is ./hack/build-antrea-linux-all.sh, it's not ./hack/build-antrea-ubuntu-all.sh anymore :)

Updated, thanks:)

antoninbas · 2024-02-01T00:50:51Z

ci/jenkins/utils.sh

@@ -14,6 +14,18 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.

+function check_and_cleanup_docker_build_cache() {
+    docker builder prune -af --filter="until=1h" > /dev/null


you changed the filter from size-based to time-based?

Replied in #5937 (comment)

* Add BuildKit cache cleanup in CI * Declare DOCKER_BUILDKIT when building image for docker compatibility. fixes antrea-io#5941 Signed-off-by: Shuyang Xin <gavinx@vmware.com>

luolanzone

LGTM

antoninbas · 2024-02-01T18:18:58Z

/test-all

* Add BuildKit cache cleanup in CI * Declare DOCKER_BUILDKIT when building image for docker compatibility. fixes antrea-io#5941 Signed-off-by: Shuyang Xin <gavinx@vmware.com>

* Add BuildKit cache cleanup in CI * Declare DOCKER_BUILDKIT when building image for docker compatibility. fixes #5941 Signed-off-by: Shuyang Xin <gavinx@vmware.com>

XinShuYang force-pushed the buildkit branch 3 times, most recently from e5ab0ee to 6669b9b Compare January 30, 2024 02:14

XinShuYang marked this pull request as ready for review January 30, 2024 02:16

XinShuYang requested a review from tnqn January 30, 2024 02:19

XinShuYang force-pushed the buildkit branch from 6669b9b to 8eda895 Compare January 30, 2024 02:42

XinShuYang force-pushed the buildkit branch from 8eda895 to 7970031 Compare January 30, 2024 04:50

luolanzone reviewed Jan 30, 2024

View reviewed changes

XinShuYang force-pushed the buildkit branch 2 times, most recently from 66e47d9 to 0d51aef Compare January 30, 2024 06:24

XinShuYang requested a review from luolanzone January 30, 2024 06:26

luolanzone reviewed Jan 30, 2024

View reviewed changes

XinShuYang force-pushed the buildkit branch 4 times, most recently from 47fba78 to 8709a85 Compare January 30, 2024 08:42

tnqn reviewed Jan 30, 2024

View reviewed changes

antoninbas reviewed Jan 30, 2024

View reviewed changes

XinShuYang force-pushed the buildkit branch 3 times, most recently from 6ef606f to 7a21195 Compare January 31, 2024 06:18

antoninbas reviewed Jan 31, 2024

View reviewed changes

XinShuYang force-pushed the buildkit branch from 7a21195 to 37146df Compare February 1, 2024 00:22

antoninbas reviewed Feb 1, 2024

View reviewed changes

XinShuYang force-pushed the buildkit branch from 37146df to 9472a59 Compare February 1, 2024 00:33

XinShuYang requested a review from antoninbas February 1, 2024 00:33

XinShuYang force-pushed the buildkit branch from 9472a59 to c9bdfbe Compare February 1, 2024 00:36

antoninbas previously approved these changes Feb 1, 2024

View reviewed changes

XinShuYang dismissed antoninbas’s stale review via 9d3736f February 1, 2024 00:43

XinShuYang force-pushed the buildkit branch 2 times, most recently from 9d3736f to 33dabc9 Compare February 1, 2024 00:46

antoninbas reviewed Feb 1, 2024

View reviewed changes

XinShuYang force-pushed the buildkit branch from 33dabc9 to c0a3ff0 Compare February 1, 2024 01:09

Update BuildKit Configurations to Enhance Compatibility

44ead5e

* Add BuildKit cache cleanup in CI * Declare DOCKER_BUILDKIT when building image for docker compatibility. fixes antrea-io#5941 Signed-off-by: Shuyang Xin <gavinx@vmware.com>

XinShuYang force-pushed the buildkit branch from c0a3ff0 to 44ead5e Compare February 1, 2024 01:13

antoninbas approved these changes Feb 1, 2024

View reviewed changes

luolanzone approved these changes Feb 1, 2024

View reviewed changes

tnqn merged commit 075efef into antrea-io:main Feb 2, 2024
48 of 53 checks passed

XinShuYang mentioned this pull request Feb 17, 2024

Use docker buildx to build base Linux images #5975

Merged

luolanzone mentioned this pull request Jun 17, 2024

Automated cherry pick of #5937: Update BuildKit Configurations to Enhance Compatibility #6450

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update BuildKit configurations to enhance compatibility #5937

Update BuildKit configurations to enhance compatibility #5937

XinShuYang commented Jan 29, 2024 •

edited

Loading

XinShuYang commented Jan 30, 2024

XinShuYang commented Jan 30, 2024

luolanzone Jan 30, 2024

XinShuYang Jan 30, 2024

luolanzone left a comment

luolanzone Jan 30, 2024

XinShuYang Jan 30, 2024

tnqn left a comment

antoninbas Jan 30, 2024

antoninbas Jan 30, 2024

XinShuYang Jan 31, 2024

antoninbas Jan 30, 2024

XinShuYang Jan 31, 2024

antoninbas Jan 31, 2024

XinShuYang Feb 1, 2024

antoninbas Feb 1, 2024

antoninbas Jan 30, 2024

XinShuYang Jan 31, 2024

antoninbas Jan 31, 2024

XinShuYang commented Jan 31, 2024

antoninbas Jan 31, 2024

XinShuYang Feb 1, 2024

antoninbas Feb 1, 2024

XinShuYang Feb 1, 2024

antoninbas Feb 1, 2024

XinShuYang Feb 1, 2024

antoninbas Feb 1, 2024

XinShuYang Feb 1, 2024

antoninbas Feb 1, 2024

XinShuYang Feb 1, 2024

antoninbas Feb 1, 2024

XinShuYang Feb 1, 2024

luolanzone left a comment

antoninbas commented Feb 1, 2024

		# If cleaning up dangling images unused in the last hour doesn't free up sufficient disk space,
		# we will have to clean up all buildkit cache to release enough disk space.

Update BuildKit configurations to enhance compatibility #5937

Update BuildKit configurations to enhance compatibility #5937

Conversation

XinShuYang commented Jan 29, 2024 • edited Loading

XinShuYang commented Jan 30, 2024

XinShuYang commented Jan 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luolanzone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XinShuYang commented Jan 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luolanzone left a comment

Choose a reason for hiding this comment

antoninbas commented Feb 1, 2024

XinShuYang commented Jan 29, 2024 •

edited

Loading