From 279b24d9b023a3d0da1de773c0ed5be0c7ffe91b Mon Sep 17 00:00:00 2001 From: Vishesh Tanksale Date: Mon, 16 Sep 2024 22:33:36 +0000 Subject: [PATCH] Deleting HPA object when NIMservice is updated Signed-off-by: Vishesh Tanksale --- .../platform/standalone/nimservice.go | 42 +++++++++++ .../platform/standalone/nimservice_test.go | 72 +++++++++++++++++++ 2 files changed, 114 insertions(+) diff --git a/internal/controller/platform/standalone/nimservice.go b/internal/controller/platform/standalone/nimservice.go index d564f3ab..73e363fc 100644 --- a/internal/controller/platform/standalone/nimservice.go +++ b/internal/controller/platform/standalone/nimservice.go @@ -121,6 +121,11 @@ func (r *NIMServiceReconciler) reconcileNIMService(ctx context.Context, nimServi if err != nil { return ctrl.Result{}, err } + } else { + err = r.cleanupResource(ctx, &networkingv1.Ingress{}, namespacedName) + if err != nil && !errors.IsNotFound(err) { + return ctrl.Result{}, err + } } // Sync HPA @@ -131,6 +136,12 @@ func (r *NIMServiceReconciler) reconcileNIMService(ctx context.Context, nimServi if err != nil { return ctrl.Result{}, err } + } else { + // If autoscaling is disabled, ensure the HPA is deleted + err = r.cleanupResource(ctx, &autoscalingv2.HorizontalPodAutoscaler{}, namespacedName) + if err != nil { + return ctrl.Result{}, err + } } // Sync Service Monitor @@ -342,6 +353,37 @@ func (r *NIMServiceReconciler) syncResource(ctx context.Context, obj client.Obje return nil } +// cleanupResource deletes the given Kubernetes resource if it exists. +// If the resource does not exist or an error occurs during deletion, the function returns nil or the error. +// +// Parameters: +// ctx (context.Context): The context for the operation. +// obj (client.Object): The Kubernetes resource to delete. +// namespacedName (types.NamespacedName): The namespaced name of the resource. +// +// Returns: +// error: An error if the resource deletion fails, or nil if the resource is not found or deletion is successful. +func (r *NIMServiceReconciler) cleanupResource(ctx context.Context, obj client.Object, namespacedName types.NamespacedName) error { + + logger := log.FromContext(ctx) + + err := r.Get(ctx, namespacedName, obj) + if err != nil && !errors.IsNotFound(err) { + return err + } + + if errors.IsNotFound(err) { + return nil + } + + err = r.Delete(ctx, obj) + if err != nil { + return err + } + logger.V(2).Info("NIM Service object changed, deleting ", "obj", obj) + return nil +} + // getNIMCachePVC returns PVC backing the NIM cache instance func (r *NIMServiceReconciler) getNIMCachePVC(ctx context.Context, nimService *appsv1alpha1.NIMService) (*appsv1alpha1.PersistentVolumeClaim, error) { logger := log.FromContext(ctx) diff --git a/internal/controller/platform/standalone/nimservice_test.go b/internal/controller/platform/standalone/nimservice_test.go index 8245c75c..e1815a0c 100644 --- a/internal/controller/platform/standalone/nimservice_test.go +++ b/internal/controller/platform/standalone/nimservice_test.go @@ -37,6 +37,7 @@ import ( corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -174,6 +175,36 @@ var _ = Describe("NIMServiceReconciler for a standalone platform", func() { Effect: corev1.TaintEffectNoSchedule, }, }, + + Expose: appsv1alpha1.Expose{ + Ingress: appsv1alpha1.Ingress{ + Enabled: ptr.To[bool](true), + Spec: networkingv1.IngressSpec{ + Rules: []networkingv1.IngressRule{ + { + Host: "test-nimservice.default.example.com", + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{ + { + Path: "/", + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "test-nimservice", + Port: networkingv1.ServiceBackendPort{ + Number: 8080, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, Scale: appsv1alpha1.Autoscaling{ Enabled: ptr.To[bool](true), HPA: appsv1alpha1.HorizontalPodAutoscalerSpec{ @@ -405,6 +436,46 @@ var _ = Describe("NIMServiceReconciler for a standalone platform", func() { err = reconciler.cleanupNIMService(context.TODO(), nimService) Expect(err).NotTo(HaveOccurred()) }) + + It("should delete HPA when NIMService is updated", func() { + namespacedName := types.NamespacedName{Name: nimService.Name, Namespace: nimService.Namespace} + err := client.Create(context.TODO(), nimService) + Expect(err).NotTo(HaveOccurred()) + + result, err := reconciler.reconcileNIMService(context.TODO(), nimService) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + + // HPA should be deployed + hpa := &autoscalingv2.HorizontalPodAutoscaler{} + err = client.Get(context.TODO(), namespacedName, hpa) + Expect(err).NotTo(HaveOccurred()) + Expect(hpa.Name).To(Equal(nimService.GetName())) + Expect(hpa.Namespace).To(Equal(nimService.GetNamespace())) + Expect(*hpa.Spec.MinReplicas).To(Equal(int32(1))) + Expect(hpa.Spec.MaxReplicas).To(Equal(int32(10))) + + nimService := &appsv1alpha1.NIMService{} + err = client.Get(context.TODO(), namespacedName, nimService) + Expect(err).NotTo(HaveOccurred()) + nimService.Spec.Scale.Enabled = ptr.To[bool](false) + nimService.Spec.Expose.Ingress.Enabled = ptr.To[bool](false) + err = client.Update(context.TODO(), nimService) + Expect(err).NotTo(HaveOccurred()) + + result, err = reconciler.reconcileNIMService(context.TODO(), nimService) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + hpa = &autoscalingv2.HorizontalPodAutoscaler{} + err = client.Get(context.TODO(), namespacedName, hpa) + Expect(err).To(HaveOccurred()) + Expect(errors.IsNotFound(err)).To(Equal(true)) + ingress := &networkingv1.Ingress{} + err = client.Get(context.TODO(), namespacedName, ingress) + Expect(err).To(HaveOccurred()) + Expect(errors.IsNotFound(err)).To(Equal(true)) + }) + }) Describe("isDeploymentReady for setting status on NIMService", func() { @@ -525,4 +596,5 @@ var _ = Describe("NIMServiceReconciler for a standalone platform", func() { Expect(msg).To(Equal(fmt.Sprintf("deployment %q successfully rolled out\n", deployment.Name))) }) }) + })