Filter out LiveMigrate workloadUpdateMethod on SNO (#1823)

Explicitly filter out LiveMigrate
workloadUpdateMethod on SNO clusters.
On SNO cluster Live Migration FG is globally
disabled because we have a single worker node
but still we allow the user (and it's
the default) to pass
workloadUpdateMethod=["LiveMigrate"]
and this causes an infinite loop in
the upgrade handler in virt-controller
that currently ignores the LiveMigration FG.
Let's explicitly filter out LiveMigrate
workloadUpdateMethod to prevent the issue.

Bug-url: https://bugzilla.redhat.com/show_bug.cgi?id=2065308

Signed-off-by: Simone Tiraboschi <stirabos@redhat.com>

api/v1beta1/hyperconverged_types.go

@@ -357,6 +357,7 @@ type HyperConvergedWorkloadUpdateStrategy struct {
 	// precedence over more disruptive methods. For example if both LiveMigrate and Evict
 	// methods are listed, only VMs which are not live migratable will be restarted/shutdown.
 	// An empty list defaults to no automated workload updating.
+	// LiveMigrate is ignored on SNO clusters.
 	//
 	// +listType=atomic
 	// +kubebuilder:default={"LiveMigrate"}

config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml

@@ -2148,7 +2148,8 @@ spec:
                       takes precedence over more disruptive methods. For example if
                       both LiveMigrate and Evict methods are listed, only VMs which
                       are not live migratable will be restarted/shutdown. An empty
-                      list defaults to no automated workload updating.
+                      list defaults to no automated workload updating. LiveMigrate
+                      is ignored on SNO clusters.
                     items:
                       type: string
                     type: array

controllers/operands/kubevirt.go

@@ -280,9 +280,11 @@ func hcWorkloadUpdateStrategyToKv(hcObject *hcov1beta1.HyperConvergedWorkloadUpd
 			*kvObject.BatchEvictionSize = *hcObject.BatchEvictionSize
 		}
 
-		if size := len(hcObject.WorkloadUpdateMethods); size > 0 {
+		filteredWUMethods := filterWorkloadupdatemethods(hcObject.WorkloadUpdateMethods)
+
+		if size := len(filteredWUMethods); size > 0 {
 			kvObject.WorkloadUpdateMethods = make([]kubevirtcorev1.WorkloadUpdateMethod, size)
-			for i, updateMethod := range hcObject.WorkloadUpdateMethods {
+			for i, updateMethod := range filteredWUMethods {
 				kvObject.WorkloadUpdateMethods[i] = kubevirtcorev1.WorkloadUpdateMethod(updateMethod)
 			}
 		}
@@ -291,6 +293,22 @@ func hcWorkloadUpdateStrategyToKv(hcObject *hcov1beta1.HyperConvergedWorkloadUpd
 	return kvObject
 }
 
+func filterWorkloadupdatemethods(workloadUpdateMethods []string) []string {
+	var filteredWUMethods []string
+
+	if hcoutil.GetClusterInfo().IsInfrastructureHighlyAvailable() {
+		return workloadUpdateMethods
+	} else {
+		for _, method := range workloadUpdateMethods {
+			if method != string(kubevirtcorev1.WorkloadUpdateMethodLiveMigrate) {
+				filteredWUMethods = append(filteredWUMethods, method)
+			}
+		}
+	}
+
+	return filteredWUMethods
+}
+
 func getKVConfig(hc *hcov1beta1.HyperConverged) (*kubevirtcorev1.KubeVirtConfiguration, error) {
 	devConfig, err := getKVDevConfig(hc)
 	if err != nil {

controllers/operands/kubevirt_test.go

@@ -2062,6 +2062,17 @@ Version: 1.2.3`)
 
 		Context("Workload Update Strategy", func() {
 			defaultBatchEvictionSize := 10
+			getClusterInfo := hcoutil.GetClusterInfo
+
+			BeforeEach(func() {
+				hcoutil.GetClusterInfo = func() hcoutil.ClusterInfo {
+					return &commonTestUtils.ClusterInfoMock{}
+				}
+			})
+
+			AfterEach(func() {
+				hcoutil.GetClusterInfo = getClusterInfo
+			})
 
 			It("should add Workload Update Strategy if missing in KV", func() {
 				existingResource, err := NewKubeVirt(hco)
@@ -2220,6 +2231,67 @@ Version: 1.2.3`)
 				Expect(*foundUpdateStrategy.BatchEvictionSize).Should(Equal(hcoModifiedBatchEvictionSize))
 				Expect(foundUpdateStrategy.BatchEvictionInterval.Duration.String()).Should(Equal("5m0s"))
 			})
+
+			DescribeTable("Should ignore LiveMigrate Workload Update Strategy on SNO",
+				func(hcoWorkloadUpdateMethods []string, expectedKVWorkloadUpdateMethods []kubevirtcorev1.WorkloadUpdateMethod) {
+
+					hcoutil.GetClusterInfo = func() hcoutil.ClusterInfo {
+						return &commonTestUtils.ClusterInfoSNOMock{}
+					}
+
+					existingKv, err := NewKubeVirt(hco)
+					Expect(err).ToNot(HaveOccurred())
+
+					hcoModifiedBatchEvictionSize := 5
+					hco.Spec.WorkloadUpdateStrategy.WorkloadUpdateMethods = hcoWorkloadUpdateMethods
+					hco.Spec.WorkloadUpdateStrategy.BatchEvictionInterval = &metav1.Duration{Duration: time.Minute * 5}
+					hco.Spec.WorkloadUpdateStrategy.BatchEvictionSize = &hcoModifiedBatchEvictionSize
+
+					cl := commonTestUtils.InitClient([]runtime.Object{hco, existingKv})
+					handler := (*genericOperand)(newKubevirtHandler(cl, commonTestUtils.GetScheme()))
+					res := handler.ensure(req)
+					Expect(res.UpgradeDone).To(BeFalse())
+					Expect(res.Updated).To(BeTrue())
+					Expect(res.Err).To(BeNil())
+
+					foundKv := &kubevirtcorev1.KubeVirt{}
+					Expect(
+						cl.Get(context.TODO(),
+							types.NamespacedName{Name: existingKv.Name, Namespace: existingKv.Namespace},
+							foundKv),
+					).To(BeNil())
+
+					Expect(foundKv.Spec.WorkloadUpdateStrategy.WorkloadUpdateMethods).Should(HaveLen(len(expectedKVWorkloadUpdateMethods)))
+					for _, expected := range expectedKVWorkloadUpdateMethods {
+						Expect(foundKv.Spec.WorkloadUpdateStrategy.WorkloadUpdateMethods).Should(ContainElements(expected))
+					}
+					Expect(foundKv.Spec.WorkloadUpdateStrategy.WorkloadUpdateMethods).ShouldNot(ContainElements(kubevirtcorev1.WorkloadUpdateMethod("LiveMigrate")))
+				},
+				Entry("LiveMigrate and others, LiveMigrate first",
+					[]string{"LiveMigrate", "test1", "test2"},
+					[]kubevirtcorev1.WorkloadUpdateMethod{"test1", "test2"},
+				),
+				Entry("LiveMigrate and others, LiveMigrate in the middle",
+					[]string{"test1", "LiveMigrate", "test2"},
+					[]kubevirtcorev1.WorkloadUpdateMethod{"test1", "test2"},
+				),
+				Entry("LiveMigrate and others, LiveMigrate last",
+					[]string{"test1", "test2", "LiveMigrate"},
+					[]kubevirtcorev1.WorkloadUpdateMethod{"test1", "test2"},
+				),
+				Entry("LiveMigrate only",
+					[]string{"LiveMigrate"},
+					[]kubevirtcorev1.WorkloadUpdateMethod{},
+				),
+				Entry("empty",
+					[]string{},
+					[]kubevirtcorev1.WorkloadUpdateMethod{},
+				),
+				Entry("LiveMigrate and Evict",
+					[]string{"LiveMigrate", "Evict"},
+					[]kubevirtcorev1.WorkloadUpdateMethod{"Evict"},
+				))
+
 		})
 
 		Context("SNO replicas", func() {

deploy/crds/hco00.crd.yaml

@@ -2148,7 +2148,8 @@ spec:
                       takes precedence over more disruptive methods. For example if
                       both LiveMigrate and Evict methods are listed, only VMs which
                       are not live migratable will be restarted/shutdown. An empty
-                      list defaults to no automated workload updating.
+                      list defaults to no automated workload updating. LiveMigrate
+                      is ignored on SNO clusters.
                     items:
                       type: string
                     type: array

deploy/index-image/community-kubevirt-hyperconverged/1.7.0/manifests/hco00.crd.yaml

@@ -2148,7 +2148,8 @@ spec:
                       takes precedence over more disruptive methods. For example if
                       both LiveMigrate and Evict methods are listed, only VMs which
                       are not live migratable will be restarted/shutdown. An empty
-                      list defaults to no automated workload updating.
+                      list defaults to no automated workload updating. LiveMigrate
+                      is ignored on SNO clusters.
                     items:
                       type: string
                     type: array

deploy/olm-catalog/community-kubevirt-hyperconverged/1.7.0/manifests/hco00.crd.yaml

@@ -2148,7 +2148,8 @@ spec:
                       takes precedence over more disruptive methods. For example if
                       both LiveMigrate and Evict methods are listed, only VMs which
                       are not live migratable will be restarted/shutdown. An empty
-                      list defaults to no automated workload updating.
+                      list defaults to no automated workload updating. LiveMigrate
+                      is ignored on SNO clusters.
                     items:
                       type: string
                     type: array

docs/api.md

@@ -164,7 +164,7 @@ HyperConvergedWorkloadUpdateStrategy defines options related to updating a KubeV
 
 | Field | Description | Scheme | Default | Required |
 | ----- | ----------- | ------ | -------- |-------- |
-| workloadUpdateMethods | WorkloadUpdateMethods defines the methods that can be used to disrupt workloads during automated workload updates. When multiple methods are present, the least disruptive method takes precedence over more disruptive methods. For example if both LiveMigrate and Evict methods are listed, only VMs which are not live migratable will be restarted/shutdown. An empty list defaults to no automated workload updating. | []string | {"LiveMigrate"} | false |
+| workloadUpdateMethods | WorkloadUpdateMethods defines the methods that can be used to disrupt workloads during automated workload updates. When multiple methods are present, the least disruptive method takes precedence over more disruptive methods. For example if both LiveMigrate and Evict methods are listed, only VMs which are not live migratable will be restarted/shutdown. An empty list defaults to no automated workload updating. LiveMigrate is ignored on SNO clusters. | []string | {"LiveMigrate"} | false |
 | batchEvictionSize | BatchEvictionSize Represents the number of VMIs that can be forced updated per the BatchShutdownInteral interval | *int | 10 | false |
 | batchEvictionInterval | BatchEvictionInterval Represents the interval to wait before issuing the next batch of shutdowns | *metav1.Duration | "1m0s" | false |
 

docs/cluster-configuration.md

@@ -508,6 +508,7 @@ The `workloadUpdateStrategy` fields are:
   An empty list defaults to no automated workload updating.
 
   The default values is `LiveMigrate`; `Evict` is not enabled by default being potentially disruptive for the existing workloads.
+* `LiveMigrate` is ignored on SNO clusters where LiveMigrate is not supported.
 
 ### workloadUpdateStrategy example
 ```yaml