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.

Sign up for GitHub

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

Jump to bottom

Add support for in-container master for vlans #781

Merged
merged 1 commit into from
Feb 6, 2023
Merged

Add support for in-container master for vlans #781

merged 1 commit into from
Feb 6, 2023

Conversation

mmirecki
Copy link
Contributor

@mmirecki mmirecki commented Nov 15, 2022 •
edited
Loading

Add support for in-container master for the vlan plugin

docs: containernetworking/cni.dev#109

@mmirecki mmirecki force-pushed the vlan_incontainermaster branch 2 times, most recently from d47dc6e to 747e94d Compare November 15, 2022 12:34
@mmirecki
Copy link
Contributor Author

@dougbtv @dcbw @s1061123 Could you please take a look?

@mmirecki
Copy link
Contributor Author

/cc @dougbtv @s1061123

@mmirecki mmirecki mentioned this pull request Dec 2, 2022
Merged
@dougbtv
Copy link

dougbtv commented Dec 6, 2022

This seems fairly straightforward to me, at least the code changes. But, I'm also interested in what we'll provide for docs / explain the usage as well.

@mmirecki
Copy link
Contributor Author

mmirecki commented Dec 7, 2022

This seems fairly straightforward to me, at least the code changes. But, I'm also interested in what we'll provide for docs / explain the usage as well.

We have this doc PR for this: containernetworking/cni.dev#109

s1061123
s1061123 reviewed Dec 13, 2022
View reviewed changes
Copy link
Contributor

@s1061123 s1061123 left a comment

Choose a reason for hiding this comment

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

Almost good. Thank you for the PR.

BTW, is there any reason why you don't add tests for 'linkInContainer' option in vlan_test.go?

@mmirecki
Copy link
Contributor Author

Almost good. Thank you for the PR.

BTW, is there any reason why you don't add tests for 'linkInContainer' option in vlan_test.go?

Tests modified. Thanks for catching this.

@mmirecki mmirecki force-pushed the vlan_incontainermaster branch 3 times, most recently from 32207e6 to d0590b0 Compare December 16, 2022 13:46
@mmirecki
Copy link
Contributor Author

mmirecki commented Jan 5, 2023

@dcbw can you please take a look?

@mmirecki mmirecki force-pushed the vlan_incontainermaster branch 2 times, most recently from c8231b6 to f4aa77a Compare January 19, 2023 11:15
@mmirecki mmirecki mentioned this pull request Feb 1, 2023
Merged
@mccv1r0
Copy link
Member

mccv1r0 commented Feb 5, 2023

/lgtm
/approve

@dcbw dcbw merged commit 755714d into containernetworking:main Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@s1061123 s1061123 s1061123 left review comments

@dougbtv dougbtv dougbtv approved these changes

@squeed squeed squeed approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mmirecki @dougbtv @mccv1r0 @s1061123 @squeed @dcbw