From e23ee7db03f2bc9da4d6eba25999d62fa92f9104 Mon Sep 17 00:00:00 2001 From: Stephen Kitt Date: Tue, 24 Oct 2023 14:25:49 +0200 Subject: [PATCH] Reduce string constant duplication Newer versions of goconst detect more instances of string duplication, in particular ".svc." in Lighthouse. Address that by providing a framework function to construct a service's DNS name based on its components. A few string constants aren't actually shared, remove them to improve code clarity. Signed-off-by: Stephen Kitt --- test/e2e/discovery/headless_services.go | 11 +++-------- test/e2e/discovery/service_discovery.go | 13 ++++--------- test/e2e/discovery/statefulsets.go | 2 ++ test/e2e/framework/framework.go | 17 +++++++++++------ 4 files changed, 20 insertions(+), 23 deletions(-) diff --git a/test/e2e/discovery/headless_services.go b/test/e2e/discovery/headless_services.go index f742cc0d8..62b417de0 100644 --- a/test/e2e/discovery/headless_services.go +++ b/test/e2e/discovery/headless_services.go @@ -30,11 +30,6 @@ import ( corev1 "k8s.io/api/core/v1" ) -const ( - httpPortName = "http" - opAre = "are" -) - var _ = Describe("Test Headless Service Discovery Across Clusters", Label(TestLabel), func() { f := lhframework.NewFramework("discovery") @@ -331,10 +326,10 @@ func verifyHeadlessSRVRecordsWithDig(f *framework.Framework, cluster framework.C for j := range ports { port := &ports[j] cmd, domainName := createSRVQuery(f, port, service, domains[i], clusterName, withPort, withcluster) - op := opAre + op := "are" if !shouldContain { - op += not + op += " not" } By(fmt.Sprintf("Executing %q to verify hostNames %v for service %q %q discoverable", @@ -384,7 +379,7 @@ func createSRVQuery(f *framework.Framework, port *corev1.ServicePort, service *c ) (cmd []string, domainName string) { cmd = []string{"dig", "+short", "SRV"} - domainName = service.Name + "." + f.Namespace + ".svc." + domain + domainName = lhframework.BuildServiceDNSName("", service.Name, f.Namespace, domain) clusterDNSName := domainName if withcluster { diff --git a/test/e2e/discovery/service_discovery.go b/test/e2e/discovery/service_discovery.go index d983c4325..09a52f3cb 100644 --- a/test/e2e/discovery/service_discovery.go +++ b/test/e2e/discovery/service_discovery.go @@ -34,7 +34,6 @@ import ( ) const ( - not = " not" TestLabel = "service-discovery" ) @@ -557,7 +556,7 @@ func RunServicesClusterAvailabilityMultiClusterTest(f *lhframework.Framework) { checkedDomains, "", false) } -//nolint:gocognit,unparam // This really isn't that complex and would be awkward to refactor. +//nolint:unparam // Keep srcCluster as a parameter for consistency and possible future extensions func verifySRVWithDig(f *framework.Framework, srcCluster framework.ClusterIndex, service *corev1.Service, targetPod *corev1.PodList, domains []string, clusterName string, withPort, shouldContain bool, ) { @@ -566,11 +565,7 @@ func verifySRVWithDig(f *framework.Framework, srcCluster framework.ClusterIndex, for _, port := range ports { cmd := []string{"dig", "+short", "SRV"} - clusterDNSName := service.Name + "." + f.Namespace + ".svc." + domains[i] - - if clusterName != "" { - clusterDNSName = clusterName + "." + clusterDNSName - } + clusterDNSName := lhframework.BuildServiceDNSName(clusterName, service.Name, f.Namespace, domains[i]) portName := clusterDNSName @@ -582,7 +577,7 @@ func verifySRVWithDig(f *framework.Framework, srcCluster framework.ClusterIndex, op := "is" if !shouldContain { - op += not + op += " not" } By(fmt.Sprintf("Executing %q to verify SRV record for service %q %q discoverable", strings.Join(cmd, " "), @@ -632,7 +627,7 @@ func verifyRoundRobinWithDig(f *framework.Framework, srcCluster framework.Cluste cmd := []string{"dig", "+short"} for i := range domains { - cmd = append(cmd, serviceName+"."+f.Namespace+".svc."+domains[i]) + cmd = append(cmd, lhframework.BuildServiceDNSName("", serviceName, f.Namespace, domains[i])) } serviceIPMap := make(map[string]int) diff --git a/test/e2e/discovery/statefulsets.go b/test/e2e/discovery/statefulsets.go index 225245efc..38edea72a 100644 --- a/test/e2e/discovery/statefulsets.go +++ b/test/e2e/discovery/statefulsets.go @@ -30,6 +30,8 @@ import ( discovery "k8s.io/api/discovery/v1" ) +const httpPortName = "http" + var _ = Describe("Test Stateful Sets Discovery Across Clusters", Label(TestLabel), func() { f := lhframework.NewFramework("discovery") diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index 35a07f969..ea0b158b6 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -629,18 +629,23 @@ func (f *Framework) VerifyServiceIPWithDig(srcCluster, targetCluster framework.C f.VerifyIPWithDig(srcCluster, service, targetPod, domains, clusterName, serviceIP, shouldContain) } +func BuildServiceDNSName(clusterName, serviceName, namespace, tld string) string { + name := "" + + if clusterName != "" { + name = clusterName + "." + } + + return name + serviceName + "." + namespace + ".svc." + tld +} + func (f *Framework) VerifyIPWithDig(srcCluster framework.ClusterIndex, service *v1.Service, targetPod *v1.PodList, domains []string, clusterName, serviceIP string, shouldContain bool, ) { cmd := []string{"dig", "+short"} - var clusterDNSName string - if clusterName != "" { - clusterDNSName = clusterName + "." - } - for i := range domains { - cmd = append(cmd, clusterDNSName+service.Name+"."+f.Namespace+".svc."+domains[i]) + cmd = append(cmd, BuildServiceDNSName(clusterName, service.Name, f.Namespace, domains[i])) } op := "is"