Skip to content

Fix extra ipv6 issues, code reduction and simplification#1243

Merged
TheRealFalcon merged 4 commits into
canonical:mainfrom
eb3095:vultr-nightly
Feb 14, 2022
Merged

Fix extra ipv6 issues, code reduction and simplification#1243
TheRealFalcon merged 4 commits into
canonical:mainfrom
eb3095:vultr-nightly

Conversation

@eb3095
Copy link
Copy Markdown
Contributor

@eb3095 eb3095 commented Feb 3, 2022

Proposed Commit Message

Fix extra ipv6 issues, code reduction and simplification

I eliminated the duplicate code and now run the entire configuration routine against both public and private interfaces. I also addressed an inconsistency from our metadata api for ipv6 address configuration.

Test Steps

Start a Vultr vm with additional IPv6's added to the instance.

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

@TheRealFalcon
Copy link
Copy Markdown
Contributor

@eb3095 , changes look good.

Would you be able to add a couple unit tests for this behavior? I think the last PR with the missing continue/return shows we could use some extra unit test coverage for this datasource.

@eb3095
Copy link
Copy Markdown
Contributor Author

eb3095 commented Feb 4, 2022

@eb3095 , changes look good.

Would you be able to add a couple unit tests for this behavior? I think the last PR with the missing continue/return shows we could use some extra unit test coverage for this datasource.

Ha, fair, indeed it could use a few more tests I think. I'll add that.

@eb3095
Copy link
Copy Markdown
Contributor Author

eb3095 commented Feb 6, 2022

Alright, I added tests for all the recent failures and this latest one.

Comment thread cloudinit/sources/helpers/vultr.py Outdated
@eb3095
Copy link
Copy Markdown
Contributor Author

eb3095 commented Feb 11, 2022

OK This should be good to go now!

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.

LGTM, thanks!

@TheRealFalcon TheRealFalcon merged commit 470b4a5 into canonical:main Feb 14, 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.

2 participants