diff --git a/controllers/openstackfloatingippool_controller.go b/controllers/openstackfloatingippool_controller.go index 78d7dba92c..6e93da49fc 100644 --- a/controllers/openstackfloatingippool_controller.go +++ b/controllers/openstackfloatingippool_controller.go @@ -29,10 +29,15 @@ import ( "k8s.io/client-go/tools/record" "k8s.io/utils/ptr" clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" ipamv1 "sigs.k8s.io/cluster-api/api/ipam/v1beta2" + "sigs.k8s.io/cluster-api/util" + "sigs.k8s.io/cluster-api/util/annotations" v1beta1conditions "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/conditions" "sigs.k8s.io/cluster-api/util/patch" + "sigs.k8s.io/cluster-api/util/predicates" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -72,6 +77,7 @@ type OpenStackFloatingIPPoolReconciler struct { // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=openstackfloatingippools/status,verbs=get;update;patch // +kubebuilder:rbac:groups=ipam.cluster.x-k8s.io,resources=ipaddressclaims;ipaddressclaims/status,verbs=get;list;watch;update;create;delete // +kubebuilder:rbac:groups=ipam.cluster.x-k8s.io,resources=ipaddresses;ipaddresses/status,verbs=get;list;watch;create;update;delete +// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters;clusters/status,verbs=get;list;watch func (r *OpenStackFloatingIPPoolReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) { log := ctrl.LoggerFrom(ctx) @@ -127,7 +133,45 @@ func (r *OpenStackFloatingIPPoolReconciler) Reconcile(ctx context.Context, req c for _, claim := range claims.Items { log := log.WithValues("claim", claim.Name) + + cluster, err := util.GetClusterFromMetadata(ctx, r.Client, claim.ObjectMeta) + if err != nil { + log.Error(err, "Failed to get owning cluster, skipping claim", "claim", claim.Name) + continue + } + + if annotations.IsPaused(cluster, &claim) { + log.V(4).Info("IPAddressClaim or linked Cluster is paused, skipping reconcile", "claim", claim.Name, "namespace", claim.Namespace) + continue + } + + // Add finalizer if it does not exist + if controllerutil.AddFinalizer(&claim, infrav1alpha1.OpenStackFloatingIPPoolFinalizer) { + if err := r.Client.Update(ctx, &claim); err != nil { + log.Error(err, "Failed to add finalizer to claim", "claim", claim.Name) + return ctrl.Result{}, err + } + continue + } + if !claim.DeletionTimestamp.IsZero() { + ipaddressName := claim.Status.AddressRef.Name + if ipaddressName != "" { + ipAddress := &ipamv1.IPAddress{} + err := r.Client.Get(ctx, client.ObjectKey{Name: claim.Name, Namespace: claim.Namespace}, ipAddress) + if err != nil { + return ctrl.Result{}, err + } + err = r.deleteIPAddress(ctx, scope, pool, ipAddress) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to delete IPAddress %q: %w", ipAddress.Name, err) + } + } + + controllerutil.RemoveFinalizer(&claim, infrav1alpha1.OpenStackFloatingIPPoolFinalizer) + if err := r.Client.Update(ctx, &claim); err != nil && reterr == nil { + reterr = err + } continue } @@ -272,10 +316,6 @@ func (r *OpenStackFloatingIPPoolReconciler) reconcileIPAddresses(ctx context.Con return err } - networkingService, err := networking.NewService(scope) - if err != nil { - return err - } pool.Status.ClaimedIPs = []string{} if pool.Status.AvailableIPs == nil { pool.Status.AvailableIPs = []string{} @@ -288,18 +328,40 @@ func (r *OpenStackFloatingIPPoolReconciler) reconcileIPAddresses(ctx context.Con continue } - if controllerutil.ContainsFinalizer(ipAddress, infrav1alpha1.DeleteFloatingIPFinalizer) { - if pool.Spec.ReclaimPolicy == infrav1alpha1.ReclaimDelete && !contains(pool.Spec.PreAllocatedFloatingIPs, ipAddress.Spec.Address) { - if err = networkingService.DeleteFloatingIP(pool, ipAddress.Spec.Address); err != nil { - return fmt.Errorf("delete floating IP %q: %w", ipAddress.Spec.Address, err) + // Check if the owning claim or its cluster is paused before processing deletion, + // and clear the claim's AddressRef so it can be re-reconciled once unpaused or re-created. + claim := &ipamv1.IPAddressClaim{} + if ipAddress.Spec.ClaimRef.Name == "" { + claim = nil + } else { + if err := r.Client.Get(ctx, client.ObjectKey{Name: ipAddress.Spec.ClaimRef.Name, Namespace: ipAddress.Namespace}, claim); err != nil { + if !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to get IPAddressClaim %q: %w", ipAddress.Spec.ClaimRef.Name, err) } + claim = nil } else { - pool.Status.AvailableIPs = append(pool.Status.AvailableIPs, ipAddress.Spec.Address) + cluster, err := util.GetClusterFromMetadata(ctx, r.Client, claim.ObjectMeta) + if err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to get owning cluster for claim %q: %w", claim.Name, err) + } + if cluster != nil && annotations.IsPaused(cluster, claim) { + scope.Logger().V(4).Info("IPAddress owner IPAddressClaim or linked Cluster is paused, skipping deletion", "ipAddress", ipAddress.Name, "claim", claim.Name) + continue + } } } - controllerutil.RemoveFinalizer(ipAddress, infrav1alpha1.DeleteFloatingIPFinalizer) - if err := r.Client.Update(ctx, ipAddress); err != nil { - return err + + err := r.deleteIPAddress(ctx, scope, pool, ipAddress) + if err != nil { + return fmt.Errorf("failed to delete IPAddress %q: %w", ipAddress.Name, err) + } + + // Clear AddressRef so the claim will be re-assigned an IP on the next reconcile. + if claim != nil && claim.Status.AddressRef.Name != "" { + claim.Status.AddressRef.Name = "" + if err := r.Client.Status().Update(ctx, claim); err != nil { + return fmt.Errorf("failed to clear AddressRef for claim %q: %w", claim.Name, err) + } } } allIPs := union(pool.Status.AvailableIPs, pool.Spec.PreAllocatedFloatingIPs) @@ -308,6 +370,30 @@ func (r *OpenStackFloatingIPPoolReconciler) reconcileIPAddresses(ctx context.Con return nil } +func (r *OpenStackFloatingIPPoolReconciler) deleteIPAddress(ctx context.Context, scope *scope.WithLogger, pool *infrav1alpha1.OpenStackFloatingIPPool, ipAddress *ipamv1.IPAddress) error { + networkingService, err := networking.NewService(scope) + if err != nil { + return err + } + + if controllerutil.ContainsFinalizer(ipAddress, infrav1alpha1.DeleteFloatingIPFinalizer) { + if pool.Spec.ReclaimPolicy == infrav1alpha1.ReclaimDelete && !contains(pool.Spec.PreAllocatedFloatingIPs, ipAddress.Spec.Address) { + if err = networkingService.DeleteFloatingIP(pool, ipAddress.Spec.Address); err != nil { + return fmt.Errorf("delete floating IP %q: %w", ipAddress.Spec.Address, err) + } + } else { + pool.Status.AvailableIPs = append(pool.Status.AvailableIPs, ipAddress.Spec.Address) + } + } + + controllerutil.RemoveFinalizer(ipAddress, infrav1alpha1.DeleteFloatingIPFinalizer) + if err := r.Client.Update(ctx, ipAddress); err != nil { + return err + } + + return nil +} + func (r *OpenStackFloatingIPPoolReconciler) getIP(ctx context.Context, scope *scope.WithLogger, pool *infrav1alpha1.OpenStackFloatingIPPool) (string, error) { // There's a potential leak of IPs here, if the reconcile loop fails after we claim an IP but before we create the IPAddress object. var ip string @@ -422,7 +508,8 @@ func (r *OpenStackFloatingIPPoolReconciler) reconcileFloatingIPNetwork(scope *sc return nil } -func (r *OpenStackFloatingIPPoolReconciler) ipAddressClaimToPoolMapper(_ context.Context, o client.Object) []ctrl.Request { +func (r *OpenStackFloatingIPPoolReconciler) ipAddressClaimToPoolMapper(ctx context.Context, o client.Object) []ctrl.Request { + log := ctrl.LoggerFrom(ctx) claim, ok := o.(*ipamv1.IPAddressClaim) if !ok { panic(fmt.Sprintf("Expected a IPAddressClaim but got a %T", o)) @@ -430,6 +517,18 @@ func (r *OpenStackFloatingIPPoolReconciler) ipAddressClaimToPoolMapper(_ context if claim.Spec.PoolRef.Kind != openStackFloatingIPPool { return nil } + + cluster, err := util.GetClusterFromMetadata(ctx, r.Client, claim.ObjectMeta) + if err != nil { + log.Error(err, "Failed to get owning cluster, skipping mapping", "claim", claim.Name, "namespace", claim.Namespace) + return nil + } + + if annotations.IsPaused(cluster, claim) { + log.V(4).Info("IPAddressClaim or linked Cluster is paused, skipping mapping", "claim", claim.Name, "namespace", claim.Namespace) + return nil + } + return []ctrl.Request{ { NamespacedName: client.ObjectKey{ @@ -440,6 +539,39 @@ func (r *OpenStackFloatingIPPoolReconciler) ipAddressClaimToPoolMapper(_ context } } +func (r *OpenStackFloatingIPPoolReconciler) clusterToPoolMapper(ctx context.Context, o client.Object) []ctrl.Request { + log := ctrl.LoggerFrom(ctx) + cluster, ok := o.(*clusterv1.Cluster) + if !ok { + panic(fmt.Sprintf("Expected a Cluster but got a %T", o)) + } + + claims := &ipamv1.IPAddressClaimList{} + if err := r.Client.List(ctx, claims, client.InNamespace(cluster.Namespace), client.MatchingLabels{clusterv1.ClusterNameLabel: cluster.Name}); err != nil { + log.Error(err, "Failed to list IPAddressClaims for cluster, skipping mapping", "cluster", cluster.Name, "namespace", cluster.Namespace) + return nil + } + + requestsByPool := make(map[client.ObjectKey]struct{}) + for i := range claims.Items { + claim := &claims.Items[i] + if claim.Spec.PoolRef.Kind != openStackFloatingIPPool { + continue + } + if annotations.IsPaused(cluster, claim) { + continue + } + requestsByPool[client.ObjectKey{Name: claim.Spec.PoolRef.Name, Namespace: claim.Namespace}] = struct{}{} + } + + requests := make([]ctrl.Request, 0, len(requestsByPool)) + for key := range requestsByPool { + requests = append(requests, ctrl.Request{NamespacedName: key}) + } + + return requests +} + func (r *OpenStackFloatingIPPoolReconciler) ipAddressToPoolMapper(_ context.Context, o client.Object) []ctrl.Request { ip, ok := o.(*ipamv1.IPAddress) if !ok { @@ -485,6 +617,11 @@ func (r *OpenStackFloatingIPPoolReconciler) SetupWithManager(ctx context.Context &ipamv1.IPAddressClaim{}, handler.EnqueueRequestsFromMapFunc(r.ipAddressClaimToPoolMapper), ). + Watches( + &clusterv1.Cluster{}, + handler.EnqueueRequestsFromMapFunc(r.clusterToPoolMapper), + builder.WithPredicates(predicates.ClusterUnpaused(mgr.GetScheme(), ctrl.LoggerFrom(ctx))), + ). Watches( &ipamv1.IPAddress{}, handler.EnqueueRequestsFromMapFunc(r.ipAddressToPoolMapper), diff --git a/controllers/openstackfloatingippool_controller_test.go b/controllers/openstackfloatingippool_controller_test.go index 77adbd7f5d..074a1cc1cf 100644 --- a/controllers/openstackfloatingippool_controller_test.go +++ b/controllers/openstackfloatingippool_controller_test.go @@ -17,17 +17,27 @@ limitations under the License. package controllers import ( + "context" "fmt" + floatingips "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/layer3/floatingips" + networks "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/networks" . "github.com/onsi/ginkgo/v2" //nolint:revive . "github.com/onsi/gomega" //nolint:revive "go.uber.org/mock/gomock" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + ipamv1 "sigs.k8s.io/cluster-api/api/ipam/v1beta2" "sigs.k8s.io/cluster-api/test/framework" v1beta1conditions "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/conditions" "sigs.k8s.io/cluster-api/util/patch" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/reconcile" infrav1alpha1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha1" @@ -175,4 +185,437 @@ var _ = Describe("OpenStackFloatingIPPool controller", func() { Expect(condition.Severity).To(Equal(clusterv1beta1.ConditionSeverityError)) Expect(condition.Message).To(ContainSubstring("Failed to create OpenStack client scope")) }) + + // Context: full IP lifecycle — tests IP allocation/release together with cluster pause/unpause. + // + // Resources: + // - Cluster A (clusterv1.Cluster) in testNamespace + // + // Scenario 1 — cluster NOT paused: + // 1. Create IPAddressClaim → reconcile → IPAddress created, pool.Status.ClaimedIPs updated + // 2. Delete IPAddressClaim → reconcile → floating IP deleted, pool.Status.ClaimedIPs updated + // + // Scenario 2 — cluster PAUSED then UNPAUSED: + // 3. Pause cluster, create IPAddressClaim → reconcile is skipped (no IP allocated) + // 4. Unpause cluster → reconcile runs → IPAddress created, pool.Status.ClaimedIPs updated + // 5. Pause cluster, delete IPAddressClaim → reconcile is skipped (IP not released) + // 6. Unpause cluster → reconcile runs → IP released, pool.Status.ClaimedIPs updated + Context("IPAddressClaim lifecycle with cluster pause/unpause", func() { + const ( + testClusterName = "test-cluster-a" + poolName = "lifecycle-pool" + ip1 = "192.168.100.1" + ip1ID = "fip-id-1" + ip2 = "192.168.100.2" + ip2ID = "fip-id-2" + networkID = "ext-net-id" + ) + + var ( + mgrCancel context.CancelFunc + mgrDone chan struct{} + mgrClient client.Client + testCluster *clusterv1.Cluster + ) + + BeforeEach(func() { + testPool.SetName(poolName) + + var mgrCtx context.Context + mgrCtx, mgrCancel = context.WithCancel(context.Background()) + mgrDone = make(chan struct{}) + + // Build a manager so we can register field indexers that MatchingFields relies on. + mgr, err := ctrl.NewManager(cfg, ctrl.Options{ + Scheme: k8sClient.Scheme(), + Metrics: metricsserver.Options{BindAddress: "0"}, + HealthProbeBindAddress: "0", + }) + Expect(err).ToNot(HaveOccurred()) + + // Register field indexers identical to those in SetupWithManager. + Expect(mgr.GetFieldIndexer().IndexField( + mgrCtx, &ipamv1.IPAddressClaim{}, + infrav1alpha1.OpenStackFloatingIPPoolNameIndex, + func(rawObj client.Object) []string { + c := rawObj.(*ipamv1.IPAddressClaim) + if c.Spec.PoolRef.Kind != openStackFloatingIPPool { + return nil + } + return []string{c.Spec.PoolRef.Name} + }, + )).To(Succeed()) + + Expect(mgr.GetFieldIndexer().IndexField( + mgrCtx, &ipamv1.IPAddress{}, + infrav1alpha1.OpenStackFloatingIPPoolNameIndex, + func(rawObj client.Object) []string { + a := rawObj.(*ipamv1.IPAddress) + if a.Spec.PoolRef.Kind != openStackFloatingIPPool { + return nil + } + return []string{a.Spec.PoolRef.Name} + }, + )).To(Succeed()) + + mgrClient = mgr.GetClient() + // Redirect the reconciler to use the manager's cached client. + poolReconciler.Client = mgrClient + + go func() { + defer close(mgrDone) + _ = mgr.Start(mgrCtx) + }() + Expect(mgr.GetCache().WaitForCacheSync(mgrCtx)).To(BeTrue()) + + // Cluster A – used as the owning cluster for all IPAddressClaims in this context. + testCluster = &clusterv1.Cluster{ + TypeMeta: metav1.TypeMeta{ + APIVersion: clusterv1.GroupVersion.String(), + Kind: "Cluster", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: testClusterName, + Namespace: testNamespace, + }, + } + Expect(mgrClient.Create(ctx, testCluster)).To(Succeed()) + Eventually(func() error { + return mgrClient.Get(ctx, client.ObjectKey{Name: testClusterName, Namespace: testNamespace}, &clusterv1.Cluster{}) + }, "5s").Should(Succeed()) + }) + + AfterEach(func() { + orphan := metav1.DeletePropagationOrphan + _ = mgrClient.Delete(ctx, testCluster, &client.DeleteOptions{PropagationPolicy: &orphan}) + mgrCancel() + Eventually(mgrDone, "10s").Should(BeClosed()) + }) + + It("should allocate and release IPs, respecting cluster pause state", func() { + tag := testPool.GetFloatingIPTag() + + // ─── Network mock expectations ─────────────────────────────────────── + net := poolMockFactory.NetworkClient.EXPECT() + + // Network discovery: GetNetworkByParam calls ListNetwork (may repeat). + net.ListNetwork(gomock.Any()). + Return([]networks.Network{{ID: networkID, Name: "external"}}, nil). + AnyTimes() + + // GetFloatingIPsByTag: no pre-tagged IPs exist. + net.ListFloatingIP(floatingips.ListOpts{Tags: tag}). + Return([]floatingips.FloatingIP{}, nil). + AnyTimes() + + // ip1 lifecycle (scenario 1 – not paused). + net.CreateFloatingIP(gomock.Any()). + Return(&floatingips.FloatingIP{FloatingIP: ip1, ID: ip1ID}, nil). + Times(1) + net.ListFloatingIP(floatingips.ListOpts{FloatingIP: ip1}). + Return([]floatingips.FloatingIP{{FloatingIP: ip1, ID: ip1ID}}, nil). + AnyTimes() + net.ReplaceAllAttributesTags("floatingips", ip1ID, gomock.Any()). + Return([]string{tag}, nil) + net.DeleteFloatingIP(ip1ID).Return(nil) + + // ip2 lifecycle (scenario 2 – paused then unpaused). + net.CreateFloatingIP(gomock.Any()). + Return(&floatingips.FloatingIP{FloatingIP: ip2, ID: ip2ID}, nil). + Times(1) + net.ListFloatingIP(floatingips.ListOpts{FloatingIP: ip2}). + Return([]floatingips.FloatingIP{{FloatingIP: ip2, ID: ip2ID}}, nil). + AnyTimes() + net.ReplaceAllAttributesTags("floatingips", ip2ID, gomock.Any()). + Return([]string{tag}, nil) + net.DeleteFloatingIP(ip2ID).Return(nil) + + // ─── Helpers ───────────────────────────────────────────────────────── + poolKey := client.ObjectKey{Name: poolName, Namespace: testNamespace} + reconcilePool := func() { + req := reconcile.Request{NamespacedName: poolKey} + _, err := poolReconciler.Reconcile(ctx, req) + Expect(err).ToNot(HaveOccurred()) + } + getPool := func() *infrav1alpha1.OpenStackFloatingIPPool { + p := &infrav1alpha1.OpenStackFloatingIPPool{} + Expect(mgrClient.Get(ctx, poolKey, p)).To(Succeed()) + return p + } + newClaim := func(name string) *ipamv1.IPAddressClaim { + return &ipamv1.IPAddressClaim{ + TypeMeta: metav1.TypeMeta{ + APIVersion: ipamv1.GroupVersion.String(), + Kind: "IPAddressClaim", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: testNamespace, + // ClusterNameLabel lets GetClusterFromMetadata locate cluster A. + Labels: map[string]string{clusterv1.ClusterNameLabel: testClusterName}, + }, + Spec: ipamv1.IPAddressClaimSpec{ + PoolRef: ipamv1.IPPoolReference{ + APIGroup: infrav1alpha1.SchemeGroupVersion.Group, + Kind: openStackFloatingIPPool, + Name: poolName, + }, + }, + } + } + pauseCluster := func() { + patched := testCluster.DeepCopy() + patched.Spec.Paused = ptr.To(true) + Expect(mgrClient.Patch(ctx, patched, client.MergeFrom(testCluster))).To(Succeed()) + testCluster = patched + Eventually(func() bool { + c := &clusterv1.Cluster{} + _ = mgrClient.Get(ctx, client.ObjectKey{Name: testClusterName, Namespace: testNamespace}, c) + return ptr.Deref(c.Spec.Paused, false) + }, "5s").Should(BeTrue()) + } + unpauseCluster := func() { + patched := testCluster.DeepCopy() + patched.Spec.Paused = nil + Expect(mgrClient.Patch(ctx, patched, client.MergeFrom(testCluster))).To(Succeed()) + testCluster = patched + Eventually(func() bool { + c := &clusterv1.Cluster{} + _ = mgrClient.Get(ctx, client.ObjectKey{Name: testClusterName, Namespace: testNamespace}, c) + return !ptr.Deref(c.Spec.Paused, false) + }, "5s").Should(BeTrue()) + } + + // ───────────────────────────────────────────────────────────────────── + // SCENARIO 1: cluster NOT paused + // ───────────────────────────────────────────────────────────────────── + By("Scenario 1: create pool and first claim (cluster not paused)") + + Expect(mgrClient.Create(ctx, testPool)).To(Succeed()) + Eventually(func() error { + return mgrClient.Get(ctx, poolKey, &infrav1alpha1.OpenStackFloatingIPPool{}) + }, "5s").Should(Succeed()) + + claim1 := newClaim("claim-1") + Expect(mgrClient.Create(ctx, claim1)).To(Succeed()) + Eventually(func() error { + return mgrClient.Get(ctx, client.ObjectKey{Name: "claim-1", Namespace: testNamespace}, &ipamv1.IPAddressClaim{}) + }, "5s").Should(Succeed()) + + // Pass 1: pool gets its finalizer (early return). + reconcilePool() + // Wait for the pool finalizer to be in the cache before the next pass. + Eventually(func() bool { + p := &infrav1alpha1.OpenStackFloatingIPPool{} + if err := mgrClient.Get(ctx, poolKey, p); err != nil { + return false + } + return controllerutil.ContainsFinalizer(p, infrav1alpha1.OpenStackFloatingIPPoolFinalizer) + }, "5s").Should(BeTrue(), "pool should have its finalizer in cache") + + // Pass 2: network discovered, claim gets its finalizer (early return via continue). + reconcilePool() + // Wait for claim1 to have its finalizer in the cache before the next pass. + // This prevents stale-cache issues in subsequent reconcile passes. + Eventually(func() bool { + c := &ipamv1.IPAddressClaim{} + if err := mgrClient.Get(ctx, client.ObjectKey{Name: "claim-1", Namespace: testNamespace}, c); err != nil { + return false + } + return controllerutil.ContainsFinalizer(c, infrav1alpha1.OpenStackFloatingIPPoolFinalizer) + }, "5s").Should(BeTrue(), "claim-1 should have its finalizer in cache") + + // Pass 3: IP allocated, IPAddress created, claim.Status.AddressRef set. + reconcilePool() + + By("Scenario 1: verify IP is claimed") + Eventually(func() []string { + return getPool().Status.ClaimedIPs + }, "5s").Should(ContainElement(ip1)) + + // Also wait for the cache to reflect claim1.Status.AddressRef so the + // claim deletion reconcile path can see it and call deleteIPAddress. + Eventually(func() string { + c := &ipamv1.IPAddressClaim{} + _ = mgrClient.Get(ctx, client.ObjectKey{Name: "claim-1", Namespace: testNamespace}, c) + return c.Status.AddressRef.Name + }, "5s").Should(Equal("claim-1")) + + ipAddress1 := &ipamv1.IPAddress{} + Expect(mgrClient.Get(ctx, client.ObjectKey{Name: "claim-1", Namespace: testNamespace}, ipAddress1)).To(Succeed()) + Expect(ipAddress1.Spec.Address).To(Equal(ip1)) + + By("Scenario 1: delete claim → IP released") + Expect(mgrClient.Delete(ctx, claim1)).To(Succeed()) + Eventually(func() bool { + c := &ipamv1.IPAddressClaim{} + if err := mgrClient.Get(ctx, client.ObjectKey{Name: "claim-1", Namespace: testNamespace}, c); err != nil { + return false + } + return !c.DeletionTimestamp.IsZero() + }, "5s").Should(BeTrue()) + + // Reconcile: processes claim deletion — removes DeleteFloatingIPFinalizer + // from IPAddress and removes pool finalizer from claim. + reconcilePool() + + // Verify the DeleteFloatingIPFinalizer was removed from the IPAddress by + // waiting for the cache to reflect the update. + Eventually(func() bool { + fresh := &ipamv1.IPAddress{} + if err := k8sClient.Get(ctx, client.ObjectKey{Name: "claim-1", Namespace: testNamespace}, fresh); err != nil { + return true // already gone + } + return len(fresh.Finalizers) == 0 + }, "5s").Should(BeTrue(), "DeleteFloatingIPFinalizer should be removed") + + // Simulate GC: manually delete the IPAddress now that its finalizer is removed. + // Use the direct k8sClient and default (background) propagation — Orphan + // propagation adds an internal GC finalizer that is never removed in envtest. + freshIPAddr1 := &ipamv1.IPAddress{} + if err := k8sClient.Get(ctx, client.ObjectKey{Name: "claim-1", Namespace: testNamespace}, freshIPAddr1); err == nil { + Expect(k8sClient.Delete(ctx, freshIPAddr1)).To(Succeed()) + } + Eventually(func() error { + return k8sClient.Get(ctx, client.ObjectKey{Name: "claim-1", Namespace: testNamespace}, &ipamv1.IPAddress{}) + }, "5s").ShouldNot(Succeed(), "IPAddress should be deleted") + + // Wait for both claim1 and ipAddress1 to be gone from the manager cache + // before the next reconcile — otherwise the reconciler would see stale + // objects and try to re-add finalizers (causing a 422 or double-delete). + Eventually(func() bool { + return apierrors.IsNotFound(mgrClient.Get(ctx, client.ObjectKey{Name: "claim-1", Namespace: testNamespace}, &ipamv1.IPAddressClaim{})) + }, "5s").Should(BeTrue(), "claim-1 should be gone from cache") + Eventually(func() bool { + return apierrors.IsNotFound(mgrClient.Get(ctx, client.ObjectKey{Name: "claim-1", Namespace: testNamespace}, &ipamv1.IPAddress{})) + }, "5s").Should(BeTrue(), "ipAddress1 should be gone from cache") + + // Reconcile: pool status rebuilt without ip1. + reconcilePool() + Expect(getPool().Status.ClaimedIPs).NotTo(ContainElement(ip1)) + + // ───────────────────────────────────────────────────────────────────── + // SCENARIO 2a: cluster PAUSED – create claim is skipped + // ───────────────────────────────────────────────────────────────────── + By("Scenario 2a: pause cluster, create claim → reconcile skipped") + + pauseCluster() + + claim2 := newClaim("claim-2") + Expect(mgrClient.Create(ctx, claim2)).To(Succeed()) + Eventually(func() error { + return mgrClient.Get(ctx, client.ObjectKey{Name: "claim-2", Namespace: testNamespace}, &ipamv1.IPAddressClaim{}) + }, "5s").Should(Succeed()) + + // Cluster is paused — reconcile runs but skips the claim entirely. + reconcilePool() + + // Claim must still have no finalizer and no IP. + fetchedClaim2 := &ipamv1.IPAddressClaim{} + Expect(mgrClient.Get(ctx, client.ObjectKey{Name: "claim-2", Namespace: testNamespace}, fetchedClaim2)).To(Succeed()) + Expect(fetchedClaim2.Finalizers).To(BeEmpty()) + Expect(fetchedClaim2.Status.AddressRef.Name).To(BeEmpty()) + Expect(getPool().Status.ClaimedIPs).NotTo(ContainElement(ip2)) + + // ───────────────────────────────────────────────────────────────────── + // SCENARIO 2b: unpause → IP allocated + // ───────────────────────────────────────────────────────────────────── + By("Scenario 2b: unpause cluster → IP allocated") + + unpauseCluster() + + // Pass 1: claim gets its finalizer. + reconcilePool() + // Wait for claim2's finalizer to appear in the cache before the next pass. + Eventually(func() bool { + c := &ipamv1.IPAddressClaim{} + if err := mgrClient.Get(ctx, client.ObjectKey{Name: "claim-2", Namespace: testNamespace}, c); err != nil { + return false + } + return controllerutil.ContainsFinalizer(c, infrav1alpha1.OpenStackFloatingIPPoolFinalizer) + }, "5s").Should(BeTrue(), "claim-2 should have its finalizer in cache") + // Pass 2: IP allocated. + reconcilePool() + + Eventually(func() []string { + return getPool().Status.ClaimedIPs + }, "5s").Should(ContainElement(ip2)) + + // Wait for claim2's AddressRef to be visible in cache. + Eventually(func() string { + c := &ipamv1.IPAddressClaim{} + _ = mgrClient.Get(ctx, client.ObjectKey{Name: "claim-2", Namespace: testNamespace}, c) + return c.Status.AddressRef.Name + }, "5s").Should(Equal("claim-2")) + + ipAddress2 := &ipamv1.IPAddress{} + Expect(mgrClient.Get(ctx, client.ObjectKey{Name: "claim-2", Namespace: testNamespace}, ipAddress2)).To(Succeed()) + Expect(ipAddress2.Spec.Address).To(Equal(ip2)) + + // ───────────────────────────────────────────────────────────────────── + // SCENARIO 2c: cluster PAUSED again – delete claim is skipped + // ───────────────────────────────────────────────────────────────────── + By("Scenario 2c: pause cluster, delete claim → deletion skipped") + + pauseCluster() + + Expect(mgrClient.Delete(ctx, claim2)).To(Succeed()) + Eventually(func() bool { + c := &ipamv1.IPAddressClaim{} + if err := mgrClient.Get(ctx, client.ObjectKey{Name: "claim-2", Namespace: testNamespace}, c); err != nil { + return false + } + return !c.DeletionTimestamp.IsZero() + }, "5s").Should(BeTrue()) + + // Cluster paused – reconcile should skip claim deletion. + reconcilePool() + + // ip2 is STILL claimed; IPAddress still exists. + Expect(getPool().Status.ClaimedIPs).To(ContainElement(ip2)) + Expect(mgrClient.Get(ctx, client.ObjectKey{Name: "claim-2", Namespace: testNamespace}, &ipamv1.IPAddress{})).To(Succeed()) + + // ───────────────────────────────────────────────────────────────────── + // SCENARIO 2d: unpause → IP released + // ───────────────────────────────────────────────────────────────────── + By("Scenario 2d: unpause cluster → IP released") + + unpauseCluster() + + // Reconcile: processes claim deletion and calls DeleteFloatingIP on OpenStack. + reconcilePool() + + // Verify the DeleteFloatingIPFinalizer was removed from the IPAddress. + Eventually(func() bool { + fresh := &ipamv1.IPAddress{} + if err := k8sClient.Get(ctx, client.ObjectKey{Name: "claim-2", Namespace: testNamespace}, fresh); err != nil { + return true // already gone + } + return len(fresh.Finalizers) == 0 + }, "5s").Should(BeTrue(), "DeleteFloatingIPFinalizer should be removed") + + // Simulate GC: manually delete the IPAddress now that its finalizer is removed. + // Use the direct k8sClient and default (background) propagation. + freshIPAddr2 := &ipamv1.IPAddress{} + if err := k8sClient.Get(ctx, client.ObjectKey{Name: "claim-2", Namespace: testNamespace}, freshIPAddr2); err == nil { + Expect(k8sClient.Delete(ctx, freshIPAddr2)).To(Succeed()) + } + Eventually(func() error { + return k8sClient.Get(ctx, client.ObjectKey{Name: "claim-2", Namespace: testNamespace}, &ipamv1.IPAddress{}) + }, "5s").ShouldNot(Succeed(), "IPAddress should be deleted") + + // Wait for claim2 and ipAddress2 to be gone from the manager cache + // before the final reconcile — prevents stale-object finalizer conflicts. + Eventually(func() bool { + return apierrors.IsNotFound(mgrClient.Get(ctx, client.ObjectKey{Name: "claim-2", Namespace: testNamespace}, &ipamv1.IPAddressClaim{})) + }, "5s").Should(BeTrue(), "claim-2 should be gone from cache") + Eventually(func() bool { + return apierrors.IsNotFound(mgrClient.Get(ctx, client.ObjectKey{Name: "claim-2", Namespace: testNamespace}, &ipamv1.IPAddress{})) + }, "5s").Should(BeTrue(), "ipAddress2 should be gone from cache") + + // Final reconcile: pool ClaimedIPs no longer contains ip2. + reconcilePool() + Expect(getPool().Status.ClaimedIPs).NotTo(ContainElement(ip2)) + }) + }) }) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 1738ccce4f..9d356e51ae 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -69,6 +69,8 @@ var _ = BeforeSuite(func() { CRDs: []*apiextensionsv1.CustomResourceDefinition{ external.TestClusterCRD.DeepCopy(), external.TestMachineCRD.DeepCopy(), + external.TestIPAddressClaimCRD.DeepCopy(), + external.TestIPAddressCRD.DeepCopy(), }, } diff --git a/test/helpers/external/ipam.go b/test/helpers/external/ipam.go new file mode 100644 index 0000000000..414f02a236 --- /dev/null +++ b/test/helpers/external/ipam.go @@ -0,0 +1,83 @@ +/* +Copyright 2026 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package external + +import ( + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" +) + +const ( + ipamAPIGroup = "ipam.cluster.x-k8s.io" + ipamAPIVersion = "v1beta2" +) + +var ( + // TestIPAddressClaimCRD is a minimal CRD for ipam.cluster.x-k8s.io/v1beta2 IPAddressClaim, + // used in envtest to exercise the floating IP pool controller. + TestIPAddressClaimCRD = generateIPAMCRD("IPAddressClaim", "ipaddressclaims") + + // TestIPAddressCRD is a minimal CRD for ipam.cluster.x-k8s.io/v1beta2 IPAddress, + // used in envtest to exercise the floating IP pool controller. + TestIPAddressCRD = generateIPAMCRD("IPAddress", "ipaddresses") +) + +func generateIPAMCRD(kind, pluralKind string) *apiextensionsv1.CustomResourceDefinition { + return &apiextensionsv1.CustomResourceDefinition{ + TypeMeta: metav1.TypeMeta{ + APIVersion: apiextensionsv1.SchemeGroupVersion.String(), + Kind: "CustomResourceDefinition", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: pluralKind + "." + ipamAPIGroup, + }, + Spec: apiextensionsv1.CustomResourceDefinitionSpec{ + Group: ipamAPIGroup, + Scope: apiextensionsv1.NamespaceScoped, + Names: apiextensionsv1.CustomResourceDefinitionNames{ + Kind: kind, + Plural: pluralKind, + }, + Versions: []apiextensionsv1.CustomResourceDefinitionVersion{ + { + Name: ipamAPIVersion, + Served: true, + Storage: true, + Subresources: &apiextensionsv1.CustomResourceSubresources{ + Status: &apiextensionsv1.CustomResourceSubresourceStatus{}, + }, + Schema: &apiextensionsv1.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "spec": { + Type: "object", + XPreserveUnknownFields: ptr.To(true), + }, + "status": { + Type: "object", + XPreserveUnknownFields: ptr.To(true), + }, + }, + }, + }, + }, + }, + }, + } +}