diff --git a/e2e/machine_migration_capi_authoritative_test.go b/e2e/machine_migration_capi_authoritative_test.go index 072291d4c..652e90a1c 100644 --- a/e2e/machine_migration_capi_authoritative_test.go +++ b/e2e/machine_migration_capi_authoritative_test.go @@ -2,8 +2,10 @@ package e2e import ( "fmt" + "time" . "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 +14,7 @@ import ( awsv1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/envtest/komega" ) var _ = Describe("[sig-cluster-lifecycle][OCPFeatureGate:MachineAPIMigration] Machine Migration CAPI Authoritative Tests", Ordered, func() { @@ -298,4 +301,161 @@ 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) + + // Add a small delay to ensure timestamps are distinguishable + time.Sleep(1 * time.Second) + + 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 and synchronized time 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) + + // Add a small delay to ensure timestamps are distinguishable + time.Sleep(1 * time.Second) + + 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 and synchronized time 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) + + // Add a small delay to ensure timestamps are distinguishable + time.Sleep(1 * time.Second) + + 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 and synchronized time 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") + }) + }) + }) }) diff --git a/e2e/machine_migration_helpers.go b/e2e/machine_migration_helpers.go index cb1b78ce2..ed07f6312 100644 --- a/e2e/machine_migration_helpers.go +++ b/e2e/machine_migration_helpers.go @@ -324,3 +324,100 @@ 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 + SynchronizedTime *metav1.Time +} + +// captureMachineStateBeforeUpdate captures the current state (generation and synchronized time) 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) + + state := &MachineState{ + MAPIGeneration: currentMAPIMachine.Generation, + CAPIGeneration: currentCAPIMachine.Generation, + } + + // Find and store the synchronized condition's last transition time + for _, condition := range currentMAPIMachine.Status.Conditions { + if condition.Type == SynchronizedCondition { + state.SynchronizedTime = &condition.LastTransitionTime + break + } + } + + return state +} + +// 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 time has changed (indicating a new synchronization occurred) +// This is used for CAPI authoritative scenarios where CAPI metadata changes are synced to MAPI spec. +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 synchronized time is updated + var currentSynchronizedTime *metav1.Time + for _, condition := range mapiMachine.Status.Conditions { + if condition.Type == SynchronizedCondition { + currentSynchronizedTime = &condition.LastTransitionTime + break + } + } + Expect(currentSynchronizedTime).ToNot(BeNil(), "Synchronized condition should exist") + By(fmt.Sprintf("Verifying synchronized time changed after %s (previous: %v, current: %v)", operation, previousState.SynchronizedTime.Time, currentSynchronizedTime.Time)) + Expect(currentSynchronizedTime.Time).To(BeTemporally(">", previousState.SynchronizedTime.Time), fmt.Sprintf("Synchronized time should change when syncing %s metadata to spec", operation)) + + // Verify status remains correct + verifyMAPIMachineSynchronizedCondition(mapiMachine, authority) + verifyMachineSynchronizedGeneration(cl, mapiMachine, authority) +} + +// verifyMachineStateUnchanged verifies that: +// 1. CAPI Machine generation remains unchanged +// 2. MAPI Machine generation remains unchanged +// 3. Synchronized time remains unchanged +// 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 synchronized time remains unchanged + if previousState.SynchronizedTime != nil { + var currentSynchronizedTime *metav1.Time + for _, condition := range mapiMachine.Status.Conditions { + if condition.Type == SynchronizedCondition { + currentSynchronizedTime = &condition.LastTransitionTime + break + } + } + Expect(currentSynchronizedTime).ToNot(BeNil(), "Synchronized condition should exist") + By(fmt.Sprintf("Verifying synchronized time unchanged after %s (previous: %v, current: %v)", operation, previousState.SynchronizedTime.Time, currentSynchronizedTime.Time)) + Expect(currentSynchronizedTime.Time).To(Equal(previousState.SynchronizedTime.Time), "Synchronized time should not change for metadata-only updates") + } + + // Verify status remains correct + verifyMAPIMachineSynchronizedCondition(mapiMachine, authority) + verifyMachineSynchronizedGeneration(cl, mapiMachine, authority) +} diff --git a/e2e/machine_migration_mapi_authoritative_test.go b/e2e/machine_migration_mapi_authoritative_test.go index 2656e9e09..5b8a7d41f 100644 --- a/e2e/machine_migration_mapi_authoritative_test.go +++ b/e2e/machine_migration_mapi_authoritative_test.go @@ -2,8 +2,10 @@ package e2e import ( "fmt" + "time" . "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" @@ -11,6 +13,7 @@ import ( 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/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/envtest/komega" ) var _ = Describe("[sig-cluster-lifecycle][OCPFeatureGate:MachineAPIMigration] Machine Migration MAPI Authoritative Tests", Ordered, func() { @@ -202,4 +205,137 @@ 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) + + // Add a small delay to ensure timestamps are distinguishable + time.Sleep(1 * time.Second) + + 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) + + // Add a small delay to ensure timestamps are distinguishable + time.Sleep(1 * time.Second) + + 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) + + // Add a small delay to ensure timestamps are distinguishable + time.Sleep(1 * time.Second) + + 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") + }) + }) + }) })