Deleting HPA object when NIMservice is updated (#143)

Signed-off-by: Vishesh Tanksale <vtanksale@nvidia.com>

internal/controller/platform/standalone/nimservice.go

@@ -123,6 +123,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
@@ -133,6 +138,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
@@ -356,6 +367,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)

internal/controller/platform/standalone/nimservice_test.go

@@ -38,6 +38,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"
 	apiResource "k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -178,6 +179,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{
@@ -421,6 +452,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() {