From a606483e642cf142c82939aa2dfda85a9da9fa25 Mon Sep 17 00:00:00 2001 From: Zhaohua Sun Date: Mon, 1 Dec 2025 16:22:08 +0800 Subject: [PATCH] add e2e tests to verify ValidatingAdmissionPolicy --- e2e/machine_migration_helpers.go | 39 +++++++++++++++++++ ...chine_migration_mapi_authoritative_test.go | 20 ++++++++++ ...neset_migration_capi_authoritative_test.go | 3 +- e2e/machineset_migration_helpers.go | 16 ++++++++ ...neset_migration_mapi_authoritative_test.go | 6 +-- 5 files changed, 78 insertions(+), 6 deletions(-) diff --git a/e2e/machine_migration_helpers.go b/e2e/machine_migration_helpers.go index cb1b78ce2..6c9224d25 100644 --- a/e2e/machine_migration_helpers.go +++ b/e2e/machine_migration_helpers.go @@ -128,6 +128,45 @@ func createMAPIMachineWithAuthority(ctx context.Context, cl client.Client, machi return newMachine } +// createSameNameMachineBlockedByVAPAuthMapi attempts to create a MAPI Machine with specified authoritativeAPI and expects the creation to fail. +func createSameNameMachineBlockedByVAPAuthMapi(ctx context.Context, cl client.Client, machineName string, authority mapiv1beta1.MachineAuthority, expectedErrorMessage string) { + Expect(machineName).NotTo(BeEmpty(), "Machine name cannot be empty") + workerLabelSelector := metav1.LabelSelector{ + MatchLabels: map[string]string{ + "machine.openshift.io/cluster-api-machine-role": "worker", + }, + } + machineList, err := mapiframework.GetMachines(ctx, cl, &workerLabelSelector) + + Expect(err).NotTo(HaveOccurred(), "Should have successfully listed MAPI machines") + Expect(machineList).NotTo(BeEmpty(), "Should have found MAPI machines in the openshift-machine-api namespace to use as a reference for creating a new one") + + referenceMachine := machineList[0] + By(fmt.Sprintf("Using MAPI machine %s as a reference", referenceMachine.Name)) + + newMachine := &mapiv1beta1.Machine{ + TypeMeta: metav1.TypeMeta{ + Kind: "Machine", + APIVersion: mapiv1beta1.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: machineName, + Namespace: referenceMachine.Namespace, + }, + Spec: *referenceMachine.Spec.DeepCopy(), + } + + newMachine.Spec.ProviderID = nil + newMachine.ObjectMeta.Labels = nil + newMachine.Status = mapiv1beta1.MachineStatus{} + newMachine.Spec.AuthoritativeAPI = authority + + By(fmt.Sprintf("Attempting to create a MAPI machine (expecting failure) with AuthoritativeAPI: %s in namespace: %s", authority, newMachine.Namespace)) + err = cl.Create(ctx, newMachine) + Expect(err).To(HaveOccurred(), "MAPI Machine %s creation should fail", machineName) + Expect(err.Error()).To(ContainSubstring(expectedErrorMessage), "Error message should contain expected text: %s", expectedErrorMessage) +} + // verifyMachineRunning verifies that a machine reaches Running state using the machine object directly. func verifyMachineRunning(cl client.Client, machine client.Object) { Expect(machine).NotTo(BeNil(), "Machine parameter cannot be nil") diff --git a/e2e/machine_migration_mapi_authoritative_test.go b/e2e/machine_migration_mapi_authoritative_test.go index 2656e9e09..f32a85622 100644 --- a/e2e/machine_migration_mapi_authoritative_test.go +++ b/e2e/machine_migration_mapi_authoritative_test.go @@ -29,6 +29,26 @@ var _ = Describe("[sig-cluster-lifecycle][OCPFeatureGate:MachineAPIMigration] Ma var newCapiMachine *clusterv1.Machine var newMapiMachine *mapiv1beta1.Machine + Context("With spec.authoritativeAPI: MachineAPI and existing CAPI Machine with that name", func() { + BeforeAll(func() { + newCapiMachine = createCAPIMachine(ctx, cl, mapiMachineAuthMAPIName) + + DeferCleanup(func() { + By("Cleaning up machine resources") + cleanupMachineResources( + ctx, + cl, + []*clusterv1.Machine{newCapiMachine}, + []*mapiv1beta1.Machine{}, + ) + }) + }) + + It("should reject creation of MAPI Machine with same name as existing CAPI Machine", func() { + createSameNameMachineBlockedByVAPAuthMapi(ctx, cl, mapiMachineAuthMAPIName, mapiv1beta1.MachineAuthorityMachineAPI, "a Cluster API Machine with the same name already exists") + }) + }) + Context("With spec.authoritativeAPI: MachineAPI and no existing CAPI Machine with that name", func() { BeforeAll(func() { newMapiMachine = createMAPIMachineWithAuthority(ctx, cl, mapiMachineAuthMAPIName, mapiv1beta1.MachineAuthorityMachineAPI) diff --git a/e2e/machineset_migration_capi_authoritative_test.go b/e2e/machineset_migration_capi_authoritative_test.go index cc4599f5d..6ca6f32cb 100644 --- a/e2e/machineset_migration_capi_authoritative_test.go +++ b/e2e/machineset_migration_capi_authoritative_test.go @@ -58,8 +58,7 @@ var _ = Describe("[sig-cluster-lifecycle][OCPFeatureGate:MachineAPIMigration] Ma verifyMachineSetPausedCondition(mapiMachineSet, mapiv1beta1.MachineAuthorityClusterAPI) }) - // bug https://issues.redhat.com/browse/OCPBUGS-55337 - PIt("should verify that the non-authoritative MAPI MachineSet providerSpec has been updated to reflect the authoritative CAPI MachineSet mirror values", func() { + It("should verify that the non-authoritative MAPI MachineSet providerSpec has been updated to reflect the authoritative CAPI MachineSet mirror values", func() { verifyMAPIMachineSetProviderSpec(mapiMachineSet, HaveField("InstanceType", Equal(instanceType))) }) }) diff --git a/e2e/machineset_migration_helpers.go b/e2e/machineset_migration_helpers.go index c1a92ce80..65830c988 100644 --- a/e2e/machineset_migration_helpers.go +++ b/e2e/machineset_migration_helpers.go @@ -82,6 +82,22 @@ func createMAPIMachineSetWithAuthoritativeAPI(ctx context.Context, cl client.Cli return mapiMachineSet } +// createSameNameMachineSetBlockedByVAPAuthMapi attempts to create a MAPI MachineSet with specified authoritativeAPI and expects the creation to fail. +func createSameNameMachineSetBlockedByVAPAuthMapi(ctx context.Context, cl client.Client, replicas int, machineSetName string, machineSetAuthority mapiv1beta1.MachineAuthority, machineAuthority mapiv1beta1.MachineAuthority, expectedErrorMessage string) { + By(fmt.Sprintf("Attempting to create MAPI MachineSet (expecting failure) with spec.authoritativeAPI: %s, spec.template.spec.authoritativeAPI: %s, replicas=%d", machineSetAuthority, machineAuthority, replicas)) + machineSetParams := mapiframework.BuildMachineSetParams(ctx, cl, replicas) + machineSetParams.Name = machineSetName + machineSetParams.Labels[mapiframework.MachineSetKey] = machineSetName + machineSetParams.MachinesetAuthoritativeAPI = machineSetAuthority + machineSetParams.MachineAuthoritativeAPI = machineAuthority + // Remove taints as CAPI MachineSets don't support them yet. This is a known limitation tracked in https://issues.redhat.com/browse/OCPCLOUD-2861 + machineSetParams.Taints = []corev1.Taint{} + + _, err := mapiframework.CreateMachineSet(cl, machineSetParams) + Expect(err).To(HaveOccurred(), "MAPI MachineSet %s creation should fail", machineSetName) + Expect(err.Error()).To(ContainSubstring(expectedErrorMessage), "Error message should contain expected text: %s", expectedErrorMessage) +} + // switchMachineSetAuthoritativeAPI updates the authoritativeAPI fields of a MAPI MachineSet and its template. func switchMachineSetAuthoritativeAPI(mapiMachineSet *mapiv1beta1.MachineSet, machineSetAuthority mapiv1beta1.MachineAuthority, machineAuthority mapiv1beta1.MachineAuthority) { By(fmt.Sprintf("Switching MachineSet %s AuthoritativeAPI to spec.authoritativeAPI: %s, spec.template.spec.authoritativeAPI: %s", mapiMachineSet.Name, machineSetAuthority, machineAuthority)) diff --git a/e2e/machineset_migration_mapi_authoritative_test.go b/e2e/machineset_migration_mapi_authoritative_test.go index 207af04c8..f979497bc 100644 --- a/e2e/machineset_migration_mapi_authoritative_test.go +++ b/e2e/machineset_migration_mapi_authoritative_test.go @@ -55,10 +55,8 @@ var _ = Describe("[sig-cluster-lifecycle][OCPFeatureGate:MachineAPIMigration] Ma }) }) - // https://issues.redhat.com/browse/OCPCLOUD-3188 - PIt("should reject creation of MAPI MachineSet with same name as existing CAPI MachineSet", func() { - By("Creating a same name MAPI MachineSet") - createMAPIMachineSetWithAuthoritativeAPI(ctx, cl, 0, existingCAPIMSAuthorityMAPIName, mapiv1beta1.MachineAuthorityMachineAPI, mapiv1beta1.MachineAuthorityMachineAPI) + It("should reject creation of MAPI MachineSet with same name as existing CAPI MachineSet", func() { + createSameNameMachineSetBlockedByVAPAuthMapi(ctx, cl, 0, existingCAPIMSAuthorityMAPIName, mapiv1beta1.MachineAuthorityMachineAPI, mapiv1beta1.MachineAuthorityMachineAPI, "a Cluster API MachineSet with the same name already exists") }) })