Kicbase/ISO: Update buildroot from 2023.02.9 to 2025.2 #20720

ComradeProgrammer · 2025-05-08T21:15:26Z

No description provided.

ComradeProgrammer · 2025-05-08T21:15:46Z

/ok-to-build-image

ComradeProgrammer · 2025-05-08T21:15:57Z

/ok-to-build-iso

ComradeProgrammer · 2025-05-08T21:16:11Z

/ok-to-test

ComradeProgrammer · 2025-05-08T21:16:51Z

It works locally on my own machine. Let's see if it can build ISO successfully on CI

minikube-bot · 2025-05-09T04:45:58Z

Hi @ComradeProgrammer, we have updated your PR with the reference to newly built ISO. Pull the changes locally if you want to test with them or update your PR further.

nirs · 2025-05-09T22:03:46Z

Makefile

 # the go version on the line below is for the ISO
-GOLANG_OPTIONS = GO_VERSION=1.21.6 GO_HASH_FILE=$(PWD)/deploy/iso/minikube-iso/go.hash
+GOLANG_OPTIONS = GO_VERSION=1.23.4 GO_HASH_FILE=$(PWD)/deploy/iso/minikube-iso/go.hash


Why 1.23.4? Latest is 1.23.8.

we currently use GO_VERSION 1.24.0 in this Makefile (set above), what is the reason for overriding it here?

i also noticed that our go.mod still uses 1.23.4 though, not sure if we're blocked on bumping it as well and then have all go versions in sync

nirs · 2025-05-09T22:04:18Z

deploy/iso/minikube-iso/arch/aarch64/package/cri-dockerd-aarch64/cri-dockerd.mk

+	GOARCH=arm64 \
+	GOPROXY="https://proxy.golang.org,direct" \
+	GOSUMDB='sum.golang.org'\
+	GOOS=linux


Why the new options are needed?

For newer version of go, if we don't set those options, the build will fail.
E.g. after go 1.21 GOPROXY no longer tolerates an empty string when GOMOD111 is turned on
See golang/go#61928 (comment)

based on Mills' comment in the linked issue, looks like if we're building go v1.21.0+ from source we also need to create the $GOROOT/go.env that sets GOPROXY and GOSUMDB, and we probably do not need to implicitly set the GOARCH and GOOS (those should be automatically inferred) so these additional changes would not be needed - here and in few other places/files below?

the additional reason to avoid setting these manually in several places would be easier maintenance - we'd avoid failing because we have not manually added it to all other places that might need it and the relevant default values should be taken from the go release itself

nirs · 2025-05-09T22:05:20Z

deploy/iso/minikube-iso/arch/aarch64/package/docker-buildx-aarch64/docker-buildx.mk

+        GOARCH=arm64 \
+	GOPROXY="https://proxy.golang.org,direct" \
+	GOSUMDB='sum.golang.org'\
+	GOOS=linux


Unify indent?

I think the intents here are already the same with other go envs?

i think that Nir meant that we should have same alignment (ie, same number of blanks before these last three lines as lines above), but see my previous comment about avoiding adding it altogether

deploy/iso/minikube-iso/arch/x86_64/package/docker-buildx/docker-buildx.mk

deploy/iso/minikube-iso/package/Config.in

nirs · 2025-05-09T22:06:58Z

deploy/iso/minikube-iso/package/crio-bin/crio.conf

@@ -102,7 +102,7 @@ decryption_keys_path = "/etc/crio/keys/"

 # Path to the conmon binary, used for monitoring the OCI runtime.
 # Will be searched for using $PATH if empty.
-conmon = "/usr/libexec/crio/conmon"
+conmon = "/usr/bin/conmon"


The path was changed?

Yes. New buildroot includes conmon by default, and the new path is this

nit: we could also update the TestDockerSystemInfo to reflect the new conmon path

deploy/iso/minikube-iso/package/falco-module/falco-module.mk

ComradeProgrammer · 2025-05-09T23:39:17Z

/ok-to-test

ComradeProgrammer · 2025-05-10T15:52:40Z

/ok-to-test

nirs · 2025-05-10T15:58:59Z

pkg/minikube/sysinit/systemd.go

+
+// run systemctl reset-failed for a service
+// some services declare a realitive small restart-limit in their .service configuration
+// so we reset reset-failed counter to override the limit


Can we change the service configuration instead? It will avoid the fake ResetFailed interface we add here.

Good idea. I guess it is possible, but perhaps we can do this in next PR. Currently buildroot issues are blocking us from building ISO and we cannot update crio, containerd or anything else which involves go>=1.22. It is a rather urgent one

The .service file from cir-dockerd is this cri-docker.service
where it declares StartLimitBurst=3 StartLimitInterval=60s . I am not sure but I guess this is the problem, because journalctl -u cri-docker.service always shows cri-docker.service: Start request repeated too quickly..

I guess it may also work if we remove these two lines from cri-containerd via go code

Why do we have a burst of start requests?

If the systemd unit is defined properly, system will start the service when dependent service are ready and we should not see such issue.

I guess we install the services dynamically when creating the machine (since we don't know at build time which container runtime will be used). And we probably start them manually without considering the dependencies between services, and then retry failed services?

And we probably start them manually without considering the dependencies between services, and then retry failed services?

I agree. For this issue specifically, accroding to the log here my guessing is that: somehow when we try to start cri-dockerd, the docker daemon/socket is not ready.

However I did tried to wait for docker service/socket with r.Init.Active("docker") before restarting cri-containerd, but it doesn't work at all. r.Init.Active("docker") return true while cri-containerd continue to complain that Cannot connect to the Docker daemon at unix:///var/run/....

So I just came up with this temporary brute-force solution, forcing restart of all those services, and it works. This is definitely not a good idea, I think we should continue to investigate it and see what we can do to actually solve this.

However I did tried to wait for docker service/socket with r.Init.Active("docker") before restarting cri-containerd, but it doesn't work at all. r.Init.Active("docker") return true while cri-containerd continue to complain that Cannot connect to the Docker daemon at unix:///var/run/....

is-active is not documented to return true when the service is ready:

is-active PATTERN... Check whether any of the specified units are active (i.e. running). Returns an exit code 0 if at least one is active, or non-zero otherwise. Unless --quiet is specified, this will also print the current unit state to standard output.

For this issue specifically, accroding to the log here my guessing is that: somehow when we try to start cri-dockerd, the docker daemon/socket is not ready.

This happens here?

minikube/pkg/minikube/cruntime/docker.go

Line 174 in 8575bee

// restart cri-docker

yes, this is exactly the place where it happened.

this behaviour is strange/unexpected i think: we have Type=notify set and NotifyAccess not set, which should mean that the service (ie, its main process) will send the READY=1 signal only when actually "ready", and that should be picked up by the is-active - not sure why it would not work in our setup

ref: https://www.freedesktop.org/software/systemd/man/latest/systemd.service.html

ComradeProgrammer · 2025-05-10T16:33:54Z

/ok-to-test

ComradeProgrammer · 2025-05-10T16:34:01Z

/ok-to-build-iso

ComradeProgrammer · 2025-05-10T16:34:14Z

/ok-to-build-image

nirs · 2025-05-10T21:50:21Z

I tested this on Fedora 42 with kvm driver and containerd runtime. My test creates 3 single node clusters, deploy ocm, olm, rook-ceph, submariner, velero, volsync, minio, and ramen and run end to end disaster recovery tests.

Looking at the first failures in KVM_linux_containerd - we have many of these:

	  Warning  Failed     5m14s (x3 over 8m6s)    kubelet            Failed to pull image "busybox:stable": failed to pull and unpack image "docker.io/library/busybox:stable": failed to copy: httpReadSeeker: failed open: unexpected status code https://registry-1.docker.io/v2/library/busybox/manifests/sha256:e246aa22ad2cbdfbd19e2a6ca2b275e26245a21920e2b2d0666324cee3f15549: 429 Too Many Requests - Server message: toomanyrequests: You have reached your unauthenticated pull rate limit. https://www.docker.com/increase-rate-limit

We don't pull any image from docker.io to avoid these failures.

nirs · 2025-05-10T21:59:20Z

Makefile

@@ -47,7 +47,7 @@ KVM_GO_VERSION ?= $(GO_VERSION:.0=)


 INSTALL_SIZE ?= $(shell du out/minikube-windows-amd64.exe | cut -f1)
-BUILDROOT_BRANCH ?= 2024.11.2
+BUILDROOT_BRANCH ?= 2025.02


Why 2024.11.2 (what we used for other parts) is not right here?

Well it is still about falco-modules(but it is not working, as you can see)

It seems that after we update the buildroot, buildroot will append the FALCO_MODULE_INSTALL_STAGING_OPTS and the FALCO_MODULE_INSTALL_TARGET_OPTS to cmake command, which causes an iso build failure. I don't know why this happens, so I tried some other versions. But it still cannot work. All those weird changes about falco-modules (for which I didn't give any reason why I made them) are basically all for falco-modules

I guess I will remove falco-modules for now, which should work, I guess

BTW I think we should still keep using 2025.02 because i found that in buildroot, 20xx.xx are version numbers for long term supports, while 20xx.xx.xx is not. If we want to update the buildroot, I think maybe LTS version is better

LTS sounds good but we don't have to do this change now.

uhh probably you misunderstood this change? The current buildroot version in main branch is BUILDROOT_BRANCH ?= 2023.02.9. In this PR I chosen 2024.11.2 at the beginning but then I submitted another commit which change it to 2025.2

Since we have to update this buildroot in this PR, why should't we update it to the latest LTS version?

The pr message says "Update buildroot from 2023.02.9 to 2024.11.2"

I guess I was also confused by the commit message: "fix falco-modules" - I assumed you update falco version. I see now that this updates BUILDROOT_BRANCH.

ComradeProgrammer · 2025-05-10T22:33:08Z

/ok-to-build-iso

/ok-to-test

prezha

thanks for the efforts you've put into this @ComradeProgrammer!
please have a look at few comments i left

prezha · 2025-05-11T21:13:00Z

Makefile

 # the go version on the line below is for the ISO
-GOLANG_OPTIONS = GO_VERSION=1.21.6 GO_HASH_FILE=$(PWD)/deploy/iso/minikube-iso/go.hash
+GOLANG_OPTIONS = GO_VERSION=1.23.4 GO_HASH_FILE=$(PWD)/deploy/iso/minikube-iso/go.hash


we currently use GO_VERSION 1.24.0 in this Makefile (set above), what is the reason for overriding it here?

i also noticed that our go.mod still uses 1.23.4 though, not sure if we're blocked on bumping it as well and then have all go versions in sync

prezha · 2025-05-11T22:54:52Z

deploy/iso/minikube-iso/go.hash

@@ -35,3 +35,4 @@ sha256  36930162a93df417d90bd22c6e14daff4705baac2b02418edda671cdfa9cd07f  go1.23
 sha256  8d6a77332487557c6afa2421131b50f83db4ae3c579c3bc72e670ee1f6968599  go1.23.3.src.tar.gz
 sha256  ad345ac421e90814293a9699cca19dd5238251c3f687980bbcae28495b263531  go1.23.4.src.tar.gz
 sha256  d14120614acb29d12bcab72bd689f257eb4be9e0b6f88a8fb7e41ac65f8556e5  go1.24.0.src.tar.gz
+sha256 6924efde5de86fe277676e929dc9917d466efa02fb934197bc2eba35d5680971 go1.23.4.linux-amd64.tar.gz


i think that the go.hash file should not be updated manually - should be managed by the updateGoHashFile func in hack/update/golang_version/update_golang_version.go

prezha · 2025-05-11T23:01:15Z

deploy/iso/minikube-iso/arch/aarch64/package/cri-dockerd-aarch64/cri-dockerd.mk

+	GOARCH=arm64 \
+	GOPROXY="https://proxy.golang.org,direct" \
+	GOSUMDB='sum.golang.org'\
+	GOOS=linux


based on Mills' comment in the linked issue, looks like if we're building go v1.21.0+ from source we also need to create the $GOROOT/go.env that sets GOPROXY and GOSUMDB, and we probably do not need to implicitly set the GOARCH and GOOS (those should be automatically inferred) so these additional changes would not be needed - here and in few other places/files below?

the additional reason to avoid setting these manually in several places would be easier maintenance - we'd avoid failing because we have not manually added it to all other places that might need it and the relevant default values should be taken from the go release itself

prezha · 2025-05-11T23:04:09Z

deploy/iso/minikube-iso/arch/aarch64/package/docker-buildx-aarch64/docker-buildx.mk

+        GOARCH=arm64 \
+	GOPROXY="https://proxy.golang.org,direct" \
+	GOSUMDB='sum.golang.org'\
+	GOOS=linux


i think that Nir meant that we should have same alignment (ie, same number of blanks before these last three lines as lines above), but see my previous comment about avoiding adding it altogether

prezha · 2025-05-11T23:12:44Z

deploy/iso/minikube-iso/package/crio-bin/crio.conf

@@ -102,7 +102,7 @@ decryption_keys_path = "/etc/crio/keys/"

 # Path to the conmon binary, used for monitoring the OCI runtime.
 # Will be searched for using $PATH if empty.
-conmon = "/usr/libexec/crio/conmon"
+conmon = "/usr/bin/conmon"


nit: we could also update the TestDockerSystemInfo to reflect the new conmon path

pkg/minikube/cruntime/docker.go

prezha · 2025-05-12T00:06:29Z

pkg/minikube/sysinit/systemd.go

+
+// run systemctl reset-failed for a service
+// some services declare a realitive small restart-limit in their .service configuration
+// so we reset reset-failed counter to override the limit


this behaviour is strange/unexpected i think: we have Type=notify set and NotifyAccess not set, which should mean that the service (ie, its main process) will send the READY=1 signal only when actually "ready", and that should be picked up by the is-active - not sure why it would not work in our setup

ref: https://www.freedesktop.org/software/systemd/man/latest/systemd.service.html

ComradeProgrammer · 2025-05-12T18:57:18Z

/ok-to-test

medyagh · 2025-05-12T23:53:01Z

/ok-to-test

minikube-pr-bot · 2025-05-13T00:43:59Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 20720) |
+----------------+----------+---------------------+
| minikube start | 54.5s    | 54.0s               |
| enable ingress | 18.8s    | 17.8s               |
+----------------+----------+---------------------+

Times for minikube start: 52.5s 53.9s 52.9s 56.8s 56.7s
Times for minikube (PR 20720) start: 57.5s 54.6s 52.5s 52.2s 53.3s

Times for minikube ingress: 19.2s 20.1s 15.6s 20.1s 19.1s
Times for minikube (PR 20720) ingress: 18.6s 15.6s 16.2s 20.1s 18.6s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 20720) |
+----------------+----------+---------------------+
| minikube start | 25.8s    | 24.9s               |
| enable ingress | 13.3s    | 13.0s               |
+----------------+----------+---------------------+

Times for minikube start: 26.7s 25.4s 26.8s 24.8s 25.3s
Times for minikube (PR 20720) start: 24.0s 23.5s 26.4s 23.6s 27.2s

Times for minikube (PR 20720) ingress: 12.4s 12.9s 13.4s 13.9s 12.4s
Times for minikube ingress: 13.4s 13.4s 13.4s 12.9s 13.4s

docker driver with containerd runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 20720) |
+----------------+----------+---------------------+
| minikube start | 24.2s    | 22.5s               |
| enable ingress | 30.5s    | 29.5s               |
+----------------+----------+---------------------+

Times for minikube start: 25.8s 23.1s 24.8s 23.4s 23.5s
Times for minikube (PR 20720) start: 22.2s 22.6s 23.9s 21.5s 22.2s

Times for minikube ingress: 22.9s 22.9s 28.9s 38.9s 38.9s
Times for minikube (PR 20720) ingress: 38.9s 39.9s 22.9s 22.9s 22.9s

medyagh · 2025-05-13T17:07:28Z

/lgtm

k8s-ci-robot · 2025-05-13T17:07:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ComradeProgrammer, medyagh

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:

~~OWNERS~~ [ComradeProgrammer,medyagh]

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

medyagh · 2025-05-13T17:07:59Z

thank you @ComradeProgrammer for fixing this ISO issue, this was a blocker for minikube release and you did an amazing work thanks

Kicbase/ISO: Update buildroot from 2023.02.9 to 2024.11.2

f0a5cfa

ComradeProgrammer requested a review from medyagh May 8, 2025 21:15

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 8, 2025

k8s-ci-robot requested a review from prezha May 8, 2025 21:15

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 8, 2025

k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label May 8, 2025

ComradeProgrammer self-assigned this May 8, 2025