Generated client: embed generated methods into VMIM interface #11639

fossedihelm · 2024-04-03T07:59:00Z

What this PR does

Before this PR:
We have our custom virtualMachineInstanceMigrationInterface interface with the common Get, Delete, etc...
From #11297 the common methods implementation works as a wrapper with different signatures for the generated ones, redirecting the requests to them.

After this PR:
Convert using the generated client signatures instead of the custom one. The generated virtualMachineInstanceMigrationInterface is embedded into our custom one, also providing new DeleteCollection and Watch methods.
The changes inherited are:

Patch and PatchStatus now use metav1.PatchOptions{} instead of *metav1.PatchOptions{}.
List now uses metav1.ListOptions{} instead of *metav1.ListOptions{}.
Get now uses metav1.GetOptions{} instead of *metav1.GetOptions{}.
Delete now uses metav1.DeleteOptions{} instead of *metav1.DeleteOptions{}.
Create now receives metav1.CreateOptions{} too.
Update and UpdateStatus now receive metav1.UpdateOptions{} too.
Added DeleteCollection, Watch methods.
All the methods above take a Context as first paramether.

Fixes #

Why we need it and why it was done in this way

The following tradeoffs were made:

The following alternatives were considered:

Links to places where the discussion took place:

Special notes for your reviewer

For an easier review process the commits was splitted in the following manner:

Align our custom common methods' signature to the generated one in the following order:
- Create - introduced CreateOptions and introduced a Context
- Get - used GetOptions instead of the pointer and introduced a Context
- List - used ListOptions instead of the pointer and introduced a Context
- Update - introduced UpdateOptions and introduced a Context
- Delete - used DeleteOptions instead of the pointer and introduced a Context
- Patch and PatchStatus - used PatchOptions instead of the pointer and introduced a Context
- UpdateStatus - introduced UpdateOptions and introduced a Context
Embed the generated VirtualMachineInstanceMigrationInterface into the custom one.

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

NONE

kubevirt-bot · 2024-04-03T07:59:02Z

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

fossedihelm · 2024-04-03T08:00:07Z

/hold
let's wait for #11523 to reduce conflicts.
/test all

fossedihelm · 2024-04-09T09:10:51Z

/unhold
#11523 was merged

fossedihelm · 2024-04-09T09:11:22Z

/cc @xpivarc @orelmisan @Barakmor1

xpivarc

/approve

pkg/util/status/status_test.go

kubevirt-bot · 2024-04-09T11:01:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: xpivarc

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

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

orelmisan

Thank you for the PR @fossedihelm.
Please see the inline comments

staging/src/kubevirt.io/client-go/kubecli/generated_mock_kubevirt.go

staging/src/kubevirt.io/client-go/kubecli/migration_test.go

staging/src/kubevirt.io/client-go/kubecli/migration.go

tests/libmigration/migration.go

orelmisan

Thank you for the changes

kubevirt-commenter-bot · 2024-04-09T16:58:16Z

Required labels detected, running phase 2 presubmits:
/test pull-kubevirt-e2e-windows2016
/test pull-kubevirt-e2e-kind-1.27-vgpu
/test pull-kubevirt-e2e-kind-sriov
/test pull-kubevirt-e2e-k8s-1.29-ipv6-sig-network
/test pull-kubevirt-e2e-k8s-1.27-sig-network
/test pull-kubevirt-e2e-k8s-1.27-sig-storage
/test pull-kubevirt-e2e-k8s-1.27-sig-compute
/test pull-kubevirt-e2e-k8s-1.27-sig-operator
/test pull-kubevirt-e2e-k8s-1.28-sig-network
/test pull-kubevirt-e2e-k8s-1.28-sig-storage
/test pull-kubevirt-e2e-k8s-1.28-sig-compute
/test pull-kubevirt-e2e-k8s-1.28-sig-operator

kubevirt-commenter-bot · 2024-04-10T02:58:50Z

/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.

vmim.Get method does not allow to pass the context and uses the pointer to GetOptions. In order to align the custom method signature with the generated one switch using the GetOptions instead of the pointer one and introduce the context. This effort, once all the common methods' signatures are aligned with the generated ones, will converge in the deletion of the custom methods' implementations, embedding the generated interface. Signed-off-by: fossedihelm <ffossemo@redhat.com>

vmim.List method does not allow to pass the context and uses the pointer to ListOptions. In order to align the custom method signature with the generated one switch using the ListOptions instead of the pointer one and introduce the context. This effort, once all the common methods' signatures are aligned with the generated ones, will converge in the deletion of the custom methods' implementations, embedding the generated interface. Signed-off-by: fossedihelm <ffossemo@redhat.com>

vmim.Create method does not allow to pass the context and uses the pointer to CreateOptions. In order to align the custom method signature with the generated one switch using the CreateOptions instead of the pointer one and introduce the context. This effort, once all the common methods' signatures are aligned with the generated ones, will converge in the deletion of the custom methods' implementations, embedding the generated interface. Signed-off-by: fossedihelm <ffossemo@redhat.com>

fossedihelm · 2024-04-10T05:55:43Z

@orelmisan Rebased because there were some conflicts in pkg/virt-controller/watch/drain/evacuation/evacuation_test.go PTAL

orelmisan

Thank you @fossedihelm

staging/src/kubevirt.io/client-go/kubecli/migration.go

staging/src/kubevirt.io/client-go/kubecli/migration_test.go

kubevirt-commenter-bot · 2024-04-10T08:16:01Z

Required labels detected, running phase 2 presubmits:
/test pull-kubevirt-e2e-windows2016
/test pull-kubevirt-e2e-kind-1.27-vgpu
/test pull-kubevirt-e2e-kind-sriov
/test pull-kubevirt-e2e-k8s-1.29-ipv6-sig-network
/test pull-kubevirt-e2e-k8s-1.27-sig-network
/test pull-kubevirt-e2e-k8s-1.27-sig-storage
/test pull-kubevirt-e2e-k8s-1.27-sig-compute
/test pull-kubevirt-e2e-k8s-1.27-sig-operator
/test pull-kubevirt-e2e-k8s-1.28-sig-network
/test pull-kubevirt-e2e-k8s-1.28-sig-storage
/test pull-kubevirt-e2e-k8s-1.28-sig-compute
/test pull-kubevirt-e2e-k8s-1.28-sig-operator

vmim.Update method does not allow to pass the context and UpdateOptions. In order to align the custom method signature with the generated one introduce the context and the UpdateOptions. This effort, once all the common methods' signatures are aligned with the generated ones, will converge in the deletion of the custom methods' implementations, embedding the generated interface. Signed-off-by: fossedihelm <ffossemo@redhat.com>

vmim.Delete method does not allow to pass the context and uses the pointer to DeleteOptions. In order to align the custom method signature with the generated one switch using the DeleteOptions instead of the pointer one and introduce the context. This effort, once all the common methods' signatures are aligned with the generated ones, will converge in the deletion of the custom methods' implementations, embedding the generated interface. Signed-off-by: fossedihelm <ffossemo@redhat.com>

vmim.Patch and vmim.PatchStatus methods do not allow to pass the context and PatchOptions. In order to align the custom methods signature with the generated ones introduce the context and the PatchOptions. This effort, once all the common methods' signatures are aligned with the generated ones, will converge in the deletion of the custom methods' implementations, embedding the generated interface. Signed-off-by: fossedihelm <ffossemo@redhat.com>

vmim.UpdateStatus method does not allow to pass the context and UpdateOptions. In order to align the custom method signature with the generated one introduce the context and the UpdateOptions. This effort, once all the common methods' signatures are aligned with the generated ones, will converge in the deletion of the custom methods' implementations, embedding the generated interface. Also, removed duplicate metav1 import with different names. Signed-off-by: fossedihelm <ffossemo@redhat.com>

The embed will provide us new methods (`DeleteCollection` and `Watch`). Signed-off-by: fossedihelm <ffossemo@redhat.com>

kubevirt-commenter-bot · 2024-04-10T08:25:10Z

Required labels detected, running phase 2 presubmits:
/test pull-kubevirt-e2e-windows2016
/test pull-kubevirt-e2e-kind-1.27-vgpu
/test pull-kubevirt-e2e-kind-sriov
/test pull-kubevirt-e2e-k8s-1.29-ipv6-sig-network
/test pull-kubevirt-e2e-k8s-1.27-sig-network
/test pull-kubevirt-e2e-k8s-1.27-sig-storage
/test pull-kubevirt-e2e-k8s-1.27-sig-compute
/test pull-kubevirt-e2e-k8s-1.27-sig-operator
/test pull-kubevirt-e2e-k8s-1.28-sig-network
/test pull-kubevirt-e2e-k8s-1.28-sig-storage
/test pull-kubevirt-e2e-k8s-1.28-sig-compute
/test pull-kubevirt-e2e-k8s-1.28-sig-operator

fossedihelm · 2024-04-10T15:24:13Z

/retest-required

kubevirt-commenter-bot · 2024-04-10T23:00:47Z

/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-bot · 2024-04-11T04:31:03Z

@fossedihelm: 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-check-tests-for-flakes	`108d5de`	link	false	`/test pull-kubevirt-check-tests-for-flakes`
pull-kubevirt-conformance-arm64	`108d5de`	link	false	`/test pull-kubevirt-conformance-arm64`

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.

fossedihelm · 2024-04-11T06:09:44Z

/retest-required

kubevirt-commenter-bot · 2024-04-11T12:02:48Z

/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-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Apr 3, 2024

kubevirt-bot requested review from kbidarkar, mhenriks and oshoval April 3, 2024 07:59

kubevirt-bot added size/L area/virtctl labels Apr 3, 2024

kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 3, 2024

fossedihelm force-pushed the embed-generated-vmim branch 4 times, most recently from cde592b to af304c0 Compare April 9, 2024 09:09

kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 9, 2024

fossedihelm marked this pull request as ready for review April 9, 2024 09:10

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 Apr 9, 2024

kubevirt-bot requested a review from AlonaKaplan April 9, 2024 09:11

kubevirt-bot requested review from Barakmor1, orelmisan and xpivarc April 9, 2024 09:11

xpivarc reviewed Apr 9, 2024

View reviewed changes

pkg/util/status/status_test.go Outdated Show resolved Hide resolved

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 9, 2024

fossedihelm force-pushed the embed-generated-vmim branch from af304c0 to fb97532 Compare April 9, 2024 11:26

orelmisan reviewed Apr 9, 2024

View reviewed changes

fossedihelm force-pushed the embed-generated-vmim branch from fb97532 to 0bb19fc Compare April 9, 2024 16:31

orelmisan approved these changes Apr 9, 2024

View reviewed changes

kubevirt-bot assigned orelmisan Apr 9, 2024

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 9, 2024

fossedihelm added 3 commits April 10, 2024 07:45

fossedihelm force-pushed the embed-generated-vmim branch from 0bb19fc to 1fde966 Compare April 10, 2024 05:55

kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Apr 10, 2024

orelmisan approved these changes Apr 10, 2024

View reviewed changes

staging/src/kubevirt.io/client-go/kubecli/migration.go Show resolved Hide resolved

staging/src/kubevirt.io/client-go/kubecli/migration_test.go Outdated Show resolved Hide resolved

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 10, 2024

fossedihelm added 5 commits April 10, 2024 10:20

feat: embed generated client for VMIM interface

108d5de

The embed will provide us new methods (`DeleteCollection` and `Watch`). Signed-off-by: fossedihelm <ffossemo@redhat.com>

fossedihelm force-pushed the embed-generated-vmim branch from 1fde966 to 108d5de Compare April 10, 2024 08:21

kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Apr 10, 2024

orelmisan approved these changes Apr 10, 2024

View reviewed changes

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 10, 2024

kubevirt-bot merged commit fa25a88 into kubevirt:main Apr 11, 2024
36 of 38 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generated client: embed generated methods into VMIM interface #11639

Generated client: embed generated methods into VMIM interface #11639

fossedihelm commented Apr 3, 2024 •

edited

kubevirt-bot commented Apr 3, 2024

fossedihelm commented Apr 3, 2024

fossedihelm commented Apr 9, 2024

fossedihelm commented Apr 9, 2024

xpivarc left a comment

kubevirt-bot commented Apr 9, 2024

orelmisan left a comment

orelmisan left a comment

kubevirt-commenter-bot commented Apr 9, 2024

kubevirt-commenter-bot commented Apr 10, 2024

fossedihelm commented Apr 10, 2024

orelmisan left a comment

kubevirt-commenter-bot commented Apr 10, 2024

kubevirt-commenter-bot commented Apr 10, 2024

fossedihelm commented Apr 10, 2024

kubevirt-commenter-bot commented Apr 10, 2024

kubevirt-bot commented Apr 11, 2024 •

edited

fossedihelm commented Apr 11, 2024

kubevirt-commenter-bot commented Apr 11, 2024

Generated client: embed generated methods into VMIM interface #11639

Generated client: embed generated methods into VMIM interface #11639

Conversation

fossedihelm commented Apr 3, 2024 • edited

What this PR does

Why we need it and why it was done in this way

Special notes for your reviewer

Checklist

Release note

kubevirt-bot commented Apr 3, 2024

fossedihelm commented Apr 3, 2024

fossedihelm commented Apr 9, 2024

fossedihelm commented Apr 9, 2024

xpivarc left a comment

Choose a reason for hiding this comment

kubevirt-bot commented Apr 9, 2024

orelmisan left a comment

Choose a reason for hiding this comment

orelmisan left a comment

Choose a reason for hiding this comment

kubevirt-commenter-bot commented Apr 9, 2024

kubevirt-commenter-bot commented Apr 10, 2024

fossedihelm commented Apr 10, 2024

orelmisan left a comment

Choose a reason for hiding this comment

kubevirt-commenter-bot commented Apr 10, 2024

kubevirt-commenter-bot commented Apr 10, 2024

fossedihelm commented Apr 10, 2024

kubevirt-commenter-bot commented Apr 10, 2024

kubevirt-bot commented Apr 11, 2024 • edited

fossedihelm commented Apr 11, 2024

kubevirt-commenter-bot commented Apr 11, 2024

fossedihelm commented Apr 3, 2024 •

edited

kubevirt-bot commented Apr 11, 2024 •

edited