Repository: kubermatic/machine-controller
Pull request #1897 (2025-01-27 20:53:25)

Support kubevirt evection strategy in kv provider
**What this PR does / why we need it**:
Currently, the KubeVirt VMs which are created using machine controller have an eviction strategy of `External` . With `EvictionStrategy: External`, kubevirt will create a PDB for the VMI which blocks eviction, and it will also set the `vmi.Status.EvacuationNodeName` on the vmi's status. When a VMI is being evicted, we need a way to have that eviction blocked so the cluster api related controllers can properly drain the k8s node running within the VMI before the VMI is torn down.

However, in some scenarios, users would like to be able to migrate VMs without blocking the migration by machine controller thus we would like to enable the users to decide which eviction strategy they would like to use.
**Which issue(s) this PR fixes**:
<!--optional, in `fixes #<issue number>` format, will close the issue(s) when PR gets merged-->
Fixes #

**What type of PR is this?**
/kind feature
<!--
Add one of the following kinds:
/kind bug
/kind cleanup
/kind documentation
/kind feature
/kind design

Optionally add one or more of the following kinds if applicable:
/kind api-change
/kind deprecation
/kind failing-test
/kind flake
/kind regression
/kind chore
-->

**Special notes for your reviewer**:

**Does this PR introduce a user-facing change? Then add your Release Note here**:
<!--
Write your release note:
1. Enter your extended release note in the below block. If the PR requires additional action from users switching to the new release, include the string "action required".
2. If no release note is required, just write "NONE".
-->
```release-note
Support multiple eviction strategy for KubeVirt VMs
```

**Documentation**:
<!--
Please do one of the following options:
- Add a link to the existing documentation
- Add a link to the kubermatic/docs pull request
- If no documentation change is applicable then add:
  - TBD (documentation will be added later)
  - NONE (no documentation needed for this PR)
-->
```documentation
None
```


kubermatic-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
**Once this PR has been reviewed and has the lgtm label**, please ask for approval from [moadqassem](https://github.com/moadqassem). For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubermatic%2Fmachine-controller).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubermatic/machine-controller/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["moadqassem"]} -->
kubermatic-bot: @moadqassem: 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-machine-controller-yamllint | 9fb8a3a1e8e37d6514fc4396d9ea8c0dc5954632 | [link](https://public-prow.loodse.com/view/gs/prow-dev-public-data/pr-logs/pull/kubermatic_machine-controller/1897/pull-machine-controller-yamllint/1883981726296838144) | true | `/test pull-machine-controller-yamllint`

[Full PR test history](https://public-prow.loodse.com/pr-history?org=kubermatic&repo=machine-controller&pr=1897)

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
kubermatic-bot: @moadqassem: 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-machine-controller-test | 5c25d2cada2aa4a16b0cb6fdcd28c56f31ba9ba2 | [link](https://public-prow.loodse.com/view/gs/prow-dev-public-data/pr-logs/pull/kubermatic_machine-controller/1897/pull-machine-controller-test/1883983482510643200) | true | `/test pull-machine-controller-test`

[Full PR test history](https://public-prow.loodse.com/pr-history?org=kubermatic&repo=machine-controller&pr=1897)

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
kubermatic-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubermatic/machine-controller/pull/1897#pullrequestreview-2577440360" title="Approved">ahmedwaleedmalik</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubermatic%2Fmachine-controller).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubermatic/machine-controller/blob/main/OWNERS)~~ [ahmedwaleedmalik]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
kubermatic-bot: LGTM label has been added.  <details>Git tree hash: 00772a2ce5cd0808a6d24e700e3ee5b4ad5a5144</details>

Patch:
diff --git a/pkg/cloudprovider/provider/kubevirt/provider.go b/pkg/cloudprovider/provider/kubevirt/provider.go
index 8f1e35d9..dbcb0dc8 100644
--- a/pkg/cloudprovider/provider/kubevirt/provider.go
+++ b/pkg/cloudprovider/provider/kubevirt/provider.go
@@ -117,6 +117,7 @@ type Config struct {
 	ExtraHeaders              []string
 	ExtraHeadersSecretRef     string
 	DataVolumeSecretRef       string
+	EvictionStrategy          kubevirtv1.EvictionStrategy
 
 	ProviderNetworkName string
 	SubnetName          string
@@ -366,6 +367,10 @@ func (p *provider) getConfig(provSpec clusterv1alpha1.ProviderSpec) (*Config, *p
 		}
 	}
 
+	if rawConfig.VirtualMachine.EvictionStrategy != "" {
+		config.EvictionStrategy = kubevirtv1.EvictionStrategy(rawConfig.VirtualMachine.EvictionStrategy)
+	}
+
 	return &config, pconfig, nil
 }
 
@@ -635,6 +640,13 @@ func (p *provider) Validate(ctx context.Context, _ *zap.SugaredLogger, spec clus
 		return fmt.Errorf("failed to request VirtualMachineInstances: %w", err)
 	}
 
+	if c.EvictionStrategy != "" {
+		if c.EvictionStrategy != kubevirtv1.EvictionStrategyExternal &&
+			c.EvictionStrategy != kubevirtv1.EvictionStrategyLiveMigrate {
+			return fmt.Errorf("unsupported vm eviction strategy: %s", c.EvictionStrategy)
+		}
+	}
+
 	return nil
 }
 
@@ -729,6 +741,9 @@ func (p *provider) newVirtualMachine(c *Config, pc *providerconfigtypes.Config,
 	terminationGracePeriodSeconds := int64(30)
 
 	evictionStrategy := kubevirtv1.EvictionStrategyExternal
+	if c.EvictionStrategy != "" {
+		evictionStrategy = c.EvictionStrategy
+	}
 
 	resourceRequirements := kubevirtv1.ResourceRequirements{}
 	labels["kubevirt.io/vm"] = machine.Name
diff --git a/pkg/cloudprovider/provider/kubevirt/types/types.go b/pkg/cloudprovider/provider/kubevirt/types/types.go
index e86c8c4e..79e7100f 100644
--- a/pkg/cloudprovider/provider/kubevirt/types/types.go
+++ b/pkg/cloudprovider/provider/kubevirt/types/types.go
@@ -59,6 +59,7 @@ type VirtualMachine struct {
 	Location                *Location                           `json:"location,omitempty"`
 	ProviderNetwork         *ProviderNetwork                    `json:"providerNetwork,omitempty"`
 	EnableNetworkMultiQueue providerconfigtypes.ConfigVarBool   `json:"enableNetworkMultiQueue,omitempty"`
+	EvictionStrategy        string                              `json:"evictionStrategy,omitempty"`
 }
 
 // Flavor.


Test patch:
diff --git a/pkg/cloudprovider/provider/kubevirt/provider_test.go b/pkg/cloudprovider/provider/kubevirt/provider_test.go
index 49b5b1a5..4651889d 100644
--- a/pkg/cloudprovider/provider/kubevirt/provider_test.go
+++ b/pkg/cloudprovider/provider/kubevirt/provider_test.go
@@ -71,6 +71,7 @@ type kubevirtProviderSpecConf struct {
 	PullMethod               cdiv1beta1.RegistryPullMethod
 	ProviderNetwork          *types.ProviderNetwork
 	ExtraHeadersSet          bool
+	EvictStrategy            string
 }
 
 func (k kubevirtProviderSpecConf) rawProviderSpec(t *testing.T) []byte {
@@ -104,6 +105,9 @@ func (k kubevirtProviderSpecConf) rawProviderSpec(t *testing.T) []byte {
 		},
 		{{- end }}
 		"virtualMachine": {
+            {{- if .EvictStrategy }}
+            "evictionStrategy": "LiveMigrate",        
+            {{- end }}
             {{- if .ProviderNetwork }}
             "providerNetwork": {
                "name": "kubeovn",
@@ -275,6 +279,10 @@ func TestNewVirtualMachine(t *testing.T) {
 			name:     "pvc-image-source",
 			specConf: kubevirtProviderSpecConf{OsImageSource: pvcSource, OsImageDV: "ns/dvname"},
 		},
+		{
+			name:     "eviction-strategy-live-migrate",
+			specConf: kubevirtProviderSpecConf{EvictStrategy: "LiveMigrate"},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
diff --git a/pkg/cloudprovider/provider/kubevirt/testdata/eviction-strategy-live-migrate.yaml b/pkg/cloudprovider/provider/kubevirt/testdata/eviction-strategy-live-migrate.yaml
new file mode 100644
index 00000000..709d0f4a
--- /dev/null
+++ b/pkg/cloudprovider/provider/kubevirt/testdata/eviction-strategy-live-migrate.yaml
@@ -0,0 +1,80 @@
+apiVersion: kubevirt.io/v1
+kind: VirtualMachine
+metadata:
+  annotations:
+  labels:
+    cluster.x-k8s.io/cluster-name: cluster-name
+    cluster.x-k8s.io/role: worker
+    kubevirt.io/vm: eviction-strategy-live-migrate
+    md: md-name
+  name: eviction-strategy-live-migrate
+  namespace: test-namespace
+spec:
+  dataVolumeTemplates:
+    - metadata:
+        name: eviction-strategy-live-migrate
+      spec:
+        storage:
+          accessModes:
+            - ReadWriteMany
+          resources:
+            requests:
+              storage: 10Gi
+          storageClassName: longhorn
+        source:
+          http:
+            url: http://x.y.z.t/ubuntu.img
+  runStrategy: Once
+  template:
+    metadata:
+      creationTimestamp: null
+      annotations:
+        "kubevirt.io/allow-pod-bridge-network-live-migration": "true"
+        "ovn.kubernetes.io/allow_live_migration": "true"
+      labels:
+        cluster.x-k8s.io/cluster-name: cluster-name
+        cluster.x-k8s.io/role: worker
+        kubevirt.io/vm: eviction-strategy-live-migrate
+        md: md-name
+    spec:
+      affinity: {}
+      domain:
+        devices:
+          disks:
+            - disk:
+                bus: virtio
+              name: datavolumedisk
+            - disk:
+                bus: virtio
+              name: cloudinitdisk
+          interfaces:
+            - name: default
+              bridge: {}
+          networkInterfaceMultiqueue: true
+        resources:
+          limits:
+            cpu: "2"
+            memory: 2Gi
+          requests:
+            cpu: "2"
+            memory: 2Gi
+      networks:
+        - name: default
+          pod: {}
+      terminationGracePeriodSeconds: 30
+      topologyspreadconstraints:
+        - maxskew: 1
+          topologykey: kubernetes.io/hostname
+          whenunsatisfiable: ScheduleAnyway
+          labelselector:
+            matchlabels:
+              md: md-name
+      volumes:
+        - dataVolume:
+            name: eviction-strategy-live-migrate
+          name: datavolumedisk
+        - cloudInitNoCloud:
+            secretRef:
+              name: udsn
+          name: cloudinitdisk
+      evictionStrategy: LiveMigrate