Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
May 13, 2025
Merged

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

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f0a5cfa
Kicbase/ISO: Update buildroot from 2023.02.9 to 2024.11.2
ComradeProgrammer May 8, 2025
ff5fb39
Updating ISO to v1.35.0-1746739450-20720
minikube-bot May 9, 2025
5bb96d0
fix restart
ComradeProgrammer May 9, 2025
4f6ad36
fix kvm containerd
ComradeProgrammer May 10, 2025
b7440dc
fix falco-modules
ComradeProgrammer May 10, 2025
4ef9e51
remove falco for good
ComradeProgrammer May 10, 2025
df8794e
use expo retry
ComradeProgrammer May 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ KIC_VERSION ?= $(shell grep -E "Version =" pkg/drivers/kic/types.go | cut -d \"
HUGO_VERSION ?= $(shell grep -E "HUGO_VERSION = \"" netlify.toml | cut -d \" -f2)

# Default to .0 for higher cache hit rates, as build increments typically don't require new ISO versions
ISO_VERSION ?= v1.35.0
ISO_VERSION ?= v1.35.0-1746739450-20720

# Dashes are valid in semver, but not Linux packaging. Use ~ to delimit alpha/beta
DEB_VERSION ?= $(subst -,~,$(RAW_VERSION))
Expand All @@ -47,9 +47,9 @@ KVM_GO_VERSION ?= $(GO_VERSION:.0=)


INSTALL_SIZE ?= $(shell du out/minikube-windows-amd64.exe | cut -f1)
BUILDROOT_BRANCH ?= 2023.02.9
BUILDROOT_BRANCH ?= 2025.02
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

@ComradeProgrammer ComradeProgrammer May 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

@ComradeProgrammer ComradeProgrammer May 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

# 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
Copy link
Contributor

Choose a reason for hiding this comment

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

Why 1.23.4? Latest is 1.23.8.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

BUILDROOT_OPTIONS = BR2_EXTERNAL=../../deploy/iso/minikube-iso $(GOLANG_OPTIONS)
REGISTRY ?= gcr.io/k8s-minikube

Expand Down
5 changes: 4 additions & 1 deletion deploy/iso/minikube-iso/arch/aarch64/package/cri-dockerd-aarch64/cri-dockerd.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ CRI_DOCKERD_AARCH64_ENV = \
GO111MODULE=on \
GOPATH="$(CRI_DOCKERD_AARCH64_GOPATH)" \
PATH=$(CRI_DOCKERD_AARCH64_GOPATH)/bin:$(BR_PATH) \
GOARCH=arm64
GOARCH=arm64 \
GOPROXY="https://proxy.golang.org,direct" \
GOSUMDB='sum.golang.org'\
GOOS=linux
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the new options are needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

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)

Copy link
Contributor

Choose a reason for hiding this comment

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

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


CRI_DOCKERD_AARCH64_COMPILE_SRC = $(CRI_DOCKERD_AARCH64_GOPATH)/src/github.com/Mirantis/cri-dockerd
CRI_DOCKERD_AARCH64_BUILDFLAGS = "-ldflags '-X github.com/Mirantis/cri-dockerd/version.Version=$(CRI_DOCKERD_AARCH64_VER) -X github.com/Mirantis/cri-dockerd/version.GitCommit=$(CRI_DOCKERD_AARCH64_REV)'"
Expand Down
5 changes: 4 additions & 1 deletion deploy/iso/minikube-iso/arch/aarch64/package/docker-buildx-aarch64/docker-buildx.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ DOCKER_BUILDX_AARCH64_ENV = \
GOPATH="$(DOCKER_BUILDX_AARCH64_GOPATH)" \
GOBIN="$(DOCKER_BUILDX_AARCH64_GOPATH)/bin" \
PATH=$(DOCKER_BUILDX_AARCH64_GOPATH)/bin:$(BR_PATH) \
GOARCH=arm64
GOARCH=arm64 \
GOPROXY="https://proxy.golang.org,direct" \
GOSUMDB='sum.golang.org'\
GOOS=linux
Copy link
Contributor

Choose a reason for hiding this comment

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

Unify indent?

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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


DOCKER_BUILDX_AARCH64_COMPILE_SRC = $(DOCKER_BUILDX_AARCH64_GOPATH)/src/github.com/docker/buildx

Expand Down
5 changes: 4 additions & 1 deletion deploy/iso/minikube-iso/arch/x86_64/package/cri-dockerd/cri-dockerd.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ CRI_DOCKERD_ENV = \
GO111MODULE=on \
GOPATH="$(CRI_DOCKERD_GOPATH)" \
PATH=$(CRI_DOCKERD_GOPATH)/bin:$(BR_PATH) \
GOARCH=amd64
GOARCH=amd64 \
GOPROXY="https://proxy.golang.org,direct" \
GOSUMDB='sum.golang.org'\
GOOS=linux

CRI_DOCKERD_COMPILE_SRC = $(CRI_DOCKERD_GOPATH)/src/github.com/Mirantis/cri-dockerd
CRI_DOCKERD_BUILDFLAGS = "-ldflags '-X github.com/Mirantis/cri-dockerd/version.Version=$(CRI_DOCKERD_VER) -X github.com/Mirantis/cri-dockerd/version.GitCommit=$(CRI_DOCKERD_REV)'"
Expand Down
5 changes: 4 additions & 1 deletion deploy/iso/minikube-iso/arch/x86_64/package/docker-buildx/docker-buildx.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ DOCKER_BUILDX_ENV = \
GOPATH="$(DOCKER_BUILDX_GOPATH)" \
GOBIN="$(DOCKER_BUILDX_GOPATH)/bin" \
PATH=$(DOCKER_BUILDX_GOPATH)/bin:$(BR_PATH) \
GOARCH=amd64
GOARCH=amd64 \
GOPROXY="https://proxy.golang.org,direct" \
GOSUMDB='sum.golang.org'\
GOOS=linux

DOCKER_BUILDX_COMPILE_SRC = $(DOCKER_BUILDX_GOPATH)/src/github.com/docker/buildx

Expand Down
1 change: 1 addition & 0 deletions deploy/iso/minikube-iso/go.hash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

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

1 change: 0 additions & 1 deletion deploy/iso/minikube-iso/package/Config.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
menu "System tools"
source "$BR2_EXTERNAL_MINIKUBE_PATH/package/conmon/Config.in"
source "$BR2_EXTERNAL_MINIKUBE_PATH/package/crio-bin/Config.in"
source "$BR2_EXTERNAL_MINIKUBE_PATH/package/sysdig/Config.in"
source "$BR2_EXTERNAL_MINIKUBE_PATH/package/crun-latest/Config.in"
Expand Down
7 changes: 0 additions & 7 deletions deploy/iso/minikube-iso/package/conmon/Config.in
View file
Open in desktop

This file was deleted.

12 changes: 0 additions & 12 deletions deploy/iso/minikube-iso/package/conmon/conmon.hash
View file
Open in desktop

This file was deleted.

25 changes: 0 additions & 25 deletions deploy/iso/minikube-iso/package/conmon/conmon.mk
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion deploy/iso/minikube-iso/package/crio-bin/crio.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

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

The path was changed?

Copy link
Member Author

@ComradeProgrammer ComradeProgrammer May 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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


# Cgroup setting for conmon
conmon_cgroup = "system.slice"
Expand Down
3 changes: 2 additions & 1 deletion deploy/iso/minikube-iso/package/falco-module/falco-module.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,5 @@ FALCO_MODULE_INSTALL_STAGING_OPTS = INSTALL_MOD_PATH=$(STAGING_DIR) install_driv
FALCO_MODULE_INSTALL_TARGET_OPTS = INSTALL_MOD_PATH=$(TARGET_DIR) install_driver

$(eval $(kernel-module))
$(eval $(cmake-package))
$(eval $(generic-package))

5 changes: 4 additions & 1 deletion deploy/iso/minikube-iso/package/podman/podman.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ PODMAN_BIN_ENV = \
CGO_ENABLED=1 \
GOPATH="$(PODMAN_GOPATH)" \
PATH=$(PODMAN_GOPATH)/bin:$(BR_PATH) \
GOARCH=$(PODMAN_GOARCH)
GOARCH=$(PODMAN_GOARCH) \
GOPROXY="https://proxy.golang.org,direct" \
GOSUMDB='sum.golang.org'\
GOOS=linux

define PODMAN_USERS
- -1 podman -1 - - - - -
Expand Down
2 changes: 2 additions & 0 deletions pkg/minikube/cruntime/cruntime_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,9 @@ func (f *FakeRunner) systemctl(args []string, root bool) (string, error) { // no
case "disable":
case "mask":
case "unmask":
case "reset-failed":
f.t.Logf("fake systemctl: %s %s: %v", svc, action, state)

default:
return out, fmt.Errorf("unimplemented fake action: %q", action)
}
Expand Down
12 changes: 10 additions & 2 deletions pkg/minikube/cruntime/docker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ func (r *Docker) Enable(disOthers bool, cgroupDriver string, inUserNamespace boo
return err
}

_ = r.Init.ResetFailed("docker")
if err := r.Init.Restart("docker"); err != nil {
return err
}
Expand Down Expand Up @@ -197,9 +198,15 @@ func (r *Docker) Enable(disOthers bool, cgroupDriver string, inUserNamespace boo
return err
}

// try to restart service if stopped, intentionally continue on any error
if !r.Init.Active(service) {
_ = r.Init.ResetFailed(service)
_ = r.Init.Restart(service)
// try to restart service if stopped, restart until it works
for !r.Init.Active(service) {
time.Sleep(5 * time.Second)
_ = r.Init.ResetFailed(service)
_ = r.Init.Restart(service)
time.Sleep(5 * time.Second)

}
}

Expand All @@ -208,6 +215,7 @@ func (r *Docker) Enable(disOthers bool, cgroupDriver string, inUserNamespace boo

// Restart restarts Docker on a host
func (r *Docker) Restart() error {
_ = r.Init.ResetFailed("docker")
return r.Init.Restart("docker")
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/minikube/download/iso.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const fileScheme = "file"
// DefaultISOURLs returns a list of ISO URL's to consult by default, in priority order
func DefaultISOURLs() []string {
v := version.GetISOVersion()
isoBucket := "minikube/iso"
isoBucket := "minikube-builds/iso/20720"

return []string{
fmt.Sprintf("https://storage.googleapis.com/%s/minikube-%s-%s.iso", isoBucket, v, runtime.GOARCH),
Expand Down
4 changes: 4 additions & 0 deletions pkg/minikube/sysinit/openrc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ func (s *OpenRC) Disable(_ string) error {
return nil
}

func (s *OpenRC) ResetFailed(_ string) error {
return nil
}

// DisableNow does Disable + Stop
func (s *OpenRC) DisableNow(svc string) error {
// supposed to do disable + stop
Expand Down
3 changes: 3 additions & 0 deletions pkg/minikube/sysinit/sysinit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ type Manager interface {
// Reload restarts a service
Reload(string) error

// ResetFailed reset the fail counter of a service
ResetFailed(string) error

// Stop stops a service
Stop(string) error

Expand Down
9 changes: 9 additions & 0 deletions pkg/minikube/sysinit/systemd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,19 @@ func (s *Systemd) Restart(svc string) error {
if err := s.daemonReload(); err != nil {
return err
}

_, err := s.r.RunCmd(exec.Command("sudo", "systemctl", "restart", svc))
return s.appendJournalctlLogsOnFailure(svc, err)
}

// 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
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

@ComradeProgrammer ComradeProgrammer May 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, this is exactly the place where it happened.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

func (s *Systemd) ResetFailed(svc string) error {
_, err := s.r.RunCmd(exec.Command("sudo", "systemctl", "reset-failed", svc))
return s.appendJournalctlLogsOnFailure(svc, err)
}

// Reload reloads a service
func (s *Systemd) Reload(svc string) error {
if err := s.daemonReload(); err != nil {
Expand Down