From 5e347c5487f29d8d6faddc84f4e5b5c5406155f8 Mon Sep 17 00:00:00 2001 From: Dakota Crowder Date: Fri, 24 Apr 2026 11:13:58 -0400 Subject: [PATCH] MGMT-23908: Public IP feedback controller --- cmd/main.go | 12 +- .../publicip_feedback_controller.go | 266 ++++++++ .../publicip_feedback_controller_test.go | 622 ++++++++++++++++++ internal/controller/publicip_names.go | 5 +- 4 files changed, 901 insertions(+), 4 deletions(-) create mode 100644 internal/controller/publicip_feedback_controller.go create mode 100644 internal/controller/publicip_feedback_controller_test.go diff --git a/cmd/main.go b/cmd/main.go index 5fa941c9..18de444a 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -525,13 +525,23 @@ func setupNetworkingControllers( } // Setup PublicIP controller - // Feedback controller is tracked separately in MGMT-23908. if err := controller.NewPublicIPReconciler(mgr, networkingNamespace, networkingProvider, statusPollInterval, maxJobHistory, targetCluster, ).SetupWithManager(mgr); err != nil { return fmt.Errorf("publicip controller: %w", err) } + // Setup PublicIP feedback controller + if grpcConn != nil { + if err := controller.NewPublicIPFeedbackReconciler( + localMgr.GetClient(), + grpcConn, + networkingNamespace, + ).SetupWithManager(mgr); err != nil { + return fmt.Errorf("publicip feedback controller: %w", err) + } + } + return nil } diff --git a/internal/controller/publicip_feedback_controller.go b/internal/controller/publicip_feedback_controller.go new file mode 100644 index 00000000..388d2383 --- /dev/null +++ b/internal/controller/publicip_feedback_controller.go @@ -0,0 +1,266 @@ +/* +Copyright (c) 2026 Red Hat Inc. + +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 controller + +import ( + "context" + "errors" + "fmt" + + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + clnt "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + ctrllog "sigs.k8s.io/controller-runtime/pkg/log" + mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager" + + "github.com/osac-project/osac-operator/api/v1alpha1" + privatev1 "github.com/osac-project/osac-operator/internal/api/osac/private/v1" +) + +var ErrPublicIPNotFound = errors.New("public IP not found in fulfillment service") + +// PublicIPFeedbackReconciler sends updates to the fulfillment service. +type PublicIPFeedbackReconciler struct { + hubClient clnt.Client + publicIPsClient privatev1.PublicIPsClient + networkingNamespace string +} + +type publicIPFeedbackReconcilerTask struct { + r *PublicIPFeedbackReconciler + object *v1alpha1.PublicIP + publicIP *privatev1.PublicIP +} + +// NewPublicIPFeedbackReconciler creates a reconciler that sends to the fulfillment service updates about public IPs. +func NewPublicIPFeedbackReconciler(hubClient clnt.Client, grpcConn *grpc.ClientConn, networkingNamespace string) *PublicIPFeedbackReconciler { + return &PublicIPFeedbackReconciler{ + hubClient: hubClient, + publicIPsClient: privatev1.NewPublicIPsClient(grpcConn), + networkingNamespace: networkingNamespace, + } +} + +// SetupWithManager adds the reconciler to the controller manager. +func (r *PublicIPFeedbackReconciler) SetupWithManager(mgr mcmanager.Manager) error { + localMgr := mgr.GetLocalManager() + if localMgr == nil { + return fmt.Errorf("local manager is nil") + } + + return ctrl.NewControllerManagedBy(localMgr). + Named("publicip-feedback"). + For(&v1alpha1.PublicIP{}, builder.WithPredicates(NetworkingNamespacePredicate(r.networkingNamespace))). + Complete(r) +} + +// Reconcile is the implementation of the reconciler interface. +func (r *PublicIPFeedbackReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) { + log := ctrllog.FromContext(ctx) + + // Step 1: Fetch the object to reconcile, and do nothing if it no longer exists: + object := &v1alpha1.PublicIP{} + if err := r.hubClient.Get(ctx, request.NamespacedName, object); err != nil { + return ctrl.Result{}, clnt.IgnoreNotFound(err) + } + + // Step 2: Get the identifier of the public IP from the labels. If this isn't present it means that the object + // wasn't created by the fulfillment service, so we ignore it. + publicIPID, ok := object.Labels[osacPublicIPIDLabel] + if !ok { + if !object.DeletionTimestamp.IsZero() && controllerutil.ContainsFinalizer(object, osacPublicIPFeedbackFinalizer) { + log.Info("CR without public IP ID label is being deleted, removing feedback finalizer") + if controllerutil.RemoveFinalizer(object, osacPublicIPFeedbackFinalizer) { + return ctrl.Result{}, r.hubClient.Update(ctx, object) + } + } + log.Info( + "There is no label containing the public IP identifier, will ignore it", + "label", osacPublicIPIDLabel, + ) + return ctrl.Result{}, nil + } + + // Step 3: Fetch the public IP from the fulfillment service so we can compare before/after. + publicIP, err := r.fetchPublicIP(ctx, publicIPID) + if err != nil { + if !object.DeletionTimestamp.IsZero() && errors.Is(err, ErrPublicIPNotFound) { + log.Info("PublicIP record not found during deletion, removing feedback finalizer", "publicIPID", publicIPID) + if controllerutil.RemoveFinalizer(object, osacPublicIPFeedbackFinalizer) { + return ctrl.Result{}, r.hubClient.Update(ctx, object) + } + } + return ctrl.Result{}, err + } + + // Create a task to do the rest of the job, but using copies of the objects, so that we can later compare the + // before and after values and save only the objects that have changed. + t := &publicIPFeedbackReconcilerTask{ + r: r, + object: object, + publicIP: clone(publicIP), + } + + // Step 4: Sync CR state to the fulfillment service record. + if object.DeletionTimestamp.IsZero() { + if err := t.handleUpdate(ctx); err != nil { + return ctrl.Result{}, err + } + } else { + t.handleDelete() + } + + // Step 5: Persist synced state to the fulfillment service. + if err := r.savePublicIP(ctx, publicIP, t.publicIP); err != nil { + return ctrl.Result{}, err + } + + // Step 6: Handle finalizer removal and signal for deletions. + if !object.DeletionTimestamp.IsZero() && controllerutil.ContainsFinalizer(object, osacPublicIPFeedbackFinalizer) { + if len(object.GetFinalizers()) == 1 { + log.Info( + "Feedback finalizer is last remaining, removing finalizer and signaling", + "publicIPID", publicIPID, + ) + if controllerutil.RemoveFinalizer(object, osacPublicIPFeedbackFinalizer) { + if err := r.hubClient.Update(ctx, object); err != nil { + return ctrl.Result{}, err + } + } + _, signalErr := r.publicIPsClient.Signal(ctx, privatev1.PublicIPsSignalRequest_builder{ + Id: publicIPID, + }.Build()) + if signalErr != nil { + log.Error( + signalErr, + "Failed to signal fulfillment service, periodic sync will handle cleanup", + "publicIPID", publicIPID, + ) + } + } else { + log.Info( + "Other finalizers still present, waiting", + "finalizers", object.GetFinalizers(), + ) + } + } + + return ctrl.Result{}, nil +} + +func (r *PublicIPFeedbackReconciler) fetchPublicIP(ctx context.Context, id string) (*privatev1.PublicIP, error) { + response, err := r.publicIPsClient.Get(ctx, privatev1.PublicIPsGetRequest_builder{ + Id: id, + }.Build()) + if err != nil { + if status.Code(err) == codes.NotFound { + return nil, fmt.Errorf("%w: %w", ErrPublicIPNotFound, err) + } + return nil, err + } + publicIP := response.GetObject() + if publicIP == nil { + return nil, fmt.Errorf("%w: response contained nil object", ErrPublicIPNotFound) + } + if !publicIP.HasSpec() { + publicIP.SetSpec(&privatev1.PublicIPSpec{}) + } + if !publicIP.HasStatus() { + publicIP.SetStatus(&privatev1.PublicIPStatus{}) + } + return publicIP, nil +} + +func (r *PublicIPFeedbackReconciler) savePublicIP(ctx context.Context, before, after *privatev1.PublicIP) error { + log := ctrllog.FromContext(ctx) + + if !equal(after, before) { + log.Info( + "Updating public IP", + "before", before, + "after", after, + ) + _, err := r.publicIPsClient.Update(ctx, privatev1.PublicIPsUpdateRequest_builder{ + Object: after, + }.Build()) + if err != nil { + return err + } + } + return nil +} + +func (t *publicIPFeedbackReconcilerTask) handleUpdate(ctx context.Context) error { + if controllerutil.AddFinalizer(t.object, osacPublicIPFeedbackFinalizer) { + if err := t.r.hubClient.Update(ctx, t.object); err != nil { + return err + } + } + t.syncPhase(ctx) + t.syncAddress() + return nil +} + +func (t *publicIPFeedbackReconcilerTask) handleDelete() { + if t.object.Status.Phase == v1alpha1.PublicIPPhaseFailed { + t.publicIP.GetStatus().SetState(privatev1.PublicIPState_PUBLIC_IP_STATE_FAILED) + return + } + t.publicIP.GetStatus().SetState(privatev1.PublicIPState_PUBLIC_IP_STATE_RELEASING) +} + +func (t *publicIPFeedbackReconcilerTask) syncPhase(ctx context.Context) { + switch t.object.Status.Phase { + case v1alpha1.PublicIPPhaseProgressing: + t.syncPhaseProgressing() + case v1alpha1.PublicIPPhaseFailed: + t.syncPhaseFailed() + case v1alpha1.PublicIPPhaseReady: + t.syncPhaseReady() + case v1alpha1.PublicIPPhaseDeleting: + t.syncPhaseDeleting() + default: + log := ctrllog.FromContext(ctx) + log.Info( + "Unknown phase, will ignore it", + "phase", t.object.Status.Phase, + ) + } +} + +func (t *publicIPFeedbackReconcilerTask) syncPhaseProgressing() { + t.publicIP.GetStatus().SetState(privatev1.PublicIPState_PUBLIC_IP_STATE_PENDING) +} + +func (t *publicIPFeedbackReconcilerTask) syncPhaseFailed() { + t.publicIP.GetStatus().SetState(privatev1.PublicIPState_PUBLIC_IP_STATE_FAILED) +} + +func (t *publicIPFeedbackReconcilerTask) syncPhaseReady() { + t.publicIP.GetStatus().SetState(privatev1.PublicIPState_PUBLIC_IP_STATE_ALLOCATED) +} + +func (t *publicIPFeedbackReconcilerTask) syncPhaseDeleting() { + t.publicIP.GetStatus().SetState(privatev1.PublicIPState_PUBLIC_IP_STATE_RELEASING) +} + +func (t *publicIPFeedbackReconcilerTask) syncAddress() { + if t.object.Status.Address != "" { + t.publicIP.GetStatus().SetAddress(t.object.Status.Address) + } +} diff --git a/internal/controller/publicip_feedback_controller_test.go b/internal/controller/publicip_feedback_controller_test.go new file mode 100644 index 00000000..e9146c9f --- /dev/null +++ b/internal/controller/publicip_feedback_controller_test.go @@ -0,0 +1,622 @@ +/* +Copyright 2026. + +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 controller + +import ( + "context" + "errors" + "net" + "sync" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials/insecure" + grpcstatus "google.golang.org/grpc/status" + "google.golang.org/grpc/test/bufconn" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/osac-project/osac-operator/api/v1alpha1" + privatev1 "github.com/osac-project/osac-operator/internal/api/osac/private/v1" +) + +var _ = Describe("PublicIPFeedbackController", func() { + const ( + publicIPName = "test-publicip" + publicIPNamespace = "test-namespace" + publicIPID = "publicip-123" + testPool = "pool-abc" + testAddress = "192.168.1.100" + ) + + var ( + ctx context.Context + k8sClient client.Client + mockServer *mockPublicIPsServer + reconciler *PublicIPFeedbackReconciler + grpcServer *grpc.Server + listener *bufconn.Listener + ) + + BeforeEach(func() { + ctx = context.Background() + + scheme := runtime.NewScheme() + Expect(v1alpha1.AddToScheme(scheme)).To(Succeed()) + k8sClient = fake.NewClientBuilder().WithScheme(scheme).Build() + + mockServer = &mockPublicIPsServer{ + publicIPs: make(map[string]*privatev1.PublicIP), + updates: make([]*privatev1.PublicIP, 0), + signals: make([]string, 0), + } + listener = bufconn.Listen(1024 * 1024) + grpcServer = grpc.NewServer() + privatev1.RegisterPublicIPsServer(grpcServer, mockServer) + + go func() { + _ = grpcServer.Serve(listener) + }() + + conn, err := grpc.NewClient("passthrough:///bufnet", + grpc.WithContextDialer(func(ctx context.Context, s string) (net.Conn, error) { + return listener.Dial() + }), + grpc.WithTransportCredentials(insecure.NewCredentials()), + ) + Expect(err).NotTo(HaveOccurred()) + + reconciler = NewPublicIPFeedbackReconciler(k8sClient, conn, publicIPNamespace) + }) + + AfterEach(func() { + if grpcServer != nil { + grpcServer.Stop() + } + if listener != nil { + _ = listener.Close() + } + }) + + Context("when reconciling a PublicIP CR", func() { + It("should sync Phase=Ready to database state=ALLOCATED", func() { + publicIP := &privatev1.PublicIP{ + Id: publicIPID, + Metadata: &privatev1.Metadata{ + Name: publicIPName, + }, + Spec: &privatev1.PublicIPSpec{ + Pool: testPool, + }, + Status: &privatev1.PublicIPStatus{ + State: privatev1.PublicIPState_PUBLIC_IP_STATE_PENDING, + }, + } + mockServer.addPublicIP(publicIP) + + cr := &v1alpha1.PublicIP{ + ObjectMeta: metav1.ObjectMeta{ + Name: publicIPName, + Namespace: publicIPNamespace, + Labels: map[string]string{ + osacPublicIPIDLabel: publicIPID, + }, + }, + Spec: v1alpha1.PublicIPSpec{ + Pool: testPool, + }, + Status: v1alpha1.PublicIPStatus{ + Phase: v1alpha1.PublicIPPhaseReady, + }, + } + Expect(k8sClient.Create(ctx, cr)).To(Succeed()) + + _, err := reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: publicIPName, + Namespace: publicIPNamespace, + }, + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(mockServer.updates).To(HaveLen(1)) + Expect(mockServer.updates[0].GetStatus().GetState()).To(Equal(privatev1.PublicIPState_PUBLIC_IP_STATE_ALLOCATED)) + + updated := &v1alpha1.PublicIP{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: publicIPName, Namespace: publicIPNamespace}, updated)).To(Succeed()) + Expect(controllerutil.ContainsFinalizer(updated, osacPublicIPFeedbackFinalizer)).To(BeTrue()) + }) + + It("should sync Phase=Progressing to database state=PENDING", func() { + publicIP := &privatev1.PublicIP{ + Id: publicIPID, + Metadata: &privatev1.Metadata{ + Name: publicIPName, + }, + Spec: &privatev1.PublicIPSpec{ + Pool: testPool, + }, + Status: &privatev1.PublicIPStatus{ + State: privatev1.PublicIPState_PUBLIC_IP_STATE_ALLOCATED, + }, + } + mockServer.addPublicIP(publicIP) + + cr := &v1alpha1.PublicIP{ + ObjectMeta: metav1.ObjectMeta{ + Name: publicIPName, + Namespace: publicIPNamespace, + Labels: map[string]string{ + osacPublicIPIDLabel: publicIPID, + }, + }, + Spec: v1alpha1.PublicIPSpec{ + Pool: testPool, + }, + Status: v1alpha1.PublicIPStatus{ + Phase: v1alpha1.PublicIPPhaseProgressing, + }, + } + Expect(k8sClient.Create(ctx, cr)).To(Succeed()) + + _, err := reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: publicIPName, + Namespace: publicIPNamespace, + }, + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(mockServer.updates).To(HaveLen(1)) + Expect(mockServer.updates[0].GetStatus().GetState()).To(Equal(privatev1.PublicIPState_PUBLIC_IP_STATE_PENDING)) + }) + + It("should sync Phase=Failed to database state=FAILED", func() { + publicIP := &privatev1.PublicIP{ + Id: publicIPID, + Metadata: &privatev1.Metadata{ + Name: publicIPName, + }, + Spec: &privatev1.PublicIPSpec{ + Pool: testPool, + }, + Status: &privatev1.PublicIPStatus{ + State: privatev1.PublicIPState_PUBLIC_IP_STATE_PENDING, + }, + } + mockServer.addPublicIP(publicIP) + + cr := &v1alpha1.PublicIP{ + ObjectMeta: metav1.ObjectMeta{ + Name: publicIPName, + Namespace: publicIPNamespace, + Labels: map[string]string{ + osacPublicIPIDLabel: publicIPID, + }, + }, + Spec: v1alpha1.PublicIPSpec{ + Pool: testPool, + }, + Status: v1alpha1.PublicIPStatus{ + Phase: v1alpha1.PublicIPPhaseFailed, + }, + } + Expect(k8sClient.Create(ctx, cr)).To(Succeed()) + + _, err := reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: publicIPName, + Namespace: publicIPNamespace, + }, + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(mockServer.updates).To(HaveLen(1)) + Expect(mockServer.updates[0].GetStatus().GetState()).To(Equal(privatev1.PublicIPState_PUBLIC_IP_STATE_FAILED)) + }) + + It("should sync address to database address field", func() { + publicIP := &privatev1.PublicIP{ + Id: publicIPID, + Metadata: &privatev1.Metadata{ + Name: publicIPName, + }, + Spec: &privatev1.PublicIPSpec{ + Pool: testPool, + }, + Status: &privatev1.PublicIPStatus{ + State: privatev1.PublicIPState_PUBLIC_IP_STATE_PENDING, + }, + } + mockServer.addPublicIP(publicIP) + + cr := &v1alpha1.PublicIP{ + ObjectMeta: metav1.ObjectMeta{ + Name: publicIPName, + Namespace: publicIPNamespace, + Labels: map[string]string{ + osacPublicIPIDLabel: publicIPID, + }, + }, + Spec: v1alpha1.PublicIPSpec{ + Pool: testPool, + }, + Status: v1alpha1.PublicIPStatus{ + Phase: v1alpha1.PublicIPPhaseReady, + Address: testAddress, + }, + } + Expect(k8sClient.Create(ctx, cr)).To(Succeed()) + + _, err := reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: publicIPName, + Namespace: publicIPNamespace, + }, + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(mockServer.updates).To(HaveLen(1)) + Expect(mockServer.updates[0].GetStatus().GetAddress()).To(Equal(testAddress)) + }) + + It("should skip CRs without publicip-uuid label", func() { + cr := &v1alpha1.PublicIP{ + ObjectMeta: metav1.ObjectMeta{ + Name: publicIPName, + Namespace: publicIPNamespace, + Labels: map[string]string{}, + }, + Spec: v1alpha1.PublicIPSpec{ + Pool: testPool, + }, + Status: v1alpha1.PublicIPStatus{ + Phase: v1alpha1.PublicIPPhaseReady, + }, + } + Expect(k8sClient.Create(ctx, cr)).To(Succeed()) + + _, err := reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: publicIPName, + Namespace: publicIPNamespace, + }, + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(mockServer.updates).To(BeEmpty()) + }) + + It("should sync Phase=Deleting to database state=RELEASING during deletion", func() { + publicIP := &privatev1.PublicIP{ + Id: publicIPID, + Metadata: &privatev1.Metadata{ + Name: publicIPName, + }, + Spec: &privatev1.PublicIPSpec{ + Pool: testPool, + }, + Status: &privatev1.PublicIPStatus{ + State: privatev1.PublicIPState_PUBLIC_IP_STATE_ALLOCATED, + }, + } + mockServer.addPublicIP(publicIP) + + cr := &v1alpha1.PublicIP{ + ObjectMeta: metav1.ObjectMeta{ + Name: publicIPName, + Namespace: publicIPNamespace, + Labels: map[string]string{ + osacPublicIPIDLabel: publicIPID, + }, + Finalizers: []string{osacPublicIPFeedbackFinalizer, "osac.openshift.io/publicip-finalizer"}, + }, + Spec: v1alpha1.PublicIPSpec{ + Pool: testPool, + }, + Status: v1alpha1.PublicIPStatus{ + Phase: v1alpha1.PublicIPPhaseDeleting, + }, + } + Expect(k8sClient.Create(ctx, cr)).To(Succeed()) + + Expect(k8sClient.Delete(ctx, cr)).To(Succeed()) + + _, err := reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: publicIPName, + Namespace: publicIPNamespace, + }, + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(mockServer.updates).To(HaveLen(1)) + Expect(mockServer.updates[0].GetStatus().GetState()).To(Equal(privatev1.PublicIPState_PUBLIC_IP_STATE_RELEASING)) + + updated := &v1alpha1.PublicIP{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: publicIPName, Namespace: publicIPNamespace}, updated)).To(Succeed()) + Expect(controllerutil.ContainsFinalizer(updated, osacPublicIPFeedbackFinalizer)).To(BeTrue()) + }) + + It("should sync Phase=Failed during deletion to database state=FAILED", func() { + publicIP := &privatev1.PublicIP{ + Id: publicIPID, + Metadata: &privatev1.Metadata{ + Name: publicIPName, + }, + Spec: &privatev1.PublicIPSpec{ + Pool: testPool, + }, + Status: &privatev1.PublicIPStatus{ + State: privatev1.PublicIPState_PUBLIC_IP_STATE_ALLOCATED, + }, + } + mockServer.addPublicIP(publicIP) + + cr := &v1alpha1.PublicIP{ + ObjectMeta: metav1.ObjectMeta{ + Name: publicIPName, + Namespace: publicIPNamespace, + Labels: map[string]string{ + osacPublicIPIDLabel: publicIPID, + }, + Finalizers: []string{osacPublicIPFeedbackFinalizer, "osac.openshift.io/publicip-finalizer"}, + }, + Spec: v1alpha1.PublicIPSpec{ + Pool: testPool, + }, + Status: v1alpha1.PublicIPStatus{ + Phase: v1alpha1.PublicIPPhaseFailed, + }, + } + Expect(k8sClient.Create(ctx, cr)).To(Succeed()) + + Expect(k8sClient.Delete(ctx, cr)).To(Succeed()) + + _, err := reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: publicIPName, + Namespace: publicIPNamespace, + }, + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(mockServer.updates).To(HaveLen(1)) + Expect(mockServer.updates[0].GetStatus().GetState()).To(Equal(privatev1.PublicIPState_PUBLIC_IP_STATE_FAILED)) + }) + + It("should remove feedback finalizer and signal when it is the last finalizer", func() { + publicIP := &privatev1.PublicIP{ + Id: publicIPID, + Metadata: &privatev1.Metadata{ + Name: publicIPName, + }, + Spec: &privatev1.PublicIPSpec{ + Pool: testPool, + }, + Status: &privatev1.PublicIPStatus{ + State: privatev1.PublicIPState_PUBLIC_IP_STATE_ALLOCATED, + }, + } + mockServer.addPublicIP(publicIP) + + cr := &v1alpha1.PublicIP{ + ObjectMeta: metav1.ObjectMeta{ + Name: publicIPName, + Namespace: publicIPNamespace, + Labels: map[string]string{ + osacPublicIPIDLabel: publicIPID, + }, + Finalizers: []string{osacPublicIPFeedbackFinalizer}, + }, + Spec: v1alpha1.PublicIPSpec{ + Pool: testPool, + }, + Status: v1alpha1.PublicIPStatus{ + Phase: v1alpha1.PublicIPPhaseDeleting, + }, + } + Expect(k8sClient.Create(ctx, cr)).To(Succeed()) + + Expect(k8sClient.Delete(ctx, cr)).To(Succeed()) + + _, err := reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: publicIPName, + Namespace: publicIPNamespace, + }, + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(mockServer.updates).To(HaveLen(1)) + Expect(mockServer.updates[0].GetStatus().GetState()).To(Equal(privatev1.PublicIPState_PUBLIC_IP_STATE_RELEASING)) + + Expect(mockServer.signals).To(HaveLen(1)) + Expect(mockServer.signals[0]).To(Equal(publicIPID)) + + updated := &v1alpha1.PublicIP{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: publicIPName, Namespace: publicIPNamespace}, updated) + Expect(err).To(HaveOccurred()) + }) + + It("should remove feedback finalizer when publicip record is NotFound during deletion", func() { + cr := &v1alpha1.PublicIP{ + ObjectMeta: metav1.ObjectMeta{ + Name: publicIPName, + Namespace: publicIPNamespace, + Labels: map[string]string{ + osacPublicIPIDLabel: publicIPID, + }, + Finalizers: []string{osacPublicIPFeedbackFinalizer}, + }, + Spec: v1alpha1.PublicIPSpec{ + Pool: testPool, + }, + Status: v1alpha1.PublicIPStatus{ + Phase: v1alpha1.PublicIPPhaseDeleting, + }, + } + Expect(k8sClient.Create(ctx, cr)).To(Succeed()) + + Expect(k8sClient.Delete(ctx, cr)).To(Succeed()) + + _, err := reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: publicIPName, + Namespace: publicIPNamespace, + }, + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(mockServer.updates).To(BeEmpty()) + Expect(mockServer.signals).To(BeEmpty()) + + updated := &v1alpha1.PublicIP{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: publicIPName, Namespace: publicIPNamespace}, updated) + Expect(err).To(HaveOccurred()) + }) + + It("should return error when publicip record is NotFound but CR is not being deleted", func() { + cr := &v1alpha1.PublicIP{ + ObjectMeta: metav1.ObjectMeta{ + Name: publicIPName, + Namespace: publicIPNamespace, + Labels: map[string]string{ + osacPublicIPIDLabel: publicIPID, + }, + Finalizers: []string{osacPublicIPFeedbackFinalizer}, + }, + Spec: v1alpha1.PublicIPSpec{ + Pool: testPool, + }, + Status: v1alpha1.PublicIPStatus{ + Phase: v1alpha1.PublicIPPhaseReady, + }, + } + Expect(k8sClient.Create(ctx, cr)).To(Succeed()) + + _, err := reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: publicIPName, + Namespace: publicIPNamespace, + }, + }) + Expect(errors.Is(err, ErrPublicIPNotFound)).To(BeTrue()) + }) + + It("should not update if status unchanged", func() { + publicIP := &privatev1.PublicIP{ + Id: publicIPID, + Metadata: &privatev1.Metadata{ + Name: publicIPName, + }, + Spec: &privatev1.PublicIPSpec{ + Pool: testPool, + }, + Status: &privatev1.PublicIPStatus{ + State: privatev1.PublicIPState_PUBLIC_IP_STATE_ALLOCATED, + }, + } + mockServer.addPublicIP(publicIP) + + cr := &v1alpha1.PublicIP{ + ObjectMeta: metav1.ObjectMeta{ + Name: publicIPName, + Namespace: publicIPNamespace, + Labels: map[string]string{ + osacPublicIPIDLabel: publicIPID, + }, + Finalizers: []string{osacPublicIPFeedbackFinalizer}, + }, + Spec: v1alpha1.PublicIPSpec{ + Pool: testPool, + }, + Status: v1alpha1.PublicIPStatus{ + Phase: v1alpha1.PublicIPPhaseReady, + }, + } + Expect(k8sClient.Create(ctx, cr)).To(Succeed()) + + _, err := reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: publicIPName, + Namespace: publicIPNamespace, + }, + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(mockServer.updates).To(BeEmpty()) + }) + }) +}) + +type mockPublicIPsServer struct { + privatev1.UnimplementedPublicIPsServer + mu sync.Mutex + publicIPs map[string]*privatev1.PublicIP + updates []*privatev1.PublicIP + signals []string +} + +func (m *mockPublicIPsServer) addPublicIP(publicIP *privatev1.PublicIP) { + m.mu.Lock() + defer m.mu.Unlock() + m.publicIPs[publicIP.GetId()] = publicIP +} + +func (m *mockPublicIPsServer) Get(ctx context.Context, req *privatev1.PublicIPsGetRequest) (*privatev1.PublicIPsGetResponse, error) { + m.mu.Lock() + defer m.mu.Unlock() + + publicIP, ok := m.publicIPs[req.GetId()] + if !ok { + return nil, grpcstatus.Errorf(codes.NotFound, "object with identifier '%s' not found", req.GetId()) + } + + return &privatev1.PublicIPsGetResponse{ + Object: publicIP, + }, nil +} + +func (m *mockPublicIPsServer) Update(ctx context.Context, req *privatev1.PublicIPsUpdateRequest) (*privatev1.PublicIPsUpdateResponse, error) { + m.mu.Lock() + defer m.mu.Unlock() + + publicIP := req.GetObject() + m.publicIPs[publicIP.GetId()] = publicIP + m.updates = append(m.updates, publicIP) + + return &privatev1.PublicIPsUpdateResponse{ + Object: publicIP, + }, nil +} + +func (m *mockPublicIPsServer) Signal(ctx context.Context, req *privatev1.PublicIPsSignalRequest) (*privatev1.PublicIPsSignalResponse, error) { + m.mu.Lock() + defer m.mu.Unlock() + + m.signals = append(m.signals, req.GetId()) + + return &privatev1.PublicIPsSignalResponse{}, nil +} diff --git a/internal/controller/publicip_names.go b/internal/controller/publicip_names.go index 55862095..dda72341 100644 --- a/internal/controller/publicip_names.go +++ b/internal/controller/publicip_names.go @@ -17,8 +17,7 @@ import ( "fmt" ) -// Used by the feedback controller (MGMT-23908). var ( - osacPublicIPIDLabel string = fmt.Sprintf("%s/publicip-uuid", osacPrefix) //nolint:unused - osacPublicIPFeedbackFinalizer string = fmt.Sprintf("%s/publicip-feedback", osacPrefix) //nolint:unused + osacPublicIPIDLabel string = fmt.Sprintf("%s/publicip-uuid", osacPrefix) + osacPublicIPFeedbackFinalizer string = fmt.Sprintf("%s/publicip-feedback", osacPrefix) )