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

Parsing of Kubernetes version strings is brittle and doesn't display useful error messages #3253

Closed
jeamland opened this issue Oct 14, 2018 · 4 comments
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. r/2019q2 Issue was last reviewed 2019q2

Comments

@jeamland
Copy link

jeamland commented Oct 14, 2018

Is this a BUG REPORT or FEATURE REQUEST? (choose one): Bug report

Please provide the following details:

Environment:

  • Minikube version: v0.30.0
  • OS: (e.g. from /etc/os-release): macOS Mojave 10.14
  • VM Driver: vmwarefusion (VMware Fusion Pro 10.1.3)
  • ISO version: minikube-v0.30.0.iso

What happened:

Tried to create a Kubernetes 1.12.1 cluster with minikube like so:

$ minikube start --kubernetes-version 1.12.1

Got the following error:

Starting local Kubernetes 1.12.1 cluster...
Starting VM...
Getting VM IP address...
Moving files into cluster...
E1015 10:23:32.389960   16692 start.go:254] Error updating cluster:  generating kubeadm cfg: parsing kubernetes version: parsing kubernetes version: strconv.ParseUint: parsing "": invalid syntax
================================================================================
An error has occurred. Would you like to opt in to sending anonymized crash
information to minikube to help prevent future errors?
To opt out of these messages, run the command:
	minikube config set WantReportErrorPrompt false
================================================================================
Please enter your response [Y/n]:

What you expected to happen:

Either have a 1.12.1 cluster created or an error message telling me that versions need to be in a vX.Y.Z format (the v is actually important and nothing works without it).

How to reproduce it (as minimally and precisely as possible):

$ minikube start --kubernetes-version 1.12.1

The actual VM driver is irrelevant.

@mlgibbons
Copy link
Contributor

I agree regarding the error msg. Took me a while to figure out what was going wrong too.

jeamland added a commit to jeamland/minikube that referenced this issue Oct 16, 2018
Version was generally being passed around as a string, I suspect due to
the use of JSON as a configuration store and wanting it to map to certain
configuration structures. This unfortunately led to issues where it was
implicitly required that all Kubernetes versions be specified with a
leading 'v' prefix or various operations would fail.

This patch:
- Replaces as many instances of string-based versions with semver.Version
  objects as possible.
- Makes version parsing more reliable by consistently using
  strings.TrimPrefix to remove the 'v' prefix when needed.
- Kubernetes versions stored in profiles are normalised to not having the
  'v' prefix. Existing profile configurations with a 'v' prefix will still
  work.
- Validates versions being passed in to make sure they'll parse.

Fixes kubernetes#3253
jeamland added a commit to jeamland/minikube that referenced this issue Oct 16, 2018
Version was generally being passed around as a string, I suspect due to
the use of JSON as a configuration store and wanting it to map to certain
configuration structures. This unfortunately led to issues where it was
implicitly required that all Kubernetes versions be specified with a
leading 'v' prefix or various operations would fail.

This patch:
- Replaces as many instances of string-based versions with semver.Version
  objects as possible.
- Makes version parsing more reliable by consistently using
  strings.TrimPrefix to remove the 'v' prefix when needed.
- Kubernetes versions stored in profiles are normalised to not having the
  'v' prefix. Existing profile configurations with a 'v' prefix will still
  work.
- Validates versions being passed in to make sure they'll parse.

Fixes kubernetes#3253

Remove tests that are no longer needed.

Fix templates.
@tstromberg tstromberg added the kind/bug Categorizes issue or PR as related to a bug. label Oct 30, 2018
@tstromberg tstromberg self-assigned this Jan 8, 2019
@dmlemos
Copy link

dmlemos commented Jan 15, 2019

Had the same issue, but different error:

$ minikube start --kubernetes-version 1.12 --vm-driver xhyve
Starting local Kubernetes 1.12 cluster...
Starting VM...
 Getting VM IP address...
E0115 18:44:09.766844    1037 start.go:211] Error parsing version semver:  No Major.Minor.Patch elements found
E0115 18:44:09.767444    1037 start.go:216] Error parsing version semver:  No Major.Minor.Patch elements found
Moving files into cluster...
E0115 18:44:09.815792    1037 start.go:268] Error updating cluster:  generating kubeadm cfg: parsing kubernetes version: parsing kubernetes version: No Major.Minor.Patch elements found

What is interesting is what happened next 😂

$ minikube start --kubernetes-version 1.12.4
Starting local Kubernetes 1.12.4 cluster...
Starting VM...
Getting VM IP address...
E0115 18:47:57.975359    1265 start.go:211] Error parsing version semver:  No Major.Minor.Patch elements found
Moving files into cluster...
E0115 18:47:58.027925    1265 start.go:268] Error updating cluster:  generating kubeadm cfg: parsing kubernetes version: parsing kubernetes version: strconv.ParseUint: parsing "": invalid syntax

$ minikube start --kubernetes-version 1.12.3
Starting local Kubernetes 1.12.3 cluster...
Starting VM...
Getting VM IP address...
Kubernetes version downgrade is not supported. Using version: v1.12.4
Moving files into cluster...
Downloading kubeadm v1.12.4
Downloading kubelet v1.12.4
Finished Downloading kubeadm v1.12.4
Finished Downloading kubelet v1.12.4
Setting up certs...
Connecting to cluster...
Setting up kubeconfig...
Stopping extra container runtimes...
Machine exists, restarting cluster components...

minikube version: v0.32.0

@tstromberg tstromberg added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. labels Jan 23, 2019
@tstromberg tstromberg removed their assignment Jan 23, 2019
@tstromberg tstromberg added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Jan 23, 2019
@palash25
Copy link

Hi, is the referenced PR for this issue still being worked on or is this issue up for grabs?

flyingcircle pushed a commit to flyingcircle/minikube that referenced this issue Jan 28, 2019
flyingcircle added a commit to flyingcircle/minikube that referenced this issue Mar 22, 2019
flyingcircle added a commit to flyingcircle/minikube that referenced this issue Mar 22, 2019
# This is the 1st commit message:

Issue kubernetes#3253, improve kubernetes-version error string

# This is the commit message kubernetes#2:

Issue kubernetes#3253, improve kubernetes-version error string
tstromberg added a commit that referenced this issue Mar 26, 2019
Issue #3253, improve kubernetes-version error string
@tstromberg tstromberg added the r/2019q2 Issue was last reviewed 2019q2 label Apr 4, 2019
@tstromberg
Copy link
Contributor

#3766 should address this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. r/2019q2 Issue was last reviewed 2019q2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants