From 37e335d05ce6e04f449539e0bec848e00f4f0c5e Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Tue, 12 Dec 2023 14:31:29 -0500 Subject: [PATCH 1/2] Change VerifyIPsWithDig to expect one occurrence of each IP Signed-off-by: Tom Pantelis --- test/e2e/framework/framework.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index ea0b158b6..b93d48f7c 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -723,13 +723,13 @@ func (f *Framework) VerifyIPsWithDig(cluster framework.ClusterIndex, service *v1 return false, fmt.Sprintf("expected execution result %q to be empty", result), nil } for _, ip := range ipList { - doesContain := strings.Contains(result.(string), ip) - if doesContain && !shouldContain { + count := strings.Count(result.(string), ip) + if count > 0 && !shouldContain { return false, fmt.Sprintf("expected execution result %q not to contain %q", result, ip), nil } - if !doesContain && shouldContain { - return false, fmt.Sprintf("expected execution result %q to contain %q", result, ip), nil + if count != 1 && shouldContain { + return false, fmt.Sprintf("expected execution result %q to contain one occurrence of %q", result, ip), nil } } From dbe4b88d1d6325a48094863c50dd6190316c0376 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Wed, 13 Dec 2023 08:11:25 -0500 Subject: [PATCH 2/2] Ignore broker EndplointSlices in the resolver controller ...in getAllEndpointSlices to avoid duplicate DNS records being added. This has also caused failures in the E2E test case that removes all headless services pod replicas - it expects no DNS records returned but residual addresses on the broker EndplointSlice can remain. Signed-off-by: Tom Pantelis --- coredns/resolver/controller.go | 9 ++++++--- coredns/resolver/controller_test.go | 4 ++++ coredns/resolver/resolver_suite_test.go | 12 +++++++----- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/coredns/resolver/controller.go b/coredns/resolver/controller.go index 35e8f7dae..c30dae6f2 100644 --- a/coredns/resolver/controller.go +++ b/coredns/resolver/controller.go @@ -120,7 +120,7 @@ func (c *controller) getAllEndpointSlices(forEPS *discovery.EndpointSlice) []*di var epSlices []*discovery.EndpointSlice for i := range list { eps := list[i].(*discovery.EndpointSlice) - if !isLegacyEndpointSlice(eps) { + if !isOnBroker(eps) && !isLegacyEndpointSlice(eps) { epSlices = append(epSlices, eps) } } @@ -157,8 +157,11 @@ func (c *controller) onServiceImportDelete(obj runtime.Object, _ int) bool { } func (c *controller) ignoreEndpointSlice(eps *discovery.EndpointSlice) bool { - isOnBroker := eps.Namespace != eps.Labels[constants.LabelSourceNamespace] - return isOnBroker || (isLegacyEndpointSlice(eps) && len(c.getAllEndpointSlices(eps)) > 0) + return isOnBroker(eps) || (isLegacyEndpointSlice(eps) && len(c.getAllEndpointSlices(eps)) > 0) +} + +func isOnBroker(eps *discovery.EndpointSlice) bool { + return eps.Namespace != eps.Labels[constants.LabelSourceNamespace] } func isLegacyEndpointSlice(eps *discovery.EndpointSlice) bool { diff --git a/coredns/resolver/controller_test.go b/coredns/resolver/controller_test.go index 2399ec69a..2514ede90 100644 --- a/coredns/resolver/controller_test.go +++ b/coredns/resolver/controller_test.go @@ -134,6 +134,10 @@ var _ = Describe("Controller", func() { ) epsName2 = eps.Name t.createEndpointSlice(eps) + + epsOnBroker := eps.DeepCopy() + epsOnBroker.Namespace = test.RemoteNamespace + t.createEndpointSlice(epsOnBroker) }) Specify("GetDNSRecords should return their DNS record", func() { diff --git a/coredns/resolver/resolver_suite_test.go b/coredns/resolver/resolver_suite_test.go index 249ba5ee6..5cbebfa3a 100644 --- a/coredns/resolver/resolver_suite_test.go +++ b/coredns/resolver/resolver_suite_test.go @@ -19,6 +19,7 @@ limitations under the License. package resolver_test import ( + "context" "flag" "fmt" "reflect" @@ -175,12 +176,13 @@ func (t *testDriver) awaitDNSRecordsFound(ns, name, cluster, hostname string, ex var records []resolver.DNSRecord var found, isHeadless bool - err := wait.PollImmediate(50*time.Millisecond, 5*time.Second, func() (bool, error) { - records, isHeadless, found = t.resolver.GetDNSRecords(ns, name, cluster, hostname) - sortRecords(records) + err := wait.PollUntilContextTimeout(context.Background(), 50*time.Millisecond, 5*time.Second, true, + func(_ context.Context) (bool, error) { + records, isHeadless, found = t.resolver.GetDNSRecords(ns, name, cluster, hostname) + sortRecords(records) - return found && isHeadless == expIsHeadless && reflect.DeepEqual(records, expRecords), nil - }) + return found && isHeadless == expIsHeadless && reflect.DeepEqual(records, expRecords), nil + }) if err == nil { return }