🌱 Minor improvements to api validation tests

Makefile

@@ -133,14 +133,15 @@ ifdef KUBEBUILDER_ASSETS_DIR
 	setup_envtest_extra_args += --bin-dir $(KUBEBUILDER_ASSETS_DIR)
 endif
 
+.PHONY: kubebuilder_assets
+kubebuilder_assets: $(SETUP_ENVTEST)
+	@echo Fetching assets for $(KUBEBUILDER_ENVTEST_KUBERNETES_VERSION)
+	$(eval KUBEBUILDER_ASSETS ?= $(shell $(SETUP_ENVTEST) use --use-env -p path $(setup_envtest_extra_args) $(KUBEBUILDER_ENVTEST_KUBERNETES_VERSION)))
+
 .PHONY: test
 TEST_PATHS ?= ./...
-test: $(SETUP_ENVTEST) ## Run tests
-	set -xeuf -o pipefail; \
-	if [ -z "$(KUBEBUILDER_ASSETS)" ]; then \
-		KUBEBUILDER_ASSETS=`$(SETUP_ENVTEST) use --use-env -p path $(setup_envtest_extra_args) $(KUBEBUILDER_ENVTEST_KUBERNETES_VERSION)`; \
-	fi; \
-	KUBEBUILDER_ASSETS="$$KUBEBUILDER_ASSETS" go test -v $(TEST_PATHS) $(TEST_ARGS)
+test: kubebuilder_assets
+	KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test -v $(TEST_PATHS) $(TEST_ARGS)
 
 E2E_TEMPLATES_DIR=test/e2e/data/infrastructure-openstack
 E2E_KUSTOMIZE_DIR=test/e2e/data/kustomize

test/e2e/suites/apivalidations/openstackcluster_test.go

@@ -23,142 +23,158 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/pointer"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
 )
 
 var _ = Describe("OpenStackCluster API validations", func() {
-	var cluster *infrav1.OpenStackCluster
 	var namespace *corev1.Namespace
 
-	BeforeEach(func() {
-		namespace = createNamespace()
-
-		// Initialise a basic cluster object in the correct namespace
-		cluster = &infrav1.OpenStackCluster{}
-		cluster.Namespace = namespace.Name
-		cluster.GenerateName = "cluster-"
-	})
-
-	It("should allow the smallest permissible cluster spec", func() {
-		Expect(k8sClient.Create(ctx, cluster)).To(Succeed(), "OpenStackCluster creation should succeed")
-	})
-
-	It("should only allow controlPlaneEndpoint to be set once", func() {
-		By("Creating a bare cluster")
-		Expect(k8sClient.Create(ctx, cluster)).To(Succeed(), "OpenStackCluster creation should succeed")
-
-		By("Setting the control plane endpoint")
-		cluster.Spec.ControlPlaneEndpoint = &clusterv1.APIEndpoint{
-			Host: "foo",
-			Port: 1234,
+	create := func(obj client.Object) error {
+		err := k8sClient.Create(ctx, obj)
+		if err == nil {
+			DeferCleanup(func() error {
+				return k8sClient.Delete(ctx, obj)
+			})
 		}
-		Expect(k8sClient.Update(ctx, cluster)).To(Succeed(), "Setting control plane endpoint should succeed")
-
-		By("Modifying the control plane endpoint")
-		cluster.Spec.ControlPlaneEndpoint.Host = "bar"
-		Expect(k8sClient.Update(ctx, cluster)).NotTo(Succeed(), "Updating control plane endpoint should fail")
-	})
-
-	It("should allow an empty managed security groups definition", func() {
-		cluster.Spec.ManagedSecurityGroups = &infrav1.ManagedSecurityGroups{}
-		Expect(k8sClient.Create(ctx, cluster)).To(Succeed(), "OpenStackCluster creation should succeed")
-	})
-
-	It("should default enabled to true if APIServerLoadBalancer is specified without enabled=true", func() {
-		cluster.Spec.APIServerLoadBalancer = &infrav1.APIServerLoadBalancer{}
-		Expect(k8sClient.Create(ctx, cluster)).To(Succeed(), "OpenStackCluster creation should succeed")
-
-		// Fetch the cluster and check the defaulting
-		fetchedCluster := &infrav1.OpenStackCluster{}
-		Expect(k8sClient.Get(ctx, types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, fetchedCluster)).To(Succeed(), "OpenStackCluster fetch should succeed")
-
-		Expect(fetchedCluster.Spec.APIServerLoadBalancer.Enabled).ToNot(BeNil(), "APIServerLoadBalancer.Enabled should have been defaulted")
-		Expect(*fetchedCluster.Spec.APIServerLoadBalancer.Enabled).To(BeTrue(), "APIServerLoadBalancer.Enabled should default to true")
-	})
-
-	It("should not default APIServerLoadBalancer if it is not specifid", func() {
-		Expect(k8sClient.Create(ctx, cluster)).To(Succeed(), "OpenStackCluster creation should succeed")
-
-		// Fetch the cluster and check the defaulting
-		fetchedCluster := &infrav1.OpenStackCluster{}
-		Expect(k8sClient.Get(ctx, types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, fetchedCluster)).To(Succeed(), "OpenStackCluster fetch should succeed")
+		return err
+	}
 
-		Expect(fetchedCluster.Spec.APIServerLoadBalancer).To(BeNil(), "APIServerLoadBalancer should not have been defaulted")
-		Expect(fetchedCluster.Spec.APIServerLoadBalancer.IsEnabled()).To(BeFalse(), "APIServerLoadBalancer.Enabled should not have been defaulted")
+	BeforeEach(func() {
+		namespace = createNamespace()
 	})
 
-	It("should allow bastion.enabled=true with a spec", func() {
-		cluster.Spec.Bastion = &infrav1.Bastion{
-			Enabled: pointer.Bool(true),
-			Spec: &infrav1.OpenStackMachineSpec{
-				Image: infrav1.ImageParam{
-					Filter: &infrav1.ImageFilter{
-						Name: pointer.String("fake-image"),
+	Context("infrav1", func() {
+		var cluster *infrav1.OpenStackCluster
+
+		BeforeEach(func() {
+			// Initialise a basic cluster object in the correct namespace
+			cluster = &infrav1.OpenStackCluster{}
+			cluster.Namespace = namespace.Name
+			cluster.GenerateName = "cluster-"
+		})
+
+		It("should allow the smallest permissible cluster spec", func() {
+			Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed")
+		})
+
+		It("should only allow controlPlaneEndpoint to be set once", func() {
+			By("Creating a bare cluster")
+			Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed")
+
+			By("Setting the control plane endpoint")
+			cluster.Spec.ControlPlaneEndpoint = &clusterv1.APIEndpoint{
+				Host: "foo",
+				Port: 1234,
+			}
+			Expect(k8sClient.Update(ctx, cluster)).To(Succeed(), "Setting control plane endpoint should succeed")
+
+			By("Modifying the control plane endpoint")
+			cluster.Spec.ControlPlaneEndpoint.Host = "bar"
+			Expect(k8sClient.Update(ctx, cluster)).NotTo(Succeed(), "Updating control plane endpoint should fail")
+		})
+
+		It("should allow an empty managed security groups definition", func() {
+			cluster.Spec.ManagedSecurityGroups = &infrav1.ManagedSecurityGroups{}
+			Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed")
+		})
+
+		It("should default enabled to true if APIServerLoadBalancer is specified without enabled=true", func() {
+			cluster.Spec.APIServerLoadBalancer = &infrav1.APIServerLoadBalancer{}
+			Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed")
+
+			// Fetch the cluster and check the defaulting
+			fetchedCluster := &infrav1.OpenStackCluster{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, fetchedCluster)).To(Succeed(), "OpenStackCluster fetch should succeed")
+
+			Expect(fetchedCluster.Spec.APIServerLoadBalancer.Enabled).ToNot(BeNil(), "APIServerLoadBalancer.Enabled should have been defaulted")
+			Expect(*fetchedCluster.Spec.APIServerLoadBalancer.Enabled).To(BeTrue(), "APIServerLoadBalancer.Enabled should default to true")
+		})
+
+		It("should not default APIServerLoadBalancer if it is not specifid", func() {
+			Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed")
+
+			// Fetch the cluster and check the defaulting
+			fetchedCluster := &infrav1.OpenStackCluster{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, fetchedCluster)).To(Succeed(), "OpenStackCluster fetch should succeed")
+
+			Expect(fetchedCluster.Spec.APIServerLoadBalancer).To(BeNil(), "APIServerLoadBalancer should not have been defaulted")
+			Expect(fetchedCluster.Spec.APIServerLoadBalancer.IsEnabled()).To(BeFalse(), "APIServerLoadBalancer.Enabled should not have been defaulted")
+		})
+
+		It("should allow bastion.enabled=true with a spec", func() {
+			cluster.Spec.Bastion = &infrav1.Bastion{
+				Enabled: pointer.Bool(true),
+				Spec: &infrav1.OpenStackMachineSpec{
+					Image: infrav1.ImageParam{
+						Filter: &infrav1.ImageFilter{
+							Name: pointer.String("fake-image"),
+						},
 					},
 				},
-			},
-		}
-		Expect(k8sClient.Create(ctx, cluster)).To(Succeed(), "OpenStackCluster creation should succeed")
-	})
-
-	It("should not allow bastion.enabled=true without a spec", func() {
-		cluster.Spec.Bastion = &infrav1.Bastion{
-			Enabled: pointer.Bool(true),
-		}
-		Expect(k8sClient.Create(ctx, cluster)).NotTo(Succeed(), "OpenStackCluster creation should not succeed")
-	})
-
-	It("should not allow an empty Bastion", func() {
-		cluster.Spec.Bastion = &infrav1.Bastion{}
-		Expect(k8sClient.Create(ctx, cluster)).NotTo(Succeed(), "OpenStackCluster creation should not succeed")
-	})
-
-	It("should default bastion.enabled=true", func() {
-		cluster.Spec.Bastion = &infrav1.Bastion{
-			Spec: &infrav1.OpenStackMachineSpec{
-				Image: infrav1.ImageParam{
-					Filter: &infrav1.ImageFilter{
-						Name: pointer.String("fake-image"),
+			}
+			Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed")
+		})
+
+		It("should not allow bastion.enabled=true without a spec", func() {
+			cluster.Spec.Bastion = &infrav1.Bastion{
+				Enabled: pointer.Bool(true),
+			}
+			Expect(create(cluster)).NotTo(Succeed(), "OpenStackCluster creation should not succeed")
+		})
+
+		It("should not allow an empty Bastion", func() {
+			cluster.Spec.Bastion = &infrav1.Bastion{}
+			Expect(create(cluster)).NotTo(Succeed(), "OpenStackCluster creation should not succeed")
+		})
+
+		It("should default bastion.enabled=true", func() {
+			cluster.Spec.Bastion = &infrav1.Bastion{
+				Spec: &infrav1.OpenStackMachineSpec{
+					Image: infrav1.ImageParam{
+						Filter: &infrav1.ImageFilter{
+							Name: pointer.String("fake-image"),
+						},
 					},
 				},
-			},
-		}
-		Expect(k8sClient.Create(ctx, cluster)).To(Succeed(), "OpenStackCluster creation should not succeed")
-
-		// Fetch the cluster and check the defaulting
-		fetchedCluster := &infrav1.OpenStackCluster{}
-		Expect(k8sClient.Get(ctx, types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, fetchedCluster)).To(Succeed(), "OpenStackCluster fetch should succeed")
-		Expect(fetchedCluster.Spec.Bastion.Enabled).ToNot(BeNil(), "Bastion.Enabled should have been defaulted")
-		Expect(*fetchedCluster.Spec.Bastion.Enabled).To(BeTrueBecause("Bastion.Enabled should default to true"))
-	})
-
-	It("should allow IPv4 as bastion floatingIP", func() {
-		cluster.Spec.Bastion = &infrav1.Bastion{
-			Enabled: pointer.Bool(true),
-			Spec: &infrav1.OpenStackMachineSpec{
-				Image: infrav1.ImageParam{
-					Filter: &infrav1.ImageFilter{
-						Name: pointer.String("fake-image"),
+			}
+			Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should not succeed")
+
+			// Fetch the cluster and check the defaulting
+			fetchedCluster := &infrav1.OpenStackCluster{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, fetchedCluster)).To(Succeed(), "OpenStackCluster fetch should succeed")
+			Expect(fetchedCluster.Spec.Bastion.Enabled).ToNot(BeNil(), "Bastion.Enabled should have been defaulted")
+			Expect(*fetchedCluster.Spec.Bastion.Enabled).To(BeTrueBecause("Bastion.Enabled should default to true"))
+		})
+
+		It("should allow IPv4 as bastion floatingIP", func() {
+			cluster.Spec.Bastion = &infrav1.Bastion{
+				Enabled: pointer.Bool(true),
+				Spec: &infrav1.OpenStackMachineSpec{
+					Image: infrav1.ImageParam{
+						Filter: &infrav1.ImageFilter{
+							Name: pointer.String("fake-image"),
+						},
 					},
 				},
-			},
-			FloatingIP: pointer.String("10.0.0.0"),
-		}
-		Expect(k8sClient.Create(ctx, cluster)).To(Succeed(), "OpenStackCluster creation should succeed")
-	})
-
-	It("should not allow non-IPv4 as bastion floating IP", func() {
-		cluster.Spec.Bastion = &infrav1.Bastion{
-			Spec: &infrav1.OpenStackMachineSpec{
-				Image: infrav1.ImageParam{
-					Filter: &infrav1.ImageFilter{
-						Name: pointer.String("fake-image"),
+				FloatingIP: pointer.String("10.0.0.0"),
+			}
+			Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed")
+		})
+
+		It("should not allow non-IPv4 as bastion floating IP", func() {
+			cluster.Spec.Bastion = &infrav1.Bastion{
+				Spec: &infrav1.OpenStackMachineSpec{
+					Image: infrav1.ImageParam{
+						Filter: &infrav1.ImageFilter{
+							Name: pointer.String("fake-image"),
+						},
 					},
 				},
-			},
-			FloatingIP: pointer.String("foobar"),
-		}
-		Expect(k8sClient.Create(ctx, cluster)).NotTo(Succeed(), "OpenStackCluster creation should not succeed")
+				FloatingIP: pointer.String("foobar"),
+			}
+			Expect(create(cluster)).NotTo(Succeed(), "OpenStackCluster creation should not succeed")
+		})
 	})
 })

test/e2e/suites/apivalidations/suite_test.go

@@ -63,12 +63,21 @@ func TestAPIs(t *testing.T) {
 }
 
 var _ = BeforeSuite(func() {
+	testScheme = scheme.Scheme
+	for _, f := range []func(*runtime.Scheme) error{
+		infrav1alpha1.AddToScheme,
+		infrav1alpha5.AddToScheme,
+		infrav1alpha6.AddToScheme,
+		infrav1alpha7.AddToScheme,
+		infrav1.AddToScheme,
+	} {
+		Expect(f(testScheme)).To(Succeed())
+	}
+
 	By("bootstrapping test environment")
+	testCRDs := filepath.Join("..", "..", "..", "..", "config", "crd", "bases")
 	testEnv = &envtest.Environment{
-		CRDDirectoryPaths: []string{
-			// NOTE: These are the bare CRDs without conversion webhooks
-			filepath.Join("..", "..", "..", "..", "config", "crd", "bases"),
-		},
+		CRDDirectoryPaths:     []string{testCRDs},
 		ErrorIfCRDPathMissing: true,
 		WebhookInstallOptions: envtest.WebhookInstallOptions{
 			Paths: []string{
@@ -86,17 +95,6 @@ var _ = BeforeSuite(func() {
 		return testEnv.Stop()
 	})
 
-	testScheme = scheme.Scheme
-	for _, f := range []func(*runtime.Scheme) error{
-		infrav1alpha1.AddToScheme,
-		infrav1alpha5.AddToScheme,
-		infrav1alpha6.AddToScheme,
-		infrav1alpha7.AddToScheme,
-		infrav1.AddToScheme,
-	} {
-		Expect(f(testScheme)).To(Succeed())
-	}
-
 	k8sClient, err = client.New(cfg, client.Options{Scheme: testScheme})
 	Expect(err).NotTo(HaveOccurred())
 	Expect(k8sClient).NotTo(BeNil())
@@ -128,6 +126,7 @@ var _ = BeforeSuite(func() {
 			Host:    testEnv.WebhookInstallOptions.LocalServingHost,
 			CertDir: testEnv.WebhookInstallOptions.LocalServingCertDir,
 		}),
+		Logger: GinkgoLogr,
 	})
 	Expect(err).ToNot(HaveOccurred(), "Manager setup should succeed")
 	Expect(webhooks.RegisterAllWithManager(mgr)).To(BeEmpty(), "Failed to register webhooks")