Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-0.53]: Fix: Align Reenlightenment flows between converter.go and template.go #8996

Conversation

iholder101
Copy link
Contributor

What this PR does / why we need it:
Manual backport of #8798. As a dependency, I've also backported #8793.

The reasons for the manual backport are mostly minor issues.

Release note:

Fix: Align Reenlightenment flows between converter.go and template.go

@kubevirt-bot kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Dec 25, 2022
@iholder101
Copy link
Contributor Author

/cc @enp0s3
/cc @Barakmor1

can you please take a look?

@iholder101 iholder101 marked this pull request as draft December 25, 2022 13:50
@kubevirt-bot 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 25, 2022
@iholder101 iholder101 force-pushed the backport/hyperv_reenlightenment_flow_alignment branch from 6c64705 to 81c4ff9 Compare December 25, 2022 14:01
@iholder101 iholder101 marked this pull request as ready for review December 25, 2022 14:04
@kubevirt-bot 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 Dec 25, 2022
@iholder101
Copy link
Contributor Author

/test pull-kubevirt-unit-test

@kubevirt-bot
Copy link
Contributor

@iholder101: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test pull-kubevirt-apidocs-0.53
  • /test pull-kubevirt-build-0.53
  • /test pull-kubevirt-build-arm64-0.53
  • /test pull-kubevirt-check-unassigned-tests-0.53
  • /test pull-kubevirt-client-python-0.53
  • /test pull-kubevirt-e2e-k8s-1.21-operator-0.53
  • /test pull-kubevirt-e2e-k8s-1.21-sig-compute-0.53
  • /test pull-kubevirt-e2e-k8s-1.21-sig-compute-migrations-0.53
  • /test pull-kubevirt-e2e-k8s-1.21-sig-network-0.53
  • /test pull-kubevirt-e2e-k8s-1.21-sig-storage-0.53
  • /test pull-kubevirt-e2e-k8s-1.22-ipv6-sig-network-0.53
  • /test pull-kubevirt-e2e-k8s-1.22-operator-0.53
  • /test pull-kubevirt-e2e-k8s-1.22-sig-compute-0.53
  • /test pull-kubevirt-e2e-k8s-1.22-sig-compute-migrations-0.53
  • /test pull-kubevirt-e2e-k8s-1.22-sig-monitoring-0.53
  • /test pull-kubevirt-e2e-k8s-1.22-sig-network-0.53
  • /test pull-kubevirt-e2e-k8s-1.22-sig-storage-0.53
  • /test pull-kubevirt-e2e-k8s-1.23-operator-0.53
  • /test pull-kubevirt-e2e-k8s-1.23-operator-nonroot-0.53
  • /test pull-kubevirt-e2e-k8s-1.23-sig-compute-0.53
  • /test pull-kubevirt-e2e-k8s-1.23-sig-compute-cgroupsv2-0.53
  • /test pull-kubevirt-e2e-k8s-1.23-sig-compute-migrations-0.53
  • /test pull-kubevirt-e2e-k8s-1.23-sig-compute-migrations-nonroot-0.53
  • /test pull-kubevirt-e2e-k8s-1.23-sig-compute-nonroot-0.53
  • /test pull-kubevirt-e2e-k8s-1.23-sig-network-0.53
  • /test pull-kubevirt-e2e-k8s-1.23-sig-network-nonroot-0.53
  • /test pull-kubevirt-e2e-k8s-1.23-sig-storage-0.53
  • /test pull-kubevirt-e2e-k8s-1.23-sig-storage-nonroot-0.53
  • /test pull-kubevirt-e2e-kind-1.22-sriov-0.53
  • /test pull-kubevirt-e2e-kind-1.23-vgpu-0.53
  • /test pull-kubevirt-e2e-kind-1.23-vgpu-nonroot-0.53
  • /test pull-kubevirt-e2e-windows2016-0.53
  • /test pull-kubevirt-generate-0.53
  • /test pull-kubevirt-manifests-0.53
  • /test pull-kubevirt-prom-rules-verify-0.53
  • /test pull-kubevirt-unit-test-0.53

The following commands are available to trigger optional jobs:

  • /test build-kubevirt-builder-0.53
  • /test pull-kubevirt-check-tests-for-flakes-0.53
  • /test pull-kubevirt-e2e-arm64-0.53
  • /test pull-kubevirt-e2e-k8s-1.21-sig-performance-0.53
  • /test pull-kubevirt-e2e-k8s-1.22-sig-compute-realtime-0.53
  • /test pull-kubevirt-e2e-k8s-1.23-sig-storage-cgroupsv2-0.53
  • /test pull-kubevirt-e2e-k8s-1.23-single-node-0.53
  • /test pull-kubevirt-e2e-k8s-1.23-swap-enabled-0.53
  • /test pull-kubevirt-e2e-kind-1.22-sriov-nonroot-0.53
  • /test pull-kubevirt-gosec-0.53
  • /test pull-kubevirt-goveralls-0.53
  • /test pull-kubevirt-verify-go-mod-0.53
  • /test pull-kubevirt-verify-rpms-0.53

Use /test all to run the following jobs that were automatically triggered:

  • pull-kubevirt-apidocs-0.53
  • pull-kubevirt-build-0.53
  • pull-kubevirt-build-arm64-0.53
  • pull-kubevirt-check-tests-for-flakes-0.53
  • pull-kubevirt-check-unassigned-tests-0.53
  • pull-kubevirt-client-python-0.53
  • pull-kubevirt-e2e-k8s-1.21-operator-0.53
  • pull-kubevirt-e2e-k8s-1.21-sig-compute-0.53
  • pull-kubevirt-e2e-k8s-1.21-sig-compute-migrations-0.53
  • pull-kubevirt-e2e-k8s-1.21-sig-network-0.53
  • pull-kubevirt-e2e-k8s-1.21-sig-performance-0.53
  • pull-kubevirt-e2e-k8s-1.21-sig-storage-0.53
  • pull-kubevirt-e2e-k8s-1.22-ipv6-sig-network-0.53
  • pull-kubevirt-e2e-k8s-1.22-operator-0.53
  • pull-kubevirt-e2e-k8s-1.22-sig-compute-0.53
  • pull-kubevirt-e2e-k8s-1.22-sig-compute-migrations-0.53
  • pull-kubevirt-e2e-k8s-1.22-sig-network-0.53
  • pull-kubevirt-e2e-k8s-1.22-sig-storage-0.53
  • pull-kubevirt-e2e-k8s-1.23-operator-0.53
  • pull-kubevirt-e2e-k8s-1.23-operator-nonroot-0.53
  • pull-kubevirt-e2e-k8s-1.23-sig-compute-0.53
  • pull-kubevirt-e2e-k8s-1.23-sig-compute-cgroupsv2-0.53
  • pull-kubevirt-e2e-k8s-1.23-sig-compute-migrations-0.53
  • pull-kubevirt-e2e-k8s-1.23-sig-compute-migrations-nonroot-0.53
  • pull-kubevirt-e2e-k8s-1.23-sig-compute-nonroot-0.53
  • pull-kubevirt-e2e-k8s-1.23-sig-network-0.53
  • pull-kubevirt-e2e-k8s-1.23-sig-network-nonroot-0.53
  • pull-kubevirt-e2e-k8s-1.23-sig-storage-0.53
  • pull-kubevirt-e2e-k8s-1.23-sig-storage-cgroupsv2-0.53
  • pull-kubevirt-e2e-k8s-1.23-sig-storage-nonroot-0.53
  • pull-kubevirt-e2e-kind-1.22-sriov-0.53
  • pull-kubevirt-e2e-kind-1.23-vgpu-0.53
  • pull-kubevirt-e2e-kind-1.23-vgpu-nonroot-0.53
  • pull-kubevirt-e2e-windows2016-0.53
  • pull-kubevirt-generate-0.53
  • pull-kubevirt-goveralls-0.53
  • pull-kubevirt-manifests-0.53
  • pull-kubevirt-prom-rules-verify-0.53
  • pull-kubevirt-unit-test-0.53

In response to this:

/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/test-infra repository.

@iholder101
Copy link
Contributor Author

/test pull-kubevirt-unit-test-0.53

@enp0s3
Copy link
Contributor

enp0s3 commented Dec 26, 2022

/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enp0s3

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:

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

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 26, 2022
@Barakmor1
Copy link
Member

Please divide the first commit to the original commit and another commit with your changes so it will be easier to review.

xpivarc and others added 8 commits December 26, 2022 10:53
These test doesn't require Windows OS to be
tested. CI has only one copy of Windows license
and therefore limited bw on Windows lane.

Signed-off-by: L. Pivarc <lpivarc@redhat.com>
Signed-off-by: Itamar Holder <iholder@redhat.com>
The previous commit copied the some functests
from windows_tests to hyperv_tests but did not
remove some crucial helper functions over there
as well.

Signed-off-by: Itamar Holder <iholder@redhat.com>
Prove That currently whenever EVMCS HyperV feature
Is not nil in the vm spec than all dependencies
features are enabled.
Even if we set to vmi.Spec.Domain.Features.Hyperv.EVMCS.Enabled
to be false.

Signed-off-by: bmordeha <bmodeha@redhat.com>
Currently whenever EVMCS HyperV feature
Is not nil in the vm spec than all dependencies
features are enabled.
Even if we set to vmi.Spec.Domain.Features.Hyperv.EVMCS.Enabled
to be false.

Signed-off-by: bmordeha <bmodeha@redhat.com>
Signed-off-by: bmordeha <bmodeha@redhat.com>
make sure that if we set vmi.Spec.Domain.Features.Hyperv.EVMCS.Enabled
to be false than all dependencies features are not enabled.

Signed-off-by: bmordeha <bmodeha@redhat.com>
While the converter.go logic checked whether we have topology hints
defined and that tsc frequency is required (which is the right thing
to do), template.go logic did not check the later.

In addition, improve IsManualTSCFrequencyRequired() to also check
whether topology hints are defined.

Signed-off-by: Itamar Holder <iholder@redhat.com>
Signed-off-by: Itamar Holder <iholder@redhat.com>
Signed-off-by: Itamar Holder <iholder@redhat.com>
…e updated

Signed-off-by: Itamar Holder <iholder@redhat.com>
That's how we can distinguish between us waiting for a TSC frequency
and knowing there is no TSC frequency information, hence we can
stop waiting for it.

This is necessary, since the vmi controller executes "sync()" and
"updateStatus()" in every control loop. If TSC frequency is required,
sync() would wait until topology hints would be set up by
updateStatus(). Today, updateStatus() doesn't update the topology
hints at all if no TSC frequency information exists, therefore
sync will continue waiting forever and the VM wouldn't be able
to start.

Signed-off-by: Itamar Holder <iholder@redhat.com>
…gy hints are updated

Signed-off-by: Itamar Holder <iholder@redhat.com>
…ined

Signed-off-by: Itamar Holder <iholder@redhat.com>
@iholder101 iholder101 force-pushed the backport/hyperv_reenlightenment_flow_alignment branch from 81c4ff9 to 7555f2b Compare December 26, 2022 09:01
@iholder101
Copy link
Contributor Author

Please divide the first commit to the original commit and another commit with your changes so it will be easier to review.

done

Comment on lines -1880 to +1881
// The return value is overhead memory quantity
// # The return value is overhead memory quantity
//
// Note: This is the best estimation we were able to come up with
// and is still not 100% accurate
//
// and is still not 100% accurate
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: i think that these are leftovers

@Barakmor1
Copy link
Member

/lgtm
Thank you!

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 26, 2022
@iholder101
Copy link
Contributor Author

/retest

@kubevirt-bot
Copy link
Contributor

kubevirt-bot commented Dec 26, 2022

@iholder101: 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-e2e-k8s-1.21-sig-performance 7555f2b link false /test pull-kubevirt-e2e-k8s-1.21-sig-performance-0.53

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.

@iholder101
Copy link
Contributor Author

/test pull-kubevirt-e2e-k8s-1.21-sig-performance-0.53

@kubevirt-bot kubevirt-bot merged commit 94d5c71 into kubevirt:release-0.53 Dec 26, 2022
41 of 42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants