OCPBUGS-647: SDN branch: Fix Prefer local endpoints for cluster DNS service

[martinkennelly]

Carry "Prefer local endpoint for cluster DNS service"
(54dc363) was incorrectly applied which did not restrict
DNS calls to a local endpoint.

Signed-off-by: Martin Kennelly mkennell@redhat.com

/cc @danwinship

[openshift-ci-robot]

@martinkennelly: the contents of this pull request could not be automatically validated.

The following commits could not be validated and must be approved by a top-level approver:

• 3b57265|Fix Prefer local endpoints for cluster DNS service: does not specify an upstream backport in the commit message

Comment /validate-backports to re-evaluate validity of the upstream PRs, for example when they are merged upstream.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: martinkennelly
Once this PR has been reviewed and has the lgtm label, please assign knobunc for approval by writing /assign @knobunc in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• DOWNSTREAM_OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[danwinship]

I added a unit test for this in 4.11, 45738de#diff-92f41de420a097c0419a3cb53cf176a459a742c39f3fa482ab48b1c2fa6fd084. Can you add that here and confirm that it now passes? (go test -mod=vendor ./pkg/proxy/iptables). (It will need a little tweaking: the expected rules would be different in 4.10. But you can confirm that the generated rules in the first test use only the local endpoint and the rules in the second test use all endpoints.)

(Yeah, it's a bit late to be adding the test now, but this way, at least if we need to modify this code again in 4.10, we'll have the test there to confirm it. Maybe grab the local-with-fallback unit test from 16c6939 too?)

[openshift-ci-robot]

@martinkennelly: the contents of this pull request could not be automatically validated.

The following commits could not be validated and must be approved by a top-level approver:

• 3b57265|Fix Prefer local endpoints for cluster DNS service: does not specify an upstream backport in the commit message
• cdcd8b3|Add test case for OCP hack prefer local DNS endpoint: does not specify an upstream backport in the commit message
• e50bb74|Add test case for OCP hack 'local-with-fallback': does not specify an upstream backport in the commit message

Comment /validate-backports to re-evaluate validity of the upstream PRs, for example when they are merged upstream.

[martinkennelly]

But you can confirm that the generated rules in the first test use only the local endpoint and the rules in the second test use all endpoints.)

How do you want me to confirm? unit test in SDN repo? I was planning to do this once I vendor this in.
I have confirmed with manual testing but Id like to write a unit test.
I see OVN-K already has a unit test.

[danwinship]

How do you want me to confirm? unit test in SDN repo?

No, I meant you, personally, can just look at the rules, and say "yup, those are the expected rules".

It's awkward to have unit tests of this because (1) "go mod vendor" won't copy the unit tests over to sdn, (2) unit tests in sdn's pkg/network/ can't access the private methods in the vendored packages, and (3) "make verify-deps" in sdn won't let us manually add additional unit tests under vendor/. So basically we just depend on having unit tests in o/k and running them by hand.

[openshift-ci-robot]

@martinkennelly: This pull request references Jira Issue OCPBUGS-647, which is invalid:

• expected Jira Issue OCPBUGS-647 to depend on a bug in one of the following states: VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), but no dependents were found

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Carry "Prefer local endpoint for cluster DNS service"
(54dc363) was incorrectly applied which did not restrict
DNS calls to a local endpoint.

Signed-off-by: Martin Kennelly mkennell@redhat.com

/cc @danwinship

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[danwinship]

This looks good. Can you open a WIP PR against SDN, vendoring this commit in via your own fork (eg, like this commit) and then once it passes SDN CI, I'll merge this PR and you can update the SDN PR to use the real o/k commit.

[openshift-ci-robot]

@martinkennelly: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-647 has been moved to the MODIFIED state.

In response to this:

Carry "Prefer local endpoint for cluster DNS service"
(54dc363) was incorrectly applied which did not restrict
DNS calls to a local endpoint.

Signed-off-by: Martin Kennelly mkennell@redhat.com

/cc @danwinship

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.