Skip to content

Commit

Permalink
fix(azure): disable use-dns for secondary nics
Browse files Browse the repository at this point in the history
DNS resolution through secondary NICs is not supported on Azure. Disable
it.

Without this, we see seconds of delay resolving urls in cloud-init logs
from Jammy+, see SF ticket.

Per cjp256's comment, the first NIC under metadata.imds.network is ensured
to be the primary one. We use this to determine primary NICs instead of
relying on fragile driver and/or NIC names.

Fixes: SF: #00380708

Co-authored-by: Calvin Mwadime <calvin.mwadime@canonical.com>
  • Loading branch information
CalvoM authored and aciba90 committed May 22, 2024
1 parent 23136e6 commit 7955e11
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 8 deletions.
3 changes: 3 additions & 0 deletions cloudinit/sources/DataSourceAzure.py
Original file line number Diff line number Diff line change
Expand Up @@ -1965,6 +1965,9 @@ def generate_network_config_from_instance_network_metadata(
# addresses.
nicname = "eth{idx}".format(idx=idx)
dhcp_override = {"route-metric": (idx + 1) * 100}
# DNS resolution through secondary NICs is not supported, disable it.
if idx > 0:
dhcp_override["use-dns"] = False
dev_config: Dict[str, Any] = {
"dhcp4": True,
"dhcp4-overrides": dhcp_override,
Expand Down
72 changes: 71 additions & 1 deletion tests/integration_tests/datasources/test_azure.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import datetime

import pytest
from pycloudlib.azure.util import AzureCreateParams, AzureParams
from pycloudlib.cloud import ImageType

from tests.integration_tests.clouds import IntegrationCloud
from tests.integration_tests.conftest import get_validated_source
from tests.integration_tests.instances import IntegrationInstance
from tests.integration_tests.integration_settings import PLATFORM
from tests.integration_tests.releases import CURRENT_RELEASE
from tests.integration_tests.releases import BIONIC, CURRENT_RELEASE


def _check_for_eject_errors(
Expand Down Expand Up @@ -45,3 +48,70 @@ def test_azure_eject(session_cloud: IntegrationCloud):
session_cloud.cloud_instance.delete_image(snapshot_id)
else:
_check_for_eject_errors(instance)


def parse_resolvectl_dns(output: str) -> dict:
"""Parses the output of 'resolvectl dns'.
>>> parse_resolvectl_dns(
... "Global:",
... "Link 2 (eth0): 168.63.129.16",
... "Link 3 (eth1): 168.63.129.16",
... )
{'Global': '',
'Link 2 (eth0)': '168.63.129.16',
'Link 3 (eth1)': '168.63.129.16'}
"""

parsed = dict()
for line in output.splitlines():
if line.isspace():
continue
splitted = line.split(":")
k = splitted.pop(0).strip()
v = splitted.pop(0).strip() if splitted else ""
parsed[k] = v
return parsed


@pytest.mark.skipif(PLATFORM != "azure", reason="Test is Azure specific")
@pytest.mark.skipif(
CURRENT_RELEASE < BIONIC, reason="Easier to test on Bionic+"
)
def test_azure_multi_nic_setup(
setup_image, session_cloud: IntegrationCloud
) -> None:
"""Integration test for https://warthogs.atlassian.net/browse/CPC-3999.
Azure should have the primary NIC only route to DNS.
Ensure other NICs do not have route to DNS.
"""
us = datetime.datetime.now().strftime("%f")
rg_params = AzureParams(f"ci-test-multi-nic-setup-{us}", None)
nic_one = AzureCreateParams(f"ci-nic1-test-{us}", rg_params.name, None)
nic_two = AzureCreateParams(f"ci-nic2-test-{us}", rg_params.name, None)
with session_cloud.launch(
launch_kwargs={
"resource_group_params": rg_params,
"network_interfaces_params": [nic_one, nic_two],
}
) as client:
_check_for_eject_errors(client)
if CURRENT_RELEASE.series == "bionic":
ret = client.execute("systemd-resolve --status")
assert ret.ok, ret.stderr
assert ret.stdout.count("Current Scopes: DNS") == 1
else:
ret = client.execute("resolvectl dns")
assert ret.ok, ret.stderr
routes = parse_resolvectl_dns(ret.stdout)
routes_devices = list(routes.keys())
eth1_dev = [dev for dev in routes_devices if "(eth1)" in dev][0]
assert not routes[eth1_dev], (
f"Expected eth1 to not have routes to dns."
f" Found: {routes[eth1_dev]}"
)

# check the instance can resolve something
res = client.execute("resolvectl query google.com")
assert res.ok, res.stderr
20 changes: 13 additions & 7 deletions tests/unittests/sources/test_azure.py
Original file line number Diff line number Diff line change
Expand Up @@ -742,14 +742,20 @@ class TestGenerateNetworkConfig:
"match": {"macaddress": "00:0d:3a:04:75:98"},
"dhcp6": False,
"dhcp4": True,
"dhcp4-overrides": {"route-metric": 200},
"dhcp4-overrides": {
"route-metric": 200,
"use-dns": False,
},
},
"eth2": {
"set-name": "eth2",
"match": {"macaddress": "00:0d:3a:04:75:98"},
"dhcp6": False,
"dhcp4": True,
"dhcp4-overrides": {"route-metric": 300},
"dhcp4-overrides": {
"route-metric": 300,
"use-dns": False,
},
},
},
"version": 2,
Expand Down Expand Up @@ -976,7 +982,7 @@ def test_single_ipv4_nic_configuration(
"dhcp6": False,
"match": {"macaddress": "00:0d:3a:04:75:98"},
"set-name": "eth0",
}
},
},
"version": 2,
}
Expand Down Expand Up @@ -1557,7 +1563,7 @@ def test_network_config_set_from_imds(self):
"dhcp6": False,
"dhcp4": True,
"dhcp4-overrides": {"route-metric": 100},
}
},
},
"version": 2,
}
Expand Down Expand Up @@ -1586,14 +1592,14 @@ def test_network_config_set_from_imds_route_metric_for_secondary_nic(self):
"match": {"macaddress": "22:0d:3a:04:75:98"},
"dhcp6": False,
"dhcp4": True,
"dhcp4-overrides": {"route-metric": 200},
"dhcp4-overrides": {"route-metric": 200, "use-dns": False},
},
"eth2": {
"set-name": "eth2",
"match": {"macaddress": "33:0d:3a:04:75:98"},
"dhcp6": False,
"dhcp4": True,
"dhcp4-overrides": {"route-metric": 300},
"dhcp4-overrides": {"route-metric": 300, "use-dns": False},
},
},
"version": 2,
Expand Down Expand Up @@ -1626,7 +1632,7 @@ def test_network_config_set_from_imds_for_secondary_nic_no_ip(self):
"dhcp6": False,
"dhcp4": True,
"dhcp4-overrides": {"route-metric": 100},
}
},
},
"version": 2,
}
Expand Down

0 comments on commit 7955e11

Please sign in to comment.