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

GA VMLiveUpdateFeatures feature-gate #13091

Merged
merged 2 commits into from
Oct 23, 2024

Conversation

acardace
Copy link
Member

What this PR does

The VMLiveUpdateFeatures feature-gate has been introduced in KubeVirt v1.0.0 after being here for multiple releases and having undergone major cleanups and bug fixing I think is now ready to be GAed.

Note that this PR just GAs the feature-gate and doesn't change the default vmRolloutStrategy which remains Stage.

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

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.

Release note

GA the `VMLiveUpdateFeatures` feature-gate.

@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 Oct 18, 2024
@acardace acardace requested review from vladikr, fossedihelm, alicefr and xpivarc and removed request for kbidarkar, akalenyu and iholder101 October 18, 2024 10:56
Copy link
Contributor

@fossedihelm fossedihelm left a comment

Choose a reason for hiding this comment

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

Leftover in unit test

pkg/virt-controller/watch/vm/vm_test.go Outdated Show resolved Hide resolved
pkg/virt-controller/watch/vm/vm_test.go Outdated Show resolved Hide resolved
@acardace acardace force-pushed the ga-rollout-strategy branch from c679ae3 to a46ee21 Compare October 21, 2024 11:18
@kubevirt-bot kubevirt-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L and removed size/M labels Oct 21, 2024
@acardace acardace force-pushed the ga-rollout-strategy branch from a46ee21 to 5dc6859 Compare October 21, 2024 11:28
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 21, 2024
@fossedihelm
Copy link
Contributor

@acardace Failures are relevant

@acardace acardace force-pushed the ga-rollout-strategy branch from 5dc6859 to 07c713c Compare October 22, 2024 08:25
@kubevirt-bot kubevirt-bot added dco-signoff: no Indicates the PR's author has not DCO signed all their commits. and removed dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Oct 22, 2024
@acardace acardace force-pushed the ga-rollout-strategy branch from 07c713c to fde7e71 Compare October 22, 2024 08:28
@kubevirt-bot 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 Oct 22, 2024
Copy link
Member

@xpivarc xpivarc 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: 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:

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 Oct 22, 2024
@fossedihelm
Copy link
Contributor

LGTM but I will wait for #13090 to be merged first, since there will be conflict with the decorator

@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 23, 2024
@acardace acardace force-pushed the ga-rollout-strategy branch from fde7e71 to 79a37fd Compare October 23, 2024 08:18
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 23, 2024
@acardace acardace force-pushed the ga-rollout-strategy branch from 79a37fd to cf7c1c3 Compare October 23, 2024 08:22
@acardace acardace force-pushed the ga-rollout-strategy branch from cf7c1c3 to 135d74d Compare October 23, 2024 08:28
@fossedihelm
Copy link
Contributor

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 23, 2024
@kubevirt-commenter-bot
Copy link

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

@kubevirt-bot
Copy link
Contributor

kubevirt-bot commented Oct 23, 2024

@acardace: 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-check-tests-for-flakes 135d74d link false /test pull-kubevirt-check-tests-for-flakes

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.

@xpivarc
Copy link
Member

xpivarc commented Oct 23, 2024

/hold
@vladikr

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 23, 2024
@vladikr
Copy link
Member

vladikr commented Oct 23, 2024

Thanks @xpivarc .
/unhold

I'm not against this change. I just wanted to make sure that we are following kubevirt/community#251 going forward.

@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 Oct 23, 2024
@kubevirt-bot kubevirt-bot merged commit 891122a into kubevirt:main Oct 23, 2024
39 of 40 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. sig/storage size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants