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

Backport of Disable DNS redirection when tproxy is disabled into release/1.1.x #2178

Conversation

hc-github-team-consul-core
Copy link
Collaborator

Backport

This PR is auto-generated from #2176 to be assessed for backporting due to the inclusion of the label backport/1.1.x.

The below text is copied from the body of the original PR.


Changes proposed in this PR:

  • DNS redirection and the various settings that make that possible (like
    the dataplane binding to a port for DNS) is only useful if tproxy is
    enabled. Most of the code checked if tproxy was enabled but there
    was one location where we didn't check. This resulted in a bug
    with our multiport support where even though tproxy is disabled,
    we tried to setup the dataplane to proxy DNS. This meant each dataplane
    tried to bind to 8600 but because there are >1 dataplanes with
    multiport, there was a port conflict.

    This PR fixes the location where we didn't check if tproxy was enabled
    and as a result fixes the multiport issue.

  • the reason this wasn't caught in our acceptance tests is because we don't run the multiport test when global tproxy is enabled and this bug only occurs when global tproxy is enabled but pod-level tproxy is disabled

  • Fixes consul-dataplane fails to bind to port 8600 in multiport connect-inject service mesh using local kind cluster #2144

How I've tested this PR:

  • built out a dataplane and manually deployed it with multiport: go test ./... -p 1 -timeout 20m -run TestConnectInject_MultiportServices -no-cleanup-on-failure -v -consul-image hashicorppreview/consul:1.16-dev -enable-transparent-proxy -consul-k8s-image ghcr.io/lkysow/consul-k8s-dev:mar25-23@sha256:fba790b616687308a2b1159da087eb52109d8acda239b1b69a1775bf12db9570

How I expect reviewers to test this PR:

  • code

Checklist:

  • Tests added
  • CHANGELOG entry added

    HashiCorp engineers only, community PRs should not add a changelog entry.
    Entries should use present tense (e.g. Add support for...)


Overview of commits

@hc-github-team-consul-core hc-github-team-consul-core force-pushed the backport/lkysow/dns-redirection-multiport/smoothly-mutual-panda branch from 667db40 to c64b40c Compare May 25, 2023 17:06
@hc-github-team-consul-core hc-github-team-consul-core enabled auto-merge (squash) May 25, 2023 17:06
@hc-github-team-consul-core hc-github-team-consul-core force-pushed the backport/lkysow/dns-redirection-multiport/smoothly-mutual-panda branch from b12e93b to af2eedd Compare May 25, 2023 17:06
@lkysow
Copy link
Member

lkysow commented May 26, 2023

Going to merge because the failing tests are unrelated.

@curtbushko curtbushko self-requested a review May 26, 2023 15:47
Copy link
Contributor

@curtbushko curtbushko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@hc-github-team-consul-core hc-github-team-consul-core merged commit 27ea1bd into release/1.1.x May 26, 2023
@hc-github-team-consul-core hc-github-team-consul-core deleted the backport/lkysow/dns-redirection-multiport/smoothly-mutual-panda branch May 26, 2023 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants