test: ensure cleanup of LBs

Author: lukasmetzner

tests/e2e/helper_test.go

@@ -23,6 +23,7 @@ import (
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/clientcmd"
 
+	"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/hcops"
 	"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/testsupport"
 	"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/utils"
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
@@ -112,10 +113,21 @@ func (tc *TestCluster) Stop() error {
 	errs := make([]error, 0, tc.loadBalancers.Size()+tc.certificates.Size())
 	ctx := context.Background()
 
-	for _, item := range tc.loadBalancers.All() {
-		fmt.Printf("deleting load balancer %d\n", item)
-		if _, err := tc.hcloud.LoadBalancer.Delete(ctx, &hcloud.LoadBalancer{ID: item}); err != nil {
-			errs = append(errs, fmt.Errorf("delete load balancer %d failed: %w", item, err))
+	// Leak sweep: any registered Load Balancer still present here means the
+	// hccm finalizer did not release it during namespace teardown. This can
+	// cause issues when deleting the Private Network afterward.
+	for _, id := range tc.loadBalancers.All() {
+		lb, _, err := tc.hcloud.LoadBalancer.GetByID(ctx, id)
+		if err != nil {
+			errs = append(errs, fmt.Errorf("checking load balancer %d for leak: %w", id, err))
+			continue
+		}
+		if lb == nil {
+			continue // released by hccm finalizer, nothing to do
+		}
+		fmt.Printf("force-deleting leaked load balancer %d (%s)\n", id, lb.Name)
+		if _, err := tc.hcloud.LoadBalancer.Delete(ctx, lb); err != nil {
+			errs = append(errs, fmt.Errorf("delete leaked load balancer %d failed: %w", id, err))
 		}
 	}
 
@@ -228,7 +240,7 @@ func (l *lbTestHelper) DeployTestPod() (*corev1.Pod, error) {
 		return nil, fmt.Errorf("could not create test pod: %w", err)
 	}
 
-	err = wait.PollUntilContextTimeout(ctx, 1*time.Second, 1*time.Minute, false, func(ctx context.Context) (done bool, err error) {
+	err = wait.PollUntilContextTimeout(ctx, 1*time.Second, 2*time.Minute, false, func(ctx context.Context) (done bool, err error) {
 		p, err := testCluster.k8sClient.CoreV1().Pods(l.namespace).Get(ctx, podName, metav1.GetOptions{})
 		if err != nil {
 			return false, err
@@ -242,7 +254,7 @@ func (l *lbTestHelper) DeployTestPod() (*corev1.Pod, error) {
 		return false, nil
 	})
 	if err != nil {
-		return nil, fmt.Errorf("pod %s did not come up after 1 minute: %w", podName, err)
+		return nil, fmt.Errorf("pod %s did not come up after 2 minutes: %w", podName, err)
 	}
 
 	return pod, nil
@@ -299,6 +311,9 @@ func (l *lbTestHelper) CreateService(lbSvc *corev1.Service) (*corev1.Service, er
 		}
 
 		if len(svc.Status.LoadBalancer.Ingress) > 0 {
+			if err := testCluster.registerServiceLoadBalancers(ctx, svc); err != nil {
+				return nil, err
+			}
 			return svc, nil
 		}
 
@@ -311,6 +326,24 @@ func (l *lbTestHelper) CreateService(lbSvc *corev1.Service) (*corev1.Service, er
 	}
 }
 
+// registerServiceLoadBalancers looks up the hcloud Load Balancers that hccm
+// created for svc (identified by the service-uid label) and tracks their IDs
+// so TestCluster.Stop can detect finalizer leaks after the suite runs.
+func (tc *TestCluster) registerServiceLoadBalancers(ctx context.Context, svc *corev1.Service) error {
+	lbs, err := tc.hcloud.LoadBalancer.AllWithOpts(ctx, hcloud.LoadBalancerListOpts{
+		ListOpts: hcloud.ListOpts{
+			LabelSelector: fmt.Sprintf("%s=%s", hcops.LabelServiceUID, svc.UID),
+		},
+	})
+	if err != nil {
+		return fmt.Errorf("listing hcloud load balancers for service %s/%s: %w", svc.Namespace, svc.Name, err)
+	}
+	for _, lb := range lbs {
+		tc.loadBalancers.Add(lb.ID)
+	}
+	return nil
+}
+
 // TearDown deletes the created pod and service.
 func (l *lbTestHelper) TearDown() {
 	l.t.Helper()
@@ -323,7 +356,15 @@ func (l *lbTestHelper) TearDown() {
 
 	// Use context.Background() rather than t.Context(): cleanup must run to
 	// completion even when the test has already been cancelled or failed.
-	err := wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 3*time.Minute, true, func(ctx context.Context) (bool, error) {
+	ctx := context.Background()
+
+	// Delete LoadBalancer Services explicitly before the namespace. If the
+	// hccm finalizer is stuck releasing Hetzner resources, the error surfaces
+	// here as an attributable Service-delete timeout instead of a generic
+	// namespace-delete timeout.
+	l.deleteLoadBalancerServices(ctx)
+
+	err := wait.PollUntilContextTimeout(ctx, 1*time.Second, 3*time.Minute, true, func(ctx context.Context) (bool, error) {
 		err := testCluster.k8sClient.CoreV1().Namespaces().Delete(ctx, l.namespace, metav1.DeleteOptions{})
 		if err != nil && !k8serrors.IsNotFound(err) {
 			return false, err
@@ -336,9 +377,41 @@ func (l *lbTestHelper) TearDown() {
 	}
 }
 
+func (l *lbTestHelper) deleteLoadBalancerServices(ctx context.Context) {
+	svcClient := testCluster.k8sClient.CoreV1().Services(l.namespace)
+
+	svcList, err := svcClient.List(ctx, metav1.ListOptions{})
+	if err != nil {
+		if !k8serrors.IsNotFound(err) {
+			l.t.Logf("error listing services in namespace %s: %v", l.namespace, err)
+		}
+		return
+	}
+
+	for _, svc := range svcList.Items {
+		if svc.Spec.Type != corev1.ServiceTypeLoadBalancer {
+			continue
+		}
+		if err := svcClient.Delete(ctx, svc.Name, metav1.DeleteOptions{}); err != nil && !k8serrors.IsNotFound(err) {
+			l.t.Logf("error deleting service %s/%s: %v", l.namespace, svc.Name, err)
+			continue
+		}
+		err := wait.PollUntilContextTimeout(ctx, 1*time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) {
+			_, err := svcClient.Get(ctx, svc.Name, metav1.GetOptions{})
+			if k8serrors.IsNotFound(err) {
+				return true, nil
+			}
+			return false, err
+		})
+		if err != nil {
+			l.t.Logf("service %s/%s did not delete within 2m (hccm finalizer may be stuck): %v", l.namespace, svc.Name, err)
+		}
+	}
+}
+
 // WaitForHTTPAvailable tries to connect to the given IP via HTTP or HTTPS
 // (controlled by useHTTPS). It uses exponential backoff starting at 1s and
-// capping at 30s, waiting up to 6 minutes for a successful HTTP 200 response.
+// capping at 30s, waiting up to 8 minutes for a successful HTTP 200 response.
 // Each individual request has a 5s timeout.
 func (l *lbTestHelper) WaitForHTTPAvailable(ingressIP string, useHTTPS bool) error {
 	l.t.Helper()
@@ -356,7 +429,7 @@ func (l *lbTestHelper) WaitForHTTPAvailable(ingressIP string, useHTTPS bool) err
 		proto = "https"
 	}
 
-	ctx, cancel := context.WithTimeout(l.t.Context(), 6*time.Minute)
+	ctx, cancel := context.WithTimeout(l.t.Context(), 8*time.Minute)
 	defer cancel()
 
 	retries := 0
@@ -378,7 +451,7 @@ func (l *lbTestHelper) WaitForHTTPAvailable(ingressIP string, useHTTPS bool) err
 
 		select {
 		case <-ctx.Done():
-			return fmt.Errorf("timed out after 6m waiting for %s to be available", ingressIP)
+			return fmt.Errorf("timed out after 8m waiting for %s to be available", ingressIP)
 		case <-time.After(pollBackoff(retries)):
 			retries++
 		}