Skip to content

Fix lo being used for DHCP, try next on cmd fail#1208

Merged
TheRealFalcon merged 3 commits into
canonical:mainfrom
eb3095:vultr-nightly
Jan 28, 2022
Merged

Fix lo being used for DHCP, try next on cmd fail#1208
TheRealFalcon merged 3 commits into
canonical:mainfrom
eb3095:vultr-nightly

Conversation

@eb3095
Copy link
Copy Markdown
Contributor

@eb3095 eb3095 commented Jan 22, 2022

Proposed Commit Message

Fix lo being used for DHCP, try next on cmd fail

Arch didn't like the last fix and lo found its way into this list. Checking for this and skipping interfaces that generate subp failures.

Test Steps

Deployed Arch on Vultr

Checklist:

  • [ X ] My code follows the process laid out in the documentation
  • [ X ] I have updated or added any unit tests accordingly
  • [ X ] I have updated or added any documentation accordingly

Comment thread cloudinit/sources/helpers/vultr.py Outdated
# Skip dummy interfaces
if "dummy" in iface[0]:
# Skip dummy, lo interfaces
if iface[0] in ['lo', 'dummy']:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This check would now fail to filter out numbered dummy interfaces, e.g. "dummy0".

FWIW I've been considering taking the filtering done for net.find_fallback_nic() and providing a method so we can query the set of viable interfaces without the data sources having to add similar filtering logic for themselves if fallback_interface is not sufficient.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

@eb3095 eb3095 Jan 24, 2022

Choose a reason for hiding this comment

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

Ah indeed it would. Not sure why I didn't see that... Anyways its fixed, and it looks like find_candidate_nics is exactly what I need. Once that's merged in I will immediately refactor this to use that instead. Probably when I go through and simplify the generation of our metadata. I am looking at making a new endpoint on our end that feeds cloud-init format configs directly instead of doing what we are doing here to eliminate some of these inconsistencies we are seeing due to our metadata bumping heads with the cloud-init format here and there.

@eb3095
Copy link
Copy Markdown
Contributor Author

eb3095 commented Jan 27, 2022

Seems like travis is stuck here? Am I able to kick that from my end to get it going? I dont see an option.

@TheRealFalcon
Copy link
Copy Markdown
Contributor

Hey @eb3095 , it should work now if you push anything to the branch. If there's nothing new, you can force push a trivial change (i.e., an extra space) in the commit message.

Copy link
Copy Markdown
Contributor

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

Thanks!

@TheRealFalcon TheRealFalcon merged commit 6a4b6dc into canonical:main Jan 28, 2022
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