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.58]: Fix: Align Reenlightenment flows between converter.go and template.go #8798

Conversation

iholder101
Copy link
Contributor

What this PR does / why we need it:
Manual backport for #8740.
The reason a manual backport is required is because the functional tests are added to hyperv_test.go which didn't exist in release-58 branch because the PR that created this file wasn't backported.

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 Nov 16, 2022
@iholder101
Copy link
Contributor Author

/cc @vladikr @acardace

@iholder101 iholder101 force-pushed the manual_backport/fix/reenlightenment-align-flows branch 2 times, most recently from ac91562 to 112ba5c Compare November 16, 2022 12:05
@iholder101 iholder101 changed the title [Manual backport]: Fix: Align Reenlightenment flows between converter.go and template.go [release-0.58]: Fix: Align Reenlightenment flows between converter.go and template.go Nov 16, 2022
@iholder101
Copy link
Contributor Author

/hold
waiting for #8793 to get in

@kubevirt-bot kubevirt-bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 16, 2022
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>
Signed-off-by: Itamar Holder <iholder@redhat.com>
@iholder101 iholder101 force-pushed the manual_backport/fix/reenlightenment-align-flows branch from 112ba5c to 03d63fc Compare November 17, 2022 09:31
@kubevirt-bot kubevirt-bot added size/L and removed size/XL needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 17, 2022
@iholder101
Copy link
Contributor Author

/hold cancel

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 17, 2022
Copy link
Contributor

@enp0s3 enp0s3 left a comment

Choose a reason for hiding this comment

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

/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 Nov 17, 2022
@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 17, 2022
@enp0s3
Copy link
Contributor

enp0s3 commented Nov 17, 2022

/retest-required

@kubevirt-bot
Copy link
Contributor

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

  • /test pull-kubevirt-apidocs-0.58
  • /test pull-kubevirt-build-0.58
  • /test pull-kubevirt-build-arm64-0.58
  • /test pull-kubevirt-check-unassigned-tests-0.58
  • /test pull-kubevirt-client-python-0.58
  • /test pull-kubevirt-e2e-k8s-1.22-operator-0.58
  • /test pull-kubevirt-e2e-k8s-1.22-sig-compute-0.58
  • /test pull-kubevirt-e2e-k8s-1.22-sig-monitoring-0.58
  • /test pull-kubevirt-e2e-k8s-1.22-sig-network-0.58
  • /test pull-kubevirt-e2e-k8s-1.22-sig-storage-0.58
  • /test pull-kubevirt-e2e-k8s-1.23-operator-0.58
  • /test pull-kubevirt-e2e-k8s-1.23-operator-nonroot-0.58
  • /test pull-kubevirt-e2e-k8s-1.23-sig-compute-0.58
  • /test pull-kubevirt-e2e-k8s-1.23-sig-compute-cgroupsv2-0.58
  • /test pull-kubevirt-e2e-k8s-1.23-sig-compute-migrations-0.58
  • /test pull-kubevirt-e2e-k8s-1.23-sig-compute-nonroot-0.58
  • /test pull-kubevirt-e2e-k8s-1.23-sig-network-0.58
  • /test pull-kubevirt-e2e-k8s-1.23-sig-network-nonroot-0.58
  • /test pull-kubevirt-e2e-k8s-1.23-sig-storage-0.58
  • /test pull-kubevirt-e2e-k8s-1.23-sig-storage-cgroupsv2-0.58
  • /test pull-kubevirt-e2e-k8s-1.23-sig-storage-nonroot-0.58
  • /test pull-kubevirt-e2e-k8s-1.24-ipv6-sig-network-0.58
  • /test pull-kubevirt-e2e-k8s-1.24-operator-0.58
  • /test pull-kubevirt-e2e-k8s-1.24-sig-compute-0.58
  • /test pull-kubevirt-e2e-k8s-1.24-sig-compute-migrations-0.58
  • /test pull-kubevirt-e2e-k8s-1.24-sig-compute-migrations-nonroot-0.58
  • /test pull-kubevirt-e2e-k8s-1.24-sig-network-0.58
  • /test pull-kubevirt-e2e-k8s-1.24-sig-storage-0.58
  • /test pull-kubevirt-e2e-k8s-1.24-sig-storage-dv-gc-0.58
  • /test pull-kubevirt-e2e-kind-1.22-sriov-nonroot-0.58
  • /test pull-kubevirt-e2e-kind-1.23-vgpu-0.58
  • /test pull-kubevirt-e2e-kind-1.23-vgpu-nonroot-0.58
  • /test pull-kubevirt-e2e-windows2016-0.58
  • /test pull-kubevirt-generate-0.58
  • /test pull-kubevirt-manifests-0.58
  • /test pull-kubevirt-prom-rules-verify-0.58
  • /test pull-kubevirt-unit-test-0.58
  • /test pull-kubevirt-verify-go-mod-0.58
  • /test pull-kubevirtci-bump-kubevirt-0.58

The following commands are available to trigger optional jobs:

  • /test build-kubevirt-builder-0.58
  • /test pull-kubevirt-check-tests-for-flakes-0.58
  • /test pull-kubevirt-code-lint-0.58
  • /test pull-kubevirt-e2e-arm64-0.58
  • /test pull-kubevirt-e2e-k8s-1.22-sig-compute-realtime-0.58
  • /test pull-kubevirt-e2e-k8s-1.22-sig-performance-0.58
  • /test pull-kubevirt-e2e-k8s-1.23-single-node-0.58
  • /test pull-kubevirt-e2e-k8s-1.23-swap-enabled-0.58
  • /test pull-kubevirt-e2e-k8s-1.24-psa-sig-compute-0.58
  • /test pull-kubevirt-e2e-k8s-1.24-psa-sig-storage-0.58
  • /test pull-kubevirt-e2e-kind-1.22-sriov-0.58
  • /test pull-kubevirt-gosec-0.58
  • /test pull-kubevirt-goveralls-0.58
  • /test pull-kubevirt-verify-rpms-0.58

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

  • pull-kubevirt-apidocs-0.58
  • pull-kubevirt-build-0.58
  • pull-kubevirt-build-arm64-0.58
  • pull-kubevirt-check-tests-for-flakes-0.58
  • pull-kubevirt-check-unassigned-tests-0.58
  • pull-kubevirt-client-python-0.58
  • pull-kubevirt-code-lint-0.58
  • pull-kubevirt-e2e-k8s-1.22-operator-0.58
  • pull-kubevirt-e2e-k8s-1.22-sig-compute-0.58
  • pull-kubevirt-e2e-k8s-1.22-sig-network-0.58
  • pull-kubevirt-e2e-k8s-1.22-sig-performance-0.58
  • pull-kubevirt-e2e-k8s-1.22-sig-storage-0.58
  • pull-kubevirt-e2e-k8s-1.23-operator-0.58
  • pull-kubevirt-e2e-k8s-1.23-operator-nonroot-0.58
  • pull-kubevirt-e2e-k8s-1.23-sig-compute-0.58
  • pull-kubevirt-e2e-k8s-1.23-sig-compute-migrations-0.58
  • pull-kubevirt-e2e-k8s-1.23-sig-compute-nonroot-0.58
  • pull-kubevirt-e2e-k8s-1.23-sig-network-0.58
  • pull-kubevirt-e2e-k8s-1.23-sig-network-nonroot-0.58
  • pull-kubevirt-e2e-k8s-1.23-sig-storage-0.58
  • pull-kubevirt-e2e-k8s-1.23-sig-storage-nonroot-0.58
  • pull-kubevirt-e2e-k8s-1.24-ipv6-sig-network-0.58
  • pull-kubevirt-e2e-k8s-1.24-operator-0.58
  • pull-kubevirt-e2e-k8s-1.24-sig-compute-0.58
  • pull-kubevirt-e2e-k8s-1.24-sig-compute-migrations-0.58
  • pull-kubevirt-e2e-k8s-1.24-sig-compute-migrations-nonroot-0.58
  • pull-kubevirt-e2e-k8s-1.24-sig-network-0.58
  • pull-kubevirt-e2e-k8s-1.24-sig-storage-0.58
  • pull-kubevirt-e2e-k8s-1.24-sig-storage-dv-gc-0.58
  • pull-kubevirt-e2e-kind-1.22-sriov-nonroot-0.58
  • pull-kubevirt-e2e-kind-1.23-vgpu-0.58
  • pull-kubevirt-e2e-kind-1.23-vgpu-nonroot-0.58
  • pull-kubevirt-e2e-windows2016-0.58
  • pull-kubevirt-generate-0.58
  • pull-kubevirt-goveralls-0.58
  • pull-kubevirt-manifests-0.58
  • pull-kubevirt-prom-rules-verify-0.58
  • pull-kubevirt-unit-test-0.58
  • pull-kubevirt-verify-go-mod-0.58

In response to this:

/test pull-kubevirt-build-release-0.58

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-build-0.58

@iholder101
Copy link
Contributor Author

/retest-required

@kubevirt-bot
Copy link
Contributor

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

In response to this:

/retest-required

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.

@fossedihelm
Copy link
Contributor

/retest-required

1 similar comment
@iholder101
Copy link
Contributor Author

/retest-required

@kubevirt-commenter-bot
Copy link

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

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/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants