From c221f3f27b6ba15f2da671e4a10aaa7d4d11424c Mon Sep 17 00:00:00 2001
From: Danilo Hashimoto <97529980+hash-d@users.noreply.github.com>
Date: Thu, 23 May 2024 13:29:09 -0300
Subject: [PATCH] Assorted fixes on -use-cluster tests (#1391)

- Use routes as ingress, when available
- Ignore unexpected additional clusterroles, when running as -use-cluster
- Shutdown previous informers between iterations
- Ignore additional cm openshift-service-ca.crt, along with kube-root-ca.crt
- Cherry pick Fernando's
- Router Create test improvement #1386 into 1.5
---
 client/router_create_test.go           | 18 +++++++++++++++---
 client/serviceinterface_create_test.go |  6 +++++-
 pkg/domain/podman/link_test.go         | 25 +++++++++++++++++++++++++
 pkg/domain/podman/main_test.go         |  6 ++++++
 4 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/client/router_create_test.go b/client/router_create_test.go
index 8b2c705ac..01e4465aa 100644
--- a/client/router_create_test.go
+++ b/client/router_create_test.go
@@ -345,14 +345,23 @@ func TestRouterCreateDefaults(t *testing.T) {
 				rolesFound = append(rolesFound, role.Name)
 			},
 		})
+
+		expectedClusterRoles := sets.NewString(c.clusterRolesExpected...)
 		clusterRoleInformer := clusterRoleInformerFactory.Rbac().V1().ClusterRoles().Informer()
 		clusterRoleInformer.AddEventHandler(&cache.ResourceEventHandlerFuncs{
 			AddFunc: func(obj interface{}) {
 				clusterRole := obj.(*rbacv1.ClusterRole)
 				if strings.HasPrefix(clusterRole.Name, "skupper") {
-					clusterRolesFound = append(clusterRolesFound, clusterRole.Name)
-					for _, p := range clusterRole.Rules {
-						clusterRolesResourcesFound = clusterRolesResourcesFound.Insert(p.Resources...)
+					if isCluster && !expectedClusterRoles.Has(clusterRole.Name) {
+						// A real cluster may have pre-existing clusterroles that would
+						// make this test flaky, so we ignore clusterroles not listed
+						// on the test.
+						fmt.Printf("clusterrole %q ignored due to -use-cluster\n", clusterRole.Name)
+					} else {
+						clusterRolesFound = append(clusterRolesFound, clusterRole.Name)
+						for _, p := range clusterRole.Rules {
+							clusterRolesResourcesFound = clusterRolesResourcesFound.Insert(p.Resources...)
+						}
 					}
 				}
 			},
@@ -476,6 +485,9 @@ func TestRouterCreateDefaults(t *testing.T) {
 		if diff := cmp.Diff(c.secretsExpected, secretsFound, c.opts...); diff != "" {
 			t.Errorf("TestRouterCreateDefaults "+c.doc+" secrets mismatch (-want +got):\n%s", diff)
 		}
+
+		// Close informers
+		cancel()
 	}
 }
 
diff --git a/client/serviceinterface_create_test.go b/client/serviceinterface_create_test.go
index fe9295930..8b5025230 100644
--- a/client/serviceinterface_create_test.go
+++ b/client/serviceinterface_create_test.go
@@ -32,6 +32,9 @@ func check_result(t *testing.T, name string, timeoutSeconds float64, resultType
 	if len(expected) <= 0 {
 		return
 	}
+
+	fmt.Printf("Checking %q results for test %q on %q\n", resultType, doc, name)
+
 	// Sometimes it requires a little time for the requested entities to be
 	// created and for the informers to tell us about them.
 	// So -- count down by tenths of a second until the allotted timeout expires,
@@ -227,7 +230,8 @@ func TestServiceInterfaceCreate(t *testing.T) {
 		cmInformer.AddEventHandler(&cache.ResourceEventHandlerFuncs{
 			AddFunc: func(obj interface{}) {
 				cm := obj.(*corev1.ConfigMap)
-				if cm.Name != "kube-root-ca.crt" { // seems to be something added in more recent kubernetes?
+				if cm.Name != "kube-root-ca.crt" && // auto-created, introduced in K8S 1.20
+					cm.Name != "openshift-service-ca.crt" { // auto-created, OCP 4.7
 					cmsFound = append(cmsFound, cm.Name)
 				}
 			},
diff --git a/pkg/domain/podman/link_test.go b/pkg/domain/podman/link_test.go
index 08ead78cb..73f737090 100644
--- a/pkg/domain/podman/link_test.go
+++ b/pkg/domain/podman/link_test.go
@@ -6,9 +6,13 @@ package podman
 import (
 	"context"
 	"fmt"
+	"log"
+	"net"
+	"net/url"
 	"testing"
 	"time"
 
+	"github.com/skupperproject/skupper/pkg/utils"
 	"gotest.tools/assert"
 	corev1 "k8s.io/api/core/v1"
 )
@@ -42,6 +46,27 @@ func TestLinkHandlerPodman(t *testing.T) {
 			}
 			assert.Assert(t, err)
 			assert.Assert(t, token != nil)
+
+			// On some clouds, it may take a while for the service DNS name to be externally
+			// resolvable.  So, we extract that URL and wait for the name resolution to work
+			// before creating the link.  If anything fails, we just log and keep going, as
+			// that's not the focus of the test; the whole thing  may fail down the road,
+			// but with additional information for debugging.
+			skupperUrl := token.Annotations["skupper.io/url"]
+			if skupperUrl != "" {
+				parsed, err := url.Parse(skupperUrl)
+				if err != nil {
+					log.Printf("The skupper.io/url annotation did not parse as an URL (%q): %v", skupperUrl, err)
+				} else {
+					err = utils.RetryError(time.Second*2, 60, func() error {
+						_, err := net.ResolveIPAddr("ip", parsed.Hostname())
+						return err
+					})
+					if err != nil {
+						log.Printf("Name resolution for skupper.io/url (%q) still failing after 2 minutes: %v", parsed.Hostname(), err)
+					}
+				}
+			}
 			err = linkHandler.Create(token, linkName, 2)
 			assert.Assert(t, err)
 		})
diff --git a/pkg/domain/podman/main_test.go b/pkg/domain/podman/main_test.go
index 7ada9a352..8333d62a1 100644
--- a/pkg/domain/podman/main_test.go
+++ b/pkg/domain/podman/main_test.go
@@ -110,10 +110,16 @@ func teardownKube() {
 }
 
 func configureSiteAndCreateRouter(ctx context.Context, cli *client.VanClient, name string) error {
+	var ingressType string
+	if cli.GetRouteClient() != nil {
+		// Like the product, we default to routes on OpenShift
+		ingressType = types.IngressRouteString
+	}
 	routerCreateOpts := types.SiteConfigSpec{
 		SkupperName:      "skupper",
 		RouterMode:       string(types.TransportModeInterior),
 		EnableController: true,
+		Ingress:          ingressType,
 	}
 	siteConfig, err := cli.SiteConfigCreate(ctx, routerCreateOpts)
 	if err != nil {