Skip to content
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

--kubernetes-version: assume latest patch version if not specified by user #16569

Merged
merged 9 commits into from
Jun 1, 2023
90 changes: 75 additions & 15 deletions cmd/minikube/cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,13 @@ type versionJSON struct {
}

var (
registryMirror []string
insecureRegistry []string
apiServerNames []string
apiServerIPs []net.IP
hostRe = regexp.MustCompile(`^[^-][\w\.-]+$`)
// ErrKubernetesPatchNotFound is when a patch was not found for the given <major>.<minor> version
ErrKubernetesPatchNotFound = errors.New("Unable to detect the latest patch release for specified Kubernetes version")
registryMirror []string
insecureRegistry []string
apiServerNames []string
apiServerIPs []net.IP
hostRe = regexp.MustCompile(`^[^-][\w\.-]+$`)
)

func init() {
Expand Down Expand Up @@ -318,7 +320,10 @@ func provisionWithDriver(cmd *cobra.Command, ds registry.DriverState, existing *
stopk8s = true
}

k8sVersion := getKubernetesVersion(existing)
k8sVersion, err := getKubernetesVersion(existing)
if err != nil {
klog.Warningf("failed getting Kubernetes version: %v", err)
}
rtime := getContainerRuntime(existing)
cc, n, err := generateClusterConfig(cmd, existing, k8sVersion, rtime, driverName)
if err != nil {
Expand Down Expand Up @@ -1585,17 +1590,23 @@ func createNode(cc config.ClusterConfig, existing *config.ClusterConfig) (config
// Create the initial node, which will necessarily be a control plane
if existing != nil {
cp, err := config.PrimaryControlPlane(existing)
cp.KubernetesVersion = getKubernetesVersion(&cc)
cp.ContainerRuntime = getContainerRuntime(&cc)
if err != nil {
return cc, config.Node{}, err
}
cp.KubernetesVersion, err = getKubernetesVersion(&cc)
if err != nil {
klog.Warningf("failed getting Kubernetes version: %v", err)
}
cp.ContainerRuntime = getContainerRuntime(&cc)

// Make sure that existing nodes honor if KubernetesVersion gets specified on restart
// KubernetesVersion is the only attribute that the user can override in the Node object
nodes := []config.Node{}
for _, n := range existing.Nodes {
n.KubernetesVersion = getKubernetesVersion(&cc)
n.KubernetesVersion, err = getKubernetesVersion(&cc)
if err != nil {
klog.Warningf("failed getting Kubernetes version: %v", err)
}
n.ContainerRuntime = getContainerRuntime(&cc)
nodes = append(nodes, n)
}
Expand All @@ -1604,9 +1615,13 @@ func createNode(cc config.ClusterConfig, existing *config.ClusterConfig) (config
return cc, cp, nil
}

kubeVer, err := getKubernetesVersion(&cc)
if err != nil {
klog.Warningf("failed getting Kubernetes version: %v", err)
}
cp := config.Node{
Port: cc.KubernetesConfig.NodePort,
KubernetesVersion: getKubernetesVersion(&cc),
KubernetesVersion: kubeVer,
ContainerRuntime: getContainerRuntime(&cc),
ControlPlane: true,
Worker: true,
Expand Down Expand Up @@ -1653,12 +1668,27 @@ func autoSetDriverOptions(cmd *cobra.Command, drvName string) (err error) {

// validateKubernetesVersion ensures that the requested version is reasonable
func validateKubernetesVersion(old *config.ClusterConfig) {
nvs, _ := semver.Make(strings.TrimPrefix(getKubernetesVersion(old), version.VersionPrefix))
paramVersion := viper.GetString(kubernetesVersion)
paramVersion = strings.TrimPrefix(strings.ToLower(paramVersion), version.VersionPrefix)
kubernetesVer, err := getKubernetesVersion(old)
if err != nil {
if errors.Is(err, ErrKubernetesPatchNotFound) {
exit.Message(reason.PatchNotFound, "Unable to detect the latest patch release for specified major.minor version v{{.majorminor}}",
out.V{"majorminor": paramVersion})
}
exit.Message(reason.Usage, `Unable to parse "{{.kubernetes_version}}": {{.error}}`, out.V{"kubernetes_version": paramVersion, "error": err})

}

nvs, _ := semver.Make(strings.TrimPrefix(kubernetesVer, version.VersionPrefix))
oldestVersion := semver.MustParse(strings.TrimPrefix(constants.OldestKubernetesVersion, version.VersionPrefix))
defaultVersion := semver.MustParse(strings.TrimPrefix(constants.DefaultKubernetesVersion, version.VersionPrefix))
newestVersion := semver.MustParse(strings.TrimPrefix(constants.NewestKubernetesVersion, version.VersionPrefix))
zeroVersion := semver.MustParse(strings.TrimPrefix(constants.NoKubernetesVersion, version.VersionPrefix))

if isTwoDigitSemver(paramVersion) && getLatestPatch(paramVersion) != "" {
out.Styled(style.Workaround, `Using Kubernetes {{.version}} since patch version was unspecified`, out.V{"version": nvs})
}
if nvs.Equals(zeroVersion) {
klog.Infof("No Kubernetes version set for minikube, setting Kubernetes version to %s", constants.NoKubernetesVersion)
return
Expand Down Expand Up @@ -1729,7 +1759,7 @@ func isBaseImageApplicable(drv string) bool {
return registry.IsKIC(drv)
}

func getKubernetesVersion(old *config.ClusterConfig) string {
func getKubernetesVersion(old *config.ClusterConfig) (string, error) {
if viper.GetBool(noKubernetes) {
// Exit if --kubernetes-version is specified.
if viper.GetString(kubernetesVersion) != "" {
Expand Down Expand Up @@ -1758,12 +1788,20 @@ $ minikube config unset kubernetes-version`)
paramVersion = constants.NewestKubernetesVersion
}

nvs, err := semver.Make(strings.TrimPrefix(paramVersion, version.VersionPrefix))
kubernetesSemver := strings.TrimPrefix(strings.ToLower(paramVersion), version.VersionPrefix)
if isTwoDigitSemver(kubernetesSemver) {
potentialPatch := getLatestPatch(kubernetesSemver)
if potentialPatch == "" {
return "", ErrKubernetesPatchNotFound
}
kubernetesSemver = potentialPatch
}
nvs, err := semver.Make(kubernetesSemver)
if err != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if here we want to send back an error here instead of just exiting. With the error return value it could be better to now return an error and handle it outside getKubernetesVersion

Copy link
Member

Choose a reason for hiding this comment

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

good point and good catch ! I think since it is already been implemented like that we can leave this one alone, and if we want we can refactor it in a follow up PR (for sake of keeping this PR small)

exit.Message(reason.Usage, `Unable to parse "{{.kubernetes_version}}": {{.error}}`, out.V{"kubernetes_version": paramVersion, "error": err})
}

return version.VersionPrefix + nvs.String()
return version.VersionPrefix + nvs.String(), nil
}

// validateDockerStorageDriver checks that docker is using overlay2
Expand Down Expand Up @@ -1848,7 +1886,11 @@ func validateBareMetal(drvName string) {
}

// conntrack is required starting with Kubernetes 1.18, include the release candidates for completion
version, _ := util.ParseKubernetesVersion(getKubernetesVersion(nil))
kubeVer, err := getKubernetesVersion(nil)
if err != nil {
klog.Warningf("failed getting Kubernetes version: %v", err)
}
version, _ := util.ParseKubernetesVersion(kubeVer)
if version.GTE(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": version.String()})
Expand Down Expand Up @@ -1878,3 +1920,21 @@ func exitGuestProvision(err error) {
}
exit.Error(reason.GuestProvision, "error provisioning guest", err)
}

// Example input = 1.26 => output = "1.26.5"
// Example input = 1.26.5 => output = "1.26.5"
// Example input = 1.26.999 => output = ""
func getLatestPatch(majorMinorVer string) string {
for _, k := range constants.ValidKubernetesVersions {
if strings.HasPrefix(k, fmt.Sprintf("v%s.", majorMinorVer)) {
return strings.TrimPrefix(k, version.VersionPrefix)
}

}
return ""
}

func isTwoDigitSemver(ver string) bool {
majorMinorOnly := regexp.MustCompile(`^(?P<major>0|[1-9]\d*)\.(?P<minor>0|[1-9]\d*)$`)
return majorMinorOnly.MatchString(ver)
}
6 changes: 5 additions & 1 deletion cmd/minikube/cmd/start_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -811,7 +811,11 @@ func updateExistingConfigFromFlags(cmd *cobra.Command, existing *config.ClusterC
updateStringFromFlag(cmd, &cc.SocketVMnetPath, socketVMnetPath)

if cmd.Flags().Changed(kubernetesVersion) {
cc.KubernetesConfig.KubernetesVersion = getKubernetesVersion(existing)
kubeVer, err := getKubernetesVersion(existing)
if err != nil {
klog.Warningf("failed getting Kubernetes version: %v", err)
}
cc.KubernetesConfig.KubernetesVersion = kubeVer
}
if cmd.Flags().Changed(containerRuntime) {
cc.KubernetesConfig.ContainerRuntime = getContainerRuntime(existing)
Expand Down
119 changes: 118 additions & 1 deletion cmd/minikube/cmd/start_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/spf13/cobra"
"github.com/spf13/viper"

"k8s.io/klog/v2"
cfg "k8s.io/minikube/pkg/minikube/config"
"k8s.io/minikube/pkg/minikube/constants"
"k8s.io/minikube/pkg/minikube/cruntime"
Expand Down Expand Up @@ -62,6 +63,16 @@ func TestGetKubernetesVersion(t *testing.T) {
paramVersion: "v1.16.0",
cfg: &cfg.ClusterConfig{KubernetesConfig: cfg.KubernetesConfig{KubernetesVersion: "v1.15.0"}},
},
{
description: "kubernetes-version without patch version",
expectedVersion: "v1.16.15",
paramVersion: "v1.16",
},
{
description: "kubernetes-version without patch version",
expectedVersion: "v1.16.15",
paramVersion: "1.16",
},
{
description: "kubernetes-version given as 'stable', no config",
expectedVersion: constants.DefaultKubernetesVersion,
Expand Down Expand Up @@ -92,7 +103,10 @@ func TestGetKubernetesVersion(t *testing.T) {
for _, test := range tests {
t.Run(test.description, func(t *testing.T) {
viper.SetDefault(kubernetesVersion, test.paramVersion)
version := getKubernetesVersion(test.cfg)
version, err := getKubernetesVersion(test.cfg)
if err != nil {
klog.Warningf("failed getting Kubernetes version: %v", err)
}

// check whether we are getting the expected version
if version != test.expectedVersion {
Expand Down Expand Up @@ -451,6 +465,109 @@ func TestValidateRuntime(t *testing.T) {
}
}

func TestIsTwoDigitSemver(t *testing.T) {
var tcs = []struct {
desc string
version string
expected bool
}{
{
desc: "a valid three digit version",
version: "1.26.5",
expected: false,
},
{
desc: "a valid two digit version",
version: "1.26",
expected: true,
},
{
desc: "a valid two digit version with a period",
version: "1.26.",
expected: false,
},
{
desc: "an invalid major version",
version: "2",
expected: false,
},
{
desc: "a valid major version",
version: "1",
expected: false,
},
{
desc: "a two digit version with a 0 as the major/minor components",
version: "0.0",
expected: true,
},
{
desc: "a two digit version with negative major version",
version: "-1.0",
expected: false,
},
{
desc: "a two digit version with negative minor version",
version: "1.-1",
expected: false,
},
{
desc: "a missing minor version",
version: "1.",
expected: false,
},
{
desc: "a missing major version",
version: ".2",
expected: false,
},
{
desc: "a valid two digit version with whitespace between components",
version: "1. 1",
expected: false,
},
{
desc: "a two digit version with a non-digit major component",
version: "a.12",
expected: false,
},
{
desc: "a two digit version with a non-digit minor component",
version: "1.a",
expected: false,
},
{
desc: "a two digit version with extraneous non-digits in minor component",
version: "1.2a",
expected: false,
},
{
desc: "a two digit version larger major/minor components",
version: "123456789.987654321",
expected: true,
},
{
desc: "a valid two digit version with a version prefix",
version: "v1.26",
expected: false,
},
{
desc: "a valid three digit version with a version prefix",
version: "v1.26.5",
expected: false,
},
}
for _, tc := range tcs {
t.Run(tc.desc, func(t *testing.T) {
actual := isTwoDigitSemver(tc.version)
// check whether the function correctly verifies if it is a 2 digit semver
if actual != tc.expected {
t.Fatalf("test failed. Expected version %s to return %t", tc.version, tc.expected)
}
})
}
}

func TestValidatePorts(t *testing.T) {
isMicrosoftWSL := detect.IsMicrosoftWSL()
type portTest struct {
Expand Down
2 changes: 1 addition & 1 deletion pkg/minikube/out/out.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ import (
"bytes"
"fmt"
"html"
"html/template"
"io"
"os"
"path/filepath"
"strconv"
"strings"
"text/template"
"time"

"github.com/Delta456/box-cli-maker/v2"
Expand Down
5 changes: 5 additions & 0 deletions pkg/minikube/reason/reason.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ func (k *Kind) IssueURLs() []string {

// Sections are ordered roughly by stack dependencies
var (
// minikube could not find a patch for the provided major.minor version
PatchNotFound = Kind{ID: "MK_PATCH_NOT_FOUND", ExitCode: ExProgramUsage,
Advice: translate.T("Specify --kubernetes-version in v<major>.<minor.<build> form. example: 'v1.1.14'"),
}

// minikube has been passed an incorrect parameter
Usage = Kind{ID: "MK_USAGE", ExitCode: ExProgramUsage}
// minikube has no current cluster running
Expand Down
Loading