Support to set a specify hostName for a CloneVM #10834

matthewei · 2023-12-05T10:41:44Z

What this PR does / why we need it:

Sometimes we want to set hostname for CloneVM just as the VM which can be set hostname.

we can realize this function based on the following

kind: VirtualMachineClone
apiVersion: "clone.kubevirt.io/v1alpha1"
metadata:
  name: testclone
spec:
  # target definitions hostname
  hostname: vmclone

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #10742

Special notes for your reviewer:

Checklist

This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

Design: A design document was considered and is present (link) or not required
PR: The PR description is expressive enough and will help future contributors
Code: Write code that humans can understand and Keep it simple
Refactor: You have left the code cleaner than you found it (Boy Scout Rule)
Upgrade: Impact of this change on upgrade flows was considered and addressed if required
Testing: New code requires new unit tests. New features and bug fixes require at least on e2e test
Documentation: A user-guide update was considered and is present (link) or not required. You want a user-guide update if it's a user facing feature / API change.
Community: Announcement to kubevirt-dev was considered

Release note:

Add the possibility to specify vm target hostname during clone

kubevirt-bot · 2023-12-05T10:41:55Z

Hi @matthewei. Thanks for your PR.

I'm waiting for a kubevirt 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.

kubevirt-bot · 2023-12-05T10:41:56Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign alonakaplan for approval. For more information see the Kubernetes Code Review Process.

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

matthewei · 2023-12-05T10:42:56Z

I forget to add e2e test and unit-test. I will add it soon.

alaypatel07 · 2023-12-05T14:04:31Z

/cc for api change review

kubevirt-bot · 2023-12-05T14:04:35Z

@alaypatel07: GitHub didn't allow me to request PR reviews from the following users: change, review, for.

Note that only kubevirt members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc for api change review

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.

xpivarc · 2023-12-07T13:58:12Z

/cc @alaypatel07

alaypatel07

In terms of API, the changes look good, added couple of nits. Pending tests, this would be a good add to vmclone, thanks @matthewei

api/openapi-spec/swagger.json

pkg/virt-api/webhooks/validating-webhook/admitters/vmclone-admitter.go

matthewei

Plese help me to review this PR

pkg/virt-api/webhooks/validating-webhook/admitters/vmclone-admitter.go

api/openapi-spec/swagger.json

pkg/virt-api/webhooks/validating-webhook/admitters/validate-k8s-utils.go

pkg/virt-api/webhooks/validating-webhook/admitters/vmclone-admitter.go

pkg/virt-controller/watch/clone/clone_test.go

pkg/virt-controller/watch/clone/vm-target.go

tests/clone_test.go

matthewei · 2024-05-07T06:46:51Z

@fossedihelm Hi, please help me review this PR. Many thanks!

matthewei · 2024-05-08T05:35:03Z

/retest

kubevirt-bot · 2024-05-08T05:35:18Z

@matthewei: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

iholder101

Thanks a lot @matthewei!
Almost there! :) left some small comments.

Great job and thanks for your patience!

iholder101 · 2024-05-08T05:58:38Z

staging/src/kubevirt.io/api/clone/v1alpha1/types.go

@@ -65,7 +65,9 @@ type VirtualMachineCloneSpec struct {
 	// inspecting status "TargetName" field below.
 	// +optional
 	Target *corev1.TypedLocalObjectReference `json:"target,omitempty"`
-
+	// Specifies the hostname of the clone vmi.


Suggested change

// Specifies the hostname of the clone vmi.

// Specifies the hostname of the clone vm.

Small but important correction: clones operate on the VM level, not the VMI level

iholder101 · 2024-05-08T06:00:29Z

pkg/virt-api/webhooks/validating-webhook/admitters/vmclone-admitter.go

@@ -108,6 +110,10 @@ func (admitter *VirtualMachineCloneAdmitter) Admit(ar *admissionv1.AdmissionRevi
 		causes = append(causes, newCauses...)
 	}

+	if newCauses := validateCloneHostNameNotConformingToDNSLabelRules(vmClone); newCauses != nil {


In the future we may add more general validations to hostname

Suggested change

if newCauses := validateCloneHostNameNotConformingToDNSLabelRules(vmClone); newCauses != nil {

if newCauses := validateCloneHostName(vmClone); newCauses != nil {

iholder101 · 2024-05-09T10:31:02Z

Thanks a lot @matthewei!
/test all

iholder101 · 2024-05-09T10:50:44Z

@matthewei There are lane failures. For example, the unit tests lane says:

pkg/virt-api/webhooks/validating-webhook/admitters/vmclone-admitter_test.go:409:28: cannot use "test" (untyped string constant) as *string value in assignment
pkg/virt-api/webhooks/validating-webhook/admitters/vmclone-admitter_test.go:414:28: cannot use "test+bad" (untyped string constant) as *string value in assignment

Can you please to build and test locally? To do so:

Run make
Run make test
Optional: change Describe to FDescribe here and run make cluster-up && make cluster-sync && make functest

kubevirt/tests/clone_test.go

Line 50 in ddc9d60

var _ = Describe("[Serial]VirtualMachineClone Tests", Serial, func() {

matthewei · 2024-05-10T01:20:10Z

@matthewei There are lane failures. For example, the unit tests lane says:
pkg/virt-api/webhooks/validating-webhook/admitters/vmclone-admitter_test.go:409:28: cannot use "test" (untyped string constant) as *string value in assignment
pkg/virt-api/webhooks/validating-webhook/admitters/vmclone-admitter_test.go:414:28: cannot use "test+bad" (untyped string constant) as *string value in assignment
Can you please to build and test locally? To do so:

Run make

Run make test

Optional: change Describe to FDescribe here and run make cluster-up && make cluster-sync && make functest

kubevirt/tests/clone_test.go

Line 50 in ddc9d60

var _ = Describe("[Serial]VirtualMachineClone Tests", Serial, func() {

OK, will do it

matthewei · 2024-05-14T02:11:38Z

@matthewei There are lane failures. For example, the unit tests lane says:
pkg/virt-api/webhooks/validating-webhook/admitters/vmclone-admitter_test.go:409:28: cannot use "test" (untyped string constant) as *string value in assignment
pkg/virt-api/webhooks/validating-webhook/admitters/vmclone-admitter_test.go:414:28: cannot use "test+bad" (untyped string constant) as *string value in assignment
Can you please to build and test locally? To do so:

Run make

Run make test

Optional: change Describe to FDescribe here and run make cluster-up && make cluster-sync && make functest

kubevirt/tests/clone_test.go

Line 50 in ddc9d60

var _ = Describe("[Serial]VirtualMachineClone Tests", Serial, func() {

Hi， I got the following errors,Could you give me a hand?

iholder101 · 2024-05-15T06:55:09Z

@matthewei you probably need to deploy storage to the cluster. Try:

> export KUBEVIRT_STORAGE=rook-ceph-default
> make cluster-up && make cluster-sync

matthewei · 2024-05-28T12:03:13Z

make cluster-up && make cluster-sync

ok , Come back and ready going on it

Signed-off-by: shenwei <shenwei_yewu@cmss.chinamobile.com>

matthewei · 2024-06-05T11:32:38Z

@iholder101 If I want to exec the command test all. How can i realize it?

iholder101 · 2024-06-09T12:36:06Z

@iholder101 If I want to exec the command test all. How can i realize it?

In order to do so you need to be a community member.
Let me help you with that.

/test pull-kubevirt-build
/test pull-kubevirt-unit-test
/test pull-kubevirt-e2e-k8s-1.30-sig-compute
/test pull-kubevirt-e2e-k8s-1.30-sig-compute

I also suggest to try and building, running unit tests, and preferably running some functional tests locally before triggering CI.

kubevirt-bot · 2024-06-09T12:50:33Z

@matthewei: The following tests 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-e2e-k8s-1.29-sig-compute	`4a1d770`	link	true	`/test pull-kubevirt-e2e-k8s-1.29-sig-compute`
pull-kubevirt-unit-test-arm64	`6c960f4`	link	false	`/test pull-kubevirt-unit-test-arm64`
pull-kubevirt-code-lint	`6c960f4`	link	true	`/test pull-kubevirt-code-lint`
pull-kubevirt-goveralls	`6c960f4`	link	false	`/test pull-kubevirt-goveralls`
pull-kubevirt-unit-test	`1aad167`	link	true	`/test pull-kubevirt-unit-test`

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

kubevirt-bot · 2024-07-31T19:00:56Z

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-sigs/prow repository.

kubevirt-bot requested review from stu-gott and xpivarc December 5, 2023 10:41

kubevirt-bot added size/M needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 5, 2023

matthewei force-pushed the CloneVMSupportSetHostname branch from 776e88c to d00e7bc Compare December 5, 2023 12:29

kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Dec 5, 2023

kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 7, 2023

kubevirt-bot requested a review from alaypatel07 December 7, 2023 13:58

alaypatel07 suggested changes Dec 11, 2023

View reviewed changes

api/openapi-spec/swagger.json Outdated Show resolved Hide resolved

pkg/virt-api/webhooks/validating-webhook/admitters/vmclone-admitter.go Outdated Show resolved Hide resolved

kubevirt-bot assigned alaypatel07 Dec 11, 2023

matthewei requested a review from alaypatel07 December 25, 2023 11:38

matthewei force-pushed the CloneVMSupportSetHostname branch from d00e7bc to d296f54 Compare December 26, 2023 04:39

kubevirt-bot added dco-signoff: no Indicates the PR's author has not DCO signed all their commits. size/L and removed dco-signoff: yes Indicates the PR's author has DCO signed all their commits. size/M labels Dec 26, 2023

matthewei force-pushed the CloneVMSupportSetHostname branch 2 times, most recently from 543a3a7 to 157119d Compare December 26, 2023 04:43

kubevirt-bot added size/M and removed size/L labels Dec 26, 2023

matthewei commented Apr 26, 2024

View reviewed changes

matthewei force-pushed the CloneVMSupportSetHostname branch from d6d0604 to 323d2ad Compare May 7, 2024 06:43

iholder101 reviewed May 8, 2024

View reviewed changes

matthewei force-pushed the CloneVMSupportSetHostname branch from 323d2ad to 6c960f4 Compare May 9, 2024 01:14

matthewei requested a review from iholder101 May 9, 2024 01:15

kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 28, 2024

matthewei force-pushed the CloneVMSupportSetHostname branch from 6c960f4 to 969c17d Compare June 5, 2024 06:44

kubevirt-bot added size/M and removed size/L needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 5, 2024

matthewei force-pushed the CloneVMSupportSetHostname branch 2 times, most recently from 0a6da2b to 58afafd Compare June 5, 2024 07:21

kubevirt-bot added size/L and removed size/M labels Jun 5, 2024

Support to set a specify hostName for a CloneVM

1aad167

Signed-off-by: shenwei <shenwei_yewu@cmss.chinamobile.com>

matthewei force-pushed the CloneVMSupportSetHostname branch from 58afafd to 1aad167 Compare June 5, 2024 07:43

kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support to set a specify hostName for a CloneVM #10834

Support to set a specify hostName for a CloneVM #10834

matthewei commented Dec 5, 2023 •

edited

Loading

kubevirt-bot commented Dec 5, 2023

kubevirt-bot commented Dec 5, 2023

matthewei commented Dec 5, 2023

alaypatel07 commented Dec 5, 2023

kubevirt-bot commented Dec 5, 2023

xpivarc commented Dec 7, 2023

alaypatel07 left a comment

matthewei left a comment •

edited

Loading

matthewei commented May 7, 2024

matthewei commented May 8, 2024

kubevirt-bot commented May 8, 2024

iholder101 left a comment

iholder101 May 8, 2024

matthewei May 9, 2024

iholder101 May 8, 2024

matthewei May 9, 2024

iholder101 commented May 9, 2024

iholder101 commented May 9, 2024

matthewei commented May 10, 2024

matthewei commented May 14, 2024

iholder101 commented May 15, 2024

matthewei commented May 28, 2024

matthewei commented Jun 5, 2024

iholder101 commented Jun 9, 2024 •

edited

Loading

kubevirt-bot commented Jun 9, 2024

kubevirt-bot commented Jul 31, 2024

	// Specifies the hostname of the clone vmi.
	// Specifies the hostname of the clone vm.

	if newCauses := validateCloneHostNameNotConformingToDNSLabelRules(vmClone); newCauses != nil {
	if newCauses := validateCloneHostName(vmClone); newCauses != nil {

Support to set a specify hostName for a CloneVM #10834

Are you sure you want to change the base?

Support to set a specify hostName for a CloneVM #10834

Conversation

matthewei commented Dec 5, 2023 • edited Loading

kubevirt-bot commented Dec 5, 2023

kubevirt-bot commented Dec 5, 2023

matthewei commented Dec 5, 2023

alaypatel07 commented Dec 5, 2023

kubevirt-bot commented Dec 5, 2023

xpivarc commented Dec 7, 2023

alaypatel07 left a comment

Choose a reason for hiding this comment

matthewei left a comment • edited Loading

Choose a reason for hiding this comment

matthewei commented May 7, 2024

matthewei commented May 8, 2024

kubevirt-bot commented May 8, 2024

iholder101 left a comment

Choose a reason for hiding this comment

iholder101 May 8, 2024

Choose a reason for hiding this comment

matthewei May 9, 2024

Choose a reason for hiding this comment

iholder101 May 8, 2024

Choose a reason for hiding this comment

matthewei May 9, 2024

Choose a reason for hiding this comment

iholder101 commented May 9, 2024

iholder101 commented May 9, 2024

matthewei commented May 10, 2024

matthewei commented May 14, 2024

iholder101 commented May 15, 2024

matthewei commented May 28, 2024

matthewei commented Jun 5, 2024

iholder101 commented Jun 9, 2024 • edited Loading

kubevirt-bot commented Jun 9, 2024

kubevirt-bot commented Jul 31, 2024

matthewei commented Dec 5, 2023 •

edited

Loading

matthewei left a comment •

edited

Loading

iholder101 commented Jun 9, 2024 •

edited

Loading