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

azure: check for primary interface when performing DHCP #4465

Merged
merged 3 commits into from
Oct 23, 2023

Conversation

cjp256
Copy link
Contributor

@cjp256 cjp256 commented Sep 28, 2023

For Savable PPS we rely on a connectivity check to IMDS to determine if the NIC is primary. However, in some cases, connectivity may be delayed and we incorrectly assume the NIC is not primary.

Instead of relying on connectivity, check the DHCP-provided route configuration for the presence of Wireserver and/or IMDS IPs.

  • Return bool from _setup_ephemeral_networking() indicating primary NIC or not.

  • Remove _check_if_nic_is_primary() used in Savable PPS.

  • Add a relevant diagnostic to assist debugging failures when a secondary NIC is chosen.

  • Remove some tests that are redundant.

Copy link
Contributor

@anhvoms anhvoms left a comment

Choose a reason for hiding this comment

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

LGTM

For Savable PPS we rely on a connectivity check to IMDS to determine
if the NIC is primary.  However, in some cases, connectivity may be
delayed and we incorrectly assume the NIC is not primary.

Instead of relying on connectivity, check the DHCP-provided route
configuration for the presence of Wireserver and/or IMDS IPs.

- Return bool from _setup_ephemeral_networking() indicating primary
NIC or not.

- Remove _check_if_nic_is_primary() used in Savable PPS.

- Add a relevant diagnostic to assist debugging failures when a
secondary NIC is chosen.

- Remove some tests that are redundant.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
@cjp256 cjp256 force-pushed the azure-primary-check branch from 510a391 to 1011437 Compare September 28, 2023 18:51
@cjp256 cjp256 changed the title sources/azure: check for primary interface when performing DHCP azure: check for primary interface when performing DHCP Sep 29, 2023
@holmanb holmanb self-assigned this Sep 29, 2023
…ption 245, increase test coverage

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
@github-actions
Copy link

Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close.

If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging TheRealFalcon, and he will ensure that someone takes a look soon.

(If the pull request is closed and you would like to continue working on it, please do tag TheRealFalcon to reopen it.)

@github-actions github-actions bot added the stale-pr Pull request is stale; will be auto-closed soon label Oct 17, 2023
@anhvoms
Copy link
Contributor

anhvoms commented Oct 17, 2023

@holmanb can you review and merge? Thanks

@holmanb
Copy link
Member

holmanb commented Oct 17, 2023

@holmanb can you review and merge? Thanks

Ack, sorry for the delay on this one. EOD now, but will do this tomorrow.

@holmanb holmanb removed the stale-pr Pull request is stale; will be auto-closed soon label Oct 17, 2023
Copy link
Member

@holmanb holmanb left a comment

Choose a reason for hiding this comment

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

I would also assume this performs a bit better too than getting metadata?

I left one question inline, but then I'll merge.

Looks good to me, thanks!

cloudinit/sources/DataSourceAzure.py Outdated Show resolved Hide resolved
Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
@holmanb holmanb merged commit afc7584 into canonical:main Oct 23, 2023
holmanb pushed a commit to holmanb/cloud-init that referenced this pull request Oct 31, 2023
* azure: check for primary interface when performing DHCP

For Savable PPS we rely on a connectivity check to IMDS to determine
if the NIC is primary.  However, in some cases, connectivity may be
delayed and we incorrectly assume the NIC is not primary.

Instead of relying on connectivity, check the DHCP-provided route
configuration for the presence of Wireserver and/or IMDS IPs.

- Return bool from _setup_ephemeral_networking() indicating primary
NIC or not.

- Remove _check_if_nic_is_primary() used in Savable PPS.

- Add a relevant diagnostic to assist debugging failures when a
secondary NIC is chosen.

- Remove some tests that are redundant.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
blackboxsw pushed a commit that referenced this pull request Nov 2, 2023
* azure: check for primary interface when performing DHCP

For Savable PPS we rely on a connectivity check to IMDS to determine
if the NIC is primary.  However, in some cases, connectivity may be
delayed and we incorrectly assume the NIC is not primary.

Instead of relying on connectivity, check the DHCP-provided route
configuration for the presence of Wireserver and/or IMDS IPs.

- Return bool from _setup_ephemeral_networking() indicating primary
NIC or not.

- Remove _check_if_nic_is_primary() used in Savable PPS.

- Add a relevant diagnostic to assist debugging failures when a
secondary NIC is chosen.

- Remove some tests that are redundant.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
blackboxsw pushed a commit to blackboxsw/cloud-init that referenced this pull request Nov 6, 2023
* azure: check for primary interface when performing DHCP

For Savable PPS we rely on a connectivity check to IMDS to determine
if the NIC is primary.  However, in some cases, connectivity may be
delayed and we incorrectly assume the NIC is not primary.

Instead of relying on connectivity, check the DHCP-provided route
configuration for the presence of Wireserver and/or IMDS IPs.

- Return bool from _setup_ephemeral_networking() indicating primary
NIC or not.

- Remove _check_if_nic_is_primary() used in Savable PPS.

- Add a relevant diagnostic to assist debugging failures when a
secondary NIC is chosen.

- Remove some tests that are redundant.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
holmanb pushed a commit that referenced this pull request Nov 14, 2023
* azure: check for primary interface when performing DHCP

For Savable PPS we rely on a connectivity check to IMDS to determine
if the NIC is primary.  However, in some cases, connectivity may be
delayed and we incorrectly assume the NIC is not primary.

Instead of relying on connectivity, check the DHCP-provided route
configuration for the presence of Wireserver and/or IMDS IPs.

- Return bool from _setup_ephemeral_networking() indicating primary
NIC or not.

- Remove _check_if_nic_is_primary() used in Savable PPS.

- Add a relevant diagnostic to assist debugging failures when a
secondary NIC is chosen.

- Remove some tests that are redundant.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
holmanb pushed a commit that referenced this pull request Nov 14, 2023
* azure: check for primary interface when performing DHCP

For Savable PPS we rely on a connectivity check to IMDS to determine
if the NIC is primary.  However, in some cases, connectivity may be
delayed and we incorrectly assume the NIC is not primary.

Instead of relying on connectivity, check the DHCP-provided route
configuration for the presence of Wireserver and/or IMDS IPs.

- Return bool from _setup_ephemeral_networking() indicating primary
NIC or not.

- Remove _check_if_nic_is_primary() used in Savable PPS.

- Add a relevant diagnostic to assist debugging failures when a
secondary NIC is chosen.

- Remove some tests that are redundant.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
holmanb pushed a commit that referenced this pull request Nov 14, 2023
* azure: check for primary interface when performing DHCP

For Savable PPS we rely on a connectivity check to IMDS to determine
if the NIC is primary.  However, in some cases, connectivity may be
delayed and we incorrectly assume the NIC is not primary.

Instead of relying on connectivity, check the DHCP-provided route
configuration for the presence of Wireserver and/or IMDS IPs.

- Return bool from _setup_ephemeral_networking() indicating primary
NIC or not.

- Remove _check_if_nic_is_primary() used in Savable PPS.

- Add a relevant diagnostic to assist debugging failures when a
secondary NIC is chosen.

- Remove some tests that are redundant.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
holmanb pushed a commit that referenced this pull request Nov 14, 2023
* azure: check for primary interface when performing DHCP

For Savable PPS we rely on a connectivity check to IMDS to determine
if the NIC is primary.  However, in some cases, connectivity may be
delayed and we incorrectly assume the NIC is not primary.

Instead of relying on connectivity, check the DHCP-provided route
configuration for the presence of Wireserver and/or IMDS IPs.

- Return bool from _setup_ephemeral_networking() indicating primary
NIC or not.

- Remove _check_if_nic_is_primary() used in Savable PPS.

- Add a relevant diagnostic to assist debugging failures when a
secondary NIC is chosen.

- Remove some tests that are redundant.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
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