refactor: use slices.Contains to simplify code #21381

deepdring · 2025-08-20T08:00:47Z

There is a new function added in the go1.21 standard library, which can make the code more concise and easy to read.

linux-foundation-easycla · 2025-08-20T08:00:54Z

The committers listed above are authorized under a signed CLA.

✅ login: deepdring / name: deepdring (cbd57b9)

k8s-ci-robot · 2025-08-20T08:00:57Z

Welcome @deepdring!

It looks like this is your first PR to kubernetes/minikube 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/minikube has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2025-08-20T08:00:57Z

Hi @deepdring. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

minikube-bot · 2025-08-20T08:03:02Z

Can one of the admins verify this patch?

medyagh · 2025-08-20T17:28:50Z

thank you @deepdring was that the only place we could replace it ?

medyagh · 2025-08-21T03:55:45Z

/ok-to-test

deepdring · 2025-08-21T09:17:12Z

thank you @deepdring was that the only place we could replace it ?

Thank you for your response. So far, I have only checked and modified the changes in the cmd directory, but I just noticed that there are still many modifications under the pkg directory.

Should I continue to submit on this PR, or would it be better for the review if I create a separate PR for the changes under the pkg directory?

deepdring · 2025-08-21T09:18:08Z

/ok-to-test

Looking at the CI errors, they should not be related to this change.

FAIL	k8s.io/minikube/cmd/minikube/cmd	0.761s
# k8s.io/minikube/pkg/util [k8s.io/minikube/pkg/util.test]
Error: pkg\util\utils_test.go:118:92: undefined: syscall.Stat_t
make: *** [Makefile:421: test] Interrupt
Error: The action 'unit test' has timed out after 5 minutes.

nirs

Nice!

cmd/minikube/cmd/start_test.go

cmd/minikube/cmd/start_flags.go

cmd/minikube/cmd/start.go

cmd/minikube/cmd/root.go

cmd/minikube/cmd/delete.go

cmd/minikube/cmd/config/defaults_test.go

deepdring · 2025-08-22T06:22:21Z

Nice!

Modified. Please review again. 😄

deepdring · 2025-08-25T02:53:41Z

The changes have been made; please review again.

k8s-ci-robot · 2025-08-26T22:17:21Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: deepdring, nirs
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.

Needs approval from an approver in each of these files:

OWNERS

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

nirs · 2025-08-26T22:24:42Z

Finding all code that can be replaced with slices.Contains is not easy. We have more than 1000 for loops:

% git grep -E 'for .+ := range' | wc -l
    1048

Searching for found = true:

% git grep -A5 -B5 'found = true'
cmd/minikube/cmd/root.go-	// Check whether this is a windows binary (.exe) running inisde WSL.
cmd/minikube/cmd/root.go-	if runtime.GOOS == "windows" && detect.IsMicrosoftWSL() {
cmd/minikube/cmd/root.go-		var found = false
cmd/minikube/cmd/root.go-		for _, a := range os.Args {
cmd/minikube/cmd/root.go-			if a == "--force" {
cmd/minikube/cmd/root.go:				found = true
cmd/minikube/cmd/root.go-				break
cmd/minikube/cmd/root.go-			}
cmd/minikube/cmd/root.go-		}
cmd/minikube/cmd/root.go-		if !found {
cmd/minikube/cmd/root.go-			exit.Message(reason.WrongBinaryWSL, "You are trying to run a windows .exe binary inside WSL. For better integration please use a Linux binary instead (Download at https://minikube.sigs.k8s.io/docs/start/.). Otherwise if you still want to do this, you can do it using --force")
--
cmd/minikube/cmd/start_test.go-				} else {
cmd/minikube/cmd/start_test.go-					// proxy must in config
cmd/minikube/cmd/start_test.go-					found := false
cmd/minikube/cmd/start_test.go-					for _, v := range config.DockerEnv {
cmd/minikube/cmd/start_test.go-						if v == proxyEnv {
cmd/minikube/cmd/start_test.go:							found = true
cmd/minikube/cmd/start_test.go-							break
cmd/minikube/cmd/start_test.go-						}
cmd/minikube/cmd/start_test.go-					}
cmd/minikube/cmd/start_test.go-					if !found {
cmd/minikube/cmd/start_test.go-						t.Fatalf("Value %s expected in dockerEnv but not occurred", test.proxy)
--
pkg/drivers/kvm/gpu.go-		}
pkg/drivers/kvm/gpu.go-
pkg/drivers/kvm/gpu.go-		// Check if this is an NVIDIA device
pkg/drivers/kvm/gpu.go-		if strings.EqualFold(strings.TrimSpace(string(content)), nvidiaVendorID) {
pkg/drivers/kvm/gpu.go-			log.Infof("Found device %v with NVIDIA's vendorId %v", device.Name(), nvidiaVendorID)
pkg/drivers/kvm/gpu.go:			found = true
pkg/drivers/kvm/gpu.go-
pkg/drivers/kvm/gpu.go-			// Check whether it's unbound. We don't want the device to be bound to nvidia/nouveau etc.
pkg/drivers/kvm/gpu.go-			if isUnbound(device.Name()) {
pkg/drivers/kvm/gpu.go-				// Add the unbound device to the map. The value is set to false initially,
pkg/drivers/kvm/gpu.go-				// it will be set to true later if the device is also isolated.
--
pkg/drivers/kvm/kvm.go-	}
pkg/drivers/kvm/kvm.go-
pkg/drivers/kvm/kvm.go-	var found bool
pkg/drivers/kvm/kvm.go-	for _, domain := range definedDomains {
pkg/drivers/kvm/kvm.go-		if domain == d.MachineName {
pkg/drivers/kvm/kvm.go:			found = true
pkg/drivers/kvm/kvm.go-			break
pkg/drivers/kvm/kvm.go-		}
pkg/drivers/kvm/kvm.go-	}
pkg/drivers/kvm/kvm.go-
pkg/drivers/kvm/kvm.go-	if !found {
--
pkg/minikube/cruntime/containerd.go-		found := false
pkg/minikube/cruntime/containerd.go-		for _, ji := range jsonImages.Images {
pkg/minikube/cruntime/containerd.go-			for _, rt := range ji.RepoTags {
pkg/minikube/cruntime/containerd.go-				i = addRepoTagToImageName(i)
pkg/minikube/cruntime/containerd.go-				if i == rt {
pkg/minikube/cruntime/containerd.go:					found = true
pkg/minikube/cruntime/containerd.go-					break
pkg/minikube/cruntime/containerd.go-				}
pkg/minikube/cruntime/containerd.go-			}
pkg/minikube/cruntime/containerd.go-			if found {
pkg/minikube/cruntime/containerd.go-				break
--
pkg/minikube/cruntime/crio.go-		found := false
pkg/minikube/cruntime/crio.go-		for _, ji := range jsonImages.Images {
pkg/minikube/cruntime/crio.go-			for _, rt := range ji.RepoTags {
pkg/minikube/cruntime/crio.go-				i = addRepoTagToImageName(i)
pkg/minikube/cruntime/crio.go-				if i == rt {
pkg/minikube/cruntime/crio.go:					found = true
pkg/minikube/cruntime/crio.go-					break
pkg/minikube/cruntime/crio.go-				}
pkg/minikube/cruntime/crio.go-			}
pkg/minikube/cruntime/crio.go-			if found {
pkg/minikube/cruntime/crio.go-				break
--
pkg/minikube/driver/driver_test.go-func TestSupportedDrivers(t *testing.T) {
pkg/minikube/driver/driver_test.go-	got := SupportedDrivers()
pkg/minikube/driver/driver_test.go-	found := false
pkg/minikube/driver/driver_test.go-	for _, s := range SupportedDrivers() {
pkg/minikube/driver/driver_test.go-		if s == SSH {
pkg/minikube/driver/driver_test.go:			found = true
pkg/minikube/driver/driver_test.go-		}
pkg/minikube/driver/driver_test.go-	}
pkg/minikube/driver/driver_test.go-
pkg/minikube/driver/driver_test.go-	if found == false {
pkg/minikube/driver/driver_test.go-		t.Errorf("%s not in supported drivers: %v", SSH, got)
--
pkg/minikube/logs/logs_test.go-	expectedPods := []string{"kubernetes-dashboard", "gcp-auth", "controller_ingress", "storage-provisioner"}
pkg/minikube/logs/logs_test.go-	for _, expectedPod := range expectedPods {
pkg/minikube/logs/logs_test.go-		found := false
pkg/minikube/logs/logs_test.go-		for _, pod := range got {
pkg/minikube/logs/logs_test.go-			if expectedPod == pod {
pkg/minikube/logs/logs_test.go:				found = true
pkg/minikube/logs/logs_test.go-				break
pkg/minikube/logs/logs_test.go-			}
pkg/minikube/logs/logs_test.go-		}
pkg/minikube/logs/logs_test.go-		if !found {
pkg/minikube/logs/logs_test.go-			t.Errorf("%q was not found; got = %s", expectedPod, got)
--
pkg/minikube/machine/cache_images.go-	// if the image is loaded with a tarball without a registry specified in tag
pkg/minikube/machine/cache_images.go-	// see https://github.com/containers/podman/issues/15974
pkg/minikube/machine/cache_images.go-	tryImageExist := []string{imgName, cruntime.AddDockerIO(imgName), cruntime.AddLocalhostPrefix(imgName)}
pkg/minikube/machine/cache_images.go-	for _, imgName = range tryImageExist {
pkg/minikube/machine/cache_images.go-		if r.ImageExists(imgName, "") {
pkg/minikube/machine/cache_images.go:			found = true
pkg/minikube/machine/cache_images.go-			break
pkg/minikube/machine/cache_images.go-		}
pkg/minikube/machine/cache_images.go-	}
pkg/minikube/machine/cache_images.go-
pkg/minikube/machine/cache_images.go-	if !found {
--
pkg/minikube/machine/cluster_test.go-	}
pkg/minikube/machine/cluster_test.go-
pkg/minikube/machine/cluster_test.go-	found := false
pkg/minikube/machine/cluster_test.go-	for _, def := range registry.List() {
pkg/minikube/machine/cluster_test.go-		if h.DriverName == def.Name {
pkg/minikube/machine/cluster_test.go:			found = true
pkg/minikube/machine/cluster_test.go-			break
pkg/minikube/machine/cluster_test.go-		}
pkg/minikube/machine/cluster_test.go-	}
pkg/minikube/machine/cluster_test.go-
pkg/minikube/machine/cluster_test.go-	if !found {
--
pkg/minikube/reason/match_test.go-			}
pkg/minikube/reason/match_test.go-
pkg/minikube/reason/match_test.go-			found := false
pkg/minikube/reason/match_test.go-			for _, i := range got.Issues {
pkg/minikube/reason/match_test.go-				if i == tc.issue {
pkg/minikube/reason/match_test.go:					found = true
pkg/minikube/reason/match_test.go-				}
pkg/minikube/reason/match_test.go-			}
pkg/minikube/reason/match_test.go-			if !found {
pkg/minikube/reason/match_test.go-				t.Errorf("Issue %d is not listed in %+v", tc.issue, got.Issues)
pkg/minikube/reason/match_test.go-			}
--
test/integration/functional_test.go-	}
test/integration/functional_test.go-	expectedDir := "cloud.google.com"
test/integration/functional_test.go-	found := false
test/integration/functional_test.go-	for _, file := range files {
test/integration/functional_test.go-		if file.Name() == expectedDir {
test/integration/functional_test.go:			found = true
test/integration/functional_test.go-			break
test/integration/functional_test.go-		}
test/integration/functional_test.go-	}
test/integration/functional_test.go-	if !found {
test/integration/functional_test.go-		t.Fatalf("expected licenses dir to contain %s dir, but was not found", expectedDir)

Maybe AI can help here?

nirs · 2025-08-26T22:37:08Z

Searching with claude-sonnet-4 with this PR reports:

Find all code that can be replaced with slices.Contains. Don't change anything, just give me list of file names and line numbers.

I'll help you find code that can be replaced with slices.Contains. Let me search for common patterns that manually check if a slice contains an element.
Search files...
Ran tool
Ran tool
Ran tool

Let me search more specifically for the patterns I found and get the actual function implementations:
Ran tool
Ran tool
Ran tool
Ran tool

Now let me search for more patterns that could be replaced with slices.Contains:
Ran tool
Ran tool
Ran tool

Let me search for more specific patterns of contains functions:
Search files...
Ran tool
Ran tool
Ran tool

Let me check the containsString function I just found:
Ran tool
Search files...
Ran tool

Let me check the posString function that containsString depends on:
Ran tool
Ran tool
Search files...

Based on my analysis of the codebase, I found several functions that can be replaced with slices.Contains. Here's the complete list:

Functions That Can Be Replaced with `slices.Contains`:

1. pkg/addons/validations.go

Line 98-105: contains function

func contains(slice []string, val string) bool {
    for _, item := range slice {
        if item == val {
            return true
        }
    }
    return false
}

2. pkg/minikube/config/extra_options.go

Line 146-153: ContainsParam function

func ContainsParam(slice []string, s string) bool {
    for _, item := range slice {
        if item == s {
            return true
        }
    }
    return false
}

3. cmd/minikube/cmd/config/prompt.go

Line 142-149: posString function (used by containsString)

func posString(slice []string, element string) int {
    for index, elem := range slice {
        if elem == element {
            return index
        }
    }
    return -1
}

Line 152-154: containsString function (which depends on posString)

func containsString(slice []string, element string) bool {
    return posString(slice, element) != -1
}

4. pkg/minikube/cluster/pause.go

Line 134-145: doesNamespaceContainKubeSystem function

func doesNamespaceContainKubeSystem(namespaces []string) bool {
    // nil slice indicates all namespaces
    if namespaces == nil {
        return true
    }
    for _, ns := range namespaces {
        if ns == "kube-system" {
            return true
        }
    }
    return false
}

5. pkg/perf/monitor/github.go

Line 88-98: prContainsLabel function (Note: This one works with []*github.Label but the pattern is similar)

func prContainsLabel(labels []*github.Label, label string) bool {
    for _, l := range labels {
        if l == nil {
            continue
        }
        if l.GetName() == label {
            return true
        }
    }
    return false
}

Note: The prContainsLabel function works with a slice of pointers to github.Label structs, so it would need a custom implementation or a type conversion to use slices.Contains directly, but the basic pattern is the same.

Summary: 4-5 functions can be directly replaced with slices.Contains(slice, element), which would simplify the code and make it more idiomatic with modern Go practices.

nirs · 2025-08-26T22:40:18Z

@deepdring check #21381 (comment) for other places that can use slices.Contains.

nirs · 2025-08-26T22:51:15Z

@deepdring see #21428 you want to more work on modernizing minikube.

deepdring · 2025-08-28T17:13:07Z

@deepdring see #21428 you want to more work on modernizing minikube.

Thank you for your help. I have found more and made the changes accordingly.

Please review it again.

pkg/perf/monitor/github.go

nirs

ContainsFunc has a bug

medyagh

deepdring can you please rebase?

minikube-pr-bot · 2025-08-28T18:49:27Z

kvm2 driver with docker runtime

┌────────────────┬──────────┬────────────────────────┐
│    COMMAND     │ MINIKUBE │ MINIKUBE  ( PR 21381 ) │
├────────────────┼──────────┼────────────────────────┤
│ minikube start │ 46.4s    │ 47.4s                  │
│ enable ingress │ 15.3s    │ 15.6s                  │
└────────────────┴──────────┴────────────────────────┘

Times for minikube start: 48.5s 44.8s 47.4s 44.8s 46.6s
Times for minikube (PR 21381) start: 47.8s 46.4s 48.7s 46.6s 47.2s

Times for minikube ingress: 15.7s 15.7s 15.2s 14.7s 15.2s
Times for minikube (PR 21381) ingress: 15.7s 15.7s 15.2s 15.7s 15.7s

docker driver with docker runtime

┌────────────────┬──────────┬────────────────────────┐
│    COMMAND     │ MINIKUBE │ MINIKUBE  ( PR 21381 ) │
├────────────────┼──────────┼────────────────────────┤
│ minikube start │ 23.5s    │ 23.7s                  │
│ enable ingress │ 13.2s    │ 13.5s                  │
└────────────────┴──────────┴────────────────────────┘

Times for minikube start: 22.0s 22.8s 26.2s 24.2s 22.6s
Times for minikube (PR 21381) start: 22.9s 23.0s 26.7s 23.1s 23.0s

Times for minikube ingress: 13.6s 13.6s 11.6s 13.6s 13.6s
Times for minikube (PR 21381) ingress: 13.6s 13.6s 13.6s 13.6s 13.1s

docker driver with containerd runtime

┌────────────────┬──────────┬────────────────────────┐
│    COMMAND     │ MINIKUBE │ MINIKUBE  ( PR 21381 ) │
├────────────────┼──────────┼────────────────────────┤
│ minikube start │ 21.6s    │ 23.1s                  │
│ enable ingress │ 23.7s    │ 24.6s                  │
└────────────────┴──────────┴────────────────────────┘

Times for minikube start: 21.0s 20.4s 22.2s 23.5s 21.0s
Times for minikube (PR 21381) start: 22.8s 21.7s 21.8s 24.0s 25.0s

Times for minikube ingress: 23.1s 24.1s 24.1s 23.1s 24.1s
Times for minikube (PR 21381) ingress: 28.1s 24.1s 23.6s 24.1s 23.1s

Signed-off-by: deepdring <[email protected]>

deepdring · 2025-08-29T02:28:04Z

deepdring can you please rebase?

Of course. Rebased.

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 20, 2025

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 20, 2025

k8s-ci-robot requested review from medyagh and prezha August 20, 2025 08:01

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Aug 20, 2025

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 21, 2025

This comment has been minimized.

Sign in to view

nirs reviewed Aug 21, 2025

View reviewed changes

deepdring force-pushed the master branch from 326733b to 4331568 Compare August 22, 2025 06:20

This comment has been minimized.

Sign in to view

deepdring force-pushed the master branch from 4331568 to 886b0dd Compare August 25, 2025 02:50

This comment has been minimized.

Sign in to view

nirs approved these changes Aug 26, 2025

View reviewed changes

deepdring force-pushed the master branch from 886b0dd to 73e955f Compare August 28, 2025 17:07

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 28, 2025

deepdring force-pushed the master branch from 73e955f to a70e920 Compare August 28, 2025 17:12

deepdring force-pushed the master branch from a70e920 to 2e8e312 Compare August 28, 2025 17:17

nirs reviewed Aug 28, 2025

View reviewed changes

pkg/perf/monitor/github.go Outdated Show resolved Hide resolved

deepdring force-pushed the master branch from 2e8e312 to d4f8c56 Compare August 28, 2025 17:30

nirs reviewed Aug 28, 2025

View reviewed changes

pkg/perf/monitor/github.go Outdated Show resolved Hide resolved

nirs reviewed Aug 28, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

medyagh requested changes Aug 28, 2025

View reviewed changes

deepdring force-pushed the master branch from d4f8c56 to 1f0d2fe Compare August 29, 2025 02:23

refactor: use slices.Contains to simplify

cbd57b9

Signed-off-by: deepdring <[email protected]>

deepdring force-pushed the master branch from 1f0d2fe to cbd57b9 Compare August 29, 2025 02:27

refactor: use slices.Contains to simplify code #21381

Are you sure you want to change the base?

refactor: use slices.Contains to simplify code #21381

Conversation

deepdring commented Aug 20, 2025

Uh oh!

linux-foundation-easycla bot commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Aug 20, 2025

Uh oh!

k8s-ci-robot commented Aug 20, 2025

Uh oh!

minikube-bot commented Aug 20, 2025

Uh oh!

medyagh commented Aug 20, 2025

Uh oh!

medyagh commented Aug 21, 2025

Uh oh!

This comment has been minimized.

deepdring commented Aug 21, 2025

Uh oh!

deepdring commented Aug 21, 2025

Uh oh!

nirs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

deepdring commented Aug 22, 2025

Uh oh!

This comment has been minimized.

deepdring commented Aug 25, 2025

Uh oh!

This comment has been minimized.

k8s-ci-robot commented Aug 26, 2025

Uh oh!

nirs commented Aug 26, 2025

Uh oh!

nirs commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Functions That Can Be Replaced with slices.Contains:

1. pkg/addons/validations.go

2. pkg/minikube/config/extra_options.go

3. cmd/minikube/cmd/config/prompt.go

4. pkg/minikube/cluster/pause.go

5. pkg/perf/monitor/github.go

Uh oh!

nirs commented Aug 26, 2025

Uh oh!

nirs commented Aug 26, 2025

Uh oh!

deepdring commented Aug 28, 2025

Uh oh!

Uh oh!

Uh oh!

nirs left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

medyagh left a comment

Choose a reason for hiding this comment

Uh oh!

minikube-pr-bot commented Aug 28, 2025

Uh oh!

deepdring commented Aug 29, 2025

Uh oh!

Uh oh!

linux-foundation-easycla bot commented Aug 20, 2025 •

edited

Loading

nirs commented Aug 26, 2025 •

edited

Loading

Functions That Can Be Replaced with `slices.Contains`: