-
Notifications
You must be signed in to change notification settings - Fork 5.2k
wip: deps: Replace blang/semver with Masterminds/semver/v3 #22462
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.
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
base: master
Are you sure you want to change the base?
Conversation
…sterminds/semver and update API usage.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: medyagh The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/ok-to-test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR migrates the minikube codebase from using blang/semver/v4 to Masterminds/semver/v3 as part of issue #21639. The migration updates all version comparison logic throughout the codebase to use the new library's API.
Changes:
- Replaced all imports of
github.com/blang/semver/v4withgithub.com/Masterminds/semver/v3 - Updated version comparison methods (GTE → GreaterThanEqual, LT → LessThan, GT → GreaterThan, EQ → Equal, NE → not Equal)
- Changed version parsing from
semver.Make/Parse/ParseToleranttosemver.NewVersion - Converted struct field access to method calls (Major → Major(), Minor → Minor(), Patch → Patch())
- Updated return types from value types to pointer types (
semver.Version→*semver.Version) - Updated range-based version checking to use
NewConstraintandCheckmethods - Added nil checks for version pointers where needed
Reviewed changes
Copilot reviewed 53 out of 53 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Added Masterminds/semver/v3 v3.4.0 as direct dependency, moved blang/semver/v4 to indirect |
| hack/go.mod | Added Masterminds/semver/v3 as indirect dependency |
| pkg/util/utils.go | Updated ParseKubernetesVersion to return pointer and use NewVersion |
| pkg/util/utils_test.go | Updated test to use Equal method instead of NE |
| pkg/version/version.go | Updated GetSemverVersion return type to pointer |
| pkg/minikube/cruntime/* | Updated runtime version handling with pointer types and new comparison methods |
| pkg/minikube/bootstrapper/* | Updated kubeadm configuration version checks |
| pkg/minikube/registry/drvs/* | Updated driver version checks for docker, podman, qemu |
| pkg/minikube/node/* | Updated node configuration version handling |
| pkg/minikube/machine/start.go | Updated version comparisons for machine startup |
| pkg/minikube/driver/auxdriver/* | Updated driver version management with pointer types |
| pkg/minikube/download/* | Updated binary download URL generation based on versions |
| pkg/minikube/config/types.go | Updated VersionedExtraOption fields to use pointer types |
| pkg/minikube/cni/* | Updated CNI version checks |
| pkg/minikube/bootstrapper/images/* | Updated image selection logic based on versions |
| pkg/minikube/notify/* | Updated version notification logic |
| pkg/minikube/reason/k8s.go | Updated K8s issue detection version parameter |
| pkg/drivers/kic/oci/* | Updated OCI driver version handling |
| pkg/addons/addons.go | Updated addon version checks |
| cmd/minikube/cmd/start*.go | Updated Kubernetes version validation and comparison |
| test/integration/* | Updated integration tests to use new comparison methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func (r *Docker) Available() error { | ||
| // If Kubernetes version >= 1.24, require both cri-dockerd and dockerd. | ||
| if r.KubernetesVersion.GTE(semver.Version{Major: 1, Minor: 24}) { | ||
| if !r.KubernetesVersion.LessThan(semver.MustParse("1.24.0")) { |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR uses two different patterns for checking if a version is greater than or equal to another: !version.LessThan(...) and version.GreaterThanEqual(...). Both are semantically correct, but for better code maintainability and readability, it would be more consistent to use version.GreaterThanEqual(...) throughout, as it more clearly expresses the intent.
| if !r.KubernetesVersion.LessThan(semver.MustParse("1.24.0")) { | |
| if r.KubernetesVersion.GreaterThanEqual(semver.MustParse("1.24.0")) { |
| // note: containerd supports 'enable_unprivileged_ports' option since v1.6.0-beta.3 (Nov 19, 2021) (ref: https://github.com/containerd/containerd/releases/tag/v1.6.0-beta.3; https://github.com/containerd/containerd/pull/6170) | ||
| // note: minikube bumped containerd version to greater than v1.6.0 on May 19, 2022 (ref: https://github.com/kubernetes/minikube/pull/14152) | ||
| if kv.GTE(semver.Version{Major: 1, Minor: 22}) { | ||
| if !kv.LessThan(semver.MustParse("1.22.0")) { |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency and readability, consider using kv.GreaterThanEqual(semver.MustParse("1.22.0")) instead of !kv.LessThan(semver.MustParse("1.22.0")). Both are semantically correct, but the former more clearly expresses the intent.
pkg/minikube/cruntime/crio.go
Outdated
| // note: 'net.ipv4.ip_unprivileged_port_start' sysctl was marked as safe since Kubernetes v1.22 (Aug 4, 2021) (ref: https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.22.md#feature-9) | ||
| // note: cri-o supports 'default_sysctls' option since v1.12.0 (Oct 19, 2018) (ref: https://github.com/cri-o/cri-o/releases/tag/v1.12.0; https://github.com/cri-o/cri-o/pull/1721) | ||
| if kv.GTE(semver.Version{Major: 1, Minor: 22}) { | ||
| if !kv.LessThan(semver.MustParse("1.22.0")) { |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency and readability, consider using kv.GreaterThanEqual(semver.MustParse("1.22.0")) instead of !kv.LessThan(semver.MustParse("1.22.0")). Both are semantically correct, but the former more clearly expresses the intent.
| // ref: https://kubernetes.io/docs/reference/command-line-tools-reference/kubelet/#options | ||
| // ref: https://github.com/kubernetes/kubernetes/issues/118787 | ||
| if version.GTE(semver.MustParse("1.27.0")) { | ||
| if !version.LessThan(semver.MustParse("1.27.0")) { |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency and readability, consider using version.GreaterThanEqual(semver.MustParse("1.27.0")) instead of !version.LessThan(semver.MustParse("1.27.0")). Both are semantically correct, but the former more clearly expresses the intent.
| if !version.LessThan(semver.MustParse("1.27.0")) { | |
| if version.GreaterThanEqual(semver.MustParse("1.27.0")) { |
| // ref: https://kubernetes.io/docs/reference/command-line-tools-reference/kubelet/#options | ||
| // ref: https://github.com/kubernetes/kubernetes/issues/118787 | ||
| if version.GTE(semver.MustParse("1.27.0")) { | ||
| if !version.LessThan(semver.MustParse("1.27.0")) { |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency and readability, consider using version.GreaterThanEqual(semver.MustParse("1.27.0")) instead of !version.LessThan(semver.MustParse("1.27.0")). Both are semantically correct, but the former more clearly expresses the intent.
| if !version.LessThan(semver.MustParse("1.27.0")) { | |
| if version.GreaterThanEqual(semver.MustParse("1.27.0")) { |
| "NumCPU", // For "none" users who have too few CPUs | ||
| } | ||
| if ver.GE(semver.MustParse("1.20.0")) { | ||
| if !ver.LessThan(semver.MustParse("1.20.0")) { |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency and readability, consider using ver.GreaterThanEqual(semver.MustParse("1.20.0")) instead of !ver.LessThan(semver.MustParse("1.20.0")). Both are semantically correct, but the former more clearly expresses the intent.
| if !ver.LessThan(semver.MustParse("1.20.0")) { | |
| if ver.GreaterThanEqual(semver.MustParse("1.20.0")) { |
| if !ver.LessThan(semver.MustParse("1.18.0-beta.1")) { | ||
| if _, err := exec.LookPath("conntrack"); err != nil { | ||
| exit.Message(reason.GuestMissingConntrack, "Sorry, Kubernetes {{.k8sVersion}} requires conntrack to be installed in root's path", out.V{"k8sVersion": ver.String()}) | ||
| } | ||
| } | ||
| // crictl is required starting with Kubernetes 1.24, for all runtimes since the removal of dockershim | ||
| if ver.GTE(semver.MustParse("1.24.0-alpha.0")) { | ||
| if !ver.LessThan(semver.MustParse("1.24.0-alpha.0")) { |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency and readability, consider using ver.GreaterThanEqual(semver.MustParse("1.18.0-beta.1")) and ver.GreaterThanEqual(semver.MustParse("1.24.0-alpha.0")) instead of !ver.LessThan(...). Both are semantically correct, but the former more clearly expresses the intent.
… sysctl and extend Docker version command timeout.
|
@medyagh: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions 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. I understand the commands that are listed here. |
|
kvm2 driver with docker runtime DetailsTimes for minikube ingress: 15.8s 15.3s 15.8s 16.3s 15.8s Times for minikube start: 36.9s 39.3s 38.0s 37.9s 36.0s docker driver with docker runtime DetailsTimes for minikube start: 20.9s 18.5s 21.7s 20.9s 18.0s Times for minikube ingress: 10.6s 10.7s 10.7s 10.7s 10.6s docker driver with containerd runtime DetailsTimes for minikube start: 19.4s 16.1s 19.3s 16.2s 18.8s Times for minikube ingress: 22.1s 21.1s 21.1s 22.2s 21.1s |
|
PR needs rebase. DetailsInstructions 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. |
part of Part of #21639