Skip to content

feat: Update zk netplan render to handle docker bridge network interface #8

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

Conversation

sydefz
Copy link
Contributor

@sydefz sydefz commented Feb 27, 2024

Problem: We have noticed when docker has a network interface the zookeeper netplan render is confused and can't correctly pick the ENI ifname. For example with the following network interfaces, the zookeeper_user_data will pick up both br-146bd07d2fbf and eth1 then render into /etc/netplan/99_config.yaml, and the netplan is broken.

ip -j link show | jq .
[
  {
    "ifindex": 3,
    "ifname": "br-146bd07d2fbf",
    "flags": [
      "NO-CARRIER",
      "BROADCAST",
      "MULTICAST",
      "UP"
    ],
    "mtu": 1500,
    "qdisc": "noqueue",
    "operstate": "DOWN",
    "linkmode": "DEFAULT",
    "group": "default",
    "link_type": "ether",
    "address": "02:42:01:07:1e:1c",
    "broadcast": "ff:ff:ff:ff:ff:ff"
  },
  {
    "ifindex": 4,
    "ifname": "eth1",
    "flags": [
      "BROADCAST",
      "MULTICAST"
    ],
    "mtu": 1500,
    "qdisc": "noop",
    "operstate": "DOWN",
    "linkmode": "DEFAULT",
    "group": "default",
    "txqlen": 1000,
    "link_type": "ether",
    "address": "0e:f1:0b:65:44:d3",
    "broadcast": "ff:ff:ff:ff:ff:ff"
  }
]

Solution: We are adding in additional filtering when finding the INTFERFACE_NAME, so it filter out any docker bridge network interface, which start with "br-".

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@chn217
Copy link
Contributor

chn217 commented Feb 27, 2024

Hi Paul,

Thanks for the fix. Nice work. 👍

@chn217 chn217 self-assigned this Feb 27, 2024
@chn217 chn217 merged commit 15d9265 into aws-solutions:main Feb 27, 2024
@chn217 chn217 self-requested a review February 27, 2024 05:33
Copy link

@msalman-atl msalman-atl left a comment

Choose a reason for hiding this comment

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

Looks good

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