Add KubeVirt addon #8275

ashleyschuett · 2020-05-26T11:59:14Z

Description

This PR fixes #2178

It adds the KubeVirt operator as an addon that can be enabled, as well as allowing users to separately create an instance of the KubeVirt custom resource to deploy the full environment.

k8s-ci-robot · 2020-05-26T11:59:21Z

Welcome @ashleyschuett!

It looks like this is your first PR to kubernetes/minikube 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/minikube has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2020-05-26T11:59:22Z

Hi @ashleyschuett. 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 · 2020-05-26T11:59:29Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

minikube-bot · 2020-05-26T12:03:03Z

Can one of the admins verify this patch?

codecov-commenter · 2020-05-26T12:08:20Z

Codecov Report

Merging #8275 into master will decrease coverage by 0.08%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #8275      +/-   ##
==========================================
- Coverage   34.46%   34.37%   -0.09%     
==========================================
  Files         147      147              
  Lines        9428     9469      +41     
==========================================
+ Hits         3249     3255       +6     
- Misses       5780     5814      +34     
- Partials      399      400       +1

Impacted Files	Coverage Δ
cmd/minikube/cmd/docker-env.go	`41.59% <0.00%> (-7.89%)`	⬇️
cmd/minikube/cmd/start.go	`13.51% <0.00%> (-0.22%)`	⬇️
pkg/minikube/cluster/ip.go	`0.00% <0.00%> (ø)`
pkg/minikube/machine/fix.go	`38.31% <0.00%> (ø)`
pkg/minikube/machine/start.go	`62.67% <0.00%> (ø)`
pkg/minikube/driver/driver_linux.go	`0.00% <0.00%> (ø)`
cmd/minikube/cmd/start_flags.go	`50.80% <0.00%> (+0.96%)`	⬆️
pkg/minikube/machine/ssh.go	`44.44% <0.00%> (+1.58%)`	⬆️
cmd/minikube/cmd/ssh.go	`15.00% <0.00%> (+2.50%)`	⬆️
pkg/minikube/cruntime/cruntime.go	`60.60% <0.00%> (+2.54%)`	⬆️

ashleyschuett · 2020-05-26T12:56:51Z

/assign @sharifelgamal

medyagh

thank you for adding this addon, this looks good just minior nits !

medyagh · 2020-05-26T23:24:28Z

deploy/addons/kubevirt/README.md

@@ -0,0 +1,45 @@
+## kubevirt Addon


@ashleyschuett
thank you for adding this readme, could u please this read me to our site section ?
maybe under tutorials ?

Hey, I added the readme under the tutorials section. Would you like the readme in the deploy folder to be removed, or is it okay for it to live in both places?

thank you we can have a readme in the addon folder but with a link to the website, so anyone goes to that can find out where is the tutorial for this addon.

fabiand

@ashleyschuett it is awesome to finally see this PR. Many thanks.

A few notes inside.

fabiand · 2020-05-27T08:49:07Z

deploy/addons/kubevirt/README.md

+To disable this addon, simply run:
+```shell script
+minikube addons disable kubevirt
+minikube addons disable kubevirt-provisioner


@ashleyschuett what about sqashing these two commands into one (just kubevirt).
If somebody wants to separate operator deployment from kubevirt deployment then the user can just do it manually using the yamls.

After looking into this a bit and trying to get this to work I am unable to.

The issue is if we enable kubevirt with a single command, it then also needs to be able to be disabled. When disabling all the manifests get removed at the same time. In this case, the operator deletes much quicker than the KubeVirt CR. When this happens the CR will still have a finalizer and will get stuck in a terminating state along with the namespace since the operator is gone and unable to remove it.

I attempted to hook into the "preStop" lifecycle of the container but while the pod will then wait to delete it will still be in the "terminating" state meaning that the validating webhook will not respond and the kubevirt cr will not delete.

Because of this, I think the options are to keep the deployments/commands as is. Or if we change them to be kubevirt and kubevirt-with-emulation then in the read me we have the user create the kubevirt CR, and let them know to remove it before disabling the addon. Does either of the options sound okay? Or is there another way to get around the previously mentioned issue?

Nice catch. And sad.

Now, I dislike to say it, but what about writing a pod which does all this:

Deployment on creation and start

Undeploy on deletion of the pod

The reason that I think that this could work is, because we can set a terminationGrace period on the pod - which gives the pod some time to complete - and inside the pod we can just serialize the deleition of the CR and the followed deletion of the operator.

What do you think?

I think what you mean by this is that the enable/disable would only be deploying a pod, which would then have / run a script that would match the minikube quick start documentation we have? Then on disable delete the resources in the proper order?

The only problem I see what that is that namespace would still need to be created first in the manifests, and it would then also be deleted first which would mean that the operator deployment could be deleted before the pod runs the deletion.

I have changed the minikube code to delete resources in the opposite that they are created in, but i think that would need buy in from more people in this project.

I have update the addon to use your suggestions, let me know what you think!

fabiand · 2020-05-27T08:49:42Z

site/content/en/docs/tutorials/kubevirt.md

+```
+If the command does not generate any output, create the following ConfigMap so that KubeVirt uses emulation mode, otherwise skip to the next section:
+```shell script
+kubectl create configmap kubevirt-config -n kubevirt --from-literal debug.useEmulation=true


As discussed:

We can consider doing this with two options:

kubevirt -- regular
kubevirt -with-emmulation -- if emualation is needed to workaround missing kvm

medyagh · 2020-05-28T00:21:42Z

site/content/en/docs/tutorials/kubevirt.md

@@ -0,0 +1,60 @@
+---
+title: "KubeVirt support"


I would change this to a
How To... format, for example how to use KubeVirt with minikube

this has been updated! and the PR is ready for a rereview!

fabiand

@ashleyschuett this looks good. If this works, then this is pretty cool!

fabiand · 2020-06-02T09:07:55Z

deploy/addons/kubevirt/pod.yaml.tmpl

+  install.sh: |
+    #!/bin/bash
+
+    export KUBEVIRT_VERSION=$(curl -s https://api.github.com/repos/kubevirt/kubevirt/releases | grep tag_name | grep -v -- - | sort -V | tail -1 | awk -F':' '{print $2}' | sed 's/,//' | xargs)


Ha. Nifty, nice to see that this is not the naive approach of retrieving the highest version :)

sharifelgamal · 2020-06-03T18:43:54Z

/ok-to-test

minikube-pr-bot · 2020-06-03T18:49:37Z

kvm2 Driver
docker Driver

medyagh

looks good to me, thank you for adding this addon @ashleyschuett

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 26, 2020

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 26, 2020

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label May 26, 2020

k8s-ci-robot requested review from blueelvis and priyawadhwa May 26, 2020 11:59

k8s-ci-robot assigned sharifelgamal May 26, 2020

medyagh reviewed May 26, 2020

View reviewed changes

fabiand reviewed May 27, 2020

View reviewed changes

medyagh reviewed May 28, 2020

View reviewed changes

Add KubeVirt addon

5dafe6b

ashleyschuett force-pushed the add-kubevirt-addon branch from a616a80 to 5dafe6b Compare May 29, 2020 07:53

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 29, 2020

fabiand approved these changes Jun 2, 2020

View reviewed changes

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 Jun 3, 2020

medyagh approved these changes Jun 3, 2020

View reviewed changes

medyagh merged commit 4eb09e7 into kubernetes:master Jun 3, 2020

ashleyschuett deleted the add-kubevirt-addon branch June 4, 2020 06:45

zcahana mentioned this pull request Nov 10, 2021

kubevirt addon: use KubeVirt CR instead of deprecated ConfigMap #12921

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add KubeVirt addon #8275

Add KubeVirt addon #8275

ashleyschuett commented May 26, 2020

k8s-ci-robot commented May 26, 2020

k8s-ci-robot commented May 26, 2020

k8s-ci-robot commented May 26, 2020

minikube-bot commented May 26, 2020

codecov-commenter commented May 26, 2020 •

edited

Loading

ashleyschuett commented May 26, 2020

medyagh left a comment

medyagh May 26, 2020 •

edited

Loading

ashleyschuett May 27, 2020

medyagh May 28, 2020

fabiand left a comment

fabiand May 27, 2020

ashleyschuett May 28, 2020

fabiand May 28, 2020

ashleyschuett May 28, 2020

ashleyschuett May 29, 2020

fabiand May 27, 2020

medyagh May 28, 2020

ashleyschuett Jun 2, 2020

fabiand left a comment

fabiand Jun 2, 2020

sharifelgamal commented Jun 3, 2020

minikube-pr-bot commented Jun 3, 2020

medyagh left a comment •

edited

Loading

Add KubeVirt addon #8275

Add KubeVirt addon #8275

Conversation

ashleyschuett commented May 26, 2020

Description

k8s-ci-robot commented May 26, 2020

k8s-ci-robot commented May 26, 2020

k8s-ci-robot commented May 26, 2020

minikube-bot commented May 26, 2020

codecov-commenter commented May 26, 2020 • edited Loading

Codecov Report

ashleyschuett commented May 26, 2020

medyagh left a comment

Choose a reason for hiding this comment

medyagh May 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabiand left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabiand left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sharifelgamal commented Jun 3, 2020

minikube-pr-bot commented Jun 3, 2020

medyagh left a comment • edited Loading

Choose a reason for hiding this comment

codecov-commenter commented May 26, 2020 •

edited

Loading

medyagh May 26, 2020 •

edited

Loading

medyagh left a comment •

edited

Loading