OCPCLOUD-2992: machine update

e2e/machine_migration_capi_authoritative_test.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
 	configv1 "github.com/openshift/api/config/v1"
 	mapiv1beta1 "github.com/openshift/api/machine/v1beta1"
 	mapiframework "github.com/openshift/cluster-api-actuator-pkg/pkg/framework"
@@ -12,6 +13,7 @@ import (
 
 	awsv1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
 	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
+	"sigs.k8s.io/controller-runtime/pkg/envtest/komega"
 )
 
 var _ = Describe("[sig-cluster-lifecycle][OCPFeatureGate:MachineAPIMigration] Machine Migration CAPI Authoritative Tests", Ordered, func() {
@@ -298,4 +300,152 @@ var _ = Describe("[sig-cluster-lifecycle][OCPFeatureGate:MachineAPIMigration] Ma
 				})
 			})*/
 	})
+
+	var _ = Describe("Update CAPI Machine", Ordered, func() {
+		var capiUpdateTestName = "machine-update-capi-test"
+		var newMapiMachine *mapiv1beta1.Machine
+		var newCapiMachine *clusterv1.Machine
+		var previousState *MachineState
+		Context("with spec.authoritativeAPI: ClusterAPI (both MAPI and CAPI machine exist)", func() {
+			BeforeAll(func() {
+				newMapiMachine = createMAPIMachineWithAuthority(ctx, cl, capiUpdateTestName, mapiv1beta1.MachineAuthorityClusterAPI)
+				newCapiMachine = capiframework.GetMachine(cl, newMapiMachine.Name, capiframework.CAPINamespace)
+				verifyMachineRunning(cl, newCapiMachine)
+
+				DeferCleanup(func() {
+					By("Cleaning up machine resources")
+					cleanupMachineResources(
+						ctx,
+						cl,
+						[]*clusterv1.Machine{newCapiMachine},
+						[]*mapiv1beta1.Machine{newMapiMachine},
+					)
+				})
+			})
+
+			It("should add labels/annotations on CAPI machine", func() {
+				// Capture state before the update
+				previousState = captureMachineStateBeforeUpdate(cl, newMapiMachine)
+
+				Eventually(komega.Update(newCapiMachine, func() {
+					if newCapiMachine.Labels == nil {
+						newCapiMachine.Labels = make(map[string]string)
+					}
+					if newCapiMachine.Annotations == nil {
+						newCapiMachine.Annotations = make(map[string]string)
+					}
+					newCapiMachine.Labels["test-capi-label"] = "test-capi-label-value"
+					newCapiMachine.Annotations["test-capi-annotation"] = "test-capi-annotation-value"
+				}), capiframework.WaitMedium, capiframework.RetryMedium).Should(Succeed(), "Failed to add CAPI Machine labels/annotations")
+			})
+
+			It("should synchronize added labels/annotations to both metadata and spec on MAPI", func() {
+				Eventually(komega.Object(newMapiMachine), capiframework.WaitMedium, capiframework.RetryMedium).Should(
+					SatisfyAll(
+						// Check metadata
+						WithTransform(func(m *mapiv1beta1.Machine) string {
+							return m.Labels["test-capi-label"]
+						}, Equal("test-capi-label-value")),
+						WithTransform(func(m *mapiv1beta1.Machine) string {
+							return m.Annotations["test-capi-annotation"]
+						}, Equal("test-capi-annotation-value")),
+						// Check spec template metadata
+						WithTransform(func(m *mapiv1beta1.Machine) string {
+							return m.Spec.ObjectMeta.Labels["test-capi-label"]
+						}, Equal("test-capi-label-value")),
+						WithTransform(func(m *mapiv1beta1.Machine) string {
+							return m.Spec.ObjectMeta.Annotations["test-capi-annotation"]
+						}, Equal("test-capi-annotation-value")),
+					),
+					"Added labels should be copied to both metadata and spec on MAPI Machine",
+				)
+			})
+
+			It("should verify MAPI generation changed after syncing labels to spec", func() {
+				Eventually(komega.Get(newMapiMachine), capiframework.WaitMedium, capiframework.RetryMedium).Should(Succeed(), "Failed to get updated MAPI Machine")
+
+				verifyMachineStateChanged(cl, newMapiMachine, previousState, mapiv1beta1.MachineAuthorityClusterAPI, "adding labels/annotations")
+			})
+
+			It("should modify existing labels/annotations on CAPI machine", func() {
+				// Capture state before the update
+				previousState = captureMachineStateBeforeUpdate(cl, newMapiMachine)
+
+				Eventually(komega.Update(newCapiMachine, func() {
+					newCapiMachine.Labels["test-capi-label"] = "modified-capi-label-value"
+					newCapiMachine.Annotations["test-capi-annotation"] = "modified-capi-annotation-value"
+				}), capiframework.WaitMedium, capiframework.RetryMedium).Should(Succeed(), "Failed to modify CAPI Machine labels/annotations")
+			})
+
+			It("should synchronize modified labels/annotations to both metadata and spec on MAPI", func() {
+				Eventually(komega.Object(newMapiMachine), capiframework.WaitMedium, capiframework.RetryMedium).Should(
+					SatisfyAll(
+						// Check metadata
+						WithTransform(func(m *mapiv1beta1.Machine) string {
+							return m.Labels["test-capi-label"]
+						}, Equal("modified-capi-label-value")),
+						WithTransform(func(m *mapiv1beta1.Machine) string {
+							return m.Annotations["test-capi-annotation"]
+						}, Equal("modified-capi-annotation-value")),
+						// Check spec template metadata
+						WithTransform(func(m *mapiv1beta1.Machine) string {
+							return m.Spec.ObjectMeta.Labels["test-capi-label"]
+						}, Equal("modified-capi-label-value")),
+						WithTransform(func(m *mapiv1beta1.Machine) string {
+							return m.Spec.ObjectMeta.Annotations["test-capi-annotation"]
+						}, Equal("modified-capi-annotation-value")),
+					),
+					"Modified labels/annotations should be synchronized to both metadata and spec on MAPI Machine",
+				)
+			})
+
+			It("should verify MAPI generation changed after syncing modified labels to spec", func() {
+				Eventually(komega.Get(newMapiMachine), capiframework.WaitMedium, capiframework.RetryMedium).Should(Succeed(), "Failed to get updated MAPI Machine")
+
+				verifyMachineStateChanged(cl, newMapiMachine, previousState, mapiv1beta1.MachineAuthorityClusterAPI, "modifying labels/annotations")
+			})
+
+			It("should delete labels/annotations on CAPI machine", func() {
+				// Capture state before the update
+				previousState = captureMachineStateBeforeUpdate(cl, newMapiMachine)
+
+				Eventually(komega.Update(newCapiMachine, func() {
+					delete(newCapiMachine.Labels, "test-capi-label")
+					delete(newCapiMachine.Annotations, "test-capi-annotation")
+				}), capiframework.WaitMedium, capiframework.RetryMedium).Should(Succeed(), "Failed to delete CAPI Machine labels/annotations")
+			})
+
+			It("should synchronize label/annotation deletion to both metadata and spec on MAPI", func() {
+				Eventually(komega.Object(newMapiMachine), capiframework.WaitMedium, capiframework.RetryMedium).Should(
+					SatisfyAll(
+						// Check metadata deletion
+						WithTransform(func(m *mapiv1beta1.Machine) bool {
+							_, exists := m.Labels["test-capi-label"]
+							return exists
+						}, BeFalse()),
+						WithTransform(func(m *mapiv1beta1.Machine) bool {
+							_, exists := m.Annotations["test-capi-annotation"]
+							return exists
+						}, BeFalse()),
+						// Verify deleted labels are also removed from spec
+						WithTransform(func(m *mapiv1beta1.Machine) bool {
+							_, exists := m.Spec.ObjectMeta.Labels["test-capi-label"]
+							return exists
+						}, BeFalse()),
+						WithTransform(func(m *mapiv1beta1.Machine) bool {
+							_, exists := m.Spec.ObjectMeta.Annotations["test-capi-annotation"]
+							return exists
+						}, BeFalse()),
+					),
+					"Deleted labels/annotations should be synchronized to both metadata and spec on MAPI Machine",
+				)
+			})
+
+			It("should verify MAPI generation changed after syncing deleted labels to spec", func() {
+				Eventually(komega.Get(newMapiMachine), capiframework.WaitMedium, capiframework.RetryMedium).Should(Succeed(), "Failed to get updated MAPI Machine")
+
+				verifyMachineStateChanged(cl, newMapiMachine, previousState, mapiv1beta1.MachineAuthorityClusterAPI, "deleting labels/annotations")
+			})
+		})
+	})
 })

e2e/machine_migration_helpers.go

@@ -333,3 +333,65 @@ func verifyMachineSynchronizedGeneration(cl client.Client, mapiMachine *mapiv1be
 		fmt.Sprintf("MAPI Machine SynchronizedGeneration should equal %s Machine Generation (%d)", authoritativeMachineType, expectedGeneration),
 	)
 }
+
+// MachineState holds the state of both MAPI and CAPI Machines before an update operation.
+type MachineState struct {
+	MAPIGeneration int64
+	CAPIGeneration int64
+}
+
+// captureMachineStateBeforeUpdate captures the current state (generation) of both MAPI and CAPI Machines
+// before an update operation. The captured state is later used by verifyMachineStateChanged or verifyMachineStateUnchanged
+// to verify whether the state has changed or remained the same after the operation.
+func captureMachineStateBeforeUpdate(cl client.Client, currentMAPIMachine *mapiv1beta1.Machine) *MachineState {
+	Eventually(komega.Get(currentMAPIMachine), capiframework.WaitMedium, capiframework.RetryMedium).Should(Succeed(), "Failed to get MAPI Machine before capturing state")
+
+	currentCAPIMachine := capiframework.GetMachine(cl, currentMAPIMachine.Name, capiframework.CAPINamespace)
+
+	return &MachineState{
+		MAPIGeneration: currentMAPIMachine.Generation,
+		CAPIGeneration: currentCAPIMachine.Generation,
+	}
+}
+
+// verifyMachineStateChanged verifies that:
+// 1. CAPI Machine generation remains unchanged (metadata-only changes don't bump spec generation)
+// 2. MAPI Machine generation has changed (spec.objectMeta was updated)
+// 3. Synchronized condition and generation are correct
+// This is used for CAPI authoritative scenarios where CAPI metadata changes are synced to MAPI spec.
+// Note: SynchronizedGeneration equals the authoritative machine's generation, so for metadata-only
+// changes it will not increase (since CAPI Machine generation doesn't change).
+func verifyMachineStateChanged(cl client.Client, mapiMachine *mapiv1beta1.Machine, previousState *MachineState, authority mapiv1beta1.MachineAuthority, operation string) {
+	// First verify CAPI Machine generation remains unchanged
+	currentCAPIMachine := capiframework.GetMachine(cl, mapiMachine.Name, capiframework.CAPINamespace)
+	By(fmt.Sprintf("Verifying CAPI Machine generation unchanged after %s (previous: %d, current: %d)", operation, previousState.CAPIGeneration, currentCAPIMachine.Generation))
+	Expect(currentCAPIMachine.Generation).To(Equal(previousState.CAPIGeneration), "CAPI Machine generation should not change for metadata-only updates")
+
+	// Then verify MAPI Machine generation has changed
+	By(fmt.Sprintf("Verifying MAPI Machine generation changed after %s (previous: %d, current: %d)", operation, previousState.MAPIGeneration, mapiMachine.Generation))
+	Expect(mapiMachine.Generation).To(BeNumerically(">", previousState.MAPIGeneration), "MAPI Machine generation should increment when spec.objectMeta is updated")
+
+	// Verify status remains correct (SynchronizedGeneration should equal authoritative machine's generation)
+	verifyMAPIMachineSynchronizedCondition(mapiMachine, authority)
+	verifyMachineSynchronizedGeneration(cl, mapiMachine, authority)
+}
+
+// verifyMachineStateUnchanged verifies that:
+// 1. CAPI Machine generation remains unchanged
+// 2. MAPI Machine generation remains unchanged
+// 3. Synchronized condition and generation are correct
+// This is used for MAPI authoritative scenarios where MAPI metadata-only changes should not trigger any updates.
+func verifyMachineStateUnchanged(cl client.Client, mapiMachine *mapiv1beta1.Machine, previousState *MachineState, authority mapiv1beta1.MachineAuthority, operation string) {
+	// Verify CAPI Machine generation remains unchanged
+	currentCAPIMachine := capiframework.GetMachine(cl, mapiMachine.Name, capiframework.CAPINamespace)
+	By(fmt.Sprintf("Verifying CAPI Machine generation unchanged after %s (previous: %d, current: %d)", operation, previousState.CAPIGeneration, currentCAPIMachine.Generation))
+	Expect(currentCAPIMachine.Generation).To(Equal(previousState.CAPIGeneration), "CAPI Machine generation should not change for metadata-only updates")
+
+	// Verify MAPI Machine generation remains unchanged
+	By(fmt.Sprintf("Verifying MAPI Machine generation unchanged after %s (previous: %d, current: %d)", operation, previousState.MAPIGeneration, mapiMachine.Generation))
+	Expect(mapiMachine.Generation).To(Equal(previousState.MAPIGeneration), "MAPI Machine generation should not change for metadata-only updates")
+
+	// Verify status remains correct (SynchronizedGeneration should equal authoritative machine's generation)
+	verifyMAPIMachineSynchronizedCondition(mapiMachine, authority)
+	verifyMachineSynchronizedGeneration(cl, mapiMachine, authority)
+}

e2e/machine_migration_mapi_authoritative_test.go

@@ -4,13 +4,15 @@ import (
 	"fmt"
 
 	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
 	configv1 "github.com/openshift/api/config/v1"
 	mapiv1beta1 "github.com/openshift/api/machine/v1beta1"
 	mapiframework "github.com/openshift/cluster-api-actuator-pkg/pkg/framework"
 	capiframework "github.com/openshift/cluster-capi-operator/e2e/framework"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	awsv1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
 	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
+	"sigs.k8s.io/controller-runtime/pkg/envtest/komega"
 )
 
 var _ = Describe("[sig-cluster-lifecycle][OCPFeatureGate:MachineAPIMigration] Machine Migration MAPI Authoritative Tests", Ordered, func() {
@@ -202,4 +204,128 @@ var _ = Describe("[sig-cluster-lifecycle][OCPFeatureGate:MachineAPIMigration] Ma
 			})
 		})
 	})
+
+	var _ = Describe("Update MAPI Machine", Ordered, func() {
+		var mapiUpdateTestName = "machine-update-mapi-test"
+		var newMapiMachine *mapiv1beta1.Machine
+		var newCapiMachine *clusterv1.Machine
+		var previousState *MachineState
+		Context("with spec.authoritativeAPI: MachineAPI (both MAPI and CAPI machine exist)", func() {
+			BeforeAll(func() {
+				newMapiMachine = createMAPIMachineWithAuthority(ctx, cl, mapiUpdateTestName, mapiv1beta1.MachineAuthorityMachineAPI)
+				verifyMachineRunning(cl, newMapiMachine)
+				newCapiMachine = capiframework.GetMachine(cl, newMapiMachine.Name, capiframework.CAPINamespace)
+
+				DeferCleanup(func() {
+					By("Cleaning up machine resources")
+					cleanupMachineResources(
+						ctx,
+						cl,
+						[]*clusterv1.Machine{newCapiMachine},
+						[]*mapiv1beta1.Machine{newMapiMachine},
+					)
+				})
+			})
+
+			It("should add labels/annotations on MAPI machine", func() {
+				// Capture state before the update
+				previousState = captureMachineStateBeforeUpdate(cl, newMapiMachine)
+
+				Eventually(komega.Update(newMapiMachine, func() {
+					if newMapiMachine.Labels == nil {
+						newMapiMachine.Labels = make(map[string]string)
+					}
+					if newMapiMachine.Annotations == nil {
+						newMapiMachine.Annotations = make(map[string]string)
+					}
+					newMapiMachine.Labels["test-mapi-label"] = "test-mapi-label-value"
+					newMapiMachine.Annotations["test-mapi-annotation"] = "test-mapi-annotation-value"
+				}), capiframework.WaitMedium, capiframework.RetryMedium).Should(Succeed(), "Failed to add MAPI Machine labels/annotations")
+			})
+
+			It("should synchronize added labels/annotations to CAPI", func() {
+				Eventually(komega.Object(newCapiMachine), capiframework.WaitMedium, capiframework.RetryMedium).Should(
+					SatisfyAll(
+						WithTransform(func(m *clusterv1.Machine) string {
+							return m.Labels["test-mapi-label"]
+						}, Equal("test-mapi-label-value")),
+						WithTransform(func(m *clusterv1.Machine) string {
+							return m.Annotations["test-mapi-annotation"]
+						}, Equal("test-mapi-annotation-value")),
+					),
+					"Added labels/annotations should be synchronized to CAPI Machine",
+				)
+			})
+
+			It("should not update synchronized conditions and generation after adding labels/annotations", func() {
+				Eventually(komega.Get(newMapiMachine), capiframework.WaitMedium, capiframework.RetryMedium).Should(Succeed(), "Failed to get updated MAPI Machine")
+
+				verifyMachineStateUnchanged(cl, newMapiMachine, previousState, mapiv1beta1.MachineAuthorityMachineAPI, "adding labels/annotations")
+			})
+
+			It("should modify existing labels/annotations on MAPI machine", func() {
+				// Capture state before the update
+				previousState = captureMachineStateBeforeUpdate(cl, newMapiMachine)
+
+				Eventually(komega.Update(newMapiMachine, func() {
+					newMapiMachine.Labels["test-mapi-label"] = "modified-mapi-label-value"
+					newMapiMachine.Annotations["test-mapi-annotation"] = "modified-mapi-annotation-value"
+				}), capiframework.WaitMedium, capiframework.RetryMedium).Should(Succeed(), "Failed to modify MAPI Machine labels/annotations")
+			})
+
+			It("should synchronize modified labels/annotations to CAPI", func() {
+				Eventually(komega.Object(newCapiMachine), capiframework.WaitMedium, capiframework.RetryMedium).Should(
+					SatisfyAll(
+						WithTransform(func(m *clusterv1.Machine) string {
+							return m.Labels["test-mapi-label"]
+						}, Equal("modified-mapi-label-value")),
+						WithTransform(func(m *clusterv1.Machine) string {
+							return m.Annotations["test-mapi-annotation"]
+						}, Equal("modified-mapi-annotation-value")),
+					),
+					"Modified labels/annotations should be synchronized to CAPI Machine",
+				)
+			})
+
+			It("should not update synchronized conditions and generation after modifying labels/annotations", func() {
+				Eventually(komega.Get(newMapiMachine), capiframework.WaitMedium, capiframework.RetryMedium).Should(Succeed(), "Failed to get updated MAPI Machine")
+
+				verifyMachineStateUnchanged(cl, newMapiMachine, previousState, mapiv1beta1.MachineAuthorityMachineAPI, "modifying labels/annotations")
+			})
+
+			It("should delete labels/annotations on MAPI machine", func() {
+				// Capture state before the update
+				previousState = captureMachineStateBeforeUpdate(cl, newMapiMachine)
+
+				Eventually(komega.Update(newMapiMachine, func() {
+					delete(newMapiMachine.Labels, "test-mapi-label")
+					delete(newMapiMachine.Annotations, "test-mapi-annotation")
+				}), capiframework.WaitMedium, capiframework.RetryMedium).Should(Succeed(), "Failed to delete MAPI Machine labels/annotations")
+			})
+
+			// It didn't sync to CAPI for deleting labels/annotations
+			// https://issues.redhat.com/browse/OCPBUGS-61596
+			PIt("should synchronize label/annotation deletion to CAPI", func() {
+				Eventually(komega.Object(newCapiMachine), capiframework.WaitMedium, capiframework.RetryMedium).Should(
+					SatisfyAll(
+						WithTransform(func(m *clusterv1.Machine) bool {
+							_, exists := m.Labels["test-mapi-label"]
+							return exists
+						}, BeFalse()),
+						WithTransform(func(m *clusterv1.Machine) bool {
+							_, exists := m.Annotations["test-mapi-annotation"]
+							return exists
+						}, BeFalse()),
+					),
+					"Deleted labels/annotations should be synchronized to CAPI Machine",
+				)
+			})
+
+			It("should not update synchronized conditions and generation after deleting labels/annotations", func() {
+				Eventually(komega.Get(newMapiMachine), capiframework.WaitMedium, capiframework.RetryMedium).Should(Succeed(), "Failed to get updated MAPI Machine")
+
+				verifyMachineStateUnchanged(cl, newMapiMachine, previousState, mapiv1beta1.MachineAuthorityMachineAPI, "deleting labels/annotations")
+			})
+		})
+	})
 })