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.

Sign up for GitHub

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

Jump to bottom

Add user flag to all minikube command invocations #5649

Merged
merged 8 commits into from
Apr 13, 2021
Merged

Add user flag to all minikube command invocations #5649

merged 8 commits into from
Apr 13, 2021

Conversation

PriyaModali
Copy link
Contributor

@PriyaModali PriyaModali commented Apr 8, 2021 •
edited
Loading

Fixes: #5478

Add --user=skaffold flag to all minikube command invocations.

In v1.8.0, minikube team added a flag --user to audit commands.

We should use this flag, if user is on v1.8.0 or higher.

@google-cla google-cla bot added the cla: yes label Apr 8, 2021
@codecov
Copy link

codecov bot commented Apr 8, 2021 •
edited
Loading

Codecov Report

Merging #5649 (e26cfff) into master (5e2a4ec) will increase coverage by 0.01%.
The diff coverage is 73.91%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5649      +/-   ##
==========================================
+ Coverage   70.63%   70.64%   +0.01%     
==========================================
  Files         411      411              
  Lines       15777    15793      +16     
==========================================
+ Hits        11144    11157      +13     
- Misses       3812     3814       +2     
- Partials      821      822       +1
Impacted Files Coverage Δ
pkg/skaffold/cluster/minikube.go 71.64% <73.91%> (+3.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5e2a4ec...e26cfff. Read the comment docs.

@pull-request-size pull-request-size bot added size/M and removed size/L labels Apr 8, 2021
tejal29
tejal29 reviewed Apr 8, 2021
View reviewed changes
pkg/skaffold/cluster/minikube.go Outdated Show resolved Hide resolved
pkg/skaffold/cluster/minikube.go Outdated Show resolved Hide resolved
pkg/skaffold/cluster/minikube.go Outdated Show resolved Hide resolved
pkg/skaffold/cluster/minikube.go Outdated Show resolved Hide resolved
@pull-request-size pull-request-size bot added size/L and removed size/M labels Apr 10, 2021
@PriyaModali PriyaModali marked this pull request as ready for review April 10, 2021 23:56
@PriyaModali PriyaModali requested a review from a team as a code owner April 10, 2021 23:56
briandealwis
briandealwis approved these changes Apr 13, 2021
View reviewed changes
Copy link
Member

@briandealwis briandealwis left a comment

Choose a reason for hiding this comment

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

Just a few small things to address.

pkg/skaffold/cluster/minikube.go Outdated Show resolved Hide resolved
pkg/skaffold/cluster/minikube.go Outdated Show resolved Hide resolved
pkg/skaffold/cluster/minikube_test.go Show resolved Hide resolved
pkg/skaffold/cluster/minikube_test.go Outdated Show resolved Hide resolved
pkg/skaffold/cluster/minikube_test.go Outdated Show resolved Hide resolved
pkg/skaffold/cluster/minikube_test.go Outdated Show resolved Hide resolved
pkg/skaffold/test/structure/structure_test.go Outdated Show resolved Hide resolved
@PriyaModali PriyaModali merged commit 1ef4fd8 into GoogleContainerTools:master Apr 13, 2021
gsquared94 added a commit that referenced this pull request Apr 15, 2021
@gsquared94
Revert "Add user flag to all minikube command invocations (#5649)"
4ee4c46 
This reverts commit 1ef4fd8.
This was referenced Apr 15, 2021
Closed
Merged
@briandealwis
Copy link
Member

Unfortunately this change broke the minikube support. The output of minikube version is not a single semver version.

$ minikube version
minikube version: v1.18.1
commit: 09ee84d530de4a92f00f1c5dbc34cead092b95bc

This PR should:

  • use minikube version --short — or perhaps minikube version -o json
  • add explicit tests for different inputs
  • treat an unparseable result from minikube version as an old version of minikube.

gsquared94 added a commit that referenced this pull request Apr 15, 2021
@gsquared94
Revert mk user flag (#5682)
f247da4 
* Revert "Add user flag to all minikube command invocations (#5649)"

This reverts commit 1ef4fd8.

* fix test
@gsquared94 gsquared94 mentioned this pull request Apr 15, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsquared94 gsquared94 gsquared94 left review comments

@tejal29 tejal29 tejal29 left review comments

@briandealwis briandealwis briandealwis approved these changes

Assignees
No one assigned
Labels
cla: yes platform/minikube size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add --user=skaffold flag to all minikube command invokations.
4 participants
@PriyaModali @briandealwis @tejal29 @gsquared94