Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore broker EndplointSlices in the resolver controller #1457

Merged
merged 2 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions coredns/resolver/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions coredns/resolver/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
12 changes: 7 additions & 5 deletions coredns/resolver/resolver_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ limitations under the License.
package resolver_test

import (
"context"
"flag"
"fmt"
"reflect"
Expand Down Expand Up @@ -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
}
Expand Down
8 changes: 4 additions & 4 deletions test/e2e/framework/framework.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
Loading