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

wip:Support multi control plane #9920

Closed
wants to merge 10 commits into from

Conversation

lingsamuel
Copy link
Contributor

@lingsamuel lingsamuel commented Dec 10, 2020

/kind feature

Changes:

  • fix tests - hope finished
  • fix missing InitialSetup in node add
  • fix/implement node add --control-plane
  • add flag --control-planes to config control planes number: minikube start --control-planes 2. --nodes will automatically enlarge if nodes < control-planes and outputs a warning.
  • tag primary control plane: add field PrimaryControlPlane
  • add IP and primary tag in status
  • reset control plane node before stop it, make sure primary control plane can start without others control planes
  • add some step output
  • change KindNet.String() from CNI to KindNet
  • status command: stacked control plane node kubeconfig should be irrelevant. kubeconfig still exist in stacked control plane node but configured at control-plane.minikube.internal:8443 to generate certificate keys.
  • backward compatibility
    • start from a stopped old cluster config
    • node add, node stop, node start, stop a running old cluster
    • status warn user outdated config, provides flag --update-primary-control-plane-tag to update profile.
  • multiple control planes integration tests

Fixes: #7461, part of #7538

Currently finished:

minikube start
minikube node add --control-plane # Support multi control plane
minikube stop # make sure stop && start works
minikube start
minikube node stop m02 # make sure node stop && node start works
minikube node start m02

Some output changes:

$ minikube status
minikube
type: Control Plane (Primary)
host: Running
kubelet: Running
apiserver: Running
kubeconfig: Configured
timeToStop: Nonexistent
IP: 192.168.39.13

minikube-m02
type: Control Plane
host: Running
kubelet: Running
apiserver: Running
kubeconfig: Irrelevant
timeToStop: Nonexistent
IP: 192.168.39.252

Outdated profile and status command output:

// ...
"Nodes": [
        {
            "Name": "",
            "IP": "192.168.39.164",
            "Port": 8443,
            "KubernetesVersion": "v1.19.3",
            "PrimaryControlPlane": false, // <- outdated config is false here
            "ControlPlane": true,
            "Worker": true
        }
    ],
// ...
$ minikube status
There is no primary control plane, set --update-primary-control-plane-tag=true to update profile.
minikube
type: Control Plane
host: Running
kubelet: Running
apiserver: Running
kubeconfig: Irrelevant
timeToStop: Nonexistent
IP: 192.168.39.164

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 10, 2020
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@k8s-ci-robot
Copy link
Contributor

Hi @lingsamuel. 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 needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 10, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lingsamuel
To complete the pull request process, please assign priyawadhwa after the PR has been reviewed.
You can assign the PR to them by writing /assign @priyawadhwa 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

@lingsamuel lingsamuel force-pushed the multi-control-plane branch 22 times, most recently from 60b77a7 to 9bc6c87 Compare December 10, 2020 09:54
@lingsamuel lingsamuel requested a review from tstromberg January 6, 2021 18:53
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 10, 2021
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 11, 2021
@lingsamuel
Copy link
Contributor Author

Rebased.

@lingsamuel lingsamuel force-pushed the multi-control-plane branch 2 times, most recently from 5303fa9 to b57d57b Compare January 13, 2021 12:18
@lingsamuel
Copy link
Contributor Author

Add a k8s version check, this may let tests pass.
image

Signed-off-by: Ling Samuel <[email protected]>
@medyagh
Copy link
Member

medyagh commented Jan 27, 2021

@lingsamuel thanks for updating this, I still need to verify if this PR could be accepted in minikube's vision without affection the code complexity. will do a discuss on next office hours if you like to attend on monday please we could discuss it

@medyagh medyagh added the triage/discuss Items for discussion label Jan 27, 2021
@lingsamuel
Copy link
Contributor Author

I am afraid i can't participant.

@medyagh
Copy link
Member

medyagh commented Feb 3, 2021

@lingsamuel thanks for this PR, at this release we dont have the bandwith to accept this PR but we eventually we can accept this.
the main concerns the use case for a local developement is hard to emulate, the end user would need to deploy a loadbalacner.

we decideded once our Flake Tests failure for multinode are fixed

@k8s-ci-robot
Copy link
Contributor

@lingsamuel: 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.

@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 3, 2021
@medyagh
Copy link
Member

medyagh commented Feb 10, 2021

@lingsamuel update: we discussed this in the triage meeting, we like to re-introduce this PR again after we deliver the minikube auto pause feature. #10326

for now I will close this PR and lets reopen a MEP for this feature for milestone v1.19.0
similar to https://github.com/kubernetes/minikube/tree/master/enhancements/proposed

I would like the MEP to discuss backward compatiblity, how it affects other minikube features such as pause, service, docker-env,....

@medyagh medyagh changed the title Support multi control plane wip:Support multi control plane Feb 10, 2021
@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 Feb 10, 2021
@medyagh medyagh closed this Feb 10, 2021
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. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. 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/XL Denotes a PR that changes 500-999 lines, ignoring generated files. triage/discuss Items for discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow multi control plane clusters by knowing which node is the api server
9 participants