Skip to content

Commit acb9d57

Browse files
prezhaprezha
committed
Fix KVM driver tests timeouts
Rewrite KVM driver waiting logic for domain start, getting ip address and shutting domain down. Add more config/state outputs to aid future debugging. Bump go/libvirt to v1.11002.0 and set the minimum memory required for running all tests to 3GB to avoid some really weird system behaviour.
1 parent 3cf1e63 commit acb9d57

32 files changed

+269
-197
lines changed

‎Makefile

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ $(shell mkdir -p $(BUILD_DIR))
103103
CURRENT_GIT_BRANCH ?= $(shell git branch | grep \* | cut -d ' ' -f2)
104104

105105
# Use system python if it exists, otherwise use Docker.
106-
PYTHON := $(shell command -v python || echo "docker run --rm -it -v $(shell pwd):/minikube:Z -w /minikube python python")
106+
PYTHON := $(shell command -v python || echo "docker run --rm -it -v $(shell pwd):/minikube -w /minikube python python")
107107
BUILD_OS := $(shell uname -s)
108108

109109
SHA512SUM=$(shell command -v sha512sum || echo "shasum -a 512")
@@ -189,7 +189,7 @@ endef
189189

190190
# $(call DOCKER, image, command)
191191
define DOCKER
192-
docker run --rm -e GOCACHE=/app/.cache -e IN_DOCKER=1 --user $(shell id -u):$(shell id -g) -w /app -v $(PWD):/app:Z -v $(GOPATH):/go --init $(1) /bin/bash -c '$(2)'
192+
docker run --rm -e GOCACHE=/app/.cache -e IN_DOCKER=1 --user $(shell id -u):$(shell id -g) -w /app -v $(PWD):/app -v $(GOPATH):/go --init $(1) /bin/bash -c '$(2)'
193193
endef
194194

195195
ifeq ($(BUILD_IN_DOCKER),y)
@@ -341,13 +341,13 @@ out/minikube-%.iso: $(shell find "deploy/iso/minikube-iso" -type f)
341341
ifeq ($(IN_DOCKER),1)
342342
$(MAKE) minikube-iso-$*
343343
else
344-
docker run --rm --workdir /mnt --volume $(CURDIR):/mnt:Z $(ISO_DOCKER_EXTRA_ARGS) \
344+
docker run --rm --workdir /mnt --volume $(CURDIR):/mnt $(ISO_DOCKER_EXTRA_ARGS) \
345345
--user $(shell id -u):$(shell id -g) --env HOME=/tmp --env IN_DOCKER=1 \
346346
$(ISO_BUILD_IMAGE) /bin/bash -lc '/usr/bin/make minikube-iso-$*'
347347
endif
348348

349349
iso_in_docker:
350-
docker run -it --rm --workdir /mnt --volume $(CURDIR):/mnt:Z $(ISO_DOCKER_EXTRA_ARGS) \
350+
docker run -it --rm --workdir /mnt --volume $(CURDIR):/mnt $(ISO_DOCKER_EXTRA_ARGS) \
351351
--user $(shell id -u):$(shell id -g) --env HOME=/tmp --env IN_DOCKER=1 \
352352
$(ISO_BUILD_IMAGE) /bin/bash
353353

@@ -523,7 +523,7 @@ out/linters/golangci-lint-$(GOLINT_VERSION):
523523
.PHONY: lint
524524
ifeq ($(MINIKUBE_BUILD_IN_DOCKER),y)
525525
lint:
526-
docker run --rm -v `pwd`:/app:Z -w /app golangci/golangci-lint:$(GOLINT_VERSION) \
526+
docker run --rm -v `pwd`:/app -w /app golangci/golangci-lint:$(GOLINT_VERSION) \
527527
golangci-lint run ${GOLINT_OPTIONS} ./..."
528528
# --skip-dirs "cmd/drivers/kvm|cmd/drivers/hyperkit|pkg/drivers/kvm|pkg/drivers/hyperkit"
529529
# The "--skip-dirs" parameter is no longer supported in the V2 version. If you need to skip the directory,
@@ -657,7 +657,7 @@ out/docker-machine-driver-hyperkit:
657657
ifeq ($(MINIKUBE_BUILD_IN_DOCKER),y)
658658
docker run --rm -e GOCACHE=/app/.cache -e IN_DOCKER=1 \
659659
--user $(shell id -u):$(shell id -g) -w /app \
660-
-v $(PWD):/app:Z -v $(GOPATH):/go:Z --init --entrypoint "" \
660+
-v $(PWD):/app -v $(GOPATH):/go --init --entrypoint "" \
661661
$(HYPERKIT_BUILD_IMAGE) /bin/bash -c 'CC=o64-clang CXX=o64-clang++ /usr/bin/make $@'
662662
else
663663
$(if $(quiet),@echo " GO $@")

‎cmd/minikube/cmd/start.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,8 +1098,8 @@ func suggestMemoryAllocation(sysLimit, containerLimit, nodes int) int {
10981098
return mem
10991099
}
11001100

1101-
const fallback = 2200
1102-
maximum := 6000
1101+
const fallback = 3072
1102+
maximum := 6144
11031103

11041104
if sysLimit > 0 && fallback > sysLimit {
11051105
return sysLimit

‎cmd/minikube/cmd/start_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -277,26 +277,26 @@ func TestSuggestMemoryAllocation(t *testing.T) {
277277
nodes int
278278
want int
279279
}{
280-
{"128GB sys", 128000, 0, 1, 6000},
281-
{"64GB sys", 64000, 0, 1, 6000},
282-
{"32GB sys", 32768, 0, 1, 6000},
280+
{"128GB sys", 128000, 0, 1, 6144},
281+
{"64GB sys", 64000, 0, 1, 6144},
282+
{"32GB sys", 32768, 0, 1, 6144},
283283
{"16GB sys", 16384, 0, 1, 4000},
284284
{"odd sys", 14567, 0, 1, 3600},
285-
{"4GB sys", 4096, 0, 1, 2200},
285+
{"4GB sys", 4096, 0, 1, 3072},
286286
{"2GB sys", 2048, 0, 1, 2048},
287-
{"Unable to poll sys", 0, 0, 1, 2200},
287+
{"Unable to poll sys", 0, 0, 1, 3072},
288288
{"128GB sys, 16GB container", 128000, 16384, 1, 16336},
289289
{"64GB sys, 16GB container", 64000, 16384, 1, 16000},
290290
{"16GB sys, 4GB container", 16384, 4096, 1, 4000},
291291
{"4GB sys, 3.5GB container", 16384, 3500, 1, 3452},
292292
{"16GB sys, 2GB container", 16384, 2048, 1, 2048},
293293
{"16GB sys, unable to poll container", 16384, 0, 1, 4000},
294-
{"128GB sys 2 nodes", 128000, 0, 2, 6000},
295-
{"8GB sys 3 nodes", 8192, 0, 3, 2200},
296-
{"16GB sys 2 nodes", 16384, 0, 2, 2200},
294+
{"128GB sys 2 nodes", 128000, 0, 2, 6144},
295+
{"8GB sys 3 nodes", 8192, 0, 3, 3072},
296+
{"16GB sys 2 nodes", 16384, 0, 2, 3072},
297297
{"32GB sys 2 nodes", 32768, 0, 2, 4050},
298-
{"odd sys 2 nodes", 14567, 0, 2, 2200},
299-
{"4GB sys 2 nodes", 4096, 0, 2, 2200},
298+
{"odd sys 2 nodes", 14567, 0, 2, 3072},
299+
{"4GB sys 2 nodes", 4096, 0, 2, 3072},
300300
{"2GB sys 3 nodes", 2048, 0, 3, 2048},
301301
}
302302
for _, test := range tests {

‎deploy/iso/minikube-iso/go.hash

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,4 +35,4 @@ sha256 36930162a93df417d90bd22c6e14daff4705baac2b02418edda671cdfa9cd07f go1.23
3535
sha256 8d6a77332487557c6afa2421131b50f83db4ae3c579c3bc72e670ee1f6968599 go1.23.3.src.tar.gz
3636
sha256 ad345ac421e90814293a9699cca19dd5238251c3f687980bbcae28495b263531 go1.23.4.src.tar.gz
3737
sha256 d14120614acb29d12bcab72bd689f257eb4be9e0b6f88a8fb7e41ac65f8556e5 go1.24.0.src.tar.gz
38-
sha256 6924efde5de86fe277676e929dc9917d466efa02fb934197bc2eba35d5680971 go1.23.4.linux-amd64.tar.gz
38+
sha256 6924efde5de86fe277676e929dc9917d466efa02fb934197bc2eba35d5680971 go1.23.4.linux-amd64.tar.gz

‎go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ require (
8787
k8s.io/klog/v2 v2.130.1
8888
k8s.io/kubectl v0.32.2
8989
k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738
90-
libvirt.org/go/libvirt v1.11001.0
90+
libvirt.org/go/libvirt v1.11002.0
9191
sigs.k8s.io/sig-storage-lib-external-provisioner/v6 v6.3.0
9292
)
9393

‎go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3218,8 +3218,8 @@ k8s.io/utils v0.0.0-20210930125809-cb0fa318a74b/go.mod h1:jPW/WVKK9YHAvNhRxK0md/
32183218
k8s.io/utils v0.0.0-20211116205334-6203023598ed/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
32193219
k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738 h1:M3sRQVHv7vB20Xc2ybTt7ODCeFj6JSWYFzOFnYeS6Ro=
32203220
k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
3221-
libvirt.org/go/libvirt v1.11001.0 h1:QJgpslxY7qkpXZIDxdMHpkDl7FfhgQJwqRTGBbg/S8E=
3222-
libvirt.org/go/libvirt v1.11001.0/go.mod h1:1WiFE8EjZfq+FCVog+rvr1yatKbKZ9FaFMZgEqxEJqQ=
3221+
libvirt.org/go/libvirt v1.11002.0 h1:cb8KJG3D97pc/hxQ2n6P82hRX3rlgdzO7bih6W1AAQ8=
3222+
libvirt.org/go/libvirt v1.11002.0/go.mod h1:1WiFE8EjZfq+FCVog+rvr1yatKbKZ9FaFMZgEqxEJqQ=
32233223
lukechampine.com/uint128 v1.1.1/go.mod h1:c4eWIwlEGaxC/+H1VguhU4PHXNWDCDMUlWdIWl2j1gk=
32243224
lukechampine.com/uint128 v1.2.0/go.mod h1:c4eWIwlEGaxC/+H1VguhU4PHXNWDCDMUlWdIWl2j1gk=
32253225
modernc.org/cc/v3 v3.36.0/go.mod h1:NFUHyPn4ekoC/JHeZFfZurN6ixxawE1BnVonP/oahEI=

‎hack/metrics/metrics.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ func getLabels(containerRuntime string) *stackdriver.Labels {
145145
func minikubeStartTime(ctx context.Context, projectID, minikubePath, containerRuntime string) (float64, error) {
146146
defer deleteMinikube(ctx, minikubePath)
147147

148-
cmd := exec.CommandContext(ctx, minikubePath, "start", "--driver=docker", "-p", profile, "--memory=2048", "--trace=gcp", fmt.Sprintf("--container-runtime=%s", containerRuntime))
148+
cmd := exec.CommandContext(ctx, minikubePath, "start", "--driver=docker", "-p", profile, "--memory=3072", "--trace=gcp", fmt.Sprintf("--container-runtime=%s", containerRuntime))
149149
cmd.Env = append(os.Environ(), fmt.Sprintf("%s=%s", pkgtrace.ProjectEnvVar, projectID))
150150
cmd.Stdout = os.Stderr
151151
cmd.Stderr = os.Stderr

‎pkg/drivers/kvm/domain.go

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,12 @@ import (
3131
func (d *Driver) getDomain() (*libvirt.Domain, *libvirt.Connect, error) {
3232
conn, err := getConnection(d.ConnectionURI)
3333
if err != nil {
34-
return nil, nil, errors.Wrap(err, "getting libvirt connection")
34+
return nil, nil, fmt.Errorf("failed opening libvirt connection: %w", err)
3535
}
3636

3737
dom, err := conn.LookupDomainByName(d.MachineName)
3838
if err != nil {
39-
return nil, nil, errors.Wrap(err, "looking up domain")
39+
return nil, nil, fmt.Errorf("failed looking up domain: %w", lvErr(err))
4040
}
4141

4242
return dom, conn, nil
@@ -45,13 +45,17 @@ func (d *Driver) getDomain() (*libvirt.Domain, *libvirt.Connect, error) {
4545
func getConnection(connectionURI string) (*libvirt.Connect, error) {
4646
conn, err := libvirt.NewConnect(connectionURI)
4747
if err != nil {
48-
return nil, errors.Wrap(err, "connecting to libvirt socket")
48+
return nil, fmt.Errorf("failed connecting to libvirt socket: %w", lvErr(err))
4949
}
5050

5151
return conn, nil
5252
}
5353

5454
func closeDomain(dom *libvirt.Domain, conn *libvirt.Connect) error {
55+
if dom == nil {
56+
return fmt.Errorf("nil domain, cannot close")
57+
}
58+
5559
if err := dom.Free(); err != nil {
5660
return err
5761
}
@@ -62,25 +66,24 @@ func closeDomain(dom *libvirt.Domain, conn *libvirt.Connect) error {
6266
return err
6367
}
6468

65-
func (d *Driver) createDomain() (*libvirt.Domain, error) {
66-
// create the XML for the domain using our domainTmpl template
69+
// defineDomain defines the XML for the domain using our domainTmpl template
70+
func (d *Driver) defineDomain() (*libvirt.Domain, error) {
6771
tmpl := template.Must(template.New("domain").Parse(domainTmpl))
6872
var domainXML bytes.Buffer
6973
if err := tmpl.Execute(&domainXML, d); err != nil {
7074
return nil, errors.Wrap(err, "executing domain xml")
7175
}
7276
conn, err := getConnection(d.ConnectionURI)
7377
if err != nil {
74-
return nil, errors.Wrap(err, "getting libvirt connection")
78+
return nil, fmt.Errorf("failed opening libvirt connection: %w", err)
7579
}
7680
defer func() {
7781
if _, err := conn.Close(); err != nil {
78-
log.Errorf("unable to close libvirt connection: %v", err)
82+
log.Errorf("failed closing libvirt connection: %v", lvErr(err))
7983
}
8084
}()
8185

82-
log.Infof("define libvirt domain using xml: %v", domainXML.String())
83-
// define the domain in libvirt using the generated XML
86+
log.Infof("defining domain using XML: %v", domainXML.String())
8487
dom, err := conn.DomainDefineXML(domainXML.String())
8588
if err != nil {
8689
return nil, errors.Wrapf(err, "error defining domain xml: %s", domainXML.String())

‎pkg/drivers/kvm/domain_definition_arm64.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,16 @@ const domainTmpl = `
2727
<acpi/>
2828
<apic/>
2929
<pae/>
30-
{{if .Hidden}}
30+
{{- if .Hidden}}
3131
<kvm>
3232
<hidden state='on'/>
3333
</kvm>
34-
{{end}}
34+
{{- end}}
3535
</features>
3636
<cpu mode='host-passthrough'>
37-
{{if gt .NUMANodeCount 1}}
37+
{{- if gt .NUMANodeCount 1}}
3838
{{.NUMANodeXML}}
39-
{{end}}
39+
{{- end}}
4040
</cpu>
4141
<os>
4242
<type machine='virt-4.2' arch='aarch64'>hvm</type>
@@ -75,12 +75,12 @@ const domainTmpl = `
7575
<rng model='virtio'>
7676
<backend model='random'>/dev/random</backend>
7777
</rng>
78-
{{if .GPU}}
78+
{{- if .GPU}}
7979
{{.DevicesXML}}
80-
{{end}}
81-
{{if gt .ExtraDisks 0}}
80+
{{- end}}
81+
{{- if gt .ExtraDisks 0}}
8282
{{.ExtraDisksXML}}
83-
{{end}}
83+
{{- end}}
8484
</devices>
8585
</domain>
8686
`

‎pkg/drivers/kvm/domain_definition_x86.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,16 @@ const domainTmpl = `
2727
<acpi/>
2828
<apic/>
2929
<pae/>
30-
{{if .Hidden}}
30+
{{- if .Hidden}}
3131
<kvm>
3232
<hidden state='on'/>
3333
</kvm>
34-
{{end}}
34+
{{- end}}
3535
</features>
3636
<cpu mode='host-passthrough'>
37-
{{if gt .NUMANodeCount 1}}
37+
{{- if gt .NUMANodeCount 1}}
3838
{{.NUMANodeXML}}
39-
{{end}}
39+
{{- end}}
4040
</cpu>
4141
<os>
4242
<type>hvm</type>
@@ -72,12 +72,12 @@ const domainTmpl = `
7272
<rng model='virtio'>
7373
<backend model='random'>/dev/random</backend>
7474
</rng>
75-
{{if .GPU}}
75+
{{- if .GPU}}
7676
{{.DevicesXML}}
77-
{{end}}
78-
{{if gt .ExtraDisks 0}}
77+
{{- end}}
78+
{{- if gt .ExtraDisks 0}}
7979
{{.ExtraDisksXML}}
80-
{{end}}
80+
{{- end}}
8181
</devices>
8282
</domain>
8383
`

0 commit comments

Comments
 (0)