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

Added support for "stable" and "latest"aliases for --kubernetes-version #6142

Closed
wants to merge 4 commits into from
Closed

Conversation

ghost
Copy link

@ghost ghost commented Dec 20, 2019

Added the check for the aliases.
In the issue's comment it was mentioned that the constant NewestKubernetesVersion should be 1.18.0-alpha.1 (now it is 1.17.0). I don't know if it was part of this issue so I left it unchanged.
Related to #6126

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 20, 2019
@k8s-ci-robot
Copy link
Contributor

Welcome @ths-laurent!

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 k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 20, 2019
@k8s-ci-robot
Copy link
Contributor

Hi @ths-laurent. 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/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 20, 2019
@ghost ghost changed the title Added support for "stable" and "latest"aliases for --kubernetes-version fixes##6126 Added support for "stable" and "latest"aliases for --kubernetes-version fixes#6126 Dec 20, 2019
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ths-laurent
To complete the pull request process, please assign afbjorklund
You can assign the PR to them by writing /assign @afbjorklund in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

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

@codecov-io
Copy link

codecov-io commented Dec 20, 2019

Codecov Report

Merging #6142 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6142      +/-   ##
==========================================
- Coverage   38.29%   38.27%   -0.02%     
==========================================
  Files         119      119              
  Lines        8077     8081       +4     
==========================================
  Hits         3093     3093              
- Misses       4581     4583       +2     
- Partials      403      405       +2
Impacted Files Coverage Δ
cmd/minikube/cmd/start.go 22.6% <0%> (-0.14%) ⬇️

@ghost ghost changed the title Added support for "stable" and "latest"aliases for --kubernetes-version fixes#6126 WIP:Added support for "stable" and "latest"aliases for --kubernetes-version fixes#6126 Dec 21, 2019
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 21, 2019
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 21, 2019
@ghost ghost changed the title WIP:Added support for "stable" and "latest"aliases for --kubernetes-version fixes#6126 Added support for "stable" and "latest"aliases for --kubernetes-version fixes#6126 Dec 21, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 21, 2019
@ghost ghost requested a review from sharifelgamal December 21, 2019 12:21
@ghost ghost changed the title Added support for "stable" and "latest"aliases for --kubernetes-version fixes#6126 Added support for "stable" and "latest"aliases for --kubernetes-version Dec 22, 2019
@sharifelgamal
Copy link
Collaborator

/ok-to-test

@k8s-ci-robot 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 Dec 23, 2019
@minikube-bot
Copy link
Collaborator

Error: running mkcmp: exit status 1

@minikube-pr-bot
Copy link

All Times minikube: [ 112.716941 115.992934 113.660741]
All Times Minikube (PR 6142): [ 126.837507 122.764142 126.912963]

Average minikube: 114.123539
Average Minikube (PR 6142): 125.504871

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 6142) |
+----------------------+-----------+--------------------+
| minikube v           |  0.294201 |           0.421536 |
| Creating kvm2        | 40.674218 |          47.676499 |
| Preparing Kubernetes | 47.396010 |          49.477623 |
| Pulling images       |  3.762576 |           2.899562 |
| Launching Kubernetes | 18.677894 |          20.296815 |
| Waiting for cluster  |  2.726629 |           2.888240 |
+----------------------+-----------+--------------------+

@@ -68,7 +68,7 @@ var DefaultISOSHAURL = DefaultISOURL + SHASuffix
var DefaultKubernetesVersion = "v1.17.0"

// NewestKubernetesVersion is the newest Kubernetes version to test against
var NewestKubernetesVersion = "v1.17.0"
var NewestKubernetesVersion = "v1.18.0-alpha.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mind removing the part of your PR that updates Newest to v1.18? I feel like the PR is doing too much. Otherwise, I think it's good to go.

@priyawadhwa
Copy link

Hey @ths-laurent, this PR is pretty close to merging! Are you still working on it? I'd love to get it merged, so if you are, please:

  1. Rebase on master to resolve merge conflicts
  2. Address any remaining review comments

and we can get this in!

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 5, 2020
@k8s-ci-robot
Copy link
Contributor

@ths-laurent: PR needs rebase.

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/test-infra repository.

@tstromberg
Copy link
Contributor

any hope for a rebase?

@tstromberg
Copy link
Contributor

I'm going to close this for now since it's been stale for a while. Please reopen once the conflicts are resolved.

@tstromberg tstromberg closed this Mar 18, 2020
@ghost ghost deleted the k8s-version-latest-or-stable branch March 18, 2020 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants