common-instancetypes: Ignore objects owned by virt-operator

With this change the common-instancetypes operand ignores resources that are labelled as being owned by KubeVirt's virt-operator during reconciliation. Co-authored-by: Lee Yarwood <[email protected]> Signed-off-by: Felix Matouschek <[email protected]>
kubevirt · Oct 24, 2023 · 434d4c5 · 434d4c5
1 parent 345d81d
commit 434d4c5
Show file tree

Hide file tree

Showing 4 changed files with 129 additions and 0 deletions.
diff --git a/internal/common/resource.go b/internal/common/resource.go
@@ -28,6 +28,7 @@ const (
 	OperationResultCreated OperationResult = "created"
 	OperationResultUpdated OperationResult = "updated"
 	OperationResultDeleted OperationResult = "deleted"
+	OperationResultIgnored OperationResult = "ignored"
 )
 
 type StatusMessage = *string

diff --git a/internal/operands/common-instancetypes/reconcile.go b/internal/operands/common-instancetypes/reconcile.go
@@ -8,13 +8,15 @@ import (
 	"path/filepath"
 	"strings"
 
+	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/util/yaml"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/kustomize/api/krusty"
 	"sigs.k8s.io/kustomize/api/resmap"
 	"sigs.k8s.io/kustomize/api/resource"
 	"sigs.k8s.io/kustomize/kyaml/filesys"
 
+	virtv1 "kubevirt.io/api/core/v1"
 	instancetypeapi "kubevirt.io/api/instancetype"
 	instancetypev1beta1 "kubevirt.io/api/instancetype/v1beta1"
 	"kubevirt.io/ssp-operator/internal/common"
@@ -321,6 +323,12 @@ func (c *CommonInstancetypes) reconcileVirtualMachineClusterInstancetypesFuncs()
 	for i := range c.virtualMachineClusterInstancetypes {
 		clusterInstancetype := &c.virtualMachineClusterInstancetypes[i]
 		funcs = append(funcs, func(request *common.Request) (common.ReconcileResult, error) {
+			if ignore, err := ignoreObjectOwnedByVirtOperator(request, clusterInstancetype.DeepCopy()); err != nil {
+				return common.ReconcileResult{}, err
+			} else if ignore {
+				return common.ReconcileResult{Resource: clusterInstancetype, OperationResult: common.OperationResultIgnored}, nil
+			}
+
 			return common.CreateOrUpdate(request).
 				ClusterResource(clusterInstancetype).
 				WithAppLabels(operandName, operandComponent).
@@ -335,6 +343,12 @@ func (c *CommonInstancetypes) reconcileVirtualMachineClusterPreferencesFuncs() [
 	for i := range c.virtualMachineClusterPreferences {
 		clusterPreference := &c.virtualMachineClusterPreferences[i]
 		funcs = append(funcs, func(request *common.Request) (common.ReconcileResult, error) {
+			if ignore, err := ignoreObjectOwnedByVirtOperator(request, clusterPreference.DeepCopy()); err != nil {
+				return common.ReconcileResult{}, err
+			} else if ignore {
+				return common.ReconcileResult{Resource: clusterPreference, OperationResult: common.OperationResultIgnored}, nil
+			}
+
 			return common.CreateOrUpdate(request).
 				ClusterResource(clusterPreference).
 				WithAppLabels(operandName, operandComponent).
@@ -343,3 +357,25 @@ func (c *CommonInstancetypes) reconcileVirtualMachineClusterPreferencesFuncs() [
 	}
 	return funcs
 }
+
+func ignoreObjectOwnedByVirtOperator(request *common.Request, obj client.Object) (bool, error) {
+	// During an upgrade to v0.19.0 we might encounter virt-operator attempting
+	// to reconcile the same set of common-instancetype resources. Ignore these
+	// requests to reconcile such objects once owned by virt-operator.
+	if err := request.Client.Get(request.Context, client.ObjectKeyFromObject(obj), obj); err != nil {
+		if errors.IsNotFound(err) {
+			return false, nil
+		}
+		return true, err
+	}
+
+	return isOwnedByVirtOperator(obj), nil
+}
+
+func isOwnedByVirtOperator(obj client.Object) bool {
+	if obj.GetLabels() == nil {
+		return false
+	}
+	managedValue, managedOk := obj.GetLabels()[virtv1.ManagedByLabel]
+	return managedOk && managedValue == virtv1.ManagedByLabelOperatorValue
+}
diff --git a/internal/operands/common-instancetypes/reconcile_test.go b/internal/operands/common-instancetypes/reconcile_test.go
@@ -26,6 +26,7 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/filesys"
 	"sigs.k8s.io/kustomize/kyaml/kio"
 
+	virtv1 "kubevirt.io/api/core/v1"
 	instancetypeapi "kubevirt.io/api/instancetype"
 	instancetypev1beta1 "kubevirt.io/api/instancetype/v1beta1"
 	ssp "kubevirt.io/ssp-operator/api/v1beta2"
@@ -251,6 +252,56 @@ var _ = Describe("Common-Instancetypes operand", func() {
 		ExpectResourceExists(preference, request)
 	})
 
+	It("should ignore virt-operator owned objects during reconcile when also provided by bundle", func() {
+		_, err = operand.Reconcile(&request)
+		Expect(err).ToNot(HaveOccurred())
+
+		instancetypeList := &instancetypev1beta1.VirtualMachineClusterInstancetypeList{}
+		Expect(request.Client.List(request.Context, instancetypeList, &client.ListOptions{})).To(Succeed())
+		Expect(len(instancetypeList.Items) > 0).To(BeTrue())
+
+		preferenceList := &instancetypev1beta1.VirtualMachineClusterPreferenceList{}
+		Expect(request.Client.List(request.Context, preferenceList, &client.ListOptions{})).To(Succeed())
+		Expect(len(preferenceList.Items) > 0).To(BeTrue())
+
+		// Mutate the instance type while also adding the labels for virt-operator
+		instancetypeToUpdate := instancetypeList.Items[0]
+		updatedCPUGuestCount := instancetypeToUpdate.Spec.CPU.Guest + 1
+		instancetypeToUpdate.Spec.CPU.Guest = updatedCPUGuestCount
+		instancetypeToUpdate.Labels = map[string]string{
+			virtv1.ManagedByLabel: virtv1.ManagedByLabelOperatorValue,
+		}
+		Expect(request.Client.Update(request.Context, &instancetypeToUpdate, &client.UpdateOptions{})).To(Succeed())
+
+		// Mutate the preference while also adding the labels for virt-operator
+		preferenceToUpdate := preferenceList.Items[0]
+		updatedPreferredCPUTopology := instancetypev1beta1.PreferCores
+		updatedPreferenceCPU := &instancetypev1beta1.CPUPreferences{
+			PreferredCPUTopology: &updatedPreferredCPUTopology,
+		}
+		preferenceToUpdate.Spec.CPU = updatedPreferenceCPU
+		preferenceToUpdate.Labels = map[string]string{
+			virtv1.ManagedByLabel: virtv1.ManagedByLabelOperatorValue,
+		}
+		Expect(request.Client.Update(request.Context, &preferenceToUpdate, &client.UpdateOptions{})).To(Succeed())
+
+		results, err := operand.Reconcile(&request)
+		Expect(err).ToNot(HaveOccurred())
+
+		// Assert that we have reported ignoring the attempt to reconcile the objects owned by virt-operator
+		for _, res := range results {
+			if res.Resource.GetName() == instancetypeToUpdate.Name || res.Resource.GetName() == preferenceToUpdate.Name {
+				Expect(res.OperationResult).To(Equal(common.OperationResultIgnored))
+			}
+		}
+
+		// Assert that the mutations made above persist as the reconcile is being ignored
+		Expect(request.Client.Get(request.Context, client.ObjectKeyFromObject(&instancetypeToUpdate), &instancetypeToUpdate, &client.GetOptions{})).To(Succeed())
+		Expect(instancetypeToUpdate.Spec.CPU.Guest).To(Equal(updatedCPUGuestCount))
+		Expect(request.Client.Get(request.Context, client.ObjectKeyFromObject(&preferenceToUpdate), &preferenceToUpdate, &client.GetOptions{})).To(Succeed())
+		Expect(preferenceToUpdate.Spec.CPU).To(Equal(updatedPreferenceCPU))
+	})
+
 	It("should create and cleanup resources from an external URL", func() {
 		// Generate a mock ResMap and resources for the test
 		mockResMap, virtualMachineClusterInstancetypes, virtualMachineClusterPreferences, err := newMockResources(10, 10)

diff --git a/tests/common_instancetypes_test.go b/tests/common_instancetypes_test.go
@@ -4,6 +4,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"k8s.io/utils/pointer"
+	virtv1 "kubevirt.io/api/core/v1"
 	instancetypev1beta1 "kubevirt.io/api/instancetype/v1beta1"
 	ssp "kubevirt.io/ssp-operator/api/v1beta2"
 	common_instancetypes "kubevirt.io/ssp-operator/internal/operands/common-instancetypes"
@@ -60,6 +61,46 @@ var _ = Describe("Common Instance Types", func() {
 				Expect(apiClient.Get(ctx, client.ObjectKey{Name: preference.Name}, &instancetypev1beta1.VirtualMachineClusterPreference{})).To(Succeed())
 			}
 		})
+		It("should ignore resources owned by virt-operator", func() {
+			virtualMachineClusterInstancetypes, err := common_instancetypes.FetchBundleResource[instancetypev1beta1.VirtualMachineClusterInstancetype]("../" + common_instancetypes.BundleDir + common_instancetypes.ClusterInstancetypesBundle)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(virtualMachineClusterInstancetypes).ToNot(BeEmpty())
+
+			virtualMachineClusterPreferences, err := common_instancetypes.FetchBundleResource[instancetypev1beta1.VirtualMachineClusterPreference]("../" + common_instancetypes.BundleDir + common_instancetypes.ClusterPreferencesBundle)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(virtualMachineClusterPreferences).ToNot(BeEmpty())
+
+			// Mutate the preference while also adding the labels for virt-operator
+			instancetypeToUpdate := &virtualMachineClusterInstancetypes[0]
+			Expect(apiClient.Get(ctx, client.ObjectKey{Name: instancetypeToUpdate.Name}, instancetypeToUpdate)).To(Succeed())
+			updatedCPUGuestCount := instancetypeToUpdate.Spec.CPU.Guest + 1
+			instancetypeToUpdate.Spec.CPU.Guest = updatedCPUGuestCount
+			instancetypeToUpdate.Labels = map[string]string{
+				virtv1.ManagedByLabel: virtv1.ManagedByLabelOperatorValue,
+			}
+			Expect(apiClient.Update(ctx, instancetypeToUpdate)).To(Succeed())
+
+			// Mutate the preference while also adding the labels for virt-operator
+			preferenceToUpdate := &virtualMachineClusterPreferences[0]
+			Expect(apiClient.Get(ctx, client.ObjectKey{Name: preferenceToUpdate.Name}, preferenceToUpdate)).To(Succeed())
+			updatedPreferredCPUTopology := instancetypev1beta1.PreferCores
+			updatedPreferenceCPU := &instancetypev1beta1.CPUPreferences{
+				PreferredCPUTopology: &updatedPreferredCPUTopology,
+			}
+			preferenceToUpdate.Spec.CPU = updatedPreferenceCPU
+			preferenceToUpdate.Labels = map[string]string{
+				virtv1.ManagedByLabel: virtv1.ManagedByLabelOperatorValue,
+			}
+			Expect(apiClient.Update(ctx, preferenceToUpdate)).To(Succeed())
+
+			triggerReconciliation()
+
+			// Assert that the mutations made above persist as the reconcile is being ignored
+			Expect(apiClient.Get(ctx, client.ObjectKey{Name: instancetypeToUpdate.Name}, instancetypeToUpdate)).To(Succeed())
+			Expect(instancetypeToUpdate.Spec.CPU.Guest).To(Equal(updatedCPUGuestCount))
+			Expect(apiClient.Get(ctx, client.ObjectKey{Name: preferenceToUpdate.Name}, preferenceToUpdate)).To(Succeed())
+			Expect(preferenceToUpdate.Spec.CPU).To(Equal(updatedPreferenceCPU))
+		})
 	})
 	Context("webhook", func() {
 		DescribeTable("should reject URL", func(URL string) {