Stop K8s if running, when '--no-kubernetes' flag used #13072

klaases · 2021-12-02T22:40:11Z

If the user starts minikube with Kubernetes, and then later starts minikube with "--no-kubernetes" flag, stop Kubernetes.

Before:

minikube start

😄  minikube v1.24.0 on Darwin 11.6.1
✨  Automatically selected the docker driver
👍  Starting control plane node minikube in cluster minikube
🚜  Pulling base image ...
🔥  Creating docker container (CPUs=2, Memory=1985MB) ...
🐳  Preparing Kubernetes v1.22.4 on Docker 20.10.8 ...
    ▪ Generating certificates and keys ...
    ▪ Booting up control plane ...
    ▪ Configuring RBAC rules ...
🔎  Verifying Kubernetes components...
    ▪ Using image gcr.io/k8s-minikube/storage-provisioner:v5
🌟  Enabled addons: storage-provisioner, default-storageclass
🏄  Done! kubectl is now configured to use "minikube" cluster and "default" namespace by default

minikube status

type: Control Plane
host: Running
kubelet: Running
apiserver: Running
kubeconfig: Configured

minikube start --no-kubernetes

😄  minikube v1.24.0 on Darwin 11.6.1
✨  Using the docker driver based on existing profile
👍  Starting minikube without Kubernetes minikube in cluster minikube
🚜  Pulling base image ...
🏃  Updating the running docker "minikube" container ...
🏄  Done! minikube is ready without Kubernetes!
╭───────────────────────────────────────────────────────────────────────────────────────╮
│                                                                                       │
│                       💡  Things to try without Kubernetes ...                        │
│                                                                                       │
│    - "minikube ssh" to SSH into minikube's node.                                      │
│    - "minikube docker-env" to point your docker-cli to the docker inside minikube.    │
│    - "minikube image" to build images without docker.                                 │
│                                                                                       │
╰───────────────────────────────────────────────────────────────────────────────────────╯

minikube status

type: Control Plane
host: Running
kubelet: Running
apiserver: Running
kubeconfig: Configured

After:

minikube start

😄  minikube v1.24.0 on Darwin 11.6.1
✨  Automatically selected the docker driver
👍  Starting control plane node minikube in cluster minikube
🚜  Pulling base image ...
🔥  Creating docker container (CPUs=2, Memory=1985MB) ...
🐳  Preparing Kubernetes v1.22.4 on Docker 20.10.8 ...
    ▪ Generating certificates and keys ...
    ▪ Booting up control plane ...
    ▪ Configuring RBAC rules ...
🔎  Verifying Kubernetes components...
    ▪ Using image gcr.io/k8s-minikube/storage-provisioner:v5
🌟  Enabled addons: storage-provisioner, default-storageclass
🏄  Done! kubectl is now configured to use "minikube" cluster and "default" namespace by default

minikube status

minikube
type: Control Plane
host: Running
kubelet: Running
apiserver: Running
kubeconfig: Configured

minikube start --no-kubernetes

😄  minikube v1.24.0 on Darwin 11.6.1
✨  Using the docker driver based on existing profile
👍  Starting minikube without Kubernetes minikube in cluster minikube
🚜  Pulling base image ...
🏃  Updating the running docker "minikube" container ...
    ▪ Kubernetes: Stopping ...
    ▪ Kubernetes: Stopped
🏄  Done! minikube is ready without Kubernetes!
╭───────────────────────────────────────────────────────────────────────────────────────╮
│                                                                                       │
│                       💡  Things to try without Kubernetes ...                        │
│                                                                                       │
│    - "minikube ssh" to SSH into minikube's node.                                      │
│    - "minikube docker-env" to point your docker-cli to the docker inside minikube.    │
│    - "minikube image" to build images without docker.                                 │
│                                                                                       │
╰───────────────────────────────────────────────────────────────────────────────────────╯

minikube status

minikube
type: Control Plane
host: Running
kubelet: Stopped
apiserver: Stopped
kubeconfig: Configured

k8s-ci-robot · 2021-12-02T22:40:19Z

Hi @klaases. 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.

minikube-bot · 2021-12-02T22:48:05Z

Can one of the admins verify this patch?

medyagh

lets add before and after this PR in the description for easier PR review

medyagh · 2021-12-03T00:06:42Z

pkg/minikube/bootstrapper/kubeadm/kubeadm.go

@@ -801,6 +801,30 @@ func (k *Bootstrapper) GenerateToken(cc config.ClusterConfig) (string, error) {
 	return joinCmd, nil
 }

+// StopKubernetes stops existing kubernetes.
+func StopKubernetes(stop bool, cr cruntime.Manager, k8s config.KubernetesConfig, runner command.Runner) {


I suggest we still return the error and the caller should log the error with klog.Warn that only shows up in verbose logs

Hi, I have updated this function. In this case the function does not return a value, and we are retaining the logging of the original error:

klog.Warningf("stop kubelet: %v", err)

Have added additional out.Infof to indicate the Kubernetes status a the end of the function as well.

klaases

Have updated description and resolved lint errors, including resolving the cyclomatic complexity.

klaases · 2021-12-03T22:58:20Z

pkg/minikube/bootstrapper/kubeadm/kubeadm.go

@@ -801,6 +801,30 @@ func (k *Bootstrapper) GenerateToken(cc config.ClusterConfig) (string, error) {
 	return joinCmd, nil
 }

+// StopKubernetes stops existing kubernetes.
+func StopKubernetes(stop bool, cr cruntime.Manager, k8s config.KubernetesConfig, runner command.Runner) {


Hi, I have updated this function. In this case the function does not return a value, and we are retaining the logging of the original error:

klog.Warningf("stop kubelet: %v", err)

Have added additional out.Infof to indicate the Kubernetes status a the end of the function as well.

medyagh · 2021-12-03T23:07:30Z

test/integration/no_kubernetes_test.go

+	}
+}
+
+// validateStartWithStopK8s starts a minikube cluster while stopping Kubernetes.


Add more comment of all the thing this test does
Since it's used in auto generated docs.
This test also deletes . So comment should say all things it does in the test and maybe rename subtest to include delete in it too

klaases · 2021-12-04T00:00:25Z

Refactor of handleAPIServer() to reduce cyclomatic complexity of node.Start(), in order satisfy lint checking.

medyagh · 2021-12-04T21:52:05Z

/ok-to-test

minikube-pr-bot · 2021-12-04T23:26:36Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 13072) |
+----------------+----------+---------------------+
| minikube start | 46.2s    | 46.2s               |
| enable ingress | 30.9s    | 31.8s               |
+----------------+----------+---------------------+

Times for minikube ingress: 30.8s 31.7s 30.2s 31.3s 30.3s
Times for minikube (PR 13072) ingress: 36.2s 31.8s 30.2s 30.7s 30.2s

Times for minikube start: 47.0s 45.5s 45.7s 45.5s 47.1s
Times for minikube (PR 13072) start: 46.0s 45.1s 46.7s 46.1s 47.3s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 13072) |
+----------------+----------+---------------------+
| minikube start | 21.0s    | 20.9s               |
| enable ingress | 37.5s    | 39.2s               |
+----------------+----------+---------------------+

Times for minikube start: 22.0s 21.0s 20.6s 20.8s 20.9s
Times for minikube (PR 13072) start: 20.6s 21.0s 21.2s 21.4s 20.4s

Times for minikube (PR 13072) ingress: 81.9s 26.4s 34.9s 26.4s 26.4s
Times for minikube ingress: 26.5s 24.9s 82.4s 26.9s 26.9s

docker driver with containerd runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 13072) |
+----------------+----------+---------------------+
| minikube start | 41.1s    | 38.2s               |
| enable ingress | 24.0s    | 26.1s               |
+----------------+----------+---------------------+

Times for minikube start: 40.8s 41.3s 40.1s 40.9s 42.5s
Times for minikube (PR 13072) start: 41.4s 42.0s 40.6s 42.1s 24.7s

Times for minikube ingress: 18.5s 32.4s 17.4s 32.4s 19.4s
Times for minikube (PR 13072) ingress: 27.9s 19.4s 17.9s 32.4s 32.9s

minikube-pr-bot · 2021-12-05T00:43:57Z

These are the flake rates of all failed tests.

Environment	Failed Tests	Flake Rate (%)
Docker_macOS	TestNoKubernetes/serial/StartWithK8s (gopogh)	n/a
Docker_macOS	TestNoKubernetes/serial/StartWithStopK8s (gopogh)	n/a
Docker_Windows	TestNoKubernetes/serial/StartWithK8s (gopogh)	n/a
Docker_Windows	TestNoKubernetes/serial/StartWithStopK8s (gopogh)	n/a
Hyper-V_Windows	TestNoKubernetes/serial/StartWithK8s (gopogh)	n/a
Hyper-V_Windows	TestStartStop/group/default-k8s-different-port/serial/Pause (gopogh)	0.79 (chart)
Hyper-V_Windows	TestStartStop/group/no-preload/serial/Pause (gopogh)	1.57 (chart)
Hyper-V_Windows	TestStartStop/group/newest-cni/serial/Pause (gopogh)	2.36 (chart)
Hyperkit_macOS	TestNetworkPlugins/group/kindnet/Start (gopogh)	5.13 (chart)
Docker_macOS	TestMissingContainerUpgrade (gopogh)	6.57 (chart)
Docker_Cloud_Shell	TestFunctional/parallel/TunnelCmd/serial/WaitService/IngressIP (gopogh)	6.73 (chart)
Docker_Cloud_Shell	TestFunctional/parallel/TunnelCmd/serial/AccessDirect (gopogh)	7.62 (chart)
Hyperkit_macOS	TestStartStop/group/no-preload/serial/AddonExistsAfterStop (gopogh)	7.69 (chart)
Docker_macOS	TestStoppedBinaryUpgrade/MinikubeLogs (gopogh)	10.95 (chart)
Docker_macOS	TestStoppedBinaryUpgrade/Upgrade (gopogh)	10.95 (chart)
Hyperkit_macOS	TestStartStop/group/embed-certs/serial/DeployApp (gopogh)	11.11 (chart)
Hyperkit_macOS	TestStartStop/group/no-preload/serial/DeployApp (gopogh)	11.11 (chart)
Hyperkit_macOS	TestStartStop/group/no-preload/serial/UserAppExistsAfterStop (gopogh)	12.82 (chart)
Docker_macOS	TestFunctional/parallel/DashboardCmd (gopogh)	14.60 (chart)
Docker_macOS	TestKubernetesUpgrade (gopogh)	14.60 (chart)
Docker_macOS	TestNoKubernetes/serial/ProfileList (gopogh)	16.06 (chart)
Docker_macOS	TestNoKubernetes/serial/Start (gopogh)	16.06 (chart)
Docker_macOS	TestNoKubernetes/serial/Stop (gopogh)	16.06 (chart)
Docker_macOS	TestPause/serial/SecondStartNoReconfiguration (gopogh)	16.06 (chart)
Docker_macOS	TestPause/serial/Start (gopogh)	16.06 (chart)
Docker_macOS	TestPause/serial/Unpause (gopogh)	16.06 (chart)
Docker_macOS	TestPause/serial/VerifyDeletedResources (gopogh)	16.06 (chart)
Docker_macOS	TestPause/serial/Pause (gopogh)	16.79 (chart)
Docker_macOS	TestPause/serial/PauseAgain (gopogh)	16.79 (chart)
Docker_macOS	TestPause/serial/VerifyStatus (gopogh)	16.79 (chart)
More tests...	Continued...

Too many tests failed - See test logs for more details.

To see the flake rates of all tests by environment, click here.

klaases · 2021-12-06T18:48:05Z

/assign

k8s-ci-robot · 2021-12-06T20:49:23Z

[APPROVALNOTIFIER] This PR is APPROVED

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

Stop K8s if running, when --no-kubernetes.

3074132

k8s-ci-robot added 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. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 2, 2021

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 2, 2021

k8s-ci-robot requested review from blueelvis and prasadkatti December 2, 2021 22:40

Merge branch 'master' into kstop1

e468a42

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 2, 2021

fix spacing

5b4e429

remove 123 dev comment

31a0b6d

medyagh reviewed Dec 2, 2021

View reviewed changes

resolve linter errors

a156f26

medyagh reviewed Dec 3, 2021

View reviewed changes

klaases added 4 commits December 3, 2021 11:22

Refactor Start() to reduce cyclomatic complexity.

c6fc55a

Detect whether k8s needs to be stopped.

f3186d0

Fix check for stopping k8s.

47a280b

Remove extra space.

3ac94ae

klaases commented Dec 3, 2021

View reviewed changes

medyagh requested changes Dec 3, 2021

View reviewed changes

medyagh changed the title ~~Stop K8s if running, when --no-kubernetes.~~ Stop K8s if running, when '--no-kubernetes' flag used Dec 3, 2021

klaases added 2 commits December 3, 2021 15:42

Add tuple to make sure we handle error properly.

a37ea8e

Add test documentation.

1bae4f1

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 4, 2021

klaases requested a review from medyagh December 6, 2021 18:40

k8s-ci-robot assigned klaases Dec 6, 2021

sharifelgamal approved these changes Dec 6, 2021

View reviewed changes

sharifelgamal merged commit 30afddc into kubernetes:master Dec 6, 2021

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

klaases mentioned this pull request Jan 5, 2022

REQUEST: New membership for klaases kubernetes/org#3162

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stop K8s if running, when '--no-kubernetes' flag used #13072

Stop K8s if running, when '--no-kubernetes' flag used #13072

klaases commented Dec 2, 2021 •

edited

Loading

k8s-ci-robot commented Dec 2, 2021

minikube-bot commented Dec 2, 2021

medyagh left a comment

medyagh Dec 3, 2021

klaases Dec 3, 2021

klaases left a comment

klaases Dec 3, 2021

medyagh Dec 3, 2021

klaases commented Dec 4, 2021

medyagh commented Dec 4, 2021

minikube-pr-bot commented Dec 4, 2021

minikube-pr-bot commented Dec 5, 2021

klaases commented Dec 6, 2021

k8s-ci-robot commented Dec 6, 2021

Stop K8s if running, when '--no-kubernetes' flag used #13072

Stop K8s if running, when '--no-kubernetes' flag used #13072

Conversation

klaases commented Dec 2, 2021 • edited Loading

k8s-ci-robot commented Dec 2, 2021

minikube-bot commented Dec 2, 2021

medyagh left a comment

Choose a reason for hiding this comment

medyagh Dec 3, 2021

Choose a reason for hiding this comment

klaases Dec 3, 2021

Choose a reason for hiding this comment

klaases left a comment

Choose a reason for hiding this comment

klaases Dec 3, 2021

Choose a reason for hiding this comment

medyagh Dec 3, 2021

Choose a reason for hiding this comment

klaases commented Dec 4, 2021

medyagh commented Dec 4, 2021

minikube-pr-bot commented Dec 4, 2021

minikube-pr-bot commented Dec 5, 2021

klaases commented Dec 6, 2021

k8s-ci-robot commented Dec 6, 2021

klaases commented Dec 2, 2021 •

edited

Loading