fix(vm): patch kvvm.spec.template.metadata on VM metadata changes

images/virtualization-artifact/pkg/common/vm/vm.go

@@ -27,6 +27,7 @@ import (
 	virtv1 "kubevirt.io/api/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
+	"github.com/deckhouse/virtualization-controller/pkg/common/annotations"
 	"github.com/deckhouse/virtualization-controller/pkg/common/object"
 	"github.com/deckhouse/virtualization/api/core/v1alpha2"
 )
@@ -115,3 +116,21 @@ func GetActivePodName(vm *v1alpha2.VirtualMachine) (string, bool) {
 
 	return "", false
 }
+
+// RemoveNonPropagatableAnnotations removes well known annotations that are dangerous to propagate.
+func RemoveNonPropagatableAnnotations(anno map[string]string) map[string]string {
+	res := make(map[string]string)
+
+	for k, v := range anno {
+		if k == annotations.LastPropagatedVMAnnotationsAnnotation || k == annotations.LastPropagatedVMLabelsAnnotation {
+			continue
+		}
+
+		if strings.HasPrefix(k, "kubectl.kubernetes.io") {
+			continue
+		}
+
+		res[k] = v
+	}
+	return res
+}

images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go

@@ -648,6 +648,8 @@ func (b *KVVM) SetMetadata(metadata metav1.ObjectMeta) {
 	}
 	maps.Copy(b.Resource.Spec.Template.ObjectMeta.Labels, metadata.Labels)
 	maps.Copy(b.Resource.Spec.Template.ObjectMeta.Annotations, metadata.Annotations)
+
+	b.Resource.Spec.Template.ObjectMeta.Annotations = vm.RemoveNonPropagatableAnnotations(b.Resource.Spec.Template.ObjectMeta.Annotations)
 }
 
 func (b *KVVM) SetUpdateVolumesStrategy(strategy *virtv1.UpdateVolumesStrategy) {

images/virtualization-artifact/pkg/controller/vm/internal/suite_test.go

@@ -62,6 +62,19 @@ func setupEnvironment(vm *v1alpha2.VirtualMachine, objs ...client.Object) (clien
 	return fakeClient, resource, vmState
 }
 
+func newEmptyKVVM(name, namespace string) *virtv1.VirtualMachine {
+	return &virtv1.VirtualMachine{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: virtv1.GroupVersion.String(),
+			Kind:       "VirtualMachine",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+		},
+	}
+}
+
 func newEmptyKVVMI(name, namespace string) *virtv1.VirtualMachineInstance {
 	return &virtv1.VirtualMachineInstance{
 		TypeMeta: metav1.TypeMeta{

images/virtualization-artifact/pkg/controller/vm/internal/sync_metadata.go

@@ -20,7 +20,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"strings"
+	"maps"
 
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
@@ -33,6 +33,7 @@ import (
 	"github.com/deckhouse/virtualization-controller/pkg/common/annotations"
 	"github.com/deckhouse/virtualization-controller/pkg/common/merger"
 	"github.com/deckhouse/virtualization-controller/pkg/common/patch"
+	commonvm "github.com/deckhouse/virtualization-controller/pkg/common/vm"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state"
 	"github.com/deckhouse/virtualization/api/core/v1alpha2"
 )
@@ -63,7 +64,8 @@ func (h *SyncMetadataHandler) Handle(ctx context.Context, s state.VirtualMachine
 	current := s.VirtualMachine().Current()
 
 	// Propagate user specified labels and annotations from the d8 VM to kubevirt VM.
-	kvvmMetaUpdated, err := PropagateVMMetadata(current, kvvm, kvvm)
+	kvvmNewMetadata := &metav1.ObjectMeta{}
+	kvvmMetaUpdated, err := PropagateVMMetadata(current, kvvm, kvvm, kvvmNewMetadata)
 	if err != nil {
 		return reconcile.Result{}, err
 	}
@@ -74,13 +76,14 @@ func (h *SyncMetadataHandler) Handle(ctx context.Context, s state.VirtualMachine
 	}
 	// Propagate user specified labels and annotations from the d8 VM to the kubevirt VirtualMachineInstance.
 	if kvvmi != nil {
-		metaUpdated, err := PropagateVMMetadata(current, kvvm, kvvmi)
+		kvvmiNewMetadata := &metav1.ObjectMeta{}
+		metaUpdated, err := PropagateVMMetadata(current, kvvm, kvvmi, kvvmiNewMetadata)
 		if err != nil {
 			return reconcile.Result{}, err
 		}
 
 		if metaUpdated {
-			if err = h.patchLabelsAndAnnotations(ctx, kvvmi, kvvmi.ObjectMeta); err != nil && !k8serrors.IsNotFound(err) {
+			if err = h.patchLabelsAndAnnotations(ctx, kvvmi, kvvmiNewMetadata); err != nil && !k8serrors.IsNotFound(err) {
 				return reconcile.Result{}, fmt.Errorf("failed to patch metadata KubeVirt VMI %q: %w", kvvmi.GetName(), err)
 			}
 		}
@@ -98,31 +101,32 @@ func (h *SyncMetadataHandler) Handle(ctx context.Context, s state.VirtualMachine
 			if pod.Status.Phase != corev1.PodRunning {
 				continue
 			}
-			metaUpdated, err := PropagateVMMetadata(current, kvvm, &pod)
+			podNewMetadata := &metav1.ObjectMeta{}
+			metaUpdated, err := PropagateVMMetadata(current, kvvm, &pod, podNewMetadata)
 			if err != nil {
 				return reconcile.Result{}, err
 			}
 
 			if metaUpdated {
-				if err = h.patchLabelsAndAnnotations(ctx, &pod, pod.ObjectMeta); err != nil {
+				if err = h.patchLabelsAndAnnotations(ctx, &pod, podNewMetadata); err != nil {
 					return reconcile.Result{}, fmt.Errorf("failed to patch KubeVirt Pod %q: %w", pod.GetName(), err)
 				}
 			}
 		}
 	}
 
-	labelsChanged, err := SetLastPropagatedLabels(kvvm, current)
+	labelsChanged, err := SetLastPropagatedLabels(kvvmNewMetadata, current)
 	if err != nil {
 		return reconcile.Result{}, fmt.Errorf("failed to set last propagated labels: %w", err)
 	}
 
-	annosChanged, err := SetLastPropagatedAnnotations(kvvm, current)
+	annosChanged, err := SetLastPropagatedAnnotations(kvvmNewMetadata, current)
 	if err != nil {
 		return reconcile.Result{}, fmt.Errorf("failed to set last propagated annotations: %w", err)
 	}
 
 	if labelsChanged || annosChanged || kvvmMetaUpdated {
-		if err = h.patchLabelsAndAnnotations(ctx, kvvm, kvvm.ObjectMeta); err != nil {
+		if err = h.patchLabelsAndAnnotations(ctx, kvvm, kvvmNewMetadata); err != nil {
 			return reconcile.Result{}, fmt.Errorf("failed to patch metadata KubeVirt VM %q: %w", kvvm.GetName(), err)
 		}
 	}
@@ -134,27 +138,67 @@ func (h *SyncMetadataHandler) Name() string {
 	return nameSyncMetadataHandler
 }
 
-func (h *SyncMetadataHandler) patchLabelsAndAnnotations(ctx context.Context, obj client.Object, metadata metav1.ObjectMeta) error {
-	jp := patch.NewJSONPatch(
-		patch.NewJSONPatchOperation(patch.PatchReplaceOp, "/metadata/labels", metadata.Labels),
-		patch.NewJSONPatchOperation(patch.PatchReplaceOp, "/metadata/annotations", metadata.Annotations),
-	)
+func (h *SyncMetadataHandler) patchLabelsAndAnnotations(ctx context.Context, obj client.Object, metadata *metav1.ObjectMeta) error {
+	jp := patch.NewJSONPatch()
+
+	newLabels := metadata.GetLabels()
+	newAnnotations := metadata.GetAnnotations()
+
+	// For KubeVirt VirtualMachine, also patch spec.template.metadata
+	// to ensure consistency with future VMI instances.
+	// KubeVirt doesn't trigger VMI restart on template metadata changes.
+	kvvm, ok := obj.(*virtv1.VirtualMachine)
+
+	if newLabels != nil {
+		jp.Append(
+			patch.WithTest("/metadata/labels", obj.GetLabels()),
+			patch.WithReplace("/metadata/labels", newLabels),
+		)
+		if ok {
+			jp.Append(
+				patch.WithTest("/spec/template/metadata/labels", kvvm.Spec.Template.ObjectMeta.Labels),
+				patch.WithReplace("/spec/template/metadata/labels", newLabels),
+			)
+		}
+	}
+
+	if newAnnotations != nil {
+		jp.Append(
+			patch.WithTest("/metadata/annotations", obj.GetAnnotations()),
+			patch.WithReplace("/metadata/annotations", newAnnotations),
+		)
+		if ok {
+			filteredAnno := commonvm.RemoveNonPropagatableAnnotations(newAnnotations)
+			jp.Append(
+				patch.WithTest("/spec/template/metadata/annotations", kvvm.Spec.Template.ObjectMeta.Annotations),
+				patch.WithReplace("/spec/template/metadata/annotations", filteredAnno),
+			)
+		}
+	}
+
 	bytes, err := jp.Bytes()
 	if err != nil {
 		return err
 	}
+
 	return h.client.Patch(ctx, obj, client.RawPatch(types.JSONPatchType, bytes))
 }
 
 // PropagateVMMetadata merges labels and annotations from the input VM into destination object.
 // Attach related labels and some dangerous annotations are not copied.
 // Return true if destination object was changed.
-func PropagateVMMetadata(vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine, destObj client.Object) (bool, error) {
-	// No changes if dest is nil.
-	if destObj == nil {
+func PropagateVMMetadata(vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine, origObj client.Object, metadata *metav1.ObjectMeta) (bool, error) {
+	// No changes if origObj is nil.
+	if origObj == nil {
 		return false, nil
 	}
 
+	metadata.Labels = make(map[string]string, len(origObj.GetLabels()))
+	metadata.Annotations = make(map[string]string, len(origObj.GetAnnotations()))
+
+	maps.Copy(metadata.Labels, origObj.GetLabels())
+	maps.Copy(metadata.Annotations, origObj.GetAnnotations())
+
 	// 1. Propagate labels.
 	lastPropagatedLabels, err := GetLastPropagatedLabels(kvvm)
 	if err != nil {
@@ -176,9 +220,9 @@ func PropagateVMMetadata(vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachin
 		propagateLabels[annotations.QuotaDiscountMemory] = vm.Status.Resources.Memory.RuntimeOverhead.String()
 	}
 
-	newLabels, labelsChanged := merger.ApplyMapChanges(destObj.GetLabels(), lastPropagatedLabels, propagateLabels)
+	newLabels, labelsChanged := merger.ApplyMapChanges(metadata.Labels, lastPropagatedLabels, propagateLabels)
 	if labelsChanged {
-		destObj.SetLabels(newLabels)
+		metadata.SetLabels(newLabels)
 	}
 
 	// 1. Propagate annotations.
@@ -188,11 +232,11 @@ func PropagateVMMetadata(vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachin
 	}
 
 	// Remove dangerous annotations.
-	curAnno := RemoveNonPropagatableAnnotations(vm.GetAnnotations())
+	curAnno := commonvm.RemoveNonPropagatableAnnotations(vm.GetAnnotations())
 
-	newAnno, annoChanged := merger.ApplyMapChanges(destObj.GetAnnotations(), lastPropagatedAnno, curAnno)
+	newAnno, annoChanged := merger.ApplyMapChanges(metadata.Annotations, lastPropagatedAnno, curAnno)
 	if annoChanged {
-		destObj.SetAnnotations(newAnno)
+		metadata.SetAnnotations(newAnno)
 	}
 
 	return labelsChanged || annoChanged, nil
@@ -211,19 +255,19 @@ func GetLastPropagatedLabels(kvvm *virtv1.VirtualMachine) (map[string]string, er
 	return lastPropagatedLabels, nil
 }
 
-func SetLastPropagatedLabels(kvvm *virtv1.VirtualMachine, vm *v1alpha2.VirtualMachine) (bool, error) {
+func SetLastPropagatedLabels(metadata *metav1.ObjectMeta, vm *v1alpha2.VirtualMachine) (bool, error) {
 	data, err := json.Marshal(vm.GetLabels())
 	if err != nil {
 		return false, err
 	}
 
 	newAnnoValue := string(data)
 
-	if kvvm.Annotations[annotations.LastPropagatedVMLabelsAnnotation] == newAnnoValue {
+	if metadata.Annotations[annotations.LastPropagatedVMLabelsAnnotation] == newAnnoValue {
 		return false, nil
 	}
 
-	annotations.AddAnnotation(kvvm, annotations.LastPropagatedVMLabelsAnnotation, newAnnoValue)
+	annotations.AddAnnotation(metadata, annotations.LastPropagatedVMLabelsAnnotation, newAnnoValue)
 	return true, nil
 }
 
@@ -240,36 +284,18 @@ func GetLastPropagatedAnnotations(kvvm *virtv1.VirtualMachine) (map[string]strin
 	return lastPropagatedAnno, nil
 }
 
-func SetLastPropagatedAnnotations(kvvm *virtv1.VirtualMachine, vm *v1alpha2.VirtualMachine) (bool, error) {
-	data, err := json.Marshal(RemoveNonPropagatableAnnotations(vm.GetAnnotations()))
+func SetLastPropagatedAnnotations(metadata *metav1.ObjectMeta, vm *v1alpha2.VirtualMachine) (bool, error) {
+	data, err := json.Marshal(commonvm.RemoveNonPropagatableAnnotations(vm.GetAnnotations()))
 	if err != nil {
 		return false, err
 	}
 
 	newAnnoValue := string(data)
 
-	if kvvm.Annotations[annotations.LastPropagatedVMAnnotationsAnnotation] == newAnnoValue {
+	if metadata.Annotations[annotations.LastPropagatedVMAnnotationsAnnotation] == newAnnoValue {
 		return false, nil
 	}
 
-	annotations.AddAnnotation(kvvm, annotations.LastPropagatedVMAnnotationsAnnotation, newAnnoValue)
+	annotations.AddAnnotation(metadata, annotations.LastPropagatedVMAnnotationsAnnotation, newAnnoValue)
 	return true, nil
 }
-
-// RemoveNonPropagatableAnnotations removes well known annotations that are dangerous to propagate.
-func RemoveNonPropagatableAnnotations(anno map[string]string) map[string]string {
-	res := make(map[string]string)
-
-	for k, v := range anno {
-		if k == annotations.LastPropagatedVMAnnotationsAnnotation || k == annotations.LastPropagatedVMLabelsAnnotation {
-			continue
-		}
-
-		if strings.HasPrefix(k, "kubectl.kubernetes.io") {
-			continue
-		}
-
-		res[k] = v
-	}
-	return res
-}