virtctl: Add create vm command #8878

0xFelix · 2022-11-29T15:20:12Z

What this PR does / why we need it:

This adds the 'create' / 'create vm' command to virtctl, which allows to
create simple VM manifests via the CLI. Certain fields of the VM
definition can be set with according flags.

Fixes #7760

Release note:

Add 'create vm' command to virtctl

kubevirt-bot · 2022-11-29T15:20:14Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

0xFelix · 2022-11-30T13:33:16Z

/cc @lyarwood

davidvossel

I think cloud-init needs to be taken into account here as well since it's often used as the entry point for injecting the credentials necessary to access a VM.

davidvossel · 2022-11-30T15:20:24Z

pkg/virtctl/create/vm.go

+	cmd.Flags().StringVar(&c.pvc, PvcFlag, c.pvc, "Specify the PVC of the VirtualMachine.")
+	cmd.Flags().StringVar(&c.dataVolume, DatavolumeFlag, c.dataVolume, "Specify the DataVolume of the VirtualMachine.")
+	cmd.MarkFlagsMutuallyExclusive(PvcFlag, DatavolumeFlag)


Users typically don't specify the PVC or DataVolume they want, instead they specify the source image they want to use for the VM which then gets imported into a pvc using the DataVolume flow.

Rather than a --pvc or --datavolume flag, i think a --image-src flag would make more sense. If the image source starts with an http://|https:// then it's a http import, if it starts with a docker:// then its a container disk import... We could come up with other options as well like perhaps a pvc:// prefix for pvc cloning

I added a --image-source flag but it works a bit differently than you described. I think we should discuss this a bit further.

Just as a counter point to David's initial suggestion, we already have a command to orchestrate the upload of an image into the cluster, IMHO this command just needs to handle the creation of the VirtualMachine itself assuming the image is already present as a DataSource and/or PVC.

pkg/virtctl/create/vm.go

fabiand · 2022-11-30T20:14:34Z

Awesome to see this work!

@0xFelix in order to understand the user impact better, do you mind adding user documentation first to show how you envision the workflow?
I mainly want to stress how we provide the dvs or rather PVCs.

My problem is that the use of datavolumetemplate and pvcs lead to totally different code paths. maybe this is not an issue... seeing the user documentation would at least show how tihs would look for a user.

Can we also try to keep in mind that we might want to add great support for volume populators down the road. IOW let's try to think about how populator parameter would look if we'd add them to kubevirt., i.e. virtctl create vm mine --volume anewvol --populated-with rhel9 m1.large or along the lines

lyarwood · 2022-12-01T10:28:42Z

I mainly want to stress how we provide the dvs or rather PVCs.

My problem is that the use of datavolumetemplate and pvcs lead to totally different code paths. maybe this is not an issue... seeing the
user documentation would at least show how tihs would look for a user.

I really think we should focus on the simple existing storage resource cases first in this PR and maybe follow up with something more complex targeting the dynamic DataVolumeTemplates flow later.

Can we also try to keep in mind that we might want to add great support for volume populators down the road. IOW let's try to think
about how populator parameter would look if we'd add them to kubevirt., i.e. virtctl create vm mine --volume anewvol --populated-with rhel9 m1.large or along the lines

+1 to keeping this as abstract as possible going forward, otherwise we could easily end up in the same situation as OpenStack having to expose every API tunable for storage, for example --block-device-mapping:

https://docs.openstack.org/python-openstackclient/latest/cli/command-objects/server.html#cmdoption-openstack-server-create-block-device-mapping

--block-device-mapping <dev-name=mapping>
Deprecated Create a block device on the server. Block device mapping in the format <dev-name>=<id>:<type>:<size(GB)>:<delete-on-terminate> <dev-name>: block device name, like: vdb, xvdc (required) <id>: Name or ID of the volume, volume snapshot or image (required) <type>: volume, snapshot or image; default: volume (optional) <size(GB)>: volume size if create from image or snapshot (optional) <delete-on-terminate>: true or false; default: false (optional) Replaced by –block-device

0xFelix · 2022-12-02T15:34:39Z

I pushed a new revision of this PR which adds the --image-source and --direct-image-source flag. WDYT? It's a WIP and there are still tests and docs missing.

0xFelix · 2022-12-02T15:37:56Z

Possible uses of --image-src in this revision:

Clone the source before using it:

--image-src=pvc/my-pvc
--image-src=pvc/my-pvc.my-ns

--image-src=dv/my-dv
--image-src=dv/my-dv.my-ns

--image-src=ds/my-ds
--image-src=ds/my-ds.my-ns

Use the source directly:

--direct-image-src=pvc/my-pvc
--direct-image-src=dv/my-dv

lyarwood

Thanks Felix, I'm not a fan of the new switches, I'd rather we expose more specific switches for each Volume chain we currently support.

pkg/virtctl/create/vm.go

lyarwood · 2022-12-02T16:47:07Z

pkg/virtctl/create/vm.go

+		vm.Spec.DataVolumeTemplates[0].Spec.SourceRef.Namespace = &namespace
+	}
+
+	vm.Spec.Template.Spec.Volumes = []v1.Volume{


I assumed we'd be chaining these switches togther to allow multiple volumes to be defined?

How to determine the boot order? The order in which the switches were defined?

Yes, the ordering is then retained if we generate a list of disks in the VMI mutation webhook.

There are four switches, their order is:

--volume-dvt-ds --volume-dvt-pvc --volume-dv --volume-pvc

Each switch can be specified multiple times, the order in which it was specified will be kept.

Example 1:

virtctl create vm --volume-dvt-ds my-ds1,my-ds2 --volume-dv my-dv1 --volume-dv my-dv2 --> vm.Spec.Template.Spec.Volumes = my-ds1,my-ds2,my-dv1,my-dv2

Example 2:

virtctl create vm --volume-dv my-dv1 --volume-dv my-dv2 --volume-dvt-pvc my-ns/my-pvc --> vm.Spec.Template.Spec.Volumes = my-ns/my-pvc,my-dv1,my-dv2

davidvossel

/approve

excellent work!

kubevirt-bot · 2023-01-18T16:51:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: davidvossel

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~~ [davidvossel]

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

0xFelix · 2023-01-18T18:44:11Z

@dhiller Could you override the failing pull-kubevirt-fossa?

kubevirt-commenter-bot · 2023-01-18T20:53:27Z

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

kubevirt-commenter-bot · 2023-01-18T23:53:28Z

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

kubevirt-commenter-bot · 2023-01-19T02:55:28Z

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

kubevirt-commenter-bot · 2023-01-19T05:55:28Z

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

dhiller · 2023-01-19T08:54:32Z

/override pull-kubevirt-fossa

kubevirt-bot · 2023-01-19T08:54:35Z

@dhiller: Overrode contexts on behalf of dhiller: pull-kubevirt-fossa

In response to this:

/override pull-kubevirt-fossa

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.

0xFelix · 2023-01-19T09:20:12Z

/hold Need to check something about containerdisk image names.

0xFelix · 2023-01-19T09:48:37Z

/hold cancel

Removed the docker:// prefix from containerdisks.

opokornyy · 2023-01-19T10:04:37Z

/lgtm

This adds the 'create' / 'create vm' command to virtctl which, allows to create simple VM manifests via the CLI. Certain fields of the VM definition can be set with according flags. Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

opokornyy · 2023-01-19T10:20:29Z

/lgtm

codingben · 2023-01-19T10:43:07Z

/lgtm

0xFelix · 2023-01-19T13:05:41Z

/retest

kubevirt-bot · 2023-01-19T13:09:36Z

@0xFelix: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-kubevirt-fossa	`7e268b6`	link	false	`/test pull-kubevirt-fossa`

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. I understand the commands that are listed here.

kubevirt-bot requested review from acardace and alicefr November 29, 2022 15:21

0xFelix force-pushed the virtctl-create-vm branch from 97a5840 to 5f4dd29 Compare November 30, 2022 13:30

kubevirt-bot added size/XL and removed size/L labels Nov 30, 2022

0xFelix changed the title ~~[WIP] virtctl: Add create vm command~~ virtctl: Add create vm command Nov 30, 2022

0xFelix marked this pull request as ready for review November 30, 2022 13:32

kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 30, 2022

kubevirt-bot requested review from kbidarkar and vladikr November 30, 2022 13:33

kubevirt-bot requested a review from lyarwood November 30, 2022 13:33

0xFelix force-pushed the virtctl-create-vm branch 2 times, most recently from e8c90ad to 0927681 Compare November 30, 2022 13:52

davidvossel reviewed Nov 30, 2022

View reviewed changes

0xFelix force-pushed the virtctl-create-vm branch from 0927681 to d87f31b Compare December 2, 2022 15:32

0xFelix changed the title ~~virtctl: Add create vm command~~ [WIP] virtctl: Add create vm command Dec 2, 2022

kubevirt-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 2, 2022

lyarwood suggested changes Dec 2, 2022

View reviewed changes

kubevirt-bot assigned lyarwood Dec 2, 2022

0xFelix force-pushed the virtctl-create-vm branch from d87f31b to 28b998b Compare December 5, 2022 14:32

davidvossel reviewed Jan 18, 2023

View reviewed changes

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 18, 2023

kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 19, 2023

0xFelix force-pushed the virtctl-create-vm branch from 98d6eaa to 5624360 Compare January 19, 2023 09:48

kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 19, 2023

kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 19, 2023

kubevirt-bot assigned opokornyy Jan 19, 2023

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 19, 2023

virtctl: Add create vm command

7e268b6

This adds the 'create' / 'create vm' command to virtctl which, allows to create simple VM manifests via the CLI. Certain fields of the VM definition can be set with according flags. Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

0xFelix force-pushed the virtctl-create-vm branch from 5624360 to 7e268b6 Compare January 19, 2023 10:18

kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 19, 2023

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 19, 2023

kubevirt-bot assigned codingben Jan 19, 2023

kubevirt-bot merged commit 92bc324 into kubevirt:main Jan 19, 2023

kubevirt-bot mentioned this pull request Mar 1, 2023

Macvtap binding mode for pod network #7648

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

virtctl: Add create vm command #8878

virtctl: Add create vm command #8878

0xFelix commented Nov 29, 2022 •

edited

Loading

kubevirt-bot commented Nov 29, 2022

0xFelix commented Nov 30, 2022

davidvossel left a comment

davidvossel Nov 30, 2022

0xFelix Dec 2, 2022

lyarwood Dec 2, 2022

fabiand commented Nov 30, 2022

lyarwood commented Dec 1, 2022

0xFelix commented Dec 2, 2022

0xFelix commented Dec 2, 2022

lyarwood left a comment

lyarwood Dec 2, 2022

0xFelix Dec 5, 2022

lyarwood Dec 5, 2022

0xFelix Dec 5, 2022 •

edited

Loading

davidvossel left a comment

kubevirt-bot commented Jan 18, 2023

0xFelix commented Jan 18, 2023 •

edited

Loading

kubevirt-commenter-bot commented Jan 18, 2023

kubevirt-commenter-bot commented Jan 18, 2023

kubevirt-commenter-bot commented Jan 19, 2023

kubevirt-commenter-bot commented Jan 19, 2023

dhiller commented Jan 19, 2023

kubevirt-bot commented Jan 19, 2023

0xFelix commented Jan 19, 2023

0xFelix commented Jan 19, 2023

opokornyy commented Jan 19, 2023

opokornyy commented Jan 19, 2023

codingben commented Jan 19, 2023

0xFelix commented Jan 19, 2023

kubevirt-bot commented Jan 19, 2023 •

edited

Loading

virtctl: Add create vm command #8878

virtctl: Add create vm command #8878

Conversation

0xFelix commented Nov 29, 2022 • edited Loading

kubevirt-bot commented Nov 29, 2022

0xFelix commented Nov 30, 2022

davidvossel left a comment

Choose a reason for hiding this comment

davidvossel Nov 30, 2022

Choose a reason for hiding this comment

0xFelix Dec 2, 2022

Choose a reason for hiding this comment

lyarwood Dec 2, 2022

Choose a reason for hiding this comment

fabiand commented Nov 30, 2022

lyarwood commented Dec 1, 2022

0xFelix commented Dec 2, 2022

0xFelix commented Dec 2, 2022

lyarwood left a comment

Choose a reason for hiding this comment

lyarwood Dec 2, 2022

Choose a reason for hiding this comment

0xFelix Dec 5, 2022

Choose a reason for hiding this comment

lyarwood Dec 5, 2022

Choose a reason for hiding this comment

0xFelix Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

davidvossel left a comment

Choose a reason for hiding this comment

kubevirt-bot commented Jan 18, 2023

0xFelix commented Jan 18, 2023 • edited Loading

kubevirt-commenter-bot commented Jan 18, 2023

kubevirt-commenter-bot commented Jan 18, 2023

kubevirt-commenter-bot commented Jan 19, 2023

kubevirt-commenter-bot commented Jan 19, 2023

dhiller commented Jan 19, 2023

kubevirt-bot commented Jan 19, 2023

0xFelix commented Jan 19, 2023

0xFelix commented Jan 19, 2023

opokornyy commented Jan 19, 2023

opokornyy commented Jan 19, 2023

codingben commented Jan 19, 2023

0xFelix commented Jan 19, 2023

kubevirt-bot commented Jan 19, 2023 • edited Loading

0xFelix commented Nov 29, 2022 •

edited

Loading

0xFelix Dec 5, 2022 •

edited

Loading

0xFelix commented Jan 18, 2023 •

edited

Loading

kubevirt-bot commented Jan 19, 2023 •

edited

Loading