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

Refactor config.Config to prepare for multinode #5889

Merged
merged 14 commits into from
Nov 12, 2019

Conversation

sharifelgamal
Copy link
Collaborator

This PR allows a profile to have multiple MachineConfigs, to account for multiple nodes in a cluster. It also inlines KubernetesConfig under MachineConfig, so that each node in a multinode cluster can have its own k8s config.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 11, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sharifelgamal

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

The pull request process is described 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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 11, 2019
@codecov-io
Copy link

Codecov Report

Merging #5889 into master will decrease coverage by 0.03%.
The diff coverage is 50.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5889      +/-   ##
==========================================
- Coverage   36.47%   36.44%   -0.04%     
==========================================
  Files         110      110              
  Lines        8159     8161       +2     
==========================================
- Hits         2976     2974       -2     
- Misses       4794     4796       +2     
- Partials      389      391       +2
Impacted Files Coverage Δ
cmd/minikube/cmd/config/profile_list.go 5.26% <0%> (ø) ⬆️
cmd/minikube/cmd/mount.go 8.33% <0%> (ø) ⬆️
pkg/minikube/machine/cache_images.go 5.12% <0%> (ø) ⬆️
cmd/minikube/cmd/delete.go 28.42% <0%> (ø) ⬆️
cmd/minikube/cmd/config/profile.go 0% <0%> (ø) ⬆️
cmd/minikube/cmd/logs.go 11.11% <0%> (ø) ⬆️
cmd/minikube/cmd/config/util.go 34.3% <0%> (ø) ⬆️
cmd/minikube/cmd/dashboard.go 1.76% <0%> (ø) ⬆️
cmd/minikube/cmd/ssh.go 8.69% <0%> (ø) ⬆️
cmd/minikube/cmd/tunnel.go 5.88% <0%> (ø) ⬆️
... and 8 more

@sharifelgamal
Copy link
Collaborator Author

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Nov 11, 2019
@sharifelgamal sharifelgamal requested review from medyagh and removed request for RA489 November 11, 2019 23:30
@minikube-bot
Copy link
Collaborator

Error: building minikube at head: updating minikube master branch: running [git pull origin master] in /home/performance-monitor/minikube:
From https://github.com/kubernetes/minikube

  • branch master -> FETCH_HEAD
    error: Your local changes to the following files would be overwritten by merge:
    go.mod
    Please commit your changes or stash them before you merge.
    Aborting
    Updating 1d0ca6c..fb5430f
    : exit status 1

Copy link
Contributor

@tstromberg tstromberg left a comment

Choose a reason for hiding this comment

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

It might not be perfect, but it's a start!

@sharifelgamal
Copy link
Collaborator Author

Note: we'll eventually have to deal with supporting configs in the old style, but for now this will do.

@sharifelgamal sharifelgamal merged commit c03aee8 into kubernetes:master Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. 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.

5 participants