Disable VMI migration upon K8s unschedulable taint #823

danielBelenky · 2020-09-22T11:42:18Z

Currently, the default behaviour is to migrate the VMI when a node is
tainted with the node.kubernetes.io/unschedulable key. This behaviour is
wrong and can be un-expected from the user's perspective because the
unschedulable taint means "do not schedule new workloads on this node"
and not "evict existing workloads from this node".

kubevirt/kubevirt#4012 adds support for the
eviction API so we can properly handle node drains and specific
evictions on VMI pods.

This patch avoid setting (and explicitly removes during upgrades if
set in the past) the default node drain key so we won't migrate VMIs
when we're not expected to do so.

Fixes: https://bugzilla.redhat.com/1881676

Signed-off-by: Daniel Belenky [email protected]

Release note:

VMIs will no longer migrate when node is tainted with node.kubernetes.io/unschedulable by default. Users can now use the proper node drain API to evacuate multiple VMIs from a node.

danielBelenky · 2020-09-22T11:42:32Z

/hold

Waiting for kubevirt/kubevirt#4012

ovirt-infra · 2020-09-22T11:45:07Z

All tests passed

tiraboschi

Please notice that virtconfig.MigrationsConfigKey is not reconciled so this is enough for new deployment but not for upgrades

openshift-ci-robot · 2020-09-22T13:07:26Z

@danielBelenky: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/hco-e2e-image-index-gcp	6d3459be2e239e9e5a334b9dd182d93c3857a233	link	`/test hco-e2e-image-index-gcp`
ci/prow/hco-e2e-image-index-azure	6d3459be2e239e9e5a334b9dd182d93c3857a233	link	`/test hco-e2e-image-index-azure`

Full PR test history. Your PR dashboard.

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.

hco-bot · 2020-09-22T14:56:43Z

hco-e2e-image-index-aws lane succeeded.
/override ci/prow/hco-e2e-image-index-azure
hco-e2e-image-index-aws lane succeeded.
/override ci/prow/hco-e2e-image-index-gcp

kubevirt-bot · 2020-09-22T14:56:48Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-azure, ci/prow/hco-e2e-image-index-gcp

In response to this:

hco-e2e-image-index-aws lane succeeded.
/override ci/prow/hco-e2e-image-index-azure
hco-e2e-image-index-aws lane succeeded.
/override ci/prow/hco-e2e-image-index-gcp

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 · 2020-10-14T12:04:04Z

@danielBelenky: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-hco-node-placement-k8s-1.17	6d3459be2e239e9e5a334b9dd182d93c3857a233	link	`/test pull-hco-node-placement-k8s-1.17`

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.

stu-gott

/lgtm

stu-gott · 2020-10-23T16:56:15Z

/hold cancel

ovirt-infra · 2020-10-27T15:43:02Z

All tests passed

tiraboschi · 2020-10-27T15:47:43Z

/cherry-pick release-1.2

kubevirt-bot · 2020-10-27T15:47:45Z

@tiraboschi: once the present PR merges, I will cherry-pick it on top of release-1.2 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.2

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.

Currently, the default behaviour is to migrate the VMI when a node is tainted with the node.kubernetes.io/unschedulable key. This behaviour is wrong and can be un-expected from the user's perspective because the unschedulable taint means "do not schedule new workloads on this node" and not "evict existing workloads from this node". kubevirt/kubevirt#4012 adds support for the eviction API so we can properly handle node drains and specific evictions on VMI pods. This patch avoid setting (and explicitly removes during upgrades if set in the past) the default node drain key so we won't migrate VMIs when we're not expected to do so. Fixes: https://bugzilla.redhat.com/1881676 Signed-off-by: Daniel Belenky <[email protected]> Signed-off-by: Simone Tiraboschi <[email protected]>

nunnatsa

/approve

kubevirt-bot · 2020-10-28T09:43:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nunnatsa

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

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

openshift-merge-robot · 2020-10-28T10:42:35Z

@danielBelenky: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/hco-e2e-upgrade-prev-aws	`3c05b7f`	link	`/test hco-e2e-upgrade-prev-aws`

Full PR test history. Your PR dashboard.

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.

hco-bot · 2020-10-28T10:55:28Z

hco-e2e-upgrade-prev-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-aws

kubevirt-bot · 2020-10-28T10:55:32Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-prev-aws

In response to this:

hco-e2e-upgrade-prev-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-aws

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 · 2020-10-28T11:00:01Z

@tiraboschi: #823 failed to apply on top of branch "release-1.2":

error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	pkg/controller/hyperconverged/hyperconverged_controller.go
M	pkg/controller/hyperconverged/hyperconverged_controller_components_test.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/controller/hyperconverged/hyperconverged_controller_components_test.go
Auto-merging pkg/controller/hyperconverged/hyperconverged_controller.go
CONFLICT (content): Merge conflict in pkg/controller/hyperconverged/hyperconverged_controller.go
Patch failed at 0001 Disable VMI migration upon K8s unschedulable taint

In response to this:

/cherry-pick release-1.2

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.

Currently, the default behaviour is to migrate the VMI when a node is tainted with the node.kubernetes.io/unschedulable key. This behaviour is wrong and can be un-expected from the user's perspective because the unschedulable taint means "do not schedule new workloads on this node" and not "evict existing workloads from this node". kubevirt/kubevirt#4012 adds support for the eviction API so we can properly handle node drains and specific evictions on VMI pods. This patch avoid setting (and explicitly removes during upgrades if set in the past) the default node drain key so we won't migrate VMIs when we're not expected to do so. Fixes: https://bugzilla.redhat.com/1881676 Signed-off-by: Daniel Belenky <[email protected]> Signed-off-by: Simone Tiraboschi <[email protected]> Co-authored-by: Daniel Belenky <[email protected]>

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 Sep 22, 2020

kubevirt-bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/XS labels Sep 22, 2020

kubevirt-bot requested review from sradco and yuvalturg September 22, 2020 11:42

tiraboschi requested changes Sep 22, 2020

View reviewed changes

kubevirt-bot assigned tiraboschi Sep 22, 2020

stu-gott approved these changes Oct 23, 2020

View reviewed changes

kubevirt-bot assigned stu-gott Oct 23, 2020

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 23, 2020

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, 2020

tiraboschi force-pushed the disable-migration-when-unschedulable branch from 6d3459b to 9a3fc2b Compare October 27, 2020 15:40

kubevirt-bot added size/M and removed lgtm Indicates that a PR is ready to be merged. size/XS labels Oct 27, 2020

tiraboschi force-pushed the disable-migration-when-unschedulable branch from 9a3fc2b to 3c05b7f Compare October 28, 2020 09:27

tiraboschi approved these changes Oct 28, 2020

View reviewed changes

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 28, 2020

nunnatsa approved these changes Oct 28, 2020

View reviewed changes

kubevirt-bot assigned nunnatsa Oct 28, 2020

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 28, 2020

nunnatsa mentioned this pull request Oct 28, 2020

move KV and CDI to the new operand pkg #903

Merged

kubevirt-bot merged commit 87fbc56 into kubevirt:master Oct 28, 2020

tiraboschi mentioned this pull request Oct 28, 2020

[release-1.2] Disable VMI migration upon K8s unschedulable taint (#823) #904

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable VMI migration upon K8s unschedulable taint #823

Disable VMI migration upon K8s unschedulable taint #823

danielBelenky commented Sep 22, 2020 •

edited by tiraboschi

Loading

danielBelenky commented Sep 22, 2020

ovirt-infra commented Sep 22, 2020

tiraboschi left a comment

openshift-ci-robot commented Sep 22, 2020

hco-bot commented Sep 22, 2020

kubevirt-bot commented Sep 22, 2020

kubevirt-bot commented Oct 14, 2020

stu-gott left a comment

stu-gott commented Oct 23, 2020

ovirt-infra commented Oct 27, 2020

tiraboschi commented Oct 27, 2020

kubevirt-bot commented Oct 27, 2020

nunnatsa left a comment

kubevirt-bot commented Oct 28, 2020

openshift-merge-robot commented Oct 28, 2020

hco-bot commented Oct 28, 2020

kubevirt-bot commented Oct 28, 2020

kubevirt-bot commented Oct 28, 2020

Disable VMI migration upon K8s unschedulable taint #823

Disable VMI migration upon K8s unschedulable taint #823

Conversation

danielBelenky commented Sep 22, 2020 • edited by tiraboschi Loading

danielBelenky commented Sep 22, 2020

ovirt-infra commented Sep 22, 2020

tiraboschi left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Sep 22, 2020

hco-bot commented Sep 22, 2020

kubevirt-bot commented Sep 22, 2020

kubevirt-bot commented Oct 14, 2020

stu-gott left a comment

Choose a reason for hiding this comment

stu-gott commented Oct 23, 2020

ovirt-infra commented Oct 27, 2020

tiraboschi commented Oct 27, 2020

kubevirt-bot commented Oct 27, 2020

nunnatsa left a comment

Choose a reason for hiding this comment

kubevirt-bot commented Oct 28, 2020

openshift-merge-robot commented Oct 28, 2020

hco-bot commented Oct 28, 2020

kubevirt-bot commented Oct 28, 2020

kubevirt-bot commented Oct 28, 2020

danielBelenky commented Sep 22, 2020 •

edited by tiraboschi

Loading