Move node config deletion out of drainNode and into Delete #11731

andriyDev · 2021-06-22T18:27:36Z

Previously, drainNode would delete the node from the config, but this means the machine can no longer be properly cleaned up if the node is drained for non-deletion reasons (rejoining a cluster).

This fixes 2 issues, first using an old version of the config after calling drainNode, second losing track of machines after drainNode has been called.

fixes #11687.

In particular, this PR fixes what happens when restarting a >2 multinode.

Before

$ minikube start -n 3
😄  minikube v1.21.0 on Debian rodete
✨  Automatically selected the docker driver. Other choices: kvm2, ssh
👍  Starting control plane node minikube in cluster minikube
🚜  Pulling base image ...
🔥  Creating docker container (CPUs=2, Memory=2200MB) ...
🐳  Preparing Kubernetes v1.20.7 on Docker 20.10.7 ...
    ▪ Generating certificates and keys ...
    ▪ Booting up control plane ...
    ▪ Configuring RBAC rules ...
🔗  Configuring CNI (Container Networking Interface) ...
🔎  Verifying Kubernetes components...
    ▪ Using image gcr.io/k8s-minikube/storage-provisioner:v5
🌟  Enabled addons: storage-provisioner, default-storageclass

👍  Starting node minikube-m02 in cluster minikube
🚜  Pulling base image ...
🔥  Creating docker container (CPUs=2, Memory=2200MB) ...
🌐  Found network options:
    ▪ NO_PROXY=192.168.49.2
🐳  Preparing Kubernetes v1.20.7 on Docker 20.10.7 ...
    ▪ env NO_PROXY=192.168.49.2
🔎  Verifying Kubernetes components...

👍  Starting node minikube-m03 in cluster minikube
🚜  Pulling base image ...
🔥  Creating docker container (CPUs=2, Memory=2200MB) ...
🌐  Found network options:
    ▪ NO_PROXY=192.168.49.2,192.168.49.3
🐳  Preparing Kubernetes v1.20.7 on Docker 20.10.7 ...
    ▪ env NO_PROXY=192.168.49.2
    ▪ env NO_PROXY=192.168.49.2,192.168.49.3
🔎  Verifying Kubernetes components...
🏄  Done! kubectl is now configured to use "minikube" cluster and "default" namespace by default
$ minikube stop
✋  Stopping node "minikube"  ...
🛑  Powering off "minikube" via SSH ...
✋  Stopping node "minikube-m02"  ...
🛑  Powering off "minikube-m02" via SSH ...
✋  Stopping node "minikube-m03"  ...
🛑  Powering off "minikube-m03" via SSH ...
🛑  3 nodes stopped.
$ minikube start
😄  minikube v1.21.0 on Debian rodete
✨  Using the docker driver based on existing profile
👍  Starting control plane node minikube in cluster minikube
🚜  Pulling base image ...
🔄  Restarting existing docker container for "minikube" ...
🐳  Preparing Kubernetes v1.20.7 on Docker 20.10.7 ...
🔗  Configuring CNI (Container Networking Interface) ...
🔎  Verifying Kubernetes components...
    ▪ Using image gcr.io/k8s-minikube/storage-provisioner:v5
🌟  Enabled addons: storage-provisioner, default-storageclass
👍  Starting node minikube-m02 in cluster minikube
🚜  Pulling base image ...
🔄  Restarting existing docker container for "minikube-m02" ...
🌐  Found network options:
    ▪ NO_PROXY=192.168.49.2
🐳  Preparing Kubernetes v1.20.7 on Docker 20.10.7 ...
    ▪ env NO_PROXY=192.168.49.2
🔎  Verifying Kubernetes components...
👍  Starting node minikube-m03 in cluster minikube
🚜  Pulling base image ...
🔄  Restarting existing docker container for "minikube-m03" ...
🌐  Found network options:
    ▪ NO_PROXY=192.168.49.2,192.168.49.3
🐳  Preparing Kubernetes v1.20.7 on Docker 20.10.7 ...
    ▪ env NO_PROXY=192.168.49.2
    ▪ env NO_PROXY=192.168.49.2,192.168.49.3
🔎  Verifying Kubernetes components...
🏄  Done! kubectl is now configured to use "minikube" cluster and "default" namespace by default
$ minikube node list
minikube	192.168.49.2
minikube-m03	192.168.49.4
minikube-m03	192.168.49.4

After

$ minikube start -n 3
😄  minikube v1.21.0 on Debian rodete
✨  Automatically selected the docker driver. Other choices: kvm2, ssh
👍  Starting control plane node minikube in cluster minikube
🚜  Pulling base image ...
🔥  Creating docker container (CPUs=2, Memory=2200MB) ...
🐳  Preparing Kubernetes v1.20.7 on Docker 20.10.7 ...
    ▪ Generating certificates and keys ...
    ▪ Booting up control plane ...
    ▪ Configuring RBAC rules ...
🔗  Configuring CNI (Container Networking Interface) ...
🔎  Verifying Kubernetes components...
    ▪ Using image gcr.io/k8s-minikube/storage-provisioner:v5
🌟  Enabled addons: storage-provisioner, default-storageclass

👍  Starting node minikube-m02 in cluster minikube
🚜  Pulling base image ...
🔥  Creating docker container (CPUs=2, Memory=2200MB) ...
🌐  Found network options:
    ▪ NO_PROXY=192.168.49.2
🐳  Preparing Kubernetes v1.20.7 on Docker 20.10.7 ...
    ▪ env NO_PROXY=192.168.49.2
🔎  Verifying Kubernetes components...

👍  Starting node minikube-m03 in cluster minikube
🚜  Pulling base image ...
🔥  Creating docker container (CPUs=2, Memory=2200MB) ...
🌐  Found network options:
    ▪ NO_PROXY=192.168.49.2,192.168.49.3
🐳  Preparing Kubernetes v1.20.7 on Docker 20.10.7 ...
    ▪ env NO_PROXY=192.168.49.2
    ▪ env NO_PROXY=192.168.49.2,192.168.49.3
🔎  Verifying Kubernetes components...
🏄  Done! kubectl is now configured to use "minikube" cluster and "default" namespace by default
$ minikube stop
✋  Stopping node "minikube"  ...
🛑  Powering off "minikube" via SSH ...
✋  Stopping node "minikube-m02"  ...
🛑  Powering off "minikube-m02" via SSH ...
✋  Stopping node "minikube-m03"  ...
🛑  Powering off "minikube-m03" via SSH ...
🛑  3 nodes stopped.
$ minikube start
😄  minikube v1.21.0 on Debian rodete
✨  Using the docker driver based on existing profile
👍  Starting control plane node minikube in cluster minikube
🚜  Pulling base image ...
🔄  Restarting existing docker container for "minikube" ...
🐳  Preparing Kubernetes v1.20.7 on Docker 20.10.7 ...
🔗  Configuring CNI (Container Networking Interface) ...
🔎  Verifying Kubernetes components...
    ▪ Using image gcr.io/k8s-minikube/storage-provisioner:v5
🌟  Enabled addons: storage-provisioner, default-storageclass
👍  Starting node minikube-m02 in cluster minikube
🚜  Pulling base image ...
🔄  Restarting existing docker container for "minikube-m02" ...
🌐  Found network options:
    ▪ NO_PROXY=192.168.49.2
🐳  Preparing Kubernetes v1.20.7 on Docker 20.10.7 ...
    ▪ env NO_PROXY=192.168.49.2
🔎  Verifying Kubernetes components...
👍  Starting node minikube-m03 in cluster minikube
🚜  Pulling base image ...
🔄  Restarting existing docker container for "minikube-m03" ...
🌐  Found network options:
    ▪ NO_PROXY=192.168.49.2,192.168.49.3
🐳  Preparing Kubernetes v1.20.7 on Docker 20.10.7 ...
    ▪ env NO_PROXY=192.168.49.2
    ▪ env NO_PROXY=192.168.49.2,192.168.49.3
🔎  Verifying Kubernetes components...
🏄  Done! kubectl is now configured to use "minikube" cluster and "default" namespace by default
$ minikube node list
minikube	192.168.49.2
minikube-m02	192.168.49.3
minikube-m03	192.168.49.4

andriyDev · 2021-06-22T18:28:50Z

/ok-to-test

minikube-bot · 2021-06-22T18:32:05Z

Can one of the admins verify this patch?

medyagh

this is a good catch ! how about we add an integration subtest for multinode test...that checks what u did in the PR description ?

minikube-pr-bot · 2021-06-22T19:11:59Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11731) |
+----------------+----------+---------------------+
| minikube start | 47.6s    | 47.3s               |
| enable ingress | 38.9s    | 38.7s               |
+----------------+----------+---------------------+

Times for minikube start: 49.8s 47.5s 46.8s 47.3s 46.7s
Times for minikube (PR 11731) start: 47.8s 47.7s 47.8s 46.4s 46.9s

Times for minikube (PR 11731) ingress: 35.8s 35.7s 43.2s 36.7s 42.2s
Times for minikube ingress: 35.8s 36.8s 43.3s 36.3s 42.2s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11731) |
+----------------+----------+---------------------+
| minikube start | 21.9s    | 21.5s               |
| enable ingress | 35.4s    | 37.1s               |
+----------------+----------+---------------------+

Times for minikube start: 22.4s 21.7s 21.2s 21.2s 22.8s
Times for minikube (PR 11731) start: 21.7s 21.3s 21.1s 21.4s 21.9s

Times for minikube ingress: 40.5s 33.0s 32.0s 37.5s 34.0s
Times for minikube (PR 11731) ingress: 36.5s 40.5s 37.5s 37.0s 34.0s

docker driver with containerd runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11731) |
+----------------+----------+---------------------+
| minikube start | 42.7s    | 43.6s               |
| enable ingress |          |                     |
+----------------+----------+---------------------+

Times for minikube start: 31.5s 43.6s 47.3s 43.8s 47.2s
Times for minikube (PR 11731) start: 46.5s 42.9s 42.7s 43.0s 43.0s

minikube-bot · 2021-06-22T19:28:50Z

These are the flake rates of all failed tests on Docker_Linux.

Failed Tests	Flake Rate (%)
TestFunctional/parallel/InternationalLanguage	0.00 (chart)
TestRunningBinaryUpgrade	2.50 (chart)

minikube-bot · 2021-06-22T19:32:17Z

These are the flake rates of all failed tests on Docker_Linux_containerd.

Failed Tests	Flake Rate (%)
TestFunctional/parallel/InternationalLanguage	0.00 (chart)
TestPause/serial/Pause	15.85 (chart)
TestPause/serial/VerifyStatus	15.85 (chart)
TestPause/serial/PauseAgain	20.73 (chart)

minikube-bot · 2021-06-22T19:49:46Z

These are the flake rates of all failed tests on KVM_Linux_containerd.

Failed Tests	Flake Rate (%)
TestAddons/parallel/Registry	0.00 (chart)
TestFunctional/parallel/InternationalLanguage	0.00 (chart)

minikube-bot · 2021-06-22T19:50:40Z

These are the flake rates of all failed tests on KVM_Linux.

Failed Tests	Flake Rate (%)
TestFunctional/parallel/InternationalLanguage	0.00 (chart)
TestAddons/parallel/Olm	2.50 (chart)

Previously, drainNode would delete the node from the config, but this means the machine can no longer be properly cleaned up if the node is drained for non-deletion reasons (rejoining a cluster).

andriyDev · 2021-06-22T21:21:16Z

Created new integration test - tested on master and breaks, tested with this PR and works! Also rebased to get rid of old breakages.

minikube-pr-bot · 2021-06-22T22:06:52Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11731) |
+----------------+----------+---------------------+
| minikube start | 47.8s    | 47.9s               |
| enable ingress | 37.2s    | 34.9s               |
+----------------+----------+---------------------+

Times for minikube start: 50.8s 46.2s 47.0s 46.5s 48.6s
Times for minikube (PR 11731) start: 46.5s 47.2s 46.1s 46.3s 53.4s

Times for minikube ingress: 36.2s 42.3s 36.1s 36.7s 34.7s
Times for minikube (PR 11731) ingress: 34.7s 34.7s 35.3s 34.7s 34.8s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11731) |
+----------------+----------+---------------------+
| minikube start | 21.6s    | 21.2s               |
| enable ingress | 36.1s    | 34.6s               |
+----------------+----------+---------------------+

Times for minikube (PR 11731) start: 20.9s 21.4s 20.8s 21.2s 21.5s
Times for minikube start: 21.0s 22.9s 21.6s 21.3s 21.5s

Times for minikube ingress: 38.0s 33.5s 37.5s 33.5s 38.0s
Times for minikube (PR 11731) ingress: 33.0s 33.0s 33.5s 36.0s 37.5s

docker driver with containerd runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11731) |
+----------------+----------+---------------------+
| minikube start | 45.7s    | 44.9s               |
| enable ingress |          |                     |
+----------------+----------+---------------------+

Times for minikube start: 47.2s 43.1s 48.0s 47.2s 43.1s
Times for minikube (PR 11731) start: 42.7s 46.6s 43.1s 44.4s 47.6s

minikube-bot · 2021-06-22T22:29:37Z

These are the flake rates of all failed tests on Docker_Linux_containerd.

Failed Tests	Flake Rate (%)
TestFunctional/parallel/LogsCmd	4.65 (chart)
TestFunctional/parallel/LogsFileCmd	4.65 (chart)

minikube-pr-bot · 2021-06-22T22:39:08Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11731) |
+----------------+----------+---------------------+
| minikube start | 47.6s    | 46.5s               |
| enable ingress | 34.6s    | 36.1s               |
+----------------+----------+---------------------+

Times for minikube ingress: 34.2s 33.7s 36.2s 34.2s 34.7s
Times for minikube (PR 11731) ingress: 35.8s 34.2s 34.8s 41.7s 34.2s

Times for minikube start: 48.4s 47.7s 47.1s 47.5s 47.1s
Times for minikube (PR 11731) start: 45.9s 46.2s 46.7s 47.3s 46.5s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11731) |
+----------------+----------+---------------------+
| minikube start | 22.0s    | 21.6s               |
| enable ingress | 34.3s    | 35.9s               |
+----------------+----------+---------------------+

Times for minikube start: 22.3s 21.5s 22.3s 21.5s 22.7s
Times for minikube (PR 11731) start: 21.6s 21.5s 21.4s 22.0s 21.6s

Times for minikube ingress: 34.5s 34.5s 34.0s 33.5s 35.0s
Times for minikube (PR 11731) ingress: 34.0s 37.5s 35.5s 34.5s 38.0s

docker driver with containerd runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11731) |
+----------------+----------+---------------------+
| minikube start | 44.0s    | 46.4s               |
| enable ingress |          |                     |
+----------------+----------+---------------------+

Times for minikube start: 47.5s 42.7s 43.0s 43.8s 43.0s
Times for minikube (PR 11731) start: 43.9s 47.9s 49.2s 43.8s 46.9s

minikube-bot · 2021-06-22T22:53:32Z

These are the flake rates of all failed tests on KVM_Linux_containerd.

Failed Tests	Flake Rate (%)
TestPause/serial/PauseAgain	16.67 (chart)

minikube-bot · 2021-06-23T00:11:32Z

These are the flake rates of all failed tests on KVM_Linux_containerd.

Failed Tests	Flake Rate (%)
TestPause/serial/PauseAgain	16.67 (chart)

andriyDev · 2021-06-23T17:00:52Z

/retest-this-please

minikube-pr-bot · 2021-06-23T18:47:50Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11731) |
+----------------+----------+---------------------+
| minikube start | 47.1s    | 47.7s               |
| enable ingress | 34.0s    | 35.2s               |
+----------------+----------+---------------------+

Times for minikube (PR 11731) start: 46.4s 46.2s 52.6s 47.4s 45.9s
Times for minikube start: 48.3s 47.3s 45.3s 47.3s 47.0s

Times for minikube ingress: 34.2s 34.2s 32.7s 35.2s 33.8s
Times for minikube (PR 11731) ingress: 33.8s 35.7s 35.8s 34.3s 36.2s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11731) |
+----------------+----------+---------------------+
| minikube start | 21.6s    | 21.4s               |
| enable ingress | 29.6s    | 28.7s               |
+----------------+----------+---------------------+

Times for minikube start: 22.5s 21.3s 21.1s 21.2s 21.8s
Times for minikube (PR 11731) start: 21.6s 20.8s 22.4s 20.7s 21.8s

Times for minikube ingress: 28.0s 29.0s 29.0s 30.9s 31.0s
Times for minikube (PR 11731) ingress: 26.5s 28.5s 29.0s 30.0s 29.5s

docker driver with containerd runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11731) |
+----------------+----------+---------------------+
| minikube start | 41.7s    | 43.9s               |
| enable ingress |          |                     |
+----------------+----------+---------------------+

Times for minikube start: 31.9s 42.7s 42.9s 46.9s 43.8s
Times for minikube (PR 11731) start: 42.6s 46.9s 43.0s 43.2s 43.6s

minikube-bot · 2021-06-23T18:55:50Z

These are the flake rates of all failed tests on Docker_Linux_containerd.

Failed Tests	Flake Rate (%)
TestPause/serial/PauseAgain	20.65 (chart)

k8s-ci-robot · 2021-06-24T17:23:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andriyDev, 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:

~~OWNERS~~ [sharifelgamal]

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 added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 22, 2021

k8s-ci-robot requested review from blueelvis and prasadkatti June 22, 2021 18:28

k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Jun 22, 2021

medyagh reviewed Jun 22, 2021

View reviewed changes

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 22, 2021

andriyDev added 2 commits June 22, 2021 14:20

Move node config deletion out of drainNode and into Delete.

1aae098

Previously, drainNode would delete the node from the config, but this means the machine can no longer be properly cleaned up if the node is drained for non-deletion reasons (rejoining a cluster).

Create integration test for multinode restart.

ae8cfa9

andriyDev force-pushed the fix-multinode-restart branch from 15fcba1 to ae8cfa9 Compare June 22, 2021 21:20

Run make generate-docs.

5727823

medyagh mentioned this pull request Jun 23, 2021

is KVM crio test results over-writting KVM Linux test results? #11751

Closed

medyagh mentioned this pull request Jun 23, 2021

Istio addon not working on multinode #10248

Closed

sharifelgamal approved these changes Jun 24, 2021

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 24, 2021

sharifelgamal merged commit 16d2f2a into kubernetes:master Jun 24, 2021

andriyDev deleted the fix-multinode-restart branch June 25, 2021 16:20

medyagh mentioned this pull request Jun 30, 2021

multi-nodes-minikube loss node label after restart #11705

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move node config deletion out of drainNode and into Delete #11731

Move node config deletion out of drainNode and into Delete #11731

andriyDev commented Jun 22, 2021 •

edited

Loading

andriyDev commented Jun 22, 2021

minikube-bot commented Jun 22, 2021

medyagh left a comment

minikube-pr-bot commented Jun 22, 2021

minikube-bot commented Jun 22, 2021

minikube-bot commented Jun 22, 2021

minikube-bot commented Jun 22, 2021

minikube-bot commented Jun 22, 2021

andriyDev commented Jun 22, 2021

minikube-pr-bot commented Jun 22, 2021

minikube-bot commented Jun 22, 2021

minikube-pr-bot commented Jun 22, 2021

minikube-bot commented Jun 22, 2021

minikube-bot commented Jun 23, 2021

andriyDev commented Jun 23, 2021

minikube-pr-bot commented Jun 23, 2021

minikube-bot commented Jun 23, 2021

k8s-ci-robot commented Jun 24, 2021

Move node config deletion out of drainNode and into Delete #11731

Move node config deletion out of drainNode and into Delete #11731

Conversation

andriyDev commented Jun 22, 2021 • edited Loading

andriyDev commented Jun 22, 2021

minikube-bot commented Jun 22, 2021

medyagh left a comment

Choose a reason for hiding this comment

minikube-pr-bot commented Jun 22, 2021

minikube-bot commented Jun 22, 2021

minikube-bot commented Jun 22, 2021

minikube-bot commented Jun 22, 2021

minikube-bot commented Jun 22, 2021

andriyDev commented Jun 22, 2021

minikube-pr-bot commented Jun 22, 2021

minikube-bot commented Jun 22, 2021

minikube-pr-bot commented Jun 22, 2021

minikube-bot commented Jun 22, 2021

minikube-bot commented Jun 23, 2021

andriyDev commented Jun 23, 2021

minikube-pr-bot commented Jun 23, 2021

minikube-bot commented Jun 23, 2021

k8s-ci-robot commented Jun 24, 2021

andriyDev commented Jun 22, 2021 •

edited

Loading