Clean up HPA when necessary

Author: sjberman

internal/controller/provisioner/provisioner.go

@@ -12,6 +12,7 @@ import (
 	"golang.org/x/text/cases"
 	"golang.org/x/text/language"
 	appsv1 "k8s.io/api/apps/v1"
+	autoscalingv2 "k8s.io/api/autoscaling/v2"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -487,6 +488,7 @@ func (p *NginxProvisioner) RegisterGateway(
 		}
 
 		// If NGINX deployment type switched between Deployment and DaemonSet, clean up the old one.
+		// If HPA was disabled, remove it.
 		nginxResources := p.store.getNginxResourcesForGateway(gatewayNSName)
 		if nginxResources != nil {
 			if needToDeleteDaemonSet(nginxResources) {
@@ -498,6 +500,12 @@ func (p *NginxProvisioner) RegisterGateway(
 					p.cfg.Logger.Error(err, "error deleting nginx resource")
 				}
 			}
+
+			if needToDeleteHPA(nginxResources) {
+				if err := p.deleteObject(ctx, &autoscalingv2.HorizontalPodAutoscaler{ObjectMeta: nginxResources.HPA}); err != nil {
+					p.cfg.Logger.Error(err, "error deleting nginx resource")
+				}
+			}
 		}
 
 		if err := p.provisionNginx(ctx, resourceName, gateway.Source, objects); err != nil {
@@ -539,3 +547,18 @@ func needToDeleteDaemonSet(cfg *NginxResources) bool {
 
 	return false
 }
+
+func needToDeleteHPA(cfg *NginxResources) bool {
+	if cfg.HPA.Name != "" && cfg.Gateway != nil {
+		if cfg.Gateway.EffectiveNginxProxy != nil &&
+			cfg.Gateway.EffectiveNginxProxy.Kubernetes != nil &&
+			!isAutoscalingEnabled(cfg.Gateway.EffectiveNginxProxy.Kubernetes.Deployment) {
+			return true
+		} else if cfg.Gateway.EffectiveNginxProxy == nil ||
+			cfg.Gateway.EffectiveNginxProxy.Kubernetes == nil {
+			return true
+		}
+	}
+
+	return false
+}

internal/controller/provisioner/provisioner_test.go

@@ -422,6 +422,54 @@ func TestRegisterGateway_CleansUpOldDeploymentOrDaemonSet(t *testing.T) {
 	g.Expect(err).ToNot(HaveOccurred())
 }
 
+func TestRegisterGateway_CleansUpOldHPA(t *testing.T) {
+	t.Parallel()
+	g := NewWithT(t)
+
+	// Setup: Gateway previously referenced an HPA, but now does not
+	// Previous state: HPA exists and is tracked
+	oldHPA := &autoscalingv2.HorizontalPodAutoscaler{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "gw-nginx",
+			Namespace: "default",
+		},
+	}
+	gateway := &graph.Gateway{
+		Source: &gatewayv1.Gateway{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "gw",
+				Namespace: "default",
+			},
+		},
+		Valid: true,
+		EffectiveNginxProxy: &graph.EffectiveNginxProxy{
+			Kubernetes: &ngfAPIv1alpha2.KubernetesSpec{
+				Deployment: &ngfAPIv1alpha2.DeploymentSpec{
+					Autoscaling: &ngfAPIv1alpha2.HPASpec{
+						Enable: false,
+					},
+				},
+			},
+		},
+	}
+
+	provisioner, fakeClient, _ := defaultNginxProvisioner(gateway.Source, oldHPA)
+	provisioner.store.nginxResources[types.NamespacedName{Name: "gw", Namespace: "default"}] = &NginxResources{
+		HPA: oldHPA.ObjectMeta,
+	}
+
+	// Simulate update: EffectiveNginxProxy no longer references HPA
+	g.Expect(provisioner.RegisterGateway(t.Context(), gateway, "gw-nginx")).To(Succeed())
+
+	// HPA should be deleted
+	hpaErr := fakeClient.Get(
+		t.Context(),
+		types.NamespacedName{Name: "gw-nginx", Namespace: "default"},
+		&autoscalingv2.HorizontalPodAutoscaler{},
+	)
+	g.Expect(hpaErr).To(HaveOccurred())
+}
+
 func TestNonLeaderProvisioner(t *testing.T) {
 	t.Parallel()
 	g := NewWithT(t)