diff --git a/test/e2e/anti_affinity_test.go b/test/e2e/anti_affinity_test.go index ec0c515d1d..ac78fe809b 100644 --- a/test/e2e/anti_affinity_test.go +++ b/test/e2e/anti_affinity_test.go @@ -50,15 +50,13 @@ var _ = Describe("Cluster creation with anti affined nodes", func() { var ( testSpecificClusterctlConfigPath string - testSpecificIPAMCleanupFunc func() + testSpecificIPAddressClaims ipam.IPAddressClaims ) BeforeEach(func() { - testSpecificClusterctlConfigPath, testSpecificIPAMCleanupFunc = ipam.ConfigureControlPlaneIPs(ctx, e2eIPAMKubeconfig, clusterctlConfigPath, skipCleanup) + testSpecificClusterctlConfigPath, testSpecificIPAddressClaims = ipamHelper.ClaimIPs(ctx, clusterctlConfigPath) }) defer AfterEach(func() { - if testSpecificIPAMCleanupFunc != nil { - testSpecificIPAMCleanupFunc() - } + Expect(ipamHelper.Cleanup(ctx, testSpecificIPAddressClaims)).To(Succeed()) }) BeforeEach(func() { diff --git a/test/e2e/capi_machine_deployment_rollout_test.go b/test/e2e/capi_machine_deployment_rollout_test.go index 60d32bad5b..d9bcdb6bd0 100644 --- a/test/e2e/capi_machine_deployment_rollout_test.go +++ b/test/e2e/capi_machine_deployment_rollout_test.go @@ -18,6 +18,7 @@ package e2e import ( . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" capi_e2e "sigs.k8s.io/cluster-api/test/e2e" "sigs.k8s.io/cluster-api-provider-vsphere/test/e2e/ipam" @@ -27,15 +28,13 @@ var _ = Describe("ClusterAPI Machine Deployment Tests", func() { Context("Running the MachineDeployment rollout spec", func() { var ( testSpecificClusterctlConfigPath string - testSpecificIPAMCleanupFunc func() + testSpecificIPAddressClaims ipam.IPAddressClaims ) BeforeEach(func() { - testSpecificClusterctlConfigPath, testSpecificIPAMCleanupFunc = ipam.ConfigureControlPlaneIPs(ctx, e2eIPAMKubeconfig, clusterctlConfigPath, skipCleanup) + testSpecificClusterctlConfigPath, testSpecificIPAddressClaims = ipamHelper.ClaimIPs(ctx, clusterctlConfigPath) }) defer AfterEach(func() { - if testSpecificIPAMCleanupFunc != nil { - testSpecificIPAMCleanupFunc() - } + Expect(ipamHelper.Cleanup(ctx, testSpecificIPAddressClaims)).To(Succeed()) }) capi_e2e.MachineDeploymentRolloutSpec(ctx, func() capi_e2e.MachineDeploymentRolloutSpecInput { diff --git a/test/e2e/cluster_upgrade_test.go b/test/e2e/cluster_upgrade_test.go index 7bf51f5471..b4f9fa4731 100644 --- a/test/e2e/cluster_upgrade_test.go +++ b/test/e2e/cluster_upgrade_test.go @@ -18,6 +18,7 @@ package e2e import ( . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" "k8s.io/utils/ptr" capi_e2e "sigs.k8s.io/cluster-api/test/e2e" @@ -27,15 +28,13 @@ import ( var _ = Describe("When upgrading a workload cluster using ClusterClass [ClusterClass]", func() { var ( testSpecificClusterctlConfigPath string - testSpecificIPAMCleanupFunc func() + testSpecificIPAddressClaims ipam.IPAddressClaims ) BeforeEach(func() { - testSpecificClusterctlConfigPath, testSpecificIPAMCleanupFunc = ipam.ConfigureControlPlaneIPs(ctx, e2eIPAMKubeconfig, clusterctlConfigPath, skipCleanup) + testSpecificClusterctlConfigPath, testSpecificIPAddressClaims = ipamHelper.ClaimIPs(ctx, clusterctlConfigPath) }) defer AfterEach(func() { - if testSpecificIPAMCleanupFunc != nil { - testSpecificIPAMCleanupFunc() - } + Expect(ipamHelper.Cleanup(ctx, testSpecificIPAddressClaims)).To(Succeed()) }) capi_e2e.ClusterUpgradeConformanceSpec(ctx, func() capi_e2e.ClusterUpgradeConformanceSpecInput { diff --git a/test/e2e/clusterclass_changes_test.go b/test/e2e/clusterclass_changes_test.go index b29ffdffdf..be0bba2976 100644 --- a/test/e2e/clusterclass_changes_test.go +++ b/test/e2e/clusterclass_changes_test.go @@ -18,6 +18,7 @@ package e2e import ( . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" capie2e "sigs.k8s.io/cluster-api/test/e2e" "sigs.k8s.io/cluster-api-provider-vsphere/test/e2e/ipam" @@ -26,15 +27,13 @@ import ( var _ = Describe("When testing ClusterClass changes [ClusterClass]", func() { var ( testSpecificClusterctlConfigPath string - testSpecificIPAMCleanupFunc func() + testSpecificIPAddressClaims ipam.IPAddressClaims ) BeforeEach(func() { - testSpecificClusterctlConfigPath, testSpecificIPAMCleanupFunc = ipam.ConfigureControlPlaneIPs(ctx, e2eIPAMKubeconfig, clusterctlConfigPath, skipCleanup) + testSpecificClusterctlConfigPath, testSpecificIPAddressClaims = ipamHelper.ClaimIPs(ctx, clusterctlConfigPath) }) defer AfterEach(func() { - if testSpecificIPAMCleanupFunc != nil { - testSpecificIPAMCleanupFunc() - } + Expect(ipamHelper.Cleanup(ctx, testSpecificIPAddressClaims)).To(Succeed()) }) capie2e.ClusterClassChangesSpec(ctx, func() capie2e.ClusterClassChangesSpecInput { diff --git a/test/e2e/clusterctl_upgrade_test.go b/test/e2e/clusterctl_upgrade_test.go index c8fcf865f9..6f05b08262 100644 --- a/test/e2e/clusterctl_upgrade_test.go +++ b/test/e2e/clusterctl_upgrade_test.go @@ -18,6 +18,7 @@ package e2e import ( . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" capi_e2e "sigs.k8s.io/cluster-api/test/e2e" "sigs.k8s.io/cluster-api-provider-vsphere/test/e2e/ipam" @@ -26,15 +27,13 @@ import ( var _ = Describe("When testing clusterctl upgrades using ClusterClass (CAPV 1.8=>current, CAPI 1.5=>1.6) [ClusterClass]", func() { var ( testSpecificClusterctlConfigPath string - testSpecificIPAMCleanupFunc func() + testSpecificIPAddressClaims ipam.IPAddressClaims ) BeforeEach(func() { - testSpecificClusterctlConfigPath, testSpecificIPAMCleanupFunc = ipam.ConfigureControlPlaneIPs(ctx, e2eIPAMKubeconfig, clusterctlConfigPath, skipCleanup, "WORKLOAD_CONTROL_PLANE_ENDPOINT_IP") + testSpecificClusterctlConfigPath, testSpecificIPAddressClaims = ipamHelper.ClaimIPs(ctx, clusterctlConfigPath, "WORKLOAD_CONTROL_PLANE_ENDPOINT_IP") }) defer AfterEach(func() { - if testSpecificIPAMCleanupFunc != nil { - testSpecificIPAMCleanupFunc() - } + Expect(ipamHelper.Cleanup(ctx, testSpecificIPAddressClaims)).To(Succeed()) }) capi_e2e.ClusterctlUpgradeSpec(ctx, func() capi_e2e.ClusterctlUpgradeSpecInput { @@ -63,15 +62,13 @@ var _ = Describe("When testing clusterctl upgrades using ClusterClass (CAPV 1.8= var _ = Describe("When testing clusterctl upgrades using ClusterClass (CAPV 1.7=>current, CAPI 1.4=>1.6) [ClusterClass]", func() { var ( testSpecificClusterctlConfigPath string - testSpecificIPAMCleanupFunc func() + testSpecificIPAddressClaims ipam.IPAddressClaims ) BeforeEach(func() { - testSpecificClusterctlConfigPath, testSpecificIPAMCleanupFunc = ipam.ConfigureControlPlaneIPs(ctx, e2eIPAMKubeconfig, clusterctlConfigPath, skipCleanup, "WORKLOAD_CONTROL_PLANE_ENDPOINT_IP") + testSpecificClusterctlConfigPath, testSpecificIPAddressClaims = ipamHelper.ClaimIPs(ctx, clusterctlConfigPath, "WORKLOAD_CONTROL_PLANE_ENDPOINT_IP") }) defer AfterEach(func() { - if testSpecificIPAMCleanupFunc != nil { - testSpecificIPAMCleanupFunc() - } + Expect(ipamHelper.Cleanup(ctx, testSpecificIPAddressClaims)).To(Succeed()) }) capi_e2e.ClusterctlUpgradeSpec(ctx, func() capi_e2e.ClusterctlUpgradeSpecInput { diff --git a/test/e2e/dhcp_overrides_test.go b/test/e2e/dhcp_overrides_test.go index d65b6debe7..ed615f5ef5 100644 --- a/test/e2e/dhcp_overrides_test.go +++ b/test/e2e/dhcp_overrides_test.go @@ -55,15 +55,13 @@ var _ = Describe("DHCPOverrides configuration test", func() { When("Creating a cluster with DHCPOverrides configured", func() { var ( testSpecificClusterctlConfigPath string - testSpecificIPAMCleanupFunc func() + testSpecificIPAddressClaims ipam.IPAddressClaims ) BeforeEach(func() { - testSpecificClusterctlConfigPath, testSpecificIPAMCleanupFunc = ipam.ConfigureControlPlaneIPs(ctx, e2eIPAMKubeconfig, clusterctlConfigPath, skipCleanup) + testSpecificClusterctlConfigPath, testSpecificIPAddressClaims = ipamHelper.ClaimIPs(ctx, clusterctlConfigPath) }) defer AfterEach(func() { - if testSpecificIPAMCleanupFunc != nil { - testSpecificIPAMCleanupFunc() - } + Expect(ipamHelper.Cleanup(ctx, testSpecificIPAddressClaims)).To(Succeed()) }) const specName = "dhcp-overrides" diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index df8e3839d3..47c94f122a 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -90,6 +90,9 @@ var ( // e2eIPAMKubeconfig is a kubeconfig to a cluster which provides IP address management via an in-cluster // IPAM provider to claim IPs for the control plane IPs of created clusters. e2eIPAMKubeconfig string + + // ipamHelper is used to claim and cleanup IP addresses used for kubernetes control plane API Servers. + ipamHelper ipam.Helper ) func init() { @@ -152,23 +155,31 @@ var _ = SynchronizedBeforeSuite(func() []byte { By("Initializing the bootstrap cluster") vsphereframework.InitBootstrapCluster(ctx, bootstrapClusterProxy, e2eConfig, clusterctlConfigPath, artifactFolder) + ipamLabels := ipam.GetIPAddressClaimLabels() + var ipamLabelsRaw []string + for k, v := range ipamLabels { + ipamLabelsRaw = append(ipamLabelsRaw, fmt.Sprintf("%s=%s", k, v)) + } + return []byte( strings.Join([]string{ artifactFolder, configPath, clusterctlConfigPath, bootstrapClusterProxy.GetKubeconfigPath(), + strings.Join(ipamLabelsRaw, ";"), }, ","), ) }, func(data []byte) { // Before each ParallelNode. parts := strings.Split(string(data), ",") - Expect(parts).To(HaveLen(4)) + Expect(parts).To(HaveLen(5)) artifactFolder = parts[0] configPath = parts[1] clusterctlConfigPath = parts[2] kubeconfigPath := parts[3] + ipamLabelsRaw := parts[4] namespaces = map[*corev1.Namespace]context.CancelFunc{} @@ -178,6 +189,18 @@ var _ = SynchronizedBeforeSuite(func() []byte { e2eConfig, err = vsphereframework.LoadE2EConfig(ctx, configPath) Expect(err).NotTo(HaveOccurred()) bootstrapClusterProxy = framework.NewClusterProxy("bootstrap", kubeconfigPath, initScheme(), framework.WithMachineLogCollector(LogCollector{})) + + ipamLabels := map[string]string{} + for _, s := range strings.Split(ipamLabelsRaw, ";") { + splittedLabel := strings.Split(s, "=") + Expect(splittedLabel).To(HaveLen(2)) + + ipamLabels[splittedLabel[0]] = splittedLabel[1] + } + if e2eIPAMKubeconfig != "" { + ipamHelper, err = ipam.New(e2eIPAMKubeconfig, ipamLabels, skipCleanup) + Expect(err).ToNot(HaveOccurred()) + } }) // Using a SynchronizedAfterSuite for controlling how to delete resources shared across ParallelNodes (~ginkgo threads). @@ -191,12 +214,12 @@ var _ = SynchronizedAfterSuite(func() { By("Cleaning up orphaned IPAddressClaims") vSphereFolderName, err := getClusterctlConfigVariable(clusterctlConfigPath, "VSPHERE_FOLDER") Expect(err).ToNot(HaveOccurred()) - Expect(ipam.Teardown(ctx, e2eIPAMKubeconfig, vSphereFolderName, vsphereClient)).To(Succeed()) + Expect(ipamHelper.Teardown(ctx, vSphereFolderName, vsphereClient)).To(Succeed()) } By("Cleaning up the vSphere session", terminateVSphereSession) - By("Tearing down the management cluster") if !skipCleanup { + By("Tearing down the management cluster") vsphereframework.TearDown(ctx, bootstrapClusterProvider, bootstrapClusterProxy) } }) diff --git a/test/e2e/gpu_pci_passthrough_test.go b/test/e2e/gpu_pci_passthrough_test.go index 4cedaa962b..6e98980211 100644 --- a/test/e2e/gpu_pci_passthrough_test.go +++ b/test/e2e/gpu_pci_passthrough_test.go @@ -40,15 +40,13 @@ var _ = Describe("Cluster creation with GPU devices as PCI passthrough [speciali var ( testSpecificClusterctlConfigPath string - testSpecificIPAMCleanupFunc func() + testSpecificIPAddressClaims ipam.IPAddressClaims ) BeforeEach(func() { - testSpecificClusterctlConfigPath, testSpecificIPAMCleanupFunc = ipam.ConfigureControlPlaneIPs(ctx, e2eIPAMKubeconfig, clusterctlConfigPath, skipCleanup) + testSpecificClusterctlConfigPath, testSpecificIPAddressClaims = ipamHelper.ClaimIPs(ctx, clusterctlConfigPath) }) defer AfterEach(func() { - if testSpecificIPAMCleanupFunc != nil { - testSpecificIPAMCleanupFunc() - } + Expect(ipamHelper.Cleanup(ctx, testSpecificIPAddressClaims)).To(Succeed()) }) BeforeEach(func() { diff --git a/test/e2e/hardware_upgrade_test.go b/test/e2e/hardware_upgrade_test.go index cb5f152793..ff5baf7f66 100644 --- a/test/e2e/hardware_upgrade_test.go +++ b/test/e2e/hardware_upgrade_test.go @@ -49,15 +49,13 @@ var _ = Describe("Hardware version upgrade", func() { var ( testSpecificClusterctlConfigPath string - testSpecificIPAMCleanupFunc func() + testSpecificIPAddressClaims ipam.IPAddressClaims ) BeforeEach(func() { - testSpecificClusterctlConfigPath, testSpecificIPAMCleanupFunc = ipam.ConfigureControlPlaneIPs(ctx, e2eIPAMKubeconfig, clusterctlConfigPath, skipCleanup) + testSpecificClusterctlConfigPath, testSpecificIPAddressClaims = ipamHelper.ClaimIPs(ctx, clusterctlConfigPath) }) defer AfterEach(func() { - if testSpecificIPAMCleanupFunc != nil { - testSpecificIPAMCleanupFunc() - } + Expect(ipamHelper.Cleanup(ctx, testSpecificIPAddressClaims)).To(Succeed()) }) BeforeEach(func() { diff --git a/test/e2e/ipam/ipamhelper.go b/test/e2e/ipam/ipamhelper.go index d442e486fe..cc35d0ed60 100644 --- a/test/e2e/ipam/ipamhelper.go +++ b/test/e2e/ipam/ipamhelper.go @@ -56,26 +56,61 @@ func init() { _ = ipamv1.AddToScheme(ipamScheme) } -// ConfigureControlPlaneIPs claims one or more ip addresses if the variable e2eIPAMKubeconfig is set. -// It returns a path to a new clusterctl configuration, which contains the variable for CONTROL_PLANE_ENDPOINT_IP as well as -// for additional variables passed via additionalIPVariableNames and a cleanup function which is supposed to get called -// in an AfterEach ginkgo function. -func ConfigureControlPlaneIPs(ctx context.Context, e2eIPAMKubeconfig, clusterctlConfigPath string, skipCleanup bool, additionalIPVariableNames ...string) (string, func()) { - // If there is no e2eIPAMKubeconfig set, we skip allocation and fallback to pre-configured variables or env variables. - // The fallback is helpful for local development where we don't have an IPAM cluster, but does not allow parallelism. - if e2eIPAMKubeconfig == "" { - By("Skipping IPAM because no kubeconfig is set") - return clusterctlConfigPath, func() {} +type IPAddressClaims []*ipamv1.IPAddressClaim + +type Helper interface { + // ClaimIPs claims IP addresses with the variable name `CONTROL_PLANE_ENDPOINT_IP` and whatever is passed as + // additionalIPVariableNames and creates a new clusterctl config file. + // It returns the path to the new clusterctl config file and a slice of IPAddressClaims. + ClaimIPs(ctx context.Context, clusterctlConfigPath string, additionalIPVariableNames ...string) (localClusterctlConfigFile string, claims IPAddressClaims) + + // Cleanup deletes the given IPAddressClaims. + Cleanup(ctx context.Context, claims IPAddressClaims) error + + // Teardown tries to cleanup orphaned IPAddressClaims by checking if the corresponding IPs are still in use in vSphere. + // It identifies IPAddressClaims via labels. + Teardown(ctx context.Context, folderName string, vSphereClient *govmomi.Client) error +} + +// New returns an ipam.Helper. If e2eIPAMKubeconfig is an empty string or skipCleanup is true +// it will return a noop helper which does nothing so we cann fallback on setting environment variables. +func New(e2eIPAMKubeconfig string, labels map[string]string, skipCleanup bool) (Helper, error) { + if len(labels) == 0 { + return nil, fmt.Errorf("expecting labels to be set to prevent deletion of other IPAddressClaims") + } + + if e2eIPAMKubeconfig == "" || skipCleanup { + return &noopHelper{}, nil } + ipamClient, err := getClient(e2eIPAMKubeconfig) + if err != nil { + return nil, err + } + + return &helper{ + labels: labels, + client: ipamClient, + skipCleanup: skipCleanup, + }, nil +} + +type helper struct { + client client.Client + labels map[string]string + skipCleanup bool +} + +func (h *helper) ClaimIPs(ctx context.Context, clusterctlConfigPath string, additionalIPVariableNames ...string) (string, IPAddressClaims) { variables := map[string]string{} - ipamService, err := newIPAMService(e2eIPAMKubeconfig, skipCleanup) - Expect(err).ToNot(HaveOccurred()) + + ipAddressClaims := []*ipamv1.IPAddressClaim{} // Claim an IP per variable. for _, variable := range append(additionalIPVariableNames, controlPlaneEndpointVariable) { - ip, err := ipamService.claimIPAddress(ctx) + ip, ipAddressClaim, err := h.claimIPAddress(ctx) Expect(err).ToNot(HaveOccurred()) + ipAddressClaims = append(ipAddressClaims, ipAddressClaim) Byf("Setting clusterctl variable %s to %s", variable, ip) variables[variable] = ip } @@ -89,36 +124,67 @@ func ConfigureControlPlaneIPs(ctx context.Context, e2eIPAMKubeconfig, clusterctl Variables: variables, }) - return modifiedClusterctlConfigPath, ipamService.cleanup + return modifiedClusterctlConfigPath, ipAddressClaims } -// Teardown lists all IPAddressClaims matching the passed labels and deletes the IPAddressClaim -// if there are no VirtualMachines in vCenter using the IP address. -func Teardown(ctx context.Context, e2eIPAMKubeconfig, folderName string, vSphereClient *govmomi.Client) error { - if e2eIPAMKubeconfig == "" { - By("Skipping IPAM Teardown because no kubeconfig is set") +// Cleanup deletes the IPAddressClaims passed. +func (h *helper) Cleanup(ctx context.Context, ipAddressClaims IPAddressClaims) error { + if CurrentSpecReport().Failed() { + By("Skipping cleanup of IPAddressClaims because the tests failed and the IPs could still be in use") return nil } - ipamClient, err := getClient(e2eIPAMKubeconfig) - if err != nil { - return err + var errList []error + + for _, ipAddressClaim := range ipAddressClaims { + ipAddressClaim := ipAddressClaim + Byf("Deleting IPAddressClaim %s", klog.KObj(ipAddressClaim)) + if err := h.client.Delete(ctx, ipAddressClaim); err != nil { + errList = append(errList, err) + } + } + + if len(errList) > 0 { + return kerrors.NewAggregate(errList) } + return nil +} +// GetIPAddressClaimLabels returns a labels map from the prow environment variables +// BUILD_ID and JOB_NAME. If none of both is set it falls back to add a custom random +// label. +func GetIPAddressClaimLabels() map[string]string { + labels := map[string]string{} + if val := os.Getenv("BUILD_ID"); val != "" { + labels["prow.k8s.io/build-id"] = val + } + if val := os.Getenv("JOB_NAME"); val != "" { + labels["prow.k8s.io/job"] = val + } + if len(labels) == 0 { + // Adding a custom label so we don't accidentally cleanup other IPAddressClaims + labels["capv-testing/random-uid"] = rand.String(32) + } + return labels +} + +// Teardown lists all IPAddressClaims matching the passed labels and deletes the IPAddressClaim +// if there are no VirtualMachines in vCenter using the IP address. +func (h *helper) Teardown(ctx context.Context, folderName string, vSphereClient *govmomi.Client) error { virtualMachineIPAddresses, err := getVirtualMachineIPAddresses(ctx, folderName, vSphereClient) if err != nil { return err } // List all IPAddressClaims created matching the labels. ipAddressClaims := &ipamv1.IPAddressClaimList{} - if err := ipamClient.List(ctx, ipAddressClaims, - client.MatchingLabels(getIPAddressClaimLabels()), + if err := h.client.List(ctx, ipAddressClaims, + client.MatchingLabels(h.labels), client.InNamespace(metav1.NamespaceDefault), ); err != nil { return err } - ipAddressClaimsToDelete := []ipamv1.IPAddressClaim{} + ipAddressClaimsToDelete := []*ipamv1.IPAddressClaim{} // Collect errors and skip these ip address claims, but report at the end. var errList []error @@ -128,7 +194,8 @@ func Teardown(ctx context.Context, e2eIPAMKubeconfig, folderName string, vSphere if ipAddressClaim.Status.AddressRef.Name == "" { continue } - if err := ipamClient.Get(ctx, client.ObjectKey{Namespace: ipAddressClaim.GetNamespace(), Name: ipAddressClaim.Status.AddressRef.Name}, ip); err != nil { + if err := h.client.Get(ctx, client.ObjectKey{Namespace: ipAddressClaim.GetNamespace(), Name: ipAddressClaim.Status.AddressRef.Name}, ip); err != nil { + // If we are not able to get an IP Address we skip the deletion for it but collect and return the error. errList = append(errList, errors.Wrapf(err, "getting IPAddress for IPAddressClaim %s", klog.KObj(&ipAddressClaim))) continue } @@ -138,23 +205,34 @@ func Teardown(ctx context.Context, e2eIPAMKubeconfig, folderName string, vSphere continue } - ipAddressClaimsToDelete = append(ipAddressClaimsToDelete, ipAddressClaim) + ipAddressClaimsToDelete = append(ipAddressClaimsToDelete, &ipAddressClaim) } - for _, ipAddressClaim := range ipAddressClaimsToDelete { - ipAddressClaim := ipAddressClaim - Byf("Deleting IP Addressclaim %s/%s", ipAddressClaim.GetNamespace(), ipAddressClaim.GetName()) - if err := ipamClient.Delete(ctx, &ipAddressClaim); err != nil { - errList = append(errList, errors.Wrapf(err, "deleting IPAddressClaim %s", klog.KObj(&ipAddressClaim))) - } + if err := h.Cleanup(ctx, ipAddressClaimsToDelete); err != nil { + // Group with possible previous errors. + errList = append(errList, err) } - if len(errList) > 0 { + if len(errList) > 1 { return kerrors.NewAggregate(errList) } return nil } +func getClient(e2eIPAMKubeconfig string) (client.Client, error) { + kubeConfig, err := os.ReadFile(filepath.Clean(e2eIPAMKubeconfig)) + if err != nil { + return nil, err + } + + restConfig, err := clientcmd.RESTConfigFromKubeConfig(kubeConfig) + if err != nil { + return nil, err + } + + return client.New(restConfig, client.Options{Scheme: ipamScheme}) +} + // getVirtualMachineIPAddresses lists all VirtualMachines in the given folder and // returns a map which contains the IP addresses of all machines. // If the given folder is not found it will return an error. @@ -196,45 +274,12 @@ func getVirtualMachineIPAddresses(ctx context.Context, folderName string, vSpher return virtualMachineIPAddresses, nil } -type ipamService struct { - client client.Client - ipAddressClaims []ipamv1.IPAddressClaim - skipCleanup bool -} - -func newIPAMService(e2eIPAMKubeconfig string, skipCleanup bool) (*ipamService, error) { - ipamClient, err := getClient(e2eIPAMKubeconfig) - if err != nil { - return nil, err - } - - return &ipamService{ - ipAddressClaims: []ipamv1.IPAddressClaim{}, - client: ipamClient, - skipCleanup: skipCleanup, - }, nil -} - -func getClient(e2eIPAMKubeconfig string) (client.Client, error) { - kubeConfig, err := os.ReadFile(filepath.Clean(e2eIPAMKubeconfig)) - if err != nil { - return nil, err - } - - restConfig, err := clientcmd.RESTConfigFromKubeConfig(kubeConfig) - if err != nil { - return nil, err - } - - return client.New(restConfig, client.Options{Scheme: ipamScheme}) -} - -func (s *ipamService) claimIPAddress(ctx context.Context) (_ string, err error) { +func (h *helper) claimIPAddress(ctx context.Context) (_ string, _ *ipamv1.IPAddressClaim, err error) { claim := &ipamv1.IPAddressClaim{ ObjectMeta: metav1.ObjectMeta{ Name: "ipclaim-" + rand.String(32), Namespace: metav1.NamespaceDefault, - Labels: getIPAddressClaimLabels(), + Labels: h.labels, }, Spec: ipamv1.IPAddressClaimSpec{ PoolRef: corev1.TypedLocalObjectReference{ @@ -247,17 +292,16 @@ func (s *ipamService) claimIPAddress(ctx context.Context) (_ string, err error) // Create an IPAddressClaim Byf("Creating IPAddressClaim %s", klog.KObj(claim)) - if err := s.client.Create(ctx, claim); err != nil { - return "", err + if err := h.client.Create(ctx, claim); err != nil { + return "", nil, err } // Store claim inside the service so the cleanup function knows what to delete. - s.ipAddressClaims = append(s.ipAddressClaims, *claim) ip := &ipamv1.IPAddress{} var retryError error // Wait for the IPAddressClaim to refer an IPAddress. _ = wait.PollUntilContextTimeout(ctx, time.Second, time.Second*30, true, func(ctx context.Context) (done bool, err error) { - if err := s.client.Get(ctx, client.ObjectKeyFromObject(claim), claim); err != nil { + if err := h.client.Get(ctx, client.ObjectKeyFromObject(claim), claim); err != nil { retryError = errors.Wrap(err, "getting IPAddressClaim") return false, nil } @@ -267,7 +311,7 @@ func (s *ipamService) claimIPAddress(ctx context.Context) (_ string, err error) return false, nil } - if err := s.client.Get(ctx, client.ObjectKey{Namespace: claim.GetNamespace(), Name: claim.Status.AddressRef.Name}, ip); err != nil { + if err := h.client.Get(ctx, client.ObjectKey{Namespace: claim.GetNamespace(), Name: claim.Status.AddressRef.Name}, ip); err != nil { retryError = errors.Wrap(err, "getting IPAddress") return false, nil } @@ -280,49 +324,10 @@ func (s *ipamService) claimIPAddress(ctx context.Context) (_ string, err error) return true, nil }) if retryError != nil { - return "", retryError + return "", claim, retryError } - return ip.Spec.Address, nil -} - -// cleanup deletes the IPAddressClaims created by the service if the test succeeded. -func (s *ipamService) cleanup() { - if s.skipCleanup { - By("Skipping cleanup of IPAddressClaims because skipCleanup is set") - return - } - if CurrentSpecReport().Failed() { - By("Skipping cleanup of IPAddressClaims because the tests failed and the IPs could still be in use") - return - } - - var errList []error - - for _, ipAddressClaim := range s.ipAddressClaims { - ipAddressClaim := ipAddressClaim - Byf("Deleting IPAddressClaim %s", klog.KObj(&ipAddressClaim)) - if err := s.client.Delete(context.Background(), &ipAddressClaim); err != nil { - errList = append(errList, err) - } - } - - if len(errList) > 0 { - Expect(kerrors.NewAggregate(errList)).ToNot(HaveOccurred()) - } -} - -func getIPAddressClaimLabels() map[string]string { - labels := map[string]string{ - "unique-hash": rand.String(16), - } - if val := os.Getenv("BUILD_ID"); val != "" { - labels["prow.k8s.io/build-id"] = val - } - if val := os.Getenv("JOB_NAME"); val != "" { - labels["prow.k8s.io/job"] = val - } - return labels + return ip.Spec.Address, claim, nil } // Note: Copy-paste from CAPI below. diff --git a/test/e2e/k8s_conformance_test.go b/test/e2e/k8s_conformance_test.go index 7b58e79f9e..7928d67ca6 100644 --- a/test/e2e/k8s_conformance_test.go +++ b/test/e2e/k8s_conformance_test.go @@ -18,6 +18,7 @@ package e2e import ( . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" capi_e2e "sigs.k8s.io/cluster-api/test/e2e" "sigs.k8s.io/cluster-api-provider-vsphere/test/e2e/ipam" @@ -26,15 +27,13 @@ import ( var _ = Describe("When testing K8S conformance [Conformance]", func() { var ( testSpecificClusterctlConfigPath string - testSpecificIPAMCleanupFunc func() + testSpecificIPAddressClaims ipam.IPAddressClaims ) BeforeEach(func() { - testSpecificClusterctlConfigPath, testSpecificIPAMCleanupFunc = ipam.ConfigureControlPlaneIPs(ctx, e2eIPAMKubeconfig, clusterctlConfigPath, skipCleanup) + testSpecificClusterctlConfigPath, testSpecificIPAddressClaims = ipamHelper.ClaimIPs(ctx, clusterctlConfigPath) }) defer AfterEach(func() { - if testSpecificIPAMCleanupFunc != nil { - testSpecificIPAMCleanupFunc() - } + Expect(ipamHelper.Cleanup(ctx, testSpecificIPAddressClaims)).To(Succeed()) }) capi_e2e.K8SConformanceSpec(ctx, func() capi_e2e.K8SConformanceSpecInput { diff --git a/test/e2e/md_scale_test.go b/test/e2e/md_scale_test.go index a02e7cf225..4e30a3894c 100644 --- a/test/e2e/md_scale_test.go +++ b/test/e2e/md_scale_test.go @@ -18,6 +18,7 @@ package e2e import ( . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" capi_e2e "sigs.k8s.io/cluster-api/test/e2e" "sigs.k8s.io/cluster-api-provider-vsphere/test/e2e/ipam" @@ -26,15 +27,13 @@ import ( var _ = Describe("When testing MachineDeployment scale out/in", func() { var ( testSpecificClusterctlConfigPath string - testSpecificIPAMCleanupFunc func() + testSpecificIPAddressClaims ipam.IPAddressClaims ) BeforeEach(func() { - testSpecificClusterctlConfigPath, testSpecificIPAMCleanupFunc = ipam.ConfigureControlPlaneIPs(ctx, e2eIPAMKubeconfig, clusterctlConfigPath, skipCleanup) + testSpecificClusterctlConfigPath, testSpecificIPAddressClaims = ipamHelper.ClaimIPs(ctx, clusterctlConfigPath) }) defer AfterEach(func() { - if testSpecificIPAMCleanupFunc != nil { - testSpecificIPAMCleanupFunc() - } + Expect(ipamHelper.Cleanup(ctx, testSpecificIPAddressClaims)).To(Succeed()) }) capi_e2e.MachineDeploymentScaleSpec(ctx, func() capi_e2e.MachineDeploymentScaleSpecInput { diff --git a/test/e2e/node_drain_timeout_test.go b/test/e2e/node_drain_timeout_test.go index 73fdb8b9d6..e8ad042577 100644 --- a/test/e2e/node_drain_timeout_test.go +++ b/test/e2e/node_drain_timeout_test.go @@ -18,6 +18,7 @@ package e2e import ( . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" capi_e2e "sigs.k8s.io/cluster-api/test/e2e" "sigs.k8s.io/cluster-api-provider-vsphere/test/e2e/ipam" @@ -26,15 +27,13 @@ import ( var _ = Describe("When testing node drain timeout", func() { var ( testSpecificClusterctlConfigPath string - testSpecificIPAMCleanupFunc func() + testSpecificIPAddressClaims ipam.IPAddressClaims ) BeforeEach(func() { - testSpecificClusterctlConfigPath, testSpecificIPAMCleanupFunc = ipam.ConfigureControlPlaneIPs(ctx, e2eIPAMKubeconfig, clusterctlConfigPath, skipCleanup) + testSpecificClusterctlConfigPath, testSpecificIPAddressClaims = ipamHelper.ClaimIPs(ctx, clusterctlConfigPath) }) defer AfterEach(func() { - if testSpecificIPAMCleanupFunc != nil { - testSpecificIPAMCleanupFunc() - } + Expect(ipamHelper.Cleanup(ctx, testSpecificIPAddressClaims)).To(Succeed()) }) capi_e2e.NodeDrainTimeoutSpec(ctx, func() capi_e2e.NodeDrainTimeoutSpecInput { diff --git a/test/e2e/node_labeling_test.go b/test/e2e/node_labeling_test.go index 787fe6b277..efc7078819 100644 --- a/test/e2e/node_labeling_test.go +++ b/test/e2e/node_labeling_test.go @@ -44,15 +44,13 @@ var _ = Describe("Label nodes with ESXi host info", func() { var ( testSpecificClusterctlConfigPath string - testSpecificIPAMCleanupFunc func() + testSpecificIPAddressClaims ipam.IPAddressClaims ) BeforeEach(func() { - testSpecificClusterctlConfigPath, testSpecificIPAMCleanupFunc = ipam.ConfigureControlPlaneIPs(ctx, e2eIPAMKubeconfig, clusterctlConfigPath, skipCleanup) + testSpecificClusterctlConfigPath, testSpecificIPAddressClaims = ipamHelper.ClaimIPs(ctx, clusterctlConfigPath) }) defer AfterEach(func() { - if testSpecificIPAMCleanupFunc != nil { - testSpecificIPAMCleanupFunc() - } + Expect(ipamHelper.Cleanup(ctx, testSpecificIPAddressClaims)).To(Succeed()) }) BeforeEach(func() { diff --git a/test/e2e/ownerreference_test.go b/test/e2e/ownerreference_test.go index e1d66cab3b..88664cb27d 100644 --- a/test/e2e/ownerreference_test.go +++ b/test/e2e/ownerreference_test.go @@ -45,15 +45,13 @@ import ( var _ = Describe("OwnerReference checks with FailureDomains and ClusterIdentity", func() { var ( testSpecificClusterctlConfigPath string - testSpecificIPAMCleanupFunc func() + testSpecificIPAddressClaims ipam.IPAddressClaims ) BeforeEach(func() { - testSpecificClusterctlConfigPath, testSpecificIPAMCleanupFunc = ipam.ConfigureControlPlaneIPs(ctx, e2eIPAMKubeconfig, clusterctlConfigPath, skipCleanup) + testSpecificClusterctlConfigPath, testSpecificIPAddressClaims = ipamHelper.ClaimIPs(ctx, clusterctlConfigPath) }) defer AfterEach(func() { - if testSpecificIPAMCleanupFunc != nil { - testSpecificIPAMCleanupFunc() - } + Expect(ipamHelper.Cleanup(ctx, testSpecificIPAddressClaims)).To(Succeed()) }) // Before running the test create the secret used by the VSphereClusterIdentity to connect to the vCenter. diff --git a/test/e2e/quick_start_test.go b/test/e2e/quick_start_test.go index 18bb57f7f9..4368b9aa98 100644 --- a/test/e2e/quick_start_test.go +++ b/test/e2e/quick_start_test.go @@ -18,6 +18,7 @@ package e2e import ( . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" "k8s.io/utils/ptr" capi_e2e "sigs.k8s.io/cluster-api/test/e2e" @@ -27,15 +28,13 @@ import ( var _ = Describe("Cluster Creation using Cluster API quick-start test", func() { var ( testSpecificClusterctlConfigPath string - testSpecificIPAMCleanupFunc func() + testSpecificIPAddressClaims ipam.IPAddressClaims ) BeforeEach(func() { - testSpecificClusterctlConfigPath, testSpecificIPAMCleanupFunc = ipam.ConfigureControlPlaneIPs(ctx, e2eIPAMKubeconfig, clusterctlConfigPath, skipCleanup) + testSpecificClusterctlConfigPath, testSpecificIPAddressClaims = ipamHelper.ClaimIPs(ctx, clusterctlConfigPath) }) defer AfterEach(func() { - if testSpecificIPAMCleanupFunc != nil { - testSpecificIPAMCleanupFunc() - } + Expect(ipamHelper.Cleanup(ctx, testSpecificIPAddressClaims)).To(Succeed()) }) capi_e2e.QuickStartSpec(ctx, func() capi_e2e.QuickStartSpecInput { @@ -52,15 +51,13 @@ var _ = Describe("Cluster Creation using Cluster API quick-start test", func() { var _ = Describe("ClusterClass Creation using Cluster API quick-start test [PR-Blocking] [ClusterClass]", func() { var ( testSpecificClusterctlConfigPath string - testSpecificIPAMCleanupFunc func() + testSpecificIPAddressClaims ipam.IPAddressClaims ) BeforeEach(func() { - testSpecificClusterctlConfigPath, testSpecificIPAMCleanupFunc = ipam.ConfigureControlPlaneIPs(ctx, e2eIPAMKubeconfig, clusterctlConfigPath, skipCleanup) + testSpecificClusterctlConfigPath, testSpecificIPAddressClaims = ipamHelper.ClaimIPs(ctx, clusterctlConfigPath) }) defer AfterEach(func() { - if testSpecificIPAMCleanupFunc != nil { - testSpecificIPAMCleanupFunc() - } + Expect(ipamHelper.Cleanup(ctx, testSpecificIPAddressClaims)).To(Succeed()) }) capi_e2e.QuickStartSpec(ctx, func() capi_e2e.QuickStartSpecInput { @@ -78,15 +75,13 @@ var _ = Describe("ClusterClass Creation using Cluster API quick-start test [PR-B var _ = Describe("Cluster creation with [Ignition] bootstrap [PR-Blocking]", func() { var ( testSpecificClusterctlConfigPath string - testSpecificIPAMCleanupFunc func() + testSpecificIPAddressClaims ipam.IPAddressClaims ) BeforeEach(func() { - testSpecificClusterctlConfigPath, testSpecificIPAMCleanupFunc = ipam.ConfigureControlPlaneIPs(ctx, e2eIPAMKubeconfig, clusterctlConfigPath, skipCleanup) + testSpecificClusterctlConfigPath, testSpecificIPAddressClaims = ipamHelper.ClaimIPs(ctx, clusterctlConfigPath) }) defer AfterEach(func() { - if testSpecificIPAMCleanupFunc != nil { - testSpecificIPAMCleanupFunc() - } + Expect(ipamHelper.Cleanup(ctx, testSpecificIPAddressClaims)).To(Succeed()) }) capi_e2e.QuickStartSpec(ctx, func() capi_e2e.QuickStartSpecInput { diff --git a/test/e2e/storage_policy_test.go b/test/e2e/storage_policy_test.go index ac36e59f98..b551bbcf36 100644 --- a/test/e2e/storage_policy_test.go +++ b/test/e2e/storage_policy_test.go @@ -48,15 +48,13 @@ var _ = Describe("Cluster creation with storage policy", func() { var ( testSpecificClusterctlConfigPath string - testSpecificIPAMCleanupFunc func() + testSpecificIPAddressClaims ipam.IPAddressClaims ) BeforeEach(func() { - testSpecificClusterctlConfigPath, testSpecificIPAMCleanupFunc = ipam.ConfigureControlPlaneIPs(ctx, e2eIPAMKubeconfig, clusterctlConfigPath, skipCleanup) + testSpecificClusterctlConfigPath, testSpecificIPAddressClaims = ipamHelper.ClaimIPs(ctx, clusterctlConfigPath) }) defer AfterEach(func() { - if testSpecificIPAMCleanupFunc != nil { - testSpecificIPAMCleanupFunc() - } + Expect(ipamHelper.Cleanup(ctx, testSpecificIPAddressClaims)).To(Succeed()) }) BeforeEach(func() {